From 0ad4e82a024aa97d085e520d3a81710d5ed9b014 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Tue, 10 Feb 2026 10:22:04 -0800 Subject: [PATCH] Redact keys in trace logs Fixes #294158 --- src/vs/platform/terminal/node/ptyService.ts | 23 ++++- .../terminal/node/terminalEnvironment.ts | 53 +++++++++++ .../platform/terminal/node/terminalProcess.ts | 5 +- .../test/node/terminalEnvironment.test.ts | 90 ++++++++++++++++++- 4 files changed, 166 insertions(+), 5 deletions(-) diff --git a/src/vs/platform/terminal/node/ptyService.ts b/src/vs/platform/terminal/node/ptyService.ts index a600f7b27d1..826b09f47c8 100644 --- a/src/vs/platform/terminal/node/ptyService.ts +++ b/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 (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); diff --git a/src/vs/platform/terminal/node/terminalEnvironment.ts b/src/vs/platform/terminal/node/terminalEnvironment.ts index 1d150cd6b8e..62a0c53bb88 100644 --- a/src/vs/platform/terminal/node/terminalEnvironment.ts +++ b/src/vs/platform/terminal/node/terminalEnvironment.ts @@ -375,3 +375,56 @@ function areZshBashFishLoginArgs(originalArgs: SingleOrMany): 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] = ''; + 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 ? '' : value; + } + return sanitized; +} diff --git a/src/vs/platform/terminal/node/terminalProcess.ts b/src/vs/platform/terminal/node/terminalProcess.ts index aba2699e665..968009e20e6 100644 --- a/src/vs/platform/terminal/node/terminalProcess.ts +++ b/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 { 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)); diff --git a/src/vs/platform/terminal/test/node/terminalEnvironment.test.ts b/src/vs/platform/terminal/test/node/terminalEnvironment.test.ts index a4d17386a22..bb03a05958b 100644 --- a/src/vs/platform/terminal/test/node/terminalEnvironment.test.ts +++ b/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: '', + GITHUB_TOKEN: '', + MY_SECRET: '', + PASSWORD: '', + AWS_ACCESS_KEY: '', + DATABASE_PASSWORD: '', + CLIENT_SECRET: '', + AUTH_TOKEN: '', + PRIVATE_KEY: '' + }); + }); + + test('should redact JWT tokens by value pattern', () => { + deepStrictEqual(sanitizeEnvForLogging({ + SOME_VAR: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U' + }), { + SOME_VAR: '' + }); + }); + + test('should redact GitHub tokens by value pattern', () => { + deepStrictEqual(sanitizeEnvForLogging({ + MY_GH: 'ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' + }), { + MY_GH: '' + }); + }); + + test('should redact Google API keys by value pattern', () => { + deepStrictEqual(sanitizeEnvForLogging({ + GOOGLE_KEY: 'AIzaSyDaGmWKa4JsXZ-HjGw7ISLn_3namBGewQe' + }), { + GOOGLE_KEY: '' + }); + }); + + test('should redact long alphanumeric strings (potential secrets)', () => { + deepStrictEqual(sanitizeEnvForLogging({ + LONG_VALUE: 'abcdefghijklmnopqrstuvwxyz123456' + }), { + LONG_VALUE: '' + }); + }); + + test('should skip undefined values', () => { + const env: { [key: string]: string | undefined } = { + DEFINED: 'value', + UNDEFINED: undefined + }; + deepStrictEqual(sanitizeEnvForLogging(env), { + DEFINED: 'value' + }); + }); + }); });