diff --git a/apps/frontend/src/main/ipc-handlers/__tests__/claude-code-installations.test.ts b/apps/frontend/src/main/ipc-handlers/__tests__/claude-code-installations.test.ts new file mode 100644 index 0000000000..6ebdf9aac2 --- /dev/null +++ b/apps/frontend/src/main/ipc-handlers/__tests__/claude-code-installations.test.ts @@ -0,0 +1,201 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { EventEmitter } from 'events'; +import path from 'path'; +import { mkdtempSync, writeFileSync, rmSync, existsSync } from 'fs'; +import { tmpdir } from 'os'; +import { promisify } from 'util'; +import { IPC_CHANNELS } from '../../../shared/constants'; + +class MockIpcMain extends EventEmitter { + private handlers = new Map(); + + handle(channel: string, handler: Function): void { + this.handlers.set(channel, handler); + } + + removeHandler(channel: string): void { + this.handlers.delete(channel); + } + + clearHandlers(): void { + this.handlers.clear(); + } + + async invokeHandler(channel: string, event: unknown, ...args: unknown[]): Promise { + const handler = this.handlers.get(channel); + if (!handler) { + throw new Error(`No handler registered for channel: ${channel}`); + } + return handler(event, ...args); + } +} + +const ipcMain = new MockIpcMain(); + +const execFileMock = vi.fn(); +const spawnMock = vi.fn(); +const execFileSyncMock = vi.fn(); + +// `claude-code-handlers.ts` uses `promisify(execFile)` (which relies on execFile's custom promisify behavior). +// When we mock `execFile` with `vi.fn()`, it loses `util.promisify.custom` and would resolve to stdout only, +// breaking code that expects `{ stdout, stderr }`. Re-add the custom promisify shape here for tests. +(execFileMock as unknown as Record)[promisify.custom] = ( + file: string, + args: string[] = [], + options: Record = {} +): Promise<{ stdout: string; stderr: string }> => new Promise((resolve, reject) => { + execFileMock(file, args, options, (err: unknown, stdout: string, stderr: string) => { + if (err) { + reject(err); + return; + } + resolve({ stdout, stderr }); + }); +}); + +vi.mock('electron', () => ({ ipcMain })); + +vi.mock('child_process', () => ({ + execFile: execFileMock, + execFileSync: execFileSyncMock, + spawn: spawnMock, +})); + +vi.mock('../../settings-utils', () => ({ + readSettingsFile: () => null, + writeSettingsFile: vi.fn(), +})); + +vi.mock('../../cli-tool-manager', () => ({ + getToolInfo: vi.fn(), + configureTools: vi.fn(), + sortNvmVersionDirs: () => [], + getClaudeDetectionPaths: () => ({ + homebrewPaths: [], + platformPaths: [], + // Only used on non-Windows in scanClaudeInstallations + nvmVersionsDir: '', + }), +})); + +describe.skipIf(process.platform !== 'win32')('Claude Code installations scan (Windows)', () => { + let testDir = ''; + let shimPath = ''; + let cmdPath = ''; + + beforeEach(async () => { + vi.resetModules(); + ipcMain.clearHandlers(); + execFileMock.mockReset(); + spawnMock.mockReset(); + execFileSyncMock.mockReset(); + + testDir = mkdtempSync(path.join(tmpdir(), 'claude-install-scan-')); + shimPath = path.join(testDir, 'claude'); + cmdPath = path.join(testDir, 'claude.cmd'); + writeFileSync(shimPath, ''); + writeFileSync(cmdPath, ''); + if (!existsSync(shimPath) || !existsSync(cmdPath)) { + throw new Error(`Test setup failed; expected paths to exist: ${shimPath}, ${cmdPath}`); + } + + execFileMock.mockImplementation(( + file: string, + args: string[], + options: Record | ((err: unknown, stdout: string, stderr: string) => void), + callback?: (err: unknown, stdout: string, stderr: string) => void + ) => { + const cb = typeof options === 'function' ? options : callback; + const opts = typeof options === 'function' ? {} : (options || {}); + if (!cb) { + throw new Error('execFile callback is required'); + } + + // Simulate `where claude` returning both an extensionless shim and a quoted .cmd path. + if (file === 'where') { + const stdout = [ + `"${shimPath}"`, + `"${cmdPath}"`, + ].join('\r\n'); + cb(null, `${stdout}\r\n`, ''); + return; + } + + // Simulate validating an extensionless shim: CreateProcess-style spawn fails (ENOENT) + const fileLower = String(file).toLowerCase(); + if (fileLower === shimPath.toLowerCase() && args[0] === '--version') { + const err = Object.assign(new Error('spawn ENOENT'), { code: 'ENOENT' }); + cb(err, '', ''); + return; + } + + // Simulate validating a .cmd via cmd.exe: require verbatim arguments to succeed + if (fileLower.endsWith('\\cmd.exe') && args[0] === '/d' && args[1] === '/s' && args[2] === '/c') { + if (opts.windowsVerbatimArguments !== true) { + const err = Object.assign(new Error('Command failed'), { code: 1 }); + cb(err, '', 'The system cannot find the path specified.\r\n'); + return; + } + + // Ensure the cmdline references the unquoted .cmd path (not a quoted string from `where`) + if (!String(args[3] || '').includes(cmdPath)) { + cb(new Error(`cmd.exe received unexpected cmdline: ${String(args[3])}`), '', ''); + return; + } + + cb(null, '2.1.6 (Claude Code)\r\n', ''); + return; + } + + cb(new Error(`Unexpected execFile invocation: ${file} ${args.join(' ')}`), '', ''); + }); + }); + + afterEach(() => { + if (testDir) { + rmSync(testDir, { recursive: true, force: true }); + } + testDir = ''; + shimPath = ''; + cmdPath = ''; + }); + + it('skips extensionless shims and validates quoted .cmd paths', async () => { + const { isSecurePath } = await import('../../utils/windows-paths'); + expect(cmdPath.endsWith('.cmd')).toBe(true); + expect(isSecurePath(cmdPath)).toBe(true); + + const { registerClaudeCodeHandlers } = await import('../claude-code-handlers'); + registerClaudeCodeHandlers(); + + const result = await ipcMain.invokeHandler(IPC_CHANNELS.CLAUDE_CODE_GET_INSTALLATIONS, {}); + expect(result).toEqual(expect.objectContaining({ success: true })); + + const data = (result as { success: true; data: { installations: Array<{ path: string }> } }).data; + expect(data.installations).toHaveLength(1); + expect(data.installations[0]?.path).toBe(path.resolve(cmdPath)); + + const validatedShim = execFileMock.mock.calls.some(([file, args]) => ( + file === shimPath + && Array.isArray(args) + && args[0] === '--version' + )); + expect(validatedShim).toBe(false); + + const calledExecutables = execFileMock.mock.calls.map(([file]) => String(file)); + expect(calledExecutables).toContain('where'); + + const cmdExeCall = execFileMock.mock.calls.find(([file, args]) => ( + typeof file === 'string' + && /\\cmd\.exe$/i.test(file) + && Array.isArray(args) + && args[0] === '/d' + && args[1] === '/s' + && args[2] === '/c' + )); + if (!cmdExeCall) { + throw new Error(`Expected cmd.exe call, got: ${JSON.stringify(execFileMock.mock.calls, null, 2)}`); + } + expect(cmdExeCall?.[2]).toEqual(expect.objectContaining({ windowsVerbatimArguments: true })); + }); +}); diff --git a/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts b/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts index 28f912d6c0..7892f46eb0 100644 --- a/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts @@ -29,6 +29,15 @@ let cachedVersionList: { versions: string[]; timestamp: number } | null = null; const CACHE_DURATION_MS = 24 * 60 * 60 * 1000; // 24 hours const VERSION_LIST_CACHE_DURATION_MS = 60 * 60 * 1000; // 1 hour for version list +function stripWrappingQuotes(value: string): string { + const trimmed = value.trim(); + if ((trimmed.startsWith('"') && trimmed.endsWith('"')) + || (trimmed.startsWith("'") && trimmed.endsWith("'"))) { + return trimmed.slice(1, -1); + } + return trimmed; +} + /** * Validate a Claude CLI path and get its version * @param cliPath - Path to the Claude CLI executable @@ -37,9 +46,10 @@ const VERSION_LIST_CACHE_DURATION_MS = 60 * 60 * 1000; // 1 hour for version lis async function validateClaudeCliAsync(cliPath: string): Promise<[boolean, string | null]> { try { const isWindows = process.platform === 'win32'; + const unquotedPath = stripWrappingQuotes(cliPath); // Augment PATH with the CLI directory for proper resolution - const cliDir = path.dirname(cliPath); + const cliDir = path.dirname(unquotedPath); const env = { ...process.env, PATH: cliDir ? `${cliDir}${path.delimiter}${process.env.PATH || ''}` : process.env.PATH, @@ -50,21 +60,22 @@ async function validateClaudeCliAsync(cliPath: string): Promise<[boolean, string // /d = disable AutoRun registry commands // /s = strip first and last quotes, preserving inner quotes // /c = run command then terminate - if (isWindows && /\.(cmd|bat)$/i.test(cliPath)) { + if (isWindows && /\.(cmd|bat)$/i.test(unquotedPath)) { // Get cmd.exe path from environment or use default const cmdExe = process.env.ComSpec || path.join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'cmd.exe'); // Use double-quoted command line for paths with spaces - const cmdLine = `""${cliPath}" --version"`; + const cmdLine = `""${unquotedPath}" --version"`; const result = await execFileAsync(cmdExe, ['/d', '/s', '/c', cmdLine], { encoding: 'utf-8', timeout: 5000, windowsHide: true, + windowsVerbatimArguments: true, env, }); stdout = result.stdout; } else { - const result = await execFileAsync(cliPath, ['--version'], { + const result = await execFileAsync(unquotedPath, ['--version'], { encoding: 'utf-8', timeout: 5000, windowsHide: true, @@ -101,50 +112,62 @@ async function scanClaudeInstallations(activePath: string | null): Promise { + const unquotedPath = stripWrappingQuotes(cliPath); + + // On Windows, only attempt to validate executable file types. Some package managers + // can create extensionless shims (e.g., "claude") that exist on disk but cannot be + // executed directly via CreateProcess. + if (isWindows && !/\.(cmd|bat|exe)$/i.test(unquotedPath)) return; + // Normalize path for comparison - const normalizedPath = path.resolve(cliPath); - if (seenPaths.has(normalizedPath)) return; + const normalizedPath = path.resolve(unquotedPath); + const seenKey = isWindows ? normalizedPath.toLowerCase() : normalizedPath; + if (seenPaths.has(seenKey)) return; - if (!existsSync(cliPath)) return; + if (!existsSync(unquotedPath)) return; // Security validation: reject paths with shell metacharacters or directory traversal - if (!isSecurePath(cliPath)) { - console.warn('[Claude Code] Rejecting insecure path:', cliPath); + if (!isSecurePath(unquotedPath)) { + console.warn('[Claude Code] Rejecting insecure path:', unquotedPath); return; } - const [isValid, version] = await validateClaudeCliAsync(cliPath); + const [isValid, version] = await validateClaudeCliAsync(unquotedPath); if (!isValid) return; - seenPaths.add(normalizedPath); + seenPaths.add(seenKey); installations.push({ path: normalizedPath, version, source, - isActive: activePath ? path.resolve(activePath) === normalizedPath : false, + isActive: normalizedActiveKey ? seenKey === normalizedActiveKey : false, }); }; // 1. Check user-configured path first (if set) - if (activePath && existsSync(activePath)) { + if (activePath) { await addInstallation(activePath, 'user-config'); } // 2. Check system PATH via which/where try { if (isWindows) { - const result = await execFileAsync('where', ['claude'], { timeout: 5000 }); - const paths = result.stdout.trim().split('\n').filter(p => p.trim()); + const result = await execFileAsync('where', ['claude'], { timeout: 5000, encoding: 'utf-8' }); + const paths = result.stdout.trim().split(/\r?\n/).filter(p => p.trim()); for (const p of paths) { await addInstallation(p.trim(), 'system-path'); } } else { - const result = await execFileAsync('which', ['-a', 'claude'], { timeout: 5000 }); - const paths = result.stdout.trim().split('\n').filter(p => p.trim()); + const result = await execFileAsync('which', ['-a', 'claude'], { timeout: 5000, encoding: 'utf-8' }); + const paths = result.stdout.trim().split(/\r?\n/).filter(p => p.trim()); for (const p of paths) { await addInstallation(p.trim(), 'system-path'); }