From 5a89d815457403c0f707f87af4f9cb47507a1cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caleb=20=E3=83=84=20Everett?= Date: Tue, 20 Oct 2026 17:17:23 +0000 Subject: [PATCH] child_process: inherit a reassigned process.env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since v26.11.0, spawning without options.env builds the child's environment from the OS environment block. Code that replaces process.env, e.g. process.env = { ...process.env, FOO: 'x' }, had its changes silently dropped. The child_process docs give process.env as the default for options.env, and { env: process.env } still saw the replacement object, so the two disagreed. Make getEnvPairs() take the object to read and return undefined unless it is still the native env proxy. When it isn't, copy process.env as before. A plain object is copied without going through the env interceptors, so that path stays cheap. Assisted-by: a closed-source coding agent Signed-off-by: Caleb ツ Everett --- lib/child_process.js | 7 +- src/process_wrap.cc | 10 +- .../test-child-process-env-reassigned.js | 103 ++++++++++++++++++ 3 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-child-process-env-reassigned.js diff --git a/lib/child_process.js b/lib/child_process.js index 6b411f43f07..3db7ec752be 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -741,8 +741,11 @@ function normalizeSpawnArguments(file, args, options) { // instead of copying process.env (one interceptor round trip and one // getenv() scan per variable). Everything copyProcessEnvToEnv() would // propagate below is part of it by definition, and entries of the real - // environment block cannot contain null bytes. - envPairs = getEnvPairs(); + // environment block cannot contain null bytes. Undefined if process.env + // was reassigned. + envPairs = getEnvPairs(process.env); + } + if (envPairs !== undefined) { if (process.platform === 'win32') { envPairs = dedupeWindowsEnvPairs(envPairs); } diff --git a/src/process_wrap.cc b/src/process_wrap.cc index 8ca42a784de..34e10b4b2a7 100644 --- a/src/process_wrap.cc +++ b/src/process_wrap.cc @@ -346,9 +346,17 @@ class ProcessWrap : public HandleWrap { // The current environment as ["KEY=value", ...], i.e. what a spawned // child inherits by default, produced in one pass over the environment - // block instead of one interceptor round trip per variable. + // block instead of one interceptor round trip per variable. Returns + // undefined if args[0] is not the env proxy, e.g. because process.env was + // reassigned, in which case the caller has to copy that object instead. static void GetEnvPairs(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + Local env_proxy_ctor_template = + env->env_proxy_ctor_template(); + if (env_proxy_ctor_template.IsEmpty() || + !env_proxy_ctor_template->HasInstance(args[0])) { + return; + } Local pairs; if (env->env_vars()->Pairs(env->isolate()).ToLocal(&pairs)) { args.GetReturnValue().Set(pairs); diff --git a/test/parallel/test-child-process-env-reassigned.js b/test/parallel/test-child-process-env-reassigned.js new file mode 100644 index 00000000000..44cd42fc4c9 --- /dev/null +++ b/test/parallel/test-child-process-env-reassigned.js @@ -0,0 +1,103 @@ +'use strict'; +// When no `env` option is given, a child process inherits process.env, even +// if process.env has been replaced with another object. +// Refs: https://github.com/nodejs/node/issues/66882 +const common = require('../common'); +const assert = require('assert'); +const { + exec, + execFile, + execFileSync, + execSync, + fork, + spawn, + spawnSync, +} = require('child_process'); + +if (process.argv[2] === 'child') { + process.send(process.env.TEST_REASSIGNED); + return; +} + +const printEnv = ['-p', 'process.env.TEST_REASSIGNED']; +const shellEcho = common.isWindows ? + 'echo %TEST_REASSIGNED%' : + 'echo "$TEST_REASSIGNED"'; + +function runSync(options) { + const child = spawnSync(process.execPath, printEnv, + { encoding: 'utf8', ...options }); + assert.strictEqual(child.status, 0, child.stderr || String(child.error)); + return child.stdout.trim(); +} + +const originalEnv = process.env; +// Lives in the real environment, but not in the replacement object. +process.env.TEST_ONLY_IN_ORIGINAL = 'original'; +process.env = { ...process.env, TEST_REASSIGNED: 'reassigned' }; +delete process.env.TEST_ONLY_IN_ORIGINAL; + +assert.strictEqual(runSync(), 'reassigned'); +assert.strictEqual( + execFileSync(process.execPath, printEnv, { encoding: 'utf8' }).trim(), + 'reassigned'); +assert.strictEqual(execSync(shellEcho, { encoding: 'utf8' }).trim(), + 'reassigned'); + +{ + const child = spawnSync( + process.execPath, + ['-p', 'process.env.TEST_ONLY_IN_ORIGINAL'], + { encoding: 'utf8' }); + assert.strictEqual(child.stdout.trim(), 'undefined'); +} + +// The default env matches an explicit { env: process.env }. +{ + const printAll = ['-e', 'process.stdout.write(JSON.stringify(process.env))']; + const run = (options) => JSON.parse( + spawnSync(process.execPath, printAll, { encoding: 'utf8', ...options }) + .stdout); + assert.deepStrictEqual(run(), run({ env: process.env })); +} + +// The replacement object is used for command lookup. +if (!common.isWindows) { + const saved = process.env; + process.env = { ...saved, PATH: '/nonexistent' }; + const child = spawnSync('ls'); + process.env = saved; + assert.strictEqual(child.error?.code, 'ENOENT'); +} + +{ + const child = spawn(process.execPath, printEnv); + let out = ''; + child.stdout.setEncoding('utf8').on('data', (d) => { out += d; }); + let err = ''; + child.stderr.setEncoding('utf8').on('data', (d) => { err += d; }); + child.on('close', common.mustCall((code) => { + assert.strictEqual(code, 0, err); + assert.strictEqual(out.trim(), 'reassigned'); + })); +} + +exec(shellEcho, common.mustSucceed((stdout) => { + assert.strictEqual(stdout.trim(), 'reassigned'); +})); + +execFile(process.execPath, printEnv, common.mustSucceed((stdout) => { + assert.strictEqual(stdout.trim(), 'reassigned'); +})); + +fork(__filename, ['child']).on('message', common.mustCall((value) => { + assert.strictEqual(value, 'reassigned'); +})); + +// Putting the original object back restores inheriting the real environment. +process.env = originalEnv; +assert.strictEqual(runSync(), 'undefined'); +assert.strictEqual( + spawnSync(process.execPath, ['-p', 'process.env.TEST_ONLY_IN_ORIGINAL'], + { encoding: 'utf8' }).stdout.trim(), + 'original');