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');