esbuild leaves the sandbox

[gregmagolan]

Since we turn preserveSymlinks off so that node_modules resolution works with the rules_js symlinks node_modules tree, esbuild will currently leave the sandbox by following symlinks. The rules_js node fs patches don't apply to the golang binary. There is currently no hook in esbuild where we can customize module resolution so that esbuild stays in the bazel sandbox.

If bundling .js files that come from transpiled sources, it is currently recommended to set the following configuration

config = {
        "resolveExtensions": [".js"],
    },

so that esbuild doesn't pick up and re-transpile the .ts sources it can find outside of the sandbox and seems to prefer (https://github.com/aspect-build/rules_esbuild/pull/57#issuecomment-1209540383).

[matthewjh]

@gregmagolan do you guys have a plan for how to deal with this? I remember you mentioning experimental_allow_unresolved_symlinks, but I'm not familiar with that and wonder what the long-term solution looks like.

Having some pretty major issues with the esbuild rules as-is, due to the sandbox escape. One is that, on developer machines, if an NPM package is missing from the inputs, it will probably be resolved from the PNPM-created node_modules folder in the workspace created by the developer running pnpm install. You only need one of these to pull in a bunch of transitive modules from the pnpm install rather than the rules_js-managed ones, and then end up with a load of weird bugs due to duplicated modules and different ctor refs breaking instanceOf checks. Thankfully, this should not be an issue on CI. There might be a simple config or even a lightweight plugin we can use for ESBuild to block this while the longer-term solution unfolds.

[gregmagolan]

It's a hard one to fix. We'll probably have to make changes to esbuild itself to expose the hooks needed to prevent it from following symlinks out of the sandbox. experimental_allow_unresolved_symlinks won't affect esbuild's behaviour. Adding the hook to esbuild is the plan for now but its not high on our priority list at the moment since its non-trivial and we don't have funding for it.

[alexeagle]

I wonder if we should file a bug on esbuild asking for what we want. Unlikely, but never hurts to ask?

[gonzojive]

I wonder if this is the same issue causing building //lib in this repo to succeed when //location:location_ts_proto is built first but fails if //location:location_ts_proto hasn't yet been built.

esbuild(
    name = "lib",
    entry_point = "program.mjs",
    deps = [
        #"//location:location_ts_proto", # a real dependency of program.mjs that we leave out, and yet still esbuild succeeds.
        "//:node_modules/google-protobuf",
    ],
)

[gonzojive]

Turning on preserve_symlinks does seem to resolve the issue for me. It is unclear to me precisely why preserving symlinks cannot be enabled. It seems like every tool like esbuild will need to be patched if preserve_symlinks is set to false when the tool is invoked.

In any case, I tried to modify the launcher.js with an onResolved plugin based on https://github.com/aspect-build/rules_esbuild/pull/32. I don't think the plugin hook has enough information to fix the problem. The resolved import path and set of files used to perform the resolution is not passed to the plugin, so it seems the plugin would have to implement its own import resolution (traversing node_modules, reasoning about file extensions, etc.).

esbuild seems to use its own file system library as sort of a virtual file system. It seems that could be modified to hide non-sandbox files from esbuild's resolver. https://pkg.go.dev/github.com/evanw/esbuild@v0.17.0/internal/fs Maybe a simple hook like isVisible func(path) bool would be sufficient for a Go plugin.

It seems the desired behavior (when preserveSymlinks=off) is to have a file system that consists of

1. every sandbox file, directory, and symlink. I think this is $BAZEL_BINDIR/**
2. every file that each of the symlinks from the sandbox resolves to

... naively, I would expect bazel to provide this functionality as part of the sandbox it sets up. However, it seems the sandbox environment allows reading files from any path on disk.

---

FYI, here some debug output from the onResolve plugin. You can see how the final resolved path isn't passed to the plugin.

resolved args {
  path: './program.mjs',
  importer: '',
  namespace: 'file',
  resolveDir: '/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/sandbox/linux-sandbox/1867/execroot/__main__/bazel-out/k8-fastbuild/bin',
  kind: 'entry-point',
  pluginData: undefined
}; sandbox files = [{"path":"lib.args.json","isSymbolicLink":false,"realPath":"lib.args.json","realPathResolved":"/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/sandbox/linux-sandbox/1867/execroot/__main__/bazel-out/k8-fastbuild/bin/lib.args.json"},{"path":"lib_sandbox.txt","isSymbolicLink":false,"realPath":"lib_sandbox.txt","realPathResolved":"/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/sandbox/linux-sandbox/1867/execroot/__main__/bazel-out/k8-fastbuild/bin/lib_sandbox.txt"},{"path":"program.mjs","isSymbolicLink":false,"realPath":"program.mjs","realPathResolved":"/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/sandbox/linux-sandbox/1867/execroot/__main__/bazel-out/k8-fastbuild/bin/program.mjs"}]
resolved args {
  path: './location/location_pb.mjs',
  importer: '/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/execroot/__main__/bazel-out/k8-fastbuild/bin/program.mjs',
  namespace: 'file',
  resolveDir: '/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/execroot/__main__/bazel-out/k8-fastbuild/bin',
  kind: 'import-statement',
  pluginData: undefined
}; sandbox files = [{"path":"lib.args.json","isSymbolicLink":false,"realPath":"lib.args.json","realPathResolved":"/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/sandbox/linux-sandbox/1867/execroot/__main__/bazel-out/k8-fastbuild/bin/lib.args.json"},{"path":"lib_sandbox.txt","isSymbolicLink":false,"realPath":"lib_sandbox.txt","realPathResolved":"/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/sandbox/linux-sandbox/1867/execroot/__main__/bazel-out/k8-fastbuild/bin/lib_sandbox.txt"},{"path":"program.mjs","isSymbolicLink":false,"realPath":"program.mjs","realPathResolved":"/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/sandbox/linux-sandbox/1867/execroot/__main__/bazel-out/k8-fastbuild/bin/program.mjs"}]
resolved args {
  path: 'google-protobuf',
  importer: '/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/execroot/__main__/bazel-out/k8-fastbuild/bin/location/location_pb.mjs',
  namespace: 'file',
  resolveDir: '/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/execroot/__main__/bazel-out/k8-fastbuild/bin/location',
  kind: 'import-statement',
  pluginData: undefined
}; sandbox files = [{"path":"lib.args.json","isSymbolicLink":false,"realPath":"lib.args.json","realPathResolved":"/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/sandbox/linux-sandbox/1867/execroot/__main__/bazel-out/k8-fastbuild/bin/lib.args.json"},{"path":"lib_sandbox.txt","isSymbolicLink":false,"realPath":"lib_sandbox.txt","realPathResolved":"/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/sandbox/linux-sandbox/1867/execroot/__main__/bazel-out/k8-fastbuild/bin/lib_sandbox.txt"},{"path":"program.mjs","isSymbolicLink":false,"realPath":"program.mjs","realPathResolved":"/home/red/.cache/bazel/_bazel_red/9f975204b5973dd8a05172c6f645e273/sandbox/linux-sandbox/1867/execroot/__main__/bazel-out/k8-fastbuild/bin/program.mjs"}]

[gonzojive]

On second thought to the above comment, it seems the "onLoad" plugin callback might have enough information to catch sandbox escapes. https://esbuild.github.io/plugins/#on-load-arguments

[alexeagle]

I think adding a capability to esbuild makes sense, if the maintainers accept it. That's how we get other tools to behave how we expect under Bazel.

I imagine another answer is just a more hermetic sandbox. You could try the docker strategy for all esbuild actions for example, or remote if you have access to a RBE cluster. Maybe we should try that and add to the docs.

[gonzojive]

I think https://github.com/aspect-build/rules_esbuild/pull/112 fixes this to some extent. I'm guessing there are some edge cases that it doesn't handle, like if esbuild loads a package.json file that's not in the sandbox even though the .js files for the package.json are.

[vpanta]

Just an addition here, I was able to make a plugin with onResolve which uses https://www.npmjs.com/package/resolve and https://www.npmjs.com/package/resolve.exports to remain in the sandbox. The downside is that this resolver will replace any other resolver used (only one can take affect), so all onResolve actions have to occur within it.

[Aghassi]

Just an addition here, I was able to make a plugin with onResolve which uses https://www.npmjs.com/package/resolve and https://www.npmjs.com/package/resolve.exports to remain in the sandbox. The downside is that this resolver will replace any other resolver used (only one can take affect), so all onResolve actions have to occur within it.

Do you have any examples of code you could share publicly? This would be great to have!

[vpanta]

Just threw up https://github.com/aspect-build/rules_esbuild/pull/160 in case it's something that can be used to help.

@Aghassi the above PR contains what we ended up doing, depending on esbuild's own resolver (much simpler than doing our own resolution). Sorry for the delay, it's been a busy quarter.

[alexeagle]

THere's a flag --experimental_use_hermetic_linux_sandbox which we should try here, maybe it neatly solves this already?

[gregmagolan]

Fix for this (from #160) is included in the v0.17.0 release