fix: shell default profile name type guard (#687)

daewoongoh · web-flow · commit 515437b453de · 2026-06-29T01:23:25.000Z
* fix(shell): validate defaultProfileName is a string before calling .toLowerCase()

config.get&lt;string&gt;() has no runtime type check, so non-string values
(number, boolean, array, object) from settings.json pass through and
cause a TypeError. Add typeof guard in each platform's terminal config
helper and add corresponding test cases.

* refactor(shell): unify platform terminal config helpers into generic getTerminalConfig&lt;T&gt;()

* test(shell): add test cases for getTerminalConfig behavior

* refactor(shell): apply code review feedback to getTerminalConfig

- Replace TerminalProfiles union with PlatformProfilesMap mapped type so
  the return type is inferred from the platform key, eliminating explicit
  type parameters at each call site and making mismatches a compile error
- Change || to ?? when falling back to empty profiles object
- Add JSDoc explaining the key constraint and inferred return type
- Add missing boolean/array non-string defaultProfileName test cases for
  macOS and Linux to match Windows coverage
- Add mutation-resistant tests for macOS and Linux that verify the typeof
  guard is load-bearing (numeric key matching a real profile entry)
diff --git a/src/utils/__tests__/shell.spec.ts b/src/utils/__tests__/shell.spec.ts
@@ -370,6 +370,293 @@ describe("Shell Detection Tests", () => {
 		})
 	})
 
+	// --------------------------------------------------------------------------
+	// getTerminalConfig Behavior (tested via getShell)
+	// --------------------------------------------------------------------------
+	describe("getTerminalConfig", () => {
+		it("returns defaultProfileName and matching profile for Windows", () => {
+			Object.defineProperty(process, "platform", { value: "win32" })
+			mockVsCodeConfig("windows", "Command Prompt", {
+				"Command Prompt": { path: "C:\\Windows\\System32\\cmd.exe" },
+			})
+			expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
+		})
+
+		it("returns defaultProfileName and matching profile for macOS", () => {
+			Object.defineProperty(process, "platform", { value: "darwin" })
+			mockVsCodeConfig("osx", "fish", {
+				fish: { path: "/usr/local/bin/fish" },
+			})
+			expect(getShell()).toBe("/usr/local/bin/fish")
+		})
+
+		it("returns defaultProfileName and matching profile for Linux", () => {
+			Object.defineProperty(process, "platform", { value: "linux" })
+			mockVsCodeConfig("linux", "zsh", {
+				zsh: { path: "/usr/bin/zsh" },
+			})
+			expect(getShell()).toBe("/usr/bin/zsh")
+		})
+
+		it("returns null defaultProfileName when config value is undefined", () => {
+			Object.defineProperty(process, "platform", { value: "linux" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.linux") return undefined
+					if (key === "profiles.linux") return { bash: { path: "/bin/bash" } }
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+			expect(getShell()).toBe("/bin/bash")
+		})
+
+		it("returns empty profiles when profiles config is null", () => {
+			Object.defineProperty(process, "platform", { value: "linux" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.linux") return "bash"
+					if (key === "profiles.linux") return null
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+			expect(getShell()).toBe("/bin/bash")
+		})
+
+		it("returns fallback when getConfiguration throws", () => {
+			Object.defineProperty(process, "platform", { value: "darwin" })
+			vi.mocked(vscode.workspace.getConfiguration).mockImplementation(() => {
+				throw new Error("config error")
+			})
+			expect(getShell()).toBe("/bin/zsh")
+		})
+
+		it("returns fallback when config.get throws", () => {
+			Object.defineProperty(process, "platform", { value: "linux" })
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
+				get: () => {
+					throw new Error("get error")
+				},
+			} as any)
+			expect(getShell()).toBe("/bin/bash")
+		})
+	})
+
+	// --------------------------------------------------------------------------
+	// Non-string defaultProfileName Handling
+	// --------------------------------------------------------------------------
+	describe("Non-string defaultProfileName handling", () => {
+		it("Windows: handles numeric defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "win32" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.windows") return 1
+					if (key === "profiles.windows") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+			vi.mocked(existsSync).mockReturnValue(false)
+
+			expect(getShell()).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")
+		})
+
+		it("Windows: handles boolean defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "win32" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.windows") return true
+					if (key === "profiles.windows") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+			vi.mocked(existsSync).mockReturnValue(false)
+
+			expect(getShell()).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")
+		})
+
+		it("Windows: handles array defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "win32" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.windows") return ["PowerShell"]
+					if (key === "profiles.windows") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+			vi.mocked(existsSync).mockReturnValue(false)
+
+			expect(getShell()).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")
+		})
+
+		it("Windows: handles object defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "win32" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.windows") return { name: "PowerShell" }
+					if (key === "profiles.windows") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+			vi.mocked(existsSync).mockReturnValue(false)
+
+			expect(getShell()).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")
+		})
+
+		it("macOS: handles numeric defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "darwin" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.osx") return 1
+					if (key === "profiles.osx") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+
+			expect(getShell()).toBe("/bin/zsh")
+		})
+
+		it("macOS: handles boolean defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "darwin" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.osx") return true
+					if (key === "profiles.osx") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+
+			expect(getShell()).toBe("/bin/zsh")
+		})
+
+		it("macOS: handles array defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "darwin" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.osx") return ["zsh"]
+					if (key === "profiles.osx") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+
+			expect(getShell()).toBe("/bin/zsh")
+		})
+
+		it("macOS: handles object defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "darwin" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.osx") return {}
+					if (key === "profiles.osx") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+
+			expect(getShell()).toBe("/bin/zsh")
+		})
+
+		// Mutation-resistant: without the typeof guard, profiles[1] === profiles["1"] in JS,
+		// so a numeric key that matches a real profile would return its path instead of falling back.
+		it("macOS: ignores numeric defaultProfileName even when it matches a profile key", () => {
+			Object.defineProperty(process, "platform", { value: "darwin" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.osx") return 1
+					// Profile keyed as "1" — would be reached by profiles[1] if the guard were absent
+					if (key === "profiles.osx") return { "1": { path: "/usr/local/bin/zsh" } }
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+
+			// Guard treats 1 as null → getMacShellFromVSCode returns null → fallback to /bin/zsh
+			// Without the guard it would return /usr/local/bin/zsh
+			expect(getShell()).toBe("/bin/zsh")
+		})
+
+		it("Linux: handles numeric defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "linux" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.linux") return 1
+					if (key === "profiles.linux") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+
+			expect(getShell()).toBe("/bin/bash")
+		})
+
+		it("Linux: handles boolean defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "linux" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.linux") return true
+					if (key === "profiles.linux") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+
+			expect(getShell()).toBe("/bin/bash")
+		})
+
+		it("Linux: handles array defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "linux" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.linux") return ["bash"]
+					if (key === "profiles.linux") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+
+			expect(getShell()).toBe("/bin/bash")
+		})
+
+		it("Linux: handles object defaultProfileName without TypeError", () => {
+			Object.defineProperty(process, "platform", { value: "linux" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.linux") return {}
+					if (key === "profiles.linux") return {}
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+
+			expect(getShell()).toBe("/bin/bash")
+		})
+
+		// Mutation-resistant: same pattern as macOS — numeric key matches profile "1" only if unguarded.
+		it("Linux: ignores numeric defaultProfileName even when it matches a profile key", () => {
+			Object.defineProperty(process, "platform", { value: "linux" })
+			const mockConfig = {
+				get: vi.fn((key: string) => {
+					if (key === "defaultProfile.linux") return 1
+					// Profile keyed as "1" — would be reached by profiles[1] if the guard were absent
+					if (key === "profiles.linux") return { "1": { path: "/usr/bin/fish" } }
+					return undefined
+				}),
+			}
+			vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
+
+			// Guard treats 1 as null → getLinuxShellFromVSCode returns null → fallback to /bin/bash
+			// Without the guard it would return /usr/bin/fish
+			expect(getShell()).toBe("/bin/bash")
+		})
+	})
+
 	// --------------------------------------------------------------------------
 	// Shell Validation Tests
 	// --------------------------------------------------------------------------
diff --git a/src/utils/shell.ts b/src/utils/shell.ts
@@ -136,36 +136,36 @@ type LinuxTerminalProfiles = Record<string, LinuxTerminalProfile>
 // 1) VS Code Terminal Configuration Helpers
 // -----------------------------------------------------
 
-function getWindowsTerminalConfig() {
-	try {
-		const config = vscode.workspace.getConfiguration("terminal.integrated")
-		const defaultProfileName = config.get<string>("defaultProfile.windows")
-		const profiles = config.get<WindowsTerminalProfiles>("profiles.windows") || {}
-		return { defaultProfileName, profiles }
-	} catch {
-		return { defaultProfileName: null, profiles: {} as WindowsTerminalProfiles }
-	}
-}
-
-function getMacTerminalConfig() {
-	try {
-		const config = vscode.workspace.getConfiguration("terminal.integrated")
-		const defaultProfileName = config.get<string>("defaultProfile.osx")
-		const profiles = config.get<MacTerminalProfiles>("profiles.osx") || {}
-		return { defaultProfileName, profiles }
-	} catch {
-		return { defaultProfileName: null, profiles: {} as MacTerminalProfiles }
-	}
+type PlatformProfilesMap = {
+	windows: WindowsTerminalProfiles
+	osx: MacTerminalProfiles
+	linux: LinuxTerminalProfiles
 }
 
-function getLinuxTerminalConfig() {
+/**
+ * Reads the VS Code terminal profile configuration for the given platform.
+ *
+ * The key must be one of `"windows"`, `"osx"`, or `"linux"` — the exact strings
+ * VS Code uses in `terminal.integrated.defaultProfile.<key>` and
+ * `terminal.integrated.profiles.<key>`. Passing any other value is a compile-time
+ * error, which prevents silent mismatches such as `"darwin"` returning empty config.
+ *
+ * The return type (`defaultProfileName` and `profiles`) is inferred from `K` via
+ * `PlatformProfilesMap`, so callers don't need an explicit type parameter.
+ *
+ * Returns `{ defaultProfileName: null, profiles: {} }` on any VS Code API error.
+ */
+function getTerminalConfig<K extends keyof PlatformProfilesMap>(
+	platformKey: K,
+): { defaultProfileName: string | null; profiles: PlatformProfilesMap[K] } {
 	try {
 		const config = vscode.workspace.getConfiguration("terminal.integrated")
-		const defaultProfileName = config.get<string>("defaultProfile.linux")
-		const profiles = config.get<LinuxTerminalProfiles>("profiles.linux") || {}
+		const rawProfileName = config.get<string>(`defaultProfile.${platformKey}`)
+		const defaultProfileName = typeof rawProfileName === "string" ? rawProfileName : null
+		const profiles = config.get<PlatformProfilesMap[K]>(`profiles.${platformKey}`) ?? ({} as PlatformProfilesMap[K])
 		return { defaultProfileName, profiles }
 	} catch {
-		return { defaultProfileName: null, profiles: {} as LinuxTerminalProfiles }
+		return { defaultProfileName: null, profiles: {} as PlatformProfilesMap[K] }
 	}
 }
 
@@ -187,7 +187,7 @@ function normalizeShellPath(path: string | string[] | undefined): string | null
 
 /** Attempts to retrieve a shell path from VS Code config on Windows. */
 function getWindowsShellFromVSCode(): string | null {
-	const { defaultProfileName, profiles } = getWindowsTerminalConfig()
+	const { defaultProfileName, profiles } = getTerminalConfig("windows")
 	if (!defaultProfileName) {
 		// No explicit Windows terminal profile is configured. VS Code auto-detects
 		// the default on modern Windows and prefers PowerShell 7 (pwsh.exe) when it
@@ -232,7 +232,7 @@ function getWindowsShellFromVSCode(): string | null {
 
 /** Attempts to retrieve a shell path from VS Code config on macOS. */
 function getMacShellFromVSCode(): string | null {
-	const { defaultProfileName, profiles } = getMacTerminalConfig()
+	const { defaultProfileName, profiles } = getTerminalConfig("osx")
 	if (!defaultProfileName) {
 		return null
 	}
@@ -243,7 +243,7 @@ function getMacShellFromVSCode(): string | null {
 
 /** Attempts to retrieve a shell path from VS Code config on Linux. */
 function getLinuxShellFromVSCode(): string | null {
-	const { defaultProfileName, profiles } = getLinuxTerminalConfig()
+	const { defaultProfileName, profiles } = getTerminalConfig("linux")
 	if (!defaultProfileName) {
 		return null
 	}
