Merge pull request microsoft#256793 from microsoft/tyriar/256280_commandlines__explain

Explain why a command/command line was auto approved or not

Author: Tyriar

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/commandLineAutoApprover.ts

@@ -9,11 +9,16 @@ import { IConfigurationService } from '../../../../../platform/configuration/com
 import { TerminalChatAgentToolsSettingId } from '../common/terminalChatAgentToolsConfiguration.js';
 import { isPowerShell } from './runInTerminalHelpers.js';
 
+interface IAutoApproveRule {
+	regex: RegExp;
+	sourceText: string;
+}
+
 export class CommandLineAutoApprover extends Disposable {
-	private _denyListRegexes: RegExp[] = [];
-	private _allowListRegexes: RegExp[] = [];
-	private _allowListCommandLineRegexes: RegExp[] = [];
-	private _denyListCommandLineRegexes: RegExp[] = [];
+	private _denyListRules: IAutoApproveRule[] = [];
+	private _allowListRules: IAutoApproveRule[] = [];
+	private _allowListCommandLineRules: IAutoApproveRule[] = [];
+	private _denyListCommandLineRules: IAutoApproveRule[] = [];
 
 	constructor(
 		@IConfigurationService private readonly _configurationService: IConfigurationService,
@@ -28,49 +33,49 @@ export class CommandLineAutoApprover extends Disposable {
 	}
 
 	updateConfiguration() {
-		const { denyList, allowList, allowListCommandLine, denyListCommandLine } = this._mapAutoApproveConfigToRegexList(this._configurationService.getValue(TerminalChatAgentToolsSettingId.AutoApprove));
-		this._allowListRegexes = allowList;
-		this._denyListRegexes = denyList;
-		this._allowListCommandLineRegexes = allowListCommandLine;
-		this._denyListCommandLineRegexes = denyListCommandLine;
+		const { denyListRules, allowListRules, allowListCommandLineRules, denyListCommandLineRules } = this._mapAutoApproveConfigToRules(this._configurationService.getValue(TerminalChatAgentToolsSettingId.AutoApprove));
+		this._allowListRules = allowListRules;
+		this._denyListRules = denyListRules;
+		this._allowListCommandLineRules = allowListCommandLineRules;
+		this._denyListCommandLineRules = denyListCommandLineRules;
 	}
 
-	isCommandAutoApproved(command: string, shell: string, os: OperatingSystem): boolean {
+	isCommandAutoApproved(command: string, shell: string, os: OperatingSystem): { isAutoApproved: boolean; reason: string } {
 		// Check the deny list to see if this command requires explicit approval
-		for (const regex of this._denyListRegexes) {
-			if (this._commandMatchesRegex(regex, command, shell, os)) {
-				return false;
+		for (const rule of this._denyListRules) {
+			if (this._commandMatchesRegex(rule.regex, command, shell, os)) {
+				return { isAutoApproved: false, reason: `Command '${command}' is denied by deny list rule: ${rule.sourceText}` };
 			}
 		}
 
 		// Check the allow list to see if the command is allowed to run without explicit approval
-		for (const regex of this._allowListRegexes) {
-			if (this._commandMatchesRegex(regex, command, shell, os)) {
-				return true;
+		for (const rule of this._allowListRules) {
+			if (this._commandMatchesRegex(rule.regex, command, shell, os)) {
+				return { isAutoApproved: true, reason: `Command '${command}' is approved by allow list rule: ${rule.sourceText}` };
 			}
 		}
 
 		// TODO: LLM-based auto-approval https://github.com/microsoft/vscode/issues/253267
 
 		// Fallback is always to require approval
-		return false;
+		return { isAutoApproved: false, reason: `Command '${command}' has no matching auto approve entries` };
 	}
 
-	isCommandLineAutoApproved(commandLine: string): boolean {
+	isCommandLineAutoApproved(commandLine: string): { isAutoApproved: boolean; reason: string } {
 		// Check the deny list first to see if this command line requires explicit approval
-		for (const regex of this._denyListCommandLineRegexes) {
-			if (regex.test(commandLine)) {
-				return false;
+		for (const rule of this._denyListCommandLineRules) {
+			if (rule.regex.test(commandLine)) {
+				return { isAutoApproved: false, reason: `Command line '${commandLine}' is denied by deny list rule: ${rule.sourceText}` };
 			}
 		}
 
 		// Check if the full command line matches any of the allow list command line regexes
-		for (const regex of this._allowListCommandLineRegexes) {
-			if (regex.test(commandLine)) {
-				return true;
+		for (const rule of this._allowListCommandLineRules) {
+			if (rule.regex.test(commandLine)) {
+				return { isAutoApproved: true, reason: `Command line '${commandLine}' is approved by allow list rule: ${rule.sourceText}` };
 			}
 		}
-		return false;
+		return { isAutoApproved: false, reason: `Command line '${commandLine}' has no matching auto approve entries` };
 	}
 
 	private _commandMatchesRegex(regex: RegExp, command: string, shell: string, os: OperatingSystem): boolean {
@@ -86,24 +91,34 @@ export class CommandLineAutoApprover extends Disposable {
 		return false;
 	}
 
-	private _mapAutoApproveConfigToRegexList(config: unknown): { denyList: RegExp[]; allowList: RegExp[]; allowListCommandLine: RegExp[]; denyListCommandLine: RegExp[] } {
+	private _mapAutoApproveConfigToRules(config: unknown): {
+		denyListRules: IAutoApproveRule[];
+		allowListRules: IAutoApproveRule[];
+		allowListCommandLineRules: IAutoApproveRule[];
+		denyListCommandLineRules: IAutoApproveRule[];
+	} {
 		if (!config || typeof config !== 'object') {
-			return { denyList: [], allowList: [], allowListCommandLine: [], denyListCommandLine: [] };
+			return {
+				denyListRules: [],
+				allowListRules: [],
+				allowListCommandLineRules: [],
+				denyListCommandLineRules: []
+			};
 		}
 
-		const denyList: RegExp[] = [];
-		const allowList: RegExp[] = [];
-		const allowListCommandLine: RegExp[] = [];
-		const denyListCommandLine: RegExp[] = [];
+		const denyListRules: IAutoApproveRule[] = [];
+		const allowListRules: IAutoApproveRule[] = [];
+		const allowListCommandLineRules: IAutoApproveRule[] = [];
+		const denyListCommandLineRules: IAutoApproveRule[] = [];
 
 		Object.entries(config).forEach(([key, value]) => {
 			if (typeof value === 'boolean') {
 				const regex = this._convertAutoApproveEntryToRegex(key);
 				// IMPORTANT: Only true and false are used, null entries need to be ignored
 				if (value === true) {
-					allowList.push(regex);
+					allowListRules.push({ regex, sourceText: key });
 				} else if (value === false) {
-					denyList.push(regex);
+					denyListRules.push({ regex, sourceText: key });
 				}
 			} else if (typeof value === 'object' && value !== null) {
 				// Handle object format like { approve: true/false, matchCommandLine: true/false }
@@ -112,22 +127,27 @@ export class CommandLineAutoApprover extends Disposable {
 					const regex = this._convertAutoApproveEntryToRegex(key);
 					if (objectValue.approve === true) {
 						if (objectValue.matchCommandLine === true) {
-							allowListCommandLine.push(regex);
+							allowListCommandLineRules.push({ regex, sourceText: key });
 						} else {
-							allowList.push(regex);
+							allowListRules.push({ regex, sourceText: key });
 						}
 					} else if (objectValue.approve === false) {
 						if (objectValue.matchCommandLine === true) {
-							denyListCommandLine.push(regex);
+							denyListCommandLineRules.push({ regex, sourceText: key });
 						} else {
-							denyList.push(regex);
+							denyListRules.push({ regex, sourceText: key });
 						}
 					}
 				}
 			}
 		});
 
-		return { denyList, allowList, allowListCommandLine, denyListCommandLine };
+		return {
+			denyListRules,
+			allowListRules,
+			allowListCommandLineRules,
+			denyListCommandLineRules
+		};
 	}
 
 	private _convertAutoApproveEntryToRegex(value: string): RegExp {

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalTool.ts

@@ -171,22 +171,34 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
 			const subCommands = splitCommandLineIntoSubCommands(args.command, shell, os);
 			const inlineSubCommands = subCommands.map(e => Array.from(extractInlineSubCommands(e, shell, os))).flat();
 			const allSubCommands = [...subCommands, ...inlineSubCommands];
-			if (allSubCommands.every(e => this._commandLineAutoApprover.isCommandAutoApproved(e, shell, os))) {
+			const subCommandResults = allSubCommands.map(e => this._commandLineAutoApprover.isCommandAutoApproved(e, shell, os));
+			const autoApproveReasons: string[] = [...subCommandResults.map(e => e.reason)];
+
+			if (subCommandResults.every(e => e.isAutoApproved)) {
+				this._logService.info('autoApprove: All sub-commands auto-approved');
 				confirmationMessages = undefined;
 			} else {
-				if (this._commandLineAutoApprover.isCommandLineAutoApproved(args.command)) {
+				this._logService.info('autoApprove: All sub-commands NOT auto-approved');
+				const commandLineResults = this._commandLineAutoApprover.isCommandLineAutoApproved(args.command);
+				autoApproveReasons.push(commandLineResults.reason);
+				if (commandLineResults.isAutoApproved) {
+					this._logService.info('autoApprove: Command line auto-approved');
 					confirmationMessages = undefined;
 				} else {
+					this._logService.info('autoApprove: Command line NOT auto-approved');
 					confirmationMessages = {
 						title: args.isBackground
 							? localize('runInTerminal.background', "Run command in background terminal")
 							: localize('runInTerminal.foreground', "Run command in terminal"),
-						message: new MarkdownString(
-							args.explanation
-						),
+						message: new MarkdownString(args.explanation),
 					};
 				}
 			}
+
+			// TODO: Surface reason on tool part https://github.com/microsoft/vscode/pull/256793
+			for (const reason of autoApproveReasons) {
+				this._logService.info(`- ${reason}`);
+			}
 		}
 
 		const instance = context.chatSessionId ? this._sessionTerminalAssociations.get(context.chatSessionId)?.instance : undefined;

src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts

@@ -35,7 +35,7 @@ export const terminalChatAgentToolsConfiguration: IStringDictionary<IConfigurati
 			localize('autoApprove.description.intro', "A list of commands or regular expressions that control whether the run in terminal tool commands require explicit approval. These will be matched against the start of a command. A regular expression can be provided by wrapping the string in {0} characters followed by optional flags such as {1} for case-insensitivity.", '`/`', '`i`'),
 			localize('autoApprove.description.values', "Set to {0} to automatically approve commands, {1} to always require explicit approval or {2} to unset the value.", '`true`', '`false`', '`null`'),
 			localize('autoApprove.description.subCommands', "Note that these commands and regular expressions are evaluated for every _sub-command_ within the full _command line_, so {0} for example will need both {1} and {2} to match a {3} entry and must not match a {4} entry in order to auto approve. Inline commands are also detected so {5} will need both {5} and {6} to pass.", '`foo && bar`', '`foo`', '`bar`', '`true`', '`false`', '`echo $(rm file)`', '`rm file`'),
-			localize('autoApprove.description.commandLine', "An object can be used to match against the full command line instead of matching sub-commands and inline commands, for example {0}. This will be checked _after_ sub-commands are checked, so denied sub-commands will take precedence.", '`{ approve: false, matchCommandLine: true }`'),
+			localize('autoApprove.description.commandLine', "An object can be used to match against the full command line instead of matching sub-commands and inline commands, for example {0}. This will be checked _after_ sub-commands are checked, taking precedence over even denied sub-commands.", '`{ approve: false, matchCommandLine: true }`'),
 			[
 				localize('autoApprove.description.examples.title', 'Examples:'),
 				`|${localize('autoApprove.description.examples.value', "Value")}|${localize('autoApprove.description.examples.description', "Description")}|`,

src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/commandLineAutoApprover.test.ts

@@ -11,7 +11,7 @@ import { workbenchInstantiationService } from '../../../../../test/browser/workb
 import { TerminalChatAgentToolsSettingId } from '../../common/terminalChatAgentToolsConfiguration.js';
 import { CommandLineAutoApprover } from '../../browser/commandLineAutoApprover.js';
 import { ConfigurationTarget } from '../../../../../../platform/configuration/common/configuration.js';
-import { ok } from 'assert';
+import { ok, strictEqual } from 'assert';
 
 suite('CommandLineAutoApprover', () => {
 	const store = ensureNoDisposablesAreLeakedInTestSuite();
@@ -38,6 +38,10 @@ suite('CommandLineAutoApprover', () => {
 		setConfig(TerminalChatAgentToolsSettingId.AutoApprove, value);
 	}
 
+	function setAutoApproveWithCommandLine(value: { [key: string]: { approve: boolean; matchCommandLine?: boolean } | boolean }) {
+		setConfig(TerminalChatAgentToolsSettingId.AutoApprove, value);
+	}
+
 	function setConfig(key: string, value: unknown) {
 		configurationService.setUserConfiguration(key, value);
 		configurationService.onDidChangeConfigurationEmitter.fire({
@@ -49,11 +53,11 @@ suite('CommandLineAutoApprover', () => {
 	}
 
 	function isAutoApproved(commandLine: string): boolean {
-		return commandLineAutoApprover.isCommandAutoApproved(commandLine, shell, os);
+		return commandLineAutoApprover.isCommandAutoApproved(commandLine, shell, os).isAutoApproved;
 	}
 
 	function isCommandLineAutoApproved(commandLine: string): boolean {
-		return commandLineAutoApprover.isCommandLineAutoApproved(commandLine);
+		return commandLineAutoApprover.isCommandLineAutoApproved(commandLine).isAutoApproved;
 	}
 
 	suite('autoApprove with allow patterns only', () => {
@@ -331,10 +335,6 @@ suite('CommandLineAutoApprover', () => {
 	});
 
 	suite('isCommandLineAutoApproved - matchCommandLine functionality', () => {
-		function setAutoApproveWithCommandLine(value: { [key: string]: { approve: boolean; matchCommandLine: boolean } | boolean }) {
-			setConfig(TerminalChatAgentToolsSettingId.AutoApprove, value);
-		}
-
 		test('should auto-approve command line patterns with matchCommandLine: true', () => {
 			setAutoApproveWithCommandLine({
 				"echo": { approve: true, matchCommandLine: true }
@@ -440,4 +440,44 @@ suite('CommandLineAutoApprover', () => {
 			ok(!isCommandLineAutoApproved('powershell -File script.ps1 -ExecutionPolicy Bypass'));
 		});
 	});
+
+	suite('reasons', () => {
+		function getCommandReason(command: string): string {
+			return commandLineAutoApprover.isCommandAutoApproved(command, shell, os).reason;
+		}
+
+		function getCommandLineReason(commandLine: string): string {
+			return commandLineAutoApprover.isCommandLineAutoApproved(commandLine).reason;
+		}
+
+		suite('command', () => {
+			test('approved', () => {
+				setAutoApprove({ echo: true });
+				strictEqual(getCommandReason('echo hello'), `Command 'echo hello' is approved by allow list rule: echo`);
+			});
+			test('not approved', () => {
+				setAutoApprove({ echo: false });
+				strictEqual(getCommandReason('echo hello'), `Command 'echo hello' is denied by deny list rule: echo`);
+			});
+			test('no match', () => {
+				setAutoApprove({});
+				strictEqual(getCommandReason('echo hello'), `Command 'echo hello' has no matching auto approve entries`);
+			});
+		});
+
+		suite('command line', () => {
+			test('approved', () => {
+				setAutoApproveWithCommandLine({ echo: { approve: true, matchCommandLine: true } });
+				strictEqual(getCommandLineReason('echo hello'), `Command line 'echo hello' is approved by allow list rule: echo`);
+			});
+			test('not approved', () => {
+				setAutoApproveWithCommandLine({ echo: { approve: false, matchCommandLine: true } });
+				strictEqual(getCommandLineReason('echo hello'), `Command line 'echo hello' is denied by deny list rule: echo`);
+			});
+			test('no match', () => {
+				setAutoApproveWithCommandLine({});
+				strictEqual(getCommandLineReason('echo hello'), `Command line 'echo hello' has no matching auto approve entries`);
+			});
+		});
+	});
 });