Redact keys in trace logs

Fixes #294158

src/vs/platform/terminal/node/ptyService.ts

@@ -18,7 +18,7 @@ import { escapeNonWindowsPath } from '../common/terminalEnvironment.js';
 import type { ISerializeOptions, SerializeAddon as XtermSerializeAddon } from '@xterm/addon-serialize';
 import type { Unicode11Addon as XtermUnicode11Addon } from '@xterm/addon-unicode11';
 import { IGetTerminalLayoutInfoArgs, IProcessDetails, ISetTerminalLayoutInfoArgs, ITerminalTabLayoutInfoDto } from '../common/terminalProcess.js';
-import { getWindowsBuildNumber } from './terminalEnvironment.js';
+import { getWindowsBuildNumber, sanitizeEnvForLogging } from './terminalEnvironment.js';
 import { TerminalProcess } from './terminalProcess.js';
 import { localize } from '../../../nls.js';
 import { ignoreProcessNames } from './childProcessMonitor.js';
@@ -37,6 +37,24 @@ import { hasKey, isFunction, isNumber, isString } from '../../../base/common/typ
 type XtermTerminal = pkg.Terminal;
 const { Terminal: XtermTerminal } = pkg;
 
+/**
+ * Sanitizes arguments for logging, specifically handling env objects in createProcess calls.
+ */
+function sanitizeArgsForLogging(fnName: string, args: unknown[]): unknown[] {
+	// createProcess signature: shellLaunchConfig, cwd, cols, rows, unicodeVersion, env (index 5), executableEnv (index 6), ...
+	if (fnName === 'createProcess' && args.length > 5) {
+		const sanitizedArgs = [...args];
+		if (args[5] && typeof args[5] === 'object') {
+			sanitizedArgs[5] = sanitizeEnvForLogging(args[5] as IProcessEnvironment);
+		}
+		if (args[6] && typeof args[6] === 'object') {
+			sanitizedArgs[6] = sanitizeEnvForLogging(args[6] as IProcessEnvironment);
+		}
+		return sanitizedArgs;
+	}
+	return args;
+}
+
 interface ITraceRpcArgs {
 	logService: ILogService;
 	simulatedLatency: number;
@@ -50,7 +68,8 @@ export function traceRpc(_target: Object, key: string, descriptor: PropertyDescr
 	const fn = descriptor.value;
 	descriptor[fnKey] = async function <TThis extends { traceRpcArgs: ITraceRpcArgs }>(this: TThis, ...args: unknown[]) {
 		if (this.traceRpcArgs.logService.getLevel() === LogLevel.Trace) {
-			this.traceRpcArgs.logService.trace(`[RPC Request] PtyService#${fn.name}(${args.map(e => JSON.stringify(e)).join(', ')})`);
+			const sanitizedArgs = sanitizeArgsForLogging(fn.name, args);
+			this.traceRpcArgs.logService.trace(`[RPC Request] PtyService#${fn.name}(${sanitizedArgs.map(e => JSON.stringify(e)).join(', ')})`);
 		}
 		if (this.traceRpcArgs.simulatedLatency) {
 			await timeout(this.traceRpcArgs.simulatedLatency);

src/vs/platform/terminal/node/terminalEnvironment.ts

@@ -375,3 +375,56 @@ function areZshBashFishLoginArgs(originalArgs: SingleOrMany<string>): boolean {
 	return isString(originalArgs) && shLoginArgs.includes(originalArgs.toLowerCase())
 		|| !isString(originalArgs) && originalArgs.length === 1 && shLoginArgs.includes(originalArgs[0].toLowerCase());
 }
+
+/**
+ * Patterns that indicate sensitive environment variable names.
+ */
+const sensitiveEnvVarNames = /^(?:.*_)?(?:API_?KEY|TOKEN|SECRET|PASSWORD|PASSWD|PWD|CREDENTIAL|AUTH|PRIVATE_?KEY|ACCESS_?KEY|CLIENT_?SECRET|APIKEY)(?:_.*)?$/i;
+
+/**
+ * Patterns for detecting secret values in environment variables.
+ */
+const secretValuePatterns = [
+	// JWT tokens
+	/^eyJ[a-zA-Z0-9\-_]+\.[a-zA-Z0-9\-_]+\.[a-zA-Z0-9\-_]+$/,
+	// GitHub tokens
+	/^gh[psuro]_[a-zA-Z0-9]{36}$/,
+	/^github_pat_[a-zA-Z0-9]{22}_[a-zA-Z0-9]{59}$/,
+	// Google API keys
+	/^AIza[A-Za-z0-9_\-]{35}$/,
+	// Slack tokens
+	/^xox[pbar]\-[A-Za-z0-9\-]+$/,
+	// Azure/MS tokens (common patterns)
+	/^[a-zA-Z0-9]{32,}$/,
+];
+
+/**
+ * Sanitizes environment variables for logging by redacting sensitive values.
+ */
+export function sanitizeEnvForLogging(env: IProcessEnvironment | undefined): IProcessEnvironment | undefined {
+	if (!env) {
+		return env;
+	}
+	const sanitized: IProcessEnvironment = {};
+	for (const key of Object.keys(env)) {
+		const value = env[key];
+		if (value === undefined) {
+			continue;
+		}
+		// Check if the key name suggests a sensitive value
+		if (sensitiveEnvVarNames.test(key)) {
+			sanitized[key] = '<REDACTED>';
+			continue;
+		}
+		// Check if the value matches known secret patterns
+		let isSecret = false;
+		for (const pattern of secretValuePatterns) {
+			if (pattern.test(value)) {
+				isSecret = true;
+				break;
+			}
+		}
+		sanitized[key] = isSecret ? '<REDACTED>' : value;
+	}
+	return sanitized;
+}

src/vs/platform/terminal/node/terminalProcess.ts

@@ -17,7 +17,7 @@ import { ILogService, LogLevel } from '../../log/common/log.js';
 import { IProductService } from '../../product/common/productService.js';
 import { FlowControlConstants, IShellLaunchConfig, ITerminalChildProcess, ITerminalLaunchError, IProcessProperty, IProcessPropertyMap, ProcessPropertyType, TerminalShellType, IProcessReadyEvent, ITerminalProcessOptions, PosixShellType, IProcessReadyWindowsPty, GeneralShellType, ITerminalLaunchResult } from '../common/terminal.js';
 import { ChildProcessMonitor } from './childProcessMonitor.js';
-import { getShellIntegrationInjection, getWindowsBuildNumber, IShellIntegrationConfigInjection } from './terminalEnvironment.js';
+import { getShellIntegrationInjection, getWindowsBuildNumber, IShellIntegrationConfigInjection, sanitizeEnvForLogging } from './terminalEnvironment.js';
 import { WindowsShellHelper } from './windowsShellHelper.js';
 import { IPty, IPtyForkOptions, IWindowsPtyForkOptions, spawn } from 'node-pty';
 import { isNumber } from '../../../base/common/types.js';
@@ -301,7 +301,8 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess
 	): Promise<void> {
 		const args = shellIntegrationInjection?.newArgs || shellLaunchConfig.args || [];
 		await this._throttleKillSpawn();
-		this._logService.trace('node-pty.IPty#spawn', shellLaunchConfig.executable, args, options);
+		const sanitizedOptions = { ...options, env: sanitizeEnvForLogging(options.env as IProcessEnvironment | undefined) };
+		this._logService.trace('node-pty.IPty#spawn', shellLaunchConfig.executable, args, sanitizedOptions);
 		const ptyProcess = spawn(shellLaunchConfig.executable!, args, options);
 		this._ptyProcess = ptyProcess;
 		this._childProcessMonitor = this._register(new ChildProcessMonitor(ptyProcess.pid, this._logService));

src/vs/platform/terminal/test/node/terminalEnvironment.test.ts

@@ -10,7 +10,7 @@ import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/c
 import { NullLogService } from '../../../log/common/log.js';
 import { IProductService } from '../../../product/common/productService.js';
 import { ITerminalProcessOptions } from '../../common/terminal.js';
-import { getShellIntegrationInjection, getWindowsBuildNumber, IShellIntegrationConfigInjection, type IShellIntegrationInjectionFailure } from '../../node/terminalEnvironment.js';
+import { getShellIntegrationInjection, getWindowsBuildNumber, IShellIntegrationConfigInjection, type IShellIntegrationInjectionFailure, sanitizeEnvForLogging } from '../../node/terminalEnvironment.js';
 
 const enabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true, suggestEnabled: false, nonce: '' }, windowsUseConptyDll: false, environmentVariableCollections: undefined, workspaceFolder: undefined, isScreenReaderOptimized: false };
 const disabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: false, suggestEnabled: false, nonce: '' }, windowsUseConptyDll: false, environmentVariableCollections: undefined, workspaceFolder: undefined, isScreenReaderOptimized: false };
@@ -257,4 +257,92 @@ suite('platform - terminalEnvironment', async () => {
 			});
 		});
 	});
+
+	suite('sanitizeEnvForLogging', () => {
+		test('should return undefined for undefined input', () => {
+			strictEqual(sanitizeEnvForLogging(undefined), undefined);
+		});
+
+		test('should return empty object for empty input', () => {
+			deepStrictEqual(sanitizeEnvForLogging({}), {});
+		});
+
+		test('should pass through non-sensitive values', () => {
+			deepStrictEqual(sanitizeEnvForLogging({
+				PATH: '/usr/bin',
+				HOME: '/home/user',
+				TERM: 'xterm-256color'
+			}), {
+				PATH: '/usr/bin',
+				HOME: '/home/user',
+				TERM: 'xterm-256color'
+			});
+		});
+
+		test('should redact sensitive env var names', () => {
+			deepStrictEqual(sanitizeEnvForLogging({
+				API_KEY: 'secret123',
+				GITHUB_TOKEN: 'ghp_xxxx',
+				MY_SECRET: 'hidden',
+				PASSWORD: 'pass123',
+				AWS_ACCESS_KEY: 'AKIA...',
+				DATABASE_PASSWORD: 'dbpass',
+				CLIENT_SECRET: 'client_secret_value',
+				AUTH_TOKEN: 'auth_value',
+				PRIVATE_KEY: 'private_key_value'
+			}), {
+				API_KEY: '<REDACTED>',
+				GITHUB_TOKEN: '<REDACTED>',
+				MY_SECRET: '<REDACTED>',
+				PASSWORD: '<REDACTED>',
+				AWS_ACCESS_KEY: '<REDACTED>',
+				DATABASE_PASSWORD: '<REDACTED>',
+				CLIENT_SECRET: '<REDACTED>',
+				AUTH_TOKEN: '<REDACTED>',
+				PRIVATE_KEY: '<REDACTED>'
+			});
+		});
+
+		test('should redact JWT tokens by value pattern', () => {
+			deepStrictEqual(sanitizeEnvForLogging({
+				SOME_VAR: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U'
+			}), {
+				SOME_VAR: '<REDACTED>'
+			});
+		});
+
+		test('should redact GitHub tokens by value pattern', () => {
+			deepStrictEqual(sanitizeEnvForLogging({
+				MY_GH: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
+			}), {
+				MY_GH: '<REDACTED>'
+			});
+		});
+
+		test('should redact Google API keys by value pattern', () => {
+			deepStrictEqual(sanitizeEnvForLogging({
+				GOOGLE_KEY: 'AIzaSyDaGmWKa4JsXZ-HjGw7ISLn_3namBGewQe'
+			}), {
+				GOOGLE_KEY: '<REDACTED>'
+			});
+		});
+
+		test('should redact long alphanumeric strings (potential secrets)', () => {
+			deepStrictEqual(sanitizeEnvForLogging({
+				LONG_VALUE: 'abcdefghijklmnopqrstuvwxyz123456'
+			}), {
+				LONG_VALUE: '<REDACTED>'
+			});
+		});
+
+		test('should skip undefined values', () => {
+			const env: { [key: string]: string | undefined } = {
+				DEFINED: 'value',
+				UNDEFINED: undefined
+			};
+			deepStrictEqual(sanitizeEnvForLogging(env), {
+				DEFINED: 'value'
+			});
+		});
+	});
 });