Skip to content

Commit 74c7ea8

Browse files
committed
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
1 parent c659190 commit 74c7ea8

File tree

2 files changed

+189
-8
lines changed

2 files changed

+189
-8
lines changed

src/utils/__tests__/shell.test.ts

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,4 +361,132 @@ describe("shell utilities", () => {
361361
expect(result).toBe("C:\\Windows\\System32\\cmd.exe")
362362
})
363363
})
364+
365+
describe("validateShellPath", () => {
366+
it("should validate string shell paths", async () => {
367+
const { validateShellPath } = await import("../shell")
368+
369+
// Valid shells
370+
expect(validateShellPath("/bin/bash")).toBe(true)
371+
expect(validateShellPath("/bin/zsh")).toBe(true)
372+
expect(validateShellPath("C:\\Windows\\System32\\cmd.exe")).toBe(true)
373+
expect(validateShellPath("C:\\Program Files\\PowerShell\\7\\pwsh.exe")).toBe(true)
374+
375+
// Invalid shells
376+
expect(validateShellPath("/usr/bin/malicious")).toBe(false)
377+
expect(validateShellPath("C:\\malicious\\shell.exe")).toBe(false)
378+
})
379+
380+
it("should validate array shell paths using first element", async () => {
381+
const { validateShellPath } = await import("../shell")
382+
383+
// Valid array with allowed first element
384+
expect(validateShellPath(["/bin/bash", "/bin/sh"])).toBe(true)
385+
expect(validateShellPath(["C:\\Windows\\System32\\cmd.exe", "cmd"])).toBe(true)
386+
387+
// Invalid array with disallowed first element
388+
expect(validateShellPath(["/usr/bin/malicious", "/bin/bash"])).toBe(false)
389+
expect(validateShellPath(["C:\\malicious\\shell.exe", "C:\\Windows\\System32\\cmd.exe"])).toBe(false)
390+
})
391+
392+
it("should handle empty arrays", async () => {
393+
const { validateShellPath } = await import("../shell")
394+
395+
expect(validateShellPath([])).toBe(false)
396+
})
397+
398+
it("should handle null and undefined", async () => {
399+
const { validateShellPath } = await import("../shell")
400+
401+
expect(validateShellPath(null)).toBe(false)
402+
expect(validateShellPath(undefined)).toBe(false)
403+
})
404+
405+
it("should handle nested arrays (edge case)", async () => {
406+
const { validateShellPath } = await import("../shell")
407+
408+
// Nested array - should recursively check first element
409+
expect(validateShellPath([["/bin/bash"]] as any)).toBe(true)
410+
expect(validateShellPath([["/usr/bin/malicious"]] as any)).toBe(false)
411+
expect(validateShellPath([["C:\\Windows\\System32\\cmd.exe"]] as any)).toBe(true)
412+
})
413+
414+
it("should handle empty strings", async () => {
415+
const { validateShellPath } = await import("../shell")
416+
417+
expect(validateShellPath("")).toBe(false)
418+
expect(validateShellPath([""])).toBe(false)
419+
})
420+
421+
it("should handle case-insensitive comparison on Windows", async () => {
422+
// Set platform to Windows
423+
Object.defineProperty(process, "platform", {
424+
value: "win32",
425+
configurable: true,
426+
})
427+
428+
const { validateShellPath } = await import("../shell")
429+
430+
// Different case variations should all be valid
431+
expect(validateShellPath("c:\\windows\\system32\\cmd.exe")).toBe(true)
432+
expect(validateShellPath("C:\\WINDOWS\\SYSTEM32\\CMD.EXE")).toBe(true)
433+
expect(validateShellPath("C:\\Windows\\System32\\CMD.exe")).toBe(true)
434+
})
435+
436+
it("should handle case-sensitive comparison on Unix", async () => {
437+
// Set platform to Linux
438+
Object.defineProperty(process, "platform", {
439+
value: "linux",
440+
configurable: true,
441+
})
442+
443+
const { validateShellPath } = await import("../shell")
444+
445+
// Exact case match required
446+
expect(validateShellPath("/bin/bash")).toBe(true)
447+
expect(validateShellPath("/BIN/BASH")).toBe(false)
448+
expect(validateShellPath("/Bin/Bash")).toBe(false)
449+
})
450+
451+
it("should normalize paths before validation", async () => {
452+
const { validateShellPath } = await import("../shell")
453+
454+
// Paths with extra slashes or dots should be normalized
455+
expect(validateShellPath("/bin//bash")).toBe(true)
456+
expect(validateShellPath("/bin/./bash")).toBe(true)
457+
458+
// Windows paths with backslashes
459+
if (process.platform === "win32") {
460+
expect(validateShellPath("C:/Windows/System32/cmd.exe")).toBe(true)
461+
}
462+
})
463+
464+
it("should handle arrays with mixed valid and invalid paths", async () => {
465+
const { validateShellPath } = await import("../shell")
466+
467+
// Only the first element matters
468+
expect(validateShellPath(["/bin/bash", "/usr/bin/malicious", "/bin/sh"])).toBe(true)
469+
expect(validateShellPath(["/usr/bin/malicious", "/bin/bash", "/bin/sh"])).toBe(false)
470+
})
471+
472+
it("should reject non-string, non-array types", async () => {
473+
const { validateShellPath } = await import("../shell")
474+
475+
// Numbers, objects, etc. should be rejected
476+
expect(validateShellPath(123 as any)).toBe(false)
477+
expect(validateShellPath({ path: "/bin/bash" } as any)).toBe(false)
478+
expect(validateShellPath(true as any)).toBe(false)
479+
})
480+
481+
it("should handle arrays containing non-string elements", async () => {
482+
const { validateShellPath } = await import("../shell")
483+
484+
// Array with null/undefined first element
485+
expect(validateShellPath([null, "/bin/bash"] as any)).toBe(false)
486+
expect(validateShellPath([undefined, "/bin/bash"] as any)).toBe(false)
487+
488+
// Array with number first element
489+
expect(validateShellPath([123, "/bin/bash"] as any)).toBe(false)
490+
})
491+
})
364492
})

src/utils/shell.ts

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -289,16 +289,13 @@ function getShellFromEnv(): string | null {
289289
// -----------------------------------------------------
290290

291291
/**
292-
* Validates if a shell path is in the allowlist to prevent arbitrary command execution.
293-
* Can handle both string and array inputs (arrays from VSCode API).
292+
* Internal validation function that checks if a shell path is in the allowlist.
293+
* This is the core validation logic that operates on normalized string paths.
294294
*/
295-
function isShellAllowed(shellPath: string | string[]): boolean {
296-
// Handle array input by checking the first element
297-
const pathToCheck = Array.isArray(shellPath) ? (shellPath.length > 0 ? shellPath[0] : null) : shellPath
298-
299-
if (!pathToCheck) return false
295+
function isShellAllowedInternal(shellPath: string): boolean {
296+
if (!shellPath) return false
300297

301-
const normalizedPath = path.normalize(pathToCheck)
298+
const normalizedPath = path.normalize(shellPath)
302299

303300
// Direct lookup first
304301
if (SHELL_ALLOWLIST.has(normalizedPath)) {
@@ -318,6 +315,62 @@ function isShellAllowed(shellPath: string | string[]): boolean {
318315
return false
319316
}
320317

318+
/**
319+
* Proxy function that validates shell paths, handling both string and array inputs.
320+
* This function serves as a robust interface for shell path validation that can
321+
* handle various input types from VSCode API and other sources.
322+
*
323+
* @param shellPath - The shell path to validate. Can be:
324+
* - A string path to a shell executable
325+
* - An array of string paths (VSCode may return this)
326+
* - undefined or null
327+
* @returns true if the shell path is allowed, false otherwise
328+
*
329+
* @example
330+
* // String input
331+
* validateShellPath("/bin/bash") // returns true
332+
*
333+
* @example
334+
* // Array input (from VSCode API)
335+
* validateShellPath(["/usr/local/bin/bash", "/bin/bash"]) // returns true if first is allowed
336+
*
337+
* @example
338+
* // Empty or invalid input
339+
* validateShellPath([]) // returns false
340+
* validateShellPath(null) // returns false
341+
*/
342+
export function validateShellPath(shellPath: string | string[] | undefined | null): boolean {
343+
// Handle null/undefined
344+
if (!shellPath) {
345+
return false
346+
}
347+
348+
// Handle array input - validate the first element
349+
if (Array.isArray(shellPath)) {
350+
if (shellPath.length === 0) {
351+
return false
352+
}
353+
// Recursively validate the first element (in case of nested arrays)
354+
return validateShellPath(shellPath[0])
355+
}
356+
357+
// Handle string input
358+
if (typeof shellPath === "string") {
359+
return isShellAllowedInternal(shellPath)
360+
}
361+
362+
// Unknown type - reject for safety
363+
return false
364+
}
365+
366+
/**
367+
* Legacy function for backward compatibility.
368+
* @deprecated Use validateShellPath instead
369+
*/
370+
function isShellAllowed(shellPath: string | string[]): boolean {
371+
return validateShellPath(shellPath)
372+
}
373+
321374
/**
322375
* Returns a safe fallback shell based on the platform
323376
*/

0 commit comments

Comments
 (0)