diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAnalyzer.ts index 59a2a9dc3c85..5741948eb17a 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAnalyzer.ts +++ b/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[]; } diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts index 2bcf733727ac..b7588aa667a9 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineAutoApproveAnalyzer.ts +++ b/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; } diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index 972eacf11a48..7466e60ed200 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/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; } diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts index dbc9a0c29402..f318fad0d93a 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts +++ b/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 = 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'); + } + }); + }); });