Use a nonce to verify si commands

Fixes #181522

src/vs/platform/terminal/common/terminal.ts

@@ -174,6 +174,7 @@ export interface IPtyHostAttachTarget {
 	isFeatureTerminal?: boolean;
 	type?: TerminalType;
 	hasChildProcesses: boolean;
+	shellIntegrationNonce: string;
 }
 
 export interface IReconnectionProperties {
@@ -483,7 +484,23 @@ export interface IShellLaunchConfig {
 	 * This is a terminal that attaches to an already running terminal.
 	 */
 	attachPersistentProcess?: {
-		id: number; findRevivedId?: boolean; pid: number; title: string; titleSource: TitleEventSource; cwd: string; icon?: TerminalIcon; color?: string; hasChildProcesses?: boolean; fixedDimensions?: IFixedTerminalDimensions; environmentVariableCollections?: ISerializableEnvironmentVariableCollections; reconnectionProperties?: IReconnectionProperties; type?: TerminalType; waitOnExit?: WaitOnExitValue; hideFromUser?: boolean; isFeatureTerminal?: boolean;
+		id: number;
+		findRevivedId?: boolean;
+		pid: number;
+		title: string;
+		titleSource: TitleEventSource;
+		cwd: string;
+		icon?: TerminalIcon;
+		color?: string;
+		hasChildProcesses?: boolean;
+		fixedDimensions?: IFixedTerminalDimensions;
+		environmentVariableCollections?: ISerializableEnvironmentVariableCollections;
+		reconnectionProperties?: IReconnectionProperties;
+		type?: TerminalType;
+		waitOnExit?: WaitOnExitValue;
+		hideFromUser?: boolean;
+		isFeatureTerminal?: boolean;
+		shellIntegrationNonce: string;
 	};
 
 	/**
@@ -598,6 +615,7 @@ export interface ITerminalProcessOptions {
 	shellIntegration: {
 		enabled: boolean;
 		suggestEnabled: boolean;
+		nonce: string;
 	};
 	windowsEnableConpty: boolean;
 	environmentVariableCollections: ISerializableEnvironmentVariableCollections | undefined;

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

@@ -66,6 +66,7 @@ export interface IProcessDetails {
 	isFeatureTerminal?: boolean;
 	type?: TerminalType;
 	hasChildProcesses: boolean;
+	shellIntegrationNonce: string;
 }
 
 export type ITerminalTabLayoutInfoDto = IRawTerminalTabLayoutInfo<IProcessDetails>;

src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts

@@ -111,6 +111,11 @@ const enum VSCodeOscPt {
 	 * "\n" -> "\x0a"
 	 * ";"  -> "\x3b"
 	 * ```
+	 *
+	 * An optional nonce can be provided which is may be required by the terminal in order enable
+	 * some features. This helps ensure no malicious command injection has occurred.
+	 *
+	 * Format: `OSC 633 ; E [; <CommandLine> [; <Nonce>]] ST`.
 	 */
 	CommandLine = 'E',
 
@@ -201,6 +206,7 @@ export class ShellIntegrationAddon extends Disposable implements IShellIntegrati
 	readonly onDidChangeStatus = this._onDidChangeStatus.event;
 
 	constructor(
+		private _nonce: string,
 		private readonly _disableTelemetry: boolean | undefined,
 		private readonly _telemetryService: ITelemetryService | undefined,
 		@ILogService private readonly _logService: ILogService
@@ -327,8 +333,11 @@ export class ShellIntegrationAddon extends Disposable implements IShellIntegrati
 				return true;
 			}
 			case VSCodeOscPt.CommandLine: {
+				// Only trust the command line if the optional nonce (according to the spec) is
+				// send in order to prevent spoofing. This is important as some interactions do not
+				// require verification before re-running a command.
 				let commandLine: string;
-				if (args.length === 1) {
+				if (args.length === 2 && args[1] === this._nonce) {
 					commandLine = deserializeMessage(args[0]);
 				} else {
 					commandLine = '';

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

@@ -482,7 +482,8 @@ export class PtyService extends Disposable implements IPtyService {
 			hideFromUser: persistentProcess.shellLaunchConfig.hideFromUser,
 			isFeatureTerminal: persistentProcess.shellLaunchConfig.isFeatureTerminal,
 			type: persistentProcess.shellLaunchConfig.type,
-			hasChildProcesses: persistentProcess.hasChildProcesses
+			hasChildProcesses: persistentProcess.hasChildProcesses,
+			shellIntegrationNonce: persistentProcess.processLaunchOptions.options.shellIntegration.nonce
 		};
 	}
 
@@ -609,6 +610,7 @@ class PersistentTerminalProcess extends Disposable {
 			reconnectConstants.scrollback,
 			unicodeVersion,
 			reviveBuffer,
+			processLaunchOptions.options.shellIntegration.nonce,
 			shouldPersistTerminal ? rawReviveBuffer : undefined,
 			this._logService
 		);
@@ -882,6 +884,7 @@ class XtermSerializer implements ITerminalSerializer {
 		scrollback: number,
 		unicodeVersion: '6' | '11',
 		reviveBufferWithRestoreMessage: string | undefined,
+		shellIntegrationNonce: string,
 		private _rawReviveBuffer: string | undefined,
 		logService: ILogService
 	) {
@@ -895,7 +898,7 @@ class XtermSerializer implements ITerminalSerializer {
 			this._xterm.writeln(reviveBufferWithRestoreMessage);
 		}
 		this.setUnicodeVersion(unicodeVersion);
-		this._shellIntegrationAddon = new ShellIntegrationAddon(true, undefined, logService);
+		this._shellIntegrationAddon = new ShellIntegrationAddon(shellIntegrationNonce, true, undefined, logService);
 		this._xterm.loadAddon(this._shellIntegrationAddon);
 	}
 

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

@@ -129,6 +129,10 @@ export function getShellIntegrationInjection(
 		'VSCODE_INJECTION': '1'
 	};
 
+	if (options.shellIntegration.nonce) {
+		envMixin['VSCODE_NONCE'] = options.shellIntegration.nonce;
+	}
+
 	// Windows
 	if (isWindows) {
 		if (shell === 'pwsh.exe' || shell === 'powershell.exe') {

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

@@ -11,9 +11,9 @@ import { IProductService } from 'vs/platform/product/common/productService';
 import { ITerminalProcessOptions } from 'vs/platform/terminal/common/terminal';
 import { getShellIntegrationInjection, getWindowsBuildNumber, IShellIntegrationConfigInjection } from 'vs/platform/terminal/node/terminalEnvironment';
 
-const enabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true, suggestEnabled: false }, windowsEnableConpty: true, environmentVariableCollections: undefined, workspaceFolder: undefined };
-const disabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: false, suggestEnabled: false }, windowsEnableConpty: true, environmentVariableCollections: undefined, workspaceFolder: undefined };
-const winptyProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true, suggestEnabled: false }, windowsEnableConpty: false, environmentVariableCollections: undefined, workspaceFolder: undefined };
+const enabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true, suggestEnabled: false, nonce: '' }, windowsEnableConpty: true, environmentVariableCollections: undefined, workspaceFolder: undefined };
+const disabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: false, suggestEnabled: false, nonce: '' }, windowsEnableConpty: true, environmentVariableCollections: undefined, workspaceFolder: undefined };
+const winptyProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true, suggestEnabled: false, nonce: '' }, windowsEnableConpty: false, environmentVariableCollections: undefined, workspaceFolder: undefined };
 const pwshExe = process.platform === 'win32' ? 'pwsh.exe' : 'pwsh';
 const repoRoot = process.platform === 'win32' ? process.cwd()[0].toLowerCase() + process.cwd().substring(1) : process.cwd();
 const logService = new NullLogService();

src/vs/workbench/contrib/terminal/browser/media/shellIntegration-bash.sh

@@ -140,6 +140,10 @@ __vsc_custom_PS2=""
 __vsc_in_command_execution="1"
 __vsc_current_command=""
 
+# It's fine this is in the global scope as it getting at it requires access to the shell environment
+__vsc_nonce="$VSCODE_NONCE"
+unset VSCODE_NONCE
+
 __vsc_prompt_start() {
 	builtin printf '\e]633;A\a'
 }
@@ -154,7 +158,7 @@ __vsc_update_cwd() {
 
 __vsc_command_output_start() {
 	builtin printf '\e]633;C\a'
-	builtin printf '\e]633;E;%s\a' "$(__vsc_escape_value "${__vsc_current_command}")"
+	builtin printf '\e]633;E;%s;%s\a' "$(__vsc_escape_value "${__vsc_current_command}")" $__vsc_nonce
 }
 
 __vsc_continuation_start() {

src/vs/workbench/contrib/terminal/browser/media/shellIntegration.ps1

@@ -17,6 +17,10 @@ $Global:__VSCodeOriginalPrompt = $function:Prompt
 
 $Global:__LastHistoryId = -1
 
+# Store the nonce in script scope and unset the global
+$Nonce = $env:VSCODE_NONCE
+$env:VSCODE_NONCE = $null
+
 if ($env:VSCODE_ENV_REPLACE) {
 	$Split = $env:VSCODE_ENV_REPLACE.Split(":")
 	foreach ($Item in $Split) {
@@ -67,7 +71,7 @@ function Global:Prompt() {
 			$Result += "$([char]0x1b)]633;D`a"
 		} else {
 			# Command finished command line
-			# OSC 633 ; A ; <CommandLine?> ST
+			# OSC 633 ; E ; <CommandLine?> ; <Nonce?> ST
 			$Result  = "$([char]0x1b)]633;E;"
 			# Sanitize the command line to ensure it can get transferred to the terminal and can be parsed
 			# correctly. This isn't entirely safe but good for most cases, it's important for the Pt parameter
@@ -78,6 +82,7 @@ function Global:Prompt() {
 				$CommandLine = ""
 			}
 			$Result += $(__VSCode-Escape-Value $CommandLine)
+			$Result += ";$Nonce"
 			$Result += "`a"
 			# Command finished exit code
 			# OSC 633 ; D [; <ExitCode>] ST

src/vs/workbench/contrib/terminal/browser/remoteTerminalBackend.ts

@@ -262,6 +262,7 @@ class RemoteTerminalBackend extends BaseTerminalBackend implements ITerminalBack
 	async listProcesses(): Promise<IProcessDetails[]> {
 		const terms = this._remoteTerminalChannel ? await this._remoteTerminalChannel.listProcesses() : [];
 		return terms.map(termDto => {
+			// TODO: This is an unsafe cast, why not just return channel.listProcesses?
 			return <IProcessDetails>{
 				id: termDto.id,
 				pid: termDto.pid,
@@ -273,7 +274,8 @@ class RemoteTerminalBackend extends BaseTerminalBackend implements ITerminalBack
 				icon: termDto.icon,
 				color: termDto.color,
 				isOrphan: termDto.isOrphan,
-				fixedDimensions: termDto.fixedDimensions
+				fixedDimensions: termDto.fixedDimensions,
+				shellIntegrationNonce: termDto.shellIntegrationNonce
 			};
 		});
 	}

src/vs/workbench/contrib/terminal/browser/terminal.ts

@@ -263,6 +263,7 @@ interface ITerminalEditorInputObject {
 	readonly isFeatureTerminal?: boolean;
 	readonly hideFromUser?: boolean;
 	readonly reconnectionProperties?: IReconnectionProperties;
+	readonly shellIntegrationNonce: string;
 }
 
 export interface ISerializedTerminalEditorInput extends ITerminalEditorInputObject {
@@ -651,6 +652,11 @@ export interface ITerminalInstance {
 	 */
 	userHome: string | undefined;
 
+	/**
+	 * The nonce used to verify commands coming from shell integration.
+	 */
+	shellIntegrationNonce: string;
+
 	/**
 	 * Registers and returns a marker
 	 */

src/vs/workbench/contrib/terminal/browser/terminalEditorSerializer.ts

@@ -46,7 +46,8 @@ export class TerminalInputSerializer implements IEditorSerializer {
 			hasChildProcesses: instance.hasChildProcesses,
 			isFeatureTerminal: instance.shellLaunchConfig.isFeatureTerminal,
 			hideFromUser: instance.shellLaunchConfig.hideFromUser,
-			reconnectionProperties: instance.shellLaunchConfig.reconnectionProperties
+			reconnectionProperties: instance.shellLaunchConfig.reconnectionProperties,
+			shellIntegrationNonce: instance.shellIntegrationNonce
 		};
 	}
 }

src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

@@ -305,6 +305,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		return undefined;
 	}
 	get userHome(): string | undefined { return this._userHome; }
+	get shellIntegrationNonce(): string { return this._processManager.shellIntegrationNonce; }
 	get injectedArgs(): string[] | undefined { return this._injectedArgs; }
 
 	// The onExit event is special in that it fires and is disposed after the terminal instance
@@ -751,6 +752,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 				}
 			},
 			this.capabilities,
+			this._processManager.shellIntegrationNonce,
 			this._terminalSuggestWidgetVisibleContextKey,
 			this.disableShellIntegrationReporting
 		);
@@ -1379,7 +1381,14 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 		if (this.shellLaunchConfig.attachPersistentProcess?.environmentVariableCollections) {
 			deserializedCollections = deserializeEnvironmentVariableCollections(this.shellLaunchConfig.attachPersistentProcess.environmentVariableCollections);
 		}
-		const processManager = this._scopedInstantiationService.createInstance(TerminalProcessManager, this._instanceId, this._configHelper, this.shellLaunchConfig?.cwd, deserializedCollections);
+		const processManager = this._scopedInstantiationService.createInstance(
+			TerminalProcessManager,
+			this._instanceId,
+			this._configHelper,
+			this.shellLaunchConfig?.cwd,
+			deserializedCollections,
+			this.shellLaunchConfig.attachPersistentProcess?.shellIntegrationNonce
+		);
 		this.capabilities.add(processManager.capabilities);
 		processManager.onProcessReady(async (e) => {
 			this._onProcessIdReady.fire(this);
@@ -1501,7 +1510,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 			}
 		});
 		if (this.xterm?.shellIntegration) {
-			this.capabilities.add(this.xterm?.shellIntegration.capabilities);
+			this.capabilities.add(this.xterm.shellIntegration.capabilities);
 		}
 		if (originalIcon !== this.shellLaunchConfig.icon || this.shellLaunchConfig.color) {
 			this._icon = this._shellLaunchConfig.attachPersistentProcess?.icon || this._shellLaunchConfig.icon;

src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts

@@ -39,6 +39,7 @@ import Severity from 'vs/base/common/severity';
 import { INotificationService } from 'vs/platform/notification/common/notification';
 import { IEnvironmentVariableCollection, IMergedEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
 import { getWorkspaceForTerminal } from 'vs/workbench/services/configurationResolver/common/terminalResolver';
+import { generateUuid } from 'vs/base/common/uuid';
 
 const enum ProcessConstants {
 	/**
@@ -74,6 +75,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 	environmentVariableInfo: IEnvironmentVariableInfo | undefined;
 	backend: ITerminalBackend | undefined;
 	readonly capabilities = new TerminalCapabilityStore();
+	readonly shellIntegrationNonce: string;
 
 	private _isDisposed: boolean = false;
 	private _process: ITerminalChildProcess | null = null;
@@ -130,6 +132,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 		private readonly _configHelper: ITerminalConfigHelper,
 		cwd: string | URI | undefined,
 		environmentVariableCollections: ReadonlyMap<string, IEnvironmentVariableCollection> | undefined,
+		shellIntegrationNonce: string | undefined,
 		@IHistoryService private readonly _historyService: IHistoryService,
 		@IInstantiationService private readonly _instantiationService: IInstantiationService,
 		@ILogService private readonly _logService: ILogService,
@@ -177,6 +180,8 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 			this.environmentVariableInfo = this._instantiationService.createInstance(EnvironmentVariableInfoChangesActive, this._extEnvironmentVariableCollection);
 			this._onEnvironmentVariableInfoChange.fire(this.environmentVariableInfo);
 		}
+
+		this.shellIntegrationNonce = shellIntegrationNonce ?? generateUuid();
 	}
 
 	async freePortKillProcess(port: string): Promise<void> {
@@ -279,6 +284,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 						shellIntegration: {
 							enabled: this._configurationService.getValue(TerminalSettingId.ShellIntegrationEnabled),
 							suggestEnabled: this._configurationService.getValue(TerminalSettingId.ShellIntegrationSuggestEnabled),
+							nonce: this.shellIntegrationNonce
 						},
 						windowsEnableConpty: this._configHelper.config.windowsEnableConpty,
 						environmentVariableCollections: this._extEnvironmentVariableCollection?.collections ? serializeEnvironmentVariableCollections(this._extEnvironmentVariableCollection.collections) : undefined,
@@ -470,6 +476,7 @@ export class TerminalProcessManager extends Disposable implements ITerminalProce
 			shellIntegration: {
 				enabled: this._configurationService.getValue(TerminalSettingId.ShellIntegrationEnabled),
 				suggestEnabled: this._configurationService.getValue(TerminalSettingId.ShellIntegrationSuggestEnabled),
+				nonce: this.shellIntegrationNonce
 			},
 			windowsEnableConpty: this._configHelper.config.windowsEnableConpty,
 			environmentVariableCollections: this._extEnvironmentVariableCollection ? serializeEnvironmentVariableCollections(this._extEnvironmentVariableCollection.collections) : undefined,

src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts

@@ -177,6 +177,7 @@ export class XtermTerminal extends DisposableStore implements IXtermTerminal, II
 		rows: number,
 		private readonly _backgroundColorProvider: IXtermColorProvider,
 		private readonly _capabilities: ITerminalCapabilityStore,
+		shellIntegrationNonce: string,
 		private readonly _terminalSuggestWidgetVisibleContextKey: IContextKey<boolean>,
 		disableShellIntegrationReporting: boolean,
 		@IConfigurationService private readonly _configurationService: IConfigurationService,
@@ -247,7 +248,7 @@ export class XtermTerminal extends DisposableStore implements IXtermTerminal, II
 		this._decorationAddon = this._instantiationService.createInstance(DecorationAddon, this._capabilities);
 		this._decorationAddon.onDidRequestRunCommand(e => this._onDidRequestRunCommand.fire(e));
 		this.raw.loadAddon(this._decorationAddon);
-		this._shellIntegrationAddon = this._instantiationService.createInstance(ShellIntegrationAddon, disableShellIntegrationReporting, this._telemetryService);
+		this._shellIntegrationAddon = this._instantiationService.createInstance(ShellIntegrationAddon, shellIntegrationNonce, disableShellIntegrationReporting, this._telemetryService);
 		this.raw.loadAddon(this._shellIntegrationAddon);
 
 		// Load the suggest addon, this should be loaded regardless of the setting as the sequences

src/vs/workbench/contrib/terminal/common/terminal.ts

@@ -338,6 +338,7 @@ export interface IRemoteTerminalAttachTarget {
 	icon: URI | { light: URI; dark: URI } | { id: string; color?: { id: string } } | undefined;
 	color: string | undefined;
 	fixedDimensions: IFixedTerminalDimensions | undefined;
+	shellIntegrationNonce: string;
 }
 
 export interface ITerminalCommand {
@@ -380,6 +381,7 @@ export interface ITerminalProcessManager extends IDisposable {
 	readonly hasChildProcesses: boolean;
 	readonly backend: ITerminalBackend | undefined;
 	readonly capabilities: ITerminalCapabilityStore;
+	readonly shellIntegrationNonce: string;
 	readonly extEnvironmentVariableCollection: IMergedEnvironmentVariableCollection | undefined;
 
 	readonly onPtyDisconnect: Event<void>;

src/vs/workbench/contrib/terminal/test/browser/terminalProcessManager.test.ts

@@ -104,7 +104,7 @@ suite('Workbench - TerminalProcessManager', () => {
 		instantiationService.stub(ITerminalInstanceService, new TestTerminalInstanceService());
 
 		const configHelper = instantiationService.createInstance(TerminalConfigHelper);
-		manager = instantiationService.createInstance(TerminalProcessManager, 1, configHelper, undefined, undefined);
+		manager = instantiationService.createInstance(TerminalProcessManager, 1, configHelper, undefined, undefined, undefined);
 	});
 
 	teardown(() => {

src/vs/workbench/contrib/terminal/test/browser/xterm/shellIntegrationAddon.test.ts

@@ -34,7 +34,7 @@ suite('ShellIntegrationAddon', () => {
 		xterm = new Terminal({ allowProposedApi: true, cols: 80, rows: 30 });
 		const instantiationService = new TestInstantiationService();
 		instantiationService.stub(ILogService, NullLogService);
-		shellIntegrationAddon = instantiationService.createInstance(TestShellIntegrationAddon, true, undefined);
+		shellIntegrationAddon = instantiationService.createInstance(TestShellIntegrationAddon, '', true, undefined);
 		xterm.loadAddon(shellIntegrationAddon);
 		capabilities = shellIntegrationAddon.capabilities;
 	});