Skip to content

Commit 7c88b03

Browse files
Copilotchrmarti
andcommitted
Fix fish shell environment variable probing by properly quoting commands
Co-authored-by: chrmarti <[email protected]>
1 parent daa9f38 commit 7c88b03

File tree

2 files changed

+100
-1
lines changed

2 files changed

+100
-1
lines changed

src/spec-common/injectHeadless.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -849,12 +849,25 @@ async function runUserEnvProbe(userEnvProbe: UserEnvProbe, params: { allowSystem
849849

850850
// handle popular non-POSIX shells
851851
const name = path.posix.basename(systemShellUnix);
852-
const command = `echo -n ${mark}; ${cmd}; echo -n ${mark}`;
852+
let command = `echo -n ${mark}; ${cmd}; echo -n ${mark}`;
853853
let shellArgs: string[];
854854
if (/^pwsh(-preview)?$/.test(name)) {
855855
shellArgs = userEnvProbe === 'loginInteractiveShell' || userEnvProbe === 'loginShell' ?
856856
['-Login', '-Command'] : // -Login must be the first option.
857857
['-Command'];
858+
} else if (/^fish$/.test(name)) {
859+
// Fish shell needs the command to be quoted when using interactive modes
860+
// to prevent it from interpreting the -n flag as a shell option
861+
const isInteractive = userEnvProbe === 'loginInteractiveShell' || userEnvProbe === 'interactiveShell';
862+
if (isInteractive) {
863+
command = `'${command.replace(/'/g, `'\\''`)}'`;
864+
}
865+
shellArgs = [
866+
userEnvProbe === 'loginInteractiveShell' ? '-lic' :
867+
userEnvProbe === 'loginShell' ? '-lc' :
868+
userEnvProbe === 'interactiveShell' ? '-ic' :
869+
'-c'
870+
];
858871
} else {
859872
shellArgs = [
860873
userEnvProbe === 'loginInteractiveShell' ? '-lic' :

src/test/fishShell.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import * as assert from 'assert';
7+
import * as path from 'path';
8+
import * as crypto from 'crypto';
9+
10+
describe('Fish shell command construction', function () {
11+
this.timeout('20s');
12+
13+
it('should properly quote commands for fish shell in interactive mode', async () => {
14+
// Simulate the logic from runUserEnvProbe function
15+
const systemShellUnix = '/usr/bin/fish';
16+
const name = path.posix.basename(systemShellUnix);
17+
const mark = crypto.randomUUID();
18+
let command = `echo -n ${mark}; cat /proc/self/environ; echo -n ${mark}`;
19+
20+
// Test fish shell detection and command quoting
21+
if (/^fish$/.test(name)) {
22+
// Fish shell needs the command to be quoted when using interactive modes
23+
// to prevent it from interpreting the -n flag as a shell option
24+
const isInteractive = true; // simulating 'loginInteractiveShell'
25+
if (isInteractive) {
26+
command = `'${command.replace(/'/g, `'\\''`)}'`;
27+
}
28+
}
29+
30+
// Verify the command is properly quoted
31+
assert.ok(command.startsWith(`'echo -n ${mark};`));
32+
assert.ok(command.endsWith(`; echo -n ${mark}'`));
33+
assert.ok(command.includes('cat /proc/self/environ'));
34+
35+
// Test that the command handles single quotes properly
36+
const commandWithQuotes = `echo -n ${mark}; echo 'test'; echo -n ${mark}`;
37+
let quotedCommand = commandWithQuotes;
38+
if (/^fish$/.test(name)) {
39+
quotedCommand = `'${commandWithQuotes.replace(/'/g, `'\\''`)}'`;
40+
}
41+
42+
// Verify single quotes are properly escaped
43+
assert.ok(quotedCommand.includes(`'\\''test'\\''`));
44+
});
45+
46+
it('should not quote commands for fish shell in non-interactive mode', async () => {
47+
// Simulate the logic from runUserEnvProbe function
48+
const systemShellUnix = '/usr/bin/fish';
49+
const name = path.posix.basename(systemShellUnix);
50+
const mark = crypto.randomUUID();
51+
let command = `echo -n ${mark}; cat /proc/self/environ; echo -n ${mark}`;
52+
53+
// Test fish shell detection and command quoting
54+
if (/^fish$/.test(name)) {
55+
// Fish shell needs the command to be quoted when using interactive modes
56+
// to prevent it from interpreting the -n flag as a shell option
57+
const isInteractive = false; // simulating 'loginShell' (non-interactive)
58+
if (isInteractive) {
59+
command = `'${command.replace(/'/g, `'\\''`)}'`;
60+
}
61+
}
62+
63+
// Verify the command is NOT quoted for non-interactive mode
64+
assert.ok(!command.startsWith(`'echo -n ${mark};`));
65+
assert.ok(!command.endsWith(`; echo -n ${mark}'`));
66+
assert.strictEqual(command, `echo -n ${mark}; cat /proc/self/environ; echo -n ${mark}`);
67+
});
68+
69+
it('should not affect non-fish shells', async () => {
70+
// Test that other shells are not affected
71+
const systemShellUnix = '/bin/bash';
72+
const name = path.posix.basename(systemShellUnix);
73+
const mark = crypto.randomUUID();
74+
let command = `echo -n ${mark}; cat /proc/self/environ; echo -n ${mark}`;
75+
const originalCommand = command;
76+
77+
// Test non-fish shell
78+
if (/^fish$/.test(name)) {
79+
// This should not execute for bash
80+
command = `'${command.replace(/'/g, `'\\''`)}'`;
81+
}
82+
83+
// Verify the command is unchanged for non-fish shells
84+
assert.strictEqual(command, originalCommand);
85+
});
86+
});

0 commit comments

Comments
 (0)