diff --git a/lib/internal/main/watch_mode.js b/lib/internal/main/watch_mode.js index 06c2c8602da444..0d79d6ecf9dc96 100644 --- a/lib/internal/main/watch_mode.js +++ b/lib/internal/main/watch_mode.js @@ -7,6 +7,7 @@ const { ArrayPrototypePush, ArrayPrototypePushApply, ArrayPrototypeSlice, + StringPrototypeIncludes, StringPrototypeStartsWith, } = primordials; @@ -18,7 +19,7 @@ const { triggerUncaughtException, exitCodes: { kNoFailure }, } = internalBinding('errors'); -const { getOptionValue } = require('internal/options'); +const { getOptionValue, parseNodeOptionsEnvVar } = require('internal/options'); const { FilesWatcher } = require('internal/watch_mode/files_watcher'); const { green, blue, red, white, clear } = require('internal/util/colors'); const { convertToValidSignal } = require('internal/util'); @@ -84,6 +85,37 @@ for (let i = 0; i < process.execArgv.length; i++) { ArrayPrototypePushApply(argsWithoutWatchOptions, kCommand); +// Strip watch-related flags from NODE_OPTIONS to prevent infinite loop +// when NODE_OPTIONS contains --watch (see issue #61740). +const kNodeOptions = process.env.NODE_OPTIONS; +let cleanNodeOptions = kNodeOptions; +if (kNodeOptions != null) { + const keep = []; + const parts = parseNodeOptionsEnvVar(kNodeOptions); + for (let i = 0; i < parts.length; i++) { + const part = parts[i]; + if (part === '--watch' || + part === '--watch-preserve-output' || + StringPrototypeStartsWith(part, '--watch=') || + StringPrototypeStartsWith(part, '--watch-preserve-output=') || + StringPrototypeStartsWith(part, '--watch-path=') || + StringPrototypeStartsWith(part, '--watch-kill-signal=')) { + continue; + } + if (part === '--watch-path' || part === '--watch-kill-signal') { + // Skip the flag and its separate value argument + i++; + continue; + } + // The C++ tokenizer strips quotes during parsing, so values that + // originally contained spaces (e.g. --require "./path with spaces/f.js") + // need to be re-quoted before rejoining into a single string, otherwise + // the child's C++ parser would split them into separate tokens. + ArrayPrototypePush(keep, StringPrototypeIncludes(part, ' ') ? `"${part}"` : part); + } + cleanNodeOptions = ArrayPrototypeJoin(keep, ' '); +} + const watcher = new FilesWatcher({ debounce: 200, mode: kShouldFilterModules ? 'filter' : 'all' }); ArrayPrototypeForEach(kWatchedPaths, (p) => watcher.watchPath(p)); @@ -99,6 +131,7 @@ function start() { env: { ...process.env, WATCH_REPORT_DEPENDENCIES: '1', + NODE_OPTIONS: cleanNodeOptions, }, }); watcher.watchChildProcessModules(child); diff --git a/lib/internal/options.js b/lib/internal/options.js index 92993d037fb653..9366993f8f1a14 100644 --- a/lib/internal/options.js +++ b/lib/internal/options.js @@ -16,6 +16,7 @@ const { getEmbedderOptions: getEmbedderOptionsFromBinding, getEnvOptionsInputType, getNamespaceOptionsInputType, + parseNodeOptionsEnvVar, } = internalBinding('options'); let warnOnAllowUnauthorized = true; @@ -172,5 +173,6 @@ module.exports = { getAllowUnauthorized, getEmbedderOptions, generateConfigJsonSchema, + parseNodeOptionsEnvVar, refreshOptions, }; diff --git a/src/node_options.cc b/src/node_options.cc index d48641ae3ffe07..9a01cba62214b6 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -2096,6 +2096,28 @@ void GetOptionsAsFlags(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result); } +void ParseNodeOptionsEnvVarBinding(const FunctionCallbackInfo& args) { + Isolate* isolate = args.GetIsolate(); + Local context = isolate->GetCurrentContext(); + + Utf8Value node_options(isolate, args[0]); + std::string options_str(*node_options, node_options.length()); + + std::vector errors; + std::vector result = + ParseNodeOptionsEnvVar(options_str, &errors); + + if (!errors.empty()) { + Environment* env = Environment::GetCurrent(context); + env->ThrowError(errors[0].c_str()); + return; + } + + Local v8_result; + CHECK(ToV8Value(context, result).ToLocal(&v8_result)); + args.GetReturnValue().Set(v8_result); +} + void Initialize(Local target, Local unused, Local context, @@ -2116,6 +2138,8 @@ void Initialize(Local target, target, "getNamespaceOptionsInputType", GetNamespaceOptionsInputType); + SetMethodNoSideEffect( + context, target, "parseNodeOptionsEnvVar", ParseNodeOptionsEnvVarBinding); Local env_settings = Object::New(isolate); NODE_DEFINE_CONSTANT(env_settings, kAllowedInEnvvar); NODE_DEFINE_CONSTANT(env_settings, kDisallowedInEnvvar); @@ -2144,6 +2168,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(GetEmbedderOptions); registry->Register(GetEnvOptionsInputType); registry->Register(GetNamespaceOptionsInputType); + registry->Register(ParseNodeOptionsEnvVarBinding); } } // namespace options_parser diff --git a/test/parallel/test-options-binding.js b/test/parallel/test-options-binding.js index 5c910360d00fa4..71ebbd5939dc14 100644 --- a/test/parallel/test-options-binding.js +++ b/test/parallel/test-options-binding.js @@ -3,7 +3,7 @@ const common = require('../common'); const assert = require('assert'); -const { getOptionValue } = require('internal/options'); +const { getOptionValue, parseNodeOptionsEnvVar } = require('internal/options'); Map.prototype.get = common.mustNotCall('`getOptionValue` must not call user-mutable method'); @@ -14,3 +14,30 @@ assert.strictEqual(getOptionValue('--nonexistent-option'), undefined); // Make the test common global leak test happy. delete Object.prototype['--nonexistent-option']; + +// parseNodeOptionsEnvVar tokenizes a NODE_OPTIONS-style string. +assert.deepStrictEqual( + parseNodeOptionsEnvVar('--max-old-space-size=4096 --no-warnings'), + ['--max-old-space-size=4096', '--no-warnings'] +); + +// Quoted strings are unquoted during parsing. +assert.deepStrictEqual( + parseNodeOptionsEnvVar('--require "file with spaces.js"'), + ['--require', 'file with spaces.js'] +); + +// Empty string returns an empty array. +assert.deepStrictEqual(parseNodeOptionsEnvVar(''), []); + +// Throws on unterminated string. +assert.throws( + () => parseNodeOptionsEnvVar('--require "unterminated'), + { name: 'Error', message: /unterminated string/ } +); + +// Throws on invalid escape at end of string. +assert.throws( + () => parseNodeOptionsEnvVar('--require "foo\\'), + { name: 'Error', message: /invalid escape/ } +); diff --git a/test/sequential/test-watch-mode.mjs b/test/sequential/test-watch-mode.mjs index a5cac129ad1c21..4a8ce120c898cc 100644 --- a/test/sequential/test-watch-mode.mjs +++ b/test/sequential/test-watch-mode.mjs @@ -1,6 +1,7 @@ import * as common from '../common/index.mjs'; import tmpdir from '../common/tmpdir.js'; import assert from 'node:assert'; +import os from 'node:os'; import path from 'node:path'; import { execPath } from 'node:process'; import { describe, it } from 'node:test'; @@ -922,4 +923,109 @@ process.on('message', (message) => { await done(); } }); + + it('should strip all watch flags from NODE_OPTIONS in child process', async () => { + const file = createTmpFile('console.log(process.env.NODE_OPTIONS);'); + const nodeOptions = [ + '--watch', + '--watch=true', + '--watch-path=./src', + '--watch-path', './test', + '--watch-preserve-output', + '--watch-preserve-output=true', + '--watch-kill-signal=SIGKILL', + '--watch-kill-signal', 'SIGINT', + '--max-old-space-size=4096', + '--no-warnings', + ].join(' '); + const { done, restart } = runInBackground({ + args: ['--watch', file], + options: { + env: { ...process.env, NODE_OPTIONS: nodeOptions }, + }, + }); + + try { + const { stdout, stderr } = await restart(); + + assert.strictEqual(stderr, ''); + const nodeOptionsLine = stdout.find((line) => line.includes('--max-old-space-size')); + assert.ok(nodeOptionsLine); + assert.strictEqual(nodeOptionsLine, '--max-old-space-size=4096 --no-warnings'); + } finally { + await done(); + } + }); + + it('should not strip --watch when it appears inside a quoted NODE_OPTIONS value', async () => { + // Use /tmp to avoid CI directories with special characters (e.g. ") + // that would break NODE_OPTIONS parsing. + const watchDir = path.join(os.tmpdir(), 'test for --watch parsing'); + mkdirSync(watchDir, { recursive: true }); + const reqFile = path.join(watchDir, 'req.cjs'); + writeFileSync(reqFile, 'globalThis.requiredOk = true;'); + + const file = createTmpFile('console.log("required:" + !!globalThis.requiredOk);'); + const nodeOptions = `--watch --require "${reqFile}"`; + const { done, restart } = runInBackground({ + args: ['--watch', file], + options: { + env: { ...process.env, NODE_OPTIONS: nodeOptions }, + }, + }); + + try { + const { stdout, stderr } = await restart(); + + assert.strictEqual(stderr, ''); + assert.ok(stdout.some((line) => line.includes('required:true'))); + } finally { + await done(); + } + }); + + it('should handle NODE_OPTIONS containing only watch flags', async () => { + const file = createTmpFile('console.log(JSON.stringify(process.env.NODE_OPTIONS));'); + const { done, restart } = runInBackground({ + args: ['--watch', file], + options: { + env: { ...process.env, NODE_OPTIONS: '--watch' }, + }, + }); + + try { + const { stdout, stderr } = await restart(); + + assert.strictEqual(stderr, ''); + assert.ok(stdout.some((line) => line.includes('""'))); + } finally { + await done(); + } + }); + + it('should strip multiple --watch-path entries from NODE_OPTIONS', async () => { + const file = createTmpFile('console.log(process.env.NODE_OPTIONS);'); + // Use /tmp to avoid CI directories with special characters (e.g. ") + // that would break NODE_OPTIONS parsing. + const dirA = path.join(os.tmpdir(), 'node-watch-path-a'); + const dirB = path.join(os.tmpdir(), 'node-watch-path-b'); + mkdirSync(dirA, { recursive: true }); + mkdirSync(dirB, { recursive: true }); + const nodeOptions = `--watch --watch-path=${dirA} --watch-path ${dirB} --no-warnings`; + const { done, restart } = runInBackground({ + args: ['--watch', file], + options: { + env: { ...process.env, NODE_OPTIONS: nodeOptions }, + }, + }); + + try { + const { stdout, stderr } = await restart(); + + assert.strictEqual(stderr, ''); + assert.ok(stdout.some((line) => line === '--no-warnings')); + } finally { + await done(); + } + }); });