-
-
Notifications
You must be signed in to change notification settings - Fork 158
fix: two rebound-global-constructor gaps hit by rolldown-bundled CJS #8574
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
Merged
proggeramlug
merged 5 commits into
PerryTS:main
from
jdalton:fix/socketlib-spawn-perry-compat
Aug 22, 2026
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
b7c7e6b
fix(hir): gate require-namespace binding fast path on const
jdalton ec92f86
fix(runtime): recognize RegExp's dedicated ctor thunk as rebound-global
jdalton 7781817
test: cover the unwrapped-CJS require-reassign and rebound-RegExp fixes
jdalton aa26c11
test: correct the require-reassign test's claim about what it isolates
jdalton 8200a7a
fix(runtime): add RegExp to the direct constructor-name mapping too
jdalton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| //! Regression test: `new` on a value that IS the global `RegExp` constructor, | ||
| //! reached through a local rebinding rather than the literal `RegExp` | ||
| //! identifier, must produce a real, correctly-branded regex instance. | ||
| //! | ||
| //! This is the shape of `@socketsecurity/lib`'s rolldown-bundled | ||
| //! `dist/primordials/regexp.js` (`const RegExpCtor = RegExp; exports | ||
| //! .RegExpCtor = RegExpCtor;`), consumed elsewhere as | ||
| //! `new _p_RegExpCtor(pattern)`. | ||
| //! | ||
| //! Pre-fix, `identify_global_builtin_constructor` never recognized RegExp's | ||
| //! own constructor thunk, so a rebound `RegExp` value fell through to the | ||
| //! generic empty-object construction path instead of the `"RegExp"` arm | ||
| //! already in `construct.rs`. The resulting object wasn't registered as a | ||
| //! real regex, so reading `.source`/`.flags` off it threw `TypeError: get | ||
| //! RegExp.prototype.source called on incompatible receiver`, and `.test()` | ||
| //! would have silently misbehaved rather than matching. | ||
|
|
||
| use std::path::PathBuf; | ||
| use std::process::Command; | ||
|
|
||
| fn perry_bin() -> PathBuf { | ||
| PathBuf::from(env!("CARGO_BIN_EXE_perry")) | ||
| } | ||
|
|
||
| fn compile_and_run(dir: &std::path::Path, source: &str) -> String { | ||
| let entry = dir.join("main.ts"); | ||
| let output = dir.join("main_bin"); | ||
| std::fs::write(&entry, source).expect("write entry"); | ||
|
|
||
| let compile = Command::new(perry_bin()) | ||
| .current_dir(dir) | ||
| .arg("compile") | ||
| .arg(&entry) | ||
| .arg("-o") | ||
| .arg(&output) | ||
| .output() | ||
| .expect("run perry compile"); | ||
| assert!( | ||
| compile.status.success(), | ||
| "perry compile failed\nstdout:\n{}\nstderr:\n{}", | ||
| String::from_utf8_lossy(&compile.stdout), | ||
| String::from_utf8_lossy(&compile.stderr) | ||
| ); | ||
|
|
||
| let run = Command::new(&output) | ||
| .current_dir(dir) | ||
| .output() | ||
| .expect("run compiled binary"); | ||
| assert!( | ||
| run.status.success(), | ||
| "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", | ||
| run.status, | ||
| String::from_utf8_lossy(&run.stdout), | ||
| String::from_utf8_lossy(&run.stderr) | ||
| ); | ||
| assert!( | ||
| run.stderr.is_empty(), | ||
| "compiled binary wrote to stderr\nstderr:\n{}", | ||
| String::from_utf8_lossy(&run.stderr) | ||
| ); | ||
| String::from_utf8_lossy(&run.stdout).into_owned() | ||
| } | ||
|
|
||
| #[test] | ||
| fn rebound_regexp_constructor_produces_real_regex() { | ||
| let dir = tempfile::tempdir().expect("tempdir"); | ||
| let stdout = compile_and_run( | ||
| dir.path(), | ||
| r#" | ||
| const RegExpCtor = RegExp; | ||
| const re = new RegExpCtor("^\\.[a-z]+$", "i"); | ||
| console.log("source:", re.source); | ||
| console.log("flags:", re.flags); | ||
| console.log("test-match:", re.test(".JS")); | ||
| console.log("test-nomatch:", re.test("nope")); | ||
| console.log("instanceof:", re instanceof RegExp); | ||
| "#, | ||
| ); | ||
| assert_eq!( | ||
| stdout, | ||
| "source: ^\\.[a-z]+$\nflags: i\ntest-match: true\ntest-nomatch: false\ninstanceof: true\n" | ||
| ); | ||
| } | ||
|
|
||
| /// The exact socket-lib shape: derive a second regex's source from an | ||
| /// already-rebound-constructed one (`rSlash.source` inside another `new | ||
| /// RegExpCtor(...)` call) — the concrete pattern from `@npmcli/promise-spawn`'s | ||
| /// bundled `dist` that surfaced this bug. | ||
| #[test] | ||
| fn rebound_regexp_source_composes_into_another_rebound_regexp() { | ||
| let dir = tempfile::tempdir().expect("tempdir"); | ||
| let stdout = compile_and_run( | ||
| dir.path(), | ||
| r#" | ||
| const RegExpCtor = RegExp; | ||
| const rSlash = new RegExpCtor("[/]"); | ||
| const rRel = new RegExpCtor(`^\\.${rSlash.source}`); | ||
| console.log("rSlash-source:", rSlash.source); | ||
| console.log("rRel-test:", rRel.test("./foo")); | ||
| "#, | ||
| ); | ||
| assert_eq!(stdout, "rSlash-source: [/]\nrRel-test: true\n"); | ||
| } |
148 changes: 148 additions & 0 deletions
148
crates/perry/tests/require_reassign_unwrapped_module.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| //! Contract test: a CJS dependency that does `let x = require("<native | ||
| //! module>"); x = someHelper.__toESM(x);` must keep a real runtime binding | ||
| //! for `x`, so the reassignment (and every later `x.default.*` read) resolves | ||
| //! instead of throwing `ReferenceError: x is not defined`. | ||
| //! | ||
| //! This is the exact shape of `@socketsecurity/lib`'s rolldown-bundled | ||
| //! `dist/bin/trusted.js` and `dist/process/spawn/child.js`, which crashed on | ||
| //! startup at the perry commit `@socketsecurity/lib` was pinned to | ||
| //! (`06137858d`, before this fix): `register_native_fetch_and_streams` | ||
| //! treated ANY `let/const/var x = require("<native>")` as an immutable | ||
| //! namespace binding and emitted no runtime variable for `x` at all — correct | ||
| //! for a `const` that's never reassigned, wrong here. | ||
| //! | ||
| //! `#8342`'s CJS-wrap shadow check (already on `main`) independently protects | ||
| //! this exact shape when the module goes through the `compilePackages` wrap, | ||
| //! which is why this specific minimized reproduction no longer reproduces the | ||
| //! crash in isolation on top of the current tree — the two fixes overlap for | ||
| //! the common case. This test locks down the observable contract (the | ||
| //! reassignment resolves correctly) regardless of which of the two checks is | ||
| //! providing it, since `register_native_fetch_and_streams`'s own gate is the | ||
| //! only protection for a module reached outside the CJS-wrap path. | ||
|
|
||
| use std::path::PathBuf; | ||
| use std::process::Command; | ||
|
|
||
| fn perry_bin() -> PathBuf { | ||
| PathBuf::from(env!("CARGO_BIN_EXE_perry")) | ||
| } | ||
|
|
||
| fn compile_and_run(dir: &std::path::Path, entry_name: &str) -> String { | ||
| let entry = dir.join(entry_name); | ||
| let output = dir.join("main_bin"); | ||
|
|
||
| let compile = Command::new(perry_bin()) | ||
| .current_dir(dir) | ||
| .arg("compile") | ||
| .arg(&entry) | ||
| .arg("-o") | ||
| .arg(&output) | ||
| .output() | ||
| .expect("run perry compile"); | ||
| assert!( | ||
| compile.status.success(), | ||
| "perry compile failed\nstdout:\n{}\nstderr:\n{}", | ||
| String::from_utf8_lossy(&compile.stdout), | ||
| String::from_utf8_lossy(&compile.stderr) | ||
| ); | ||
|
|
||
| let run = Command::new(&output) | ||
| .current_dir(dir) | ||
| .output() | ||
| .expect("run compiled binary"); | ||
| assert!( | ||
| run.status.success(), | ||
| "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", | ||
| run.status, | ||
| String::from_utf8_lossy(&run.stdout), | ||
| String::from_utf8_lossy(&run.stderr) | ||
| ); | ||
| assert!( | ||
| run.stderr.is_empty(), | ||
| "compiled binary wrote to stderr\nstderr:\n{}", | ||
| String::from_utf8_lossy(&run.stderr) | ||
| ); | ||
| String::from_utf8_lossy(&run.stdout).into_owned() | ||
| } | ||
|
|
||
| #[test] | ||
| fn unwrapped_cjs_package_reassigning_native_require_keeps_runtime_binding() { | ||
| let dir = tempfile::tempdir().expect("tempdir"); | ||
| let root = dir.path(); | ||
|
|
||
| std::fs::write( | ||
| root.join("package.json"), | ||
| r#"{ | ||
| "name": "require-reassign-unwrapped", | ||
| "type": "module", | ||
| "perry": { | ||
| "compilePackages": ["trusted-pkg"], | ||
| "allow": { "compilePackages": ["trusted-pkg"] } | ||
| } | ||
| }"#, | ||
| ) | ||
| .expect("write root package.json"); | ||
|
|
||
| let pkg = root.join("node_modules").join("trusted-pkg"); | ||
| std::fs::create_dir_all(&pkg).expect("mkdir trusted-pkg"); | ||
| std::fs::write( | ||
| pkg.join("package.json"), | ||
| r#"{ "name": "trusted-pkg", "version": "1.0.0", "main": "./trusted.js" }"#, | ||
| ) | ||
| .expect("write trusted-pkg package.json"); | ||
|
|
||
| // Mirrors socket-lib's `dist/_virtual/_rolldown/runtime.js`: a plain CJS | ||
| // helper, required by relative path. | ||
| std::fs::write( | ||
| pkg.join("runtime.js"), | ||
| r#""use strict"; | ||
| exports.__toESM = function (mod) { | ||
| if (mod && typeof mod === "object" && mod.__esModule) return mod; | ||
| var target = {}; | ||
| Object.defineProperty(target, "default", { value: mod, enumerable: true }); | ||
| return target; | ||
| }; | ||
| "#, | ||
| ) | ||
| .expect("write runtime.js"); | ||
|
|
||
| // Mirrors socket-lib's `dist/bin/trusted.js` exactly: the | ||
| // `Symbol.toStringTag` ESM-interop marker (not the wrap-triggering | ||
| // `__esModule` string key), a relative require for the local helper, then | ||
| // the `let x = require(...); x = helper.__toESM(x);` reassignment this | ||
| // regression is about. | ||
| std::fs::write( | ||
| pkg.join("trusted.js"), | ||
| r#""use strict"; | ||
| Object.defineProperty(exports, Symbol.toStringTag, { value: "Module" }); | ||
| const require_runtime = require("./runtime.js"); | ||
| let node_process = require("node:process"); | ||
| node_process = require_runtime.__toESM(node_process); | ||
|
|
||
| function currentPlatform() { | ||
| return node_process.default.platform; | ||
| } | ||
| exports.currentPlatform = currentPlatform; | ||
| "#, | ||
| ) | ||
| .expect("write trusted.js"); | ||
|
|
||
| std::fs::write( | ||
| root.join("main.ts"), | ||
| r#" | ||
| import { currentPlatform } from "trusted-pkg"; | ||
| console.log("platform:", currentPlatform()); | ||
| "#, | ||
| ) | ||
| .expect("write entry"); | ||
|
|
||
| let stdout = compile_and_run(root, "main.ts"); | ||
| let platform = std::env::consts::OS; | ||
| let expected_platform = match platform { | ||
| "macos" => "darwin", | ||
| "linux" => "linux", | ||
| "windows" => "win32", | ||
| _ => platform, | ||
| }; | ||
| assert_eq!(stdout, format!("platform: {expected_platform}\n")); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.