Skip to content

Commit a57818c

Browse files
committed
fix(terminal): skip ZDOTDIR injection with profile override; clear cached profile on picker open
1 parent ad78cf5 commit a57818c

8 files changed

Lines changed: 143 additions & 5 deletions

File tree

src/integrations/terminal/__tests__/TerminalProcessExec.bash.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ vi.mock("vscode", () => {
3737
eventHandlers.closeTerminal = handler
3838
return { dispose: vi.fn() }
3939
}),
40+
onDidChangeTerminalShellIntegration: vi.fn().mockReturnValue({ dispose: vi.fn() }),
4041
},
4142
ThemeIcon: class ThemeIcon {
4243
constructor(id: string) {
@@ -92,7 +93,7 @@ function createRealCommandStream(command: string): { stream: AsyncIterable<strin
9293
exitCode = 1
9394
}
9495
} else {
95-
exitCode = error.status || 1 // Use status if available, default to 1
96+
exitCode = error.status ?? 1 // Use status if available, default to 1
9697
}
9798
}
9899

src/integrations/terminal/__tests__/TerminalProcessExec.cmd.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ vi.mock("vscode", () => {
4242
eventHandlers.closeTerminal = handler
4343
return { dispose: vi.fn() }
4444
}),
45+
onDidChangeTerminalShellIntegration: vi.fn().mockReturnValue({ dispose: vi.fn() }),
4546
},
4647
ThemeIcon: class ThemeIcon {
4748
constructor(id: string) {

src/integrations/terminal/__tests__/TerminalProcessExec.pwsh.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ vi.mock("vscode", () => {
4242
eventHandlers.closeTerminal = handler
4343
return { dispose: vi.fn() }
4444
}),
45+
onDidChangeTerminalShellIntegration: vi.fn().mockReturnValue({ dispose: vi.fn() }),
4546
},
4647
ThemeIcon: class ThemeIcon {
4748
constructor(id: string) {

src/integrations/terminal/__tests__/TerminalProfile.spec.ts

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as vscode from "vscode"
66

77
import { Terminal } from "../Terminal"
88
import { TerminalRegistry } from "../TerminalRegistry"
9+
import { ShellIntegrationManager } from "../ShellIntegrationManager"
910

1011
vi.mock("execa", () => ({
1112
execa: vi.fn(),
@@ -127,6 +128,102 @@ describe("Terminal VS Code terminal profile (#277)", () => {
127128

128129
expect(Terminal.getAvailableProfileNames("linux")).toEqual(["bash", "zsh"])
129130
})
131+
132+
it("excludes cmd.exe profiles on Windows (shell integration unsupported)", () => {
133+
stubProfiles({
134+
windows: {
135+
"Command Prompt": { path: "C:\\Windows\\System32\\cmd.exe" },
136+
PowerShell: { path: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" },
137+
},
138+
})
139+
140+
expect(Terminal.getAvailableProfileNames("win32")).toEqual(["PowerShell"])
141+
})
142+
143+
describe("isCmdExe", () => {
144+
it.each([
145+
["C:\\Windows\\System32\\cmd.exe", true],
146+
["C:\\WINDOWS\\SYSTEM32\\CMD.EXE", true],
147+
["/mnt/c/Windows/System32/cmd.exe", true],
148+
["/bin/bash", false],
149+
["pwsh.exe", false],
150+
["cmd", false],
151+
])("isCmdExe(%s) === %s", (input, expected) => {
152+
expect(Terminal.isCmdExe(input)).toBe(expected)
153+
})
154+
})
155+
156+
describe("isActiveShellCmdExe", () => {
157+
it("returns false on non-Windows platforms", () => {
158+
expect(Terminal.isActiveShellCmdExe("linux")).toBe(false)
159+
expect(Terminal.isActiveShellCmdExe("darwin")).toBe(false)
160+
})
161+
162+
it("returns true when profile override resolves to cmd.exe", () => {
163+
stubProfiles({ windows: { "Command Prompt": { path: "C:\\Windows\\System32\\cmd.exe" } } })
164+
Terminal.setTerminalProfile("Command Prompt")
165+
expect(Terminal.isActiveShellCmdExe("win32")).toBe(true)
166+
})
167+
168+
it("returns false when profile override resolves to a non-cmd shell", () => {
169+
stubProfiles({ windows: { PowerShell: { path: "C:\\Program Files\\PowerShell\\pwsh.exe" } } })
170+
Terminal.setTerminalProfile("PowerShell")
171+
expect(Terminal.isActiveShellCmdExe("win32")).toBe(false)
172+
})
173+
174+
it("returns true when no override and default profile is cmd.exe", () => {
175+
Terminal.setTerminalProfile(undefined)
176+
stubProfiles({ windows: { "Command Prompt": { path: "C:\\Windows\\System32\\cmd.exe" } } })
177+
getConfigurationSpy = vi
178+
.spyOn(vscode.workspace, "getConfiguration")
179+
.mockImplementation((section?: string) => {
180+
if (section === "terminal.integrated.profiles") {
181+
return {
182+
inspect: (_key: string) => ({
183+
defaultValue: { "Command Prompt": { path: "C:\\Windows\\System32\\cmd.exe" } },
184+
globalValue: undefined,
185+
}),
186+
} as any
187+
}
188+
if (section === "terminal.integrated") {
189+
return {
190+
get: (key: string) => (key === "defaultProfile.windows" ? "Command Prompt" : undefined),
191+
} as any
192+
}
193+
return { get: (_key: string, defaultValue?: unknown) => defaultValue } as any
194+
})
195+
expect(Terminal.isActiveShellCmdExe("win32")).toBe(true)
196+
})
197+
198+
it("returns false when no override and default profile is PowerShell", () => {
199+
Terminal.setTerminalProfile(undefined)
200+
getConfigurationSpy = vi
201+
.spyOn(vscode.workspace, "getConfiguration")
202+
.mockImplementation((section?: string) => {
203+
if (section === "terminal.integrated.profiles") {
204+
return {
205+
inspect: (_key: string) => ({
206+
defaultValue: { PowerShell: { path: "C:\\Program Files\\PowerShell\\pwsh.exe" } },
207+
globalValue: undefined,
208+
}),
209+
} as any
210+
}
211+
if (section === "terminal.integrated") {
212+
return {
213+
get: (key: string) => (key === "defaultProfile.windows" ? "PowerShell" : undefined),
214+
} as any
215+
}
216+
return { get: (_key: string, defaultValue?: unknown) => defaultValue } as any
217+
})
218+
expect(Terminal.isActiveShellCmdExe("win32")).toBe(false)
219+
})
220+
221+
it("returns false when no override and no default profile configured", () => {
222+
Terminal.setTerminalProfile(undefined)
223+
stubProfiles({})
224+
expect(Terminal.isActiveShellCmdExe("win32")).toBe(false)
225+
})
226+
})
130227
})
131228

132229
describe("getProfileShell", () => {
@@ -328,4 +425,41 @@ describe("Terminal VS Code terminal profile (#277)", () => {
328425
expect(options.shellArgs).toBeUndefined()
329426
})
330427
})
428+
429+
describe("ZDOTDIR injection guard", () => {
430+
let zshInitTmpDirSpy: any
431+
432+
beforeEach(() => {
433+
zshInitTmpDirSpy = vi
434+
.spyOn(ShellIntegrationManager, "zshInitTmpDir")
435+
.mockReturnValue("/tmp/roo-zdotdir-test")
436+
Terminal.setTerminalZdotdir(true)
437+
})
438+
439+
afterEach(() => {
440+
Terminal.setTerminalZdotdir(false)
441+
Terminal.setTerminalProfile(undefined)
442+
TerminalRegistry["terminals"] = []
443+
vi.restoreAllMocks()
444+
})
445+
446+
it("sets ZDOTDIR when zdotdir is enabled and no profile is configured", () => {
447+
stubProfiles({})
448+
const env = Terminal.getEnv()
449+
expect(zshInitTmpDirSpy).toHaveBeenCalledTimes(1)
450+
expect(env.ZDOTDIR).toBe("/tmp/roo-zdotdir-test")
451+
})
452+
453+
it("skips ZDOTDIR when zdotdir is enabled but a profile is configured", () => {
454+
stubProfiles({
455+
[Terminal.getPlatformProfileKey(process.platform)]: {
456+
zsh: { path: "/bin/zsh" },
457+
},
458+
})
459+
Terminal.setTerminalProfile("zsh")
460+
const env = Terminal.getEnv()
461+
expect(zshInitTmpDirSpy).not.toHaveBeenCalled()
462+
expect(env.ZDOTDIR).toBeUndefined()
463+
})
464+
})
331465
})

src/integrations/terminal/__tests__/streamUtils/bashStream.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export function createBashCommandStream(command: string): CommandStream {
4040
exitCode = 1
4141
}
4242
} else {
43-
exitCode = error.status || 1 // Use status if available, default to 1
43+
exitCode = error.status ?? 1 // Use status if available, default to 1
4444
}
4545
}
4646

src/integrations/terminal/__tests__/streamUtils/cmdStream.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export function createCmdCommandStream(command: string): CommandStream {
2525
} catch (error: any) {
2626
// Command failed - get output and exit code from error
2727
realOutput = error.stdout?.toString() || ""
28-
exitCode = error.status || 1
28+
exitCode = error.status ?? 1
2929
}
3030

3131
// Create an async iterator for the stream

src/integrations/terminal/__tests__/streamUtils/pwshStream.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ export function createPowerShellStream(command: string): CommandStream {
3838
} catch (error: any) {
3939
// Command failed - get output and exit code from error
4040
realOutput = error.stdout?.toString() || ""
41-
console.error(`PowerShell command failed with status ${error.status || "unknown"}:`, error.message)
41+
console.error(`PowerShell command failed with status ${error.status ?? "unknown"}:`, error.message)
4242
if (error.stderr) {
4343
console.error(`stderr: ${error.stderr.toString()}`)
4444
}
45-
exitCode = error.status || 1
45+
exitCode = error.status ?? 1
4646
}
4747

4848
// Create an async iterator for the stream

webview-ui/src/components/settings/TerminalSettings.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ export const TerminalSettings = ({
192192
appearance="secondary"
193193
onClick={() => {
194194
onTerminalProfilePickerOpened?.()
195+
setCachedStateField("terminalProfile", undefined)
195196
vscode.postMessage({ type: "openTerminalProfilePicker" })
196197
}}
197198
data-testid="terminal-profile-configure-button">

0 commit comments

Comments
 (0)