diff --git a/crates/perry-runtime/src/module_require.rs b/crates/perry-runtime/src/module_require.rs index f10d86c2c2..8ee259d46d 100644 --- a/crates/perry-runtime/src/module_require.rs +++ b/crates/perry-runtime/src/module_require.rs @@ -91,29 +91,16 @@ fn validate_create_require_base(filename_or_url: f64) { throw_invalid_value("filename", filename_or_url); } +/// #6651 (pi wall #5, same family as #6644's wall #3): this used to be a +/// hand-copied allowlist that drifted from `process.getBuiltinModule`'s and +/// from the static-import tables — `v8` (and `sea`, `fs/promises`, +/// `stream/consumers`, `stream/web`, `trace_events`, `test/reporters`) were +/// implemented and statically importable but rejected here as "package/file". +/// Both resolvers now share one source of truth (`MODULE_BUILTIN_MODULES`, +/// i.e. `module.builtinModules`), including the `node:` normalization and the +/// scheme-only / `_`-internal carve-outs. fn supported_require_builtin(specifier: &str) -> Option<&str> { - let name = specifier.strip_prefix("node:").unwrap_or(specifier); - match name { - "assert" | "assert/strict" | "async_hooks" | "buffer" | "child_process" | "cluster" - | "console" | "constants" | "crypto" | "dns" | "dns/promises" | "events" | "fs" - | "http" | "http2" | "https" | "module" | "net" | "os" | "path" | "path/posix" - | "path/win32" | "perf_hooks" | "process" | "punycode" | "querystring" | "readline" - | "readline/promises" | "stream" | "stream/promises" | "string_decoder" | "sys" - | "test" | "test/reporters" | "timers" | "timers/promises" | "tls" | "tty" | "url" - | "util" | "util/types" | "vm" | "wasi" | "worker_threads" | "zlib" - // Implemented native modules that were missing from the createRequire - // allowlist (they have runtime registry buckets + dispatch, but - // `require('tls')` etc. via createRequire was rejected as "package/file"). - | "dgram" | "domain" | "inspector" | "inspector/promises" | "repl" - | "sqlite" - // #6644: implemented as a node_submodules spec (real pub/sub channel - // registry in node_submodules/diagnostics.rs) but missing here, so - // `require('node:diagnostics_channel')` through createRequire (the - // esbuild banner shim in any ESM bundle of CJS deps — lru-cache's node - // build in the pi bundle) was rejected as "package/file". - | "diagnostics_channel" => Some(name), - _ => None, - } + crate::process::supported_builtin_module_name(specifier) } fn resolve_builtin(specifier: &str) -> Option<&str> { @@ -121,28 +108,11 @@ fn resolve_builtin(specifier: &str) -> Option<&str> { } fn require_builtin_value(module_name: &str) -> f64 { - if module_name == "timers/promises" { - return unsafe { - crate::node_submodules::js_node_submodule_namespace( - b"timers_promises".as_ptr(), - "timers_promises".len() as u32, - ) - }; - } - // #6644: diagnostics_channel lives in the node_submodules registry (not a - // native-module dispatch bucket); route it there like timers/promises so - // `require('diagnostics_channel')` / `require('node:diagnostics_channel')` - // return the real channel/subscribe/tracingChannel exports instead of an - // empty native-module namespace. - if module_name == "diagnostics_channel" { - return unsafe { - crate::node_submodules::js_node_submodule_namespace( - b"diagnostics_channel".as_ptr(), - "diagnostics_channel".len() as u32, - ) - }; - } - crate::object::native_module_get_builtin_module_value(module_name) + // #6651: shared routing with `process.getBuiltinModule` — submodule-spec + // modules (diagnostics_channel, timers/promises, fs/promises, …) resolve + // through the node_submodules registry, the rest through the native-module + // namespace. + crate::process::builtin_module_value(module_name) } fn throw_module_not_found(specifier: &str) -> ! { @@ -543,3 +513,27 @@ pub extern "C" fn js_module_ambient_require_apply(spec: f64) -> f64 { #[used] static KEEP_JS_MODULE_AMBIENT_REQUIRE_APPLY: extern "C" fn(f64) -> f64 = js_module_ambient_require_apply; + +/// #6651 family regression guard: createRequire's resolver must never drift +/// from `process.getBuiltinModule`'s again. Today they are the same function; +/// this pins the contract so a future re-split of the implementations still +/// has to keep the module sets identical across both spellings. +#[cfg(test)] +mod builtin_allowlist_parity_tests { + use super::*; + + #[test] + fn createrequire_allowlist_matches_get_builtin_module() { + for &entry in crate::process::MODULE_BUILTIN_MODULES { + let bare = entry.strip_prefix("node:").unwrap_or(entry); + let prefixed = format!("node:{bare}"); + for specifier in [bare, prefixed.as_str()] { + assert_eq!( + supported_require_builtin(specifier), + crate::process::supported_builtin_module_name(specifier), + "{specifier}" + ); + } + } + } +} diff --git a/crates/perry-runtime/src/node_submodules/mod.rs b/crates/perry-runtime/src/node_submodules/mod.rs index 12d09ed58a..5e7c9d6949 100644 --- a/crates/perry-runtime/src/node_submodules/mod.rs +++ b/crates/perry-runtime/src/node_submodules/mod.rs @@ -812,6 +812,14 @@ fn find_submodule(key: &str) -> Option<&'static SubmoduleSpec> { None } +/// Test-only: whether `key` names a registered submodule spec. #6651 — +/// `process::builtin_submodule_key`'s cross-check (the spec type and its +/// fields are private to this module). +#[cfg(test)] +pub(crate) fn is_registered_submodule_key(key: &str) -> bool { + ALL_SUBMODULE_SPECS.iter().any(|spec| spec.key == key) +} + /// Test-only: every submodule spec, for exhaustiveness checks (the production /// `find_submodule` resolves through the registry, not an iterable array). #[cfg(test)] diff --git a/crates/perry-runtime/src/process.rs b/crates/perry-runtime/src/process.rs index 7edd036b95..3772d657e2 100644 --- a/crates/perry-runtime/src/process.rs +++ b/crates/perry-runtime/src/process.rs @@ -100,55 +100,81 @@ pub(crate) fn is_function_value(value: f64) -> bool { crate::value::js_handle_is_function(value) } -pub(crate) fn supported_builtin_module_name(name: &str) -> Option<&str> { - match name { - "assert" - | "assert/strict" - | "async_hooks" - | "buffer" - | "child_process" - | "cluster" - | "console" - | "constants" - | "crypto" - | "diagnostics_channel" - | "dns" - | "dns/promises" - | "events" - | "fs" - | "http" - | "http2" - | "https" - | "module" - | "net" - | "os" - | "path" - | "perf_hooks" - | "process" - | "punycode" - | "querystring" - | "readline" - | "readline/promises" - | "sea" - | "stream" - | "stream/promises" - | "string_decoder" - | "sys" - | "test" - | "test/reporters" - | "timers" - | "timers/promises" - | "tty" - | "url" - | "util" - | "util/types" - | "vm" - | "worker_threads" - | "zlib" => Some(name), +/// #6651: single source of truth for the RUNTIME dynamic builtin resolvers. +/// `process.getBuiltinModule(id)` and the `require` returned by +/// `module.createRequire(...)` accept exactly the module set of +/// `module.builtinModules` (`MODULE_BUILTIN_MODULES`), so the three surfaces +/// can never drift apart again — pi walls #3 (#6644, `diagnostics_channel`) +/// and #5 (#6651, `v8`) were both a module implemented and statically +/// importable but missing from one hand-copied allowlist. Two carve-outs: +/// +/// - `_`-prefixed legacy internals (`_http_agent`, …): Node still serves +/// them, Perry has no implementation — they must keep failing with an +/// error that names the module, not resolve to a method-dead namespace. +/// - Scheme-only builtins (`node:sea`, `node:sqlite`, `node:test`, +/// `node:test/reporters` — stored WITH the prefix, exactly as Node spells +/// them in `module.builtinModules`): resolve only when the caller wrote +/// the `node:` prefix. The bare spelling is an ordinary npm package name +/// in Node (`require('sqlite')` is `MODULE_NOT_FOUND`, +/// `getBuiltinModule('sqlite')` is `undefined`). +/// +/// Takes the RAW specifier (either spelling); returns the prefixless name. +pub(crate) fn supported_builtin_module_name(specifier: &str) -> Option<&str> { + let (name, had_node_prefix) = match specifier.strip_prefix("node:") { + Some(stripped) => (stripped, true), + None => (specifier, false), + }; + if name.starts_with('_') { + return None; + } + // A residual `node:` after one strip is a double-prefixed specifier + // (`node:node:test`). Node rejects those; without this check the + // stripped form matches the scheme-only entries (stored WITH their + // prefix in MODULE_BUILTIN_MODULES) and a prefixed "prefixless" name + // escapes to the value router. + if name.starts_with("node:") { + return None; + } + if MODULE_BUILTIN_MODULES.contains(&name) + || (had_node_prefix && MODULE_BUILTIN_MODULES.contains(&specifier)) + { + return Some(name); + } + None +} + +/// Builtin modules the dynamic resolvers must route through the +/// `node_submodules` registry (submodule-spec exports) instead of a +/// native-module namespace. These have no native-module dispatch bucket — +/// `js_create_native_module_namespace` would hand back a method-dead object. +/// The registry key differs from the module name (`/` → `_`). +pub(crate) fn builtin_submodule_key(module_name: &str) -> Option<&'static str> { + match module_name { + "diagnostics_channel" => Some("diagnostics_channel"), + "fs/promises" => Some("fs_promises"), + "stream/consumers" => Some("stream_consumers"), + "stream/web" => Some("stream_web"), + "test/reporters" => Some("test_reporters"), + "timers/promises" => Some("timers_promises"), + "trace_events" => Some("trace_events"), _ => None, } } +/// Shared value resolver behind `process.getBuiltinModule` and createRequire's +/// `require` (#6651): submodule-spec modules resolve through the +/// `node_submodules` registry, everything else through the native-module +/// namespace (whose dispatch the caller's devirt entry armed via the +/// install-all hooks). +pub(crate) fn builtin_module_value(module_name: &str) -> f64 { + if let Some(key) = builtin_submodule_key(module_name) { + return unsafe { + crate::node_submodules::js_node_submodule_namespace(key.as_ptr(), key.len() as u32) + }; + } + crate::object::native_module_get_builtin_module_value(module_name) +} + pub(crate) const MODULE_BUILTIN_MODULES: &[&str] = &[ "_http_agent", "_http_client", @@ -724,3 +750,83 @@ thread_local! { std::cell::RefCell::new(None) }; } + +/// #6651 family regression guard: the dynamic builtin resolvers +/// (`createRequire(...)`'s `require` + `process.getBuiltinModule`) derive from +/// `MODULE_BUILTIN_MODULES`, so every module Perry lists in +/// `module.builtinModules` must resolve through them — and only through the +/// spellings Node itself accepts. +#[cfg(test)] +mod builtin_module_list_tests { + use super::*; + + #[test] + fn dynamic_resolvers_cover_every_builtin_modules_entry() { + for &entry in MODULE_BUILTIN_MODULES { + if entry.starts_with('_') { + // Legacy internals: listed for `module.builtinModules` parity, + // but unimplemented — both spellings must keep failing. + assert_eq!(supported_builtin_module_name(entry), None, "{entry}"); + let prefixed = format!("node:{entry}"); + assert_eq!(supported_builtin_module_name(&prefixed), None, "{prefixed}"); + } else if let Some(bare) = entry.strip_prefix("node:") { + // Scheme-only builtins (node:sea, node:sqlite, node:test, + // node:test/reporters): the prefixed spelling resolves, the + // bare spelling is an ordinary npm name (Node parity). + assert_eq!(supported_builtin_module_name(entry), Some(bare), "{entry}"); + assert_eq!(supported_builtin_module_name(bare), None, "{bare}"); + } else { + // Ordinary builtins: both spellings resolve to the bare name. + assert_eq!(supported_builtin_module_name(entry), Some(entry), "{entry}"); + let prefixed = format!("node:{entry}"); + assert_eq!( + supported_builtin_module_name(&prefixed), + Some(entry), + "{prefixed}" + ); + } + } + } + + #[test] + fn non_builtins_are_rejected() { + for specifier in [ + "lodash", + "node:nope", + "./file.js", + "/abs/file.js", + "", + // Double-prefixed spellings must not reach the scheme-only + // entries via the single strip (Node rejects them). + "node:node:test", + "node:node:fs", + ] { + assert_eq!( + supported_builtin_module_name(specifier), + None, + "{specifier}" + ); + } + } + + /// Every submodule-routed builtin must (a) itself be a resolvable builtin + /// name and (b) map to a registered `node_submodules` spec key — a typo'd + /// key would silently produce the empty unresolved-namespace stub. + #[test] + fn submodule_routes_point_at_real_specs() { + for &entry in MODULE_BUILTIN_MODULES { + let name = entry.strip_prefix("node:").unwrap_or(entry); + if let Some(key) = builtin_submodule_key(name) { + assert_eq!( + supported_builtin_module_name(entry), + Some(name), + "submodule-routed {name} must be resolvable" + ); + assert!( + crate::node_submodules::is_registered_submodule_key(key), + "builtin_submodule_key({name:?}) = {key:?} names no registered spec" + ); + } + } + } +} diff --git a/crates/perry-runtime/src/process/node_module.rs b/crates/perry-runtime/src/process/node_module.rs index 4f833ba9f7..dd329e4ff0 100644 --- a/crates/perry-runtime/src/process/node_module.rs +++ b/crates/perry-runtime/src/process/node_module.rs @@ -841,33 +841,13 @@ pub extern "C" fn js_process_get_builtin_module(id: f64) -> f64 { let Ok(specifier) = std::str::from_utf8(bytes) else { return f64::from_bits(crate::value::TAG_UNDEFINED); }; - if specifier == "sea" { - return f64::from_bits(crate::value::TAG_UNDEFINED); - } - let name = specifier.strip_prefix("node:").unwrap_or(specifier); - let Some(module_name) = supported_builtin_module_name(name) else { + // #6651: shared allowlist + routing with createRequire's `require` — one + // source of truth (`MODULE_BUILTIN_MODULES`), including the `node:` strip + // and the scheme-only / `_`-internal carve-outs. + let Some(module_name) = supported_builtin_module_name(specifier) else { return f64::from_bits(crate::value::TAG_UNDEFINED); }; - if module_name == "timers/promises" { - return unsafe { - crate::node_submodules::js_node_submodule_namespace( - b"timers_promises".as_ptr(), - "timers_promises".len() as u32, - ) - }; - } - // #6644: diagnostics_channel is a node_submodules spec, not a native-module - // dispatch bucket — route it there (mirrors createRequire's - // require_builtin_value). - if module_name == "diagnostics_channel" { - return unsafe { - crate::node_submodules::js_node_submodule_namespace( - b"diagnostics_channel".as_ptr(), - "diagnostics_channel".len() as u32, - ) - }; - } - crate::object::native_module_get_builtin_module_value(module_name) + crate::process::builtin_module_value(module_name) } fn module_bool_value(value: bool) -> f64 { diff --git a/crates/perry/tests/createrequire_builtin_modules.rs b/crates/perry/tests/createrequire_builtin_modules.rs index d6c0a5eb01..30eb3f43d7 100644 --- a/crates/perry/tests/createrequire_builtin_modules.rs +++ b/crates/perry/tests/createrequire_builtin_modules.rs @@ -132,3 +132,95 @@ console.log("ok"); "tls: object true\ndgram: object\nvm: object\ndomain: object\nok\n" ); } + +/// #6651 (pi wall #5): `require('node:v8')` through `createRequire` threw +/// `ERR_PERRY_UNSUPPORTED_CREATE_REQUIRE` — `v8` has a full native module +/// (`node_v8.rs`, nm dispatch bucket, static-import support) but was missing +/// from BOTH runtime dynamic allowlists. The pi bundle's esbuild createRequire +/// banner funnels `__require("node:v8")` through this path. Expected output +/// captured from `node v26.3.0` (byte-identical). +#[test] +fn createrequire_resolves_v8_in_both_spellings() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + r#" +import { createRequire } from "node:module"; +const require = createRequire(import.meta.url); + +const v8a = require("node:v8"); +const v8b = require("v8"); +console.log("serialize:", typeof v8a.serialize, typeof v8b.deserialize); +const round = v8b.deserialize(v8a.serialize({ pi: 5, arr: [1, 2, 3] })); +console.log("roundtrip:", JSON.stringify(round)); +console.log("getBuiltinModule:", typeof process.getBuiltinModule("node:v8").serialize); +"#, + ); + assert_eq!( + stdout, + "serialize: function function\n\ + roundtrip: {\"pi\":5,\"arr\":[1,2,3]}\n\ + getBuiltinModule: function\n" + ); +} + +/// #6651 family regression guard, fixture side: every entry of +/// `module.builtinModules` (the runtime's `MODULE_BUILTIN_MODULES`, which the +/// dynamic allowlists now derive from) must resolve through BOTH +/// `createRequire(...)`'s `require` and `process.getBuiltinModule`, in every +/// spelling Node accepts — and must keep failing in the spellings Node +/// rejects (bare scheme-only names) or Perry does not implement (`_`-prefixed +/// legacy internals, whose error must still name the module). +#[test] +fn createrequire_and_get_builtin_module_reach_every_builtin_modules_entry() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + r#" +import { createRequire } from "node:module"; +const require = createRequire(import.meta.url); +const failures: string[] = []; +const resolvable = (t: string) => t === "object" || t === "function"; +let checked = 0; +const moduleNs = require("node:module"); +const builtins = moduleNs.builtinModules; +for (const entry of builtins) { + if (entry.startsWith("_")) { + // Unimplemented legacy internals: must throw, naming the module. + try { + require(entry); + failures.push(entry + ": internal resolved"); + } catch (e: any) { + if (!String(e && e.message).includes(entry)) failures.push(entry + ": error hides module name"); + } + if (process.getBuiltinModule(entry) !== undefined) failures.push(entry + ": gbm resolved internal"); + continue; + } + const bare = entry.startsWith("node:") ? entry.slice(5) : entry; + const spellings = entry.startsWith("node:") ? [entry] : [entry, "node:" + entry]; + for (const s of spellings) { + checked++; + try { + if (!resolvable(typeof require(s))) failures.push(s + ": require gave non-namespace"); + } catch (e: any) { + failures.push(s + ": require threw " + (e && e.code)); + } + if (!resolvable(typeof process.getBuiltinModule(s))) failures.push(s + ": gbm gave non-namespace"); + } + if (entry.startsWith("node:")) { + // Scheme-only builtin: the bare spelling is an npm name (Node parity). + try { + require(bare); + failures.push(bare + ": scheme-only resolved bare"); + } catch (e: any) { + if (!String(e && e.message).includes(bare)) failures.push(bare + ": error hides module name"); + } + if (process.getBuiltinModule(bare) !== undefined) failures.push(bare + ": gbm resolved bare scheme-only"); + } +} +console.log("checked enough:", checked >= 100); +console.log("failures:", JSON.stringify(failures)); +"#, + ); + assert_eq!(stdout, "checked enough: true\nfailures: []\n"); +}