Merge pull request #285925 from microsoft/tyriar/285921

Show message about denied commands with links

Author: Tyriar

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAnalyzer.ts

@@ -60,7 +60,7 @@ export interface ICommandLineAnalyzerResult {
 	 * - `undefined`: This analyzer does not make an approval/denial decision
 	 */
 	readonly isAutoApproved?: boolean;
-	readonly disclaimers?: readonly string[];
+	readonly disclaimers?: readonly (string | IMarkdownString)[];
 	readonly autoApproveInfo?: IMarkdownString;
 	readonly customActions?: ToolConfirmationAction[];
 }

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts

@@ -161,7 +161,7 @@ export class CommandLineAutoApproveAnalyzer extends Disposable implements IComma
 		});
 
 		// Prompt injection warning for common commands that return content from the web
-		const disclaimers: string[] = [];
+		const disclaimers: (string | IMarkdownString)[] = [];
 		const subCommandsLowerFirstWordOnly = subCommands.map(command => command.split(' ')[0].toLowerCase());
 		if (!isAutoApproved && (
 			subCommandsLowerFirstWordOnly.some(command => promptInjectionWarningCommandsLower.includes(command)) ||
@@ -170,6 +170,20 @@ export class CommandLineAutoApproveAnalyzer extends Disposable implements IComma
 			disclaimers.push(localize('runInTerminal.promptInjectionDisclaimer', 'Web content may contain malicious code or attempt prompt injection attacks.'));
 		}
 
+		// Add denial reason to disclaimers when auto-approve is enabled but command was denied by a rule
+		if (isAutoApproveEnabled && isDenied) {
+			const denialInfo = this._createAutoApproveInfo(
+				isAutoApproved,
+				isDenied,
+				autoApproveReason,
+				subCommandResults,
+				commandLineResult,
+			);
+			if (denialInfo) {
+				disclaimers.push(denialInfo);
+			}
+		}
+
 		if (!isAutoApproved && isAutoApproveEnabled) {
 			customActions = generateAutoApproveActions(trimmedCommandLine, subCommands, { subCommandResults, commandLineResult });
 		}
@@ -270,9 +284,9 @@ export class CommandLineAutoApproveAnalyzer extends Disposable implements IComma
 				case 'subCommand': {
 					const uniqueRules = dedupeRules(subCommandResults.filter(e => e.result === 'denied'));
 					if (uniqueRules.length === 1) {
-						return new MarkdownString(localize('autoApproveDenied.rule', 'Auto approval denied by rule {0}', formatRuleLinks(uniqueRules)));
+						return new MarkdownString(localize('autoApproveDenied.rule', 'Auto approval denied by rule {0}', formatRuleLinks(uniqueRules)), mdTrustSettings);
 					} else if (uniqueRules.length > 1) {
-						return new MarkdownString(localize('autoApproveDenied.rules', 'Auto approval denied by rules {0}', formatRuleLinks(uniqueRules)));
+						return new MarkdownString(localize('autoApproveDenied.rules', 'Auto approval denied by rules {0}', formatRuleLinks(uniqueRules)), mdTrustSettings);
 					}
 					break;
 				}

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

@@ -55,6 +55,7 @@ import { TerminalCommandArtifactCollector } from './terminalCommandArtifactColle
 import { isNumber, isString } from '../../../../../../base/common/types.js';
 import { ChatConfiguration } from '../../../../chat/common/constants.js';
 import { IChatWidgetService } from '../../../../chat/browser/chat.js';
+import { TerminalChatCommandId } from '../../../chat/browser/terminalChat.js';
 
 // #region Tool data
 
@@ -445,10 +446,15 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
 		};
 		const commandLineAnalyzerResults = await Promise.all(this._commandLineAnalyzers.map(e => e.analyze(commandLineAnalyzerOptions)));
 
-		const disclaimersRaw = commandLineAnalyzerResults.filter(e => e.disclaimers).flatMap(e => e.disclaimers);
+		const disclaimersRaw = commandLineAnalyzerResults.map(e => e.disclaimers).filter(e => !!e).flatMap(e => e);
 		let disclaimer: IMarkdownString | undefined;
 		if (disclaimersRaw.length > 0) {
-			disclaimer = new MarkdownString(`$(${Codicon.info.id}) ` + disclaimersRaw.join(' '), { supportThemeIcons: true });
+			const disclaimerTexts = disclaimersRaw.map(d => typeof d === 'string' ? d : d.value);
+			const hasMarkdownDisclaimer = disclaimersRaw.some(d => typeof d !== 'string');
+			const mdOptions = hasMarkdownDisclaimer
+				? { supportThemeIcons: true, isTrusted: { enabledCommands: [TerminalChatCommandId.OpenTerminalSettingsLink] } }
+				: { supportThemeIcons: true };
+			disclaimer = new MarkdownString(`$(${Codicon.info.id}) ` + disclaimerTexts.join(' '), mdOptions);
 		}
 
 		const analyzersIsAutoApproveAllowed = commandLineAnalyzerResults.every(e => e.isAutoApproveAllowed);
@@ -476,9 +482,13 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
 			wouldBeAutoApproved
 		);
 
-		// Pass autoApproveInfo if command would be auto-approved (even if warning not yet accepted)
-		// This allows the confirmation widget to auto-approve after user accepts the warning
-		if (isFinalAutoApproved || (isAutoApproveEnabled && wouldBeAutoApproved)) {
+		// Pass auto approve info if the command:
+		// - Was auto approved
+		// - Would have be auto approved, but the opt-in warning was not accepted
+		// - Was denied explicitly by a rule
+		//
+		// This allows surfacing this information to the user.
+		if (isFinalAutoApproved || (isAutoApproveEnabled && commandLineAnalyzerResults.some(e => e.autoApproveInfo))) {
 			toolSpecificData.autoApproveInfo = commandLineAnalyzerResults.find(e => e.autoApproveInfo)?.autoApproveInfo;
 		}
 

src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts

@@ -39,6 +39,7 @@ import { RunInTerminalTool, type IRunInTerminalInputParams } from '../../browser
 import { ShellIntegrationQuality } from '../../browser/toolTerminalCreator.js';
 import { terminalChatAgentToolsConfiguration, TerminalChatAgentToolsSettingId } from '../../common/terminalChatAgentToolsConfiguration.js';
 import { TerminalChatService } from '../../../chat/browser/terminalChatService.js';
+import type { IMarkdownString } from '../../../../../../base/common/htmlContent.js';
 
 class TestRunInTerminalTool extends RunInTerminalTool {
 	protected override _osBackend: Promise<OperatingSystem> = Promise.resolve(OperatingSystem.Windows);
@@ -1213,4 +1214,97 @@ suite('RunInTerminalTool', () => {
 			});
 		});
 	});
+
+	suite('denial info in disclaimers', () => {
+		function getDisclaimerValue(disclaimer: string | IMarkdownString | undefined): string | undefined {
+			if (!disclaimer) {
+				return undefined;
+			}
+			return typeof disclaimer === 'string' ? disclaimer : disclaimer.value;
+		}
+
+		test('should include denial reason in disclaimer when command is denied by rule', async () => {
+			setAutoApprove({
+				npm: { approve: false }
+			});
+			const result = await executeToolTest({
+				command: 'npm run build',
+				explanation: 'Build the project'
+			});
+
+			assertConfirmationRequired(result, 'Run `bash` command?');
+			const disclaimerValue = getDisclaimerValue(result?.confirmationMessages?.disclaimer);
+			ok(disclaimerValue, 'Expected disclaimer to be defined');
+			ok(disclaimerValue.includes('denied'), 'Expected disclaimer to mention denial');
+			ok(disclaimerValue.includes('npm'), 'Expected disclaimer to mention the denied rule');
+		});
+
+		test('should include link to settings in denial disclaimer', async () => {
+			setAutoApprove({
+				rm: { approve: false }
+			});
+			const result = await executeToolTest({
+				command: 'rm -rf temp',
+				explanation: 'Remove temp folder'
+			});
+
+			assertConfirmationRequired(result, 'Run `bash` command?');
+			ok(result?.confirmationMessages?.disclaimer, 'Expected disclaimer to be defined');
+			// The disclaimer should have trusted commands enabled for settings links
+			const disclaimer = result.confirmationMessages.disclaimer;
+			ok(typeof disclaimer !== 'string' && disclaimer.isTrusted, 'Expected disclaimer to be trusted for command links');
+		});
+
+		test('should include denial reason for multiple denied sub-commands', async () => {
+			setAutoApprove({
+				rm: { approve: false },
+				sudo: { approve: false }
+			});
+			const result = await executeToolTest({
+				command: 'sudo rm -rf /',
+				explanation: 'Dangerous command'
+			});
+
+			assertConfirmationRequired(result, 'Run `bash` command?');
+			const disclaimerValue = getDisclaimerValue(result?.confirmationMessages?.disclaimer);
+			ok(disclaimerValue, 'Expected disclaimer to be defined');
+			ok(disclaimerValue.includes('denied'), 'Expected disclaimer to mention denial');
+		});
+
+		test('should not include denial info when auto-approve is disabled', async () => {
+			setConfig(TerminalChatAgentToolsSettingId.EnableAutoApprove, false);
+			setAutoApprove({
+				npm: { approve: false }
+			});
+			const result = await executeToolTest({
+				command: 'npm run build',
+				explanation: 'Build the project'
+			});
+
+			assertConfirmationRequired(result, 'Run `bash` command?');
+			// When auto-approve is disabled, there should be no denial-related disclaimer
+			const disclaimerValue = getDisclaimerValue(result?.confirmationMessages?.disclaimer);
+			if (disclaimerValue) {
+				ok(!disclaimerValue.includes('denied'), 'Should not mention denial when auto-approve is disabled');
+			}
+		});
+
+		test('should not include denial info for commands that are simply not approved', async () => {
+			// Command is not in auto-approve list, but not explicitly denied
+			setAutoApprove({
+				echo: true
+			});
+			const result = await executeToolTest({
+				command: 'npm run build',
+				explanation: 'Build the project'
+			});
+
+			assertConfirmationRequired(result, 'Run `bash` command?');
+			// There should be no denial disclaimer since npm is not explicitly denied
+			const disclaimerValue = getDisclaimerValue(result?.confirmationMessages?.disclaimer);
+			if (disclaimerValue) {
+				ok(!disclaimerValue.includes('denied'), 'Should not mention denial for non-denied commands');
+			}
+		});
+	});
 });