Skip to content

Commit c659190

Browse files
committed
fix: handle array paths from VSCode terminal profiles
- Updated terminal profile interfaces to support string | string[] for path property - Added normalizeShellPath helper to safely extract first element from array paths - Modified isShellAllowed to handle both string and array inputs - Updated getWindowsShellFromVSCode, getMacShellFromVSCode, and getLinuxShellFromVSCode to use normalizeShellPath - Added comprehensive tests for array path handling Fixes #7695
1 parent 90e7d09 commit c659190

File tree

2 files changed

+395
-13
lines changed

2 files changed

+395
-13
lines changed

src/utils/__tests__/shell.test.ts

Lines changed: 364 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,364 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
2+
import * as vscode from "vscode"
3+
4+
// Mock vscode module
5+
vi.mock("vscode", () => ({
6+
workspace: {
7+
getConfiguration: vi.fn(),
8+
},
9+
}))
10+
11+
// Mock os module
12+
vi.mock("os", () => ({
13+
userInfo: vi.fn(),
14+
}))
15+
16+
describe("shell utilities", () => {
17+
let originalPlatform: PropertyDescriptor | undefined
18+
let originalEnv: NodeJS.ProcessEnv
19+
20+
beforeEach(() => {
21+
// Save original values
22+
originalPlatform = Object.getOwnPropertyDescriptor(process, "platform")
23+
originalEnv = process.env
24+
25+
// Reset mocks
26+
vi.clearAllMocks()
27+
28+
// Reset module cache to ensure fresh imports
29+
vi.resetModules()
30+
})
31+
32+
afterEach(() => {
33+
// Restore original values
34+
if (originalPlatform) {
35+
Object.defineProperty(process, "platform", originalPlatform)
36+
}
37+
process.env = originalEnv
38+
})
39+
40+
describe("getShell", () => {
41+
it("should handle string path from VSCode terminal profile on Windows", async () => {
42+
// Set platform to Windows
43+
Object.defineProperty(process, "platform", {
44+
value: "win32",
45+
configurable: true,
46+
})
47+
48+
// Mock VSCode configuration
49+
const mockConfig = {
50+
get: vi.fn((key: string) => {
51+
if (key === "defaultProfile.windows") return "PowerShell"
52+
if (key === "profiles.windows") {
53+
return {
54+
PowerShell: {
55+
path: "C:\\Program Files\\PowerShell\\7\\pwsh.exe",
56+
},
57+
}
58+
}
59+
return undefined
60+
}),
61+
}
62+
63+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
64+
65+
// Import after mocks are set up
66+
const { getShell } = await import("../shell")
67+
68+
const result = getShell()
69+
expect(result).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
70+
})
71+
72+
it("should handle array path from VSCode terminal profile on Windows", async () => {
73+
// Set platform to Windows
74+
Object.defineProperty(process, "platform", {
75+
value: "win32",
76+
configurable: true,
77+
})
78+
79+
// Mock VSCode configuration with array path
80+
const mockConfig = {
81+
get: vi.fn((key: string) => {
82+
if (key === "defaultProfile.windows") return "PowerShell"
83+
if (key === "profiles.windows") {
84+
return {
85+
PowerShell: {
86+
// VSCode API may return path as an array
87+
path: ["C:\\Program Files\\PowerShell\\7\\pwsh.exe", "pwsh.exe"],
88+
},
89+
}
90+
}
91+
return undefined
92+
}),
93+
}
94+
95+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
96+
97+
// Import after mocks are set up
98+
const { getShell } = await import("../shell")
99+
100+
const result = getShell()
101+
// Should use the first element of the array
102+
expect(result).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
103+
})
104+
105+
it("should handle empty array path and fall back to defaults", async () => {
106+
// Set platform to Windows
107+
Object.defineProperty(process, "platform", {
108+
value: "win32",
109+
configurable: true,
110+
})
111+
112+
// Mock VSCode configuration with empty array path
113+
const mockConfig = {
114+
get: vi.fn((key: string) => {
115+
if (key === "defaultProfile.windows") return "Custom"
116+
if (key === "profiles.windows") {
117+
return {
118+
Custom: {
119+
path: [], // Empty array
120+
},
121+
}
122+
}
123+
return undefined
124+
}),
125+
}
126+
127+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
128+
129+
// Mock environment variable
130+
process.env = { ...originalEnv, COMSPEC: "C:\\Windows\\System32\\cmd.exe" }
131+
132+
// Import after mocks are set up
133+
const { getShell } = await import("../shell")
134+
135+
const result = getShell()
136+
// Should fall back to cmd.exe
137+
expect(result).toBe("C:\\Windows\\System32\\cmd.exe")
138+
})
139+
140+
it("should handle string path on macOS", async () => {
141+
// Set platform to macOS
142+
Object.defineProperty(process, "platform", {
143+
value: "darwin",
144+
configurable: true,
145+
})
146+
147+
// Mock VSCode configuration
148+
const mockConfig = {
149+
get: vi.fn((key: string) => {
150+
if (key === "defaultProfile.osx") return "zsh"
151+
if (key === "profiles.osx") {
152+
return {
153+
zsh: {
154+
path: "/bin/zsh",
155+
},
156+
}
157+
}
158+
return undefined
159+
}),
160+
}
161+
162+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
163+
164+
// Import after mocks are set up
165+
const { getShell } = await import("../shell")
166+
167+
const result = getShell()
168+
expect(result).toBe("/bin/zsh")
169+
})
170+
171+
it("should handle array path on macOS", async () => {
172+
// Set platform to macOS
173+
Object.defineProperty(process, "platform", {
174+
value: "darwin",
175+
configurable: true,
176+
})
177+
178+
// Mock VSCode configuration with array path
179+
const mockConfig = {
180+
get: vi.fn((key: string) => {
181+
if (key === "defaultProfile.osx") return "zsh"
182+
if (key === "profiles.osx") {
183+
return {
184+
zsh: {
185+
path: ["/opt/homebrew/bin/zsh", "/bin/zsh"],
186+
},
187+
}
188+
}
189+
return undefined
190+
}),
191+
}
192+
193+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
194+
195+
// Import after mocks are set up
196+
const { getShell } = await import("../shell")
197+
198+
const result = getShell()
199+
// Should use the first element of the array
200+
expect(result).toBe("/opt/homebrew/bin/zsh")
201+
})
202+
203+
it("should handle string path on Linux", async () => {
204+
// Set platform to Linux
205+
Object.defineProperty(process, "platform", {
206+
value: "linux",
207+
configurable: true,
208+
})
209+
210+
// Mock VSCode configuration
211+
const mockConfig = {
212+
get: vi.fn((key: string) => {
213+
if (key === "defaultProfile.linux") return "bash"
214+
if (key === "profiles.linux") {
215+
return {
216+
bash: {
217+
path: "/bin/bash",
218+
},
219+
}
220+
}
221+
return undefined
222+
}),
223+
}
224+
225+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
226+
227+
// Import after mocks are set up
228+
const { getShell } = await import("../shell")
229+
230+
const result = getShell()
231+
expect(result).toBe("/bin/bash")
232+
})
233+
234+
it("should handle array path on Linux", async () => {
235+
// Set platform to Linux
236+
Object.defineProperty(process, "platform", {
237+
value: "linux",
238+
configurable: true,
239+
})
240+
241+
// Mock VSCode configuration with array path
242+
const mockConfig = {
243+
get: vi.fn((key: string) => {
244+
if (key === "defaultProfile.linux") return "bash"
245+
if (key === "profiles.linux") {
246+
return {
247+
bash: {
248+
path: ["/usr/local/bin/bash", "/bin/bash"],
249+
},
250+
}
251+
}
252+
return undefined
253+
}),
254+
}
255+
256+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
257+
258+
// Import after mocks are set up
259+
const { getShell } = await import("../shell")
260+
261+
const result = getShell()
262+
// Should use the first element of the array
263+
expect(result).toBe("/usr/local/bin/bash")
264+
})
265+
})
266+
267+
describe("isShellAllowed", () => {
268+
it("should validate string shell paths", async () => {
269+
// Import the module to get access to internal functions
270+
const shellModule = await import("../shell")
271+
272+
// Access the isShellAllowed function through module internals
273+
// Since it's not exported, we need to test it indirectly through getShell
274+
// or make it exported for testing
275+
276+
// For now, we'll test the behavior through getShell
277+
Object.defineProperty(process, "platform", {
278+
value: "win32",
279+
configurable: true,
280+
})
281+
282+
const mockConfig = {
283+
get: vi.fn((key: string) => {
284+
if (key === "defaultProfile.windows") return "PowerShell"
285+
if (key === "profiles.windows") {
286+
return {
287+
PowerShell: {
288+
path: "C:\\Program Files\\PowerShell\\7\\pwsh.exe",
289+
},
290+
}
291+
}
292+
return undefined
293+
}),
294+
}
295+
296+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
297+
298+
const result = shellModule.getShell()
299+
// Should return the allowed shell
300+
expect(result).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
301+
})
302+
303+
it("should validate array shell paths", async () => {
304+
const shellModule = await import("../shell")
305+
306+
Object.defineProperty(process, "platform", {
307+
value: "win32",
308+
configurable: true,
309+
})
310+
311+
const mockConfig = {
312+
get: vi.fn((key: string) => {
313+
if (key === "defaultProfile.windows") return "PowerShell"
314+
if (key === "profiles.windows") {
315+
return {
316+
PowerShell: {
317+
path: ["C:\\Program Files\\PowerShell\\7\\pwsh.exe", "pwsh"],
318+
},
319+
}
320+
}
321+
return undefined
322+
}),
323+
}
324+
325+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
326+
327+
const result = shellModule.getShell()
328+
// Should return the first allowed shell from the array
329+
expect(result).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
330+
})
331+
332+
it("should reject non-allowed shell paths", async () => {
333+
const shellModule = await import("../shell")
334+
335+
Object.defineProperty(process, "platform", {
336+
value: "win32",
337+
configurable: true,
338+
})
339+
340+
const mockConfig = {
341+
get: vi.fn((key: string) => {
342+
if (key === "defaultProfile.windows") return "Malicious"
343+
if (key === "profiles.windows") {
344+
return {
345+
Malicious: {
346+
path: "C:\\malicious\\shell.exe",
347+
},
348+
}
349+
}
350+
return undefined
351+
}),
352+
}
353+
354+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
355+
356+
// Mock environment to provide a fallback
357+
process.env = { ...originalEnv, COMSPEC: "C:\\Windows\\System32\\cmd.exe" }
358+
359+
const result = shellModule.getShell()
360+
// Should fall back to safe default (cmd.exe)
361+
expect(result).toBe("C:\\Windows\\System32\\cmd.exe")
362+
})
363+
})
364+
})

0 commit comments

Comments
 (0)