Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
adamscybot opened this issue Aug 6, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@adamscybot
Copy link

adamscybot commented Aug 6, 2024

What happened?

I have documented the very confusing effects of this problem just an hour or so ago on #1546 (comment). 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 #1546 (comment).

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

@adamscybot adamscybot added the bug Something isn't working label Aug 6, 2024
@adamscybot adamscybot changed the title [Bug]: [Bazel 7?] [rules_nodejs 2.x?] Fix or document incompatibility with --experimental_inprocess_symlink_creation [Bug]: [Bazel 7?] [rules_nodejs 2.x?] Fix/document incompatibility with --experimental_inprocess_symlink_creation (causes transitive resolution failures) Aug 6, 2024
@adamscybot adamscybot changed the title [Bug]: [Bazel 7?] [rules_nodejs 2.x?] Fix/document incompatibility with --experimental_inprocess_symlink_creation (causes transitive resolution failures) [Bug]: [Bazel 7?] Fix/document incompatibility with --experimental_inprocess_symlink_creation (causes transitive resolution failures) Aug 6, 2024
@adamscybot adamscybot changed the title [Bug]: [Bazel 7?] Fix/document incompatibility with --experimental_inprocess_symlink_creation (causes transitive resolution failures) [Bug]: [Bazel 7?] Fix/document incompatibility with --experimental_inprocess_symlink_creation (transitive resolution failures / sandbox escape) Aug 6, 2024
@fmeum
Copy link
Contributor

fmeum commented Aug 18, 2024

This is a Bazel bug: bazelbuild/bazel#23327

@adamscybot
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants