-
-
Notifications
You must be signed in to change notification settings - Fork 116
[Bug]: [Bazel 7?] Fix/document incompatibility with --experimental_inprocess_symlink_creation
(transitive resolution failures / sandbox escape)
#1877
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
Labels
bug
Something isn't working
Comments
This was referenced Aug 6, 2024
--experimental_inprocess_symlink_creation
--experimental_inprocess_symlink_creation
(causes transitive resolution failures)
--experimental_inprocess_symlink_creation
(causes transitive resolution failures)--experimental_inprocess_symlink_creation
(causes transitive resolution failures)
--experimental_inprocess_symlink_creation
(causes transitive resolution failures)--experimental_inprocess_symlink_creation
(transitive resolution failures / sandbox escape)
This is a Bazel bug: bazelbuild/bazel#23327 |
Since the Bazel folks (thanks @fmeum) picked this up and fixed it in Bazel 7.4, I'm going to close :) |
devversion
added a commit
to devversion/angular-cli
that referenced
this issue
Apr 16, 2025
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want).
devversion
added a commit
to devversion/angular-cli
that referenced
this issue
Apr 16, 2025
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want).
devversion
added a commit
to devversion/angular-cli
that referenced
this issue
Apr 16, 2025
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want).
devversion
added a commit
to devversion/angular-cli
that referenced
this issue
Apr 16, 2025
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want).
devversion
added a commit
to devversion/angular-cli
that referenced
this issue
Apr 16, 2025
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want).
devversion
added a commit
to devversion/angular-cli
that referenced
this issue
Apr 29, 2025
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want).
devversion
added a commit
to devversion/angular-cli
that referenced
this issue
Apr 29, 2025
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want).
devversion
added a commit
to devversion/angular-cli
that referenced
this issue
Apr 29, 2025
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want).
devversion
added a commit
to angular/angular-cli
that referenced
this issue
Apr 30, 2025
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want).
devversion
added a commit
to angular/angular-cli
that referenced
this issue
Apr 30, 2025
After throrough investigation and lots of time spent, I figured out why the `fs` patch didn't resolve paths within the `.runfiles` directory. The .runfiles directory of e.g. a js_binary (with bazel run) does have symlinks for e.g. node_modules/verdaccio point to the execroot directly via an absolute symlink. This is weird because in other projects there are relative symlinks to the store (this is important for the sandbox escaping FS patches)), as otherwise the dereferencing is blocked/prevented and we'd never resolve into a store directory— preventing transitive deps from being properly resolved. After figuring this out, I've tracked down the Bazel issue and found a fix: bazelbuild/bazel@51d7a3c In addition, there seems a related `rules_js` issue that has the same investigation results: aspect-build/rules_js#1877 We should try this out, but keep an eye on Firefox and Chromium. Those browser artifacts might contain files with spaces. This is incompatible with the Bazel version we are using, so the inprocess runfile creation was a workaround for this— If we remove that flag, we might cause issues for Mac users. We have a few options I think: 1. drop the flag, and hope it doesn't break any runfile generation for browsers. I suspect it won't work...— alternatively try renaming browser files. 2. keep the flag for now, and temporarily keep the disabled FS patches (needs to be manually specified by users) 3. do solution (2) but once migration allows it, update to Bazel 7— so that the bug with inprocess resolution is nicely fixed. I think (3) is a good plan moving forward and we don't need any runfile tricks. In Bazel 8, the runfiles with space issue is also resolved— allowing us to drop the inprocess runfile creation as well (if we want). (cherry picked from commit b5b6737)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
--experimental_inprocess_symlink_creation
is enabled.package.json
. These are also declared on thedata
attribute of thenpm_package
.package.json
there is aworkspace:*
dependency to the shared package.js_binary
targets connected tojs_library
targets. Thosejs_library
targets specify the:node_modules/@example/shared-pkg
in theirdata
attributes.Due to
--experimental_inprocess_symlink_creation
, symlinks are created differently. This happens all the way down the dependency tree innode_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)
With
--experimental_inprocess_symlink_creation
turned on, symlinks look like this (snippet + masked):With
--experimental_inprocess_symlink_creation
turned off, symlinks look like this (snippet):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.1Version of the Aspect rules, or other relevant rules from your
WORKSPACE
orMODULE.bazel
file: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
onjs_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 inbazel-out
before having another workaround attempt since doing so "changes the errors". I was trying crazy stuff like shoving the workspacenode_modules
into thejs_binary
'sdata
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
The text was updated successfully, but these errors were encountered: