diff --git a/apps/frontend/src/__tests__/setup.ts b/apps/frontend/src/__tests__/setup.ts index dc2c99dd91..27643a4800 100644 --- a/apps/frontend/src/__tests__/setup.ts +++ b/apps/frontend/src/__tests__/setup.ts @@ -36,6 +36,17 @@ if (typeof HTMLElement !== 'undefined' && !HTMLElement.prototype.scrollIntoView) }); } +// Mock requestAnimationFrame/cancelAnimationFrame for jsdom +// Required by useXterm.ts which uses requestAnimationFrame for initial fit +if (typeof global.requestAnimationFrame === 'undefined') { + global.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => { + return setTimeout(() => callback(Date.now()), 0) as unknown as number; + }); + global.cancelAnimationFrame = vi.fn((id: number) => { + clearTimeout(id); + }); +} + // Test data directory for isolated file operations export const TEST_DATA_DIR = '/tmp/auto-claude-ui-tests'; diff --git a/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts b/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts index b39c588a6d..5222690779 100644 --- a/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts +++ b/apps/frontend/src/main/__tests__/cli-tool-manager.test.ts @@ -7,14 +7,19 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { existsSync, readdirSync } from 'fs'; import os from 'os'; import { execFileSync } from 'child_process'; -import { app } from 'electron'; import { getToolInfo, + getToolPathAsync, clearToolCache, getClaudeDetectionPaths, sortNvmVersionDirs, buildClaudeDetectionResult } from '../cli-tool-manager'; +import { + findWindowsExecutableViaWhere, + findWindowsExecutableViaWhereAsync +} from '../utils/windows-paths'; +import { findExecutable, findExecutableAsync } from '../env-utils'; // Mock Electron app vi.mock('electron', () => ({ @@ -32,32 +37,79 @@ vi.mock('os', () => ({ })); // Mock fs module - need to mock both sync and promises -vi.mock('fs', () => { - const mockDirent = ( - name: string, - isDir: boolean - ): { name: string; isDirectory: () => boolean } => ({ - name, - isDirectory: () => isDir +vi.mock('fs', () => ({ + existsSync: vi.fn(), + readdirSync: vi.fn(), + promises: {} +})); + +// Mock child_process for execFileSync, execFile, execSync, and exec (used in validation) +// execFile and exec need to be promisify-compatible +// IMPORTANT: execSync and execFileSync share the same mock so tests that set one will affect both +// This is because validateClaude() uses execSync for .cmd files and execFileSync for others +vi.mock('child_process', () => { + // Shared mock for sync execution - both execFileSync and execSync use this + // so when tests call vi.mocked(execFileSync).mockReturnValue(), it affects execSync too + const sharedSyncMock = vi.fn(); + + const mockExecFile = vi.fn((cmd: any, args: any, options: any, callback: any) => { + // Return a minimal ChildProcess-like object + const childProcess = { + stdout: { on: vi.fn() }, + stderr: { on: vi.fn() }, + on: vi.fn() + }; + + // If callback is provided, call it asynchronously + if (typeof callback === 'function') { + setImmediate(() => callback(null, 'claude-code version 1.0.0\n', '')); + } + + return childProcess as any; + }); + + const mockExec = vi.fn((cmd: any, options: any, callback: any) => { + // Return a minimal ChildProcess-like object + const childProcess = { + stdout: { on: vi.fn() }, + stderr: { on: vi.fn() }, + on: vi.fn() + }; + + // If callback is provided, call it asynchronously + if (typeof callback === 'function') { + setImmediate(() => callback(null, 'claude-code version 1.0.0\n', '')); + } + + return childProcess as any; }); return { - existsSync: vi.fn(), - readdirSync: vi.fn(), - promises: {} + execFileSync: sharedSyncMock, + execFile: mockExecFile, + execSync: sharedSyncMock, // Share with execFileSync so tests work for both + exec: mockExec }; }); -// Mock child_process for execFileSync and execFile (used in validation) -vi.mock('child_process', () => ({ - execFileSync: vi.fn(), - execFile: vi.fn() -})); - // Mock env-utils to avoid PATH augmentation complexity vi.mock('../env-utils', () => ({ findExecutable: vi.fn(() => null), // Return null to force platform-specific path checking - getAugmentedEnv: vi.fn(() => ({ PATH: '' })) + findExecutableAsync: vi.fn(() => Promise.resolve(null)), + getAugmentedEnv: vi.fn(() => ({ PATH: '' })), + getAugmentedEnvAsync: vi.fn(() => Promise.resolve({ PATH: '' })), + shouldUseShell: vi.fn((command: string) => { + // Mock shouldUseShell to match actual behavior + if (process.platform !== 'win32') { + return false; + } + return /\.(cmd|bat)$/i.test(command); + }), + getSpawnOptions: vi.fn((command: string, baseOptions?: any) => ({ + ...baseOptions, + shell: /\.(cmd|bat)$/i.test(command) && process.platform === 'win32' + })), + existsAsync: vi.fn(() => Promise.resolve(false)) })); // Mock homebrew-python utility @@ -65,6 +117,12 @@ vi.mock('../utils/homebrew-python', () => ({ findHomebrewPython: vi.fn(() => null) })); +// Mock windows-paths utility +vi.mock('../utils/windows-paths', () => ({ + findWindowsExecutableViaWhere: vi.fn(() => null), + findWindowsExecutableViaWhereAsync: vi.fn(() => Promise.resolve(null)) +})); + describe('cli-tool-manager - Claude CLI NVM detection', () => { beforeEach(() => { vi.clearAllMocks(); @@ -90,153 +148,97 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); // NVM versions directory exists - if (pathStr.includes('.nvm/versions/node')) { + if (pathStr.includes('.nvm/versions/node') || pathStr.includes('.nvm\\versions\\node')) { return true; } // Claude CLI exists in v22.17.0 - if (pathStr.includes('v22.17.0/bin/claude')) { + if (pathStr.includes('v22.17.0/bin/claude') || pathStr.includes('v22.17.0\\bin\\claude')) { return true; } return false; }); - // Mock readdirSync to return Node version directories - vi.mocked(readdirSync).mockImplementation((filePath, options) => { - const pathStr = String(filePath); - if (pathStr.includes('.nvm/versions/node')) { - return [ - { name: 'v20.11.0', isDirectory: () => true }, - { name: 'v22.17.0', isDirectory: () => true } - ] as any; - } - return [] as any; - }); - - // Mock execFileSync to return version for validation - vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); - - const result = getToolInfo('claude'); - - expect(result.found).toBe(true); - expect(result.path).toContain('v22.17.0'); - expect(result.path).toContain('bin/claude'); - expect(result.source).toBe('nvm'); - }); - - it('should try multiple NVM Node versions until finding Claude CLI', () => { - vi.mocked(os.homedir).mockReturnValue(mockHomeDir); - - vi.mocked(existsSync).mockImplementation((filePath) => { - const pathStr = String(filePath); - if (pathStr.includes('.nvm/versions/node')) { - return true; - } - // Only v24.12.0 has Claude CLI - if (pathStr.includes('v24.12.0/bin/claude')) { - return true; - } - return false; - }); - - vi.mocked(readdirSync).mockImplementation((filePath) => { - const pathStr = String(filePath); - if (pathStr.includes('.nvm/versions/node')) { - return [ - { name: 'v18.20.0', isDirectory: () => true }, - { name: 'v20.11.0', isDirectory: () => true }, - { name: 'v24.12.0', isDirectory: () => true } - ] as any; - } - return [] as any; - }); + // Mock Node.js version directories (three versions) + const mockDirents = [ + { name: 'v20.0.0', isDirectory: () => true }, + { name: 'v22.17.0', isDirectory: () => true }, + { name: 'v18.20.0', isDirectory: () => true }, + ]; + vi.mocked(readdirSync).mockReturnValue(mockDirents as any); + // Mock execFileSync to simulate successful version check vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); const result = getToolInfo('claude'); expect(result.found).toBe(true); - expect(result.path).toContain('v24.12.0'); + // Path should contain version and claude (works with both / and \ separators) + expect(result.path).toMatch(/v22\.17\.0[/\\]bin[/\\]claude/); + expect(result.version).toBe('1.0.0'); expect(result.source).toBe('nvm'); + expect(result.message).toContain('Using NVM Claude CLI'); }); - it('should skip non-version directories in NVM (e.g., does not start with "v")', () => { - vi.mocked(os.homedir).mockReturnValue(mockHomeDir); - - vi.mocked(existsSync).mockImplementation((filePath) => { - const pathStr = String(filePath); - if (pathStr.includes('.nvm/versions/node')) { - return true; - } - // Only the correctly named version has Claude - if (pathStr.includes('v22.17.0/bin/claude')) { - return true; - } - return false; - }); - - vi.mocked(readdirSync).mockImplementation((filePath) => { - const pathStr = String(filePath); - if (pathStr.includes('.nvm/versions/node')) { - return [ - { name: 'current', isDirectory: () => true }, // Should be skipped - { name: 'system', isDirectory: () => true }, // Should be skipped - { name: 'v22.17.0', isDirectory: () => true } // Should be checked - ] as any; - } - return [] as any; - }); - - vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); - - const result = getToolInfo('claude'); - - expect(result.found).toBe(true); - expect(result.path).toContain('v22.17.0'); - }); - - it('should not check NVM paths on Windows', () => { + it('should skip NVM path detection on Windows', () => { + // Set platform to Windows Object.defineProperty(process, 'platform', { value: 'win32', writable: true }); vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); - - // Even if NVM directory exists on Windows, should not check it vi.mocked(existsSync).mockReturnValue(false); vi.mocked(readdirSync).mockReturnValue([]); const result = getToolInfo('claude'); - // Should not be found from NVM on Windows - expect(result.source).not.toBe('nvm'); + // readdirSync should not be called for NVM on Windows + expect(readdirSync).not.toHaveBeenCalled(); + expect(result.source).toBe('fallback'); // Should fallback since no other paths exist }); it('should handle missing NVM directory gracefully', () => { vi.mocked(os.homedir).mockReturnValue(mockHomeDir); - // NVM directory does not exist + // NVM directory doesn't exist vi.mocked(existsSync).mockReturnValue(false); const result = getToolInfo('claude'); - // Should not find via NVM - expect(result.source).not.toBe('nvm'); + // Should not crash, should continue to platform paths + expect(result).toBeDefined(); expect(result.found).toBe(false); }); - it('should handle readdirSync errors gracefully', () => { + it('should try next version if Claude not found in newest Node version', () => { vi.mocked(os.homedir).mockReturnValue(mockHomeDir); - vi.mocked(existsSync).mockReturnValue(true); - vi.mocked(readdirSync).mockImplementation(() => { - throw new Error('Permission denied'); + // NVM directory exists, but Claude only exists in v20.0.0 + vi.mocked(existsSync).mockImplementation((filePath) => { + const pathStr = String(filePath); + // Check for claude binary paths first (more specific) + if (pathStr.includes('claude')) { + // Claude only exists in v20.0.0, not in v22.17.0 + return pathStr.includes('v20.0.0'); + } + // NVM versions directory exists + if (pathStr.includes('.nvm')) { + return true; + } + return false; }); + const mockDirents = [ + { name: 'v22.17.0', isDirectory: () => true }, + { name: 'v20.0.0', isDirectory: () => true }, + ]; + vi.mocked(readdirSync).mockReturnValue(mockDirents as any); + vi.mocked(execFileSync).mockReturnValue('claude-code version 1.5.0\n'); + const result = getToolInfo('claude'); - // Should not crash, should fall back to other detection methods - expect(result.source).not.toBe('nvm'); + expect(result.found).toBe(true); + expect(result.path).toMatch(/v20\.0\.0[/\\]bin[/\\]claude/); }); it('should validate Claude CLI before returning NVM path', () => { @@ -244,80 +246,120 @@ describe('cli-tool-manager - Claude CLI NVM detection', () => { vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); - if (pathStr.includes('.nvm/versions/node')) { - return true; - } - if (pathStr.includes('v22.17.0/bin/claude')) { - return true; + // Check for claude binary paths first + if (pathStr.includes('claude')) { + return pathStr.includes('v22.17.0'); } + // NVM directory exists + if (pathStr.includes('.nvm')) return true; return false; }); - vi.mocked(readdirSync).mockImplementation(() => { - return [{ name: 'v22.17.0', isDirectory: () => true }] as any; - }); + const mockDirents = [ + { name: 'v22.17.0', isDirectory: () => true }, + ]; + vi.mocked(readdirSync).mockReturnValue(mockDirents as any); - // Mock validation failure (execFileSync throws) + // Mock validation failure vi.mocked(execFileSync).mockImplementation(() => { - throw new Error('Command failed'); + throw new Error('Command not found or invalid'); }); const result = getToolInfo('claude'); - // Should not return unvalidated path + // Should not return invalid Claude path, should continue to platform paths expect(result.found).toBe(false); - expect(result.source).not.toBe('nvm'); + expect(result.source).toBe('fallback'); }); - it('should handle NVM directory with no version subdirectories', () => { + it('should use version sorting to prioritize newest Node version', () => { vi.mocked(os.homedir).mockReturnValue(mockHomeDir); vi.mocked(existsSync).mockImplementation((filePath) => { - return String(filePath).includes('.nvm/versions/node'); + const pathStr = String(filePath); + if (pathStr.includes('.nvm/versions/node') || pathStr.includes('.nvm\\versions\\node')) return true; + // Claude exists in all versions + if (pathStr.includes('/bin/claude') || pathStr.includes('\\bin\\claude')) return true; + return false; }); - // Empty NVM directory - vi.mocked(readdirSync).mockReturnValue([]); + // Versions in random order + const mockDirents = [ + { name: 'v18.20.0', isDirectory: () => true }, + { name: 'v22.17.0', isDirectory: () => true }, + { name: 'v20.5.0', isDirectory: () => true }, + ]; + vi.mocked(readdirSync).mockReturnValue(mockDirents as any); + vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); const result = getToolInfo('claude'); - expect(result.source).not.toBe('nvm'); + expect(result.found).toBe(true); + expect(result.path).toContain('v22.17.0'); // Highest version }); }); - describe('NVM on macOS', () => { - it('should detect Claude CLI via NVM on macOS', () => { + describe('Platform-specific path detection', () => { + it('should detect Claude CLI in Windows AppData npm global path', () => { Object.defineProperty(process, 'platform', { - value: 'darwin', + value: 'win32', writable: true }); - vi.mocked(os.homedir).mockReturnValue('/Users/test'); + vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); vi.mocked(existsSync).mockImplementation((filePath) => { const pathStr = String(filePath); - if (pathStr.includes('.nvm/versions/node')) { - return true; - } - if (pathStr.includes('v22.17.0/bin/claude')) { + // Check path components (path.join uses host OS separator) + if (pathStr.includes('AppData') && + pathStr.includes('npm') && + pathStr.includes('claude.cmd')) { return true; } return false; }); - vi.mocked(readdirSync).mockImplementation(() => { - return [{ name: 'v22.17.0', isDirectory: () => true }] as any; + vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + + const result = getToolInfo('claude'); + + expect(result.found).toBe(true); + expect(result.path).toMatch(/AppData[/\\]Roaming[/\\]npm[/\\]claude\.cmd/); + expect(result.source).toBe('system-path'); + }); + + it('should detect Claude CLI in Unix .local/bin path', () => { + vi.mocked(os.homedir).mockReturnValue('/home/user'); + + vi.mocked(existsSync).mockImplementation((filePath) => { + const pathStr = String(filePath); + if (pathStr.includes('.local/bin/claude') || pathStr.includes('.local\\bin\\claude')) { + return true; + } + return false; }); - vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + vi.mocked(execFileSync).mockReturnValue('claude-code version 2.0.0\n'); const result = getToolInfo('claude'); expect(result.found).toBe(true); - expect(result.source).toBe('nvm'); - expect(result.path).toContain('v22.17.0'); + expect(result.path).toMatch(/\.local[/\\]bin[/\\]claude/); + expect(result.version).toBe('2.0.0'); + }); + + it('should return fallback when Claude CLI not found anywhere', () => { + vi.mocked(os.homedir).mockReturnValue('/home/user'); + vi.mocked(existsSync).mockReturnValue(false); + + const result = getToolInfo('claude'); + + expect(result.found).toBe(false); + expect(result.source).toBe('fallback'); + expect(result.message).toContain('Claude CLI not found'); }); }); + }); /** @@ -358,14 +400,18 @@ describe('cli-tool-manager - Helper Functions', () => { const paths = getClaudeDetectionPaths('/home/test'); - expect(paths.platformPaths.some(p => p.includes('.local/bin/claude'))).toBe(true); - expect(paths.platformPaths.some(p => p.includes('bin/claude'))).toBe(true); + // Check for paths containing the expected components (works with both / and \ separators) + expect(paths.platformPaths.some(p => p.includes('.local') && p.includes('bin') && p.includes('claude'))).toBe(true); + expect(paths.platformPaths.some(p => p.includes('bin') && p.includes('claude'))).toBe(true); }); it('should return correct NVM versions directory', () => { const paths = getClaudeDetectionPaths('/home/test'); - expect(paths.nvmVersionsDir).toBe('/home/test/.nvm/versions/node'); + // Check path components exist (works with both / and \ separators) + expect(paths.nvmVersionsDir).toContain('.nvm'); + expect(paths.nvmVersionsDir).toContain('versions'); + expect(paths.nvmVersionsDir).toContain('node'); }); }); @@ -374,54 +420,54 @@ describe('cli-tool-manager - Helper Functions', () => { const entries = [ { name: 'v18.20.0', isDirectory: () => true }, { name: 'v22.17.0', isDirectory: () => true }, - { name: 'v20.11.0', isDirectory: () => true } + { name: 'v20.5.0', isDirectory: () => true }, ]; const sorted = sortNvmVersionDirs(entries); - expect(sorted).toEqual(['v22.17.0', 'v20.11.0', 'v18.20.0']); + expect(sorted).toEqual(['v22.17.0', 'v20.5.0', 'v18.20.0']); }); it('should filter out non-version directories', () => { const entries = [ - { name: 'v20.11.0', isDirectory: () => true }, - { name: '.DS_Store', isDirectory: () => false }, - { name: 'node_modules', isDirectory: () => true }, + { name: 'v20.0.0', isDirectory: () => true }, { name: 'current', isDirectory: () => true }, - { name: 'v22.17.0', isDirectory: () => true } + { name: '.DS_Store', isDirectory: () => false }, + { name: 'system', isDirectory: () => true }, ]; const sorted = sortNvmVersionDirs(entries); - expect(sorted).toEqual(['v22.17.0', 'v20.11.0']); - expect(sorted).not.toContain('.DS_Store'); - expect(sorted).not.toContain('node_modules'); + expect(sorted).toEqual(['v20.0.0']); expect(sorted).not.toContain('current'); + expect(sorted).not.toContain('system'); }); - it('should return empty array when no valid versions', () => { + it('should handle malformed version strings', () => { const entries = [ - { name: 'current', isDirectory: () => true }, - { name: 'system', isDirectory: () => true } + { name: 'v22.17.0', isDirectory: () => true }, + { name: 'v20.abc.1', isDirectory: () => true }, // Invalid version + { name: 'v18.20.0', isDirectory: () => true }, ]; const sorted = sortNvmVersionDirs(entries); - expect(sorted).toEqual([]); + // Should filter out malformed versions + expect(sorted).toContain('v22.17.0'); + expect(sorted).toContain('v18.20.0'); + expect(sorted).not.toContain('v20.abc.1'); }); - it('should handle single entry', () => { - const entries = [{ name: 'v20.11.0', isDirectory: () => true }]; + it('should handle patch version comparison correctly', () => { + const entries = [ + { name: 'v20.0.1', isDirectory: () => true }, + { name: 'v20.0.10', isDirectory: () => true }, + { name: 'v20.0.2', isDirectory: () => true }, + ]; const sorted = sortNvmVersionDirs(entries); - expect(sorted).toEqual(['v20.11.0']); - }); - - it('should handle empty array', () => { - const sorted = sortNvmVersionDirs([]); - - expect(sorted).toEqual([]); + expect(sorted).toEqual(['v20.0.10', 'v20.0.2', 'v20.0.1']); }); }); @@ -429,7 +475,7 @@ describe('cli-tool-manager - Helper Functions', () => { it('should return null when validation fails', () => { const result = buildClaudeDetectionResult( '/path/to/claude', - { valid: false, message: 'Invalid CLI' }, + { valid: false, message: 'Not valid' }, 'nvm', 'Found via NVM' ); @@ -467,3 +513,211 @@ describe('cli-tool-manager - Helper Functions', () => { }); }); }); + +/** + * Unit tests for Claude CLI Windows where.exe detection + */ +describe('cli-tool-manager - Claude CLI Windows where.exe detection', () => { + beforeEach(() => { + vi.clearAllMocks(); + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true + }); + }); + + afterEach(() => { + clearToolCache(); + }); + + it('should detect Claude CLI via where.exe when not in PATH', () => { + vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + + // Mock findExecutable returns null (not in PATH) + vi.mocked(findExecutable).mockReturnValue(null); + + // Mock where.exe finds it in nvm-windows location + vi.mocked(findWindowsExecutableViaWhere).mockReturnValue( + 'D:\\Program Files\\nvm4w\\nodejs\\claude.cmd' + ); + + // Mock file system checks + vi.mocked(existsSync).mockImplementation((filePath) => { + const pathStr = String(filePath); + if (pathStr.includes('nvm4w') && pathStr.includes('claude.cmd')) { + return true; + } + return false; + }); + + // Mock validation success + vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + + const result = getToolInfo('claude'); + + expect(result.found).toBe(true); + expect(result.path).toContain('nvm4w'); + expect(result.path).toContain('claude.cmd'); + expect(result.source).toBe('system-path'); + expect(result.message).toContain('Using Windows Claude CLI'); + }); + + it('should skip where.exe on non-Windows platforms', () => { + Object.defineProperty(process, 'platform', { + value: 'darwin', + writable: true + }); + + vi.mocked(findWindowsExecutableViaWhere).mockReturnValue(null); + + // Mock other detection methods to fail + vi.mocked(existsSync).mockReturnValue(false); + + getToolInfo('claude'); + + // where.exe should not be called on macOS + expect(findWindowsExecutableViaWhere).not.toHaveBeenCalled(); + }); + + it('should validate Claude CLI before returning where.exe path', () => { + vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + + vi.mocked(findExecutable).mockReturnValue(null); + + vi.mocked(findWindowsExecutableViaWhere).mockReturnValue( + 'D:\\Tools\\claude.cmd' + ); + + // Mock file system to return false for all paths except where.exe result + vi.mocked(existsSync).mockImplementation((filePath) => { + const pathStr = String(filePath); + if (pathStr.includes('Tools') && pathStr.includes('claude.cmd')) { + return true; + } + return false; + }); + + // Mock validation failure (executable doesn't respond correctly) + vi.mocked(execFileSync).mockImplementation(() => { + throw new Error('Command failed'); + }); + + const result = getToolInfo('claude'); + + // Should not return the unvalidated path, fallback to not found + expect(result.found).toBe(false); + expect(result.source).toBe('fallback'); + }); + + it('should fallback to platform paths if where.exe fails', () => { + vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + + vi.mocked(findExecutable).mockReturnValue(null); + + vi.mocked(findWindowsExecutableViaWhere).mockReturnValue(null); + + // Mock platform path exists (AppData npm global) + vi.mocked(existsSync).mockImplementation((filePath) => { + const pathStr = String(filePath); + if (pathStr.includes('AppData') && pathStr.includes('npm') && pathStr.includes('claude.cmd')) { + return true; + } + return false; + }); + + vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + + const result = getToolInfo('claude'); + + expect(result.found).toBe(true); + expect(result.path).toContain('AppData'); + expect(result.path).toContain('npm'); + expect(result.path).toContain('claude.cmd'); + }); + + it('should prefer .cmd/.exe paths when where.exe returns multiple results', () => { + vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + + vi.mocked(findExecutable).mockReturnValue(null); + + // Simulate where.exe returning path with .cmd extension (preferred over no extension) + vi.mocked(findWindowsExecutableViaWhere).mockReturnValue( + 'D:\\Program Files\\nvm4w\\nodejs\\claude.cmd' + ); + + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(execFileSync).mockReturnValue('claude-code version 1.0.0\n'); + + const result = getToolInfo('claude'); + + expect(result.found).toBe(true); + expect(result.path).toBe('D:\\Program Files\\nvm4w\\nodejs\\claude.cmd'); + expect(result.path).toMatch(/\.(cmd|exe)$/i); + }); + + it('should handle where.exe execution errors gracefully', () => { + vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + + vi.mocked(findExecutable).mockReturnValue(null); + + // Simulate where.exe error (returns null as designed) + vi.mocked(findWindowsExecutableViaWhere).mockReturnValue(null); + + vi.mocked(existsSync).mockReturnValue(false); + + // Should not crash, should continue to next detection method + const result = getToolInfo('claude'); + + expect(result).toBeDefined(); + expect(result.found).toBe(false); + expect(result.source).toBe('fallback'); + }); +}); + +/** + * Unit tests for async Claude CLI Windows where.exe detection + */ +describe('cli-tool-manager - Claude CLI async Windows where.exe detection', () => { + beforeEach(() => { + vi.clearAllMocks(); + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true + }); + }); + + afterEach(() => { + clearToolCache(); + }); + + it('should detect Claude CLI via where.exe asynchronously', async () => { + vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + + vi.mocked(findExecutableAsync).mockResolvedValue(null); + + vi.mocked(findWindowsExecutableViaWhereAsync).mockResolvedValue(null); + + // Mock file system - no platform paths exist + vi.mocked(existsSync).mockReturnValue(false); + + await getToolPathAsync('claude'); + + // Verify where.exe was called on Windows + expect(findWindowsExecutableViaWhereAsync).toHaveBeenCalledWith('claude', '[Claude CLI]'); + }); + + it('should handle async where.exe errors gracefully', async () => { + vi.mocked(os.homedir).mockReturnValue('C:\\Users\\test'); + + vi.mocked(findExecutableAsync).mockResolvedValue(null); + + vi.mocked(findWindowsExecutableViaWhereAsync).mockResolvedValue(null); + + vi.mocked(existsSync).mockReturnValue(false); + + // Should not crash + const result = await getToolPathAsync('claude'); + + expect(result).toBe('claude'); // Fallback + }); +}); diff --git a/apps/frontend/src/main/__tests__/env-handlers-claude-cli.test.ts b/apps/frontend/src/main/__tests__/env-handlers-claude-cli.test.ts index bbcbdc354a..3f310d5dea 100644 --- a/apps/frontend/src/main/__tests__/env-handlers-claude-cli.test.ts +++ b/apps/frontend/src/main/__tests__/env-handlers-claude-cli.test.ts @@ -41,9 +41,29 @@ vi.mock('../project-store', () => ({ }, })); -vi.mock('child_process', () => ({ - spawn: spawnMock, -})); +vi.mock('child_process', () => { + const mockExecFile = vi.fn((cmd: any, args: any, options: any, callback: any) => { + // Return a minimal ChildProcess-like object + const childProcess = { + stdout: { on: vi.fn() }, + stderr: { on: vi.fn() }, + on: vi.fn() + }; + + // If callback is provided, call it asynchronously + if (typeof callback === 'function') { + setImmediate(() => callback(null, '', '')); + } + + return childProcess as any; + }); + + return { + spawn: spawnMock, + execFileSync: vi.fn(), + execFile: mockExecFile + }; +}); vi.mock('electron', () => ({ app: { diff --git a/apps/frontend/src/main/__tests__/env-utils.test.ts b/apps/frontend/src/main/__tests__/env-utils.test.ts new file mode 100644 index 0000000000..cc0ee17c72 --- /dev/null +++ b/apps/frontend/src/main/__tests__/env-utils.test.ts @@ -0,0 +1,200 @@ +import { describe, expect, it, beforeEach, afterEach } from 'vitest'; +import { shouldUseShell, getSpawnOptions } from '../env-utils'; + +describe('shouldUseShell', () => { + const originalPlatform = process.platform; + + afterEach(() => { + // Restore original platform after each test + Object.defineProperty(process, 'platform', { + value: originalPlatform, + writable: true, + configurable: true, + }); + }); + + describe('Windows platform', () => { + beforeEach(() => { + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true, + configurable: true, + }); + }); + + it('should return true for .cmd files', () => { + expect(shouldUseShell('D:\\Program Files\\nodejs\\claude.cmd')).toBe(true); + expect(shouldUseShell('C:\\Users\\admin\\AppData\\Roaming\\npm\\claude.cmd')).toBe(true); + }); + + it('should return true for .bat files', () => { + expect(shouldUseShell('C:\\batch\\script.bat')).toBe(true); + }); + + it('should return true for .CMD (uppercase)', () => { + expect(shouldUseShell('D:\\Tools\\CLAUDE.CMD')).toBe(true); + }); + + it('should return true for .BAT (uppercase)', () => { + expect(shouldUseShell('C:\\Scripts\\SETUP.BAT')).toBe(true); + }); + + it('should return false for .exe files', () => { + expect(shouldUseShell('C:\\Windows\\System32\\git.exe')).toBe(false); + }); + + it('should return false for extensionless files', () => { + expect(shouldUseShell('D:\\Git\\bin\\bash')).toBe(false); + }); + + it('should handle paths with spaces and special characters', () => { + expect(shouldUseShell('D:\\Program Files (x86)\\tool.cmd')).toBe(true); + expect(shouldUseShell('D:\\Path&Name\\tool.cmd')).toBe(true); + expect(shouldUseShell('D:\\Program Files (x86)\\tool.exe')).toBe(false); + }); + }); + + describe('Non-Windows platforms', () => { + it('should return false on macOS', () => { + Object.defineProperty(process, 'platform', { + value: 'darwin', + writable: true, + configurable: true, + }); + expect(shouldUseShell('/usr/local/bin/claude')).toBe(false); + expect(shouldUseShell('/opt/homebrew/bin/claude.cmd')).toBe(false); + }); + + it('should return false on Linux', () => { + Object.defineProperty(process, 'platform', { + value: 'linux', + writable: true, + configurable: true, + }); + expect(shouldUseShell('/usr/bin/claude')).toBe(false); + expect(shouldUseShell('/home/user/.local/bin/claude.bat')).toBe(false); + }); + }); +}); + +describe('getSpawnOptions', () => { + const originalPlatform = process.platform; + + afterEach(() => { + // Restore original platform after each test + Object.defineProperty(process, 'platform', { + value: originalPlatform, + writable: true, + configurable: true, + }); + }); + + it('should set shell: true for .cmd files on Windows', () => { + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true, + configurable: true, + }); + + const opts = getSpawnOptions('D:\\nodejs\\claude.cmd', { + cwd: 'D:\\project', + env: { PATH: 'C:\\Windows' }, + }); + + expect(opts).toEqual({ + cwd: 'D:\\project', + env: { PATH: 'C:\\Windows' }, + shell: true, + }); + }); + + it('should set shell: false for .exe files on Windows', () => { + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true, + configurable: true, + }); + + const opts = getSpawnOptions('C:\\Windows\\git.exe', { + cwd: 'D:\\project', + }); + + expect(opts).toEqual({ + cwd: 'D:\\project', + shell: false, + }); + }); + + it('should preserve all base options including stdio', () => { + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true, + configurable: true, + }); + + const opts = getSpawnOptions('D:\\tool.cmd', { + cwd: 'D:\\project', + env: { FOO: 'bar' }, + timeout: 5000, + windowsHide: true, + stdio: 'inherit', + }); + + expect(opts).toEqual({ + cwd: 'D:\\project', + env: { FOO: 'bar' }, + timeout: 5000, + windowsHide: true, + stdio: 'inherit', + shell: true, + }); + }); + + it('should handle empty base options', () => { + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true, + configurable: true, + }); + + const opts = getSpawnOptions('D:\\tool.cmd'); + + expect(opts).toEqual({ + shell: true, + }); + }); + + it('should set shell: false on non-Windows platforms', () => { + Object.defineProperty(process, 'platform', { + value: 'darwin', + writable: true, + configurable: true, + }); + + const opts = getSpawnOptions('/usr/local/bin/claude', { + cwd: '/project', + }); + + expect(opts).toEqual({ + cwd: '/project', + shell: false, + }); + }); + + it('should handle .bat files on Windows', () => { + Object.defineProperty(process, 'platform', { + value: 'win32', + writable: true, + configurable: true, + }); + + const opts = getSpawnOptions('C:\\scripts\\setup.bat', { + cwd: 'D:\\project', + }); + + expect(opts).toEqual({ + cwd: 'D:\\project', + shell: true, + }); + }); +}); diff --git a/apps/frontend/src/main/cli-tool-manager.ts b/apps/frontend/src/main/cli-tool-manager.ts index 3f51a1a373..3aa514d1e6 100644 --- a/apps/frontend/src/main/cli-tool-manager.ts +++ b/apps/frontend/src/main/cli-tool-manager.ts @@ -20,33 +20,17 @@ * - Graceful fallbacks when tools not found */ -import { execFileSync, execFile } from 'child_process'; +import { execFileSync, execFile, execSync, exec } from 'child_process'; import { existsSync, readdirSync, promises as fsPromises } from 'fs'; import path from 'path'; import os from 'os'; import { promisify } from 'util'; import { app } from 'electron'; -import { findExecutable, findExecutableAsync, getAugmentedEnv, getAugmentedEnvAsync } from './env-utils'; +import { findExecutable, findExecutableAsync, getAugmentedEnv, getAugmentedEnvAsync, shouldUseShell, existsAsync } from './env-utils'; +import type { ToolDetectionResult } from '../shared/types'; const execFileAsync = promisify(execFile); - -/** - * Check if a path exists asynchronously (non-blocking) - * - * Uses fs.promises.access which is non-blocking, unlike fs.existsSync. - * - * @param filePath - The path to check - * @returns Promise resolving to true if path exists, false otherwise - */ -async function existsAsync(filePath: string): Promise { - try { - await fsPromises.access(filePath); - return true; - } catch { - return false; - } -} -import type { ToolDetectionResult } from '../shared/types'; +const execAsync = promisify(exec); import { findHomebrewPython as findHomebrewPythonUtil } from './utils/homebrew-python'; import { getWindowsExecutablePaths, @@ -54,6 +38,7 @@ import { WINDOWS_GIT_PATHS, findWindowsExecutableViaWhere, findWindowsExecutableViaWhereAsync, + isSecurePath, } from './utils/windows-paths'; /** @@ -705,7 +690,9 @@ class CLIToolManager { * 1. User configuration (if valid for current platform) * 2. Homebrew claude (macOS) * 3. System PATH - * 4. Windows/macOS/Linux standard locations + * 4. Windows where.exe (Windows only - finds executables via PATH + Registry) + * 5. NVM paths (Unix only - checks Node.js version managers) + * 6. Platform-specific standard locations * * @returns Detection result for Claude CLI */ @@ -719,6 +706,10 @@ class CLIToolManager { console.warn( `[Claude CLI] User-configured path is from different platform, ignoring: ${this.userConfig.claudePath}` ); + } else if (process.platform === 'win32' && !isSecurePath(this.userConfig.claudePath)) { + console.warn( + `[Claude CLI] User-configured path failed security validation, ignoring: ${this.userConfig.claudePath}` + ); } else { const validation = this.validateClaude(this.userConfig.claudePath); const result = buildClaudeDetectionResult( @@ -748,7 +739,17 @@ class CLIToolManager { if (result) return result; } - // 4. NVM paths (Unix only) - check before platform paths for better Node.js integration + // 4. Windows where.exe detection (Windows only - most reliable for custom installs) + if (process.platform === 'win32') { + const whereClaudePath = findWindowsExecutableViaWhere('claude', '[Claude CLI]'); + if (whereClaudePath) { + const validation = this.validateClaude(whereClaudePath); + const result = buildClaudeDetectionResult(whereClaudePath, validation, 'system-path', 'Using Windows Claude CLI'); + if (result) return result; + } + } + + // 5. NVM paths (Unix only) - check before platform paths for better Node.js integration if (process.platform !== 'win32') { try { if (existsSync(paths.nvmVersionsDir)) { @@ -769,7 +770,7 @@ class CLIToolManager { } } - // 5. Platform-specific standard locations + // 6. Platform-specific standard locations for (const claudePath of paths.platformPaths) { if (existsSync(claudePath)) { const validation = this.validateClaude(claudePath); @@ -778,7 +779,7 @@ class CLIToolManager { } } - // 6. Not found + // 7. Not found return { found: false, source: 'fallback', @@ -915,21 +916,30 @@ class CLIToolManager { */ private validateClaude(claudeCmd: string): ToolValidation { try { - // On Windows, .cmd files need shell: true to execute properly. - // SECURITY NOTE: shell: true is safe here because: - // 1. claudeCmd comes from internal path detection (user config or known system paths) - // 2. Only '--version' is passed as an argument (no user input) - // If claudeCmd origin ever changes to accept user input, use escapeShellArgWindows. - const needsShell = process.platform === 'win32' && - (claudeCmd.endsWith('.cmd') || claudeCmd.endsWith('.bat')); - - const version = execFileSync(claudeCmd, ['--version'], { - encoding: 'utf-8', - timeout: 5000, - windowsHide: true, - shell: needsShell, - env: getAugmentedEnv(), - }).trim(); + const needsShell = shouldUseShell(claudeCmd); + + let version: string; + + if (needsShell) { + // For .cmd/.bat files on Windows, use execSync with quoted path + // execFileSync doesn't handle spaces in .cmd paths correctly even with shell:true + const quotedCmd = `"${claudeCmd}"`; + version = execSync(`${quotedCmd} --version`, { + encoding: 'utf-8', + timeout: 5000, + windowsHide: true, + env: getAugmentedEnv(), + }).trim(); + } else { + // For .exe files and non-Windows, use execFileSync + version = execFileSync(claudeCmd, ['--version'], { + encoding: 'utf-8', + timeout: 5000, + windowsHide: true, + shell: false, + env: getAugmentedEnv(), + }).trim(); + } // Claude CLI version output format: "claude-code version X.Y.Z" or similar const match = version.match(/(\d+\.\d+\.\d+)/); @@ -1023,16 +1033,31 @@ class CLIToolManager { */ private async validateClaudeAsync(claudeCmd: string): Promise { try { - const needsShell = process.platform === 'win32' && - (claudeCmd.endsWith('.cmd') || claudeCmd.endsWith('.bat')); - - const { stdout } = await execFileAsync(claudeCmd, ['--version'], { - encoding: 'utf-8', - timeout: 5000, - windowsHide: true, - shell: needsShell, - env: await getAugmentedEnvAsync(), - }); + const needsShell = shouldUseShell(claudeCmd); + + let stdout: string; + + if (needsShell) { + // For .cmd/.bat files on Windows, use exec with quoted path + const quotedCmd = `"${claudeCmd}"`; + const result = await execAsync(`${quotedCmd} --version`, { + encoding: 'utf-8', + timeout: 5000, + windowsHide: true, + env: await getAugmentedEnvAsync(), + }); + stdout = result.stdout; + } else { + // For .exe files and non-Windows, use execFileAsync + const result = await execFileAsync(claudeCmd, ['--version'], { + encoding: 'utf-8', + timeout: 5000, + windowsHide: true, + shell: false, + env: await getAugmentedEnvAsync(), + }); + stdout = result.stdout; + } const version = stdout.trim(); const match = version.match(/(\d+\.\d+\.\d+)/); @@ -1176,7 +1201,13 @@ class CLIToolManager { /** * Detect Claude CLI asynchronously (non-blocking) * - * Same detection logic as detectClaude but uses async validation. + * Priority order: + * 1. User configuration (if valid for current platform) + * 2. Homebrew claude (macOS) + * 3. System PATH + * 4. Windows where.exe (Windows only - finds executables via PATH + Registry) + * 5. NVM paths (Unix only - checks Node.js version managers) + * 6. Platform-specific standard locations * * @returns Promise resolving to detection result */ @@ -1190,6 +1221,10 @@ class CLIToolManager { console.warn( `[Claude CLI] User-configured path is from different platform, ignoring: ${this.userConfig.claudePath}` ); + } else if (process.platform === 'win32' && !isSecurePath(this.userConfig.claudePath)) { + console.warn( + `[Claude CLI] User-configured path failed security validation, ignoring: ${this.userConfig.claudePath}` + ); } else { const validation = await this.validateClaudeAsync(this.userConfig.claudePath); const result = buildClaudeDetectionResult( @@ -1219,7 +1254,17 @@ class CLIToolManager { if (result) return result; } - // 4. NVM paths (Unix only) - check before platform paths for better Node.js integration + // 4. Windows where.exe detection (async, non-blocking) + if (process.platform === 'win32') { + const whereClaudePath = await findWindowsExecutableViaWhereAsync('claude', '[Claude CLI]'); + if (whereClaudePath) { + const validation = await this.validateClaudeAsync(whereClaudePath); + const result = buildClaudeDetectionResult(whereClaudePath, validation, 'system-path', 'Using Windows Claude CLI'); + if (result) return result; + } + } + + // 5. NVM paths (Unix only) - check before platform paths for better Node.js integration if (process.platform !== 'win32') { try { if (await existsAsync(paths.nvmVersionsDir)) { @@ -1240,7 +1285,7 @@ class CLIToolManager { } } - // 5. Platform-specific standard locations + // 6. Platform-specific standard locations for (const claudePath of paths.platformPaths) { if (await existsAsync(claudePath)) { const validation = await this.validateClaudeAsync(claudePath); @@ -1249,7 +1294,7 @@ class CLIToolManager { } } - // 6. Not found + // 7. Not found return { found: false, source: 'fallback', diff --git a/apps/frontend/src/main/env-utils.ts b/apps/frontend/src/main/env-utils.ts index 01972d6af0..8463a2ace4 100644 --- a/apps/frontend/src/main/env-utils.ts +++ b/apps/frontend/src/main/env-utils.ts @@ -26,7 +26,7 @@ const execFileAsync = promisify(execFile); * @param filePath - The path to check * @returns Promise resolving to true if path exists, false otherwise */ -async function existsAsync(filePath: string): Promise { +export async function existsAsync(filePath: string): Promise { try { await fsPromises.access(filePath); return true; @@ -438,3 +438,70 @@ export function clearNpmPrefixCache(): void { npmGlobalPrefixCache = undefined; npmGlobalPrefixCachePromise = null; } + +/** + * Determine if a command requires shell execution on Windows + * + * Windows .cmd and .bat files MUST be executed through shell, while .exe files + * can be executed directly. This function checks the file extension to determine + * the correct execution method. + * + * @param command - The command path to check + * @returns true if shell is required (Windows .cmd/.bat), false otherwise + * + * @example + * ```typescript + * shouldUseShell('D:\\nodejs\\claude.cmd') // true + * shouldUseShell('C:\\Program Files\\nodejs\\claude.cmd') // true + * shouldUseShell('C:\\Windows\\System32\\git.exe') // false + * shouldUseShell('/usr/local/bin/claude') // false (non-Windows) + * ``` + */ +export function shouldUseShell(command: string): boolean { + // Only Windows needs special handling for .cmd/.bat files + if (process.platform !== 'win32') { + return false; + } + + // Check if command ends with .cmd or .bat (case-insensitive) + return /\.(cmd|bat)$/i.test(command); +} + +/** + * Get spawn options with correct shell setting for Windows compatibility + * + * Provides a consistent way to create spawn options that work across platforms. + * Handles the shell requirement for Windows .cmd/.bat files automatically. + * + * @param command - The command path to execute + * @param baseOptions - Base spawn options to merge with (optional) + * @returns Spawn options with correct shell setting + * + * @example + * ```typescript + * const opts = getSpawnOptions(claudeCmd, { cwd: '/project', env: {...} }); + * spawn(claudeCmd, ['--version'], opts); + * ``` + */ +export function getSpawnOptions( + command: string, + baseOptions?: { + cwd?: string; + env?: Record; + timeout?: number; + windowsHide?: boolean; + stdio?: 'inherit' | 'pipe' | Array<'inherit' | 'pipe'>; + } +): { + cwd?: string; + env?: Record; + shell: boolean; + timeout?: number; + windowsHide?: boolean; + stdio?: 'inherit' | 'pipe' | Array<'inherit' | 'pipe'>; +} { + return { + ...baseOptions, + shell: shouldUseShell(command), + }; +} diff --git a/apps/frontend/src/main/ipc-handlers/env-handlers.ts b/apps/frontend/src/main/ipc-handlers/env-handlers.ts index 99ab0790c4..3a9cbe417f 100644 --- a/apps/frontend/src/main/ipc-handlers/env-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/env-handlers.ts @@ -10,6 +10,7 @@ import { projectStore } from '../project-store'; import { parseEnvFile } from './utils'; import { getClaudeCliInvocation, getClaudeCliInvocationAsync } from '../claude-cli-utils'; import { debugError } from '../../shared/utils/debug-logger'; +import { getSpawnOptions } from '../env-utils'; // GitLab environment variable keys const GITLAB_ENV_KEYS = { @@ -602,11 +603,10 @@ ${existingVars['GRAPHITI_DB_PATH'] ? `GRAPHITI_DB_PATH=${existingVars['GRAPHITI_ try { // Check if Claude CLI is available and authenticated const result = await new Promise((resolve) => { - const proc = spawn(claudeCmd, ['--version'], { + const proc = spawn(claudeCmd, ['--version'], getSpawnOptions(claudeCmd, { cwd: project.path, env: claudeEnv, - shell: false - }); + })); let _stdout = ''; let _stderr = ''; @@ -623,11 +623,10 @@ ${existingVars['GRAPHITI_DB_PATH'] ? `GRAPHITI_DB_PATH=${existingVars['GRAPHITI_ if (code === 0) { // Claude CLI is available, check if authenticated // Run a simple command that requires auth - const authCheck = spawn(claudeCmd, ['api', '--help'], { + const authCheck = spawn(claudeCmd, ['api', '--help'], getSpawnOptions(claudeCmd, { cwd: project.path, env: claudeEnv, - shell: false - }); + })); authCheck.on('close', (authCode: number | null) => { resolve({ @@ -693,12 +692,11 @@ ${existingVars['GRAPHITI_DB_PATH'] ? `GRAPHITI_DB_PATH=${existingVars['GRAPHITI_ try { // Run claude setup-token which will open browser for OAuth const result = await new Promise((resolve) => { - const proc = spawn(claudeCmd, ['setup-token'], { + const proc = spawn(claudeCmd, ['setup-token'], getSpawnOptions(claudeCmd, { cwd: project.path, env: claudeEnv, - shell: false, stdio: 'inherit' // This allows the terminal to handle the interactive auth - }); + })); proc.on('close', (code: number | null) => { if (code === 0) { diff --git a/apps/frontend/src/main/utils/windows-paths.ts b/apps/frontend/src/main/utils/windows-paths.ts index 355640ac01..00ceaf0525 100644 --- a/apps/frontend/src/main/utils/windows-paths.ts +++ b/apps/frontend/src/main/utils/windows-paths.ts @@ -37,12 +37,13 @@ export const WINDOWS_GIT_PATHS: WindowsToolPaths = { ], }; -function isSecurePath(pathStr: string): boolean { +export function isSecurePath(pathStr: string): boolean { const dangerousPatterns = [ - /[;&|`$(){}[\]<>!]/, // Shell metacharacters - /\.\.\//, // Unix directory traversal - /\.\.\\/, // Windows directory traversal - /[\r\n]/, // Newlines (command injection) + /[;&|`${}[\]<>!"^]/, // Shell metacharacters (parentheses removed - safe when quoted) + /%[^%]+%/, // Windows environment variable expansion (e.g., %PATH%) + /\.\.\//, // Unix directory traversal + /\.\.\\/, // Windows directory traversal + /[\r\n]/, // Newlines (command injection) ]; for (const pattern of dangerousPatterns) { @@ -155,11 +156,12 @@ export function findWindowsExecutableViaWhere( }).trim(); // 'where' returns multiple paths separated by newlines if found in multiple locations - // We take the first one (highest priority in PATH) + // Prefer paths with .cmd or .exe extensions (executable files) const paths = result.split(/\r?\n/).filter(p => p.trim()); if (paths.length > 0) { - const foundPath = paths[0].trim(); + // Prefer .cmd, .bat, or .exe extensions, otherwise take first path + const foundPath = (paths.find(p => /\.(cmd|bat|exe)$/i.test(p)) || paths[0]).trim(); // Validate the path exists and is secure if (existsSync(foundPath) && isSecurePath(foundPath)) { @@ -257,11 +259,12 @@ export async function findWindowsExecutableViaWhereAsync( }); // 'where' returns multiple paths separated by newlines if found in multiple locations - // We take the first one (highest priority in PATH) + // Prefer paths with .cmd, .bat, or .exe extensions (executable files) const paths = stdout.trim().split(/\r?\n/).filter(p => p.trim()); if (paths.length > 0) { - const foundPath = paths[0].trim(); + // Prefer .cmd, .bat, or .exe extensions, otherwise take first path + const foundPath = (paths.find(p => /\.(cmd|bat|exe)$/i.test(p)) || paths[0]).trim(); // Validate the path exists and is secure try {