mirror of
https://github.com/microsoft/vscode.git
synced 2026-09-28 08:04:00 +01:00
Merge pull request #285925 from microsoft/tyriar/285921
Show message about denied commands with links
This commit is contained in:
+1
-1
@@ -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[];
|
||||
}
|
||||
|
||||
+17
-3
@@ -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;
|
||||
}
|
||||
|
||||
+15
-5
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
+94
@@ -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');
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user