runtime: unify createRequire builtin allowlist with getBuiltinModule — v8 + family audit (#6651)

crates/perry-runtime/src/module_require.rs

@@ -91,58 +91,28 @@ 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> {
     supported_require_builtin(specifier).map(|_| specifier)
 }
 
 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}"
+                );
+            }
+        }
+    }
+}

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)]

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"
+                );
+            }
+        }
+    }
+}

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 {