aspect-build / aspect-build/rules_js
Imports escape sandbox when using --inspect-brk
- Dominant language
- Starlark
- Stars
- 378
- Forks
- 183
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 32
Description
When using CJS modules and `--inspect-brk` , `--preserve-symlinks-main` is not respected. I opened https://github.com/nodejs/node/issues/44856 on the node project for the underlying issue.
As a result, the basis for module resolutions is taken to be the "realpath" of the entrypoint symlink, rather than the runfiles path. Because these paths fall outside of the fs patch roots, `isSubPath(root, linkPath)` returns false in the code below, meaning `isEscape` returns false in all the fs patch methods:
https://github.com/aspect-build/rules_js/blob/main/js/private/node-patches/fs.js#L751
A workaround for this seems particularly important, as having different behaviour when debugging is (for obvious reasons) not helpful. Debugging failed module resolutions becomes almost impossible if everything breaks out when using the debugger.
I wonder whether this could be detected and corrected inside the fs patch. Don't we have the runfiles (symlink) path of the entrypoint in `process.argv[1]`? In the patches, can't we check `realpath(process.argv[1]) === thisPath` and use `process.argv[1]` instead, so that the basis for module resolutions outwards from the entry point use the symlink rather than the real path?
I don't know why rules_nodejs didn't suffer from this, but maybe that is also a path to solution.
Contributor guide
Research direction
Start at js/private/node-patches/fs.js around line 751 and reproduce the CJS import behavior with --inspect-brk and --preserve-symlinks-main. Compare the entrypoint's real path with process.argv[1] and verify that debugging preserves the runfiles-based module resolution and no longer causes imports to escape the fs patch roots.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- build-system, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100