Skip to content

Commit f490330

Browse files
author
CodeKing
committed
fix: shell default profile name type guard (Zoo-Code-Org#687)
* fix(shell): validate defaultProfileName is a string before calling .toLowerCase() config.get<string>() 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<T>() * 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)
1 parent 819139f commit f490330

2 files changed

Lines changed: 314 additions & 27 deletions

File tree

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

Lines changed: 287 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,293 @@ describe("Shell Detection Tests", () => {
370370
})
371371
})
372372

373+
// --------------------------------------------------------------------------
374+
// getTerminalConfig Behavior (tested via getShell)
375+
// --------------------------------------------------------------------------
376+
describe("getTerminalConfig", () => {
377+
it("returns defaultProfileName and matching profile for Windows", () => {
378+
Object.defineProperty(process, "platform", { value: "win32" })
379+
mockVsCodeConfig("windows", "Command Prompt", {
380+
"Command Prompt": { path: "C:\\Windows\\System32\\cmd.exe" },
381+
})
382+
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
383+
})
384+
385+
it("returns defaultProfileName and matching profile for macOS", () => {
386+
Object.defineProperty(process, "platform", { value: "darwin" })
387+
mockVsCodeConfig("osx", "fish", {
388+
fish: { path: "/usr/local/bin/fish" },
389+
})
390+
expect(getShell()).toBe("/usr/local/bin/fish")
391+
})
392+
393+
it("returns defaultProfileName and matching profile for Linux", () => {
394+
Object.defineProperty(process, "platform", { value: "linux" })
395+
mockVsCodeConfig("linux", "zsh", {
396+
zsh: { path: "/usr/bin/zsh" },
397+
})
398+
expect(getShell()).toBe("/usr/bin/zsh")
399+
})
400+
401+
it("returns null defaultProfileName when config value is undefined", () => {
402+
Object.defineProperty(process, "platform", { value: "linux" })
403+
const mockConfig = {
404+
get: vi.fn((key: string) => {
405+
if (key === "defaultProfile.linux") return undefined
406+
if (key === "profiles.linux") return { bash: { path: "/bin/bash" } }
407+
return undefined
408+
}),
409+
}
410+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
411+
expect(getShell()).toBe("/bin/bash")
412+
})
413+
414+
it("returns empty profiles when profiles config is null", () => {
415+
Object.defineProperty(process, "platform", { value: "linux" })
416+
const mockConfig = {
417+
get: vi.fn((key: string) => {
418+
if (key === "defaultProfile.linux") return "bash"
419+
if (key === "profiles.linux") return null
420+
return undefined
421+
}),
422+
}
423+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
424+
expect(getShell()).toBe("/bin/bash")
425+
})
426+
427+
it("returns fallback when getConfiguration throws", () => {
428+
Object.defineProperty(process, "platform", { value: "darwin" })
429+
vi.mocked(vscode.workspace.getConfiguration).mockImplementation(() => {
430+
throw new Error("config error")
431+
})
432+
expect(getShell()).toBe("/bin/zsh")
433+
})
434+
435+
it("returns fallback when config.get throws", () => {
436+
Object.defineProperty(process, "platform", { value: "linux" })
437+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
438+
get: () => {
439+
throw new Error("get error")
440+
},
441+
} as any)
442+
expect(getShell()).toBe("/bin/bash")
443+
})
444+
})
445+
446+
// --------------------------------------------------------------------------
447+
// Non-string defaultProfileName Handling
448+
// --------------------------------------------------------------------------
449+
describe("Non-string defaultProfileName handling", () => {
450+
it("Windows: handles numeric defaultProfileName without TypeError", () => {
451+
Object.defineProperty(process, "platform", { value: "win32" })
452+
const mockConfig = {
453+
get: vi.fn((key: string) => {
454+
if (key === "defaultProfile.windows") return 1
455+
if (key === "profiles.windows") return {}
456+
return undefined
457+
}),
458+
}
459+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
460+
vi.mocked(existsSync).mockReturnValue(false)
461+
462+
expect(getShell()).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")
463+
})
464+
465+
it("Windows: handles boolean defaultProfileName without TypeError", () => {
466+
Object.defineProperty(process, "platform", { value: "win32" })
467+
const mockConfig = {
468+
get: vi.fn((key: string) => {
469+
if (key === "defaultProfile.windows") return true
470+
if (key === "profiles.windows") return {}
471+
return undefined
472+
}),
473+
}
474+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
475+
vi.mocked(existsSync).mockReturnValue(false)
476+
477+
expect(getShell()).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")
478+
})
479+
480+
it("Windows: handles array defaultProfileName without TypeError", () => {
481+
Object.defineProperty(process, "platform", { value: "win32" })
482+
const mockConfig = {
483+
get: vi.fn((key: string) => {
484+
if (key === "defaultProfile.windows") return ["PowerShell"]
485+
if (key === "profiles.windows") return {}
486+
return undefined
487+
}),
488+
}
489+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
490+
vi.mocked(existsSync).mockReturnValue(false)
491+
492+
expect(getShell()).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")
493+
})
494+
495+
it("Windows: handles object defaultProfileName without TypeError", () => {
496+
Object.defineProperty(process, "platform", { value: "win32" })
497+
const mockConfig = {
498+
get: vi.fn((key: string) => {
499+
if (key === "defaultProfile.windows") return { name: "PowerShell" }
500+
if (key === "profiles.windows") return {}
501+
return undefined
502+
}),
503+
}
504+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
505+
vi.mocked(existsSync).mockReturnValue(false)
506+
507+
expect(getShell()).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")
508+
})
509+
510+
it("macOS: handles numeric defaultProfileName without TypeError", () => {
511+
Object.defineProperty(process, "platform", { value: "darwin" })
512+
const mockConfig = {
513+
get: vi.fn((key: string) => {
514+
if (key === "defaultProfile.osx") return 1
515+
if (key === "profiles.osx") return {}
516+
return undefined
517+
}),
518+
}
519+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
520+
521+
expect(getShell()).toBe("/bin/zsh")
522+
})
523+
524+
it("macOS: handles boolean defaultProfileName without TypeError", () => {
525+
Object.defineProperty(process, "platform", { value: "darwin" })
526+
const mockConfig = {
527+
get: vi.fn((key: string) => {
528+
if (key === "defaultProfile.osx") return true
529+
if (key === "profiles.osx") return {}
530+
return undefined
531+
}),
532+
}
533+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
534+
535+
expect(getShell()).toBe("/bin/zsh")
536+
})
537+
538+
it("macOS: handles array defaultProfileName without TypeError", () => {
539+
Object.defineProperty(process, "platform", { value: "darwin" })
540+
const mockConfig = {
541+
get: vi.fn((key: string) => {
542+
if (key === "defaultProfile.osx") return ["zsh"]
543+
if (key === "profiles.osx") return {}
544+
return undefined
545+
}),
546+
}
547+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
548+
549+
expect(getShell()).toBe("/bin/zsh")
550+
})
551+
552+
it("macOS: handles object defaultProfileName without TypeError", () => {
553+
Object.defineProperty(process, "platform", { value: "darwin" })
554+
const mockConfig = {
555+
get: vi.fn((key: string) => {
556+
if (key === "defaultProfile.osx") return {}
557+
if (key === "profiles.osx") return {}
558+
return undefined
559+
}),
560+
}
561+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
562+
563+
expect(getShell()).toBe("/bin/zsh")
564+
})
565+
566+
// Mutation-resistant: without the typeof guard, profiles[1] === profiles["1"] in JS,
567+
// so a numeric key that matches a real profile would return its path instead of falling back.
568+
it("macOS: ignores numeric defaultProfileName even when it matches a profile key", () => {
569+
Object.defineProperty(process, "platform", { value: "darwin" })
570+
const mockConfig = {
571+
get: vi.fn((key: string) => {
572+
if (key === "defaultProfile.osx") return 1
573+
// Profile keyed as "1" — would be reached by profiles[1] if the guard were absent
574+
if (key === "profiles.osx") return { "1": { path: "/usr/local/bin/zsh" } }
575+
return undefined
576+
}),
577+
}
578+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
579+
580+
// Guard treats 1 as null → getMacShellFromVSCode returns null → fallback to /bin/zsh
581+
// Without the guard it would return /usr/local/bin/zsh
582+
expect(getShell()).toBe("/bin/zsh")
583+
})
584+
585+
it("Linux: handles numeric defaultProfileName without TypeError", () => {
586+
Object.defineProperty(process, "platform", { value: "linux" })
587+
const mockConfig = {
588+
get: vi.fn((key: string) => {
589+
if (key === "defaultProfile.linux") return 1
590+
if (key === "profiles.linux") return {}
591+
return undefined
592+
}),
593+
}
594+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
595+
596+
expect(getShell()).toBe("/bin/bash")
597+
})
598+
599+
it("Linux: handles boolean defaultProfileName without TypeError", () => {
600+
Object.defineProperty(process, "platform", { value: "linux" })
601+
const mockConfig = {
602+
get: vi.fn((key: string) => {
603+
if (key === "defaultProfile.linux") return true
604+
if (key === "profiles.linux") return {}
605+
return undefined
606+
}),
607+
}
608+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
609+
610+
expect(getShell()).toBe("/bin/bash")
611+
})
612+
613+
it("Linux: handles array defaultProfileName without TypeError", () => {
614+
Object.defineProperty(process, "platform", { value: "linux" })
615+
const mockConfig = {
616+
get: vi.fn((key: string) => {
617+
if (key === "defaultProfile.linux") return ["bash"]
618+
if (key === "profiles.linux") return {}
619+
return undefined
620+
}),
621+
}
622+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
623+
624+
expect(getShell()).toBe("/bin/bash")
625+
})
626+
627+
it("Linux: handles object defaultProfileName without TypeError", () => {
628+
Object.defineProperty(process, "platform", { value: "linux" })
629+
const mockConfig = {
630+
get: vi.fn((key: string) => {
631+
if (key === "defaultProfile.linux") return {}
632+
if (key === "profiles.linux") return {}
633+
return undefined
634+
}),
635+
}
636+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
637+
638+
expect(getShell()).toBe("/bin/bash")
639+
})
640+
641+
// Mutation-resistant: same pattern as macOS — numeric key matches profile "1" only if unguarded.
642+
it("Linux: ignores numeric defaultProfileName even when it matches a profile key", () => {
643+
Object.defineProperty(process, "platform", { value: "linux" })
644+
const mockConfig = {
645+
get: vi.fn((key: string) => {
646+
if (key === "defaultProfile.linux") return 1
647+
// Profile keyed as "1" — would be reached by profiles[1] if the guard were absent
648+
if (key === "profiles.linux") return { "1": { path: "/usr/bin/fish" } }
649+
return undefined
650+
}),
651+
}
652+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
653+
654+
// Guard treats 1 as null → getLinuxShellFromVSCode returns null → fallback to /bin/bash
655+
// Without the guard it would return /usr/bin/fish
656+
expect(getShell()).toBe("/bin/bash")
657+
})
658+
})
659+
373660
// --------------------------------------------------------------------------
374661
// Shell Validation Tests
375662
// --------------------------------------------------------------------------

‎src/utils/shell.ts‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -136,36 +136,36 @@ type LinuxTerminalProfiles = Record<string, LinuxTerminalProfile>
136136
// 1) VS Code Terminal Configuration Helpers
137137
// -----------------------------------------------------
138138

139-
function getWindowsTerminalConfig() {
140-
try {
141-
const config = vscode.workspace.getConfiguration("terminal.integrated")
142-
const defaultProfileName = config.get<string>("defaultProfile.windows")
143-
const profiles = config.get<WindowsTerminalProfiles>("profiles.windows") || {}
144-
return { defaultProfileName, profiles }
145-
} catch {
146-
return { defaultProfileName: null, profiles: {} as WindowsTerminalProfiles }
147-
}
148-
}
149-
150-
function getMacTerminalConfig() {
151-
try {
152-
const config = vscode.workspace.getConfiguration("terminal.integrated")
153-
const defaultProfileName = config.get<string>("defaultProfile.osx")
154-
const profiles = config.get<MacTerminalProfiles>("profiles.osx") || {}
155-
return { defaultProfileName, profiles }
156-
} catch {
157-
return { defaultProfileName: null, profiles: {} as MacTerminalProfiles }
158-
}
139+
type PlatformProfilesMap = {
140+
windows: WindowsTerminalProfiles
141+
osx: MacTerminalProfiles
142+
linux: LinuxTerminalProfiles
159143
}
160144

161-
function getLinuxTerminalConfig() {
145+
/**
146+
* Reads the VS Code terminal profile configuration for the given platform.
147+
*
148+
* The key must be one of `"windows"`, `"osx"`, or `"linux"` — the exact strings
149+
* VS Code uses in `terminal.integrated.defaultProfile.<key>` and
150+
* `terminal.integrated.profiles.<key>`. Passing any other value is a compile-time
151+
* error, which prevents silent mismatches such as `"darwin"` returning empty config.
152+
*
153+
* The return type (`defaultProfileName` and `profiles`) is inferred from `K` via
154+
* `PlatformProfilesMap`, so callers don't need an explicit type parameter.
155+
*
156+
* Returns `{ defaultProfileName: null, profiles: {} }` on any VS Code API error.
157+
*/
158+
function getTerminalConfig<K extends keyof PlatformProfilesMap>(
159+
platformKey: K,
160+
): { defaultProfileName: string | null; profiles: PlatformProfilesMap[K] } {
162161
try {
163162
const config = vscode.workspace.getConfiguration("terminal.integrated")
164-
const defaultProfileName = config.get<string>("defaultProfile.linux")
165-
const profiles = config.get<LinuxTerminalProfiles>("profiles.linux") || {}
163+
const rawProfileName = config.get<string>(`defaultProfile.${platformKey}`)
164+
const defaultProfileName = typeof rawProfileName === "string" ? rawProfileName : null
165+
const profiles = config.get<PlatformProfilesMap[K]>(`profiles.${platformKey}`) ?? ({} as PlatformProfilesMap[K])
166166
return { defaultProfileName, profiles }
167167
} catch {
168-
return { defaultProfileName: null, profiles: {} as LinuxTerminalProfiles }
168+
return { defaultProfileName: null, profiles: {} as PlatformProfilesMap[K] }
169169
}
170170
}
171171

@@ -187,7 +187,7 @@ function normalizeShellPath(path: string | string[] | undefined): string | null
187187

188188
/** Attempts to retrieve a shell path from VS Code config on Windows. */
189189
function getWindowsShellFromVSCode(): string | null {
190-
const { defaultProfileName, profiles } = getWindowsTerminalConfig()
190+
const { defaultProfileName, profiles } = getTerminalConfig("windows")
191191
if (!defaultProfileName) {
192192
// No explicit Windows terminal profile is configured. VS Code auto-detects
193193
// the default on modern Windows and prefers PowerShell 7 (pwsh.exe) when it
@@ -232,7 +232,7 @@ function getWindowsShellFromVSCode(): string | null {
232232

233233
/** Attempts to retrieve a shell path from VS Code config on macOS. */
234234
function getMacShellFromVSCode(): string | null {
235-
const { defaultProfileName, profiles } = getMacTerminalConfig()
235+
const { defaultProfileName, profiles } = getTerminalConfig("osx")
236236
if (!defaultProfileName) {
237237
return null
238238
}
@@ -243,7 +243,7 @@ function getMacShellFromVSCode(): string | null {
243243

244244
/** Attempts to retrieve a shell path from VS Code config on Linux. */
245245
function getLinuxShellFromVSCode(): string | null {
246-
const { defaultProfileName, profiles } = getLinuxTerminalConfig()
246+
const { defaultProfileName, profiles } = getTerminalConfig("linux")
247247
if (!defaultProfileName) {
248248
return null
249249
}

0 commit comments

Comments
 (0)