From 565642345566adc711a6e77961dbc783fe7c9ab8 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 5 Jan 2026 04:04:49 -0800 Subject: [PATCH 1/5] Show message about denied commands with links Fixes #285921 --- .../commandLineAnalyzer.ts | 2 +- .../commandLineAutoApproveAnalyzer.ts | 20 ++++++++++++++++--- .../browser/tools/runInTerminalTool.ts | 15 ++++++++++---- 3 files changed, 29 insertions(+), 8 deletions(-) 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..726aad76e95f 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.filter(e => e.disclaimers).flatMap(e => e.disclaimers!); 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,10 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { wouldBeAutoApproved ); - // Pass autoApproveInfo if command would be auto-approved (even if warning not yet accepted) + // Pass autoApproveInfo if command would be auto-approved (even if warning not yet accepted), + // or if it was denied by a rule so the UI can explain why // This allows the confirmation widget to auto-approve after user accepts the warning - if (isFinalAutoApproved || (isAutoApproveEnabled && wouldBeAutoApproved)) { + if (isFinalAutoApproved || (isAutoApproveEnabled && commandLineAnalyzerResults.some(e => e.autoApproveInfo))) { toolSpecificData.autoApproveInfo = commandLineAnalyzerResults.find(e => e.autoApproveInfo)?.autoApproveInfo; } From ffc384499ae88fa1cfda8715cb5dad80ddd1b2bb Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 5 Jan 2026 04:07:49 -0800 Subject: [PATCH 2/5] Add denial tests --- .../runInTerminalTool.test.ts | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) 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..e7f7df330937 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 @@ -1213,4 +1213,97 @@ suite('RunInTerminalTool', () => { }); }); }); + + suite('denial info in disclaimers', () => { + function getDisclaimerValue(disclaimer: string | import('../../../../../../base/common/htmlContent.js').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'); + } + }); + }); }); From 76f9d67136d67158f3b966028305d3d3053536f5 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 5 Jan 2026 08:35:07 -0800 Subject: [PATCH 3/5] Remove ! assertion --- .../chatAgentTools/browser/tools/runInTerminalTool.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 726aad76e95f..0d26e18f0f05 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -446,7 +446,7 @@ 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) { const disclaimerTexts = disclaimersRaw.map(d => typeof d === 'string' ? d : d.value); From 7c36a58d4146fac7d45089e7c532c5e55c43931b Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 5 Jan 2026 08:38:23 -0800 Subject: [PATCH 4/5] Polish comment --- .../chatAgentTools/browser/tools/runInTerminalTool.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 0d26e18f0f05..7466e60ed200 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -482,9 +482,12 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { wouldBeAutoApproved ); - // Pass autoApproveInfo if command would be auto-approved (even if warning not yet accepted), - // or if it was denied by a rule so the UI can explain why - // This allows the confirmation widget to auto-approve after user accepts the warning + // 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; } From 298ef5632dbac4cc7d39a00eb44eb19be0e13c92 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 5 Jan 2026 08:44:40 -0800 Subject: [PATCH 5/5] Fix import in test --- .../test/electron-browser/runInTerminalTool.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 e7f7df330937..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); @@ -1215,7 +1216,7 @@ suite('RunInTerminalTool', () => { }); suite('denial info in disclaimers', () => { - function getDisclaimerValue(disclaimer: string | import('../../../../../../base/common/htmlContent.js').IMarkdownString | undefined): string | undefined { + function getDisclaimerValue(disclaimer: string | IMarkdownString | undefined): string | undefined { if (!disclaimer) { return undefined; }