diff --git a/src/management/checks/system-check.ts b/src/management/checks/system-check.ts index 875c0976..7e6ceba3 100644 --- a/src/management/checks/system-check.ts +++ b/src/management/checks/system-check.ts @@ -5,7 +5,11 @@ import * as fs from 'fs'; import { spawn } from 'child_process'; import { getClaudeCliInfo } from '../../utils/claude-detector'; -import { escapeShellArg, stripClaudeCodeEnv } from '../../utils/shell-executor'; +import { + escapeShellArg, + getWindowsEscapedCommandShell, + stripClaudeCodeEnv, +} from '../../utils/shell-executor'; import { ok, fail } from '../../utils/ui'; import { HealthCheck, IHealthChecker, createSpinner } from './types'; import { getCcsDir } from '../../config/config-loader-facade'; @@ -47,7 +51,7 @@ export class ClaudeCliChecker implements IHealthChecker { ? spawn([claudeCli, '--version'].map(escapeShellArg).join(' '), { stdio: 'pipe', timeout: 5000, - shell: true, + shell: getWindowsEscapedCommandShell(), env: stripClaudeCodeEnv(process.env), }) : spawn(claudeCli, ['--version'], { diff --git a/src/utils/claude-detector.ts b/src/utils/claude-detector.ts index a0cea8ed..67eb2c56 100644 --- a/src/utils/claude-detector.ts +++ b/src/utils/claude-detector.ts @@ -2,7 +2,12 @@ import * as fs from 'fs'; import { execFileSync, execSync } from 'child_process'; import { expandPath } from './helpers'; import type { ClaudeCliInfo } from '../types'; -import { escapeShellArg, stripAnthropicEnv, stripClaudeCodeEnv } from './shell-executor'; +import { + escapeShellArg, + getWindowsEscapedCommandShell, + stripAnthropicEnv, + stripClaudeCodeEnv, +} from './shell-executor'; export interface ClaudeAuthStatus { loggedIn: boolean; @@ -164,11 +169,12 @@ function runClaudeCliCommand(args: string[], envOverrides?: NodeJS.ProcessEnv): } if (needsShell) { + const shell = getWindowsEscapedCommandShell(); return execSync([claudePath, ...args].map(escapeShellArg).join(' '), { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 5000, - shell: process.env.ComSpec || 'cmd.exe', + shell: typeof shell === 'string' ? shell : undefined, env, }).trim(); } diff --git a/src/utils/shell-executor.ts b/src/utils/shell-executor.ts index 37356f28..8336938d 100644 --- a/src/utils/shell-executor.ts +++ b/src/utils/shell-executor.ts @@ -5,6 +5,7 @@ */ import { spawn, spawnSync, ChildProcess, type SpawnOptions } from 'child_process'; +import * as path from 'path'; import { ErrorManager } from './error-manager'; import { getWebSearchHookEnv } from './websearch-manager'; import { wireChildProcessSignals } from './signal-forwarder'; @@ -53,6 +54,7 @@ const TMUX_SYNC_ENV_KEYS = [ ...ANTHROPIC_MODEL_ENV_KEYS, ...ANTHROPIC_ROUTING_ENV_KEYS, ]; +const DEFAULT_WINDOWS_CMD_SHELL = 'C:\\Windows\\System32\\cmd.exe'; /** * Strip inherited Anthropic routing/auth env while preserving model intent. @@ -214,15 +216,36 @@ export function escapeShellArg(arg: string): string { /** * Return the shell that matches escapeShellArg() quoting semantics. * - * On Windows, prefer ComSpec over a bare `cmd.exe` so escaped wrapper launches - * keep the same shell contract without depending on PATH lookup. + * On Windows, use an absolute, trusted system cmd.exe path instead of a bare + * executable name so wrapper launches cannot be hijacked through the current + * directory or PATH. ComSpec is accepted only when it resolves to that same + * system shell. */ export function getWindowsEscapedCommandShell(): SpawnOptions['shell'] { if (process.platform !== 'win32') { return true; } - return process.env.ComSpec || process.env.COMSPEC || 'cmd.exe'; + const systemRoot = [process.env.SystemRoot, process.env.SYSTEMROOT, process.env.windir].find( + (candidate): candidate is string => Boolean(candidate && path.win32.isAbsolute(candidate)) + ); + const trustedSystemCmd = systemRoot + ? path.win32.normalize(path.win32.join(systemRoot, 'System32', 'cmd.exe')) + : DEFAULT_WINDOWS_CMD_SHELL; + const trustedSystemCmdLower = trustedSystemCmd.toLowerCase(); + + for (const candidate of [process.env.ComSpec, process.env.COMSPEC]) { + if (!candidate || !path.win32.isAbsolute(candidate)) { + continue; + } + + const normalizedCandidate = path.win32.normalize(candidate); + if (normalizedCandidate.toLowerCase() === trustedSystemCmdLower) { + return normalizedCandidate; + } + } + + return trustedSystemCmd; } /** diff --git a/tests/unit/management/checks/system-check.test.ts b/tests/unit/management/checks/system-check.test.ts new file mode 100644 index 00000000..e8b85ef8 --- /dev/null +++ b/tests/unit/management/checks/system-check.test.ts @@ -0,0 +1,66 @@ +import { afterEach, describe, expect, it, spyOn } from 'bun:test'; +import * as childProcess from 'child_process'; +import { EventEmitter } from 'events'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +import { HealthCheck } from '../../../../src/management/checks/types'; + +const originalPlatform = process.platform; +let originalClaudePath: string | undefined; + +function createMockChild(): EventEmitter & { + stdout: EventEmitter; + stderr: EventEmitter; +} { + const child = new EventEmitter() as EventEmitter & { + stdout: EventEmitter; + stderr: EventEmitter; + }; + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + setTimeout(() => { + child.stdout.emit('data', Buffer.from('1.2.3')); + child.emit('close', 0); + }, 0); + return child; +} + +afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + if (originalClaudePath === undefined) delete process.env.CCS_CLAUDE_PATH; + else process.env.CCS_CLAUDE_PATH = originalClaudePath; +}); + +describe('ClaudeCliChecker', () => { + it('uses the pinned Windows shell for cmd wrapper health checks', async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-system-check-')); + const fakeClaude = path.join(tmpDir, 'claude.cmd'); + fs.writeFileSync(fakeClaude, ''); + originalClaudePath = process.env.CCS_CLAUDE_PATH; + process.env.CCS_CLAUDE_PATH = fakeClaude; + Object.defineProperty(process, 'platform', { value: 'win32' }); + + const spawnSpy = spyOn(childProcess, 'spawn').mockImplementation( + () => createMockChild() as unknown as ReturnType + ); + + try { + const { ClaudeCliChecker } = await import('../../../../src/management/checks/system-check'); + const results = new HealthCheck(); + await new ClaudeCliChecker().run(results); + + expect(spawnSpy).toHaveBeenCalledTimes(1); + const [, options] = spawnSpy.mock.calls[0] as [ + string, + Record | undefined, + ]; + expect(options?.shell).toBe('C:\\Windows\\System32\\cmd.exe'); + expect(results.details['Claude CLI']?.status).toBe('OK'); + } finally { + spawnSpy.mockRestore(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/unit/targets/codex-adapter-exec.test.ts b/tests/unit/targets/codex-adapter-exec.test.ts index d5166e03..ebfe191d 100644 --- a/tests/unit/targets/codex-adapter-exec.test.ts +++ b/tests/unit/targets/codex-adapter-exec.test.ts @@ -94,7 +94,7 @@ describe('codex-adapter exec', () => { string, Record | undefined, ]; - expect(options?.shell).toBe('cmd.exe'); + expect(options?.shell).toBe('C:\\Windows\\System32\\cmd.exe'); expect(command).toContain(fakeCodex); expect(command).toContain('mcp_servers.ccs_browser.args='); expect(command).toContain('@playwright/mcp@0.0.70'); diff --git a/tests/unit/targets/codex-detector.test.ts b/tests/unit/targets/codex-detector.test.ts index b0fc8e52..b361d8ac 100644 --- a/tests/unit/targets/codex-detector.test.ts +++ b/tests/unit/targets/codex-detector.test.ts @@ -81,7 +81,7 @@ describe('codex-detector', () => { expect(spawnSyncSpy).toHaveBeenCalled(); expect(cmdWrapperProbeCall).toBeDefined(); expect((cmdWrapperProbeCall?.[1] as Record | undefined)?.shell).toBe( - 'cmd.exe' + 'C:\\Windows\\System32\\cmd.exe' ); expect(info?.needsShell).toBe(true); expect(info?.features).toContain('config-overrides'); @@ -96,7 +96,9 @@ describe('codex-detector', () => { fs.writeFileSync(fakePsCodex, ''); Object.defineProperty(process, 'platform', { value: 'win32' }); - const execSyncSpy = spyOn(childProcess, 'execSync').mockImplementation(() => `${fakeCmdCodex}\n`); + const execSyncSpy = spyOn(childProcess, 'execSync').mockImplementation( + () => `${fakeCmdCodex}\n` + ); expect(detectCodexCli()).toBe(fakeCmdCodex); @@ -110,7 +112,9 @@ describe('codex-detector', () => { fs.writeFileSync(fakePsCodex, ''); Object.defineProperty(process, 'platform', { value: 'win32' }); - const execSyncSpy = spyOn(childProcess, 'execSync').mockImplementation(() => `${fakePsCodex}\n`); + const execSyncSpy = spyOn(childProcess, 'execSync').mockImplementation( + () => `${fakePsCodex}\n` + ); expect(detectCodexCli()).toBe(fakeCmdCodex); @@ -122,19 +126,21 @@ describe('codex-detector', () => { fs.writeFileSync(fakeCodex, ''); process.env.CCS_CODEX_PATH = fakeCodex; - const execFileSyncSpy = spyOn(childProcess, 'execFileSync').mockImplementation((command, args) => { - const joinedArgs = Array.isArray(args) ? args.join(' ') : ''; + const execFileSyncSpy = spyOn(childProcess, 'execFileSync').mockImplementation( + (command, args) => { + const joinedArgs = Array.isArray(args) ? args.join(' ') : ''; - if (joinedArgs.includes('--help')) { - return 'Codex CLI\n'; - } + if (joinedArgs.includes('--help')) { + return 'Codex CLI\n'; + } + + if (joinedArgs.includes('-c') && joinedArgs.includes('--version')) { + return 'codex-cli 0.119.0-alpha.1'; + } - if (joinedArgs.includes('-c') && joinedArgs.includes('--version')) { return 'codex-cli 0.119.0-alpha.1'; } - - return 'codex-cli 0.119.0-alpha.1'; - }); + ); const info = getCodexBinaryInfo(); @@ -148,19 +154,21 @@ describe('codex-detector', () => { fs.writeFileSync(fakeCodex, ''); process.env.CCS_CODEX_PATH = fakeCodex; - const execFileSyncSpy = spyOn(childProcess, 'execFileSync').mockImplementation((command, args) => { - const joinedArgs = Array.isArray(args) ? args.join(' ') : ''; + const execFileSyncSpy = spyOn(childProcess, 'execFileSync').mockImplementation( + (command, args) => { + const joinedArgs = Array.isArray(args) ? args.join(' ') : ''; - if (joinedArgs.includes('--help')) { - return 'Codex CLI\n -c, --config \n'; + if (joinedArgs.includes('--help')) { + return 'Codex CLI\n -c, --config \n'; + } + + if (joinedArgs.includes('-c') && joinedArgs.includes('--version')) { + throw new Error('unsupported'); + } + + return 'codex-cli 0.119.0-alpha.1'; } - - if (joinedArgs.includes('-c') && joinedArgs.includes('--version')) { - throw new Error('unsupported'); - } - - return 'codex-cli 0.119.0-alpha.1'; - }); + ); const info = getCodexBinaryInfo(); diff --git a/tests/unit/utils/claudecode-env-stripping.test.ts b/tests/unit/utils/claudecode-env-stripping.test.ts index 767609ca..d3f819cd 100644 --- a/tests/unit/utils/claudecode-env-stripping.test.ts +++ b/tests/unit/utils/claudecode-env-stripping.test.ts @@ -405,7 +405,7 @@ describe('CLAUDECODE environment stripping', () => { expect(spawnCalls.length).toBeGreaterThan(0); const env = spawnCalls[0].options?.env as NodeJS.ProcessEnv; expect(Object.keys(env).map((k) => k.toUpperCase())).not.toContain('CLAUDECODE'); - expect(spawnCalls[0].options?.shell).toBe('cmd.exe'); + expect(spawnCalls[0].options?.shell).toBe('C:\\Windows\\System32\\cmd.exe'); }); it('execClaude sets DISABLE_AUTOUPDATER=1 when preferences.auto_update is false', () => { @@ -827,9 +827,9 @@ describe('CLAUDECODE environment stripping', () => { expect(persistedLaunchSettings.env?.ANTHROPIC_AUTH_TOKEN).toBeUndefined(); expect(persistedLaunchSettings.env?.ANTHROPIC_API_KEY).toBeUndefined(); expect(persistedLaunchSettings.env?.ANTHROPIC_MODEL).toBe('gpt-5.4'); - expect( - persistedLaunchSettings.hooks?.PreToolUse?.[0]?.hooks?.[0]?.command - ).toBe('echo headless-bridge-hook'); + expect(persistedLaunchSettings.hooks?.PreToolUse?.[0]?.hooks?.[0]?.command).toBe( + 'echo headless-bridge-hook' + ); expect(fs.existsSync(launchSettingsPath)).toBe(false); }); diff --git a/tests/unit/utils/shell-executor.test.ts b/tests/unit/utils/shell-executor.test.ts index f3b7e600..f09dc52a 100644 --- a/tests/unit/utils/shell-executor.test.ts +++ b/tests/unit/utils/shell-executor.test.ts @@ -82,41 +82,103 @@ describe('escapeShellArg', () => { expect(escapeShellArg('hello!')).toBe('"hello^^!"'); }); - it('prefers ComSpec when resolving the escaped command shell', async () => { + it('accepts ComSpec only when it resolves to the trusted system cmd.exe', async () => { const originalComSpec = process.env.ComSpec; const originalCOMSPEC = process.env.COMSPEC; + const originalSystemRoot = process.env.SystemRoot; try { + process.env.SystemRoot = 'C:\\Windows'; process.env.ComSpec = 'C:\\Windows\\System32\\cmd.exe'; delete process.env.COMSPEC; - const { getWindowsEscapedCommandShell } = await import( - '../../../src/utils/shell-executor' - ); + const { getWindowsEscapedCommandShell } = await import('../../../src/utils/shell-executor'); expect(getWindowsEscapedCommandShell()).toBe('C:\\Windows\\System32\\cmd.exe'); } finally { if (originalComSpec === undefined) delete process.env.ComSpec; else process.env.ComSpec = originalComSpec; if (originalCOMSPEC === undefined) delete process.env.COMSPEC; else process.env.COMSPEC = originalCOMSPEC; + if (originalSystemRoot === undefined) delete process.env.SystemRoot; + else process.env.SystemRoot = originalSystemRoot; } }); - it('falls back to cmd.exe when ComSpec is unavailable', async () => { + it('rejects relative or non-system ComSpec values', async () => { const originalComSpec = process.env.ComSpec; const originalCOMSPEC = process.env.COMSPEC; + const originalSystemRoot = process.env.SystemRoot; try { - delete process.env.ComSpec; - delete process.env.COMSPEC; - const { getWindowsEscapedCommandShell } = await import( - '../../../src/utils/shell-executor' - ); - expect(getWindowsEscapedCommandShell()).toBe('cmd.exe'); + process.env.SystemRoot = 'C:\\Windows'; + process.env.ComSpec = 'cmd.exe'; + process.env.COMSPEC = 'C:\\Temp\\cmd.exe'; + const { getWindowsEscapedCommandShell } = await import('../../../src/utils/shell-executor'); + expect(getWindowsEscapedCommandShell()).toBe('C:\\Windows\\System32\\cmd.exe'); } finally { if (originalComSpec === undefined) delete process.env.ComSpec; else process.env.ComSpec = originalComSpec; if (originalCOMSPEC === undefined) delete process.env.COMSPEC; else process.env.COMSPEC = originalCOMSPEC; + if (originalSystemRoot === undefined) delete process.env.SystemRoot; + else process.env.SystemRoot = originalSystemRoot; + } + }); + + it('rejects relative Windows root environment values', async () => { + const originalComSpec = process.env.ComSpec; + const originalCOMSPEC = process.env.COMSPEC; + const originalSystemRoot = process.env.SystemRoot; + const originalSYSTEMROOT = process.env.SYSTEMROOT; + const originalWindir = process.env.windir; + + try { + process.env.SystemRoot = 'Windows'; + delete process.env.SYSTEMROOT; + delete process.env.windir; + process.env.ComSpec = 'Windows\\System32\\cmd.exe'; + process.env.COMSPEC = 'C:\\Temp\\cmd.exe'; + const { getWindowsEscapedCommandShell } = await import('../../../src/utils/shell-executor'); + expect(getWindowsEscapedCommandShell()).toBe('C:\\Windows\\System32\\cmd.exe'); + } finally { + if (originalComSpec === undefined) delete process.env.ComSpec; + else process.env.ComSpec = originalComSpec; + if (originalCOMSPEC === undefined) delete process.env.COMSPEC; + else process.env.COMSPEC = originalCOMSPEC; + if (originalSystemRoot === undefined) delete process.env.SystemRoot; + else process.env.SystemRoot = originalSystemRoot; + if (originalSYSTEMROOT === undefined) delete process.env.SYSTEMROOT; + else process.env.SYSTEMROOT = originalSYSTEMROOT; + if (originalWindir === undefined) delete process.env.windir; + else process.env.windir = originalWindir; + } + }); + + it('falls back to the absolute default system cmd.exe when Windows root env is unavailable', async () => { + const originalComSpec = process.env.ComSpec; + const originalCOMSPEC = process.env.COMSPEC; + const originalSystemRoot = process.env.SystemRoot; + const originalSYSTEMROOT = process.env.SYSTEMROOT; + const originalWindir = process.env.windir; + + try { + delete process.env.ComSpec; + delete process.env.COMSPEC; + delete process.env.SystemRoot; + delete process.env.SYSTEMROOT; + delete process.env.windir; + const { getWindowsEscapedCommandShell } = await import('../../../src/utils/shell-executor'); + expect(getWindowsEscapedCommandShell()).toBe('C:\\Windows\\System32\\cmd.exe'); + } finally { + if (originalComSpec === undefined) delete process.env.ComSpec; + else process.env.ComSpec = originalComSpec; + if (originalCOMSPEC === undefined) delete process.env.COMSPEC; + else process.env.COMSPEC = originalCOMSPEC; + if (originalSystemRoot === undefined) delete process.env.SystemRoot; + else process.env.SystemRoot = originalSystemRoot; + if (originalSYSTEMROOT === undefined) delete process.env.SYSTEMROOT; + else process.env.SYSTEMROOT = originalSYSTEMROOT; + if (originalWindir === undefined) delete process.env.windir; + else process.env.windir = originalWindir; } }); });