Skip to content

Commit 629ef49

Browse files
authored
Merge pull request microsoft#258998 from microsoft/tyriar/258719_2
Handle empty string auto approve case
2 parents 6ab7bd7 + 97f556f commit 629ef49

File tree

2 files changed

+21
-9
lines changed

2 files changed

+21
-9
lines changed

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ interface IAutoApproveRule {
1717

1818
export type ICommandApprovalResult = 'approved' | 'denied' | 'noMatch';
1919

20+
const neverMatchRegex = /(?!.*)/;
21+
2022
export class CommandLineAutoApprover extends Disposable {
2123
private _denyListRules: IAutoApproveRule[] = [];
2224
private _allowListRules: IAutoApproveRule[] = [];
@@ -177,27 +179,29 @@ export class CommandLineAutoApprover extends Disposable {
177179
const regexPattern = regexMatch?.groups?.pattern;
178180
if (regexPattern) {
179181
let flags = regexMatch.groups?.flags;
180-
// Remove global flag as it can cause confusion
182+
// Remove global flag as it changes how the regex state works which we need to handle
183+
// internally
181184
if (flags) {
182185
flags = flags.replaceAll('g', '');
183186
}
184187

185188
try {
186189
const regex = new RegExp(regexPattern, flags || undefined);
187-
188-
// Check if the regex would lead to an endless loop
189190
if (regExpLeadsToEndlessLoop(regex)) {
190-
// Return a regex that will never match anything to prevent endless loops
191-
return /(?!.*)/;
191+
return neverMatchRegex;
192192
}
193193

194194
return regex;
195195
} catch (error) {
196-
// If the regex is invalid, return a regex that will never match anything
197-
return /(?!.*)/;
196+
return neverMatchRegex;
198197
}
199198
}
200199

200+
// The empty string should be ignored, rather than approve everything
201+
if (value === '') {
202+
return neverMatchRegex;
203+
}
204+
201205
// Escape regex special characters
202206
const sanitizedValue = value.replace(/[\\^$.*+?()[\]{}|]/g, '\\$&');
203207

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,11 +301,19 @@ suite('CommandLineAutoApprover', () => {
301301
ok(isAutoApproved('pwsh.exe -File D:\\foo.bar\\a-script.ps1 -AnotherArg'));
302302
});
303303

304+
test('should ignore the empty string key', () => {
305+
setAutoApprove({
306+
"": true
307+
});
308+
309+
ok(!isAutoApproved('echo hello'));
310+
});
311+
304312
test('should handle empty regex patterns that could cause endless loops', () => {
305313
setAutoApprove({
306314
"//": true,
307315
"/(?:)/": true,
308-
"/*/": true, // Invalid regex pattern
316+
"/*/": true, // Invalid regex pattern
309317
"/.**/": true // Invalid regex pattern
310318
});
311319

@@ -549,7 +557,7 @@ suite('CommandLineAutoApprover', () => {
549557
test('should handle invalid regex patterns with matchCommandLine gracefully', () => {
550558
setAutoApproveWithCommandLine({
551559
"/*/": { approve: true, matchCommandLine: true }, // Invalid regex - nothing to repeat
552-
"/(?:+/": { approve: true, matchCommandLine: true }, // Invalid regex - incomplete quantifier
560+
"/(?:+/": { approve: true, matchCommandLine: true }, // Invalid regex - incomplete quantifier
553561
"/[/": { approve: true, matchCommandLine: true }, // Invalid regex - unclosed character class
554562
"/^echo/": { approve: true, matchCommandLine: true }, // Valid pattern
555563
"ls": { approve: true, matchCommandLine: true } // Valid string pattern

0 commit comments

Comments
 (0)