From bb649d43ab1535ba1b1434dd68f059a87f68a1e9 Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sat, 7 Mar 2026 06:51:38 +0100 Subject: [PATCH 1/4] watch: strip watch flags from NODE_OPTIONS in child process Signed-off-by: marcopiraccini --- lib/internal/main/watch_mode.js | 28 ++++++++++++++++++++++++ test/sequential/test-watch-mode.mjs | 33 +++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/lib/internal/main/watch_mode.js b/lib/internal/main/watch_mode.js index 06c2c8602da444..6340bcda9460ae 100644 --- a/lib/internal/main/watch_mode.js +++ b/lib/internal/main/watch_mode.js @@ -7,6 +7,7 @@ const { ArrayPrototypePush, ArrayPrototypePushApply, ArrayPrototypeSlice, + RegExpPrototypeSymbolSplit, StringPrototypeStartsWith, } = primordials; @@ -84,6 +85,32 @@ for (let i = 0; i < process.execArgv.length; i++) { ArrayPrototypePushApply(argsWithoutWatchOptions, kCommand); +const kNodeOptions = process.env.NODE_OPTIONS; +let cleanNodeOptions = kNodeOptions; +if (kNodeOptions != null) { + const nodeOptionsArgs = []; + const parts = RegExpPrototypeSymbolSplit(/\s+/, kNodeOptions); + for (let i = 0; i < parts.length; i++) { + const part = parts[i]; + if (part === '') continue; + 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') { + // These flags take a separate value argument + i++; + continue; + } + ArrayPrototypePush(nodeOptionsArgs, part); + } + cleanNodeOptions = ArrayPrototypeJoin(nodeOptionsArgs, ' '); +} + const watcher = new FilesWatcher({ debounce: 200, mode: kShouldFilterModules ? 'filter' : 'all' }); ArrayPrototypeForEach(kWatchedPaths, (p) => watcher.watchPath(p)); @@ -99,6 +126,7 @@ function start() { env: { ...process.env, WATCH_REPORT_DEPENDENCIES: '1', + NODE_OPTIONS: cleanNodeOptions, }, }); watcher.watchChildProcessModules(child); diff --git a/test/sequential/test-watch-mode.mjs b/test/sequential/test-watch-mode.mjs index a5cac129ad1c21..fa2fb323565218 100644 --- a/test/sequential/test-watch-mode.mjs +++ b/test/sequential/test-watch-mode.mjs @@ -922,4 +922,37 @@ 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', // bare boolean + '--watch=true', // boolean with =value + '--watch-path=./src', // string with =value + '--watch-path', './test', // String with space-separated value + '--watch-preserve-output', // bare boolean + '--watch-preserve-output=true', // boolean with =value + '--watch-kill-signal=SIGKILL', // string with =value + '--watch-kill-signal', 'SIGINT', // String with space-separated value + '--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(); + } + }); }); From 125997bf331556e91fd86818d8f28198dd083f63 Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sun, 8 Mar 2026 16:22:11 +0100 Subject: [PATCH 2/4] watch: strip watch flags from NODE_OPTIONS in child process Signed-off-by: marcopiraccini --- lib/internal/main/watch_mode.js | 21 +++++++++++++-------- lib/internal/options.js | 2 ++ src/node_options.cc | 28 ++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/lib/internal/main/watch_mode.js b/lib/internal/main/watch_mode.js index 6340bcda9460ae..0d79d6ecf9dc96 100644 --- a/lib/internal/main/watch_mode.js +++ b/lib/internal/main/watch_mode.js @@ -7,7 +7,7 @@ const { ArrayPrototypePush, ArrayPrototypePushApply, ArrayPrototypeSlice, - RegExpPrototypeSymbolSplit, + StringPrototypeIncludes, StringPrototypeStartsWith, } = primordials; @@ -19,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'); @@ -85,14 +85,15 @@ 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 nodeOptionsArgs = []; - const parts = RegExpPrototypeSymbolSplit(/\s+/, kNodeOptions); + const keep = []; + const parts = parseNodeOptionsEnvVar(kNodeOptions); for (let i = 0; i < parts.length; i++) { const part = parts[i]; - if (part === '') continue; if (part === '--watch' || part === '--watch-preserve-output' || StringPrototypeStartsWith(part, '--watch=') || @@ -102,13 +103,17 @@ if (kNodeOptions != null) { continue; } if (part === '--watch-path' || part === '--watch-kill-signal') { - // These flags take a separate value argument + // Skip the flag and its separate value argument i++; continue; } - ArrayPrototypePush(nodeOptionsArgs, part); + // 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(nodeOptionsArgs, ' '); + cleanNodeOptions = ArrayPrototypeJoin(keep, ' '); } const watcher = new FilesWatcher({ debounce: 200, mode: kShouldFilterModules ? 'filter' : 'all' }); 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..c90a8c100a77cf 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -2096,6 +2096,29 @@ 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 +2139,10 @@ 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 +2171,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(GetEmbedderOptions); registry->Register(GetEnvOptionsInputType); registry->Register(GetNamespaceOptionsInputType); + registry->Register(ParseNodeOptionsEnvVarBinding); } } // namespace options_parser From 6bcc880f82ade591168658be45e42f587c4b4924 Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sun, 8 Mar 2026 16:43:50 +0100 Subject: [PATCH 3/4] fixup Signed-off-by: marcopiraccini --- src/node_options.cc | 9 +-- test/parallel/test-options-binding.js | 29 ++++++++- test/sequential/test-watch-mode.mjs | 86 ++++++++++++++++++++++++--- 3 files changed, 109 insertions(+), 15 deletions(-) diff --git a/src/node_options.cc b/src/node_options.cc index c90a8c100a77cf..9a01cba62214b6 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -2096,8 +2096,7 @@ void GetOptionsAsFlags(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result); } -void ParseNodeOptionsEnvVarBinding( - const FunctionCallbackInfo& args) { +void ParseNodeOptionsEnvVarBinding(const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); Local context = isolate->GetCurrentContext(); @@ -2139,10 +2138,8 @@ void Initialize(Local target, target, "getNamespaceOptionsInputType", GetNamespaceOptionsInputType); - SetMethodNoSideEffect(context, - target, - "parseNodeOptionsEnvVar", - ParseNodeOptionsEnvVarBinding); + SetMethodNoSideEffect( + context, target, "parseNodeOptionsEnvVar", ParseNodeOptionsEnvVarBinding); Local env_settings = Object::New(isolate); NODE_DEFINE_CONSTANT(env_settings, kAllowedInEnvvar); NODE_DEFINE_CONSTANT(env_settings, kDisallowedInEnvvar); 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 fa2fb323565218..999a50c98bc251 100644 --- a/test/sequential/test-watch-mode.mjs +++ b/test/sequential/test-watch-mode.mjs @@ -926,14 +926,14 @@ process.on('message', (message) => { 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', // bare boolean - '--watch=true', // boolean with =value - '--watch-path=./src', // string with =value - '--watch-path', './test', // String with space-separated value - '--watch-preserve-output', // bare boolean - '--watch-preserve-output=true', // boolean with =value - '--watch-kill-signal=SIGKILL', // string with =value - '--watch-kill-signal', 'SIGINT', // String with space-separated value + '--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(' '); @@ -955,4 +955,74 @@ process.on('message', (message) => { await done(); } }); + + it('should not strip --watch when it appears inside a quoted NODE_OPTIONS value', async () => { + const projectDir = tmpdir.resolve('project-watch-quoted'); + mkdirSync(projectDir); + const watchDir = path.join(projectDir, 'test for --watch parsing'); + mkdirSync(watchDir); + 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);'); + const dirA = tmpdir.resolve('watch-path-a'); + const dirB = tmpdir.resolve('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(); + } + }); }); From 91e21a94ef74375d0e99acd847ccf0b65c11321a Mon Sep 17 00:00:00 2001 From: marcopiraccini Date: Sun, 8 Mar 2026 18:50:21 +0100 Subject: [PATCH 4/4] fixup Signed-off-by: marcopiraccini --- test/sequential/test-watch-mode.mjs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/test/sequential/test-watch-mode.mjs b/test/sequential/test-watch-mode.mjs index 999a50c98bc251..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'; @@ -957,10 +958,10 @@ process.on('message', (message) => { }); it('should not strip --watch when it appears inside a quoted NODE_OPTIONS value', async () => { - const projectDir = tmpdir.resolve('project-watch-quoted'); - mkdirSync(projectDir); - const watchDir = path.join(projectDir, 'test for --watch parsing'); - mkdirSync(watchDir); + // 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;'); @@ -1004,8 +1005,10 @@ process.on('message', (message) => { it('should strip multiple --watch-path entries from NODE_OPTIONS', async () => { const file = createTmpFile('console.log(process.env.NODE_OPTIONS);'); - const dirA = tmpdir.resolve('watch-path-a'); - const dirB = tmpdir.resolve('watch-path-b'); + // 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`;