fix: native FS sandbox for ESM imports (#362)#2718
fix: native FS sandbox for ESM imports (#362)#2718Mivr wants to merge 6 commits intoaspect-build:mainfrom
Conversation
…dbox (aspect-build#362) Node.js ESM resolver captures `realpathSync.native()` via destructuring before `--require` patches can intercept it, causing resolved paths to escape the Bazel sandbox. This adds a native C shared library loaded via LD_PRELOAD (Linux) / DYLD_INSERT_LIBRARIES (macOS) that intercepts libc `realpath()` at the C level — before Node.js even starts — stopping symlink resolution at the first hop that would escape the sandbox roots. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
- Add load() statements for cc_library/cc_binary/cc_test from @rules_cc (required for Bazel 9+ where native cc rules are removed) - Add rules_cc as a dependency in MODULE.bazel - Add "//conditions:default" to select() in fs_patch_native alias with a noop genrule placeholder (fixes Windows where neither linux nor macos condition matches) - Add cfg = "exec" to _fs_patch_native attr (fixes cross-compilation builds like transition_js_image_layer_linux_arm64 that need a CC toolchain for the target platform) - Remove DYLD_INSERT_LIBRARIES on macOS (arm64/arm64e architecture incompatibility on macOS 15+ causes dyld abort; macOS relies on JS-level patches via register.cjs) - Run buildifier for canonical attribute ordering - Update image layer golden files (fs_patch_linux.so now in runfiles) - Update launcher golden file Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of using cfg="exec" to work around missing CC toolchains during cross-compilation (e.g. transition_js_image_layer_linux_arm64), restrict the native FS patch to Linux x86_64 via config_setting. All other platforms get a noop placeholder, so no CC toolchain is needed. The rule detects the noop by checking the file extension (.so/.dylib) and skips setting LD_PRELOAD and adding to launcher_files when the platform is unsupported. Native FS patching is not supported on ARM or macOS for now: - macOS: DYLD_INSERT_LIBRARIES requires arm64e on macOS 15+ - ARM: cross-compilation requires CC toolchain for target platform Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The .so binary size varies across compiler versions (17008 locally vs 17016 on CI). Extend the existing sanitize_cmd sed regex to replace 17XXX with xxxxx, space-anchored to avoid matching substrings of larger numbers (e.g. 217950 in acorn.js). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90abd22b53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (resolved_path) { | ||
| memcpy(resolved_path, normed, strlen(normed) + 1); | ||
| return resolved_path; | ||
| } else { | ||
| return strdup(normed); |
There was a problem hiding this comment.
In the orig_lstat(next) != 0 branch, this code returns a synthesized normalized path when there is remaining tail (rest) instead of propagating the lookup failure. That makes calls like realpath("/sandbox/missing/child") succeed with "/sandbox/missing/child" rather than returning NULL/ENOENT, which changes libc/Node realpath semantics and can cause callers (including resolution logic) to treat non-existent paths as resolved. This branch should preserve the error from the failed component lookup rather than returning a fabricated success value.
Useful? React with 👍 / 👎.
Previously, when lstat failed on a path component with remaining components, guarded_realpath returned a synthesized normalized path instead of propagating ENOENT. This diverged from libc realpath() semantics. Now always returns NULL with ENOENT when any component doesn't exist. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Enable DYLD_INSERT_LIBRARIES-based interposition on macOS to prevent ESM imports, esbuild, Vite, and Vitest from escaping the Bazel sandbox. Key changes: - Add lstat interposition to fs_patch_macos.c (Tier 2) using fstatat to avoid DYLD interpose recursion where dlsym(RTLD_NEXT) returns the interposed function - Add macOS arm64/x86_64 config settings to BUILD.bazel so the .dylib is selected instead of the noop placeholder - Work around macOS SIP stripping DYLD_* env vars through /bin/bash by passing the dylib path via JS_BINARY__NATIVE_PATCH_PATH and having node_wrapper.sh restore DYLD_INSERT_LIBRARIES before exec'ing node Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
fs_patch_linux.so/fs_patch_macos.dylib) loaded viaLD_PRELOAD/DYLD_INSERT_LIBRARIESthat intercepts libcrealpath()to prevent ESM imports from escaping the Bazel sandboxguarded_realpathalgorithm resolves paths component-by-component, stopping at the first symlink hop that would escape the configured sandbox roots (execroot + runfiles)js_binary/js_testvia the launcher template — automatically active whenpatch_node_fs = True(the default)Problem
Issue #362: Node.js ESM resolver captures
realpathSync.native()via destructuring before--requirepatches run:This means the JS-level fs patches applied by
register.cjsnever intercept the ESM resolver'srealpathcalls, causing.mjsentry points and dynamicimport()to resolve through Bazel sandbox symlinks to the real source tree.Solution
Intercept
realpath()at the C level viaLD_PRELOAD— this runs before Node.js even starts, so the ESM resolver's captured reference is already patched.Files
fs_patch.hfs_patch_init.c__attribute__((constructor))— env var parsing, dlsymfs_patch_common.cguarded_realpath,is_sub_path,check_escape,normalize_pathfs_patch_linux.crealpath,__realpath_chk,canonicalize_file_namefs_patch_macos.c__DATA,__interposejs_binary.bzl_fs_patch_nativeattrjs_binary.sh.tplLD_PRELOAD/DYLD_INSERT_LIBRARIESsetupAlgorithm
realpath(), check if the result escapes any sandbox root → if not, return itlstat+readlink, stop at the first symlink hop that escapes → return the guarded (non-escaped) pathTest plan
bazel test //js/private/fs_patches_native/test:alle2e/esm_sandbox/—.mjsentry point verifyingrealpathSync.native()stays within sandbox roots//js/...tests pass with zero regressions🤖 Generated with Claude Code