module: ensure 'node:'-only modules can access node_modules

cjihrig · web-flow · commit 93c4dc5e5a90 · 2022-04-14T14:08:55.000+01:00
This commit allows require() and import to search the node_modules directories when importing a core module that must have the node: scheme. This prevents these core modules from shadowing userland modules with the same name but no prefix. PR-URL: #42430 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Beth Griggs <bgriggs@redhat.com> Reviewed-By: Richard Lau <rlau@redhat.com>
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -666,7 +666,8 @@ if (isWindows) {
 }
 
 Module._resolveLookupPaths = function(request, parent) {
-  if (NativeModule.canBeRequiredByUsers(request)) {
+  if (NativeModule.canBeRequiredByUsers(request) &&
+      NativeModule.canBeRequiredWithoutScheme(request)) {
     debug('looking for %j in []', request);
     return null;
   }
@@ -803,11 +804,8 @@ Module._load = function(request, parent, isMain) {
   }
 
   const mod = loadNativeModule(filename, request);
-  if (mod?.canBeRequiredByUsers) {
-    if (!NativeModule.canBeRequiredWithoutScheme(filename)) {
-      throw new ERR_UNKNOWN_BUILTIN_MODULE(filename);
-    }
-
+  if (mod?.canBeRequiredByUsers &&
+      NativeModule.canBeRequiredWithoutScheme(filename)) {
     return mod.exports;
   }
 
@@ -854,7 +852,8 @@ Module._load = function(request, parent, isMain) {
 
 Module._resolveFilename = function(request, parent, isMain, options) {
   if (StringPrototypeStartsWith(request, 'node:') ||
-      NativeModule.canBeRequiredByUsers(request)) {
+      (NativeModule.canBeRequiredByUsers(request) &&
+      NativeModule.canBeRequiredWithoutScheme(request))) {
     return request;
   }
 
@@ -1286,7 +1285,8 @@ Module._preloadModules = function(requests) {
 
 Module.syncBuiltinESMExports = function syncBuiltinESMExports() {
   for (const mod of NativeModule.map.values()) {
-    if (mod.canBeRequiredByUsers) {
+    if (mod.canBeRequiredByUsers &&
+        NativeModule.canBeRequiredWithoutScheme(mod.id)) {
       mod.syncExports();
     }
   }
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -534,7 +534,8 @@ class ESMLoader {
           globalThis,
           // Param getBuiltin
           (builtinName) => {
-            if (NativeModule.canBeRequiredByUsers(builtinName)) {
+            if (NativeModule.canBeRequiredByUsers(builtinName) &&
+                NativeModule.canBeRequiredWithoutScheme(builtinName)) {
               return require(builtinName);
             }
             throw new ERR_INVALID_ARG_VALUE('builtinName', builtinName);
diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -57,7 +57,6 @@ const {
   ERR_PACKAGE_PATH_NOT_EXPORTED,
   ERR_UNSUPPORTED_DIR_IMPORT,
   ERR_NETWORK_IMPORT_DISALLOWED,
-  ERR_UNKNOWN_BUILTIN_MODULE,
   ERR_UNSUPPORTED_ESM_URL_SCHEME,
 } = require('internal/errors').codes;
 const { Module: CJSModule } = require('internal/modules/cjs/loader');
@@ -853,11 +852,8 @@ function parsePackageName(specifier, base) {
  * @returns {resolved: URL, format? : string}
  */
 function packageResolve(specifier, base, conditions) {
-  if (NativeModule.canBeRequiredByUsers(specifier)) {
-    if (!NativeModule.canBeRequiredWithoutScheme(specifier)) {
-      throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier);
-    }
-
+  if (NativeModule.canBeRequiredByUsers(specifier) &&
+      NativeModule.canBeRequiredWithoutScheme(specifier)) {
     return new URL('node:' + specifier);
   }
 
@@ -1041,7 +1037,8 @@ function checkIfDisallowedImport(specifier, parsed, parsedParentURL) {
 
         return { url: parsed.href };
       }
-      if (NativeModule.canBeRequiredByUsers(specifier)) {
+      if (NativeModule.canBeRequiredByUsers(specifier) &&
+          NativeModule.canBeRequiredWithoutScheme(specifier)) {
         throw new ERR_NETWORK_IMPORT_DISALLOWED(
           specifier,
           parentURL,
diff --git a/test/parallel/test-runner-import-no-scheme.js b/test/parallel/test-runner-import-no-scheme.js
@@ -1,15 +1,51 @@
 'use strict';
 const common = require('../common');
+const tmpdir = require('../common/tmpdir');
 const assert = require('assert');
+const { spawnSync } = require('child_process');
+const fs = require('fs');
+const path = require('path');
+const { createRequire } = require('module');
 
 assert.throws(
   () => require('test'),
-  common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }),
+  common.expectsError({ code: 'MODULE_NOT_FOUND' }),
 );
 
 (async () => {
   await assert.rejects(
     async () => import('test'),
-    common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }),
+    common.expectsError({ code: 'ERR_MODULE_NOT_FOUND' }),
   );
 })().then(common.mustCall());
+
+assert.throws(
+  () => require.resolve('test'),
+  common.expectsError({ code: 'MODULE_NOT_FOUND' }),
+);
+
+// Verify that files in node_modules can be resolved.
+tmpdir.refresh();
+
+const packageRoot = path.join(tmpdir.path, 'node_modules', 'test');
+const indexFile = path.join(packageRoot, 'index.js');
+
+fs.mkdirSync(packageRoot, { recursive: true });
+fs.writeFileSync(indexFile, 'module.exports = { marker: 1 };');
+
+function test(argv) {
+  const child = spawnSync(process.execPath, argv, { cwd: tmpdir.path });
+  assert.strictEqual(child.status, 0);
+  assert.strictEqual(child.stdout.toString().trim(), '{ marker: 1 }');
+}
+
+test(['-e', 'console.log(require("test"))']);
+test(['-e', 'import("test").then(m=>console.log(m.default))']);
+test(['--input-type=module', '-e', 'import test from "test";console.log(test)']);
+test(['--input-type=module', '-e', 'console.log((await import("test")).default)']);
+
+{
+  const dummyFile = path.join(tmpdir.path, 'file.js');
+  const require = createRequire(dummyFile);
+  assert.strictEqual(require.resolve('test'), indexFile);
+}

Original file line number	Diff line number	Diff line change
`@@ -666,7 +666,8 @@ if (isWindows) {`
`666`	`666`	`}`
`667`	`667`
`668`	`668`	`Module._resolveLookupPaths = function(request, parent) {`
`669`		`- if (NativeModule.canBeRequiredByUsers(request)) {`
	`669`	`+ if (NativeModule.canBeRequiredByUsers(request) &&`
	`670`	`+ NativeModule.canBeRequiredWithoutScheme(request)) {`
`670`	`671`	`debug('looking for %j in []', request);`
`671`	`672`	`return null;`
`672`	`673`	`}`
`@@ -803,11 +804,8 @@ Module._load = function(request, parent, isMain) {`
`803`	`804`	`}`
`804`	`805`
`805`	`806`	`const mod = loadNativeModule(filename, request);`
`806`		`- if (mod?.canBeRequiredByUsers) {`
`807`		`- if (!NativeModule.canBeRequiredWithoutScheme(filename)) {`
`808`		`- throw new ERR_UNKNOWN_BUILTIN_MODULE(filename);`
`809`		`- }`
`810`		`-`
	`807`	`+ if (mod?.canBeRequiredByUsers &&`
	`808`	`+ NativeModule.canBeRequiredWithoutScheme(filename)) {`
`811`	`809`	`return mod.exports;`
`812`	`810`	`}`
`813`	`811`
`@@ -854,7 +852,8 @@ Module._load = function(request, parent, isMain) {`
`854`	`852`
`855`	`853`	`Module._resolveFilename = function(request, parent, isMain, options) {`
`856`	`854`	`if (StringPrototypeStartsWith(request, 'node:') \|\|`
`857`		`- NativeModule.canBeRequiredByUsers(request)) {`
	`855`	`+ (NativeModule.canBeRequiredByUsers(request) &&`
	`856`	`+ NativeModule.canBeRequiredWithoutScheme(request))) {`
`858`	`857`	`return request;`
`859`	`858`	`}`
`860`	`859`
`@@ -1286,7 +1285,8 @@ Module._preloadModules = function(requests) {`
`1286`	`1285`
`1287`	`1286`	`Module.syncBuiltinESMExports = function syncBuiltinESMExports() {`
`1288`	`1287`	`for (const mod of NativeModule.map.values()) {`
`1289`		`- if (mod.canBeRequiredByUsers) {`
	`1288`	`+ if (mod.canBeRequiredByUsers &&`
	`1289`	`+ NativeModule.canBeRequiredWithoutScheme(mod.id)) {`
`1290`	`1290`	`mod.syncExports();`
`1291`	`1291`	`}`
`1292`	`1292`	`}`