From 532f1d4ab2a7791cd24bf7e63b7bf2856706016e Mon Sep 17 00:00:00 2001 From: John Richmond <5629+jr@users.noreply.github.com> Date: Thu, 4 Sep 2025 15:50:28 -0700 Subject: [PATCH] Use an allowlist to keep the prompt default shell sane --- src/utils/__tests__/shell.spec.ts | 114 ++++++++++++++++++- src/utils/shell.ts | 179 +++++++++++++++++++++++++----- 2 files changed, 262 insertions(+), 31 deletions(-) diff --git a/src/utils/__tests__/shell.spec.ts b/src/utils/__tests__/shell.spec.ts index 733c9dd78a..352aedbbe5 100644 --- a/src/utils/__tests__/shell.spec.ts +++ b/src/utils/__tests__/shell.spec.ts @@ -7,6 +7,15 @@ vi.mock("os", () => ({ userInfo: vi.fn(() => ({ shell: null })), })) +// Mock path module for testing +vi.mock("path", async () => { + const actual = await vi.importActual("path") + return { + ...actual, + normalize: vi.fn((p: string) => p), + } +}) + describe("Shell Detection Tests", () => { let originalPlatform: string let originalEnv: NodeJS.ProcessEnv @@ -106,18 +115,25 @@ describe("Shell Detection Tests", () => { expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe") }) - it("respects userInfo() if no VS Code config is available", () => { + it("respects userInfo() if no VS Code config is available and shell is allowed", () => { + vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any + vi.mocked(userInfo).mockReturnValue({ shell: "C:\\Program Files\\PowerShell\\7\\pwsh.exe" } as any) + + expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe") + }) + + it("falls back to safe shell when userInfo() returns non-allowlisted shell", () => { vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any vi.mocked(userInfo).mockReturnValue({ shell: "C:\\Custom\\PowerShell.exe" } as any) - expect(getShell()).toBe("C:\\Custom\\PowerShell.exe") + expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe") }) - it("respects an odd COMSPEC if no userInfo shell is available", () => { + it("falls back to safe shell when COMSPEC is non-allowlisted", () => { vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any process.env.COMSPEC = "D:\\CustomCmd\\cmd.exe" - expect(getShell()).toBe("D:\\CustomCmd\\cmd.exe") + expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe") }) }) @@ -191,10 +207,10 @@ describe("Shell Detection Tests", () => { // Unknown Platform & Error Handling // -------------------------------------------------------------------------- describe("Unknown Platform / Error Handling", () => { - it("falls back to /bin/sh for unknown platforms", () => { + it("falls back to /bin/bash for unknown platforms", () => { Object.defineProperty(process, "platform", { value: "sunos" }) vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any - expect(getShell()).toBe("/bin/sh") + expect(getShell()).toBe("/bin/bash") }) it("handles VS Code config errors gracefully, falling back to userInfo shell if present", () => { @@ -228,4 +244,90 @@ describe("Shell Detection Tests", () => { expect(getShell()).toBe("/bin/bash") }) }) + + // -------------------------------------------------------------------------- + // Shell Validation Tests + // -------------------------------------------------------------------------- + describe("Shell Validation", () => { + it("should allow common Windows shells", () => { + Object.defineProperty(process, "platform", { value: "win32" }) + mockVsCodeConfig("windows", "PowerShell", { + PowerShell: { path: "C:\\Program Files\\PowerShell\\7\\pwsh.exe" }, + }) + expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe") + }) + + it("should allow common Unix shells", () => { + Object.defineProperty(process, "platform", { value: "linux" }) + mockVsCodeConfig("linux", "CustomProfile", { + CustomProfile: { path: "/usr/bin/fish" }, + }) + expect(getShell()).toBe("/usr/bin/fish") + }) + + it("should handle case-insensitive matching on Windows", () => { + Object.defineProperty(process, "platform", { value: "win32" }) + mockVsCodeConfig("windows", "PowerShell", { + PowerShell: { path: "c:\\windows\\system32\\cmd.exe" }, + }) + expect(getShell()).toBe("c:\\windows\\system32\\cmd.exe") + }) + + it("should reject unknown shells and use fallback", () => { + Object.defineProperty(process, "platform", { value: "linux" }) + mockVsCodeConfig("linux", "CustomProfile", { + CustomProfile: { path: "/usr/bin/malicious-shell" }, + }) + expect(getShell()).toBe("/bin/bash") + }) + + it("should validate shells from VS Code config", () => { + Object.defineProperty(process, "platform", { value: "darwin" }) + mockVsCodeConfig("osx", "MyCustomShell", { + MyCustomShell: { path: "/usr/local/bin/custom-shell" }, + }) + + const result = getShell() + expect(result).toBe("/bin/zsh") // macOS fallback + }) + + it("should validate shells from userInfo", () => { + Object.defineProperty(process, "platform", { value: "linux" }) + vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any + vi.mocked(userInfo).mockReturnValue({ shell: "/usr/bin/evil-shell" } as any) + + const result = getShell() + expect(result).toBe("/bin/bash") // Linux fallback + }) + + it("should validate shells from environment variables", () => { + Object.defineProperty(process, "platform", { value: "linux" }) + vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any + vi.mocked(userInfo).mockReturnValue({ shell: null } as any) + process.env.SHELL = "/opt/custom/shell" + + const result = getShell() + expect(result).toBe("/bin/bash") // Linux fallback + }) + + it("should handle WSL bash correctly", () => { + Object.defineProperty(process, "platform", { value: "win32" }) + mockVsCodeConfig("windows", "WSL", { + WSL: { source: "WSL" }, + }) + + const result = getShell() + expect(result).toBe("/bin/bash") // Should be allowed + }) + + it("should handle empty or null shell paths", () => { + Object.defineProperty(process, "platform", { value: "linux" }) + vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any + vi.mocked(userInfo).mockReturnValue({ shell: "" } as any) + delete process.env.SHELL + + const result = getShell() + expect(result).toBe("/bin/bash") // Should fall back to safe default + }) + }) }) diff --git a/src/utils/shell.ts b/src/utils/shell.ts index 22a916f02c..997c699fdb 100644 --- a/src/utils/shell.ts +++ b/src/utils/shell.ts @@ -1,5 +1,97 @@ import * as vscode from "vscode" import { userInfo } from "os" +import * as path from "path" + +// Security: Allowlist of approved shell executables to prevent arbitrary command execution +const SHELL_ALLOWLIST = new Set([ + // Windows PowerShell variants + "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe", + "C:\\Program Files\\PowerShell\\7\\pwsh.exe", + "C:\\Program Files\\PowerShell\\6\\pwsh.exe", + "C:\\Program Files\\PowerShell\\5\\pwsh.exe", + + // Windows Command Prompt + "C:\\Windows\\System32\\cmd.exe", + + // Windows WSL + "C:\\Windows\\System32\\wsl.exe", + + // Git Bash on Windows + "C:\\Program Files\\Git\\bin\\bash.exe", + "C:\\Program Files\\Git\\usr\\bin\\bash.exe", + "C:\\Program Files (x86)\\Git\\bin\\bash.exe", + "C:\\Program Files (x86)\\Git\\usr\\bin\\bash.exe", + + // MSYS2/MinGW/Cygwin on Windows + "C:\\msys64\\usr\\bin\\bash.exe", + "C:\\msys32\\usr\\bin\\bash.exe", + "C:\\MinGW\\msys\\1.0\\bin\\bash.exe", + "C:\\cygwin64\\bin\\bash.exe", + "C:\\cygwin\\bin\\bash.exe", + + // Unix/Linux/macOS - Bourne-compatible shells + "/bin/sh", + "/usr/bin/sh", + "/bin/bash", + "/usr/bin/bash", + "/usr/local/bin/bash", + "/opt/homebrew/bin/bash", + "/opt/local/bin/bash", + + // Z Shell + "/bin/zsh", + "/usr/bin/zsh", + "/usr/local/bin/zsh", + "/opt/homebrew/bin/zsh", + "/opt/local/bin/zsh", + + // Dash + "/bin/dash", + "/usr/bin/dash", + + // Ash + "/bin/ash", + "/usr/bin/ash", + + // C Shells + "/bin/csh", + "/usr/bin/csh", + "/bin/tcsh", + "/usr/bin/tcsh", + "/usr/local/bin/tcsh", + + // Korn Shells + "/bin/ksh", + "/usr/bin/ksh", + "/bin/ksh93", + "/usr/bin/ksh93", + "/bin/mksh", + "/usr/bin/mksh", + "/bin/pdksh", + "/usr/bin/pdksh", + + // Fish Shell + "/usr/bin/fish", + "/usr/local/bin/fish", + "/opt/homebrew/bin/fish", + "/opt/local/bin/fish", + + // Modern shells + "/usr/bin/elvish", + "/usr/local/bin/elvish", + "/usr/bin/xonsh", + "/usr/local/bin/xonsh", + "/usr/bin/nu", + "/usr/local/bin/nu", + "/usr/bin/nushell", + "/usr/local/bin/nushell", + "/usr/bin/ion", + "/usr/local/bin/ion", + + // BusyBox + "/bin/busybox", + "/usr/bin/busybox", +]) const SHELL_PATHS = { // Windows paths @@ -179,49 +271,86 @@ function getShellFromEnv(): string | null { } // ----------------------------------------------------- -// 4) Publicly Exposed Shell Getter +// 4) Shell Validation Functions +// ----------------------------------------------------- + +/** + * Validates if a shell path is in the allowlist to prevent arbitrary command execution + */ +function isShellAllowed(shellPath: string): boolean { + if (!shellPath) return false + + const normalizedPath = path.normalize(shellPath) + + // Direct lookup first + if (SHELL_ALLOWLIST.has(normalizedPath)) { + return true + } + + // On Windows, try case-insensitive comparison + if (process.platform === "win32") { + const lowerPath = normalizedPath.toLowerCase() + for (const allowedPath of SHELL_ALLOWLIST) { + if (allowedPath.toLowerCase() === lowerPath) { + return true + } + } + } + + return false +} + +/** + * Returns a safe fallback shell based on the platform + */ +function getSafeFallbackShell(): string { + if (process.platform === "win32") { + return SHELL_PATHS.CMD + } else if (process.platform === "darwin") { + return SHELL_PATHS.MAC_DEFAULT + } else { + return SHELL_PATHS.LINUX_DEFAULT + } +} + +// ----------------------------------------------------- +// 5) Publicly Exposed Shell Getter // ----------------------------------------------------- export function getShell(): string { + let shell: string | null = null + // 1. Check VS Code config first. if (process.platform === "win32") { // Special logic for Windows - const windowsShell = getWindowsShellFromVSCode() - if (windowsShell) { - return windowsShell - } + shell = getWindowsShellFromVSCode() } else if (process.platform === "darwin") { // macOS from VS Code - const macShell = getMacShellFromVSCode() - if (macShell) { - return macShell - } + shell = getMacShellFromVSCode() } else if (process.platform === "linux") { // Linux from VS Code - const linuxShell = getLinuxShellFromVSCode() - if (linuxShell) { - return linuxShell - } + shell = getLinuxShellFromVSCode() } // 2. If no shell from VS Code, try userInfo() - const userInfoShell = getShellFromUserInfo() - if (userInfoShell) { - return userInfoShell + if (!shell) { + shell = getShellFromUserInfo() } // 3. If still nothing, try environment variable - const envShell = getShellFromEnv() - if (envShell) { - return envShell + if (!shell) { + shell = getShellFromEnv() } // 4. Finally, fall back to a default - if (process.platform === "win32") { - // On Windows, if we got here, we have no config, no COMSPEC, and one very messed up operating system. - // Use CMD as a last resort - return SHELL_PATHS.CMD + if (!shell) { + shell = getSafeFallbackShell() + } + + // 5. Validate the shell against allowlist + if (!isShellAllowed(shell)) { + shell = getSafeFallbackShell() } - // On macOS/Linux, fallback to a POSIX shell - This is the behavior of our old shell detection method. - return SHELL_PATHS.FALLBACK + + return shell }