From e3c9a873dcd749068009d2a07a17cb3a4ecc1e96 Mon Sep 17 00:00:00 2001 From: Felix-Ayush <67006255+Ayush7614@users.noreply.github.com> Date: Mon, 26 Oct 2026 02:19:07 +0530 Subject: [PATCH] fix(utils): model Node's NODE_OPTIONS grammar in hasNodeOption (#2188) * fix(utils): make hasNodeOption handle --flag=value and quoted NODE_OPTIONS - Match exact flag or flag with =value suffix to support --max-old-space-size=4096, --inspect=0.0.0.0:9229 etc. - Strip surrounding single/double quotes for quoted NODE_OPTIONS tokens - Avoid false positives where --inspect incorrectly matched --inspect-brk - Filter empty tokens from whitespace split Fixes edge case where hasNodeOption('--max-old-space-size') returned false when NODE_OPTIONS contained '--max-old-space-size=4096' * fix(utils): align hasNodeOption with Node lexer, add regression tests - Only strip double quotes (Node's NODE_OPTIONS parser treats single quotes as literal) so NODE_OPTIONS="'--use-system-ca'" correctly returns false and does not incorrectly enable system CA roots (covers 145 vs 302 cert divergence reported in review) - Keep exact / --flag=value matching, double-quoted positives, prefix boundary (--inspect vs --inspect-brk), whitespace filtering - Add table-driven tests for both CA flags (--use-system-ca, --use-openssl-ca) covering exact, equals, double-quoted positive, single-quoted negative, repeated whitespace, and --inspect boundary; restores process.env.NODE_OPTIONS after each case Fixes CodeRabbit docstring coverage and jatmn P2 quote-grammar issue in PR #2188 * fix(utils): quote-aware tokenization for hasNodeOption Parse NODE_OPTIONS respecting double quotes so option-like text inside a quoted value does not create spurious flags (e.g. --conditions "foo --use-system-ca bar" must not match --use-system-ca). Verified on Node 22.23.2 that --expose-gc inside a quoted --conditions value does not enable GC while an explicit flag does. Add table-driven cases for this grammar and keep previous single-quote literal + double-quote handling and prefix boundary fixes. Addresses CodeRabbit quote-grammar issue on c93a679 * fix(utils): precise NODE_OPTIONS parsing with escapes and value boundaries - Mirror Node's ParseNodeOptionsEnvVar: handle backslash escapes only inside double quotes, strip quotes, treat single quotes as literal, and correctly handle unterminated strings - Prevent false positives where option values look like flags: --conditions "--use-system-ca" and --conditions "foo \" --use-system-ca bar" no longer count as --use-system-ca - ALWAYS_CONSUMES_NEXT for --conditions/-C (value may be flag-like), VALUE_TAKING for other string options only when next token does not look like flag to avoid --inspect --use-system-ca false negatives - Add table-driven cases for exact --use-system-ca value, escaped-quote value, and equals form; 43 tests now pass Addresses CodeRabbit probe for quoted condition values and escaped quotes * fix(utils): fail closed on malformed NODE_OPTIONS and order CA negations * fix(utils): match Node tab grammar and fail closed on flag-like values * fix(utils): finish required-value validation and audit boolean option roles - Fail closed on missing-at-EOF, empty inline (=), and flag-like separate values for all modeled required-value options, before any CA matching, in both target-before-error and error-before-target directions (Node rejects the whole NODE_OPTIONS stream). - Add missing required-value aliases --loader, --inspect-port, --inspect-publish-uid; --conditions/-C now also fails closed on flag-like values (verified: --conditions -x exits 9). - Remove --experimental-import-meta-resolve from VALUE_TAKING: it is a no-value boolean in Node 22, so a following CA flag stays visible. - Update tests: 99 hasNodeOption cases + CA cache-rebuild coverage for EOF/empty/alias/boolean in both directions. * fix(utils): model Node's NODE_OPTIONS grammar in hasNodeOption Give each modeled option the value role Node gives it, and derive the CA state from one scoped parse instead of three loosely coupled passes. - Option roles are now explicit (boolean / value / inline value): the V8-owned heap flags accept an empty inline value and reject a separated one, while `--stack-trace-limit` requires a non-empty inline value. - The inspector family resolves through its aliases: `--inspect-port` and `--debug-port` take a value, the bare `--inspect`/`--inspect-brk`/ `--inspect-wait` forms are booleans, and the `=`-forms require a non-empty inline value. - Option processing stops at the first argument that is not an option, so an ignored suffix neither enables a CA flag nor creates a required-value error. - `--` and the negation of a non-boolean option are rejected, matching the streams Node refuses. Adds a parity script that derives the expected result from the Node 22 runtime instead of from the implementation's own assumptions. * test(utils): cover accepted heap and inspector forms through the CA selector * fix(utils): harden the NODE_OPTIONS parity check and negated queries - `hasNodeOption('--no-')` now reports the negation only when the option is actually present. Previously an unrelated option in NODE_OPTIONS made the negated query report `true`. - The parity script no longer accepts a `--flag` value it cannot measure: the Node probe and the matrix both observe `--use-openssl-ca`. - The parity script reports a usage error when an operand is missing or is another flag, instead of silently falling back to a default. - The parity script unsets NODE_EXTRA_CA_CERTS and NODE_USE_SYSTEM_CA in the child environment: both add certificates to the 'default' store and would make the observation signal unusable. - The parity script verifies that the runtime exposes `tls.getCACertificates` (Node >= 22.15) before running, counts timeouts separately, and fails when no case was checked, so a run that measures nothing can no longer report success. * fix(utils): preserve CA flags after supported test option values * fix(utils): resolve test-isolation alias through its value role --------- Co-authored-by: jatmn --- package.json | 1 + scripts/check-node-options-parity.ts | 310 ++++++++++++++++ src/utils/envUtils.test.ts | 535 +++++++++++++++++++++++++++ src/utils/envUtils.ts | 384 ++++++++++++++++++- 4 files changed, 1228 insertions(+), 2 deletions(-) create mode 100644 scripts/check-node-options-parity.ts create mode 100644 src/utils/envUtils.test.ts diff --git a/package.json b/package.json index cea4e6db6..f2c1b98a2 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "test:coverage:ui": "bun run scripts/render-coverage-heatmap.ts", "security:pr-scan": "bun run scripts/pr-intent-scan.ts", "test:provider-recommendation": "bun test src/utils/providerRecommendation.test.ts src/utils/providerProfile.test.ts", + "test:node-options-parity": "bun run scripts/check-node-options-parity.ts", "typecheck": "tsc --noEmit", "typecheck:fast": "bun run scripts/typecheck-fast.ts", "typecheck:type-tests": "bun run scripts/typecheck-type-tests.ts", diff --git a/scripts/check-node-options-parity.ts b/scripts/check-node-options-parity.ts new file mode 100644 index 000000000..d131da652 --- /dev/null +++ b/scripts/check-node-options-parity.ts @@ -0,0 +1,310 @@ +#!/usr/bin/env bun +/** + * Parity check for `hasNodeOption()` against the Node runtime. + * + * `hasNodeOption()` models scoped NODE_OPTIONS syntax and flag state. + * This script compares that answer with the Node runtime + * itself instead of from the implementation's own assumptions: it runs a + * matrix of NODE_OPTIONS values through `node` and compares the observed + * CA-store selection with the helper's result. + * + * Usage: + * bun run scripts/check-node-options-parity.ts [--node ] + * + * The CA store is observed through `tls.getCACertificates('default')`: with + * `--use-openssl-ca` the default store is empty, without it the bundled roots + * are returned. A stream Node rejects leaves no option active, so the helper + * must fail closed and report the flag as disabled. + * + * `tls.getCACertificates()` and `--use-system-ca` both landed in Node + * v22.15.0, so that is the minimum runtime this script can measure against; + * it exits with an error on anything older. + * + * Known divergence: Node rejects options it does not know ("--bogus is not + * allowed in NODE_OPTIONS"). This helper models a curated set of options and + * treats every other option as a boolean, so a stream containing an unknown + * option is reported as accepted. Validating unknown or disallowed options + * is out of scope for the helper. + */ +import { spawnSync } from 'child_process' +import { hasNodeOption } from '../src/utils/envUtils.js' + +const args = process.argv.slice(2) + +/** + * Read an operand for an option. An absent option returns undefined; a + * present option without a usable operand is a usage error, so it must not + * silently fall back to a default. + */ +function argValue(name: string): string | undefined { + const index = args.indexOf(name) + if (index === -1) return undefined + const value = args[index + 1] + if (value === undefined || value.startsWith('-')) { + console.error(`${name} requires a value`) + process.exit(2) + } + return value +} + +// The Node probe and the matrix both observe `--use-openssl-ca`, so the flag +// is fixed: making it configurable would compare the runtime's answer for one +// flag against the helper's answer for another. +const flag = '--use-openssl-ca' + +// The binary is passed as argv[0] with an argument array, never through a +// shell, so the path cannot be interpreted as a command. +const nodePath = argValue('--node') ?? process.env.NODE_BIN ?? 'node' + +const nodeVersion = spawnSync(nodePath, ['--version'], { encoding: 'utf8' }) +if (nodeVersion.status !== 0) { + console.error(`Cannot run ${nodePath}. Pass --node to a Node binary.`) + process.exit(2) +} +const version = (nodeVersion.stdout ?? '').trim() +if (Number(version.match(/^v(\d+)/)?.[1] ?? 0) < 24) { + console.warn( + `Warning: ${nodePath} is ${version}; OpenClaude requires Node >= 24.22.1.`, + ) +} + +// The observation method needs `tls.getCACertificates()`, which landed in +// Node v22.15.0 together with `--use-system-ca`. Without it every probe +// throws and the run would check nothing while still exiting successfully. +const probeSupport = spawnSync( + nodePath, + ['-p', "typeof require('tls').getCACertificates"], + { encoding: 'utf8' }, +) +if ((probeSupport.stdout ?? '').trim() !== 'function') { + console.error( + `${nodePath} (${version}) has no tls.getCACertificates(); the parity check needs Node >= 22.15.`, + ) + process.exit(2) +} + +type NodeOutcome = + | 'rejected' + | 'enabled' + | 'disabled' + | 'timeout' + | 'runtime-error' + +/** + * Node rejects the NODE_OPTIONS *stream* for these diagnostics. Other + * failures (a missing --loader module, an invalid enum value) happen after + * the options were accepted, so they say nothing about the parser contract. + */ +const PARSE_ERROR_PATTERNS = [ + 'requires an argument', + 'is not allowed in NODE_OPTIONS', + 'invalid value for NODE_OPTIONS', + 'bad option', + 'illegal value for flag', + 'out of bounds', + 'not allowed', +] + +/** Ask Node what the CA flag would be for a given NODE_OPTIONS value. */ +function nodeOutcome(nodeOptions: string): NodeOutcome { + // NODE_EXTRA_CA_CERTS and NODE_USE_SYSTEM_CA add certificates to the + // 'default' store, which would make the "empty store" signal unusable. + const env = { ...process.env, NODE_OPTIONS: nodeOptions } + delete env.NODE_EXTRA_CA_CERTS + delete env.NODE_USE_SYSTEM_CA + + const result = spawnSync( + nodePath, + [ + '-p', + "require('tls').getCACertificates('default').length === 0 ? 'enabled' : 'disabled'", + ], + { + env, + timeout: 10_000, + encoding: 'utf8', + }, + ) + if (result.error && (result.error as { code?: string }).code === 'ETIMEDOUT') + return 'timeout' + if (result.status !== 0) { + const stderr = result.stderr ?? '' + return PARSE_ERROR_PATTERNS.some((pattern) => stderr.includes(pattern)) + ? 'rejected' + : 'runtime-error' + } + const stdout = (result.stdout ?? '').trim() + if (stdout === 'enabled') return 'enabled' + if (stdout === 'disabled') return 'disabled' + return 'rejected' +} + +/** What the helper reports for the same value. */ +function helperOutcome(nodeOptions: string, target: string): NodeOutcome { + const previous = process.env.NODE_OPTIONS + process.env.NODE_OPTIONS = nodeOptions + try { + return hasNodeOption(target) ? 'enabled' : 'disabled' + } finally { + if (previous === undefined) delete process.env.NODE_OPTIONS + else process.env.NODE_OPTIONS = previous + } +} + +const CA_FORMS = [ + '--use-openssl-ca', + '--use-openssl-ca=1', + '--use-openssl-ca=0', + '--no-use-openssl-ca', + '--use-openssl-ca --no-use-openssl-ca', + '--no-use-openssl-ca --use-openssl-ca', + '--use-openssl-ca=1 --no-use-openssl-ca=1', + '--no-use-openssl-ca=1 --use-openssl-ca=1', +] + +const CONSTRUCTS = [ + '--title', + '--title=', + '--title=foo', + '--title --use-openssl-ca', + '--title=--use-openssl-ca', + '--conditions', + '--conditions=', + '--conditions=foo', + '--conditions foo', + '--conditions -x', + '--test-isolation none', + '--test-isolation=', + '--test-isolation', + '--test-global-setup ./setup.js', + '--test-global-setup=', + '--test-global-setup', + '--test-rerun-failures ./failures.json', + '--test-rerun-failures=', + '--test-rerun-failures', + '-C foo', + '-C', + '--loader', + '--loader=', + '--loader=foo', + '--inspect-port', + '--inspect-port=', + '--inspect-port=9229', + '--debug-port', + '--debug-port=', + '--debug-port=9229', + '--inspect=', + '--inspect=9229', + '--inspect-brk=', + '--inspect-brk=9229', + '--inspect-wait=', + '--inspect-wait=9229', + '--inspect-brk-node=', + '--inspect-publish-uid', + '--inspect-publish-uid=stderr', + '--max-old-space-size', + '--max-old-space-size=', + '--max-old-space-size=4096', + '--max-old-space-size 4096', + '--max-semi-space-size', + '--max-semi-space-size=', + '--max-semi-space-size=16', + '--max-semi-space-size 16', + '--max-old-space-size-percentage', + '--max-old-space-size-percentage=', + '--max-old-space-size-percentage=50', + '--max-old-space-size-percentage 50', + '--stack-trace-limit', + '--stack-trace-limit=', + '--stack-trace-limit=10', + '--stack-trace-limit 10', + '--experimental-import-meta-resolve', + 'foo', + '-', + '--', + '"foo --use-openssl-ca bar"', + '"--use-openssl-ca"', + "'--use-openssl-ca'", + '--use-openssl-ca\t', + '\t--use-openssl-ca', + '--title="foo --use-openssl-ca bar"', + '--conditions="foo --use-openssl-ca bar"', + '--conditions "foo \\" --use-openssl-ca bar"', + '--conditions=--use-openssl-ca', + '--title openclaude', + '--experimental-vm-modules', + '--enable-source-maps', + '--enable-source-maps=', + '--expose-gc', + '--no-expose-gc', + '--throw-deprecation', + '--trace-warnings', + '--no-warnings', + '--max-http-header-size=16384', + '--dns-result-order=verbatim', + '--dns-result-order=bogus', + '--unhandled-rejections=strict', + '--unhandled-rejections=bogus', + '--report-directory=out', +] + +const matrix = new Set(CA_FORMS) +for (const construct of CONSTRUCTS) { + matrix.add(`${construct} --use-openssl-ca`) + matrix.add(`--use-openssl-ca ${construct}`) + matrix.add(`--use-openssl-ca=1 ${construct}`) + matrix.add(`${construct} --use-openssl-ca=1`) +} + +let failures = 0 +let checked = 0 +let runtimeErrors = 0 +let timeouts = 0 +for (const nodeOptions of matrix) { + const expected = nodeOutcome(nodeOptions) + if (expected === 'timeout') { + // Node started and is waiting for a debugger (`--inspect-brk`, + // `--inspect-wait`), so the option stream was accepted but the CA state + // cannot be observed. + timeouts++ + continue + } + if (expected === 'runtime-error') { + runtimeErrors++ + continue + } + checked++ + // A stream Node rejects leaves no option active, so the helper must fail + // closed and report the flag as disabled. + const expectedState = expected === 'rejected' ? 'disabled' : expected + const actual = helperOutcome(nodeOptions, flag) + if (expectedState !== actual) { + failures++ + console.log( + `MISMATCH node=${expected.padEnd(9)} helper=${actual.padEnd(9)} ${JSON.stringify(nodeOptions)}`, + ) + } + + // The sibling CA flag shares the parser, so a rejected stream must make it + // fail closed as well. + if (expected === 'rejected') { + const sibling = '--use-system-ca' + if (helperOutcome(nodeOptions, sibling) !== 'disabled') { + failures++ + console.log( + `MISMATCH sibling ${sibling} did not fail closed for ${JSON.stringify(nodeOptions)}`, + ) + } + } +} + +console.log( + `\nchecked=${checked} mismatches=${failures} runtime-errors=${runtimeErrors} timeouts=${timeouts} (node=${version}, flag=${flag})`, +) + +// A run that checked nothing must not report success. +if (checked === 0) { + console.error('No case was checked; the parity result is meaningless.') + process.exit(1) +} +process.exit(failures === 0 ? 0 : 1) diff --git a/src/utils/envUtils.test.ts b/src/utils/envUtils.test.ts new file mode 100644 index 000000000..fb792a02b --- /dev/null +++ b/src/utils/envUtils.test.ts @@ -0,0 +1,535 @@ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test' +import { mkdtempSync, rmSync, writeFileSync } from 'fs' +import { createRequire } from 'module' +import { tmpdir } from 'os' +import { join } from 'path' +import { clearCACertsCache, getCACertificates } from './caCerts.js' +import { hasNodeOption } from './envUtils.js' + +// Required-value options accepted by the supported Node 24/26 runtimes. +const testValueOptions = [ + ['--test-isolation', 'none'], + ['--experimental-test-isolation', 'none'], + ['--test-global-setup', './setup.js'], + ['--test-rerun-failures', './failures.json'], +] as const + +describe('hasNodeOption', () => { + const originalNodeOptions = process.env.NODE_OPTIONS + + afterEach(() => { + if (originalNodeOptions === undefined) { + delete process.env.NODE_OPTIONS + } else { + process.env.NODE_OPTIONS = originalNodeOptions + } + }) + + beforeEach(() => { + delete process.env.NODE_OPTIONS + }) + + // Table-driven regression coverage requested in PR #2188 review: + // - exact and equals positives for both CA flags + // - double-quoted positives + // - literal single-quoted negatives (Node keeps single quotes literal) + // - repeated whitespace / empty tokens + // - --inspect vs --inspect-brk boundary + // Each row restores NODE_OPTIONS after the case via afterEach. + const cases: Array<{ + name: string + nodeOptions: string | undefined + flag: string + expected: boolean + }> = [ + // Undefined / empty + { name: 'undefined NODE_OPTIONS returns false', nodeOptions: undefined, flag: '--use-system-ca', expected: false }, + { name: 'empty string returns false', nodeOptions: '', flag: '--use-system-ca', expected: false }, + { name: 'whitespace only returns false', nodeOptions: ' \t ', flag: '--use-system-ca', expected: false }, + + // --use-system-ca exact / equals + { name: '--use-system-ca exact match', nodeOptions: '--use-system-ca', flag: '--use-system-ca', expected: true }, + { name: '--use-system-ca with =value', nodeOptions: '--use-system-ca=1', flag: '--use-system-ca', expected: true }, + { name: '--use-system-ca double-quoted exact', nodeOptions: '"--use-system-ca"', flag: '--use-system-ca', expected: true }, + { name: '--use-system-ca double-quoted with =value', nodeOptions: '"--use-system-ca=1"', flag: '--use-system-ca', expected: true }, + { name: "--use-system-ca single-quoted is literal (negative)", nodeOptions: "'--use-system-ca'", flag: '--use-system-ca', expected: false }, + { name: "--use-system-ca single-quoted with =value is literal (negative)", nodeOptions: "'--use-system-ca=1'", flag: '--use-system-ca', expected: false }, + + // --use-openssl-ca exact / equals + { name: '--use-openssl-ca exact match', nodeOptions: '--use-openssl-ca', flag: '--use-openssl-ca', expected: true }, + { name: '--use-openssl-ca with =value', nodeOptions: '--use-openssl-ca=1', flag: '--use-openssl-ca', expected: true }, + { name: '--use-openssl-ca double-quoted exact', nodeOptions: '"--use-openssl-ca"', flag: '--use-openssl-ca', expected: true }, + { name: "--use-openssl-ca single-quoted is literal (negative)", nodeOptions: "'--use-openssl-ca'", flag: '--use-openssl-ca', expected: false }, + + // Repeated whitespace / multiple tokens + { name: 'repeated whitespace between flags', nodeOptions: ' --use-system-ca --use-openssl-ca ', flag: '--use-system-ca', expected: true }, + { name: 'repeated whitespace second flag', nodeOptions: ' --use-system-ca --use-openssl-ca ', flag: '--use-openssl-ca', expected: true }, + { name: 'multiple spaces and tabs', nodeOptions: '--use-system-ca\t --max-old-space-size=4096', flag: '--max-old-space-size', expected: true }, + { name: 'flag not present with repeated whitespace', nodeOptions: ' --use-system-ca ', flag: '--use-openssl-ca', expected: false }, + // Node only treats ASCII spaces as delimiters: a tab with no spaces keeps + // the input as one token, so neither option is reported (verified on Node + // 22.23.2 where the tab-joined value is rejected as a bad option). + { name: 'tab without spaces does not delimit (second flag)', nodeOptions: '--use-system-ca\t--max-old-space-size=4096', flag: '--max-old-space-size', expected: false }, + { name: 'tab without spaces does not delimit (first flag)', nodeOptions: '--use-system-ca\t--max-old-space-size=4096', flag: '--use-system-ca', expected: false }, + + // --inspect vs --inspect-brk boundary + { name: '--inspect exact', nodeOptions: '--inspect', flag: '--inspect', expected: true }, + { name: '--inspect with =value', nodeOptions: '--inspect=0.0.0.0:9229', flag: '--inspect', expected: true }, + { name: '--inspect double-quoted with =value', nodeOptions: '"--inspect=0.0.0.0:9229"', flag: '--inspect', expected: true }, + { name: '--inspect-brk does not match --inspect (prefix without =)', nodeOptions: '--inspect-brk=9229', flag: '--inspect', expected: false }, + { name: '--inspect-brk exact matches itself', nodeOptions: '--inspect-brk=9229', flag: '--inspect-brk', expected: true }, + { name: '--inspect-brk exact', nodeOptions: '--inspect-brk', flag: '--inspect-brk', expected: true }, + { name: '--inspect does not match --inspect-brk', nodeOptions: '--inspect', flag: '--inspect-brk', expected: false }, + { name: '--inspect single-quoted is literal (negative)', nodeOptions: "'--inspect'", flag: '--inspect', expected: false }, + { name: '--inspect-brk single-quoted is literal (negative for --inspect)', nodeOptions: "'--inspect-brk=9229'", flag: '--inspect', expected: false }, + + // --max-old-space-size (original PR motivation) + { name: '--max-old-space-size with =value', nodeOptions: '--max-old-space-size=4096', flag: '--max-old-space-size', expected: true }, + { name: '--max-old-space-size double-quoted with =value', nodeOptions: '"--max-old-space-size=4096"', flag: '--max-old-space-size', expected: true }, + { name: "--max-old-space-size single-quoted is literal (negative)", nodeOptions: "'--max-old-space-size=4096'", flag: '--max-old-space-size', expected: false }, + { name: '--max-old-space-size exact without value fails closed (Node requires an argument)', nodeOptions: '--max-old-space-size', flag: '--max-old-space-size', expected: false }, + { name: '--max-old-space-size not confused with prefix', nodeOptions: '--max-old-space-size-extra', flag: '--max-old-space-size', expected: false }, + + // Mixed flags + { name: 'mixed flags contains target', nodeOptions: '--experimental-vm-modules --max-old-space-size=4096 --inspect=9229', flag: '--max-old-space-size', expected: true }, + { name: 'mixed flags does not contain absent', nodeOptions: '--experimental-vm-modules --max-old-space-size=4096', flag: '--use-system-ca', expected: false }, + + // Quoted option values must not create spurious flag tokens (Node quote grammar) + { name: 'quoted value with option-like text is not a flag', nodeOptions: '--conditions "foo --use-system-ca bar"', flag: '--use-system-ca', expected: false }, + { name: 'quoted value with option-like text — explicit flag still matches', nodeOptions: '--conditions "foo --use-system-ca bar" --use-system-ca', flag: '--use-system-ca', expected: true }, + { name: 'equals-quoted value with option-like text is not a flag', nodeOptions: '--title="foo --use-system-ca bar"', flag: '--use-system-ca', expected: false }, + { name: 'quoted value preserves other flag detection', nodeOptions: '--title="foo bar" --use-system-ca', flag: '--use-system-ca', expected: true }, + { name: 'quoted value with --expose-gc inner text is not a flag', nodeOptions: '--conditions "foo --expose-gc bar"', flag: '--expose-gc', expected: false }, + { name: '--conditions value exactly --use-system-ca is not a flag', nodeOptions: '--conditions "--use-system-ca"', flag: '--use-system-ca', expected: false }, + { name: '--conditions escaped quote does not expose inner flag', nodeOptions: '--conditions "foo \\" --use-system-ca bar"', flag: '--use-system-ca', expected: false }, + { name: '--conditions escaped value still allows explicit flag after', nodeOptions: '--conditions "foo \\" --use-system-ca bar" --use-system-ca', flag: '--use-system-ca', expected: true }, + { name: '--conditions with equals value is not a flag', nodeOptions: '--conditions=--use-system-ca', flag: '--use-system-ca', expected: false }, + + // Required option value that looks flag-like: Node rejects the whole + // value (e.g. `--title requires an argument`), so nothing is active. + { name: '--title with flag-like value fails closed', nodeOptions: '--title --use-system-ca', flag: '--use-system-ca', expected: false }, + { name: '--title with flag-like value fails closed for the option itself', nodeOptions: '--title --use-system-ca', flag: '--title', expected: false }, + { name: '--title with real value still detects later flag', nodeOptions: '--title openclaude --use-system-ca', flag: '--use-system-ca', expected: true }, + + // Malformed quoting fails closed (Node rejects the whole value) + { name: 'unterminated double quote is not a flag', nodeOptions: '"--use-system-ca', flag: '--use-system-ca', expected: false }, + { name: 'unterminated double quote with trailing escape is not a flag', nodeOptions: '"--use-system-ca\\', flag: '--use-system-ca', expected: false }, + { name: 'unterminated double quote is not a flag (--use-openssl-ca)', nodeOptions: '"--use-openssl-ca', flag: '--use-openssl-ca', expected: false }, + { name: 'unterminated double quote with trailing escape is not a flag (--use-openssl-ca)', nodeOptions: '"--use-openssl-ca\\', flag: '--use-openssl-ca', expected: false }, + { name: 'malformed leading flag does not leak later flag', nodeOptions: '"--use-system-ca --use-openssl-ca', flag: '--use-openssl-ca', expected: false }, + + // Ordered CA state: later occurrences win (equals-positive forms) + { name: '--use-system-ca=1 then --no-use-system-ca disables', nodeOptions: '--use-system-ca=1 --no-use-system-ca', flag: '--use-system-ca', expected: false }, + { name: '--no-use-system-ca then --use-system-ca=1 re-enables', nodeOptions: '--no-use-system-ca --use-system-ca=1', flag: '--use-system-ca', expected: true }, + { name: '--use-openssl-ca=1 then --no-use-openssl-ca disables', nodeOptions: '--use-openssl-ca=1 --no-use-openssl-ca', flag: '--use-openssl-ca', expected: false }, + { name: '--no-use-openssl-ca then --use-openssl-ca=1 re-enables', nodeOptions: '--no-use-openssl-ca --use-openssl-ca=1', flag: '--use-openssl-ca', expected: true }, + // Exact-positive ordering baseline (last wins) + { name: '--use-system-ca then --no-use-system-ca disables (exact)', nodeOptions: '--use-system-ca --no-use-system-ca', flag: '--use-system-ca', expected: false }, + { name: '--no-use-system-ca then --use-system-ca re-enables (exact)', nodeOptions: '--no-use-system-ca --use-system-ca', flag: '--use-system-ca', expected: true }, + { name: '--use-openssl-ca then --no-use-openssl-ca disables (exact)', nodeOptions: '--use-openssl-ca --no-use-openssl-ca', flag: '--use-openssl-ca', expected: false }, + { name: '--no-use-openssl-ca then --use-openssl-ca re-enables (exact)', nodeOptions: '--no-use-openssl-ca --use-openssl-ca', flag: '--use-openssl-ca', expected: true }, + { name: 'negation alone does not enable positive', nodeOptions: '--no-use-system-ca', flag: '--use-system-ca', expected: false }, + { name: 'negation with =value disables equals-positive', nodeOptions: '--use-system-ca=1 --no-use-system-ca=0', flag: '--use-system-ca', expected: false }, + { name: 'negated spelling is queryable', nodeOptions: '--no-use-system-ca', flag: '--no-use-system-ca', expected: true }, + { name: 'negated spelling is not active after a later positive', nodeOptions: '--no-use-system-ca --use-system-ca', flag: '--no-use-system-ca', expected: false }, + { name: 'negated spelling is active after a later negation', nodeOptions: '--use-system-ca --no-use-system-ca', flag: '--no-use-system-ca', expected: true }, + { name: 'negated spelling is absent when the option is not present', nodeOptions: '--max-old-space-size=4096', flag: '--no-use-system-ca', expected: false }, + { name: 'negated spelling is absent for an unrelated stream', nodeOptions: '--title foo', flag: '--no-use-system-ca', expected: false }, + + // Required-value failures invalidate the whole stream even when the CA + // target appears before the error (Node exits, so nothing is active). + { name: '--use-system-ca=1 then --title at EOF fails closed', nodeOptions: '--use-system-ca=1 --title', flag: '--use-system-ca', expected: false }, + { name: '--use-openssl-ca=1 then --title at EOF fails closed', nodeOptions: '--use-openssl-ca=1 --title', flag: '--use-openssl-ca', expected: false }, + { name: '--use-system-ca=1 then --title= fails closed', nodeOptions: '--use-system-ca=1 --title=', flag: '--use-system-ca', expected: false }, + { name: '--use-openssl-ca=1 then --title= fails closed', nodeOptions: '--use-openssl-ca=1 --title=', flag: '--use-openssl-ca', expected: false }, + { name: '--use-system-ca=1 then --conditions at EOF fails closed', nodeOptions: '--use-system-ca=1 --conditions', flag: '--use-system-ca', expected: false }, + { name: '--use-system-ca=1 then --conditions= fails closed', nodeOptions: '--use-system-ca=1 --conditions=', flag: '--use-system-ca', expected: false }, + { name: '--use-system-ca=1 then --conditions flag-like fails closed', nodeOptions: '--use-system-ca=1 --conditions -x', flag: '--use-system-ca', expected: false }, + { name: '--conditions flag-like then CA fails closed', nodeOptions: '--conditions -x --use-system-ca=1', flag: '--use-system-ca', expected: false }, + + // Required-value aliases follow the same rule (error before and after target). + { name: '--loader flag-like then CA fails closed', nodeOptions: '--loader --use-system-ca=1', flag: '--use-system-ca', expected: false }, + { name: 'CA then --loader at EOF fails closed', nodeOptions: '--use-system-ca=1 --loader', flag: '--use-system-ca', expected: false }, + { name: 'CA then --loader= fails closed', nodeOptions: '--use-system-ca=1 --loader=', flag: '--use-system-ca', expected: false }, + { name: '--loader flag-like then openssl CA fails closed', nodeOptions: '--loader --use-openssl-ca=1', flag: '--use-openssl-ca', expected: false }, + { name: 'openssl CA then --loader at EOF fails closed', nodeOptions: '--use-openssl-ca=1 --loader', flag: '--use-openssl-ca', expected: false }, + { name: '--inspect-port flag-like then CA fails closed', nodeOptions: '--inspect-port --use-system-ca=1', flag: '--use-system-ca', expected: false }, + { name: 'CA then --inspect-port at EOF fails closed', nodeOptions: '--use-system-ca=1 --inspect-port', flag: '--use-system-ca', expected: false }, + { name: 'CA then --inspect-port= fails closed', nodeOptions: '--use-system-ca=1 --inspect-port=', flag: '--use-system-ca', expected: false }, + { name: '--inspect-port flag-like then openssl CA fails closed', nodeOptions: '--inspect-port --use-openssl-ca=1', flag: '--use-openssl-ca', expected: false }, + { name: '--inspect-publish-uid flag-like then CA fails closed', nodeOptions: '--inspect-publish-uid --use-system-ca=1', flag: '--use-system-ca', expected: false }, + { name: 'CA then --inspect-publish-uid at EOF fails closed', nodeOptions: '--use-system-ca=1 --inspect-publish-uid', flag: '--use-system-ca', expected: false }, + { name: '--inspect-publish-uid flag-like then openssl CA fails closed', nodeOptions: '--inspect-publish-uid --use-openssl-ca=1', flag: '--use-openssl-ca', expected: false }, + { name: 'CA then --title flag-like value fails closed (openssl)', nodeOptions: '--use-openssl-ca=1 --title --use-system-ca', flag: '--use-openssl-ca', expected: false }, + + // Valid required-value forms preserve later CA detection. + { name: '--title with real value preserves later CA', nodeOptions: '--title foo --use-system-ca=1', flag: '--use-system-ca', expected: true }, + { name: '--title=foo preserves later CA', nodeOptions: '--title=foo --use-system-ca=1', flag: '--use-system-ca', expected: true }, + { name: '--loader=foo preserves later CA', nodeOptions: '--loader=foo --use-system-ca=1', flag: '--use-system-ca', expected: true }, + { name: '--title=--use-system-ca inline flag-like value is valid (no CA)', nodeOptions: '--title=--use-system-ca', flag: '--use-system-ca', expected: false }, + + // --experimental-import-meta-resolve is boolean: following CA stays visible. + { name: 'boolean import-meta-resolve then CA stays visible', nodeOptions: '--experimental-import-meta-resolve --use-system-ca', flag: '--use-system-ca', expected: true }, + { name: 'boolean import-meta-resolve then openssl CA stays visible', nodeOptions: '--experimental-import-meta-resolve --use-openssl-ca', flag: '--use-openssl-ca', expected: true }, + { name: 'boolean import-meta-resolve then equals CA stays visible', nodeOptions: '--experimental-import-meta-resolve --use-system-ca=1', flag: '--use-system-ca', expected: true }, + + // V8-owned heap flags have their own value grammar: the value must be + // attached with `=`, an empty inline value is tolerated by V8 (Node starts + // and only warns), and a separate value leaves the bare flag in front of + // V8, which rejects it. + { name: 'empty inline max-old-space-size keeps CA enabled', nodeOptions: '--use-system-ca --max-old-space-size=', flag: '--use-system-ca', expected: true }, + { name: 'empty inline max-semi-space-size keeps CA enabled', nodeOptions: '--use-system-ca --max-semi-space-size=', flag: '--use-system-ca', expected: true }, + { name: 'empty inline max-old-space-size keeps openssl CA enabled', nodeOptions: '--use-openssl-ca --max-old-space-size=', flag: '--use-openssl-ca', expected: true }, + { name: 'empty inline max-semi-space-size keeps openssl CA enabled', nodeOptions: '--use-openssl-ca --max-semi-space-size=', flag: '--use-openssl-ca', expected: true }, + { name: 'separated max-old-space-size value fails closed', nodeOptions: '--use-system-ca=1 --max-old-space-size 4096', flag: '--use-system-ca', expected: false }, + { name: 'separated max-semi-space-size value fails closed', nodeOptions: '--use-system-ca=1 --max-semi-space-size 16', flag: '--use-system-ca', expected: false }, + { name: 'separated max-old-space-size value fails closed (openssl)', nodeOptions: '--use-openssl-ca=1 --max-old-space-size 4096', flag: '--use-openssl-ca', expected: false }, + { name: 'separated stack-trace-limit value fails closed', nodeOptions: '--use-system-ca=1 --stack-trace-limit 10', flag: '--use-system-ca', expected: false }, + { name: 'empty inline stack-trace-limit fails closed', nodeOptions: '--use-system-ca=1 --stack-trace-limit=', flag: '--use-system-ca', expected: false }, + { name: 'inline stack-trace-limit value keeps CA enabled', nodeOptions: '--use-system-ca=1 --stack-trace-limit=10', flag: '--use-system-ca', expected: true }, + { name: 'inline stack-trace-limit value keeps openssl CA enabled', nodeOptions: '--use-openssl-ca=1 --stack-trace-limit=10', flag: '--use-openssl-ca', expected: true }, + { name: 'empty inline max-old-space-size-percentage fails closed', nodeOptions: '--use-system-ca=1 --max-old-space-size-percentage=', flag: '--use-system-ca', expected: false }, + { name: 'separated max-old-space-size-percentage value keeps CA enabled', nodeOptions: '--use-system-ca=1 --max-old-space-size-percentage 50', flag: '--use-system-ca', expected: true }, + { name: 'inline max-old-space-size-percentage value keeps CA enabled', nodeOptions: '--use-system-ca=1 --max-old-space-size-percentage=50', flag: '--use-system-ca', expected: true }, + + // The inspector family: `--inspect-port` (and its `--debug-port` alias) + // require a value, while the bare `--inspect`/`--inspect-brk`/ + // `--inspect-wait` forms are booleans. The `=`-forms expand through + // `--inspect-port`, so an empty inline value is rejected by Node. + { name: '--inspect-port without value fails closed', nodeOptions: '--use-system-ca=1 --inspect-port', flag: '--use-system-ca', expected: false }, + { name: '--debug-port without value fails closed', nodeOptions: '--use-system-ca=1 --debug-port', flag: '--use-system-ca', expected: false }, + { name: '--inspect= without value fails closed', nodeOptions: '--use-system-ca=1 --inspect=', flag: '--use-system-ca', expected: false }, + { name: '--inspect-brk= without value fails closed', nodeOptions: '--use-system-ca=1 --inspect-brk=', flag: '--use-system-ca', expected: false }, + { name: '--inspect-wait= without value fails closed', nodeOptions: '--use-system-ca=1 --inspect-wait=', flag: '--use-system-ca', expected: false }, + { name: '--inspect-brk-node= without value fails closed', nodeOptions: '--use-system-ca=1 --inspect-brk-node=', flag: '--use-system-ca', expected: false }, + { name: '--inspect-publish-uid without value fails closed', nodeOptions: '--use-system-ca=1 --inspect-publish-uid', flag: '--use-system-ca', expected: false }, + { name: 'bare --inspect keeps CA enabled', nodeOptions: '--use-system-ca=1 --inspect', flag: '--use-system-ca', expected: true }, + { name: 'bare --inspect-brk keeps CA enabled', nodeOptions: '--use-system-ca=1 --inspect-brk', flag: '--use-system-ca', expected: true }, + { name: 'bare --inspect-wait keeps CA enabled', nodeOptions: '--use-system-ca=1 --inspect-wait', flag: '--use-system-ca', expected: true }, + { name: '--inspect= with value keeps CA enabled', nodeOptions: '--use-system-ca=1 --inspect=9229', flag: '--use-system-ca', expected: true }, + { name: '--inspect-brk= with value keeps CA enabled', nodeOptions: '--use-system-ca=1 --inspect-brk=9229', flag: '--use-system-ca', expected: true }, + { name: '--inspect-wait= with value keeps CA enabled', nodeOptions: '--use-system-ca=1 --inspect-wait=9229', flag: '--use-system-ca', expected: true }, + { name: '--inspect-port= with value keeps CA enabled', nodeOptions: '--use-system-ca=1 --inspect-port=9229', flag: '--use-system-ca', expected: true }, + { name: '--debug-port= with value keeps CA enabled', nodeOptions: '--use-system-ca=1 --debug-port=9229', flag: '--use-system-ca', expected: true }, + { name: '--debug-port without value fails closed (openssl)', nodeOptions: '--use-openssl-ca=1 --debug-port', flag: '--use-openssl-ca', expected: false }, + { name: '--inspect= without value fails closed (openssl)', nodeOptions: '--use-openssl-ca=1 --inspect=', flag: '--use-openssl-ca', expected: false }, + + // Node stops parsing options at the first argument that is not an option, + // so the ignored suffix neither enables a CA flag nor creates a + // required-value error, while the preceding CA state is preserved. + { name: 'ignored suffix keeps preceding CA state', nodeOptions: '--use-system-ca foo --title', flag: '--use-system-ca', expected: true }, + { name: 'ignored suffix keeps preceding openssl CA state', nodeOptions: '--use-openssl-ca foo --title', flag: '--use-openssl-ca', expected: true }, + { name: 'ignored suffix keeps preceding CA state (equals)', nodeOptions: '--use-system-ca=1 foo', flag: '--use-system-ca', expected: true }, + { name: 'consumed value is not an ignored suffix', nodeOptions: '--title foo --use-system-ca', flag: '--use-system-ca', expected: true }, + { name: 'CA token inside the ignored suffix is not enabled', nodeOptions: 'foo --use-system-ca=1', flag: '--use-system-ca', expected: false }, + { name: 'CA token inside the ignored suffix is not enabled (openssl)', nodeOptions: 'foo --use-openssl-ca=1', flag: '--use-openssl-ca', expected: false }, + { name: 'lone hyphen stops option parsing', nodeOptions: '--use-system-ca -', flag: '--use-system-ca', expected: true }, + { name: 'lone hyphen stops option parsing before the CA flag', nodeOptions: '- --use-system-ca', flag: '--use-system-ca', expected: false }, + { name: '-- is not allowed in NODE_OPTIONS', nodeOptions: '--use-system-ca --', flag: '--use-system-ca', expected: false }, + { name: '-- before the CA flag is not allowed in NODE_OPTIONS', nodeOptions: '-- --use-system-ca', flag: '--use-system-ca', expected: false }, + ] + + for (const { name, nodeOptions, flag, expected } of cases) { + test(name, () => { + if (nodeOptions === undefined) { + delete process.env.NODE_OPTIONS + } else { + process.env.NODE_OPTIONS = nodeOptions + } + expect(hasNodeOption(flag)).toBe(expected) + }) + } + + test('supported test options consume their values before later CA flags', () => { + for (const [option, value] of testValueOptions) { + for (const flag of ['--use-system-ca', '--use-openssl-ca']) { + for (const spelling of [`${option} ${value}`, `${option}=${value}`]) { + process.env.NODE_OPTIONS = `${spelling} ${flag}` + expect(hasNodeOption(flag)).toBe(true) + process.env.NODE_OPTIONS = `${flag} ${spelling}` + expect(hasNodeOption(flag)).toBe(true) + } + } + } + }) + + test('supported test options reject missing, empty, flag-like and negated values', () => { + for (const [option] of testValueOptions) { + for (const flag of ['--use-system-ca', '--use-openssl-ca']) { + for (const nodeOptions of [ + `${flag}=1 ${option}`, + `${flag}=1 ${option}=`, + `${option}= ${flag}=1`, + `${option} ${flag}=1`, + `${flag}=1 --no-${option.slice(2)}`, + `--no-${option.slice(2)}=none ${flag}=1`, + ]) { + process.env.NODE_OPTIONS = nodeOptions + expect(hasNodeOption(flag)).toBe(false) + } + } + } + }) + + test('does not leak NODE_OPTIONS between cases (sanity)', () => { + // This test runs after the table; afterEach should have restored original. + // Set a distinct value and verify it does not persist to next test via afterEach, + // but here we just verify the helper reads current env. + process.env.NODE_OPTIONS = '--use-system-ca' + expect(hasNodeOption('--use-system-ca')).toBe(true) + expect(hasNodeOption('--use-openssl-ca')).toBe(false) + }) +}) + +describe('hasNodeOption CA selection via cache rebuild', () => { + const originalNodeOptions = process.env.NODE_OPTIONS + const originalExtraCerts = process.env.NODE_EXTRA_CA_CERTS + + afterEach(() => { + if (originalNodeOptions === undefined) { + delete process.env.NODE_OPTIONS + } else { + process.env.NODE_OPTIONS = originalNodeOptions + } + if (originalExtraCerts === undefined) { + delete process.env.NODE_EXTRA_CA_CERTS + } else { + process.env.NODE_EXTRA_CA_CERTS = originalExtraCerts + } + clearCACertsCache() + }) + + beforeEach(() => { + delete process.env.NODE_OPTIONS + delete process.env.NODE_EXTRA_CA_CERTS + clearCACertsCache() + }) + + function getEffectiveCACerts(nodeOptions: string): string[] | undefined { + // Simulates the settings-reload path: applyConfigEnvironmentVariables() + // replaces process.env.NODE_OPTIONS, then clears CA/proxy/mTLS caches and + // rebuilds global agents. getCACertificates() must reflect the effective + // ordered CA state after the rebuild. + process.env.NODE_OPTIONS = nodeOptions + clearCACertsCache() + return getCACertificates() + } + + test('malformed unterminated quote selects bundled roots (undefined)', () => { + expect(getEffectiveCACerts('"--use-system-ca')).toBeUndefined() + }) + + test('malformed trailing escape selects bundled roots (undefined)', () => { + expect(getEffectiveCACerts('"--use-system-ca\\')).toBeUndefined() + }) + + test('equals-positive then negation selects bundled roots (undefined)', () => { + expect( + getEffectiveCACerts('--use-system-ca=1 --no-use-system-ca'), + ).toBeUndefined() + expect( + getEffectiveCACerts('--use-openssl-ca=1 --no-use-openssl-ca'), + ).toBeUndefined() + }) + + test('required value with flag-like text selects bundled roots (undefined)', () => { + expect(getEffectiveCACerts('--title --use-system-ca')).toBeUndefined() + }) + + test('negation then equals-positive selects system roots (defined)', () => { + expect( + getEffectiveCACerts('--no-use-system-ca --use-system-ca=1'), + ).toBeDefined() + expect( + getEffectiveCACerts('--no-use-openssl-ca --use-openssl-ca=1'), + ).toBeDefined() + }) + + test('target-before-error selects bundled roots (undefined)', () => { + expect(getEffectiveCACerts('--use-system-ca=1 --title')).toBeUndefined() + expect(getEffectiveCACerts('--use-openssl-ca=1 --title')).toBeUndefined() + expect(getEffectiveCACerts('--use-system-ca=1 --title=')).toBeUndefined() + expect(getEffectiveCACerts('--use-system-ca=1 --loader')).toBeUndefined() + expect( + getEffectiveCACerts('--use-system-ca=1 --inspect-port'), + ).toBeUndefined() + expect( + getEffectiveCACerts('--use-system-ca=1 --inspect-publish-uid'), + ).toBeUndefined() + }) + + test('alias error-before-target selects bundled roots (undefined)', () => { + expect(getEffectiveCACerts('--loader --use-system-ca=1')).toBeUndefined() + expect( + getEffectiveCACerts('--inspect-port --use-system-ca=1'), + ).toBeUndefined() + expect( + getEffectiveCACerts('--inspect-publish-uid --use-openssl-ca=1'), + ).toBeUndefined() + }) + + test('boolean import-meta-resolve preserves system roots (defined)', () => { + expect( + getEffectiveCACerts('--experimental-import-meta-resolve --use-system-ca'), + ).toBeDefined() + expect( + getEffectiveCACerts( + '--experimental-import-meta-resolve --use-openssl-ca', + ), + ).toBeDefined() + }) +}) + +describe('hasNodeOption CA source selection', () => { + const originalNodeOptions = process.env.NODE_OPTIONS + const originalExtraCerts = process.env.NODE_EXTRA_CA_CERTS + const nodeRequire = createRequire(import.meta.url) + const tls = nodeRequire('tls') as typeof import('tls') + const originalGetCACertificates = tls.getCACertificates + + // The bundled and system stores can hold the same certificates, so + // "defined" alone cannot prove which root source the selector picked. + // Stubbing the system store makes the choice observable. + const SYSTEM_SENTINEL = 'SYSTEM-CA-SENTINEL' + const EXTRA_CA_SENTINEL = 'EXTRA-CA-SENTINEL' + let extraCertsDir: string | undefined + + beforeEach(() => { + delete process.env.NODE_OPTIONS + delete process.env.NODE_EXTRA_CA_CERTS + tls.getCACertificates = (type) => + type === 'system' ? [SYSTEM_SENTINEL] : [] + extraCertsDir = mkdtempSync(join(tmpdir(), 'openclaude-ca-')) + writeFileSync(join(extraCertsDir, 'extra.pem'), EXTRA_CA_SENTINEL) + clearCACertsCache() + }) + + afterEach(() => { + tls.getCACertificates = originalGetCACertificates + if (originalNodeOptions === undefined) { + delete process.env.NODE_OPTIONS + } else { + process.env.NODE_OPTIONS = originalNodeOptions + } + if (originalExtraCerts === undefined) { + delete process.env.NODE_EXTRA_CA_CERTS + } else { + process.env.NODE_EXTRA_CA_CERTS = originalExtraCerts + } + if (extraCertsDir) { + rmSync(extraCertsDir, { recursive: true, force: true }) + extraCertsDir = undefined + } + clearCACertsCache() + }) + + function getEffectiveCACerts(nodeOptions: string): string[] | undefined { + // Simulates the settings-reload path: applyConfigEnvironmentVariables() + // replaces process.env.NODE_OPTIONS, then clears CA/proxy/mTLS caches and + // rebuilds global agents. + process.env.NODE_OPTIONS = nodeOptions + clearCACertsCache() + return getCACertificates() + } + + test('active CA flag selects the system root store', () => { + expect(getEffectiveCACerts('--use-system-ca')).toEqual([SYSTEM_SENTINEL]) + expect(getEffectiveCACerts('--use-openssl-ca')).toEqual([SYSTEM_SENTINEL]) + expect(getEffectiveCACerts('--use-system-ca=1')).toEqual([SYSTEM_SENTINEL]) + }) + + test('inactive CA flag keeps the runtime default (undefined)', () => { + expect(getEffectiveCACerts('--no-use-system-ca')).toBeUndefined() + expect( + getEffectiveCACerts('--use-system-ca=1 --no-use-system-ca'), + ).toBeUndefined() + }) + + test('invalid streams keep the runtime default (undefined)', () => { + // Missing required value, flag-like value, malformed quoting, an invalid + // V8 value form and `--` all make Node reject the whole value. + expect(getEffectiveCACerts('--use-system-ca=1 --title')).toBeUndefined() + expect(getEffectiveCACerts('--title --use-system-ca')).toBeUndefined() + expect(getEffectiveCACerts('"--use-system-ca')).toBeUndefined() + expect( + getEffectiveCACerts('--use-system-ca=1 --max-old-space-size 4096'), + ).toBeUndefined() + expect(getEffectiveCACerts('--use-system-ca=1 --inspect=')).toBeUndefined() + expect(getEffectiveCACerts('--use-system-ca --')).toBeUndefined() + }) + + test('accepted empty inline heap value keeps the system store', () => { + // V8 tolerates an empty inline value for its own heap flags, so Node + // starts with the CA flag enabled. + expect( + getEffectiveCACerts('--use-system-ca --max-old-space-size='), + ).toEqual([SYSTEM_SENTINEL]) + expect( + getEffectiveCACerts('--use-system-ca --max-semi-space-size='), + ).toEqual([SYSTEM_SENTINEL]) + }) + + test('valid inspector forms keep the system store', () => { + // Bare inspector booleans and populated inline endpoints are accepted by + // Node, so the CA flag stays enabled. + expect(getEffectiveCACerts('--use-system-ca=1 --inspect')).toEqual([ + SYSTEM_SENTINEL, + ]) + expect(getEffectiveCACerts('--use-system-ca=1 --inspect=9229')).toEqual([ + SYSTEM_SENTINEL, + ]) + expect(getEffectiveCACerts('--use-system-ca=1 --inspect-port=9229')).toEqual([ + SYSTEM_SENTINEL, + ]) + }) + + test('active CA flag selects the system store plus the extra certificate', () => { + process.env.NODE_EXTRA_CA_CERTS = join(extraCertsDir!, 'extra.pem') + expect(getEffectiveCACerts('--use-system-ca')).toEqual([ + SYSTEM_SENTINEL, + EXTRA_CA_SENTINEL, + ]) + expect(getEffectiveCACerts('--use-openssl-ca')).toEqual([ + SYSTEM_SENTINEL, + EXTRA_CA_SENTINEL, + ]) + }) + + test('supported test option values preserve the explicit CA source with extra certificates', () => { + process.env.NODE_EXTRA_CA_CERTS = join(extraCertsDir!, 'extra.pem') + for (const [option, value] of testValueOptions) { + for (const flag of ['--use-system-ca', '--use-openssl-ca']) { + for (const spelling of [`${option} ${value}`, `${option}=${value}`]) { + for (const nodeOptions of [`${spelling} ${flag}`, `${flag} ${spelling}`]) { + expect(getEffectiveCACerts(nodeOptions)).toEqual([ + SYSTEM_SENTINEL, + EXTRA_CA_SENTINEL, + ]) + } + } + expect(getEffectiveCACerts(`${flag}=1 ${option}=`)).toEqual([ + ...tls.rootCertificates, + EXTRA_CA_SENTINEL, + ]) + } + } + }) + + test('inactive CA flag selects the bundled roots plus the extra certificate', () => { + process.env.NODE_EXTRA_CA_CERTS = join(extraCertsDir!, 'extra.pem') + const bundled = tls.rootCertificates + const certs = getEffectiveCACerts('--use-system-ca --no-use-system-ca') + expect(certs).toBeDefined() + expect(certs![0]).toBe(bundled[0]) + expect(certs!.length).toBe(bundled.length + 1) + expect(certs![certs!.length - 1]).toBe(EXTRA_CA_SENTINEL) + }) + + test('ignored suffix preserves the preceding CA state', () => { + // Node stops parsing options at `foo`, so the trailing `--title` never + // becomes a required-value error and the CA flag stays enabled. + expect(getEffectiveCACerts('--use-system-ca foo --title')).toEqual([ + SYSTEM_SENTINEL, + ]) + }) +}) diff --git a/src/utils/envUtils.ts b/src/utils/envUtils.ts index f50e2d49c..7f24d008b 100644 --- a/src/utils/envUtils.ts +++ b/src/utils/envUtils.ts @@ -92,16 +92,396 @@ export function getProjectsDir(): string { return join(getClaudeConfigHomeDir(), 'projects') } +/** + * How a NODE_OPTIONS option consumes its value, mirroring Node's + * `OptionsParser::Parse` (src/node_options.cc). + */ +type NodeOptionRole = + /** The option takes no value (`--use-system-ca`). */ + | 'boolean' + /** + * The option requires a value: either attached with `=` (and not empty) or + * supplied as the next argument, which must not start with `-`. + */ + | 'value' + /** + * The option requires its value to be attached with `=`. An empty inline + * value is accepted (V8-owned flags such as `--max-old-space-size`). + */ + | 'inline-value' + /** + * The option requires a non-empty inline value; a separate argument is not + * accepted (`--stack-trace-limit`). + */ + | 'inline-required' + +/** + * NODE_OPTIONS options whose role differs from the default boolean. + * Originally audited against Node 22.23.2; newer required-value test options + * are also modeled for the supported Node 24/26 runtimes. Missing values fail + * the scoped parse, including the V8 flags with distinct attachment rules. + * + * Options that are not listed here are treated as booleans: they never + * consume the following argument. Unknown-option and disallowed-option + * validation is deliberately out of scope — Node rejects those streams + * outright, which this helper does not attempt to model. + */ +const NODE_OPTION_ROLES: ReadonlyMap = new Map< + string, + NodeOptionRole +>([ + // V8-owned flags. The value must be attached with `=`; V8 tolerates an + // empty inline value (`--max-old-space-size=` starts and only warns) but + // rejects a bare flag and a separate value. + ['--max-old-space-size', 'inline-value'], + ['--max-semi-space-size', 'inline-value'], + // Node-owned flag that Node forwards to V8. The value must be attached + // with `=`: an empty inline value is rejected by Node itself, and a + // separate value is forwarded to V8 without its value, which V8 rejects. + ['--stack-trace-limit', 'inline-required'], + // Node-owned options that take a value. + ['--allow-fs-read', 'value'], + ['--allow-fs-write', 'value'], + ['--conditions', 'value'], + ['-C', 'value'], + ['--cpu-prof-dir', 'value'], + ['--cpu-prof-interval', 'value'], + ['--cpu-prof-name', 'value'], + ['--debug-port', 'value'], + ['--diagnostic-dir', 'value'], + ['--disable-proto', 'value'], + ['--disable-warning', 'value'], + ['--dns-result-order', 'value'], + ['--experimental-default-type', 'value'], + ['--experimental-loader', 'value'], + ['--heap-prof-dir', 'value'], + ['--heap-prof-interval', 'value'], + ['--heap-prof-name', 'value'], + ['--heapsnapshot-near-heap-limit', 'value'], + ['--heapsnapshot-signal', 'value'], + ['--icu-data-dir', 'value'], + ['--import', 'value'], + ['--input-type', 'value'], + ['--inspect-port', 'value'], + ['--inspect-publish-uid', 'value'], + ['--loader', 'value'], + ['--localstorage-file', 'value'], + ['--max-http-header-size', 'value'], + ['--max-old-space-size-percentage', 'value'], + ['--network-family-autoselection-attempt-timeout', 'value'], + ['--openssl-config', 'value'], + ['--redirect-warnings', 'value'], + ['--report-dir', 'value'], + ['--report-directory', 'value'], + ['--report-filename', 'value'], + ['--report-signal', 'value'], + ['--require', 'value'], + ['-r', 'value'], + ['--secure-heap', 'value'], + ['--secure-heap-min', 'value'], + ['--snapshot-blob', 'value'], + ['--test-coverage-branches', 'value'], + ['--test-coverage-exclude', 'value'], + ['--test-coverage-functions', 'value'], + ['--test-coverage-include', 'value'], + ['--test-coverage-lines', 'value'], + ['--test-global-setup', 'value'], + ['--test-isolation', 'value'], + ['--test-name-pattern', 'value'], + ['--test-reporter', 'value'], + ['--test-reporter-destination', 'value'], + ['--test-rerun-failures', 'value'], + ['--test-shard', 'value'], + ['--test-skip-pattern', 'value'], + ['--title', 'value'], + ['--tls-cipher-list', 'value'], + ['--tls-keylog', 'value'], + ['--trace-event-categories', 'value'], + ['--trace-event-file-pattern', 'value'], + ['--trace-require-module', 'value'], + ['--unhandled-rejections', 'value'], + ['--use-largepages', 'value'], + ['--v8-pool-size', 'value'], + ['--watch-kill-signal', 'value'], + ['--watch-path', 'value'], +]) + +/** + * Node's alias expansions (src/node_options.cc `AddAlias`) for the options + * this helper models. The value lists the options an alias expands to; the + * first entry owns the value. + */ +const NODE_OPTION_ALIASES: ReadonlyMap = new Map([ + ['-C', ['--conditions']], + ['-r', ['--require']], + ['--debug-port', ['--inspect-port']], + ['--loader', ['--experimental-loader']], + ['--report-directory', ['--report-dir']], + ['--experimental-test-isolation', ['--test-isolation']], +]) + +/** + * Aliases Node only matches in the `--name=value` form. They expand to + * `--inspect-port ` plus the matching boolean option, so the inline + * value is required and must not be empty. + */ +const NODE_INLINE_OPTION_ALIASES: ReadonlyMap = + new Map([ + ['--inspect=', ['--inspect-port', '--inspect']], + ['--inspect-brk=', ['--inspect-port', '--inspect-brk']], + ['--inspect-brk-node=', ['--inspect-port', '--inspect-brk-node']], + ['--inspect-wait=', ['--inspect-port', '--inspect-wait']], + ]) + +/** An option Node accepted, with its `--no-` prefix resolved. */ +interface NodeOptionOccurrence { + /** The option name, without any `--no-` prefix. */ + name: string + /** Whether the option was spelled as `--no-`. */ + negated: boolean +} + +interface NodeOptionsParse { + /** False when Node would reject the whole NODE_OPTIONS value. */ + valid: boolean + /** The options Node accepted, in the order they appear. */ + options: NodeOptionOccurrence[] +} + +/** + * Tokenize NODE_OPTIONS the way Node's `ParseNodeOptionsEnvVar` does + * (src/node_options.cc). + * + * - double quotes toggle `is_in_string` and are stripped + * - a backslash escapes the next character, but only inside a string + * - only ASCII spaces outside strings delimit arguments (tabs are literal) + * - single quotes are literal characters + * + * Returns null when Node would reject the value: an unterminated string or + * an incomplete escape. + */ +function tokenizeNodeOptions(nodeOptions: string): string[] | null { + const tokens: string[] = [] + let isInString = false + let willStartNewArg = true + for (let i = 0; i < nodeOptions.length; i++) { + let c = nodeOptions[i]! + if (c === '\\' && isInString) { + if (i + 1 >= nodeOptions.length) { + // Trailing escape with nothing to escape: Node rejects the value. + return null + } + c = nodeOptions[++i]! + } else if (c === ' ' && !isInString) { + willStartNewArg = true + continue + } else if (c === '"') { + isInString = !isInString + continue + } + + if (willStartNewArg) { + tokens.push(c) + willStartNewArg = false + } else { + tokens[tokens.length - 1] += c + } + } + + // Node rejects unterminated strings at startup. + if (isInString) { + return null + } + return tokens +} + +/** + * Resolve an option name to its role and to the option names Node records + * for it, expanding aliases the way Node does (aliases are resolved before + * the option lookup, and the `=`-only aliases only match that spelling). + */ +function resolveNodeOption( + name: string, + hasInlineValue: boolean, +): { role: NodeOptionRole; names: string[] } { + if (hasInlineValue) { + const inlineAlias = NODE_INLINE_OPTION_ALIASES.get(`${name}=`) + if (inlineAlias) { + return { + role: NODE_OPTION_ROLES.get(inlineAlias[0]!) ?? 'boolean', + names: [name, ...inlineAlias], + } + } + } + + const alias = NODE_OPTION_ALIASES.get(name) + if (alias) { + return { + role: NODE_OPTION_ROLES.get(alias[0]!) ?? 'boolean', + names: [name, ...alias], + } + } + + return { role: NODE_OPTION_ROLES.get(name) ?? 'boolean', names: [name] } +} + +/** + * Parse NODE_OPTIONS into the options Node would accept. + * + * The result is either a valid option list or `valid: false` for the inputs + * Node rejects outright: malformed quoting, a missing or invalid required + * value, the negation of a non-boolean option, or `--`. + */ +function parseNodeOptions(nodeOptions: string): NodeOptionsParse { + const tokens = tokenizeNodeOptions(nodeOptions) + if (!tokens) { + return { valid: false, options: [] } + } + + const options: NodeOptionOccurrence[] = [] + let index = 0 + while (index < tokens.length) { + const token = tokens[index]! + index += 1 + + // Node stops parsing options at the first argument that is not an option + // (src/node_options.cc: `if (args.first().size() <= 1 || + // args.first()[0] != '-') break;`), so everything from there on is + // ignored — including options that would otherwise be seen. + if (token.length <= 1 || token[0] !== '-') { + break + } + + // `--` is not allowed in NODE_OPTIONS. + if (token === '--') { + return { valid: false, options: [] } + } + + // Only `--name=value` uses `=` as a separator. + const equalsIndex = token.startsWith('--') ? token.indexOf('=') : -1 + const name = equalsIndex === -1 ? token : token.slice(0, equalsIndex) + const inlineValue = + equalsIndex === -1 ? undefined : token.slice(equalsIndex + 1) + + // Node converts `--no-foo` to `--foo` and remembers the negation. + let optionName = name + let negated = false + if (optionName.startsWith('--no-')) { + optionName = `--${optionName.slice('--no-'.length)}` + negated = true + } + + const { role, names } = resolveNodeOption(optionName, equalsIndex !== -1) + + // Node rejects the negation of any option that is not a boolean. + if (negated && role !== 'boolean') { + return { valid: false, options: [] } + } + + switch (role) { + case 'boolean': + break + case 'value': { + if (inlineValue !== undefined) { + // An empty inline value is rejected by Node + // (`--title= requires an argument`). + if (inlineValue === '') { + return { valid: false, options: [] } + } + break + } + // The value is the next argument. Node rejects it when it is missing + // or when it looks like another option (`--title --use-system-ca`). + const next = tokens[index] + if (next === undefined || next.startsWith('-')) { + return { valid: false, options: [] } + } + index += 1 + break + } + case 'inline-value': { + // V8-owned flags must carry their value inline. V8 accepts an empty + // inline value (it warns but starts) and rejects a bare flag. + if (inlineValue === undefined) { + return { valid: false, options: [] } + } + break + } + case 'inline-required': { + if (inlineValue === undefined || inlineValue === '') { + return { valid: false, options: [] } + } + break + } + } + + // An alias can expand to an option that is already part of the expansion + // (`--inspect=` expands to `--inspect-port` and `--inspect`); record each + // option once per token so repeated tokens keep their own occurrence. + const recorded = new Set() + for (const occurrenceName of names) { + if (recorded.has(occurrenceName)) { + continue + } + recorded.add(occurrenceName) + options.push({ name: occurrenceName, negated }) + } + } + + return { valid: true, options } +} + /** * Check if NODE_OPTIONS contains a specific flag. - * Splits on whitespace and checks for exact match to avoid false positives. + * + * Mirrors Node's `ParseNodeOptionsEnvVar` and `OptionsParser::Parse` + * (src/node_options.cc) closely enough to answer what the CA and telemetry + * callers ask: "would Node have enabled this option?". + * + * - double quotes toggle `is_in_string` and are stripped + * - a backslash escapes the next character, but only inside a string + * - only ASCII spaces outside strings delimit arguments (tabs are literal); + * single quotes are literal + * - `--flag=value` is supported and `--flag` never matches `--flag-other` + * - value-taking options own their value, so `--conditions "--use-system-ca"` + * does not enable `--use-system-ca`; `--experimental-import-meta-resolve` + * is a boolean and never consumes the following argument + * - Node stops parsing options at the first argument that is not an option, + * so options after it are ignored + * - malformed quoting, a missing or invalid required value, the negation of a + * non-boolean option, and `--` make Node reject the whole NODE_OPTIONS + * value; the helper then fails closed and reports no option as active + * - boolean options are applied in order, so a later `--no-` disables + * an earlier `--` and vice versa */ export function hasNodeOption(flag: string): boolean { const nodeOptions = process.env.NODE_OPTIONS if (!nodeOptions) { return false } - return nodeOptions.split(/\s+/).includes(flag) + + // This helper can also run after bootstrap (settings re-applies + // NODE_OPTIONS, clears CA/proxy/mTLS caches, rebuilds agents), so an + // invalid option string must never be read as "the flag is enabled". + const parsed = parseNodeOptions(nodeOptions) + if (!parsed.valid) { + return false + } + + // Node applies boolean options in order, so the last occurrence wins. + // A query for the negated spelling (`--no-`) asks whether the + // negation is the effective state, which requires the option to be present. + const negatedQuery = flag.startsWith('--no-') + const optionName = negatedQuery ? `--${flag.slice('--no-'.length)}` : flag + let enabled = false + let seen = false + for (const option of parsed.options) { + if (option.name === optionName) { + enabled = !option.negated + seen = true + } + } + return negatedQuery ? seen && !enabled : enabled } export function isEnvTruthy(envVar: string | boolean | undefined): boolean {