Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Apr 14, 2022
16 changes: 8 additions & 8 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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();
}
}
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,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);
Expand Down
11 changes: 4 additions & 7 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -861,11 +860,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);
}

Expand Down Expand Up @@ -1049,7 +1045,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,
Expand Down
40 changes: 38 additions & 2 deletions test/parallel/test-runner-import-no-scheme.js
Original file line number Diff line number Diff line change
@@ -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');
cjihrig marked this conversation as resolved.
Show resolved Hide resolved
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);
}