Skip to content

Commit 5d2b361

Browse files
roomote[bot]roomotejr
authored andcommitted
fix: handle array paths from VSCode terminal profiles (RooCodeInc#7697)
* 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 RooCodeInc#7695 * feat: add validateShellPath export for robust shell validation - Created validateShellPath as a public API for shell path validation - Refactored internal validation logic into isShellAllowedInternal - Added comprehensive test coverage for all edge cases - Maintains backward compatibility with deprecated isShellAllowed - Handles arrays, strings, null, undefined, and nested arrays gracefully * Simplify roomote's work a little --------- Co-authored-by: Roo Code <[email protected]> Co-authored-by: John Richmond <[email protected]>
1 parent 9a7f841 commit 5d2b361

File tree

2 files changed

+178
-9
lines changed

2 files changed

+178
-9
lines changed

src/utils/__tests__/shell.spec.ts

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
12
import * as vscode from "vscode"
23
import { userInfo } from "os"
34
import { getShell } from "../shell"
45

6+
// Mock vscode module
7+
vi.mock("vscode", () => ({
8+
workspace: {
9+
getConfiguration: vi.fn(),
10+
},
11+
}))
12+
513
// Mock the os module
614
vi.mock("os", () => ({
715
userInfo: vi.fn(() => ({ shell: null })),
@@ -74,6 +82,56 @@ describe("Shell Detection Tests", () => {
7482
expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
7583
})
7684

85+
it("should handle array path from VSCode terminal profile", () => {
86+
// Mock VSCode configuration with array path
87+
const mockConfig = {
88+
get: vi.fn((key: string) => {
89+
if (key === "defaultProfile.windows") return "PowerShell"
90+
if (key === "profiles.windows") {
91+
return {
92+
PowerShell: {
93+
// VSCode API may return path as an array
94+
path: ["C:\\Program Files\\PowerShell\\7\\pwsh.exe", "pwsh.exe"],
95+
},
96+
}
97+
}
98+
return undefined
99+
}),
100+
}
101+
102+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
103+
104+
const result = getShell()
105+
// Should use the first element of the array
106+
expect(result).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
107+
})
108+
109+
it("should handle empty array path and fall back to defaults", () => {
110+
// Mock VSCode configuration with empty array path
111+
const mockConfig = {
112+
get: vi.fn((key: string) => {
113+
if (key === "defaultProfile.windows") return "Custom"
114+
if (key === "profiles.windows") {
115+
return {
116+
Custom: {
117+
path: [], // Empty array
118+
},
119+
}
120+
}
121+
return undefined
122+
}),
123+
}
124+
125+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
126+
127+
// Mock environment variable
128+
process.env.COMSPEC = "C:\\Windows\\System32\\cmd.exe"
129+
130+
const result = getShell()
131+
// Should fall back to cmd.exe
132+
expect(result).toBe("C:\\Windows\\System32\\cmd.exe")
133+
})
134+
77135
it("uses PowerShell 7 path if source is 'PowerShell' but no explicit path", () => {
78136
mockVsCodeConfig("windows", "PowerShell", {
79137
PowerShell: { source: "PowerShell" },
@@ -152,6 +210,29 @@ describe("Shell Detection Tests", () => {
152210
expect(getShell()).toBe("/usr/local/bin/fish")
153211
})
154212

213+
it("should handle array path from VSCode terminal profile", () => {
214+
// Mock VSCode configuration with array path
215+
const mockConfig = {
216+
get: vi.fn((key: string) => {
217+
if (key === "defaultProfile.osx") return "zsh"
218+
if (key === "profiles.osx") {
219+
return {
220+
zsh: {
221+
path: ["/opt/homebrew/bin/zsh", "/bin/zsh"],
222+
},
223+
}
224+
}
225+
return undefined
226+
}),
227+
}
228+
229+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
230+
231+
const result = getShell()
232+
// Should use the first element of the array
233+
expect(result).toBe("/opt/homebrew/bin/zsh")
234+
})
235+
155236
it("falls back to userInfo().shell if no VS Code config is available", () => {
156237
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
157238
vi.mocked(userInfo).mockReturnValue({ shell: "/opt/homebrew/bin/zsh" } as any)
@@ -185,6 +266,29 @@ describe("Shell Detection Tests", () => {
185266
expect(getShell()).toBe("/usr/bin/fish")
186267
})
187268

269+
it("should handle array path from VSCode terminal profile", () => {
270+
// Mock VSCode configuration with array path
271+
const mockConfig = {
272+
get: vi.fn((key: string) => {
273+
if (key === "defaultProfile.linux") return "bash"
274+
if (key === "profiles.linux") {
275+
return {
276+
bash: {
277+
path: ["/usr/local/bin/bash", "/bin/bash"],
278+
},
279+
}
280+
}
281+
return undefined
282+
}),
283+
}
284+
285+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
286+
287+
const result = getShell()
288+
// Should use the first element of the array
289+
expect(result).toBe("/usr/local/bin/bash")
290+
})
291+
188292
it("falls back to userInfo().shell if no VS Code config is available", () => {
189293
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
190294
vi.mocked(userInfo).mockReturnValue({ shell: "/usr/bin/zsh" } as any)
@@ -281,6 +385,57 @@ describe("Shell Detection Tests", () => {
281385
expect(getShell()).toBe("/bin/bash")
282386
})
283387

388+
it("should validate array shell paths and use first allowed", () => {
389+
Object.defineProperty(process, "platform", { value: "win32" })
390+
391+
const mockConfig = {
392+
get: vi.fn((key: string) => {
393+
if (key === "defaultProfile.windows") return "PowerShell"
394+
if (key === "profiles.windows") {
395+
return {
396+
PowerShell: {
397+
path: ["C:\\Program Files\\PowerShell\\7\\pwsh.exe", "pwsh"],
398+
},
399+
}
400+
}
401+
return undefined
402+
}),
403+
}
404+
405+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
406+
407+
const result = getShell()
408+
// Should return the first allowed shell from the array
409+
expect(result).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
410+
})
411+
412+
it("should reject non-allowed shell paths and fall back to safe defaults", () => {
413+
Object.defineProperty(process, "platform", { value: "win32" })
414+
415+
const mockConfig = {
416+
get: vi.fn((key: string) => {
417+
if (key === "defaultProfile.windows") return "Malicious"
418+
if (key === "profiles.windows") {
419+
return {
420+
Malicious: {
421+
path: "C:\\malicious\\shell.exe",
422+
},
423+
}
424+
}
425+
return undefined
426+
}),
427+
}
428+
429+
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(mockConfig as any)
430+
431+
// Mock environment to provide a fallback
432+
process.env.COMSPEC = "C:\\Windows\\System32\\cmd.exe"
433+
434+
const result = getShell()
435+
// Should fall back to safe default (cmd.exe)
436+
expect(result).toBe("C:\\Windows\\System32\\cmd.exe")
437+
})
438+
284439
it("should validate shells from VS Code config", () => {
285440
Object.defineProperty(process, "platform", { value: "darwin" })
286441
mockVsCodeConfig("osx", "MyCustomShell", {

src/utils/shell.ts

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,20 +113,20 @@ const SHELL_PATHS = {
113113
} as const
114114

115115
interface MacTerminalProfile {
116-
path?: string
116+
path?: string | string[]
117117
}
118118

119119
type MacTerminalProfiles = Record<string, MacTerminalProfile>
120120

121121
interface WindowsTerminalProfile {
122-
path?: string
122+
path?: string | string[]
123123
source?: "PowerShell" | "WSL"
124124
}
125125

126126
type WindowsTerminalProfiles = Record<string, WindowsTerminalProfile>
127127

128128
interface LinuxTerminalProfile {
129-
path?: string
129+
path?: string | string[]
130130
}
131131

132132
type LinuxTerminalProfiles = Record<string, LinuxTerminalProfile>
@@ -172,6 +172,18 @@ function getLinuxTerminalConfig() {
172172
// 2) Platform-Specific VS Code Shell Retrieval
173173
// -----------------------------------------------------
174174

175+
/**
176+
* Normalizes a path that can be either a string or an array of strings.
177+
* If it's an array, returns the first element. Otherwise returns the string.
178+
*/
179+
function normalizeShellPath(path: string | string[] | undefined): string | null {
180+
if (!path) return null
181+
if (Array.isArray(path)) {
182+
return path.length > 0 ? path[0] : null
183+
}
184+
return path
185+
}
186+
175187
/** Attempts to retrieve a shell path from VS Code config on Windows. */
176188
function getWindowsShellFromVSCode(): string | null {
177189
const { defaultProfileName, profiles } = getWindowsTerminalConfig()
@@ -185,9 +197,10 @@ function getWindowsShellFromVSCode(): string | null {
185197
// In testing it was found these typically do not have a path, and this
186198
// implementation manages to deductively get the correct version of PowerShell
187199
if (defaultProfileName.toLowerCase().includes("powershell")) {
188-
if (profile?.path) {
200+
const normalizedPath = normalizeShellPath(profile?.path)
201+
if (normalizedPath) {
189202
// If there's an explicit PowerShell path, return that
190-
return profile.path
203+
return normalizedPath
191204
} else if (profile?.source === "PowerShell") {
192205
// If the profile is sourced from PowerShell, assume the newest
193206
return SHELL_PATHS.POWERSHELL_7
@@ -197,8 +210,9 @@ function getWindowsShellFromVSCode(): string | null {
197210
}
198211

199212
// If there's a specific path, return that immediately
200-
if (profile?.path) {
201-
return profile.path
213+
const normalizedPath = normalizeShellPath(profile?.path)
214+
if (normalizedPath) {
215+
return normalizedPath
202216
}
203217

204218
// If the profile indicates WSL
@@ -218,7 +232,7 @@ function getMacShellFromVSCode(): string | null {
218232
}
219233

220234
const profile = profiles[defaultProfileName]
221-
return profile?.path || null
235+
return normalizeShellPath(profile?.path)
222236
}
223237

224238
/** Attempts to retrieve a shell path from VS Code config on Linux. */
@@ -229,7 +243,7 @@ function getLinuxShellFromVSCode(): string | null {
229243
}
230244

231245
const profile = profiles[defaultProfileName]
232-
return profile?.path || null
246+
return normalizeShellPath(profile?.path)
233247
}
234248

235249
// -----------------------------------------------------

0 commit comments

Comments
 (0)