Skip to content

Commit c284eb3

Browse files
cjihrigtargos
authored andcommitted
module: ensure 'node:'-only modules can access node_modules
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 <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Richard Lau <[email protected]>
1 parent 95a823d commit c284eb3

File tree

4 files changed

+52
-18
lines changed

4 files changed

+52
-18
lines changed

lib/internal/modules/cjs/loader.js

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,8 @@ if (isWindows) {
669669
}
670670

671671
Module._resolveLookupPaths = function(request, parent) {
672-
if (NativeModule.canBeRequiredByUsers(request)) {
672+
if (NativeModule.canBeRequiredByUsers(request) &&
673+
NativeModule.canBeRequiredWithoutScheme(request)) {
673674
debug('looking for %j in []', request);
674675
return null;
675676
}
@@ -806,11 +807,8 @@ Module._load = function(request, parent, isMain) {
806807
}
807808

808809
const mod = loadNativeModule(filename, request);
809-
if (mod?.canBeRequiredByUsers) {
810-
if (!NativeModule.canBeRequiredWithoutScheme(filename)) {
811-
throw new ERR_UNKNOWN_BUILTIN_MODULE(filename);
812-
}
813-
810+
if (mod?.canBeRequiredByUsers &&
811+
NativeModule.canBeRequiredWithoutScheme(filename)) {
814812
return mod.exports;
815813
}
816814

@@ -857,7 +855,8 @@ Module._load = function(request, parent, isMain) {
857855

858856
Module._resolveFilename = function(request, parent, isMain, options) {
859857
if (StringPrototypeStartsWith(request, 'node:') ||
860-
NativeModule.canBeRequiredByUsers(request)) {
858+
(NativeModule.canBeRequiredByUsers(request) &&
859+
NativeModule.canBeRequiredWithoutScheme(request))) {
861860
return request;
862861
}
863862

@@ -1293,7 +1292,8 @@ Module._preloadModules = function(requests) {
12931292

12941293
Module.syncBuiltinESMExports = function syncBuiltinESMExports() {
12951294
for (const mod of NativeModule.map.values()) {
1296-
if (mod.canBeRequiredByUsers) {
1295+
if (mod.canBeRequiredByUsers &&
1296+
NativeModule.canBeRequiredWithoutScheme(mod.id)) {
12971297
mod.syncExports();
12981298
}
12991299
}

lib/internal/modules/esm/loader.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,8 @@ class ESMLoader {
750750
globalThis,
751751
// Param getBuiltin
752752
(builtinName) => {
753-
if (NativeModule.canBeRequiredByUsers(builtinName)) {
753+
if (NativeModule.canBeRequiredByUsers(builtinName) &&
754+
NativeModule.canBeRequiredWithoutScheme(builtinName)) {
754755
return require(builtinName);
755756
}
756757
throw new ERR_INVALID_ARG_VALUE('builtinName', builtinName);

lib/internal/modules/esm/resolve.js

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ const {
5757
ERR_PACKAGE_PATH_NOT_EXPORTED,
5858
ERR_UNSUPPORTED_DIR_IMPORT,
5959
ERR_NETWORK_IMPORT_DISALLOWED,
60-
ERR_UNKNOWN_BUILTIN_MODULE,
6160
ERR_UNSUPPORTED_ESM_URL_SCHEME,
6261
} = require('internal/errors').codes;
6362
const { Module: CJSModule } = require('internal/modules/cjs/loader');
@@ -888,11 +887,8 @@ function parsePackageName(specifier, base) {
888887
* @returns {URL}
889888
*/
890889
function packageResolve(specifier, base, conditions) {
891-
if (NativeModule.canBeRequiredByUsers(specifier)) {
892-
if (!NativeModule.canBeRequiredWithoutScheme(specifier)) {
893-
throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier);
894-
}
895-
890+
if (NativeModule.canBeRequiredByUsers(specifier) &&
891+
NativeModule.canBeRequiredWithoutScheme(specifier)) {
896892
return new URL('node:' + specifier);
897893
}
898894

@@ -1076,7 +1072,8 @@ function checkIfDisallowedImport(specifier, parsed, parsedParentURL) {
10761072

10771073
return { url: parsed.href };
10781074
}
1079-
if (NativeModule.canBeRequiredByUsers(specifier)) {
1075+
if (NativeModule.canBeRequiredByUsers(specifier) &&
1076+
NativeModule.canBeRequiredWithoutScheme(specifier)) {
10801077
throw new ERR_NETWORK_IMPORT_DISALLOWED(
10811078
specifier,
10821079
parsedParentURL,
Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,51 @@
11
'use strict';
22
const common = require('../common');
3+
const tmpdir = require('../common/tmpdir');
34
const assert = require('assert');
5+
const { spawnSync } = require('child_process');
6+
const fs = require('fs');
7+
const path = require('path');
8+
const { createRequire } = require('module');
49

510
assert.throws(
611
() => require('test'),
7-
common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }),
12+
common.expectsError({ code: 'MODULE_NOT_FOUND' }),
813
);
914

1015
(async () => {
1116
await assert.rejects(
1217
async () => import('test'),
13-
common.expectsError({ code: 'ERR_UNKNOWN_BUILTIN_MODULE' }),
18+
common.expectsError({ code: 'ERR_MODULE_NOT_FOUND' }),
1419
);
1520
})().then(common.mustCall());
21+
22+
assert.throws(
23+
() => require.resolve('test'),
24+
common.expectsError({ code: 'MODULE_NOT_FOUND' }),
25+
);
26+
27+
// Verify that files in node_modules can be resolved.
28+
tmpdir.refresh();
29+
30+
const packageRoot = path.join(tmpdir.path, 'node_modules', 'test');
31+
const indexFile = path.join(packageRoot, 'index.js');
32+
33+
fs.mkdirSync(packageRoot, { recursive: true });
34+
fs.writeFileSync(indexFile, 'module.exports = { marker: 1 };');
35+
36+
function test(argv) {
37+
const child = spawnSync(process.execPath, argv, { cwd: tmpdir.path });
38+
assert.strictEqual(child.status, 0);
39+
assert.strictEqual(child.stdout.toString().trim(), '{ marker: 1 }');
40+
}
41+
42+
test(['-e', 'console.log(require("test"))']);
43+
test(['-e', 'import("test").then(m=>console.log(m.default))']);
44+
test(['--input-type=module', '-e', 'import test from "test";console.log(test)']);
45+
test(['--input-type=module', '-e', 'console.log((await import("test")).default)']);
46+
47+
{
48+
const dummyFile = path.join(tmpdir.path, 'file.js');
49+
const require = createRequire(dummyFile);
50+
assert.strictEqual(require.resolve('test'), indexFile);
51+
}

0 commit comments

Comments
 (0)