Merge pull request microsoft#170193 from microsoft/tyriar/unsafe_path

Support detecting terminal profile with potentially unsafe paths and add an opt-in

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

@@ -755,6 +755,13 @@ export interface ITerminalProfile {
 	profileName: string;
 	path: string;
 	isDefault: boolean;
+	/**
+	 * Whether the terminal profile contains a potentially unsafe path. For example, the path
+	 *  `C:\Cygwin` is the default install for Cygwin on Windows, but it could be created by any
+	 * user in a multi-user environment. As such, we don't want to blindly present it as a profile
+	 * without a warning.
+	 */
+	isUnsafePath?: boolean;
 	isAutoDetected?: boolean;
 	/**
 	 * Whether the profile path was found on the `$PATH` environment variable, if so it will be
@@ -789,8 +796,15 @@ export interface IBaseUnresolvedTerminalProfile {
 	env?: ITerminalEnvironment;
 }
 
+type OneOrN<T> = T | T[];
+
+export interface ITerminalUnsafePath {
+	path: string;
+	isUnsafe: true;
+}
+
 export interface ITerminalExecutable extends IBaseUnresolvedTerminalProfile {
-	path: string | string[];
+	path: OneOrN<string | ITerminalUnsafePath>;
 }
 
 export interface ITerminalProfileSource extends IBaseUnresolvedTerminalProfile {

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

@@ -12,7 +12,7 @@ import * as pfs from 'vs/base/node/pfs';
 import { enumeratePowerShellInstallations } from 'vs/base/node/powershell';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { ILogService } from 'vs/platform/log/common/log';
-import { ITerminalEnvironment, ITerminalExecutable, ITerminalProfile, ITerminalProfileSource, ProfileSource, TerminalIcon, TerminalSettingId } from 'vs/platform/terminal/common/terminal';
+import { ITerminalEnvironment, ITerminalExecutable, ITerminalProfile, ITerminalProfileSource, ITerminalUnsafePath, ProfileSource, TerminalIcon, TerminalSettingId } from 'vs/platform/terminal/common/terminal';
 import { findExecutable, getWindowsBuildNumber } from 'vs/platform/terminal/node/terminalEnvironment';
 import { ThemeIcon } from 'vs/platform/theme/common/themeService';
 
@@ -108,6 +108,22 @@ async function detectAvailableWindowsProfiles(
 			icon: Codicon.terminalCmd,
 			isAutoDetected: true
 		});
+		detectedProfiles.set('Cygwin', {
+			path: [
+				{ path: `${process.env['HOMEDRIVE']}\\cygwin64\\bin\\bash.exe`, isUnsafe: true },
+				{ path: `${process.env['HOMEDRIVE']}\\cygwin\\bin\\bash.exe`, isUnsafe: true }
+			],
+			args: ['--login'],
+			isAutoDetected: true
+		});
+		detectedProfiles.set('bash (MSYS2)', {
+			path: [
+				{ path: `${process.env['HOMEDRIVE']}\\msys64\\usr\\bin\\bash.exe`, isUnsafe: true },
+			],
+			args: ['--login', '-i'],
+			icon: Codicon.terminalBash,
+			isAutoDetected: true
+		});
 	}
 
 	applyConfigProfilesToMap(configProfiles, detectedProfiles);
@@ -144,7 +160,7 @@ async function transformToTerminalProfiles(
 	const resultProfiles: ITerminalProfile[] = [];
 	for (const [profileName, profile] of entries) {
 		if (profile === null) { continue; }
-		let originalPaths: string[];
+		let originalPaths: (string | ITerminalUnsafePath)[];
 		let args: string[] | string | undefined;
 		let icon: ThemeIcon | URI | { light: URI; dark: URI } | undefined = undefined;
 		if ('source' in profile) {
@@ -167,7 +183,26 @@ async function transformToTerminalProfiles(
 			icon = validateIcon(profile.icon);
 		}
 
-		const paths = (await variableResolver?.(originalPaths)) || originalPaths.slice();
+		let paths: (string | ITerminalUnsafePath)[];
+		if (variableResolver) {
+			// Convert to string[] for resolve
+			const mapped = originalPaths.map(e => typeof e === 'string' ? e : e.path);
+			const resolved = await variableResolver(mapped);
+			// Convert resolved back to (T | string)[]
+			paths = new Array(originalPaths.length);
+			for (let i = 0; i < originalPaths.length; i++) {
+				if (typeof originalPaths[i] === 'string') {
+					paths[i] = originalPaths[i];
+				} else {
+					paths[i] = {
+						path: resolved[i],
+						isUnsafe: true
+					};
+				}
+			}
+		} else {
+			paths = originalPaths.slice();
+		}
 		const validatedProfile = await validateProfilePaths(profileName, defaultProfileName, paths, fsProvider, shellEnv, args, profile.env, profile.overrideName, profile.isAutoDetected, logService);
 		if (validatedProfile) {
 			validatedProfile.isAutoDetected = profile.isAutoDetected;
@@ -328,22 +363,33 @@ function applyConfigProfilesToMap(configProfiles: { [key: string]: IUnresolvedTe
 	}
 }
 
-async function validateProfilePaths(profileName: string, defaultProfileName: string | undefined, potentialPaths: string[], fsProvider: IFsProvider, shellEnv: typeof process.env, args?: string[] | string, env?: ITerminalEnvironment, overrideName?: boolean, isAutoDetected?: boolean, logService?: ILogService): Promise<ITerminalProfile | undefined> {
+async function validateProfilePaths(profileName: string, defaultProfileName: string | undefined, potentialPaths: (string | ITerminalUnsafePath)[], fsProvider: IFsProvider, shellEnv: typeof process.env, args?: string[] | string, env?: ITerminalEnvironment, overrideName?: boolean, isAutoDetected?: boolean, logService?: ILogService): Promise<ITerminalProfile | undefined> {
 	if (potentialPaths.length === 0) {
 		return Promise.resolve(undefined);
 	}
 	const path = potentialPaths.shift()!;
 	if (path === '') {
 		return validateProfilePaths(profileName, defaultProfileName, potentialPaths, fsProvider, shellEnv, args, env, overrideName, isAutoDetected);
 	}
-
-	const profile: ITerminalProfile = { profileName, path, args, env, overrideName, isAutoDetected, isDefault: profileName === defaultProfileName };
+	const isUnsafePath = typeof path !== 'string' && path.isUnsafe;
+	const actualPath = typeof path === 'string' ? path : path.path;
+
+	const profile: ITerminalProfile = {
+		profileName,
+		path: actualPath,
+		args,
+		env,
+		overrideName,
+		isAutoDetected,
+		isDefault: profileName === defaultProfileName,
+		isUnsafePath
+	};
 
 	// For non-absolute paths, check if it's available on $PATH
-	if (basename(path) === path) {
+	if (basename(actualPath) === actualPath) {
 		// The executable isn't an absolute path, try find it on the PATH
 		const envPaths: string[] | undefined = shellEnv.PATH ? shellEnv.PATH.split(delimiter) : undefined;
-		const executable = await findExecutable(path, undefined, envPaths, undefined, fsProvider.existsFile);
+		const executable = await findExecutable(actualPath, undefined, envPaths, undefined, fsProvider.existsFile);
 		if (!executable) {
 			return validateProfilePaths(profileName, defaultProfileName, potentialPaths, fsProvider, shellEnv, args);
 		}
@@ -352,7 +398,7 @@ async function validateProfilePaths(profileName: string, defaultProfileName: str
 		return profile;
 	}
 
-	const result = await fsProvider.existsFile(normalize(path));
+	const result = await fsProvider.existsFile(normalize(actualPath));
 	if (result) {
 		return profile;
 	}

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

@@ -16,6 +16,7 @@ import { IQuickPickTerminalObject, ITerminalInstance } from 'vs/workbench/contri
 import { IPickerQuickAccessItem } from 'vs/platform/quickinput/browser/pickerQuickAccess';
 import { getIconRegistry } from 'vs/platform/theme/common/iconRegistry';
 import { basename } from 'vs/base/common/path';
+import { INotificationService, Severity } from 'vs/platform/notification/common/notification';
 
 
 type DefaultProfileName = string;
@@ -25,7 +26,8 @@ export class TerminalProfileQuickpick {
 		@ITerminalProfileResolverService private readonly _terminalProfileResolverService: ITerminalProfileResolverService,
 		@IConfigurationService private readonly _configurationService: IConfigurationService,
 		@IQuickInputService private readonly _quickInputService: IQuickInputService,
-		@IThemeService private readonly _themeService: IThemeService
+		@IThemeService private readonly _themeService: IThemeService,
+		@INotificationService private readonly _notificationService: INotificationService
 	) { }
 
 	async showAndGetResult(type: 'setDefault' | 'createInstance'): Promise<IQuickPickTerminalObject | DefaultProfileName | undefined> {
@@ -103,6 +105,10 @@ export class TerminalProfileQuickpick {
 		const options: IPickOptions<IProfileQuickPickItem> = {
 			placeHolder: type === 'createInstance' ? nls.localize('terminal.integrated.selectProfileToCreate', "Select the terminal profile to create") : nls.localize('terminal.integrated.chooseDefaultProfile', "Select your default terminal profile"),
 			onDidTriggerItemButton: async (context) => {
+				// Get the user's explicit permission to use a potentially unsafe path
+				if (!await this._isProfileSafe(context.item.profile)) {
+					return;
+				}
 				if ('command' in context.item.profile) {
 					return;
 				}
@@ -197,12 +203,37 @@ export class TerminalProfileQuickpick {
 		if (!result) {
 			return undefined;
 		}
+		if (!await this._isProfileSafe(result.profile)) {
+			return undefined;
+		}
 		if (keyMods) {
 			result.keyMods = keyMods;
 		}
 		return result;
 	}
 
+	private async _isProfileSafe(profile: ITerminalProfile | IExtensionTerminalProfile): Promise<boolean> {
+		if (!('isUnsafePath' in profile) || profile.isUnsafePath === false) {
+			return true;
+		}
+
+		// Get the user's explicit permission to use a potentially unsafe path
+		return await new Promise<boolean>(r => {
+			const handle = this._notificationService.prompt(
+				Severity.Warning,
+				nls.localize('unsafePathWarning', 'This profile uses a potentially unsafe path that can be modified by another user: {0}. Are you use you want to use it?', `"${profile.path}"`),
+				[{
+					label: nls.localize('yes', 'Yes'),
+					run: () => r(true)
+				}, {
+					label: nls.localize('cancel', 'Cancel'),
+					run: () => r(false)
+				}]
+			);
+			handle.onDidClose(() => r(false));
+		});
+	}
+
 	private _createProfileQuickPickItem(profile: ITerminalProfile): IProfileQuickPickItem {
 		const buttons: IQuickInputButton[] = [{
 			iconClass: ThemeIcon.asClassName(configureTerminalProfileIcon),

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

@@ -243,12 +243,7 @@ export abstract class BaseTerminalProfileResolverService implements ITerminalPro
 	}
 
 	private _getUnresolvedRealDefaultProfile(os: OperatingSystem): ITerminalProfile | undefined {
-		const defaultProfileName = this._configurationService.getValue(`${TerminalSettingPrefix.DefaultProfile}${this._getOsKey(os)}`);
-		if (defaultProfileName && typeof defaultProfileName === 'string') {
-			return this._terminalProfileService.availableProfiles.find(e => e.profileName === defaultProfileName);
-		}
-
-		return undefined;
+		return this._terminalProfileService.getDefaultProfile(os);
 	}
 
 	private async _getUnresolvedShellSettingDefaultProfile(options: IShellLaunchConfigResolveOptions): Promise<ITerminalProfile | undefined> {

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

@@ -24,10 +24,12 @@ import { IExtensionService } from 'vs/workbench/services/extensions/common/exten
 import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
 
 /*
-* Links TerminalService with TerminalProfileResolverService
-* and keeps the available terminal profiles updated
-*/
+ * Links TerminalService with TerminalProfileResolverService
+ * and keeps the available terminal profiles updated
+ */
 export class TerminalProfileService implements ITerminalProfileService {
+	declare _serviceBrand: undefined;
+
 	private _webExtensionContributedProfileContextKey: IContextKey<boolean>;
 	private _profilesReadyBarrier: AutoOpenBarrier;
 	private _availableProfiles: ITerminalProfile[] | undefined;
@@ -49,6 +51,7 @@ export class TerminalProfileService implements ITerminalProfileService {
 	get contributedProfiles(): IExtensionTerminalProfile[] {
 		return this._contributedProfiles || [];
 	}
+
 	constructor(
 		@IContextKeyService private readonly _contextKeyService: IContextKeyService,
 		@IConfigurationService private readonly _configurationService: IConfigurationService,
@@ -91,12 +94,38 @@ export class TerminalProfileService implements ITerminalProfileService {
 		});
 	}
 
-	_serviceBrand: undefined;
-
 	getDefaultProfileName(): string | undefined {
 		return this._defaultProfileName;
 	}
 
+	getDefaultProfile(os?: OperatingSystem): ITerminalProfile | undefined {
+		let defaultProfileName: string | undefined;
+		if (os) {
+			const defaultProfileName = this._configurationService.getValue(`${TerminalSettingPrefix.DefaultProfile}${this._getOsKey(os)}`);
+			if (!defaultProfileName || typeof defaultProfileName !== 'string') {
+				return undefined;
+			}
+		} else {
+			defaultProfileName = this._defaultProfileName;
+		}
+		if (!defaultProfileName) {
+			return undefined;
+		}
+
+		// IMPORTANT: Only allow the default profile name to find non-auto detected profiles as
+		// to avoid unsafe path profiles being picked up.
+		return this.availableProfiles.find(e => e.profileName === this._defaultProfileName && !e.isAutoDetected);
+	}
+
+	private _getOsKey(os: OperatingSystem): string {
+		switch (os) {
+			case OperatingSystem.Linux: return 'linux';
+			case OperatingSystem.Macintosh: return 'osx';
+			case OperatingSystem.Windows: return 'windows';
+		}
+	}
+
+
 	@throttle(2000)
 	refreshAvailableProfiles(): void {
 		this._refreshAvailableProfilesNow();

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

@@ -934,7 +934,7 @@ export class TerminalService implements ITerminalService {
 			}
 		}
 
-		const config = options?.config || this._terminalProfileService.availableProfiles?.find(p => p.profileName === this._terminalProfileService.getDefaultProfileName());
+		const config = options?.config || this._terminalProfileService.getDefaultProfile();
 		const shellLaunchConfig = config && 'extensionIdentifier' in config ? {} : this._terminalInstanceService.convertProfileToShellLaunchConfig(config || {});
 
 		// Get the contributed profile if it was provided

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

@@ -80,6 +80,7 @@ export interface ITerminalProfileService {
 	getPlatformKey(): Promise<string>;
 	refreshAvailableProfiles(): void;
 	getDefaultProfileName(): string | undefined;
+	getDefaultProfile(os?: OperatingSystem): ITerminalProfile | undefined;
 	onDidChangeAvailableProfiles: Event<ITerminalProfile[]>;
 	getContributedDefaultProfile(shellLaunchConfig: IShellLaunchConfig): Promise<IExtensionTerminalProfile | undefined>;
 	registerContributedProfile(args: IRegisterContributedProfileArgs): Promise<void>;

src/vs/workbench/test/browser/workbenchTestServices.ts

@@ -1846,6 +1846,7 @@ export class TestTerminalProfileService implements ITerminalProfileService {
 	getPlatformKey(): Promise<string> { throw new Error('Method not implemented.'); }
 	refreshAvailableProfiles(): void { throw new Error('Method not implemented.'); }
 	getDefaultProfileName(): string | undefined { throw new Error('Method not implemented.'); }
+	getDefaultProfile(): ITerminalProfile | undefined { throw new Error('Method not implemented.'); }
 	getContributedDefaultProfile(shellLaunchConfig: IShellLaunchConfig): Promise<IExtensionTerminalProfile | undefined> { throw new Error('Method not implemented.'); }
 	registerContributedProfile(args: IRegisterContributedProfileArgs): Promise<void> { throw new Error('Method not implemented.'); }
 	getContributedProfileProvider(extensionIdentifier: string, id: string): ITerminalProfileProvider | undefined { throw new Error('Method not implemented.'); }