diff --git a/src/auth/commands/create-command.ts b/src/auth/commands/create-command.ts index ef59fb60..c70b2c6c 100644 --- a/src/auth/commands/create-command.ts +++ b/src/auth/commands/create-command.ts @@ -7,7 +7,11 @@ import { spawn, ChildProcess } from 'child_process'; import { initUI, header, color, fail, warn, info, infoBox, warnBox } from '../../utils/ui'; import { getClaudeCliInfo } from '../../utils/claude-detector'; -import { escapeShellArg, stripClaudeCodeEnv } from '../../utils/shell-executor'; +import { + escapeShellArg, + getWindowsEscapedCommandShell, + stripClaudeCodeEnv, +} from '../../utils/shell-executor'; import { isUnifiedMode } from '../../config/unified-config-loader'; import { ProfileMetadata } from '../../types'; import { @@ -249,7 +253,7 @@ export async function handleCreate(ctx: CommandContext, args: string[]): Promise child = spawn(cmdString, { stdio: 'inherit', windowsHide: true, - shell: true, + shell: getWindowsEscapedCommandShell(), env: childEnv, }); } else { diff --git a/src/cliproxy/executor/index.ts b/src/cliproxy/executor/index.ts index d02a12f5..3f6d1d11 100644 --- a/src/cliproxy/executor/index.ts +++ b/src/cliproxy/executor/index.ts @@ -17,7 +17,7 @@ import * as path from 'path'; import { ProgressIndicator } from '../../utils/progress-indicator'; import { ok, fail, info, warn } from '../../utils/ui'; import { getCcsDir } from '../../utils/config-manager'; -import { escapeShellArg } from '../../utils/shell-executor'; +import { escapeShellArg, getWindowsEscapedCommandShell } from '../../utils/shell-executor'; import { ensureCLIProxyBinary } from '../binary-manager'; import { generateConfig, @@ -1316,7 +1316,7 @@ export async function execClaudeWithCLIProxy( claude = spawn(cmdString, { stdio: 'inherit', windowsHide: true, - shell: true, + shell: getWindowsEscapedCommandShell(), env: tracedEnv, }); } else { diff --git a/src/utils/claude-spawner.ts b/src/utils/claude-spawner.ts index 9f82ff8b..67f9a527 100644 --- a/src/utils/claude-spawner.ts +++ b/src/utils/claude-spawner.ts @@ -6,7 +6,11 @@ */ import { spawn, ChildProcess, SpawnOptions } from 'child_process'; -import { escapeShellArg, stripClaudeCodeEnv } from './shell-executor'; +import { + escapeShellArg, + getWindowsEscapedCommandShell, + stripClaudeCodeEnv, +} from './shell-executor'; import { getClaudeCliInfo } from './claude-detector'; import { ErrorManager } from './error-manager'; @@ -56,7 +60,7 @@ export function spawnClaude(options: SpawnClaudeOptions = {}): SpawnClaudeResult child = spawn(cmdString, { stdio, windowsHide: true, - shell: true, + shell: getWindowsEscapedCommandShell(), env: mergedEnv, cwd, }); diff --git a/src/utils/shell-executor.ts b/src/utils/shell-executor.ts index 191c004a..b6f26ab2 100644 --- a/src/utils/shell-executor.ts +++ b/src/utils/shell-executor.ts @@ -4,7 +4,7 @@ * Cross-platform shell execution utilities for CCS. */ -import { spawn, spawnSync, ChildProcess } from 'child_process'; +import { spawn, spawnSync, ChildProcess, type SpawnOptions } from 'child_process'; import { ErrorManager } from './error-manager'; import { getWebSearchHookEnv } from './websearch-manager'; import { wireChildProcessSignals } from './signal-forwarder'; @@ -108,13 +108,17 @@ export function escapeShellArg(arg: string): string { } /** - * Return the Windows shell that matches escapeShellArg() quoting semantics. + * Return the shell that matches escapeShellArg() quoting semantics. * - * `shell: true` is not strict enough for npm `.cmd` wrappers because Node may - * route through a different quoting path than the one escapeShellArg() expects. + * On Windows, prefer ComSpec over a bare `cmd.exe` so escaped wrapper launches + * keep the same shell contract without depending on PATH lookup. */ -export function getWindowsEscapedCommandShell(): string { - return 'cmd.exe'; +export function getWindowsEscapedCommandShell(): SpawnOptions['shell'] { + if (process.platform !== 'win32') { + return true; + } + + return process.env.ComSpec || process.env.COMSPEC || 'cmd.exe'; } /** diff --git a/tests/unit/targets/codex-adapter-exec.test.ts b/tests/unit/targets/codex-adapter-exec.test.ts index cf8017c8..d5166e03 100644 --- a/tests/unit/targets/codex-adapter-exec.test.ts +++ b/tests/unit/targets/codex-adapter-exec.test.ts @@ -1,16 +1,13 @@ -import { afterEach, beforeEach, describe, expect, it, mock } from 'bun:test'; +import { afterEach, beforeEach, 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'; -const spawnCalls: Array<{ - command: string; - args: string[]; - options: Record | undefined; -}> = []; -const originalPlatform = process.platform; +import { CodexAdapter } from '../../../src/targets/codex-adapter'; +import { buildCodexBrowserMcpOverrides } from '../../../src/utils/browser-codex-overrides'; +import * as signalForwarder from '../../../src/utils/signal-forwarder'; function createMockChild(): EventEmitter & { stdout: EventEmitter; @@ -46,34 +43,12 @@ function createMockChild(): EventEmitter & { return child; } -mock.module('child_process', () => ({ - ...childProcess, - spawn: (...spawnArgs: unknown[]) => { - const command = String(spawnArgs[0] ?? ''); - const maybeArgs = spawnArgs[1]; - const args = Array.isArray(maybeArgs) ? (maybeArgs as string[]) : []; - const options = (Array.isArray(maybeArgs) ? spawnArgs[2] : spawnArgs[1]) as - | Record - | undefined; - - spawnCalls.push({ command, args, options }); - return createMockChild(); - }, -})); - -mock.module('../../../src/utils/signal-forwarder', () => ({ - wireChildProcessSignals: () => {}, -})); - -import { CodexAdapter } from '../../../src/targets/codex-adapter'; -import { buildCodexBrowserMcpOverrides } from '../../../src/utils/browser-codex-overrides'; - describe('codex-adapter exec', () => { + const originalPlatform = process.platform; let tmpDir: string; beforeEach(() => { tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-codex-adapter-exec-')); - spawnCalls.length = 0; }); afterEach(() => { @@ -87,29 +62,45 @@ describe('codex-adapter exec', () => { const fakeCodex = path.join(tmpDir, 'codex.cmd'); fs.writeFileSync(fakeCodex, ''); - const adapter = new CodexAdapter(); - const binaryInfo = { - path: fakeCodex, - needsShell: true, - features: ['config-overrides'], - }; - const args = adapter.buildArgs('default', ['--version'], { - profileType: 'default', - creds: { - profile: 'default', - baseUrl: '', - apiKey: '', - runtimeConfigOverrides: buildCodexBrowserMcpOverrides(), - }, - binaryInfo, - }); + const spawnSpy = spyOn(childProcess, 'spawn').mockImplementation( + () => createMockChild() as unknown as ReturnType + ); + const signalSpy = spyOn(signalForwarder, 'wireChildProcessSignals').mockImplementation( + () => undefined + ); - adapter.exec(args, {}, { binaryInfo }); + try { + const adapter = new CodexAdapter(); + const binaryInfo = { + path: fakeCodex, + needsShell: true, + features: ['config-overrides'], + }; + const args = adapter.buildArgs('default', ['--version'], { + profileType: 'default', + creds: { + profile: 'default', + baseUrl: '', + apiKey: '', + runtimeConfigOverrides: buildCodexBrowserMcpOverrides(), + }, + binaryInfo, + }); - expect(spawnCalls).toHaveLength(1); - expect(spawnCalls[0]?.options?.shell).toBe('cmd.exe'); - expect(spawnCalls[0]?.command).toContain(fakeCodex); - expect(spawnCalls[0]?.command).toContain('mcp_servers.ccs_browser.args='); - expect(spawnCalls[0]?.command).toContain('@playwright/mcp@0.0.70'); + adapter.exec(args, {}, { binaryInfo }); + + expect(spawnSpy).toHaveBeenCalledTimes(1); + const [command, options] = spawnSpy.mock.calls[0] as [ + string, + Record | undefined, + ]; + expect(options?.shell).toBe('cmd.exe'); + expect(command).toContain(fakeCodex); + expect(command).toContain('mcp_servers.ccs_browser.args='); + expect(command).toContain('@playwright/mcp@0.0.70'); + } finally { + spawnSpy.mockRestore(); + signalSpy.mockRestore(); + } }); }); diff --git a/tests/unit/utils/shell-executor.test.ts b/tests/unit/utils/shell-executor.test.ts index 78d7e274..f3b7e600 100644 --- a/tests/unit/utils/shell-executor.test.ts +++ b/tests/unit/utils/shell-executor.test.ts @@ -81,6 +81,56 @@ describe('escapeShellArg', () => { const { escapeShellArg } = await import('../../../src/utils/shell-executor'); expect(escapeShellArg('hello!')).toBe('"hello^^!"'); }); + + it('prefers ComSpec when resolving the escaped command shell', async () => { + const originalComSpec = process.env.ComSpec; + const originalCOMSPEC = process.env.COMSPEC; + + try { + process.env.ComSpec = 'C:\\Windows\\System32\\cmd.exe'; + delete process.env.COMSPEC; + 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; + } + }); + + it('falls back to cmd.exe when ComSpec is unavailable', async () => { + const originalComSpec = process.env.ComSpec; + const originalCOMSPEC = process.env.COMSPEC; + + try { + delete process.env.ComSpec; + delete process.env.COMSPEC; + const { getWindowsEscapedCommandShell } = await import( + '../../../src/utils/shell-executor' + ); + expect(getWindowsEscapedCommandShell()).toBe('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; + } + }); + }); +}); + +describe('getWindowsEscapedCommandShell', () => { + afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + }); + + it('returns shell=true outside Windows if called defensively', async () => { + Object.defineProperty(process, 'platform', { value: 'linux' }); + const { getWindowsEscapedCommandShell } = await import('../../../src/utils/shell-executor'); + expect(getWindowsEscapedCommandShell()).toBe(true); }); });