Skip to content

Commit 3cfb00b

Browse files
authored
Merge pull request microsoft#259205 from microsoft/copilot/fix-259201
Handle environment variable syntax in terminal command auto-approval
2 parents 2f0369f + 3973dc7 commit 3cfb00b

File tree

2 files changed

+114
-4
lines changed

2 files changed

+114
-4
lines changed

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,38 @@ export class CommandLineAutoApprover extends Disposable {
100100
return { result: 'noMatch', reason: `Command line '${commandLine}' has no matching auto approve entries` };
101101
}
102102

103+
private _removeEnvAssignments(command: string, shell: string, os: OperatingSystem): string {
104+
const trimmedCommand = command.trimStart();
105+
106+
// PowerShell environment variable syntax is `$env:VAR='value';` and treated as a different
107+
// command
108+
if (isPowerShell(shell, os)) {
109+
return trimmedCommand;
110+
}
111+
112+
// For bash/sh/bourne shell and unknown shells (fallback to bourne shell syntax)
113+
// Handle environment variable assignments like: VAR=value VAR2=value command
114+
// This regex matches one or more environment variable assignments at the start
115+
const envVarPattern = /^(\s*[A-Za-z_][A-Za-z0-9_]*=(?:[^\s'"]|'[^']*'|"[^"]*")*\s+)+/;
116+
const match = trimmedCommand.match(envVarPattern);
117+
118+
if (match) {
119+
const actualCommand = trimmedCommand.slice(match[0].length).trimStart();
120+
return actualCommand || trimmedCommand; // Fallback to original if nothing left
121+
}
122+
123+
return trimmedCommand;
124+
}
125+
103126
private _commandMatchesRegex(regex: RegExp, command: string, shell: string, os: OperatingSystem): boolean {
104-
if (regex.test(command)) {
127+
const actualCommand = this._removeEnvAssignments(command, shell, os);
128+
129+
if (regex.test(actualCommand)) {
105130
return true;
106-
} else if (isPowerShell(shell, os) && command.startsWith('(')) {
131+
} else if (isPowerShell(shell, os) && actualCommand.startsWith('(')) {
107132
// Allow ignoring of the leading ( for PowerShell commands as it's a command pattern to
108133
// operate on the output of a command. For example `(Get-Content README.md) ...`
109-
if (regex.test(command.slice(1))) {
134+
if (regex.test(actualCommand.slice(1))) {
110135
return true;
111136
}
112137
}

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

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,6 @@ suite('CommandLineAutoApprover', () => {
286286
});
287287

288288
ok(isAutoApproved('echo hello world'));
289-
ok(!isAutoApproved(' echo hello'));
290289
});
291290

292291
test('should be case-sensitive by default', () => {
@@ -618,4 +617,90 @@ suite('CommandLineAutoApprover', () => {
618617
});
619618
});
620619
});
620+
621+
suite('environment variable handling', () => {
622+
test('should handle environment variable assignments before commands in bash/sh', () => {
623+
shell = 'bash';
624+
os = OperatingSystem.Linux;
625+
626+
setAutoApprove({
627+
"env": true,
628+
"echo": true
629+
});
630+
631+
ok(isAutoApproved('FOO=bar env'), 'Basic environment variable assignment');
632+
ok(isAutoApproved('FOO=bar echo test'), 'Basic environment variable assignment');
633+
634+
ok(isAutoApproved('FOO=bar BAZ=qux env'), 'Multiple environment variables');
635+
ok(isAutoApproved('PATH=/usr/bin HOME=/home/user echo hello'), 'Multiple environment variables');
636+
637+
ok(isAutoApproved('MESSAGE="hello world" echo test'), 'Environment variables with quoted values');
638+
ok(isAutoApproved("GREETING='hello there' echo test"), 'Environment variables with quoted values');
639+
});
640+
641+
test('should not match denied commands even with environment variables', () => {
642+
shell = 'bash';
643+
os = OperatingSystem.Linux;
644+
645+
setAutoApprove({
646+
"env": true,
647+
"rm": false
648+
});
649+
650+
ok(isAutoApproved('FOO=bar env'), 'Should approve env command with environment variable');
651+
ok(!isAutoApproved('FOO=bar rm file.txt'), 'Should deny rm command even with environment variable');
652+
});
653+
654+
test('should handle environment variables with different shell types', () => {
655+
setAutoApprove({
656+
"echo": true
657+
});
658+
659+
shell = 'bash';
660+
os = OperatingSystem.Linux;
661+
ok(isAutoApproved('FOO=bar echo test'));
662+
663+
shell = 'powershell';
664+
os = OperatingSystem.Windows;
665+
ok(!isAutoApproved('FOO=bar echo test'), 'This should not match since FOO=bar is not recognized as env var syntax in PowerShell');
666+
});
667+
668+
test('should fallback to original command if no environment variables detected', () => {
669+
shell = 'bash';
670+
os = OperatingSystem.Linux;
671+
672+
setAutoApprove({
673+
"echo": true
674+
});
675+
676+
ok(isAutoApproved('echo hello'));
677+
ok(isAutoApproved('echo test'));
678+
ok(isAutoApproved('echo FOO=bar'), '// Commands that look like they might have env vars but don\'t match the pattern');
679+
});
680+
681+
test('should handle edge cases in environment variable parsing', () => {
682+
shell = 'bash';
683+
os = OperatingSystem.Linux;
684+
685+
setAutoApprove({
686+
"echo": true
687+
});
688+
689+
ok(isAutoApproved('FOO= echo test'), 'Empty value');
690+
ok(isAutoApproved('MY_VAR=test echo hello'), 'Underscore in variable name');
691+
ok(isAutoApproved('VAR1=test echo hello'), 'Number in variable name (but not at start)');
692+
ok(!isAutoApproved('1VAR=test echo hello'), 'Should not match if variable name starts with number (invalid)');
693+
});
694+
695+
test('should handle unknown shell types by defaulting to bourne shell syntax', () => {
696+
shell = 'unknown-shell';
697+
os = OperatingSystem.Linux;
698+
699+
setAutoApprove({
700+
"echo": true
701+
});
702+
703+
ok(isAutoApproved('FOO=bar echo test'), 'Unknown shells should default to bourne shell behavior');
704+
});
705+
});
621706
});

0 commit comments

Comments
 (0)