diff --git a/src/vs/platform/utilityProcess/common/envKey.ts b/src/vs/platform/utilityProcess/common/envKey.ts new file mode 100644 index 00000000000000..8d88705c7dcaec --- /dev/null +++ b/src/vs/platform/utilityProcess/common/envKey.ts @@ -0,0 +1,16 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +/** + * Whether `key` is a name Node.js accepts on process environments. + * + * Names beginning with a digit or containing characters other than `[A-Za-z0-9_]` + * are rejected: forwarding such keys to a forked/spawned child environment makes + * Electron throw `TypeError: Invalid value for env`, breaking the child process + * (e.g. the extension host). They must be filtered at the boundary. + */ +export function isValidEnvVariableKey(key: string): boolean { + return /^[A-Za-z_][A-Za-z0-9_]*$/.test(key); +} \ No newline at end of file diff --git a/src/vs/platform/utilityProcess/electron-main/utilityProcess.ts b/src/vs/platform/utilityProcess/electron-main/utilityProcess.ts index c1dd46b4c1771f..612ac9bdaa98b2 100644 --- a/src/vs/platform/utilityProcess/electron-main/utilityProcess.ts +++ b/src/vs/platform/utilityProcess/electron-main/utilityProcess.ts @@ -16,6 +16,7 @@ import { ITelemetryService } from '../../telemetry/common/telemetry.js'; import { ILifecycleMainService } from '../../lifecycle/electron-main/lifecycleMainService.js'; import { removeDangerousEnvVariables } from '../../../base/common/processes.js'; import { deepClone } from '../../../base/common/objects.js'; +import { isValidEnvVariableKey } from '../common/envKey.js'; import { isWindows } from '../../../base/common/platform.js'; import { isUNCAccessRestrictionsDisabled, getUNCHostAllowlist } from '../../../base/node/unc.js'; @@ -293,8 +294,17 @@ export class UtilityProcess extends Disposable { // Remove any environment variables that are not allowed removeDangerousEnvVariables(env); - // Ensure all values are strings, otherwise the process will not start + // Ensure all values are strings, otherwise the process will not start (and valid keys are forwarded) for (const key of Object.keys(env)) { + if (!isValidEnvVariableKey(key)) { + // An environment variable whose name Node.js considers invalid (e.g. leading digit, + // or characters outside [A-Za-z0-9_]) would break the utility process boundary: + // Electron's spawn() throws `TypeError: Invalid value for env`. Such keys leak in + // from the parent `process.env` via implicit (global) inheritance, so we strip them here. + console.warn(`[utilityProcess] Ignoring environment variable with invalid name: '${key}'`); + delete env[key]; + continue; + } env[key] = String(env[key]); } diff --git a/src/vs/platform/utilityProcess/test/common/envKey.test.ts b/src/vs/platform/utilityProcess/test/common/envKey.test.ts new file mode 100644 index 00000000000000..980dde10fe4431 --- /dev/null +++ b/src/vs/platform/utilityProcess/test/common/envKey.test.ts @@ -0,0 +1,50 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { isValidEnvVariableKey } from '../common/envKey.js'; + +suite('envKey', () => { + + test('accepts common valid variable names', () => { + assert.strictEqual(isValidEnvVariableKey('PATH'), true); + assert.strictEqual(isValidEnvVariableKey('HOME'), true); + assert.strictEqual(isValidEnvVariableKey('USERNAME'), true); + assert.strictEqual(isValidEnvVariableKey('VSCODE_ESM_ENTRYPOINT'), true); + assert.strictEqual(isValidEnvVariableKey('NODE_UNC_HOST_ALLOWLIST'), true); + assert.strictEqual(isValidEnvVariableKey('_private'), true); + }); + + test('accepts digits after a leading letter/underscore', () => { + assert.strictEqual(isValidEnvVariableKey('a1'), true); + assert.strictEqual(isValidEnvVariableKey('a1b2'), true); + assert.strictEqual(isValidEnvVariableKey('ABC_123'), true); + }); + + test('rejects names starting with a digit', () => { + assert.strictEqual(isValidEnvVariableKey('1'), false); + assert.strictEqual(isValidEnvVariableKey('1HOME'), false); + assert.strictEqual(isValidEnvVariableKey('123'), false); + assert.strictEqual(isValidEnvVariableKey('0a'), false); + }); + + test('rejects names containing illegal characters, regardless of case', () => { + assert.strictEqual(isValidEnvVariableKey('a-b'), false); // hyphen + assert.strictEqual(isValidEnvVariableKey('a b'), false); // space + assert.strictEqual(isValidEnvVariableKey('a+b'), false); // plus + assert.strictEqual(isValidEnvVariableKey('a(b)'), false); // parens + assert.strictEqual(isValidEnvVariableKey('clion_g++'), false); // real-world case + assert.strictEqual(isValidEnvVariableKey('CLION_G++'), false); // case variant + assert.strictEqual(isValidEnvVariableKey('IntelliJ IDEA'), false); + assert.strictEqual(isValidEnvVariableKey('CommonProgramFiles(x86)'), false); + assert.strictEqual(isValidEnvVariableKey('ProgramFiles(x86)'), false); + assert.strictEqual(isValidEnvVariableKey('PROGRAMFILES(X86)'), false); + assert.strictEqual(isValidEnvVariableKey('CommonProgramFiles(X86)'), false); + }); + + test('rejects empty and non-string inputs', () => { + assert.strictEqual(isValidEnvVariableKey(''), false); + }); +}); \ No newline at end of file