Skip to content

Commit 34e7cb0

Browse files
CopilotTyriar
andcommitted
Implement environment variable syntax handling in command line auto-approver
Co-authored-by: Tyriar <[email protected]>
1 parent 342b543 commit 34e7cb0

File tree

2 files changed

+138
-3
lines changed

2 files changed

+138
-3
lines changed

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

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

103+
private _extractCommandFromEnvAssignments(command: string, shell: string, os: OperatingSystem): string {
104+
// Trim leading/trailing whitespace
105+
const trimmedCommand = command.trim();
106+
107+
if (isPowerShell(shell, os)) {
108+
// PowerShell environment variable syntax: $env:VAR='value'; command
109+
// We'll keep the current behavior as PowerShell has different patterns
110+
return trimmedCommand;
111+
}
112+
113+
// For bash/sh/bourne shell and unknown shells (fallback to bourne shell syntax)
114+
// Handle environment variable assignments like: VAR=value VAR2=value command
115+
const envVarPattern = /^([A-Za-z_][A-Za-z0-9_]*=(?:[^\s'"]|'[^']*'|"[^"]*")*\s+)+/;
116+
const match = trimmedCommand.match(envVarPattern);
117+
118+
if (match) {
119+
// Remove the environment variable assignments from the beginning
120+
const actualCommand = trimmedCommand.slice(match[0].length).trim();
121+
return actualCommand || trimmedCommand; // Fallback to original if nothing left
122+
}
123+
124+
return trimmedCommand;
125+
}
126+
103127
private _commandMatchesRegex(regex: RegExp, command: string, shell: string, os: OperatingSystem): boolean {
104-
if (regex.test(command)) {
128+
// First extract the actual command from any environment variable assignments
129+
const actualCommand = this._extractCommandFromEnvAssignments(command, shell, os);
130+
131+
if (regex.test(actualCommand)) {
105132
return true;
106-
} else if (isPowerShell(shell, os) && command.startsWith('(')) {
133+
} else if (isPowerShell(shell, os) && actualCommand.startsWith('(')) {
107134
// Allow ignoring of the leading ( for PowerShell commands as it's a command pattern to
108135
// operate on the output of a command. For example `(Get-Content README.md) ...`
109-
if (regex.test(command.slice(1))) {
136+
if (regex.test(actualCommand.slice(1))) {
110137
return true;
111138
}
112139
}

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,4 +610,112 @@ suite('CommandLineAutoApprover', () => {
610610
});
611611
});
612612
});
613+
614+
suite('environment variable handling', () => {
615+
test('should handle environment variable assignments before commands in bash/sh', () => {
616+
shell = 'bash';
617+
os = OperatingSystem.Linux;
618+
619+
setAutoApprove({
620+
"env": true,
621+
"echo": true
622+
});
623+
624+
// Basic environment variable assignment
625+
ok(isAutoApproved('FOO=bar env'));
626+
ok(isAutoApproved('FOO=bar echo test'));
627+
628+
// Multiple environment variables
629+
ok(isAutoApproved('FOO=bar BAZ=qux env'));
630+
ok(isAutoApproved('PATH=/usr/bin HOME=/home/user echo hello'));
631+
632+
// Environment variables with quoted values
633+
ok(isAutoApproved('MESSAGE="hello world" echo test'));
634+
ok(isAutoApproved("GREETING='hello there' echo test"));
635+
636+
// Complex command after environment variables
637+
ok(isAutoApproved('DEBUG=1 env | grep DEBUG'));
638+
});
639+
640+
test('should not match denied commands even with environment variables', () => {
641+
shell = 'bash';
642+
os = OperatingSystem.Linux;
643+
644+
setAutoApprove({
645+
"env": true,
646+
"rm": false
647+
});
648+
649+
// Should approve env command with environment variable
650+
ok(isAutoApproved('FOO=bar env'));
651+
652+
// Should deny rm command even with environment variable
653+
ok(!isAutoApproved('FOO=bar rm file.txt'));
654+
});
655+
656+
test('should handle environment variables with different shell types', () => {
657+
setAutoApprove({
658+
"echo": true
659+
});
660+
661+
// Bash/sh should handle VAR=value syntax
662+
shell = 'bash';
663+
os = OperatingSystem.Linux;
664+
ok(isAutoApproved('FOO=bar echo test'));
665+
666+
// PowerShell should keep current behavior (not parse VAR=value as env vars)
667+
shell = 'powershell';
668+
os = OperatingSystem.Windows;
669+
ok(!isAutoApproved('FOO=bar echo test')); // This should not match since FOO=bar is not recognized as env var syntax in PowerShell
670+
});
671+
672+
test('should fallback to original command if no environment variables detected', () => {
673+
shell = 'bash';
674+
os = OperatingSystem.Linux;
675+
676+
setAutoApprove({
677+
"echo": true
678+
});
679+
680+
// Commands without environment variables should work as before
681+
ok(isAutoApproved('echo hello'));
682+
ok(isAutoApproved('echo test'));
683+
684+
// Commands that look like they might have env vars but don't match the pattern
685+
ok(isAutoApproved('echo FOO=bar')); // This is an argument to echo, not an env var assignment
686+
});
687+
688+
test('should handle edge cases in environment variable parsing', () => {
689+
shell = 'bash';
690+
os = OperatingSystem.Linux;
691+
692+
setAutoApprove({
693+
"echo": true
694+
});
695+
696+
// Empty value
697+
ok(isAutoApproved('FOO= echo test'));
698+
699+
// Underscore in variable name
700+
ok(isAutoApproved('MY_VAR=test echo hello'));
701+
702+
// Number in variable name (but not at start)
703+
ok(isAutoApproved('VAR1=test echo hello'));
704+
705+
// Should not match if variable name starts with number (invalid)
706+
ok(!isAutoApproved('1VAR=test echo hello')); // This should not be parsed as env var
707+
});
708+
709+
test('should handle unknown shell types by defaulting to bourne shell syntax', () => {
710+
shell = 'unknown-shell';
711+
os = OperatingSystem.Linux;
712+
713+
setAutoApprove({
714+
"echo": true
715+
});
716+
717+
// Unknown shells should default to bourne shell behavior
718+
ok(isAutoApproved('FOO=bar echo test'));
719+
});
720+
});
613721
});

0 commit comments

Comments
 (0)