aspect-build / rules_js

High-performance Bazel rules for running Node.js tools and building JavaScript projects
https://docs.aspect.build/rules/aspect_rules_js
Apache License 2.0
310 stars 107 forks source link

[Bug]: [Bazel 7?] Fix/document incompatibility with `--experimental_inprocess_symlink_creation` (transitive resolution failures / sandbox escape) #1877

Closed adamscybot closed 2 months ago

adamscybot commented 3 months ago

What happened?

I have documented the very confusing effects of this problem just an hour or so ago on https://github.com/aspect-build/rules_js/issues/1546#issuecomment-2269842960. Very shortly after that, I identified this flag as the problem and a week of debugging comes to a close!

I am unsure if this is related to rules_js 2.x, or Bazel 7 and what mix of these might trigger this problem. Primarily because this is a part of a large simultaneous upgrade. But I can say it happens on rules_js 2.0.0-rc9 combined with Bazel 7.

I feel it is more likely related to a change in Bazel 7 and may apply to older versions of rules_js too. This change to the --experimental_inprocess_symlink_creation flags behaviour was made in Bazel 7.0. See the thread for more details. However, this is speculation. In the interest of caution, I have added a note on that thread about this issue as its being proposed to turn this flag on by default in Bazel 7.3.0.

In my case the problem manifests in the following situation, but it is doubtful to be limited to just this:

  1. --experimental_inprocess_symlink_creation is enabled.
  2. A PNPM workspace with an application package with depends on another internal workspace package.
  3. The internal workspace package has many runtime deps in its package.json. These are also declared on the data attribute of the npm_package.
  4. In the app package.json there is a workspace:* dependency to the shared package.
  5. In the app package, there are js_binary targets connected to js_library targets. Those js_library targets specify the :node_modules/@example/shared-pkg in their data attributes.

Due to --experimental_inprocess_symlink_creation, symlinks are created differently. This happens all the way down the dependency tree in node_modules and I've also seen sandbox escape back to the source files in certain configurations.

This can be seen by executing commands along the lines of (masked some info)

ls -la /private/var/tmp/_bazel_xxxxxxxx/c84d9fb447025e8f818c667552514689/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/xxx-workspace-root-parent-xxx/xxx-workspace-root-xxx/xxx-app-package-xxx/bin_/bin.runfiles/_main/xxx-workspace-root-parent-xxx/xxx-workspace-root-xxx/node_modules/.aspect_rules_js/xxx-my-shared-pkg-name-xxx@0.0.0/node_modules

With --experimental_inprocess_symlink_creation turned on, symlinks look like this (snippet + masked):

axios -> /private/var/tmp/_bazel_xxxxxxxx/c84d9fb447025e8f818c667552514689/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/ui-plane/xxx-workspace-root-xxx/node_modules/.aspect_rules_js/xxx-my-shared-pkg-name-xxx@0.0.0/node_modules/axios
body-parser -> /private/var/tmp/_bazel_xxxxxxxx/c84d9fb447025e8f818c667552514689/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/ui-plane/xxx-workspace-root-xxx/node_modules/.aspect_rules_js/xxx-my-shared-pkg-name-xxx@0.0.0/node_modules/body-parser
combined-stream -> /private/var/tmp/_bazel_xxxxxxxx/c84d9fb447025e8f818c667552514689/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/ui-plane/xxx-workspace-root-xxx/node_modules/.aspect_rules_js/xxx-my-shared-pkg-name-xxx@0.0.0/node_modules/combined-stream
connect-redis -> /private/var/tmp/_bazel_xxxxxxxx/c84d9fb447025e8f818c667552514689/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/ui-plane/xxx-workspace-root-xxx/node_modules/.aspect_rules_js/xxx-my-shared-pkg-name-xxx@0.0.0/node_modules/connect-redis
cookie-parser -> /private/var/tmp/_bazel_xxxxxxxx/c84d9fb447025e8f818c667552514689/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/ui-plane/xxx-workspace-root-xxx/node_modules/.aspect_rules_js/xxx-my-shared-pkg-name-xxx@0.0.0/node_modules/cookie-parser

With --experimental_inprocess_symlink_creation turned off, symlinks look like this (snippet):

axios -> ../../axios@1.6.5/node_modules/axios
body-parser -> ../../body-parser@1.19.0/node_modules/body-parser
combined-stream -> ../../combined-stream@1.0.8/node_modules/combined-stream
connect-redis -> ../../connect-redis@5.1.0/node_modules/connect-redis
cookie-parser -> ../../cookie-parser@1.4.6/node_modules/cookie-parser

The latter works great. The former does not, throwing lots of module not found errors. I beleive it is considered a sandbox escape by the node fs patches.

Version

Development (host) and target OS/architectures: MacOSX 14.5

Output of bazel --version: 7.2.1

Version of the Aspect rules, or other relevant rules from your WORKSPACE or MODULE.bazel file:

bazel_dep(name = "rules_nodejs", version = "6.2.0")

node = use_extension("@rules_nodejs//nodejs:extensions.bzl", "node", dev_dependency = True)
node.toolchain(node_version = "20.11.1")

bazel_dep(name = "aspect_bazel_lib", version = "2.7.8")
bazel_dep(name = "aspect_rules_js", version = "2.0.0-rc9")

Language(s) and/or frameworks involved: Typescript, Javascript, PNPM workspaces

How to reproduce

See above but more details and musing can be found at https://github.com/aspect-build/rules_js/issues/1546#issuecomment-2269842960.

Ensure build --experimental_inprocess_symlink_creation is in your .bazelrc.

Any other information?

I can happily no longer use this flag, which I am doing now. But it should be blocked/documented or fixed.

I also found it can be worked around (if you for some reason must keep this flag) by using patch_node_fs = False on js_binary but thats probably very dangerous, as like I said, I've actually seen symlinks at times linking out to the src tree.

Debugging the problem has been hard because once the symlinks are bad like this, the behaviour of future ops/changes whilst the flag remains on seems to be non-hermetic and you need to rm -rf some stuff in bazel-out before having another workaround attempt since doing so "changes the errors". I was trying crazy stuff like shoving the workspace node_modules into the js_binary's data attribute. But of course, I now know any "workaround" is futile.

Also I don't know enough about the matter at hand to say, but its quite possible this issue is with Bazel 7 and there is nothing to fix here. If so, I'm happy to move this there.

Side point, but this would of been so much easier for me to realise if this type of "module not found" errors coming from node were supplemented by a an additional message/log (in some kinda of debug mode?) to say they would have resolved if not for the fs patch and why they were rejected/where it expected stuff to be contained within. I'm guessing that the symlinks go to an absolute path and not a relative one is possibly a key invariant that could be stated. As without this realisation, the paths often look plausible.

Note this flag is actually mentioned by aspect as one to (possibly) enable:

https://docs.aspect.build/guides/bazelrc/#notes https://blog.aspect.build/bazelrc-flags

fmeum commented 3 months ago

This is a Bazel bug: https://github.com/bazelbuild/bazel/issues/23327

adamscybot commented 2 months ago

Since the Bazel folks (thanks @fmeum) picked this up and fixed it in Bazel 7.4, I'm going to close :)