Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/utils/__tests__/shell.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,23 +173,27 @@ describe("Shell Detection Tests", () => {
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
})

it("respects userInfo() if no VS Code config is available and shell is allowed", () => {
it("defaults to Windows PowerShell when VS Code has no configured profile", () => {
// Modern VS Code launches PowerShell by default on Windows (issue #82), so
// getShell() should report PowerShell rather than falling through to cmd.exe.
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
vi.mocked(userInfo).mockReturnValue({ shell: "C:\\Program Files\\PowerShell\\7\\pwsh.exe" } as any)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this mock still needed? With the early return at shell.ts:197, getShellFromUserInfo() is never called in this code path — so this mock value is never read. Removing it would make the test clearer about what's actually being exercised.

Suggested change
vi.mocked(userInfo).mockReturnValue({ shell: "C:\\Program Files\\PowerShell\\7\\pwsh.exe" } as any)


expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
expect(getShell()).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.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)
it("falls back to safe shell when the configured profile path is non-allowlisted", () => {
mockVsCodeConfig("windows", "Custom", {
Custom: { path: "C:\\Custom\\evil.exe" },
})

expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
})

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"
it("uses cmd.exe when a Command Prompt profile is explicitly configured", () => {
mockVsCodeConfig("windows", "Command Prompt", {
"Command Prompt": { path: "C:\\Windows\\System32\\cmd.exe" },
})

expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
})
Expand Down
8 changes: 7 additions & 1 deletion src/utils/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,13 @@ function normalizeShellPath(path: string | string[] | undefined): string | null
function getWindowsShellFromVSCode(): string | null {
const { defaultProfileName, profiles } = getWindowsTerminalConfig()
if (!defaultProfileName) {
return null
// No explicit Windows terminal profile is configured. VS Code's built-in
// default on modern Windows is PowerShell (not cmd.exe), and that is the
// shell the integrated terminal actually launches. Mirror it here so the
// system prompt advertises the real shell instead of falling through to
// COMSPEC (cmd.exe). Windows PowerShell is always present, so it is a safe
// allowlisted default. See issue #82.
return SHELL_PATHS.POWERSHELL_LEGACY
Comment on lines +191 to +197
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a machine with PowerShell 7 installed, VS Code's auto-detection will typically select PS7 as the default terminal — not legacy 5.1. Commit 1a2c45cb fixed the analogous case (profile name includes "powershell" but no path/source) by switching from POWERSHELL_LEGACYPOWERSHELL_7 for exactly this reason (UTF-8 mojibake). Should this path apply the same heuristic — e.g. probe for PS7 at its known path before falling back to legacy?

}

const profile = profiles[defaultProfileName]
Expand Down
Loading