Skip to content

Commit 549a330

Browse files
TyriarTyriar
authored
Merge pull request microsoft#256788 from microsoft/tyriar/256280_commandlines
Support matching against full command lines
2 parents c663812 + 6747d36 commit 549a330

File tree

5 files changed

+245
-26
lines changed

5 files changed

+245
-26
lines changed

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

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import { isPowerShell } from './runInTerminalHelpers.js';
1212
export class CommandLineAutoApprover extends Disposable {
1313
private _denyListRegexes: RegExp[] = [];
1414
private _allowListRegexes: RegExp[] = [];
15+
private _allowListCommandLineRegexes: RegExp[] = [];
16+
private _denyListCommandLineRegexes: RegExp[] = [];
1517

1618
constructor(
1719
@IConfigurationService private readonly _configurationService: IConfigurationService,
@@ -26,12 +28,14 @@ export class CommandLineAutoApprover extends Disposable {
2628
}
2729

2830
updateConfiguration() {
29-
const { denyList, allowList } = this._mapAutoApproveConfigToRegexList(this._configurationService.getValue(TerminalChatAgentToolsSettingId.AutoApprove));
31+
const { denyList, allowList, allowListCommandLine, denyListCommandLine } = this._mapAutoApproveConfigToRegexList(this._configurationService.getValue(TerminalChatAgentToolsSettingId.AutoApprove));
3032
this._allowListRegexes = allowList;
3133
this._denyListRegexes = denyList;
34+
this._allowListCommandLineRegexes = allowListCommandLine;
35+
this._denyListCommandLineRegexes = denyListCommandLine;
3236
}
3337

34-
isAutoApproved(command: string, shell: string, os: OperatingSystem): boolean {
38+
isCommandAutoApproved(command: string, shell: string, os: OperatingSystem): boolean {
3539
// Check the deny list to see if this command requires explicit approval
3640
for (const regex of this._denyListRegexes) {
3741
if (this._commandMatchesRegex(regex, command, shell, os)) {
@@ -52,6 +56,23 @@ export class CommandLineAutoApprover extends Disposable {
5256
return false;
5357
}
5458

59+
isCommandLineAutoApproved(commandLine: string): boolean {
60+
// Check the deny list first to see if this command line requires explicit approval
61+
for (const regex of this._denyListCommandLineRegexes) {
62+
if (regex.test(commandLine)) {
63+
return false;
64+
}
65+
}
66+
67+
// Check if the full command line matches any of the allow list command line regexes
68+
for (const regex of this._allowListCommandLineRegexes) {
69+
if (regex.test(commandLine)) {
70+
return true;
71+
}
72+
}
73+
return false;
74+
}
75+
5576
private _commandMatchesRegex(regex: RegExp, command: string, shell: string, os: OperatingSystem): boolean {
5677
if (regex.test(command)) {
5778
return true;
@@ -65,13 +86,15 @@ export class CommandLineAutoApprover extends Disposable {
6586
return false;
6687
}
6788

68-
private _mapAutoApproveConfigToRegexList(config: unknown): { denyList: RegExp[]; allowList: RegExp[] } {
89+
private _mapAutoApproveConfigToRegexList(config: unknown): { denyList: RegExp[]; allowList: RegExp[]; allowListCommandLine: RegExp[]; denyListCommandLine: RegExp[] } {
6990
if (!config || typeof config !== 'object') {
70-
return { denyList: [], allowList: [] };
91+
return { denyList: [], allowList: [], allowListCommandLine: [], denyListCommandLine: [] };
7192
}
7293

7394
const denyList: RegExp[] = [];
7495
const allowList: RegExp[] = [];
96+
const allowListCommandLine: RegExp[] = [];
97+
const denyListCommandLine: RegExp[] = [];
7598

7699
Object.entries(config).forEach(([key, value]) => {
77100
if (typeof value === 'boolean') {
@@ -82,10 +105,29 @@ export class CommandLineAutoApprover extends Disposable {
82105
} else if (value === false) {
83106
denyList.push(regex);
84107
}
108+
} else if (typeof value === 'object' && value !== null) {
109+
// Handle object format like { approve: true/false, matchCommandLine: true/false }
110+
const objectValue = value as { approve?: boolean; matchCommandLine?: boolean };
111+
if (typeof objectValue.approve === 'boolean') {
112+
const regex = this._convertAutoApproveEntryToRegex(key);
113+
if (objectValue.approve === true) {
114+
if (objectValue.matchCommandLine === true) {
115+
allowListCommandLine.push(regex);
116+
} else {
117+
allowList.push(regex);
118+
}
119+
} else if (objectValue.approve === false) {
120+
if (objectValue.matchCommandLine === true) {
121+
denyListCommandLine.push(regex);
122+
} else {
123+
denyList.push(regex);
124+
}
125+
}
126+
}
85127
}
86128
});
87129

88-
return { denyList, allowList };
130+
return { denyList, allowList, allowListCommandLine, denyListCommandLine };
89131
}
90132

91133
private _convertAutoApproveEntryToRegex(value: string): RegExp {

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

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -171,17 +171,21 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
171171
const subCommands = splitCommandLineIntoSubCommands(args.command, shell, os);
172172
const inlineSubCommands = subCommands.map(e => Array.from(extractInlineSubCommands(e, shell, os))).flat();
173173
const allSubCommands = [...subCommands, ...inlineSubCommands];
174-
if (allSubCommands.every(e => this._commandLineAutoApprover.isAutoApproved(e, shell, os))) {
174+
if (allSubCommands.every(e => this._commandLineAutoApprover.isCommandAutoApproved(e, shell, os))) {
175175
confirmationMessages = undefined;
176176
} else {
177-
confirmationMessages = {
178-
title: args.isBackground
179-
? localize('runInTerminal.background', "Run command in background terminal")
180-
: localize('runInTerminal.foreground', "Run command in terminal"),
181-
message: new MarkdownString(
182-
args.explanation
183-
),
184-
};
177+
if (this._commandLineAutoApprover.isCommandLineAutoApproved(args.command)) {
178+
confirmationMessages = undefined;
179+
} else {
180+
confirmationMessages = {
181+
title: args.isBackground
182+
? localize('runInTerminal.background', "Run command in background terminal")
183+
: localize('runInTerminal.foreground', "Run command in terminal"),
184+
message: new MarkdownString(
185+
args.explanation
186+
),
187+
};
188+
}
185189
}
186190
}
187191

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts

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

66
import type { IStringDictionary } from '../../../../../base/common/collections.js';
7+
import type { IJSONSchema } from '../../../../../base/common/jsonSchema.js';
78
import { localize } from '../../../../../nls.js';
89
import type { IConfigurationPropertySchema } from '../../../../../platform/configuration/common/configurationRegistry.js';
910

@@ -15,23 +16,61 @@ export interface ITerminalChatAgentToolsConfiguration {
1516
autoApprove: { [key: string]: boolean };
1617
}
1718

19+
const autoApproveBoolean: IJSONSchema = {
20+
type: 'boolean',
21+
enum: [
22+
true,
23+
false,
24+
],
25+
enumDescriptions: [
26+
localize('autoApprove.true', "Automatically approve the pattern."),
27+
localize('autoApprove.false', "Require explicit approval for the pattern."),
28+
],
29+
description: localize('autoApprove.key', "The start of a command to match against. A regular expression can be provided by wrapping the string in `/` characters."),
30+
};
31+
1832
export const terminalChatAgentToolsConfiguration: IStringDictionary<IConfigurationPropertySchema> = {
1933
[TerminalChatAgentToolsSettingId.AutoApprove]: {
20-
markdownDescription: localize('autoApprove', "A list of commands or regular expressions that control whether the run in terminal tool commands require explicit approval. These will be matched against the start of a command. A regular expression can be provided by wrapping the string in `/` characters followed by optional flags such as `i`.\n\nSet to `true` to automatically approve commands, `false` to always require explicit approval or `null` to unset the value.\n\nExamples:\n- `\"mkdir\": true` Will allow all command lines starting with `mkdir`\n- `\"npm run build\": true` Will allow all command lines starting with `npm run build`\n- `\"rm\": false` Will require explicit approval for all command lines starting with `rm`\n- `\"/^git (status|show\\b.*)$/\": true` will allow `git status` and all command lines starting with `git show`\n- `\"/^Get-ChildItem\\b/i\": true` will allow Get-ChildItem command regardless of casing\n- `\"/.*/\": true` will allow all command lines\n- `\"rm\": null` will unset the default `false` value for `rm`\n\nNote that these commands and regular expressions are evaluated for every _sub-command_ within a single command line, so `foo && bar` for example will need both `foo` and `bar` to match a `true` entry and must not match a `false` entry in order to auto approve. Inline commands are also detected so `echo $(rm file)` will need both `echo $(rm file)` and `rm file` to pass."),
34+
markdownDescription: [
35+
localize('autoApprove.description.intro', "A list of commands or regular expressions that control whether the run in terminal tool commands require explicit approval. These will be matched against the start of a command. A regular expression can be provided by wrapping the string in {0} characters followed by optional flags such as {1} for case-insensitivity.", '`/`', '`i`'),
36+
localize('autoApprove.description.values', "Set to {0} to automatically approve commands, {1} to always require explicit approval or {2} to unset the value.", '`true`', '`false`', '`null`'),
37+
localize('autoApprove.description.subCommands', "Note that these commands and regular expressions are evaluated for every _sub-command_ within the full _command line_, so {0} for example will need both {1} and {2} to match a {3} entry and must not match a {4} entry in order to auto approve. Inline commands are also detected so {5} will need both {5} and {6} to pass.", '`foo && bar`', '`foo`', '`bar`', '`true`', '`false`', '`echo $(rm file)`', '`rm file`'),
38+
localize('autoApprove.description.commandLine', "An object can be used to match against the full command line instead of matching sub-commands and inline commands, for example {0}. This will be checked _after_ sub-commands are checked, so denied sub-commands will take precedence.", '`{ approve: false, matchCommandLine: true }`'),
39+
[
40+
localize('autoApprove.description.examples.title', 'Examples:'),
41+
`|${localize('autoApprove.description.examples.value', "Value")}|${localize('autoApprove.description.examples.description', "Description")}|`,
42+
'|---|---|',
43+
'| `\"mkdir\": true` | ' + localize('autoApprove.description.examples.mkdir', "Allow all commands starting with {0}", '`mkdir`'),
44+
'| `\"npm run build\": true` | ' + localize('autoApprove.description.examples.npmRunBuild', "Allow all commands starting with {0}", '`npm run build`'),
45+
'| `\"/^git (status\\|show\\b.*)$/\": true` | ' + localize('autoApprove.description.examples.regexGit', "Allow {0} and all commands starting with {1}", '`git status`', '`git show`'),
46+
'| `\"/^Get-ChildItem\\b/i\": true` | ' + localize('autoApprove.description.examples.regexCase', "will allow {0} commands regardless of casing", '`Get-ChildItem`'),
47+
'| `\"/.*/\": true` | ' + localize('autoApprove.description.examples.regexAll', "Allow all commands (denied commands still require approval)"),
48+
'| `\"rm\": false` | ' + localize('autoApprove.description.examples.rm', "Require explicit approval for all commands starting with {0}", '`rm`'),
49+
'| `\"/\.ps1/i\": { approve: false, matchCommandLine: true }` | ' + localize('autoApprove.description.examples.ps1', "Require explicit approval for any _command line_ that contains {0} regardless of casing", '`".ps1"`'),
50+
'| `\"rm\": null` | ' + localize('autoApprove.description.examples.rmUnset', "Unset the default {0} value for {1}", '`false`', '`rm`'),
51+
].join('\n')
52+
].join('\n\n'),
2153
type: 'object',
2254
additionalProperties: {
2355
anyOf: [
56+
autoApproveBoolean,
2457
{
25-
type: 'boolean',
26-
enum: [
27-
true,
28-
false,
29-
],
30-
enumDescriptions: [
31-
localize('autoApprove.true', "Automatically approve the pattern."),
32-
localize('autoApprove.false', "Require explicit approval for the pattern."),
33-
],
34-
description: localize('autoApprove.key', "The start of a command to match against. A regular expression can be provided by wrapping the string in `/` characters."),
58+
type: 'object',
59+
properties: {
60+
approve: autoApproveBoolean,
61+
matchCommandLine: {
62+
type: 'boolean',
63+
enum: [
64+
true,
65+
false,
66+
],
67+
enumDescriptions: [
68+
localize('autoApprove.matchCommandLine.true', "Match against the full command line, eg. `foo && bar`."),
69+
localize('autoApprove.matchCommandLine.false', "Match against sub-commands and inline commands, eg. `foo && bar` will need both `foo` and `bar` to match."),
70+
],
71+
description: localize('autoApprove.matchCommandLine', "Whether to match against the full command line, as opposed to splitting by sub-commands and inline commands."),
72+
}
73+
}
3574
},
3675
{
3776
type: 'null',

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

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,11 @@ suite('CommandLineAutoApprover', () => {
4949
}
5050

5151
function isAutoApproved(commandLine: string): boolean {
52-
return commandLineAutoApprover.isAutoApproved(commandLine, shell, os);
52+
return commandLineAutoApprover.isCommandAutoApproved(commandLine, shell, os);
53+
}
54+
55+
function isCommandLineAutoApproved(commandLine: string): boolean {
56+
return commandLineAutoApprover.isCommandLineAutoApproved(commandLine);
5357
}
5458

5559
suite('autoApprove with allow patterns only', () => {
@@ -325,4 +329,115 @@ suite('CommandLineAutoApprover', () => {
325329
ok(!isAutoApproved('foo'));
326330
});
327331
});
332+
333+
suite('isCommandLineAutoApproved - matchCommandLine functionality', () => {
334+
function setAutoApproveWithCommandLine(value: { [key: string]: { approve: boolean; matchCommandLine: boolean } | boolean }) {
335+
setConfig(TerminalChatAgentToolsSettingId.AutoApprove, value);
336+
}
337+
338+
test('should auto-approve command line patterns with matchCommandLine: true', () => {
339+
setAutoApproveWithCommandLine({
340+
"echo": { approve: true, matchCommandLine: true }
341+
});
342+
343+
ok(isCommandLineAutoApproved('echo hello'));
344+
ok(isCommandLineAutoApproved('echo test && ls'));
345+
});
346+
347+
test('should not auto-approve regular patterns with isCommandLineAutoApproved', () => {
348+
setAutoApprove({
349+
"echo": true
350+
});
351+
352+
// Regular patterns should not be matched by isCommandLineAutoApproved
353+
ok(!isCommandLineAutoApproved('echo hello'));
354+
});
355+
356+
test('should handle regex patterns with matchCommandLine: true', () => {
357+
setAutoApproveWithCommandLine({
358+
"/echo.*world/": { approve: true, matchCommandLine: true }
359+
});
360+
361+
ok(isCommandLineAutoApproved('echo hello world'));
362+
ok(!isCommandLineAutoApproved('echo hello'));
363+
});
364+
365+
test('should handle case-insensitive regex with matchCommandLine: true', () => {
366+
setAutoApproveWithCommandLine({
367+
"/echo/i": { approve: true, matchCommandLine: true }
368+
});
369+
370+
ok(isCommandLineAutoApproved('echo hello'));
371+
ok(isCommandLineAutoApproved('ECHO hello'));
372+
ok(isCommandLineAutoApproved('Echo hello'));
373+
});
374+
375+
test('should handle complex command line patterns', () => {
376+
setAutoApproveWithCommandLine({
377+
"/^npm run build/": { approve: true, matchCommandLine: true },
378+
"/\.ps1/i": { approve: true, matchCommandLine: true }
379+
});
380+
381+
ok(isCommandLineAutoApproved('npm run build --production'));
382+
ok(isCommandLineAutoApproved('powershell -File script.ps1'));
383+
ok(isCommandLineAutoApproved('pwsh -File SCRIPT.PS1'));
384+
ok(!isCommandLineAutoApproved('npm install'));
385+
});
386+
387+
test('should return false for empty command line', () => {
388+
setAutoApproveWithCommandLine({
389+
"echo": { approve: true, matchCommandLine: true }
390+
});
391+
392+
ok(!isCommandLineAutoApproved(''));
393+
ok(!isCommandLineAutoApproved(' '));
394+
});
395+
396+
test('should handle mixed configuration with matchCommandLine entries', () => {
397+
setAutoApproveWithCommandLine({
398+
"echo": true, // Regular pattern
399+
"ls": { approve: true, matchCommandLine: true }, // Command line pattern
400+
"rm": { approve: true, matchCommandLine: false } // Explicit regular pattern
401+
});
402+
403+
// Only the matchCommandLine: true entry should work with isCommandLineAutoApproved
404+
ok(isCommandLineAutoApproved('ls -la'));
405+
ok(!isCommandLineAutoApproved('echo hello'));
406+
ok(!isCommandLineAutoApproved('rm file.txt'));
407+
});
408+
409+
test('should handle deny patterns with matchCommandLine: true', () => {
410+
setAutoApproveWithCommandLine({
411+
"echo": { approve: true, matchCommandLine: true },
412+
"/dangerous/": { approve: false, matchCommandLine: true }
413+
});
414+
415+
ok(isCommandLineAutoApproved('echo hello'));
416+
ok(!isCommandLineAutoApproved('echo dangerous command'));
417+
ok(!isCommandLineAutoApproved('dangerous operation'));
418+
});
419+
420+
test('should prioritize deny list over allow list for command line patterns', () => {
421+
setAutoApproveWithCommandLine({
422+
"/echo/": { approve: true, matchCommandLine: true },
423+
"/echo.*dangerous/": { approve: false, matchCommandLine: true }
424+
});
425+
426+
ok(isCommandLineAutoApproved('echo hello'));
427+
ok(!isCommandLineAutoApproved('echo dangerous command'));
428+
});
429+
430+
test('should handle complex deny patterns with matchCommandLine', () => {
431+
setAutoApproveWithCommandLine({
432+
"npm": { approve: true, matchCommandLine: true },
433+
"/npm.*--force/": { approve: false, matchCommandLine: true },
434+
"/\.ps1.*-ExecutionPolicy/i": { approve: false, matchCommandLine: true }
435+
});
436+
437+
ok(isCommandLineAutoApproved('npm install'));
438+
ok(isCommandLineAutoApproved('npm run build'));
439+
ok(!isCommandLineAutoApproved('npm install --force'));
440+
ok(!isCommandLineAutoApproved('powershell -File script.ps1 -ExecutionPolicy Bypass'));
441+
});
442+
});
328443
});

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ suite('RunInTerminalTool', () => {
6262
setConfig(TerminalChatAgentToolsSettingId.AutoApprove, value);
6363
}
6464

65+
function setAutoApproveWithCommandLine(value: { [key: string]: { approve: boolean; matchCommandLine?: boolean } | boolean }) {
66+
setConfig(TerminalChatAgentToolsSettingId.AutoApprove, value);
67+
}
68+
6569
function setConfig(key: string, value: unknown) {
6670
configurationService.setUserConfiguration(key, value);
6771
configurationService.onDidChangeConfigurationEmitter.fire({
@@ -229,6 +233,21 @@ suite('RunInTerminalTool', () => {
229233
});
230234
assertConfirmationRequired(result);
231235
});
236+
237+
test('should handle matchCommandLine: true patterns', async () => {
238+
setAutoApproveWithCommandLine({
239+
"/dangerous/": { approve: false, matchCommandLine: true },
240+
"echo": { approve: true, matchCommandLine: true }
241+
});
242+
243+
// Command line pattern should be approved
244+
const result1 = await executeToolTest({ command: 'echo hello world' });
245+
assertAutoApproved(result1);
246+
247+
// Command line pattern should be denied due to dangerous content
248+
const result2 = await executeToolTest({ command: 'echo this is a dangerous command' });
249+
assertConfirmationRequired(result2);
250+
});
232251
});
233252

234253
suite('command re-writing', () => {

0 commit comments

Comments
 (0)