Skip to content

Commit 4f9b8fb

Browse files
jrmtone
jr
authored and
mtone
committed
Use an allowlist to keep the prompt default shell sane (RooCodeInc#7681)
1 parent d2fd3d5 commit 4f9b8fb

File tree

2 files changed

+262
-31
lines changed

2 files changed

+262
-31
lines changed

‎src/utils/__tests__/shell.spec.ts‎

Lines changed: 108 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ vi.mock("os", () => ({
77
userInfo: vi.fn(() => ({ shell: null })),
88
}))
99

10+
// Mock path module for testing
11+
vi.mock("path", async () => {
12+
const actual = await vi.importActual("path")
13+
return {
14+
...actual,
15+
normalize: vi.fn((p: string) => p),
16+
}
17+
})
18+
1019
describe("Shell Detection Tests", () => {
1120
let originalPlatform: string
1221
let originalEnv: NodeJS.ProcessEnv
@@ -106,18 +115,25 @@ describe("Shell Detection Tests", () => {
106115
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
107116
})
108117

109-
it("respects userInfo() if no VS Code config is available", () => {
118+
it("respects userInfo() if no VS Code config is available and shell is allowed", () => {
119+
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
120+
vi.mocked(userInfo).mockReturnValue({ shell: "C:\\Program Files\\PowerShell\\7\\pwsh.exe" } as any)
121+
122+
expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
123+
})
124+
125+
it("falls back to safe shell when userInfo() returns non-allowlisted shell", () => {
110126
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
111127
vi.mocked(userInfo).mockReturnValue({ shell: "C:\\Custom\\PowerShell.exe" } as any)
112128

113-
expect(getShell()).toBe("C:\\Custom\\PowerShell.exe")
129+
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
114130
})
115131

116-
it("respects an odd COMSPEC if no userInfo shell is available", () => {
132+
it("falls back to safe shell when COMSPEC is non-allowlisted", () => {
117133
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
118134
process.env.COMSPEC = "D:\\CustomCmd\\cmd.exe"
119135

120-
expect(getShell()).toBe("D:\\CustomCmd\\cmd.exe")
136+
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
121137
})
122138
})
123139

@@ -191,10 +207,10 @@ describe("Shell Detection Tests", () => {
191207
// Unknown Platform & Error Handling
192208
// --------------------------------------------------------------------------
193209
describe("Unknown Platform / Error Handling", () => {
194-
it("falls back to /bin/sh for unknown platforms", () => {
210+
it("falls back to /bin/bash for unknown platforms", () => {
195211
Object.defineProperty(process, "platform", { value: "sunos" })
196212
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
197-
expect(getShell()).toBe("/bin/sh")
213+
expect(getShell()).toBe("/bin/bash")
198214
})
199215

200216
it("handles VS Code config errors gracefully, falling back to userInfo shell if present", () => {
@@ -228,4 +244,90 @@ describe("Shell Detection Tests", () => {
228244
expect(getShell()).toBe("/bin/bash")
229245
})
230246
})
247+
248+
// --------------------------------------------------------------------------
249+
// Shell Validation Tests
250+
// --------------------------------------------------------------------------
251+
describe("Shell Validation", () => {
252+
it("should allow common Windows shells", () => {
253+
Object.defineProperty(process, "platform", { value: "win32" })
254+
mockVsCodeConfig("windows", "PowerShell", {
255+
PowerShell: { path: "C:\\Program Files\\PowerShell\\7\\pwsh.exe" },
256+
})
257+
expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
258+
})
259+
260+
it("should allow common Unix shells", () => {
261+
Object.defineProperty(process, "platform", { value: "linux" })
262+
mockVsCodeConfig("linux", "CustomProfile", {
263+
CustomProfile: { path: "/usr/bin/fish" },
264+
})
265+
expect(getShell()).toBe("/usr/bin/fish")
266+
})
267+
268+
it("should handle case-insensitive matching on Windows", () => {
269+
Object.defineProperty(process, "platform", { value: "win32" })
270+
mockVsCodeConfig("windows", "PowerShell", {
271+
PowerShell: { path: "c:\\windows\\system32\\cmd.exe" },
272+
})
273+
expect(getShell()).toBe("c:\\windows\\system32\\cmd.exe")
274+
})
275+
276+
it("should reject unknown shells and use fallback", () => {
277+
Object.defineProperty(process, "platform", { value: "linux" })
278+
mockVsCodeConfig("linux", "CustomProfile", {
279+
CustomProfile: { path: "/usr/bin/malicious-shell" },
280+
})
281+
expect(getShell()).toBe("/bin/bash")
282+
})
283+
284+
it("should validate shells from VS Code config", () => {
285+
Object.defineProperty(process, "platform", { value: "darwin" })
286+
mockVsCodeConfig("osx", "MyCustomShell", {
287+
MyCustomShell: { path: "/usr/local/bin/custom-shell" },
288+
})
289+
290+
const result = getShell()
291+
expect(result).toBe("/bin/zsh") // macOS fallback
292+
})
293+
294+
it("should validate shells from userInfo", () => {
295+
Object.defineProperty(process, "platform", { value: "linux" })
296+
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
297+
vi.mocked(userInfo).mockReturnValue({ shell: "/usr/bin/evil-shell" } as any)
298+
299+
const result = getShell()
300+
expect(result).toBe("/bin/bash") // Linux fallback
301+
})
302+
303+
it("should validate shells from environment variables", () => {
304+
Object.defineProperty(process, "platform", { value: "linux" })
305+
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
306+
vi.mocked(userInfo).mockReturnValue({ shell: null } as any)
307+
process.env.SHELL = "/opt/custom/shell"
308+
309+
const result = getShell()
310+
expect(result).toBe("/bin/bash") // Linux fallback
311+
})
312+
313+
it("should handle WSL bash correctly", () => {
314+
Object.defineProperty(process, "platform", { value: "win32" })
315+
mockVsCodeConfig("windows", "WSL", {
316+
WSL: { source: "WSL" },
317+
})
318+
319+
const result = getShell()
320+
expect(result).toBe("/bin/bash") // Should be allowed
321+
})
322+
323+
it("should handle empty or null shell paths", () => {
324+
Object.defineProperty(process, "platform", { value: "linux" })
325+
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
326+
vi.mocked(userInfo).mockReturnValue({ shell: "" } as any)
327+
delete process.env.SHELL
328+
329+
const result = getShell()
330+
expect(result).toBe("/bin/bash") // Should fall back to safe default
331+
})
332+
})
231333
})

‎src/utils/shell.ts‎

Lines changed: 154 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,97 @@
11
import * as vscode from "vscode"
22
import { userInfo } from "os"
3+
import * as path from "path"
4+
5+
// Security: Allowlist of approved shell executables to prevent arbitrary command execution
6+
const SHELL_ALLOWLIST = new Set<string>([
7+
// Windows PowerShell variants
8+
"C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe",
9+
"C:\\Program Files\\PowerShell\\7\\pwsh.exe",
10+
"C:\\Program Files\\PowerShell\\6\\pwsh.exe",
11+
"C:\\Program Files\\PowerShell\\5\\pwsh.exe",
12+
13+
// Windows Command Prompt
14+
"C:\\Windows\\System32\\cmd.exe",
15+
16+
// Windows WSL
17+
"C:\\Windows\\System32\\wsl.exe",
18+
19+
// Git Bash on Windows
20+
"C:\\Program Files\\Git\\bin\\bash.exe",
21+
"C:\\Program Files\\Git\\usr\\bin\\bash.exe",
22+
"C:\\Program Files (x86)\\Git\\bin\\bash.exe",
23+
"C:\\Program Files (x86)\\Git\\usr\\bin\\bash.exe",
24+
25+
// MSYS2/MinGW/Cygwin on Windows
26+
"C:\\msys64\\usr\\bin\\bash.exe",
27+
"C:\\msys32\\usr\\bin\\bash.exe",
28+
"C:\\MinGW\\msys\\1.0\\bin\\bash.exe",
29+
"C:\\cygwin64\\bin\\bash.exe",
30+
"C:\\cygwin\\bin\\bash.exe",
31+
32+
// Unix/Linux/macOS - Bourne-compatible shells
33+
"/bin/sh",
34+
"/usr/bin/sh",
35+
"/bin/bash",
36+
"/usr/bin/bash",
37+
"/usr/local/bin/bash",
38+
"/opt/homebrew/bin/bash",
39+
"/opt/local/bin/bash",
40+
41+
// Z Shell
42+
"/bin/zsh",
43+
"/usr/bin/zsh",
44+
"/usr/local/bin/zsh",
45+
"/opt/homebrew/bin/zsh",
46+
"/opt/local/bin/zsh",
47+
48+
// Dash
49+
"/bin/dash",
50+
"/usr/bin/dash",
51+
52+
// Ash
53+
"/bin/ash",
54+
"/usr/bin/ash",
55+
56+
// C Shells
57+
"/bin/csh",
58+
"/usr/bin/csh",
59+
"/bin/tcsh",
60+
"/usr/bin/tcsh",
61+
"/usr/local/bin/tcsh",
62+
63+
// Korn Shells
64+
"/bin/ksh",
65+
"/usr/bin/ksh",
66+
"/bin/ksh93",
67+
"/usr/bin/ksh93",
68+
"/bin/mksh",
69+
"/usr/bin/mksh",
70+
"/bin/pdksh",
71+
"/usr/bin/pdksh",
72+
73+
// Fish Shell
74+
"/usr/bin/fish",
75+
"/usr/local/bin/fish",
76+
"/opt/homebrew/bin/fish",
77+
"/opt/local/bin/fish",
78+
79+
// Modern shells
80+
"/usr/bin/elvish",
81+
"/usr/local/bin/elvish",
82+
"/usr/bin/xonsh",
83+
"/usr/local/bin/xonsh",
84+
"/usr/bin/nu",
85+
"/usr/local/bin/nu",
86+
"/usr/bin/nushell",
87+
"/usr/local/bin/nushell",
88+
"/usr/bin/ion",
89+
"/usr/local/bin/ion",
90+
91+
// BusyBox
92+
"/bin/busybox",
93+
"/usr/bin/busybox",
94+
])
395

496
const SHELL_PATHS = {
597
// Windows paths
@@ -179,49 +271,86 @@ function getShellFromEnv(): string | null {
179271
}
180272

181273
// -----------------------------------------------------
182-
// 4) Publicly Exposed Shell Getter
274+
// 4) Shell Validation Functions
275+
// -----------------------------------------------------
276+
277+
/**
278+
* Validates if a shell path is in the allowlist to prevent arbitrary command execution
279+
*/
280+
function isShellAllowed(shellPath: string): boolean {
281+
if (!shellPath) return false
282+
283+
const normalizedPath = path.normalize(shellPath)
284+
285+
// Direct lookup first
286+
if (SHELL_ALLOWLIST.has(normalizedPath)) {
287+
return true
288+
}
289+
290+
// On Windows, try case-insensitive comparison
291+
if (process.platform === "win32") {
292+
const lowerPath = normalizedPath.toLowerCase()
293+
for (const allowedPath of SHELL_ALLOWLIST) {
294+
if (allowedPath.toLowerCase() === lowerPath) {
295+
return true
296+
}
297+
}
298+
}
299+
300+
return false
301+
}
302+
303+
/**
304+
* Returns a safe fallback shell based on the platform
305+
*/
306+
function getSafeFallbackShell(): string {
307+
if (process.platform === "win32") {
308+
return SHELL_PATHS.CMD
309+
} else if (process.platform === "darwin") {
310+
return SHELL_PATHS.MAC_DEFAULT
311+
} else {
312+
return SHELL_PATHS.LINUX_DEFAULT
313+
}
314+
}
315+
316+
// -----------------------------------------------------
317+
// 5) Publicly Exposed Shell Getter
183318
// -----------------------------------------------------
184319

185320
export function getShell(): string {
321+
let shell: string | null = null
322+
186323
// 1. Check VS Code config first.
187324
if (process.platform === "win32") {
188325
// Special logic for Windows
189-
const windowsShell = getWindowsShellFromVSCode()
190-
if (windowsShell) {
191-
return windowsShell
192-
}
326+
shell = getWindowsShellFromVSCode()
193327
} else if (process.platform === "darwin") {
194328
// macOS from VS Code
195-
const macShell = getMacShellFromVSCode()
196-
if (macShell) {
197-
return macShell
198-
}
329+
shell = getMacShellFromVSCode()
199330
} else if (process.platform === "linux") {
200331
// Linux from VS Code
201-
const linuxShell = getLinuxShellFromVSCode()
202-
if (linuxShell) {
203-
return linuxShell
204-
}
332+
shell = getLinuxShellFromVSCode()
205333
}
206334

207335
// 2. If no shell from VS Code, try userInfo()
208-
const userInfoShell = getShellFromUserInfo()
209-
if (userInfoShell) {
210-
return userInfoShell
336+
if (!shell) {
337+
shell = getShellFromUserInfo()
211338
}
212339

213340
// 3. If still nothing, try environment variable
214-
const envShell = getShellFromEnv()
215-
if (envShell) {
216-
return envShell
341+
if (!shell) {
342+
shell = getShellFromEnv()
217343
}
218344

219345
// 4. Finally, fall back to a default
220-
if (process.platform === "win32") {
221-
// On Windows, if we got here, we have no config, no COMSPEC, and one very messed up operating system.
222-
// Use CMD as a last resort
223-
return SHELL_PATHS.CMD
346+
if (!shell) {
347+
shell = getSafeFallbackShell()
348+
}
349+
350+
// 5. Validate the shell against allowlist
351+
if (!isShellAllowed(shell)) {
352+
shell = getSafeFallbackShell()
224353
}
225-
// On macOS/Linux, fallback to a POSIX shell - This is the behavior of our old shell detection method.
226-
return SHELL_PATHS.FALLBACK
354+
355+
return shell
227356
}

0 commit comments

Comments
 (0)