Skip to content

Commit b8b82df

Browse files
authored
Merge pull request #259673 from microsoft/copilot/fix-259672
Make terminal auto approve more aware of paths
2 parents 2fa8a2f + 373f89e commit b8b82df

File tree

3 files changed

+136
-3
lines changed

3 files changed

+136
-3
lines changed

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import { Disposable } from '../../../../../base/common/lifecycle.js';
77
import type { OperatingSystem } from '../../../../../base/common/platform.js';
8-
import { regExpLeadsToEndlessLoop } from '../../../../../base/common/strings.js';
8+
import { escapeRegExpCharacters, regExpLeadsToEndlessLoop } from '../../../../../base/common/strings.js';
99
import { isObject } from '../../../../../base/common/types.js';
1010
import { structuralEquals } from '../../../../../base/common/equals.js';
1111
import { IConfigurationService, type IConfigurationValue } from '../../../../../platform/configuration/common/configuration.js';
@@ -281,8 +281,22 @@ export class CommandLineAutoApprover extends Disposable {
281281
return neverMatchRegex;
282282
}
283283

284-
// Escape regex special characters
285-
const sanitizedValue = value.replace(/[\\^$.*+?()[\]{}|]/g, '\\$&');
284+
let sanitizedValue: string;
285+
286+
// Match both path separators it if looks like a path
287+
if (value.includes('/') || value.includes('\\')) {
288+
// Replace path separators with placeholders first, apply standard sanitization, then
289+
// apply special path handling
290+
let pattern = value.replace(/[/\\]/g, '%%PATH_SEP%%');
291+
pattern = escapeRegExpCharacters(pattern);
292+
pattern = pattern.replace(/%%PATH_SEP%%*/g, '[/\\\\]');
293+
sanitizedValue = `^(?:\\.[/\\\\])?${pattern}`;
294+
}
295+
296+
// Escape regex special characters for non-path strings
297+
else {
298+
sanitizedValue = escapeRegExpCharacters(value);
299+
}
286300

287301
// Regular strings should match the start of the command line and be a word boundary
288302
return new RegExp(`^${sanitizedValue}\\b`);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export const terminalChatAgentToolsConfiguration: IStringDictionary<IConfigurati
4848
'|---|---|',
4949
'| `\"mkdir\": true` | ' + localize('autoApprove.description.examples.mkdir', "Allow all commands starting with {0}", '`mkdir`'),
5050
'| `\"npm run build\": true` | ' + localize('autoApprove.description.examples.npmRunBuild', "Allow all commands starting with {0}", '`npm run build`'),
51+
'| `\"bin/test.sh\": true` | ' + localize('autoApprove.description.examples.binTest', "Allow all commands that match the path {0} ({1}, {2}, etc.)", '`bin/test.sh`', '`bin\\test.sh`', '`./bin/test.sh`'),
5152
'| `\"/^git (status\\|show\\b.*)$/\": true` | ' + localize('autoApprove.description.examples.regexGit', "Allow {0} and all commands starting with {1}", '`git status`', '`git show`'),
5253
'| `\"/^Get-ChildItem\\b/i\": true` | ' + localize('autoApprove.description.examples.regexCase', "will allow {0} commands regardless of casing", '`Get-ChildItem`'),
5354
'| `\"/.*/\": true` | ' + localize('autoApprove.description.examples.regexAll', "Allow all commands (denied commands still require approval)"),

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

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,124 @@ suite('CommandLineAutoApprover', () => {
378378
});
379379
});
380380

381+
suite('path-aware auto approval', () => {
382+
test('should handle path variations with forward slashes', () => {
383+
setAutoApprove({
384+
"bin/foo": true
385+
});
386+
387+
// Should approve the exact match
388+
ok(isAutoApproved('bin/foo'));
389+
ok(isAutoApproved('bin/foo --arg'));
390+
391+
// Should approve with Windows backslashes
392+
ok(isAutoApproved('bin\\foo'));
393+
ok(isAutoApproved('bin\\foo --arg'));
394+
395+
// Should approve with current directory prefixes
396+
ok(isAutoApproved('./bin/foo'));
397+
ok(isAutoApproved('.\\bin/foo'));
398+
ok(isAutoApproved('./bin\\foo'));
399+
ok(isAutoApproved('.\\bin\\foo'));
400+
401+
// Should not approve partial matches
402+
ok(!isAutoApproved('bin/foobar'));
403+
ok(!isAutoApproved('notbin/foo'));
404+
});
405+
406+
test('should handle path variations with backslashes', () => {
407+
setAutoApprove({
408+
"bin\\script.bat": true
409+
});
410+
411+
// Should approve the exact match
412+
ok(isAutoApproved('bin\\script.bat'));
413+
ok(isAutoApproved('bin\\script.bat --help'));
414+
415+
// Should approve with forward slashes
416+
ok(isAutoApproved('bin/script.bat'));
417+
ok(isAutoApproved('bin/script.bat --help'));
418+
419+
// Should approve with current directory prefixes
420+
ok(isAutoApproved('./bin\\script.bat'));
421+
ok(isAutoApproved('.\\bin\\script.bat'));
422+
ok(isAutoApproved('./bin/script.bat'));
423+
ok(isAutoApproved('.\\bin/script.bat'));
424+
});
425+
426+
test('should handle deep paths', () => {
427+
setAutoApprove({
428+
"src/utils/helper.js": true
429+
});
430+
431+
ok(isAutoApproved('src/utils/helper.js'));
432+
ok(isAutoApproved('src\\utils\\helper.js'));
433+
ok(isAutoApproved('src/utils\\helper.js'));
434+
ok(isAutoApproved('src\\utils/helper.js'));
435+
ok(isAutoApproved('./src/utils/helper.js'));
436+
ok(isAutoApproved('.\\src\\utils\\helper.js'));
437+
});
438+
439+
test('should not treat non-paths as paths', () => {
440+
setAutoApprove({
441+
"echo": true, // Not a path
442+
"ls": true, // Not a path
443+
"git": true // Not a path
444+
});
445+
446+
// These should work as normal command matching, not path matching
447+
ok(isAutoApproved('echo'));
448+
ok(isAutoApproved('ls'));
449+
ok(isAutoApproved('git'));
450+
451+
// Should not be treated as paths, so these prefixes shouldn't work
452+
ok(!isAutoApproved('./echo'));
453+
ok(!isAutoApproved('.\\ls'));
454+
});
455+
456+
test('should handle paths with mixed separators in config', () => {
457+
setAutoApprove({
458+
"bin/foo\\bar": true // Mixed separators in config
459+
});
460+
461+
ok(isAutoApproved('bin/foo\\bar'));
462+
ok(isAutoApproved('bin\\foo/bar'));
463+
ok(isAutoApproved('bin/foo/bar'));
464+
ok(isAutoApproved('bin\\foo\\bar'));
465+
ok(isAutoApproved('./bin/foo\\bar'));
466+
ok(isAutoApproved('.\\bin\\foo\\bar'));
467+
});
468+
469+
test('should work with command line auto approval for paths', () => {
470+
setAutoApproveWithCommandLine({
471+
"bin/deploy": { approve: true, matchCommandLine: true }
472+
});
473+
474+
ok(isCommandLineAutoApproved('bin/deploy --prod'));
475+
ok(isCommandLineAutoApproved('bin\\deploy --prod'));
476+
ok(isCommandLineAutoApproved('./bin/deploy --prod'));
477+
ok(isCommandLineAutoApproved('.\\bin\\deploy --prod'));
478+
});
479+
480+
test('should handle special characters in paths', () => {
481+
setAutoApprove({
482+
"bin/my-script.sh": true,
483+
"scripts/build_all.py": true,
484+
"tools/run (debug).exe": true
485+
});
486+
487+
ok(isAutoApproved('bin/my-script.sh'));
488+
ok(isAutoApproved('bin\\my-script.sh'));
489+
ok(isAutoApproved('./bin/my-script.sh'));
490+
491+
ok(isAutoApproved('scripts/build_all.py'));
492+
ok(isAutoApproved('scripts\\build_all.py'));
493+
494+
ok(isAutoApproved('tools/run (debug).exe'));
495+
ok(isAutoApproved('tools\\run (debug).exe'));
496+
});
497+
});
498+
381499
suite('PowerShell-specific commands', () => {
382500
setup(() => {
383501
shell = 'pwsh';

0 commit comments

Comments
 (0)