From 73de6796be1aae91dfe6f481fdc4ec00e6949ffb Mon Sep 17 00:00:00 2001 From: Mathis Debuire <68806646+64ix@users.noreply.github.com> Date: Thu, 6 Aug 2026 11:44:31 +0200 Subject: [PATCH] fix(acp): launch Windows command shims safely --- .../node/child-process-host.test.ts | 79 +++++++++++++++++++ .../src/acp-agents/node/child-process-host.ts | 61 +++++++++++++- 2 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 packages/runtime/src/acp-agents/node/child-process-host.test.ts diff --git a/packages/runtime/src/acp-agents/node/child-process-host.test.ts b/packages/runtime/src/acp-agents/node/child-process-host.test.ts new file mode 100644 index 0000000000..f947084714 --- /dev/null +++ b/packages/runtime/src/acp-agents/node/child-process-host.test.ts @@ -0,0 +1,79 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { ChildAcpProcessHost, resolveChildProcessSpawnSpec } from './child-process-host'; + +const tempDirs: string[] = []; + +afterEach(async () => { + await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); +}); + +describe('resolveChildProcessSpawnSpec', () => { + it('wraps Windows cmd shims without enabling shell mode', () => { + expect( + resolveChildProcessSpawnSpec( + { + command: 'C:\\Users\\Test User\\AppData\\Roaming\\npm\\opencode.cmd', + args: ['acp', 'hello world', 'A&B'], + env: { ComSpec: 'C:\\Windows\\System32\\cmd.exe' }, + cwd: 'C:\\workspace', + }, + 'win32' + ) + ).toEqual({ + command: 'C:\\Windows\\System32\\cmd.exe', + args: [ + '/d', + '/s', + '/c', + '""C:\\Users\\Test User\\AppData\\Roaming\\npm\\opencode.cmd" acp "hello world" "A^&B""', + ], + }); + }); + + it('leaves native executables unchanged', () => { + expect( + resolveChildProcessSpawnSpec( + { + command: 'C:\\tools\\codex.exe', + args: ['app-server'], + env: {}, + cwd: 'C:\\workspace', + }, + 'win32' + ) + ).toEqual({ command: 'C:\\tools\\codex.exe', args: ['app-server'] }); + }); +}); + +describe.runIf(process.platform === 'win32')('ChildAcpProcessHost on Windows', () => { + it('spawns a cmd shim with piped stdio', async () => { + const dir = await mkdtemp(join(tmpdir(), 'emdash-acp-cmd-shim-')); + tempDirs.push(dir); + const shim = join(dir, 'agent.cmd'); + await writeFile(shim, '@echo off\r\necho ready\r\n', 'utf8'); + + const handle = await new ChildAcpProcessHost().spawn({ + command: shim, + args: [], + env: { PATH: process.env.PATH ?? '' }, + cwd: dir, + }); + const output = await new Promise((resolve, reject) => { + let stdout = ''; + handle.stdout.setEncoding('utf8'); + handle.stdout.on('data', (chunk: string) => { + stdout += chunk; + }); + handle.onError(reject); + handle.onExit((code) => { + if (code === 0) resolve(stdout.trim()); + else reject(new Error(`shim exited with ${String(code)}`)); + }); + }); + + expect(output).toBe('ready'); + }); +}); diff --git a/packages/runtime/src/acp-agents/node/child-process-host.ts b/packages/runtime/src/acp-agents/node/child-process-host.ts index 577402d9b0..96221d8d7f 100644 --- a/packages/runtime/src/acp-agents/node/child-process-host.ts +++ b/packages/runtime/src/acp-agents/node/child-process-host.ts @@ -1,6 +1,7 @@ import { spawn } from 'node:child_process'; import { EventEmitter } from 'node:events'; import { mkdir, readFile, writeFile } from 'node:fs/promises'; +import { win32 } from 'node:path'; import type { AcpFs, AcpProcessHandle, @@ -9,6 +10,60 @@ import type { } from '@emdash/core/acp'; import type { AcpRuntimeProcessHost } from '../runtime/types'; +type ChildProcessSpawnSpec = { + command: string; + args: string[]; + env: Record; + cwd: string; +}; + +function getEnvValue(env: Record, key: string): string | undefined { + const normalizedKey = key.toLowerCase(); + const entry = Object.entries(env).find( + ([candidate]) => candidate.toLowerCase() === normalizedKey + ); + return entry?.[1]; +} + +function quoteForCmdExe(input: string): string { + if (input.length === 0) return '""'; + if (!/[\s"^&|<>()%!]/.test(input)) return input; + return `"${input + .replace(/%/g, '%%') + .replace(/!/g, '^!') + .replace(/(["^&|<>()])/g, '^$1')}"`; +} + +function wrapCmdExeCommandLine(commandLine: string): string { + return commandLine.startsWith('"') ? `"${commandLine}"` : commandLine; +} + +/** + * Node cannot spawn Windows .cmd/.bat shims directly. Route only those files through + * cmd.exe and quote each argv token explicitly instead of enabling `shell: true`. + */ +export function resolveChildProcessSpawnSpec( + spec: ChildProcessSpawnSpec, + platform: NodeJS.Platform = process.platform +): Pick { + if (platform !== 'win32') return { command: spec.command, args: spec.args }; + + const extension = win32.extname(spec.command).toLowerCase(); + if (extension !== '.cmd' && extension !== '.bat') { + return { command: spec.command, args: spec.args }; + } + + const commandLine = [spec.command, ...spec.args].map(quoteForCmdExe).join(' '); + const command = + getEnvValue(spec.env, 'ComSpec') ?? + getEnvValue(process.env, 'ComSpec') ?? + 'C:\\Windows\\System32\\cmd.exe'; + return { + command, + args: ['/d', '/s', '/c', wrapCmdExeCommandLine(commandLine)], + }; +} + class ChildProcessHandle implements AcpProcessHandle { constructor(private readonly child: ReturnType) {} @@ -96,7 +151,8 @@ export class ChildAcpProcessHost implements AcpRuntimeProcessHost { env: Record; cwd: string; }): Promise { - const child = spawn(spec.command, spec.args, { + const resolved = resolveChildProcessSpawnSpec(spec); + const child = spawn(resolved.command, resolved.args, { cwd: spec.cwd, env: spec.env, stdio: ['pipe', 'pipe', 'pipe'], @@ -113,7 +169,8 @@ export class ChildAcpProcessHost implements AcpRuntimeProcessHost { env: Record; cwd: string; }): Promise { - const child = spawn(spec.command, spec.args, { + const resolved = resolveChildProcessSpawnSpec(spec); + const child = spawn(resolved.command, resolved.args, { cwd: spec.cwd, env: spec.env, stdio: ['ignore', 'pipe', 'pipe'],