Skip to content

Commit 918b0e4

Browse files
TyriarTyriar
committed
Prevent commands with sub commands being offered to auto approve
Fixes microsoft#264918
1 parent 5be90b9 commit 918b0e4

2 files changed

Lines changed: 82 additions & 28 deletions

File tree

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalHelpers.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Separator } from '../../../../../base/common/actions.js';
7+
import { coalesce } from '../../../../../base/common/arrays.js';
78
import { posix as pathPosix, win32 as pathWin32 } from '../../../../../base/common/path.js';
89
import { OperatingSystem } from '../../../../../base/common/platform.js';
910
import { removeAnsiEscapeCodes } from '../../../../../base/common/strings.js';
@@ -60,19 +61,25 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str
6061
// suggest just the commnad or sub-command (with that sub-command line) to always allow.
6162
const commandsWithSubcommands = new Set(['git', 'npm', 'yarn', 'docker', 'kubectl', 'cargo', 'dotnet', 'mvn', 'gradle']);
6263
const commandsWithSubSubCommands = new Set(['npm run', 'yarn run']);
63-
const subCommandsToSuggest = Array.from(new Set(unapprovedSubCommands.map(command => {
64+
const subCommandsToSuggest = Array.from(new Set(coalesce(unapprovedSubCommands.map(command => {
6465
const parts = command.trim().split(/\s+/);
6566
const baseCommand = parts[0].toLowerCase();
6667
const baseSubCommand = parts.length > 1 ? `${parts[0]} ${parts[1]}`.toLowerCase() : '';
6768

68-
if (commandsWithSubSubCommands.has(baseSubCommand) && parts.length >= 3 && !parts[2].startsWith('-')) {
69-
return `${parts[0]} ${parts[1]} ${parts[2]}`;
70-
} else if (commandsWithSubcommands.has(baseCommand) && parts.length >= 2 && !parts[1].startsWith('-')) {
71-
return `${parts[0]} ${parts[1]}`;
69+
if (commandsWithSubSubCommands.has(baseSubCommand)) {
70+
if (parts.length >= 3 && !parts[2].startsWith('-')) {
71+
return `${parts[0]} ${parts[1]} ${parts[2]}`;
72+
}
73+
return undefined;
74+
} else if (commandsWithSubcommands.has(baseCommand)) {
75+
if (parts.length >= 2 && !parts[1].startsWith('-')) {
76+
return `${parts[0]} ${parts[1]}`;
77+
}
78+
return undefined;
7279
} else {
7380
return parts[0];
7481
}
75-
})));
82+
}))));
7683

7784
if (subCommandsToSuggest.length > 0) {
7885
let subCommandLabel: string;
@@ -96,9 +103,13 @@ export function generateAutoApproveActions(commandLine: string, subCommands: str
96103
}
97104

98105
// Allow exact command line, don't do this if it's just the first sub-command's first
99-
// word
106+
// word or if it's an exact match for special sub-commands
100107
const firstSubcommandFirstWord = unapprovedSubCommands.length > 0 ? unapprovedSubCommands[0].split(' ')[0] : '';
101-
if (firstSubcommandFirstWord !== commandLine) {
108+
if (
109+
firstSubcommandFirstWord !== commandLine &&
110+
!commandsWithSubcommands.has(commandLine) &&
111+
!commandsWithSubSubCommands.has(commandLine)
112+
) {
102113
actions.push({
103114
label: localize('autoApprove.exactCommand', 'Always Allow Exact Command Line'),
104115
data: {

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

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -485,8 +485,8 @@ suite('RunInTerminalTool', () => {
485485

486486
test('should generate custom actions for single word commands', async () => {
487487
const result = await executeToolTest({
488-
command: 'git',
489-
explanation: 'Run git command'
488+
command: 'foo',
489+
explanation: 'Run foo command'
490490
});
491491

492492
assertConfirmationRequired(result);
@@ -497,7 +497,7 @@ suite('RunInTerminalTool', () => {
497497
strictEqual(customActions.length, 3);
498498

499499
ok(!isSeparator(customActions[0]));
500-
strictEqual(customActions[0].label, 'Always Allow Command: git');
500+
strictEqual(customActions[0].label, 'Always Allow Command: foo');
501501
strictEqual(customActions[0].data.type, 'newRule');
502502
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
503503

@@ -732,13 +732,13 @@ suite('RunInTerminalTool', () => {
732732
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
733733

734734
const customActions = result!.confirmationMessages!.terminalCustomActions!;
735-
strictEqual(customActions.length, 4);
735+
// Commands with flags don't get subcommand suggestions, only exact command line
736+
strictEqual(customActions.length, 3);
736737

737738
ok(!isSeparator(customActions[0]));
738-
strictEqual(customActions[0].label, 'Always Allow Command: npm');
739+
strictEqual(customActions[0].label, 'Always Allow Exact Command Line');
739740
strictEqual(customActions[0].data.type, 'newRule');
740-
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
741-
strictEqual((customActions[0].data.rule as any)[0].key, 'npm');
741+
ok(!Array.isArray(customActions[0].data.rule), 'Expected rule to be an object');
742742
});
743743

744744
test('should not suggest subcommand for git commands with flags', async () => {
@@ -751,13 +751,13 @@ suite('RunInTerminalTool', () => {
751751
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
752752

753753
const customActions = result!.confirmationMessages!.terminalCustomActions!;
754-
strictEqual(customActions.length, 4);
754+
// Commands with flags don't get subcommand suggestions, only exact command line
755+
strictEqual(customActions.length, 3);
755756

756757
ok(!isSeparator(customActions[0]));
757-
strictEqual(customActions[0].label, 'Always Allow Command: git');
758+
strictEqual(customActions[0].label, 'Always Allow Exact Command Line');
758759
strictEqual(customActions[0].data.type, 'newRule');
759-
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
760-
strictEqual((customActions[0].data.rule as any)[0].key, 'git');
760+
ok(!Array.isArray(customActions[0].data.rule), 'Expected rule to be an object');
761761
});
762762

763763
test('should not suggest subcommand for npm run with flags', async () => {
@@ -770,13 +770,13 @@ suite('RunInTerminalTool', () => {
770770
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
771771

772772
const customActions = result!.confirmationMessages!.terminalCustomActions!;
773-
strictEqual(customActions.length, 4);
773+
// Commands with flags don't get subcommand suggestions, only exact command line
774+
strictEqual(customActions.length, 3);
774775

775776
ok(!isSeparator(customActions[0]));
776-
strictEqual(customActions[0].label, 'Always Allow Command: npm run');
777+
strictEqual(customActions[0].label, 'Always Allow Exact Command Line');
777778
strictEqual(customActions[0].data.type, 'newRule');
778-
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
779-
strictEqual((customActions[0].data.rule as any)[0].key, 'npm run');
779+
ok(!Array.isArray(customActions[0].data.rule), 'Expected rule to be an object');
780780
});
781781

782782
test('should handle mixed npm run and other commands', async () => {
@@ -874,13 +874,11 @@ suite('RunInTerminalTool', () => {
874874
ok(result!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
875875

876876
const customActions = result!.confirmationMessages!.terminalCustomActions!;
877-
strictEqual(customActions.length, 3); // No full command line suggestion for single word
877+
strictEqual(customActions.length, 1);
878878

879879
ok(!isSeparator(customActions[0]));
880-
strictEqual(customActions[0].label, 'Always Allow Command: git');
881-
strictEqual(customActions[0].data.type, 'newRule');
882-
ok(Array.isArray(customActions[0].data.rule), 'Expected rule to be an array');
883-
strictEqual((customActions[0].data.rule as any)[0].key, 'git');
880+
strictEqual(customActions[0].label, 'Configure Auto Approve...');
881+
strictEqual(customActions[0].data.type, 'configure');
884882
});
885883

886884
test('should deduplicate identical subcommand suggestions', async () => {
@@ -904,6 +902,51 @@ suite('RunInTerminalTool', () => {
904902
strictEqual(rules[0].key, 'npm test');
905903
});
906904

905+
test('should handle flags differently than subcommands for suggestion logic', async () => {
906+
// Test that commands with actual subcommands get subcommand suggestions
907+
const resultWithSubcommand = await executeToolTest({
908+
command: 'npm install express',
909+
explanation: 'Install express package'
910+
});
911+
912+
assertConfirmationRequired(resultWithSubcommand);
913+
ok(resultWithSubcommand!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
914+
915+
const actionsWithSubcommand = resultWithSubcommand!.confirmationMessages!.terminalCustomActions!;
916+
strictEqual(actionsWithSubcommand.length, 4);
917+
918+
ok(!isSeparator(actionsWithSubcommand[0]));
919+
strictEqual(actionsWithSubcommand[0].label, 'Always Allow Command: npm install');
920+
strictEqual(actionsWithSubcommand[0].data.type, 'newRule');
921+
922+
ok(!isSeparator(actionsWithSubcommand[1]));
923+
strictEqual(actionsWithSubcommand[1].label, 'Always Allow Exact Command Line');
924+
strictEqual(actionsWithSubcommand[1].data.type, 'newRule');
925+
926+
// Test that commands with flags don't get subcommand suggestions
927+
const resultWithFlags = await executeToolTest({
928+
command: 'npm --version',
929+
explanation: 'Check npm version'
930+
});
931+
932+
assertConfirmationRequired(resultWithFlags);
933+
ok(resultWithFlags!.confirmationMessages!.terminalCustomActions, 'Expected custom actions to be defined');
934+
935+
const actionsWithFlags = resultWithFlags!.confirmationMessages!.terminalCustomActions!;
936+
strictEqual(actionsWithFlags.length, 3); // No subcommand suggestion, only exact command line
937+
938+
ok(!isSeparator(actionsWithFlags[0]));
939+
strictEqual(actionsWithFlags[0].label, 'Always Allow Exact Command Line');
940+
strictEqual(actionsWithFlags[0].data.type, 'newRule');
941+
ok(!Array.isArray(actionsWithFlags[0].data.rule), 'Expected rule to be an object for exact command line');
942+
943+
ok(isSeparator(actionsWithFlags[1]));
944+
945+
ok(!isSeparator(actionsWithFlags[2]));
946+
strictEqual(actionsWithFlags[2].label, 'Configure Auto Approve...');
947+
strictEqual(actionsWithFlags[2].data.type, 'configure');
948+
});
949+
907950
});
908951

909952
suite('chat session disposal cleanup', () => {

0 commit comments

Comments
 (0)