Fixup regression in getBuiltinModule

src/workerd/api/node/process.c++

@@ -24,7 +24,7 @@ jsg::JsValue ProcessModule::getBuiltinModule(jsg::Lock& js, kj::String specifier
   }
 
   if (FeatureFlags::get(js).getNewModuleRegistry()) {
-    KJ_IF_SOME(mod, js.resolveInternalModule(specifier)) {
+    KJ_IF_SOME(mod, js.resolvePublicBuiltinModule(specifier)) {
       return mod;
     }
     return js.undefined();
@@ -33,23 +33,26 @@ jsg::JsValue ProcessModule::getBuiltinModule(jsg::Lock& js, kj::String specifier
   auto registry = jsg::ModuleRegistry::from(js);
   if (registry == nullptr) return js.undefined();
 
-  // Handle process module redirection based on enable_nodejs_process_v2 flag
+  // getBuiltinModule is a user-facing API that must only return modules that
+  // are normally importable by user code. Default to BUILTIN_ONLY which only
+  // resolves user-importable built-ins, not internal-only modules.
+  auto resolveOption = jsg::ModuleRegistry::ResolveOption::BUILTIN_ONLY;
+
+  // Handle process module redirection based on enable_nodejs_process_v2 flag.
+  // This is a special case where we need to redirect to an internal module,
+  // so we escalate to INTERNAL_ONLY only for this specific redirect.
   if (isNode && specifier == "node:process") {
     auto featureFlags = FeatureFlags::get(js);
     if (featureFlags.getEnableNodeJsProcessV2()) {
       specifier = kj::str("node-internal:public_process");
     } else {
       specifier = kj::str("node-internal:legacy_process");
     }
+    resolveOption = jsg::ModuleRegistry::ResolveOption::INTERNAL_ONLY;
   }
 
   auto path = kj::Path::parse(specifier);
 
-  // Use INTERNAL_ONLY for node-internal: modules, BUILTIN_ONLY for others
-  auto resolveOption = specifier.startsWith("node-internal:")
-      ? jsg::ModuleRegistry::ResolveOption::INTERNAL_ONLY
-      : jsg::ModuleRegistry::ResolveOption::BUILTIN_ONLY;
-
   KJ_IF_SOME(info,
       registry->resolve(js, path, kj::none, resolveOption,
           jsg::ModuleRegistry::ResolveMethod::IMPORT, rawSpecifier.asPtr())) {

src/workerd/api/node/tests/BUILD.bazel

@@ -252,6 +252,12 @@ wd_test(
     data = ["process-nodejs-test.js"],
 )
 
+wd_test(
+    src = "process-getbuiltin-newmodreg-test.wd-test",
+    args = ["--experimental"],
+    data = ["process-getbuiltin-newmodreg-test.js"],
+)
+
 wd_test(
     src = "process-legacy-nodejs-test.wd-test",
     args = ["--experimental"],

src/workerd/api/node/tests/process-getbuiltin-newmodreg-test.js

@@ -0,0 +1,44 @@
+import { strictEqual } from 'node:assert';
+
+export const getBuiltinModulePublicBuiltins = {
+  test() {
+    // process.getBuiltinModule must be able to resolve known public built-ins.
+    strictEqual(typeof process.getBuiltinModule('node:process'), 'object');
+    strictEqual(typeof process.getBuiltinModule('node:buffer'), 'object');
+    strictEqual(typeof process.getBuiltinModule('node:assert'), 'object');
+  },
+};
+
+export const getBuiltinModuleNonExistent = {
+  test() {
+    // Non-existent modules return undefined.
+    strictEqual(process.getBuiltinModule('node:nonexistent'), undefined);
+    strictEqual(process.getBuiltinModule('non-existent'), undefined);
+  },
+};
+
+export const getBuiltinModuleInternalNotExposed = {
+  test() {
+    // Internal modules must not be accessible via getBuiltinModule.
+    // These are BUILTIN_ONLY modules that should only be importable
+    // by other built-in modules, never by user code.
+    strictEqual(process.getBuiltinModule('node-internal:process'), undefined);
+    strictEqual(
+      process.getBuiltinModule('node-internal:public_process'),
+      undefined
+    );
+    strictEqual(
+      process.getBuiltinModule('node-internal:legacy_process'),
+      undefined
+    );
+    strictEqual(
+      process.getBuiltinModule('node-internal:internal_timers_global_override'),
+      undefined
+    );
+    strictEqual(process.getBuiltinModule('cloudflare-internal:env'), undefined);
+    strictEqual(
+      process.getBuiltinModule('cloudflare-internal:filesystem'),
+      undefined
+    );
+  },
+};

src/workerd/api/node/tests/process-getbuiltin-newmodreg-test.wd-test

@@ -0,0 +1,18 @@
+using Workerd = import "/workerd/workerd.capnp";
+
+const unitTests :Workerd.Config = (
+  services = [
+    ( name = "process-getbuiltin-newmodreg-test",
+      worker = (
+        modules = [
+          (name = "worker", esModule = embed "process-getbuiltin-newmodreg-test.js"),
+        ],
+        compatibilityFlags = [
+          "nodejs_compat_v2",
+          "new_module_registry",
+          "experimental",
+        ],
+      ),
+    ),
+  ],
+);

src/workerd/api/node/tests/process-nodejs-test.js

@@ -588,6 +588,46 @@ export const processGetBuiltinModule = {
     // Should return null/undefined for non-existent modules
     const nonExistent = process.getBuiltinModule('node:nonexistent');
     assert.ok(nonExistent == null);
+
+    // Internal modules must not be accessible via getBuiltinModule.
+    // These are BUILTIN_ONLY modules that should only be importable
+    // by other built-in modules, never by user code.
+    assert.strictEqual(
+      process.getBuiltinModule('node-internal:process'),
+      undefined
+    );
+    assert.strictEqual(
+      process.getBuiltinModule('node-internal:public_process'),
+      undefined
+    );
+    assert.strictEqual(
+      process.getBuiltinModule('node-internal:legacy_process'),
+      undefined
+    );
+    assert.strictEqual(
+      process.getBuiltinModule('node-internal:internal_timers_global_override'),
+      undefined
+    );
+    assert.strictEqual(
+      process.getBuiltinModule('cloudflare-internal:env'),
+      undefined
+    );
+    assert.strictEqual(
+      process.getBuiltinModule('cloudflare-internal:filesystem'),
+      undefined
+    );
+    assert.strictEqual(
+      process.getBuiltinModule('node-internal:public_process'),
+      undefined
+    );
+    assert.strictEqual(
+      process.getBuiltinModule('node-internal:legacy_process'),
+      undefined
+    );
+    assert.strictEqual(
+      process.getBuiltinModule('node-internal:internal_timers_global_override'),
+      undefined
+    );
   },
 };
 

src/workerd/jsg/jsg.c++

@@ -368,6 +368,13 @@ kj::Maybe<JsObject> Lock::resolveInternalModule(kj::StringPtr specifier) {
   return jsg::JsObject(module.getHandle(*this).As<v8::Object>());
 }
 
+kj::Maybe<JsObject> Lock::resolvePublicBuiltinModule(kj::StringPtr specifier) {
+  auto& isolate = IsolateBase::from(v8Isolate);
+  KJ_ASSERT(isolate.isUsingNewModuleRegistry());
+  return jsg::modules::ModuleRegistry::tryResolveModuleNamespace(
+      *this, specifier, jsg::modules::ResolveContext::Type::PUBLIC_BUILTIN);
+}
+
 kj::Maybe<JsObject> Lock::resolveModule(kj::StringPtr specifier, RequireEsm requireEsm) {
   auto& isolate = IsolateBase::from(v8Isolate);
   if (isolate.isUsingNewModuleRegistry()) {

src/workerd/jsg/jsg.h

@@ -2849,6 +2849,14 @@ class Lock {
   // This variation can be used only for internal built-ins.
   kj::Maybe<JsObject> resolveInternalModule(kj::StringPtr specifier);
 
+  // Resolve a user-importable built-in module namespace from the given specifier.
+  // Unlike resolveInternalModule, this only searches user-importable built-ins
+  // (PUBLIC_BUILTIN context), excluding internal-only modules and worker bundle
+  // modules. Use this for user-facing APIs like process.getBuiltinModule() that
+  // must not expose internal modules or return user bundle overrides.
+  // Only valid when the new module registry is in use.
+  kj::Maybe<JsObject> resolvePublicBuiltinModule(kj::StringPtr specifier);
+
   // Resolve a module namespace from the given specifier.
   // This variation includes modules from the worker bundle.
   kj::Maybe<JsObject> resolveModule(

src/workerd/jsg/modules-new.c++

@@ -1537,6 +1537,13 @@ kj::Maybe<const Module&> ModuleRegistry::lookupImpl(
       MODULE_LOOKUP(context, kBuiltinOnly);
       break;
     }
+    case ResolveContext::Type::PUBLIC_BUILTIN: {
+      // For public built-in resolution, we only use builtin bundles.
+      // This excludes both worker bundle modules and internal-only modules,
+      // returning only built-ins that are normally importable by user code.
+      MODULE_LOOKUP(context, kBuiltin);
+      break;
+    }
   }
 
   return kj::none;

src/workerd/jsg/observer.h

@@ -36,6 +36,11 @@ struct ResolveObserver {
     // Like builtin, but it's a module that is *only* resolvable from a builtin
     // (like the `node-internal:...` modules)
     BUILTIN_ONLY,
+    // Resolves only user-importable built-in modules (the kBuiltin bundle),
+    // excluding both worker bundle modules and internal-only modules. Used
+    // by user-facing APIs like process.getBuiltinModule() that must not
+    // expose internal modules or return user bundle overrides.
+    PUBLIC_BUILTIN,
   };
 
   enum class Source {