From b7c7e6b2abded5cd08428852ffdf97061d4c0a07 Mon Sep 17 00:00:00 2001 From: John-David Dalton Date: Fri, 21 Aug 2026 22:42:17 -0400 Subject: [PATCH 1/5] fix(hir): gate require-namespace binding fast path on const register_native_fetch_and_streams optimized a native require() into a compile-time-only namespace binding with no runtime variable, but never checked the declaration was const. A let/var declarator that reassigns afterward - like rolldown's own ESM-interop preamble (let x = require(...); x = __toESM(x)) - had nothing to assign to, throwing ReferenceError at runtime. Gate the fast path on the existing mutable flag, already threaded through the caller. --- .../perry-hir/src/destructuring/var_decl.rs | 2 +- .../destructuring/var_decl/native_fetch.rs | 38 ++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/crates/perry-hir/src/destructuring/var_decl.rs b/crates/perry-hir/src/destructuring/var_decl.rs index 3edcfc0db7..0590e746df 100644 --- a/crates/perry-hir/src/destructuring/var_decl.rs +++ b/crates/perry-hir/src/destructuring/var_decl.rs @@ -54,7 +54,7 @@ pub(crate) fn lower_var_decl_with_destructuring( // native-instance registration (extracted to `native_fetch`). // Returns true when nothing observable is bound (the `require` // of a resolvable native module). - if register_native_fetch_and_streams(ctx, decl, &name, &mut ty) { + if register_native_fetch_and_streams(ctx, decl, &name, &mut ty, mutable) { return Ok(result); } diff --git a/crates/perry-hir/src/destructuring/var_decl/native_fetch.rs b/crates/perry-hir/src/destructuring/var_decl/native_fetch.rs index d357c1412b..8a445b4a3f 100644 --- a/crates/perry-hir/src/destructuring/var_decl/native_fetch.rs +++ b/crates/perry-hir/src/destructuring/var_decl/native_fetch.rs @@ -22,6 +22,7 @@ pub(crate) fn register_native_fetch_and_streams( decl: &ast::VarDeclarator, name: &str, ty: &mut Type, + mutable: bool, ) -> bool { // #5216: `const = require("")` of a statically // resolvable native/Node-builtin module lowers to the same @@ -53,19 +54,30 @@ pub(crate) fn register_native_fetch_and_streams( } } - if let Some(init_expr) = &decl.init { - // #8342: inside a CJS-wrapped module the wrap's synthetic - // `function require(...)` (with a `createRequire`-backed built-in arm) - // shadows the bare global `require`. Don't steal `let x = - // require("process")` into a native-module namespace binding here — the - // native namespace isn't initialized in a CJS-wrapped module, so `x` - // would be undefined at runtime (`ReferenceError: node_process is not - // defined`). Let the call flow through to the synthetic require, which - // resolves builtins via `createRequire` (see `cjs_wrap::wrap`). - if !require_is_shadowed_by_local(ctx) { - if let Some(module_name) = require_resolvable_native_specifier(init_expr) { - register_require_namespace_binding(ctx, name, &module_name); - return true; + // Gated on `!mutable` (a `const`, never `let`/`var`): a real namespace + // import is immutable, and this optimization emits no runtime binding at + // all, so a later reassignment of `name` would reference a slot that was + // never created. Rolldown's own ESM-interop preamble does exactly that — + // `let node_process = require("node:process"); node_process = + // __toESM(node_process);` — to wrap the require result in a `.default` + // shape. Applying the optimization there dropped the runtime `let` + // entirely, so the reassignment (and every later `node_process.default.*` + // read) threw `ReferenceError: node_process is not defined`. + if !mutable { + if let Some(init_expr) = &decl.init { + // #8342: inside a CJS-wrapped module the wrap's synthetic + // `function require(...)` (with a `createRequire`-backed built-in arm) + // shadows the bare global `require`. Don't steal `let x = + // require("process")` into a native-module namespace binding here — the + // native namespace isn't initialized in a CJS-wrapped module, so `x` + // would be undefined at runtime (`ReferenceError: node_process is not + // defined`). Let the call flow through to the synthetic require, which + // resolves builtins via `createRequire` (see `cjs_wrap::wrap`). + if !require_is_shadowed_by_local(ctx) { + if let Some(module_name) = require_resolvable_native_specifier(init_expr) { + register_require_namespace_binding(ctx, name, &module_name); + return true; + } } } } From ec92f86cfab6307950fb4914f5bfb291d6d1d416 Mon Sep 17 00:00:00 2001 From: John-David Dalton Date: Fri, 21 Aug 2026 23:10:57 -0400 Subject: [PATCH 2/5] fix(runtime): recognize RegExp's dedicated ctor thunk as rebound-global identify_global_builtin_constructor() drives new (rebound global ctor)(...) dispatch for Map/Set/WeakMap/etc, but never included RegExp's own thunk, so it never reached the RegExp arm construct.rs already has. const RegExpCtor = RegExp; new RegExpCtor(pattern) - socket-lib's rolldown-bundled primordials module does exactly this - fell through to the generic empty-object path, producing an object .source/.flags readers reject as an unbranded receiver. --- .../perry-runtime/src/object/class_registry/class_meta.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/perry-runtime/src/object/class_registry/class_meta.rs b/crates/perry-runtime/src/object/class_registry/class_meta.rs index 7d5ba8b387..3325f104ca 100644 --- a/crates/perry-runtime/src/object/class_registry/class_meta.rs +++ b/crates/perry-runtime/src/object/class_registry/class_meta.rs @@ -199,6 +199,14 @@ pub(crate) fn identify_global_builtin_constructor(func_value: f64) -> Option<&'s // bare-call thunk (which throws "Constructor WeakMap requires 'new'"). || func_ptr == map_constructor_call_thunk as *const u8 as usize || func_ptr == set_constructor_call_thunk as *const u8 as usize + // #2889's own arm handles `new (rebound RegExp)(...)`, but this + // recognition step never accepted RegExp's dedicated thunk, so it + // never reached that arm: `const RegExpCtor = RegExp; new + // RegExpCtor(pattern)` (socket-lib's rolldown-bundled primordials + // module does exactly this) fell through to the generic + // empty-object path, producing an object `.source`/`.flags` + // readers reject as an unbranded receiver. + || func_ptr == regexp_constructor_call_thunk as *const u8 as usize || func_ptr == weak_map_constructor_call_thunk as *const u8 as usize || func_ptr == weak_set_constructor_call_thunk as *const u8 as usize || func_ptr == weak_ref_constructor_call_thunk as *const u8 as usize From 77818175bdd78cb9fe3845aae2ec2b38822da309 Mon Sep 17 00:00:00 2001 From: John-David Dalton Date: Fri, 21 Aug 2026 23:39:44 -0400 Subject: [PATCH 3/5] test: cover the unwrapped-CJS require-reassign and rebound-RegExp fixes Neither existing bug had a wired regression test. #8342's cjs_wrap tests cover the CJS-wrapped case; add the genuinely top-level (unwrapped) shape that actually reproduced the node_process crash. #2889 already handles a rebound Map/Set/WeakMap/etc constructor but had no RegExp coverage at all. --- .../perry/tests/rebound_regexp_constructor.rs | 103 ++++++++++++ .../require_reassign_unwrapped_module.rs | 146 ++++++++++++++++++ 2 files changed, 249 insertions(+) create mode 100644 crates/perry/tests/rebound_regexp_constructor.rs create mode 100644 crates/perry/tests/require_reassign_unwrapped_module.rs diff --git a/crates/perry/tests/rebound_regexp_constructor.rs b/crates/perry/tests/rebound_regexp_constructor.rs new file mode 100644 index 0000000000..3edbd40bce --- /dev/null +++ b/crates/perry/tests/rebound_regexp_constructor.rs @@ -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"); +} diff --git a/crates/perry/tests/require_reassign_unwrapped_module.rs b/crates/perry/tests/require_reassign_unwrapped_module.rs new file mode 100644 index 0000000000..3d6f8718e3 --- /dev/null +++ b/crates/perry/tests/require_reassign_unwrapped_module.rs @@ -0,0 +1,146 @@ +//! Regression test: a genuinely top-level (unwrapped) CJS module that does +//! `let x = require(""); x = someHelper.__toESM(x);` must keep +//! a real runtime binding for `x`. +//! +//! This is the exact shape of `@socketsecurity/lib`'s rolldown-bundled +//! `dist/bin/trusted.js`: a plain CJS file (no enclosing `cjs_wrap` IIFE, no +//! synthetic `require`) whose top level reassigns the `require()` result to +//! wrap it in a `.default` shape, then reads `x.default.*` further down. +//! +//! Pre-fix, `register_native_fetch_and_streams` treated ANY `let/const/var +//! x = require("")` as an immutable namespace binding and emitted no +//! runtime variable for `x` at all — correct for a `const` that's never +//! reassigned, wrong here: the very next statement's `x = require_runtime +//! .__toESM(x)` then referenced a binding that was never created, and the +//! compiled binary threw `ReferenceError: node_process is not defined` on +//! every invocation even though the compile itself reported success. The fix +//! gates the optimization on the declaration actually being `const`. + +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 { createRequire } from "node:module"; +const require = createRequire(import.meta.url); +const trusted = require("trusted-pkg"); +console.log("platform:", trusted.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")); +} From aa26c11bc746697eb31b9313d07de3bccf2d2f54 Mon Sep 17 00:00:00 2001 From: John-David Dalton Date: Sat, 22 Aug 2026 00:26:32 -0400 Subject: [PATCH 4/5] test: correct the require-reassign test's claim about what it isolates #8342's existing CJS-wrap shadow check already protects this exact minimized shape on the current tree, so it does not reproduce the historical crash in isolation here - verified separately against the perry commit @socketsecurity/lib was actually pinned to (06137858d, pre-#8342) with the real 108-module dependency graph. Reframe the test as a contract test for the reassignment behavior rather than overclaiming it isolates this fix specifically. --- .../require_reassign_unwrapped_module.rs | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/crates/perry/tests/require_reassign_unwrapped_module.rs b/crates/perry/tests/require_reassign_unwrapped_module.rs index 3d6f8718e3..d8d74cf523 100644 --- a/crates/perry/tests/require_reassign_unwrapped_module.rs +++ b/crates/perry/tests/require_reassign_unwrapped_module.rs @@ -1,20 +1,24 @@ -//! Regression test: a genuinely top-level (unwrapped) CJS module that does -//! `let x = require(""); x = someHelper.__toESM(x);` must keep -//! a real runtime binding for `x`. +//! Contract test: a CJS dependency that does `let x = require(""); 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`: a plain CJS file (no enclosing `cjs_wrap` IIFE, no -//! synthetic `require`) whose top level reassigns the `require()` result to -//! wrap it in a `.default` shape, then reads `x.default.*` further down. +//! `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("")` as an immutable +//! namespace binding and emitted no runtime variable for `x` at all — correct +//! for a `const` that's never reassigned, wrong here. //! -//! Pre-fix, `register_native_fetch_and_streams` treated ANY `let/const/var -//! x = require("")` as an immutable namespace binding and emitted no -//! runtime variable for `x` at all — correct for a `const` that's never -//! reassigned, wrong here: the very next statement's `x = require_runtime -//! .__toESM(x)` then referenced a binding that was never created, and the -//! compiled binary threw `ReferenceError: node_process is not defined` on -//! every invocation even though the compile itself reported success. The fix -//! gates the optimization on the declaration actually being `const`. +//! `#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; @@ -126,10 +130,8 @@ exports.currentPlatform = currentPlatform; std::fs::write( root.join("main.ts"), r#" -import { createRequire } from "node:module"; -const require = createRequire(import.meta.url); -const trusted = require("trusted-pkg"); -console.log("platform:", trusted.currentPlatform()); +import { currentPlatform } from "trusted-pkg"; +console.log("platform:", currentPlatform()); "#, ) .expect("write entry"); From 8200a7a0c5d7cf98cccfc4bcc923090a542c972d Mon Sep 17 00:00:00 2001 From: John-David Dalton Date: Sat, 22 Aug 2026 00:31:17 -0400 Subject: [PATCH 5/5] fix(runtime): add RegExp to the direct constructor-name mapping too The is_global_builtin_func recognition list is only half of identify_global_builtin_constructor: the direct mapping below it (#5989) is what survives a REASSIGNED global binding, since it skips the globalThis singleton walk entirely. Adding the thunk to only the first list left a rebound RegExp constructor falling back to the singleton walk, which fails the same way #5989's Date case did if globalThis.RegExp is ever reassigned. --- crates/perry-runtime/src/object/class_registry/class_meta.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/perry-runtime/src/object/class_registry/class_meta.rs b/crates/perry-runtime/src/object/class_registry/class_meta.rs index 3325f104ca..423182d5bd 100644 --- a/crates/perry-runtime/src/object/class_registry/class_meta.rs +++ b/crates/perry-runtime/src/object/class_registry/class_meta.rs @@ -285,6 +285,8 @@ pub(crate) fn identify_global_builtin_constructor(func_value: f64) -> Option<&'s Some("WeakRef") } else if func_ptr == promise_constructor_call_thunk as *const u8 as usize { Some("Promise") + } else if func_ptr == regexp_constructor_call_thunk as *const u8 as usize { + Some("RegExp") } else { None };