diff --git a/.github/workflows/lint-release-proposal.yml b/.github/workflows/lint-release-proposal.yml index 4ebeff30019b34..73617cc43cd012 100644 --- a/.github/workflows/lint-release-proposal.yml +++ b/.github/workflows/lint-release-proposal.yml @@ -88,7 +88,7 @@ jobs: gh api \ -H "Accept: application/vnd.github+json" \ -H "X-GitHub-Api-Version: 2022-11-28" \ - --jq '.commits.[] | { smallSha: .sha[0:10] } + (.commit.message|capture("^(?.+)\n\n(.*\n)*PR-URL: (?<prURL>.+)\n"))' \ + --jq '.commits.[] | { smallSha: .sha[0:10] } + (.commit.message|capture("^(?<title>.+)\n\n(.*\n)*PR-URL: (?<prURL>.+)(\n|$)"))' \ "/repos/${GITHUB_REPOSITORY}/compare/v${MAJOR}.x...$GITHUB_SHA" --paginate \ | node tools/actions/lint-release-proposal-commit-list.mjs "$CHANGELOG_PATH" "$GITHUB_SHA" \ | while IFS= read -r PR_URL; do diff --git a/SECURITY.md b/SECURITY.md index 3a543216723c66..d44018e915ee80 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -320,9 +320,17 @@ the community they pose. * Avoid exposing low-level or dangerous APIs directly to untrusted users. * Examples of scenarios that are **not** Node.js vulnerabilities: - * Allowing untrusted users to register SQLite user-defined functions that can - perform arbitrary operations (e.g., closing database connections during query - execution, causing crashes or use-after-free conditions). + * Allowing untrusted users to register SQLite user-defined functions via + `node:sqlite` (`DatabaseSync`) that can perform arbitrary operations + (e.g., closing database connections during query execution, causing crashes + or use-after-free conditions). + * Loading SQLite extensions using the `allowExtension` option in + `DatabaseSync` — this option must be explicitly set to `true` by the + application, and enabling it is the application operator's responsibility. + * Using `node:sqlite` built-in SQL functions or pragmas (e.g., + `ATTACH DATABASE`) to read or write files — `DatabaseSync` operates with + the same file-system access as the process itself, and it is the + application's responsibility to restrict what SQL is executed. * Exposing `child_process.exec()` or similar APIs to untrusted users without proper input validation, allowing command injection. * Allowing untrusted users to control file paths passed to file system APIs @@ -362,6 +370,56 @@ the community they pose. responsibility to properly handle errors by attaching appropriate `'error'` event listeners to EventEmitters that may emit errors. +#### Permission Model Boundaries (`--permission`) + +The Node.js [Permission Model](https://nodejs.org/api/permissions.html) +(`--experimental-permission`) is an opt-in mechanism that limits which +resources a Node.js process may access. It is designed to reduce the blast +radius of mistakes in trusted application code, **not** to act as a security +boundary against intentional misuse or a compromised process. + +The following are **not** vulnerabilities in Node.js: + +* **Operator-controlled flags**: Behavior unlocked by flags the operator + explicitly passes (e.g., `--localstorage-file`) is the operator's + responsibility. The permission model does not restrict how Node.js behaves + when the operator intentionally configures it. + +* **`node:sqlite` and the permission model**: `DatabaseSync` operates with the + same file-system privileges as the process. Using SQL pragmas or built-in + SQLite mechanisms (e.g., `ATTACH DATABASE`) to access files does not bypass + the permission model — the permission model does not intercept SQL-level + file operations. + +* **Path resolution and symlinks**: `fs.realpathSync()`, `fs.realpath()`, and + similar functions resolve a path to its canonical form before the permission + check is applied. Accessing a file through a symlink that resolves to an + allowed path is the intended behavior, not a bypass. TOCTOU races on + symlinks that resolve within the allowed list are similarly not considered + permission model bypasses. + +* **`worker_threads` with modified `execArgv`**: Workers inherit the permission + restrictions of their parent process. Passing an empty or modified `execArgv` + to a worker does not grant it additional permissions. + +#### V8 Sandbox + +The V8 sandbox is an in-process isolation mechanism internal to V8 that is not +a Node.js security boundary. Node.js does not guarantee or document the V8 +sandbox as a security feature, and it is not enabled in a way that provides +security guarantees in production Node.js builds. Reports about escaping the V8 +sandbox are not considered Node.js vulnerabilities; they should be reported +directly to the [V8 project](https://v8.dev/docs/security-bugs). + +#### CRLF Injection in `writeEarlyHints()` + +`ServerResponse.writeEarlyHints()` accepts a `link` header value that is set +by the application. Passing arbitrary strings, including CRLF sequences, as +the `link` value is an application-level misuse of the API, not a Node.js +vulnerability. Node.js validates the structure of Early Hints per the HTTP spec +but does not sanitize free-form application data passed to it; that is the +application's responsibility. + ## Assessing experimental features reports Experimental features are eligible for security reports just like any other diff --git a/deps/merve/BUILD.gn b/deps/merve/BUILD.gn new file mode 100644 index 00000000000000..7bb318f8835dba --- /dev/null +++ b/deps/merve/BUILD.gn @@ -0,0 +1,14 @@ +############################################################################## +# # +# DO NOT EDIT THIS FILE! # +# # +############################################################################## + +# This file is used by GN for building, which is NOT the build system used for +# building official binaries. +# Please modify the gyp files if you are making changes to build system. + +import("unofficial.gni") + +merve_gn_build("merve") { +} diff --git a/deps/merve/unofficial.gni b/deps/merve/unofficial.gni new file mode 100644 index 00000000000000..dfb508d1d22f84 --- /dev/null +++ b/deps/merve/unofficial.gni @@ -0,0 +1,20 @@ +# This file is used by GN for building, which is NOT the build system used for +# building official binaries. +# Please edit the gyp files if you are making changes to build system. + +# The actual configurations are put inside a template in unofficial.gni to +# prevent accidental edits from contributors. +template("merve_gn_build") { + config("merve_config") { + include_dirs = [ "." ] + } + gypi_values = exec_script("../../tools/gypi_to_gn.py", + [ rebase_path("merve.gyp") ], + "scope", + [ "merve.gyp" ]) + source_set(target_name) { + forward_variables_from(invoker, "*") + public_configs = [ ":merve_config" ] + sources = gypi_values.merve_sources + } +} diff --git a/lib/_http_client.js b/lib/_http_client.js index 1358a441c15f90..c14e899dabbf04 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -27,6 +27,7 @@ const { Error, NumberIsFinite, ObjectAssign, + ObjectDefineProperty, ObjectKeys, ObjectSetPrototypeOf, ReflectApply, @@ -116,6 +117,7 @@ let debug = require('internal/util/debuglog').debuglog('http', (fn) => { const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/; const kError = Symbol('kError'); +const kPath = Symbol('kPath'); const kLenientAll = HTTPParser.kLenientAll | 0; const kLenientNone = HTTPParser.kLenientNone | 0; @@ -303,7 +305,7 @@ function ClientRequest(input, options, cb) { this.joinDuplicateHeaders = options.joinDuplicateHeaders; - this.path = options.path || '/'; + this[kPath] = options.path || '/'; if (cb) { this.once('response', cb); } @@ -446,6 +448,22 @@ function ClientRequest(input, options, cb) { ObjectSetPrototypeOf(ClientRequest.prototype, OutgoingMessage.prototype); ObjectSetPrototypeOf(ClientRequest, OutgoingMessage); +ObjectDefineProperty(ClientRequest.prototype, 'path', { + __proto__: null, + get() { + return this[kPath]; + }, + set(value) { + const path = String(value); + if (INVALID_PATH_REGEX.test(path)) { + throw new ERR_UNESCAPED_CHARACTERS('Request path'); + } + this[kPath] = path; + }, + configurable: true, + enumerable: true, +}); + ClientRequest.prototype._finish = function _finish() { OutgoingMessage.prototype._finish.call(this); if (hasObserver('http')) { diff --git a/lib/_http_server.js b/lib/_http_server.js index 1d1bc37c47e109..b042106d2e14da 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -55,6 +55,8 @@ const { kUniqueHeaders, parseUniqueHeadersOption, OutgoingMessage, + validateHeaderName, + validateHeaderValue, } = require('_http_outgoing'); const { kOutHeaders, @@ -333,13 +335,20 @@ ServerResponse.prototype.writeEarlyHints = function writeEarlyHints(hints, cb) { return; } + if (checkInvalidHeaderChar(link)) { + throw new ERR_INVALID_CHAR('header content', 'Link'); + } + head += 'Link: ' + link + '\r\n'; const keys = ObjectKeys(hints); for (let i = 0; i < keys.length; i++) { const key = keys[i]; if (key !== 'link') { - head += key + ': ' + hints[key] + '\r\n'; + validateHeaderName(key); + const value = hints[key]; + validateHeaderValue(key, value); + head += key + ': ' + value + '\r\n'; } } diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index a320736d1b6fd7..f5a47448ba3eb9 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1034,41 +1034,63 @@ function getExportsForCircularRequire(module) { return module.exports; } + /** - * Resolve a module request for CommonJS, invoking hooks from module.registerHooks() - * if necessary. + * Wraps result of Module._resolveFilename to include additional fields for hooks. + * See resolveForCJSWithHooks. * @param {string} specifier * @param {Module|undefined} parent * @param {boolean} isMain - * @param {boolean} shouldSkipModuleHooks + * @param {ResolveFilenameOptions} options * @returns {{url?: string, format?: string, parentURL?: string, filename: string}} */ -function resolveForCJSWithHooks(specifier, parent, isMain, shouldSkipModuleHooks) { - let defaultResolvedURL; - let defaultResolvedFilename; - let format; - - function defaultResolveImpl(specifier, parent, isMain, options) { - // For backwards compatibility, when encountering requests starting with node:, - // throw ERR_UNKNOWN_BUILTIN_MODULE on failure or return the normalized ID on success - // without going into Module._resolveFilename. - let normalized; - if (StringPrototypeStartsWith(specifier, 'node:')) { - normalized = BuiltinModule.normalizeRequirableId(specifier); - if (!normalized) { - throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier); - } - defaultResolvedURL = specifier; - format = 'builtin'; - return normalized; +function wrapResolveFilename(specifier, parent, isMain, options) { + const filename = Module._resolveFilename(specifier, parent, isMain, options).toString(); + return { __proto__: null, url: undefined, format: undefined, filename }; +} + +/** + * See resolveForCJSWithHooks. + * @param {string} specifier + * @param {Module|undefined} parent + * @param {boolean} isMain + * @param {ResolveFilenameOptions} options + * @returns {{url?: string, format?: string, parentURL?: string, filename: string}} + */ +function defaultResolveImplForCJSLoading(specifier, parent, isMain, options) { + // For backwards compatibility, when encountering requests starting with node:, + // throw ERR_UNKNOWN_BUILTIN_MODULE on failure or return the normalized ID on success + // without going into Module._resolveFilename. + let normalized; + if (StringPrototypeStartsWith(specifier, 'node:')) { + normalized = BuiltinModule.normalizeRequirableId(specifier); + if (!normalized) { + throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier); } - return Module._resolveFilename(specifier, parent, isMain, options).toString(); + return { __proto__: null, url: specifier, format: 'builtin', filename: normalized }; } + return wrapResolveFilename(specifier, parent, isMain, options); +} +/** + * Resolve a module request for CommonJS, invoking hooks from module.registerHooks() + * if necessary. + * @param {string} specifier + * @param {Module|undefined} parent + * @param {boolean} isMain + * @param {object} internalResolveOptions + * @param {boolean} internalResolveOptions.shouldSkipModuleHooks Whether to skip module hooks. + * @param {ResolveFilenameOptions} internalResolveOptions.requireResolveOptions Options from require.resolve(). + * Only used when it comes from require.resolve(). + * @returns {{url?: string, format?: string, parentURL?: string, filename: string}} + */ +function resolveForCJSWithHooks(specifier, parent, isMain, internalResolveOptions) { + const { requireResolveOptions, shouldSkipModuleHooks } = internalResolveOptions; + const defaultResolveImpl = requireResolveOptions ? + wrapResolveFilename : defaultResolveImplForCJSLoading; // Fast path: no hooks, just return simple results. if (!resolveHooks.length || shouldSkipModuleHooks) { - const filename = defaultResolveImpl(specifier, parent, isMain); - return { __proto__: null, url: defaultResolvedURL, filename, format }; + return defaultResolveImpl(specifier, parent, isMain, requireResolveOptions); } // Slow path: has hooks, do the URL conversions and invoke hooks with contexts. @@ -1097,27 +1119,25 @@ function resolveForCJSWithHooks(specifier, parent, isMain, shouldSkipModuleHooks } else { conditionSet = getCjsConditions(); } - defaultResolvedFilename = defaultResolveImpl(specifier, parent, isMain, { + + const result = defaultResolveImpl(specifier, parent, isMain, { __proto__: null, + paths: requireResolveOptions?.paths, conditions: conditionSet, }); + // If the default resolver does not return a URL, convert it for the public API. + result.url ??= convertCJSFilenameToURL(result.filename); - defaultResolvedURL = convertCJSFilenameToURL(defaultResolvedFilename); - return { __proto__: null, url: defaultResolvedURL }; + // Remove filename because it's not part of the public API. + // TODO(joyeecheung): maybe expose it in the public API to avoid re-conversion for users too. + return { __proto__: null, url: result.url, format: result.format }; } const resolveResult = resolveWithHooks(specifier, parentURL, /* importAttributes */ undefined, getCjsConditionsArray(), defaultResolve); - const { url } = resolveResult; - format = resolveResult.format; - - let filename; - if (url === defaultResolvedURL) { // Not overridden, skip the re-conversion. - filename = defaultResolvedFilename; - } else { - filename = convertURLToCJSFilename(url); - } - + const { url, format } = resolveResult; + // Convert the URL from the hook chain back to a filename for internal use. + const filename = convertURLToCJSFilename(url); const result = { __proto__: null, url, format, filename, parentURL }; debug('resolveForCJSWithHooks', specifier, parent?.id, isMain, shouldSkipModuleHooks, '->', result); return result; @@ -1212,10 +1232,10 @@ function loadBuiltinWithHooks(id, url, format) { * @param {string} request Specifier of module to load via `require` * @param {Module} parent Absolute path of the module importing the child * @param {boolean} isMain Whether the module is the main entry point - * @param {object|undefined} options Additional options for loading the module + * @param {object|undefined} internalResolveOptions Additional options for loading the module * @returns {object} */ -Module._load = function(request, parent, isMain, options = kEmptyObject) { +Module._load = function(request, parent, isMain, internalResolveOptions = kEmptyObject) { let relResolveCacheIdentifier; if (parent) { debug('Module._load REQUEST %s parent: %s', request, parent.id); @@ -1238,7 +1258,7 @@ Module._load = function(request, parent, isMain, options = kEmptyObject) { } } - const resolveResult = resolveForCJSWithHooks(request, parent, isMain, options.shouldSkipModuleHooks); + const resolveResult = resolveForCJSWithHooks(request, parent, isMain, internalResolveOptions); let { format } = resolveResult; const { url, filename } = resolveResult; diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 37eb267e154cc7..04c374c00cfc3e 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -722,13 +722,16 @@ class ModuleLoader { * `module.registerHooks()` hooks. * @param {string} [parentURL] See {@link resolve}. * @param {ModuleRequest} request See {@link resolve}. + * @param {boolean} [shouldSkipSyncHooks] Whether to skip the synchronous hooks registered by module.registerHooks(). + * This is used to maintain compatibility for the re-invented require.resolve (in imported CJS customized + * by module.register()`) which invokes the CJS resolution separately from the hook chain. * @returns {{ format: string, url: string }} */ - resolveSync(parentURL, request) { + resolveSync(parentURL, request, shouldSkipSyncHooks = false) { const specifier = `${request.specifier}`; const importAttributes = request.attributes ?? kEmptyObject; - if (syncResolveHooks.length) { + if (!shouldSkipSyncHooks && syncResolveHooks.length) { // Has module.registerHooks() hooks, chain the asynchronous hooks in the default step. return resolveWithSyncHooks(specifier, parentURL, importAttributes, this.#defaultConditions, this.#resolveAndMaybeBlockOnLoaderThread.bind(this)); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 8cf008d52ab380..2af52e4b9435ea 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -94,6 +94,8 @@ translators.set('module', function moduleStrategy(url, translateContext, parentU const { requestTypes: { kRequireInImportedCJS } } = require('internal/modules/esm/utils'); const kShouldSkipModuleHooks = { __proto__: null, shouldSkipModuleHooks: true }; +const kShouldNotSkipModuleHooks = { __proto__: null, shouldSkipModuleHooks: false }; + /** * Loads a CommonJS module via the ESM Loader sync CommonJS translator. * This translator creates its own version of the `require` function passed into CommonJS modules. @@ -164,14 +166,17 @@ function loadCJSModule(module, source, url, filename, isMain) { }; setOwnProperty(requireFn, 'resolve', function resolve(specifier) { if (!StringPrototypeStartsWith(specifier, 'node:')) { - const path = CJSModule._resolveFilename(specifier, module); - if (specifier !== path) { - specifier = `${pathToFileURL(path)}`; + const { + filename, url: resolvedURL, + } = resolveForCJSWithHooks(specifier, module, false, kShouldNotSkipModuleHooks); + if (specifier !== filename) { + specifier = resolvedURL ?? `${pathToFileURL(filename)}`; } } const request = { specifier, __proto__: null, attributes: kEmptyObject }; - const { url: resolvedURL } = cascadedLoader.resolveSync(url, request); + // Skip sync hooks in resolveSync since resolveForCJSWithHooks already ran them above. + const { url: resolvedURL } = cascadedLoader.resolveSync(url, request, /* shouldSkipSyncHooks */ true); return urlToFilename(resolvedURL); }); setOwnProperty(requireFn, 'main', process.mainModule); @@ -401,7 +406,7 @@ function cjsPreparseModuleExports(filename, source, format) { let resolved; let format; try { - ({ format, filename: resolved } = resolveForCJSWithHooks(reexport, module, false)); + ({ format, filename: resolved } = resolveForCJSWithHooks(reexport, module, false, kShouldNotSkipModuleHooks)); } catch (e) { debug(`Failed to resolve '${reexport}', skipping`, e); continue; diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index 01739fefd6a7f1..839ce9af4bb678 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -42,6 +42,7 @@ const { flushCompileCache, } = internalBinding('modules'); +const lazyCJSLoader = getLazy(() => require('internal/modules/cjs/loader')); let debug = require('internal/util/debuglog').debuglog('module', (fn) => { debug = fn; }); @@ -198,7 +199,23 @@ function makeRequireFunction(mod) { */ function resolve(request, options) { validateString(request, 'request'); - return Module._resolveFilename(request, mod, false, options); + const { resolveForCJSWithHooks } = lazyCJSLoader(); + // require.resolve() has different behaviors from the internal resolution used by + // Module._load: + // 1. When the request resolves to a non-existent built-in, it throws MODULE_NOT_FOUND + // instead of UNKNOWN_BUILTIN_MODULE. This is handled by resolveForCJSWithHooks. + // 2. If the request is a prefixed built-in, the returned value is also prefixed. This + // is handled below. + const { filename, url } = resolveForCJSWithHooks( + request, mod, /* isMain */ false, { + __proto__: null, + shouldSkipModuleHooks: false, + requireResolveOptions: options ?? {}, + }); + if (url === request && StringPrototypeStartsWith(request, 'node:')) { + return url; + } + return filename; } require.resolve = resolve; diff --git a/lib/internal/validators.js b/lib/internal/validators.js index b9845c538bb98f..110b045a063460 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -509,7 +509,7 @@ function validateUnion(value, name, union) { (not necessarily a valid URI reference) followed by zero or more link-params separated by semicolons. */ -const linkValueRegExp = /^(?:<[^>]*>)(?:\s*;\s*[^;"\s]+(?:=(")?[^;"\s]*\1)?)*$/; +const linkValueRegExp = /^(?:<[^>\r\n]*>)(?:\s*;\s*[^;"\s]+(?:=(")?[^;"\s]*\1)?)*$/; /** * @param {any} value diff --git a/node.gni b/node.gni index d4438f7fd61598..41f200189a34e1 100644 --- a/node.gni +++ b/node.gni @@ -26,8 +26,6 @@ declare_args() { # TODO(zcbenz): This is currently copied from configure.py, we should share # the list between configure.py and GN configurations. node_builtin_shareable_builtins = [ - "deps/cjs-module-lexer/lexer.js", - "deps/cjs-module-lexer/dist/lexer.js", "deps/undici/undici.js", "deps/amaro/dist/index.js", ] diff --git a/shell.nix b/shell.nix index ae2f15c7c80e25..c531642893b8ff 100644 --- a/shell.nix +++ b/shell.nix @@ -106,12 +106,6 @@ pkgs.mkShell { BUILD_WITH = if (ninja != null) then "ninja" else "make"; NINJA = pkgs.lib.optionalString (ninja != null) "${pkgs.lib.getExe ninja}"; - CI_SKIP_TESTS = pkgs.lib.concatStringsSep "," ( - pkgs.lib.optionals useSharedOpenSSL [ - # Path to the openssl.cnf is different from the expected one - "test-strace-openat-openssl" - ] - ); CONFIG_FLAGS = builtins.toString ( configureFlags ++ pkgs.lib.optional (useSeparateDerivationForV8 != false) "--without-bundled-v8" ); diff --git a/src/js_udp_wrap.cc b/src/js_udp_wrap.cc index 1c038d56ade9b5..1648d162b8f794 100644 --- a/src/js_udp_wrap.cc +++ b/src/js_udp_wrap.cc @@ -158,7 +158,7 @@ void JSUDPWrap::EmitReceived(const FunctionCallbackInfo<Value>& args) { int family = args[1].As<Int32>()->Value() == 4 ? AF_INET : AF_INET6; Utf8Value address(env->isolate(), args[2]); int port = args[3].As<Int32>()->Value(); - int flags = args[3].As<Int32>()->Value(); + int flags = args[4].As<Int32>()->Value(); sockaddr_storage addr; CHECK_EQ(sockaddr_for_family(family, *address, port, &addr), 0); diff --git a/test/fixtures/module-hooks/require-resolve-caller.js b/test/fixtures/module-hooks/require-resolve-caller.js new file mode 100644 index 00000000000000..94736e42d748c4 --- /dev/null +++ b/test/fixtures/module-hooks/require-resolve-caller.js @@ -0,0 +1,4 @@ +// Fixture CJS file that calls require.resolve and exports the result. +'use strict'; +const resolved = require.resolve('test-require-resolve-hook-target'); +module.exports = { resolved }; diff --git a/test/fixtures/module-hooks/require-resolve-paths-caller.js b/test/fixtures/module-hooks/require-resolve-paths-caller.js new file mode 100644 index 00000000000000..f18b1f37773507 --- /dev/null +++ b/test/fixtures/module-hooks/require-resolve-paths-caller.js @@ -0,0 +1,13 @@ +// Fixture CJS file that calls require.resolve with the paths option. +'use strict'; +const path = require('path'); +const fixturesDir = path.resolve(__dirname, '..', '..'); +const nodeModules = path.join(fixturesDir, 'node_modules'); + +// Use the paths option to resolve 'bar' from the fixtures node_modules. +const resolved = require.resolve('bar', { paths: [fixturesDir] }); +const expected = path.join(nodeModules, 'bar.js'); +if (resolved !== expected) { + throw new Error(`Expected ${expected}, got ${resolved}`); +} +module.exports = { resolved }; diff --git a/test/module-hooks/test-module-hooks-resolve-require-resolve-builtin.js b/test/module-hooks/test-module-hooks-resolve-require-resolve-builtin.js new file mode 100644 index 00000000000000..1adf73c48a575e --- /dev/null +++ b/test/module-hooks/test-module-hooks-resolve-require-resolve-builtin.js @@ -0,0 +1,25 @@ +'use strict'; + +// This tests that require.resolve() works with builtin redirection +// via resolve hooks registered with module.registerHooks(). + +require('../common'); +const assert = require('assert'); +const { registerHooks } = require('module'); + +const hook = registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier === 'assert') { + return { + url: 'node:zlib', + shortCircuit: true, + }; + } + return nextResolve(specifier, context); + }, +}); + +const resolved = require.resolve('assert'); +assert.strictEqual(resolved, 'zlib'); + +hook.deregister(); diff --git a/test/module-hooks/test-module-hooks-resolve-require-resolve-consistency.js b/test/module-hooks/test-module-hooks-resolve-require-resolve-consistency.js new file mode 100644 index 00000000000000..7a282248a08d83 --- /dev/null +++ b/test/module-hooks/test-module-hooks-resolve-require-resolve-consistency.js @@ -0,0 +1,38 @@ +'use strict'; + +// This tests that require.resolve() and require() both go through the same +// resolve hooks registered via module.registerHooks(). + +require('../common'); +const assert = require('assert'); +const { registerHooks } = require('module'); +const fixtures = require('../common/fixtures'); +const { pathToFileURL } = require('url'); + +const redirectedPath = fixtures.path('module-hooks', 'redirected-assert.js'); + +const resolvedSpecifiers = []; +const hook = registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier === 'test-consistency-target') { + resolvedSpecifiers.push(specifier); + return { + url: pathToFileURL(redirectedPath).href, + shortCircuit: true, + }; + } + return nextResolve(specifier, context); + }, +}); + +const resolveResult = require.resolve('test-consistency-target'); +const requireResult = require('test-consistency-target'); + +assert.strictEqual(resolveResult, redirectedPath); +assert.strictEqual(requireResult.exports_for_test, 'redirected assert'); +assert.deepStrictEqual(resolvedSpecifiers, [ + 'test-consistency-target', + 'test-consistency-target', +]); + +hook.deregister(); diff --git a/test/module-hooks/test-module-hooks-resolve-require-resolve-create-require.js b/test/module-hooks/test-module-hooks-resolve-require-resolve-create-require.js new file mode 100644 index 00000000000000..27b103c9ddb57c --- /dev/null +++ b/test/module-hooks/test-module-hooks-resolve-require-resolve-create-require.js @@ -0,0 +1,31 @@ +'use strict'; + +// This tests that require.resolve() from a require function returned by +// module.createRequire() goes through resolve hooks registered via +// module.registerHooks(). + +require('../common'); +const assert = require('assert'); +const { registerHooks, createRequire } = require('module'); +const fixtures = require('../common/fixtures'); +const { pathToFileURL } = require('url'); + +const redirectedPath = fixtures.path('module-hooks', 'redirected-assert.js'); + +const hook = registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier === 'test-create-require-resolve-target') { + return { + url: pathToFileURL(redirectedPath).href, + shortCircuit: true, + }; + } + return nextResolve(specifier, context); + }, +}); + +const customRequire = createRequire(__filename); +const resolved = customRequire.resolve('test-create-require-resolve-target'); +assert.strictEqual(resolved, redirectedPath); + +hook.deregister(); diff --git a/test/module-hooks/test-module-hooks-resolve-require-resolve-fallthrough.js b/test/module-hooks/test-module-hooks-resolve-require-resolve-fallthrough.js new file mode 100644 index 00000000000000..78cd09fb3890fc --- /dev/null +++ b/test/module-hooks/test-module-hooks-resolve-require-resolve-fallthrough.js @@ -0,0 +1,19 @@ +'use strict'; + +// This tests that require.resolve() falls through to default resolution +// when resolve hooks registered via module.registerHooks() don't override. + +require('../common'); +const assert = require('assert'); +const { registerHooks } = require('module'); + +const hook = registerHooks({ + resolve(specifier, context, nextResolve) { + return nextResolve(specifier, context); + }, +}); + +const resolved = require.resolve('assert'); +assert.strictEqual(resolved, 'assert'); + +hook.deregister(); diff --git a/test/module-hooks/test-module-hooks-resolve-require-resolve-imported-cjs.js b/test/module-hooks/test-module-hooks-resolve-require-resolve-imported-cjs.js new file mode 100644 index 00000000000000..d6fff2f8a7b956 --- /dev/null +++ b/test/module-hooks/test-module-hooks-resolve-require-resolve-imported-cjs.js @@ -0,0 +1,29 @@ +'use strict'; + +// This tests that require.resolve() in an imported CJS file goes through +// resolve hooks registered via module.registerHooks(). + +const common = require('../common'); +const assert = require('assert'); +const { registerHooks } = require('module'); +const fixtures = require('../common/fixtures'); +const { pathToFileURL } = require('url'); + +const redirectedPath = fixtures.path('module-hooks', 'redirected-assert.js'); + +const hook = registerHooks({ + resolve: common.mustCall((specifier, context, nextResolve) => { + if (specifier === 'test-require-resolve-hook-target') { + return { + url: pathToFileURL(redirectedPath).href, + shortCircuit: true, + }; + } + return nextResolve(specifier, context); + }, 2), +}); + +import('../fixtures/module-hooks/require-resolve-caller.js').then(common.mustCall((ns) => { + assert.strictEqual(ns.default.resolved, redirectedPath); + hook.deregister(); +})); diff --git a/test/module-hooks/test-module-hooks-resolve-require-resolve-loaded-with-source.js b/test/module-hooks/test-module-hooks-resolve-require-resolve-loaded-with-source.js new file mode 100644 index 00000000000000..de66beda5b2e6c --- /dev/null +++ b/test/module-hooks/test-module-hooks-resolve-require-resolve-loaded-with-source.js @@ -0,0 +1,35 @@ +'use strict'; + +// This tests that require.resolve() in a CJS file loaded via the re-invented +// require (triggered when module.register() installs an async loader that +// provides source for a CJS file) still goes through resolve hooks registered +// via module.registerHooks(). + +const common = require('../common'); +const assert = require('assert'); +const { register, registerHooks } = require('module'); +const fixtures = require('../common/fixtures'); +const { pathToFileURL } = require('url'); + +const redirectedPath = fixtures.path('module-hooks', 'redirected-assert.js'); + +// Register an async loader that provides source for CJS files, which triggers +// the re-invented require path. +register(fixtures.fileURL('module-hooks', 'logger-async-hooks.mjs')); + +const hook = registerHooks({ + resolve: common.mustCall((specifier, context, nextResolve) => { + if (specifier === 'test-require-resolve-hook-target') { + return { + url: pathToFileURL(redirectedPath).href, + shortCircuit: true, + }; + } + return nextResolve(specifier, context); + }, 2), +}); + +import('../fixtures/module-hooks/require-resolve-caller.js').then(common.mustCall((ns) => { + assert.strictEqual(ns.default.resolved, redirectedPath); + hook.deregister(); +})); diff --git a/test/module-hooks/test-module-hooks-resolve-require-resolve-paths.js b/test/module-hooks/test-module-hooks-resolve-require-resolve-paths.js new file mode 100644 index 00000000000000..03eafadeb100fb --- /dev/null +++ b/test/module-hooks/test-module-hooks-resolve-require-resolve-paths.js @@ -0,0 +1,29 @@ +'use strict'; + +// This tests that require.resolve() with the paths option work transparently +// when resolve hooks are registered via module.registerHooks(). + +require('../common'); +const assert = require('assert'); +const path = require('path'); +const { registerHooks } = require('module'); +const fixtures = require('../common/fixtures'); + +const nodeModules = path.join(fixtures.path(), 'node_modules'); +const resolveCallCount = []; + +const hook = registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier === 'bar') { + resolveCallCount.push(specifier); + } + return nextResolve(specifier, context); + }, +}); + +// require.resolve with paths option should go through hooks and resolve correctly. +const resolved = require.resolve('bar', { paths: [fixtures.path()] }); +assert.strictEqual(resolved, path.join(nodeModules, 'bar.js')); +assert.deepStrictEqual(resolveCallCount, ['bar']); + +hook.deregister(); diff --git a/test/module-hooks/test-module-hooks-resolve-require-resolve-redirect.js b/test/module-hooks/test-module-hooks-resolve-require-resolve-redirect.js new file mode 100644 index 00000000000000..bbf058bae3c41c --- /dev/null +++ b/test/module-hooks/test-module-hooks-resolve-require-resolve-redirect.js @@ -0,0 +1,29 @@ +'use strict'; + +// This tests that require.resolve() invokes resolve hooks registered +// via module.registerHooks() and can redirect to a different file. + +require('../common'); +const assert = require('assert'); +const { registerHooks } = require('module'); +const fixtures = require('../common/fixtures'); +const { pathToFileURL } = require('url'); + +const redirectedPath = fixtures.path('module-hooks', 'redirected-assert.js'); + +const hook = registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier === 'test-resolve-target') { + return { + url: pathToFileURL(redirectedPath).href, + shortCircuit: true, + }; + } + return nextResolve(specifier, context); + }, +}); + +const resolved = require.resolve('test-resolve-target'); +assert.strictEqual(resolved, redirectedPath); + +hook.deregister(); diff --git a/test/parallel/test-http-client-path-toctou.js b/test/parallel/test-http-client-path-toctou.js new file mode 100644 index 00000000000000..2975ba363d2614 --- /dev/null +++ b/test/parallel/test-http-client-path-toctou.js @@ -0,0 +1,68 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const http = require('http'); + +// Test that mutating req.path after construction to include +// invalid characters (e.g. CRLF) throws ERR_UNESCAPED_CHARACTERS. +// Regression test for a TOCTOU vulnerability where path was only +// validated at construction time but could be mutated before +// _implicitHeader() flushed it to the socket. + +// Use a createConnection that returns nothing to avoid actual connection. +const req = new http.ClientRequest({ + host: '127.0.0.1', + port: 1, + path: '/valid', + method: 'GET', + createConnection: () => {}, +}); + +// Attempting to set path with CRLF must throw +assert.throws( + () => { req.path = '/evil\r\nX-Injected: true\r\n\r\n'; }, + { + code: 'ERR_UNESCAPED_CHARACTERS', + name: 'TypeError', + message: 'Request path contains unescaped characters', + } +); + +// Path must be unchanged after failed mutation +assert.strictEqual(req.path, '/valid'); + +// Attempting to set path with lone CR must throw +assert.throws( + () => { req.path = '/evil\rpath'; }, + { + code: 'ERR_UNESCAPED_CHARACTERS', + name: 'TypeError', + } +); + +// Attempting to set path with lone LF must throw +assert.throws( + () => { req.path = '/evil\npath'; }, + { + code: 'ERR_UNESCAPED_CHARACTERS', + name: 'TypeError', + } +); + +// Attempting to set path with null byte must throw +assert.throws( + () => { req.path = '/evil\0path'; }, + { + code: 'ERR_UNESCAPED_CHARACTERS', + name: 'TypeError', + } +); + +// Valid path mutation should succeed +req.path = '/also-valid'; +assert.strictEqual(req.path, '/also-valid'); + +req.path = '/path?query=1&other=2'; +assert.strictEqual(req.path, '/path?query=1&other=2'); + +req.destroy(); diff --git a/test/parallel/test-http-early-hints-invalid-argument.js b/test/parallel/test-http-early-hints-invalid-argument.js index f776bcafa40ed3..edf613614bc721 100644 --- a/test/parallel/test-http-early-hints-invalid-argument.js +++ b/test/parallel/test-http-early-hints-invalid-argument.js @@ -47,3 +47,44 @@ const testResBody = 'response content\n'; req.on('information', common.mustNotCall()); })); } + +{ + const server = http.createServer(common.mustCall((req, res) => { + debug('Server sending early hints with CRLF injection...'); + + assert.throws(() => { + res.writeEarlyHints({ + 'link': '</styles.css>; rel=preload; as=style', + 'X-Custom': 'valid\r\nSet-Cookie: session=evil', + }); + }, (err) => err.code === 'ERR_INVALID_CHAR'); + + assert.throws(() => { + res.writeEarlyHints({ + 'link': '</styles.css>; rel=preload; as=style', + 'X-Custom\r\nSet-Cookie: session=evil': 'value', + }); + }, (err) => err.code === 'ERR_INVALID_HTTP_TOKEN'); + + assert.throws(() => { + res.writeEarlyHints({ + link: '</styles.css\r\nSet-Cookie: session=evil>; rel=preload; as=style', + }); + }, (err) => err.code === 'ERR_INVALID_ARG_VALUE'); + + debug('Server sending full response...'); + res.end(testResBody); + server.close(); + })); + + server.listen(0, common.mustCall(() => { + const req = http.request({ + port: server.address().port, path: '/' + }); + + req.end(); + debug('Client sending request...'); + + req.on('information', common.mustNotCall()); + })); +} diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index b4600113d1c536..c325abb6b4ec75 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -608,10 +608,10 @@ const errorTests = [ // access to internal modules without the --expose-internals flag. { // Shrink the stack trace to avoid having to update this test whenever the - // implementation of require() changes. It's set to 4 because somehow setting it + // implementation of require() changes. It's set to 5 because somehow setting it // to a lower value breaks the error formatting and the message becomes // "Uncaught [Error...", which is probably a bug(?). - send: 'Error.stackTraceLimit = 4; require("internal/repl")', + send: 'Error.stackTraceLimit = 5; require("internal/repl")', expect: [ /^Uncaught Error: Cannot find module 'internal\/repl'/, /^Require stack:/, @@ -619,6 +619,7 @@ const errorTests = [ /^ {4}at .*/, // Some stack frame that we have to capture otherwise error message is buggy. /^ {4}at .*/, // Some stack frame that we have to capture otherwise error message is buggy. /^ {4}at .*/, // Some stack frame that we have to capture otherwise error message is buggy. + /^ {4}at .*/, // Some stack frame that we have to capture otherwise error message is buggy. " code: 'MODULE_NOT_FOUND',", " requireStack: [ '<repl>' ]", '}', diff --git a/unofficial.gni b/unofficial.gni index f654a15da347a3..aa78f9ce60c043 100644 --- a/unofficial.gni +++ b/unofficial.gni @@ -160,6 +160,7 @@ template("node_gn_build") { "deps/cares", "deps/histogram", "deps/llhttp", + "deps/merve", "deps/nbytes", "deps/nghttp2", "deps/ngtcp2",