Skip to content

Commit 738b8c8

Browse files
authored
Merge pull request microsoft#258805 from microsoft/tyriar/258719
Handle invalid and empty regexps in auto approve parsing
2 parents ad7c495 + 8d20120 commit 738b8c8

File tree

2 files changed

+138
-1
lines changed

2 files changed

+138
-1
lines changed

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +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';
89
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
910
import { TerminalChatAgentToolsSettingId } from '../common/terminalChatAgentToolsConfiguration.js';
1011
import { isPowerShell } from './runInTerminalHelpers.js';
@@ -180,7 +181,21 @@ export class CommandLineAutoApprover extends Disposable {
180181
if (flags) {
181182
flags = flags.replaceAll('g', '');
182183
}
183-
return new RegExp(regexPattern, flags || undefined);
184+
185+
try {
186+
const regex = new RegExp(regexPattern, flags || undefined);
187+
188+
// Check if the regex would lead to an endless loop
189+
if (regExpLeadsToEndlessLoop(regex)) {
190+
// Return a regex that will never match anything to prevent endless loops
191+
return /(?!.*)/;
192+
}
193+
194+
return regex;
195+
} catch (error) {
196+
// If the regex is invalid, return a regex that will never match anything
197+
return /(?!.*)/;
198+
}
184199
}
185200

186201
// Escape regex special characters

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

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,67 @@ suite('CommandLineAutoApprover', () => {
300300
ok(isAutoApproved('pwsh.exe -File D:\\foo.bar\\a-script.ps1'));
301301
ok(isAutoApproved('pwsh.exe -File D:\\foo.bar\\a-script.ps1 -AnotherArg'));
302302
});
303+
304+
test('should handle empty regex patterns that could cause endless loops', () => {
305+
setAutoApprove({
306+
"//": true,
307+
"/(?:)/": true,
308+
"/*/": true, // Invalid regex pattern
309+
"/.**/": true // Invalid regex pattern
310+
});
311+
312+
// These patterns should not cause endless loops and should not match any commands
313+
// Invalid patterns should be handled gracefully and not match anything
314+
ok(!isAutoApproved('echo hello'));
315+
ok(!isAutoApproved('ls'));
316+
ok(!isAutoApproved(''));
317+
});
318+
319+
test('should handle regex patterns that would cause endless loops', () => {
320+
setAutoApprove({
321+
"/a*/": true,
322+
"/b?/": true,
323+
"/(x|)*/": true,
324+
"/(?:)*/": true
325+
});
326+
327+
// Commands should still work normally, endless loop patterns should be safely handled
328+
ok(!isAutoApproved('echo hello'));
329+
ok(!isAutoApproved('ls'));
330+
ok(!isAutoApproved('a'));
331+
ok(!isAutoApproved('b'));
332+
});
333+
334+
test('should handle mixed valid and problematic regex patterns', () => {
335+
setAutoApprove({
336+
"/^echo/": true, // Valid pattern
337+
"//": true, // Empty pattern
338+
"/^ls/": true, // Valid pattern
339+
"/a*/": true, // Potential endless loop
340+
"pwd": true // Valid string pattern
341+
});
342+
343+
ok(isAutoApproved('echo hello'));
344+
ok(isAutoApproved('ls -la'));
345+
ok(isAutoApproved('pwd'));
346+
ok(!isAutoApproved('rm file'));
347+
});
348+
349+
test('should handle invalid regex patterns gracefully', () => {
350+
setAutoApprove({
351+
"/*/": true, // Invalid regex - nothing to repeat
352+
"/(?:+/": true, // Invalid regex - incomplete quantifier
353+
"/[/": true, // Invalid regex - unclosed character class
354+
"/^echo/": true, // Valid pattern
355+
"ls": true // Valid string pattern
356+
});
357+
358+
// Valid patterns should still work
359+
ok(isAutoApproved('echo hello'));
360+
ok(isAutoApproved('ls -la'));
361+
// Invalid patterns should not match anything and not cause crashes
362+
ok(!isAutoApproved('random command'));
363+
});
303364
});
304365

305366
suite('PowerShell-specific commands', () => {
@@ -439,6 +500,67 @@ suite('CommandLineAutoApprover', () => {
439500
ok(!isCommandLineAutoApproved('npm install --force'));
440501
ok(!isCommandLineAutoApproved('powershell -File script.ps1 -ExecutionPolicy Bypass'));
441502
});
503+
504+
test('should handle empty regex patterns with matchCommandLine that could cause endless loops', () => {
505+
setAutoApproveWithCommandLine({
506+
"//": { approve: true, matchCommandLine: true },
507+
"/(?:)/": { approve: true, matchCommandLine: true },
508+
"/*/": { approve: true, matchCommandLine: true }, // Invalid regex pattern
509+
"/.**/": { approve: true, matchCommandLine: true } // Invalid regex pattern
510+
});
511+
512+
// These patterns should not cause endless loops and should not match any commands
513+
// Invalid patterns should be handled gracefully and not match anything
514+
ok(!isCommandLineAutoApproved('echo hello'));
515+
ok(!isCommandLineAutoApproved('ls'));
516+
ok(!isCommandLineAutoApproved(''));
517+
});
518+
519+
test('should handle regex patterns with matchCommandLine that would cause endless loops', () => {
520+
setAutoApproveWithCommandLine({
521+
"/a*/": { approve: true, matchCommandLine: true },
522+
"/b?/": { approve: true, matchCommandLine: true },
523+
"/(x|)*/": { approve: true, matchCommandLine: true },
524+
"/(?:)*/": { approve: true, matchCommandLine: true }
525+
});
526+
527+
// Commands should still work normally, endless loop patterns should be safely handled
528+
ok(!isCommandLineAutoApproved('echo hello'));
529+
ok(!isCommandLineAutoApproved('ls'));
530+
ok(!isCommandLineAutoApproved('a'));
531+
ok(!isCommandLineAutoApproved('b'));
532+
});
533+
534+
test('should handle mixed valid and problematic regex patterns with matchCommandLine', () => {
535+
setAutoApproveWithCommandLine({
536+
"/^echo/": { approve: true, matchCommandLine: true }, // Valid pattern
537+
"//": { approve: true, matchCommandLine: true }, // Empty pattern
538+
"/^ls/": { approve: true, matchCommandLine: true }, // Valid pattern
539+
"/a*/": { approve: true, matchCommandLine: true }, // Potential endless loop
540+
"pwd": { approve: true, matchCommandLine: true } // Valid string pattern
541+
});
542+
543+
ok(isCommandLineAutoApproved('echo hello'));
544+
ok(isCommandLineAutoApproved('ls -la'));
545+
ok(isCommandLineAutoApproved('pwd'));
546+
ok(!isCommandLineAutoApproved('rm file'));
547+
});
548+
549+
test('should handle invalid regex patterns with matchCommandLine gracefully', () => {
550+
setAutoApproveWithCommandLine({
551+
"/*/": { approve: true, matchCommandLine: true }, // Invalid regex - nothing to repeat
552+
"/(?:+/": { approve: true, matchCommandLine: true }, // Invalid regex - incomplete quantifier
553+
"/[/": { approve: true, matchCommandLine: true }, // Invalid regex - unclosed character class
554+
"/^echo/": { approve: true, matchCommandLine: true }, // Valid pattern
555+
"ls": { approve: true, matchCommandLine: true } // Valid string pattern
556+
});
557+
558+
// Valid patterns should still work
559+
ok(isCommandLineAutoApproved('echo hello'));
560+
ok(isCommandLineAutoApproved('ls -la'));
561+
// Invalid patterns should not match anything and not cause crashes
562+
ok(!isCommandLineAutoApproved('random command'));
563+
});
442564
});
443565

444566
suite('reasons', () => {

0 commit comments

Comments
 (0)