Skip to content

Commit d7e3f70

Browse files
committed
fix(terminal): scope profile config to user settings, filter shellArgs, fix PATH join
1 parent 2286933 commit d7e3f70

4 files changed

Lines changed: 122 additions & 122 deletions

File tree

src/core/webview/__tests__/webviewMessageHandler.spec.ts

Lines changed: 4 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -900,27 +900,14 @@ describe("webviewMessageHandler - terminalProfile", () => {
900900
describe("webviewMessageHandler - requestTerminalProfiles", () => {
901901
beforeEach(() => {
902902
vi.clearAllMocks()
903-
vi.spyOn(Terminal, "resolveProfilePath").mockImplementation((profilePath) => {
904-
const candidates = Array.isArray(profilePath) ? profilePath : [profilePath]
905-
const value = candidates.find(
906-
(candidate) =>
907-
typeof candidate === "string" && candidate.trim().length > 0 && !candidate.includes("missing"),
908-
)
909-
return typeof value === "string" ? value.trim() : undefined
910-
})
911903
})
912904

913905
afterEach(() => {
914906
vi.restoreAllMocks()
915907
})
916908

917-
it("posts sorted path-resolvable profile names for the active platform", async () => {
918-
const mockGet = vi.fn().mockReturnValue({
919-
"Git Bash": { path: "C:\\Git\\bin\\bash.exe" },
920-
bash: { path: "/bin/bash" },
921-
PowerShell: { source: "PowerShell" }, // source-only — must be excluded
922-
})
923-
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
909+
it("posts available profile names", async () => {
910+
vi.spyOn(Terminal, "getAvailableProfileNames").mockReturnValue(["Git Bash", "bash"])
924911

925912
await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
926913

@@ -930,23 +917,8 @@ describe("webviewMessageHandler - requestTerminalProfiles", () => {
930917
})
931918
})
932919

933-
it("excludes source-only profiles that have no path field", async () => {
934-
const mockGet = vi.fn().mockReturnValue({
935-
PowerShell: { source: "PowerShell" },
936-
"Windows PowerShell": { source: "PowerShell" },
937-
})
938-
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
939-
940-
await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
941-
942-
expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
943-
type: "terminalProfiles",
944-
profiles: [],
945-
})
946-
})
947-
948-
it("posts an empty array when getConfiguration throws", async () => {
949-
vi.mocked(vscode.workspace.getConfiguration).mockImplementation(() => {
920+
it("posts an empty array when profile discovery throws", async () => {
921+
vi.spyOn(Terminal, "getAvailableProfileNames").mockImplementation(() => {
950922
throw new Error("config error")
951923
})
952924

@@ -957,65 +929,6 @@ describe("webviewMessageHandler - requestTerminalProfiles", () => {
957929
profiles: [],
958930
})
959931
})
960-
961-
it("posts an empty array when no profiles are configured", async () => {
962-
const mockGet = vi.fn().mockReturnValue(undefined)
963-
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
964-
965-
await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
966-
967-
expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
968-
type: "terminalProfiles",
969-
profiles: [],
970-
})
971-
})
972-
973-
it("excludes profiles with empty or whitespace-only path strings", async () => {
974-
const mockGet = vi.fn().mockReturnValue({
975-
empty: { path: "" },
976-
whitespace: { path: " " },
977-
valid: { path: "/bin/bash" },
978-
})
979-
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
980-
981-
await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
982-
983-
expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
984-
type: "terminalProfiles",
985-
profiles: ["valid"],
986-
})
987-
})
988-
989-
it("excludes profiles with path arrays containing only empty or whitespace strings", async () => {
990-
const mockGet = vi.fn().mockReturnValue({
991-
emptyArray: { path: [] },
992-
whitespaceArray: { path: ["", " "] },
993-
valid: { path: ["/bin/bash", "/usr/bin/bash"] },
994-
})
995-
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
996-
997-
await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
998-
999-
expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
1000-
type: "terminalProfiles",
1001-
profiles: ["valid"],
1002-
})
1003-
})
1004-
1005-
it("excludes profiles whose executable cannot be resolved", async () => {
1006-
const mockGet = vi.fn().mockReturnValue({
1007-
missing: { path: "/missing/bash" },
1008-
valid: { path: "/bin/bash" },
1009-
})
1010-
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
1011-
1012-
await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
1013-
1014-
expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
1015-
type: "terminalProfiles",
1016-
profiles: ["valid"],
1017-
})
1018-
})
1019932
})
1020933

1021934
describe("webviewMessageHandler - openTerminalProfilePicker", () => {

src/core/webview/webviewMessageHandler.ts

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1527,32 +1527,9 @@ export const webviewMessageHandler = async (
15271527
// profiles (e.g. { source: "PowerShell" }) cannot be mapped to a shell
15281528
// binary by an extension and would silently fall back to the default.
15291529
try {
1530-
const names = new Set<string>()
1531-
1532-
const platformKey = Terminal.getPlatformProfileKey()
1533-
const profiles = vscode.workspace
1534-
.getConfiguration("terminal.integrated.profiles")
1535-
.get<Record<string, unknown>>(platformKey)
1536-
1537-
if (profiles && typeof profiles === "object") {
1538-
for (const [name, entry] of Object.entries(profiles)) {
1539-
if (!entry || typeof entry !== "object") {
1540-
continue
1541-
}
1542-
1543-
const { path } = entry as { path?: unknown }
1544-
1545-
// Source-only profiles and paths that cannot be found on disk or
1546-
// PATH are excluded because the override would fail at launch.
1547-
if (Terminal.resolveProfilePath(path)) {
1548-
names.add(name)
1549-
}
1550-
}
1551-
}
1552-
15531530
await provider.postMessageToWebview({
15541531
type: "terminalProfiles",
1555-
profiles: Array.from(names).sort(),
1532+
profiles: Terminal.getAvailableProfileNames(),
15561533
})
15571534
} catch (error) {
15581535
console.error("Failed to get terminal profiles:", error)

src/integrations/terminal/Terminal.ts

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ export class Terminal extends BaseTerminal {
251251
const candidates = Array.isArray(profilePath) ? profilePath : [profilePath]
252252
const pathValue = env.PATH ?? env.Path ?? env.path
253253
const pathEntries = pathValue?.split(platform === "win32" ? ";" : ":") ?? []
254+
const platformJoin = platform === "win32" ? path.win32.join : path.posix.join
254255

255256
for (const value of candidates) {
256257
if (typeof value !== "string") {
@@ -280,7 +281,7 @@ export class Terminal extends BaseTerminal {
280281
const directory = entry.replace(/^"(.*)"$/, "$1")
281282

282283
for (const extension of extensions) {
283-
const resolved = path.join(directory, `${candidate}${extension}`)
284+
const resolved = platformJoin(directory, `${candidate}${extension}`)
284285

285286
if (existsSync(resolved)) {
286287
return resolved
@@ -292,6 +293,41 @@ export class Terminal extends BaseTerminal {
292293
return undefined
293294
}
294295

296+
/**
297+
* Reads profiles from trusted settings scopes only. Workspace settings are
298+
* intentionally excluded because opening a repository must not allow its
299+
* `.vscode/settings.json` to select an executable for Zoo Code to launch.
300+
*/
301+
public static getConfiguredProfiles(platform: NodeJS.Platform = process.platform): Record<string, unknown> {
302+
const platformKey = Terminal.getPlatformProfileKey(platform)
303+
const inspected = vscode.workspace
304+
.getConfiguration("terminal.integrated.profiles")
305+
.inspect<Record<string, unknown>>(platformKey)
306+
307+
return {
308+
...(inspected?.defaultValue ?? {}),
309+
...(inspected?.globalValue ?? {}),
310+
}
311+
}
312+
313+
public static getAvailableProfileNames(platform: NodeJS.Platform = process.platform): string[] {
314+
const names = new Set<string>()
315+
316+
for (const [name, entry] of Object.entries(Terminal.getConfiguredProfiles(platform))) {
317+
if (!entry || typeof entry !== "object") {
318+
continue
319+
}
320+
321+
const { path: profilePath } = entry as { path?: unknown }
322+
323+
if (Terminal.resolveProfilePath(profilePath, platform)) {
324+
names.add(name)
325+
}
326+
}
327+
328+
return Array.from(names).sort()
329+
}
330+
295331
/**
296332
* Resolves the configured VS Code terminal profile (see `terminalProfile`
297333
* setting / {@link Terminal.getTerminalProfile}) into a shell path and args by
@@ -317,9 +353,7 @@ export class Terminal extends BaseTerminal {
317353

318354
const platformKey = Terminal.getPlatformProfileKey(platform)
319355

320-
const profiles = vscode.workspace
321-
.getConfiguration("terminal.integrated.profiles")
322-
.get<Record<string, unknown>>(platformKey)
356+
const profiles = Terminal.getConfiguredProfiles(platform)
323357

324358
const profile = profiles?.[profileName] as
325359
| {
@@ -348,7 +382,7 @@ export class Terminal extends BaseTerminal {
348382
}
349383

350384
const shellArgs = Array.isArray(profile.args)
351-
? profile.args
385+
? profile.args.filter((arg): arg is string => typeof arg === "string")
352386
: typeof profile.args === "string"
353387
? [profile.args]
354388
: undefined

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

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,17 @@ describe("Terminal VS Code terminal profile (#277)", () => {
3838
}) as any
3939

4040
// Helper to stub `terminal.integrated.profiles.<platform>` config reads.
41-
const stubProfiles = (profilesByPlatform: Record<string, unknown>) => {
41+
const stubProfiles = (
42+
profilesByPlatform: Record<string, unknown>,
43+
workspaceProfilesByPlatform: Record<string, unknown> = {},
44+
) => {
4245
getConfigurationSpy = vi.spyOn(vscode.workspace, "getConfiguration").mockImplementation((section?: string) => {
4346
if (section === "terminal.integrated.profiles") {
4447
return {
45-
get: (platformKey: string) => profilesByPlatform[platformKey],
48+
inspect: (platformKey: string) => ({
49+
defaultValue: profilesByPlatform[platformKey],
50+
workspaceValue: workspaceProfilesByPlatform[platformKey],
51+
}),
4652
} as any
4753
}
4854

@@ -84,6 +90,45 @@ describe("Terminal VS Code terminal profile (#277)", () => {
8490
})
8591
})
8692

93+
describe("getConfiguredProfiles / getAvailableProfileNames", () => {
94+
it("merges default and global profiles while ignoring workspace profiles", () => {
95+
getConfigurationSpy = vi
96+
.spyOn(vscode.workspace, "getConfiguration")
97+
.mockImplementation((section?: string) => {
98+
if (section === "terminal.integrated.profiles") {
99+
return {
100+
inspect: () => ({
101+
defaultValue: { bash: { path: "/bin/bash" } },
102+
globalValue: { zsh: { path: "/bin/zsh" } },
103+
workspaceValue: { malicious: { path: "/workspace/malicious-shell" } },
104+
}),
105+
} as any
106+
}
107+
108+
return { get: (_key: string, defaultValue?: unknown) => defaultValue } as any
109+
})
110+
111+
expect(Terminal.getConfiguredProfiles("linux")).toEqual({
112+
bash: { path: "/bin/bash" },
113+
zsh: { path: "/bin/zsh" },
114+
})
115+
})
116+
117+
it("returns sorted names for profiles with resolvable paths only", () => {
118+
stubProfiles({
119+
linux: {
120+
zsh: { path: "/bin/zsh" },
121+
PowerShell: { source: "PowerShell" },
122+
bash: { path: "/bin/bash" },
123+
missing: { path: "/missing/bash" },
124+
},
125+
})
126+
mockedExistsSync.mockImplementation((profilePath: string) => profilePath !== "/missing/bash")
127+
128+
expect(Terminal.getAvailableProfileNames("linux")).toEqual(["bash", "zsh"])
129+
})
130+
})
131+
87132
describe("getProfileShell", () => {
88133
it("returns undefined when no profile is configured (default behavior preserved)", () => {
89134
stubProfiles({})
@@ -177,6 +222,21 @@ describe("Terminal VS Code terminal profile (#277)", () => {
177222
})
178223
})
179224

225+
it("drops non-string args array entries", () => {
226+
stubProfiles({
227+
linux: {
228+
bash: { path: "/bin/bash", args: ["-l", 42, null] },
229+
},
230+
})
231+
232+
Terminal.setTerminalProfile("bash")
233+
234+
expect(Terminal.getProfileShell("linux")).toEqual({
235+
shellPath: "/bin/bash",
236+
shellArgs: ["-l"],
237+
})
238+
})
239+
180240
it("reads the osx profile section on darwin", () => {
181241
stubProfiles({
182242
osx: { zsh: { path: "/bin/zsh" } },
@@ -251,5 +311,21 @@ describe("Terminal VS Code terminal profile (#277)", () => {
251311
expect(options.shellPath).toBe("/usr/bin/bash")
252312
expect(options.shellArgs).toEqual(["-i"])
253313
})
314+
315+
it("falls back to VS Code defaults when a configured profile disappears", () => {
316+
stubProfiles({
317+
[Terminal.getPlatformProfileKey(process.platform)]: {
318+
"Git Bash": { path: "/missing/bash" },
319+
},
320+
})
321+
mockedExistsSync.mockReturnValue(false)
322+
323+
Terminal.setTerminalProfile("Git Bash")
324+
TerminalRegistry.createTerminal("/test/path", "vscode")
325+
326+
const options = createTerminalSpy.mock.calls[0][0] as vscode.TerminalOptions
327+
expect(options.shellPath).toBeUndefined()
328+
expect(options.shellArgs).toBeUndefined()
329+
})
254330
})
255331
})

0 commit comments

Comments
 (0)