diff --git a/bin/cli/commands/launch.mjs b/bin/cli/commands/launch.mjs index b737786541..f7cff8cb2a 100644 --- a/bin/cli/commands/launch.mjs +++ b/bin/cli/commands/launch.mjs @@ -90,6 +90,60 @@ export function resolveLaunchTarget(opts = {}) { return { baseUrl, authToken }; } +/** + * #8246: on Windows, npm installs claude as a `.cmd` shim — spawn() without a + * shell cannot resolve PATHEXT shims (and Node refuses to exec `.cmd` directly + * since CVE-2024-27980), so the Windows path must go through cmd.exe. + * + * @param {NodeJS.Platform|string} platform + * @returns {{ command: string, shell: true|undefined }} + */ +export function resolveClaudeSpawn(platform) { + return platform === "win32" + ? { command: "claude.cmd", shell: true } + : { command: "claude", shell: undefined }; +} + +/** cmd.exe metacharacters that stay live inside a quoted argument. */ +const WIN_META_CHARS = /([()\][%!^"`<>&|;, *?])/g; + +/** + * Escape one argument for a cmd.exe command line built by `shell: true`. + * + * Two layers, in order: + * 1. the CRT argv rules the target binary parses (double the backslashes that + * precede a quote, escape embedded quotes, wrap in quotes); + * 2. cmd.exe's metacharacters, caret-escaped — applied TWICE because the + * target is an npm `.cmd` shim that forwards `%*` to node, so the line is + * parsed by cmd a second time. Single-escaping truncated any argument at + * the first `&` or `|`. (Same rule as cross-spawn's doubleEscapeMetaChars.) + * + * @param {unknown} arg + * @returns {string} + */ +function escapeWindowsShellArg(arg) { + const s = String(arg); + if (s === "") return '""'; + let out = s.replace(/(\\*)"/g, '$1$1\\"').replace(/(\\*)$/, "$1$1"); + out = `"${out}"`; + return out.replace(WIN_META_CHARS, "^$1").replace(WIN_META_CHARS, "^$1"); +} + +/** + * `shell: true` makes Node join argv with plain spaces and no escaping (the + * DEP0190 warning), so `-p "two words"` used to reach claude as `-p two` plus + * three stray positional arguments. Quote the args ourselves on that path. + * Off Windows there is no shell, so argv is passed through untouched. + * + * @param {string[]} args + * @param {NodeJS.Platform|string} platform + * @returns {string[]} + */ +export function quoteClaudeArgs(args, platform) { + const list = [...(args ?? [])]; + return platform === "win32" ? list.map(escapeWindowsShellArg) : list; +} + /** * @param {{port?:string, remote?:string, token?:string, apiKey?:string, profile?:string, claudeHome?:string}} opts * @param {string[]} claudeArgs pass-through args for the claude binary @@ -120,13 +174,12 @@ export async function runLaunchCommand(opts = {}, claudeArgs = []) { const env = buildClaudeEnv(process.env, baseUrl, authToken, { configDir }); return await new Promise((resolve) => { - // #8246: on Windows, npm installs claude as a .cmd shim — spawn() without - // shell:true cannot resolve PATHEXT shims and fails with ENOENT. - const claudeCommand = process.platform === "win32" ? "claude.cmd" : "claude"; - const child = spawn(claudeCommand, claudeArgs, { + const { command, shell } = resolveClaudeSpawn(process.platform); + const child = spawn(command, quoteClaudeArgs(claudeArgs, process.platform), { env, stdio: "inherit", - ...(process.platform === "win32" ? { shell: true, windowsHide: true } : {}), + shell, + ...(process.platform === "win32" ? { windowsHide: true } : {}), }); child.on("error", (err) => { if (err && err.code === "ENOENT") { @@ -159,7 +212,10 @@ export function registerLaunch(program) { .allowExcessArguments(true) .argument("[claudeArgs...]", "arguments passed through to the claude binary") .action(async (claudeArgs, opts) => { - const exitCode = await runLaunchCommand(opts, claudeArgs ?? []); - if (exitCode !== 0) process.exit(exitCode); + // process.exit() here aborted the process with a libuv assertion on + // Windows (`!(handle->flags & UV_HANDLE_CLOSING)`, async.c:94): it tears + // the loop down while the inherited stdio handles of the just-exited + // child are still closing. Setting exitCode lets the loop drain first. + process.exitCode = await runLaunchCommand(opts, claudeArgs ?? []); }); } diff --git a/changelog.d/fixes/8837-cli-launch-windows-spawn.md b/changelog.d/fixes/8837-cli-launch-windows-spawn.md new file mode 100644 index 0000000000..7381ce340e --- /dev/null +++ b/changelog.d/fixes/8837-cli-launch-windows-spawn.md @@ -0,0 +1 @@ +- **CLI**: `omniroute launch` no longer truncates pass-through arguments at the first space on Windows — each argument is escaped for cmd.exe (CRT argv rules plus a double caret pass, required because `claude.cmd` re-parses `%*`). A non-zero child exit also stops aborting the process with a libuv assertion: the exit code is set on `process.exitCode` so the loop drains the inherited stdio handles first diff --git a/tests/unit/cli/launch-windows-spawn-args.test.ts b/tests/unit/cli/launch-windows-spawn-args.test.ts new file mode 100644 index 0000000000..52ef3a0629 --- /dev/null +++ b/tests/unit/cli/launch-windows-spawn-args.test.ts @@ -0,0 +1,124 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; + +import { quoteClaudeArgs, resolveClaudeSpawn } from "../../../bin/cli/commands/launch.mjs"; + +const isWindows = process.platform === "win32"; + +// Regression guard for #8246: on Windows the `claude` binary is an npm `.cmd` +// shim that spawn() cannot resolve without a shell (bare "claude" -> ENOENT). +test("resolveClaudeSpawn: win32 spawns claude.cmd through a shell", () => { + const { command, shell } = resolveClaudeSpawn("win32"); + assert.equal(command, "claude.cmd"); + assert.equal(shell, true); +}); + +test("resolveClaudeSpawn: non-Windows platforms spawn the bare binary without a shell", () => { + for (const platform of ["linux", "darwin", "freebsd"]) { + const { command, shell } = resolveClaudeSpawn(platform); + assert.equal(command, "claude", `${platform} command`); + assert.equal(shell, undefined, `${platform} shell`); + } +}); + +// Regression guard: `shell: true` makes Node concatenate argv unescaped +// (DEP0190), so `-p "two words"` reached claude as `-p two` and the rest of the +// prompt was parsed as separate arguments. +test("quoteClaudeArgs leaves argv untouched off Windows (no shell, no quoting)", () => { + const args = ["-p", "two words", "--model", "sonnet"]; + assert.deepEqual(quoteClaudeArgs(args, "linux"), args); +}); + +test("quoteClaudeArgs escapes every argument on win32", () => { + // cmd.exe parses the whole line, so each argument is quoted — not just the + // ones containing spaces. The exact encoding is asserted by the round-trip + // test below; here we only pin that nothing is passed through raw. + const input = ["-p", "two words", "--profile", "auto-best-coding"]; + const quoted = quoteClaudeArgs(input, "win32"); + assert.equal(quoted.length, input.length); + for (const [i, arg] of quoted.entries()) { + assert.notEqual(arg, input[i], `argument ${i} must be escaped`); + assert.match(arg, /"/, `argument ${i} must be quoted`); + } +}); + +// The cmd.exe round-trip below is the real proof, but it can only run on +// Windows. These golden strings pin the exact encoding so CI (Linux) still +// fails if the escaping changes — e.g. if the double caret-escape required by +// the `.cmd` shim's %* re-parse is ever reduced back to a single pass. +test("quoteClaudeArgs: exact win32 encoding (golden)", () => { + const golden: Array<[string, string]> = [ + ["-p", '^^^"-p^^^"'], + ["auto-best-coding", '^^^"auto-best-coding^^^"'], + ["two words", '^^^"two^^^ words^^^"'], + ["a & b", '^^^"a^^^ ^^^&^^^ b^^^"'], + ['q"uote', '^^^"q\\^^^"uote^^^"'], + ["trail\\", '^^^"trail\\\\^^^"'], + ["%PATH%", '^^^"^^^%PATH^^^%^^^"'], + ["", '""'], + ]; + for (const [input, expected] of golden) { + assert.equal(quoteClaudeArgs([input], "win32")[0], expected, `encoding of ${JSON.stringify(input)}`); + } +}); + +test("quoteClaudeArgs does not mutate the caller's array", () => { + const input = ["-p", "two words"]; + quoteClaudeArgs(input, "win32"); + assert.deepEqual(input, ["-p", "two words"]); +}); + +// The real contract: whatever we hand to spawn(shell:true) must arrive at the +// child's argv byte-identical. Verified against a probe .cmd through the same +// cmd.exe path the launcher uses. +test( + "quoteClaudeArgs survives a real cmd.exe round-trip", + { skip: isWindows ? false : "windows-only: exercises the cmd.exe shell path" }, + async () => { + const dir = mkdtempSync(join(tmpdir(), "omniroute-argv-")); + try { + // Mirror the real shape of `claude.cmd`: an npm .cmd shim forwarding %* + // to a node script. Printing argv as JSON keeps the oracle exact. + writeFileSync(join(dir, "argv.mjs"), "console.log(JSON.stringify(process.argv.slice(2)));\n"); + const probe = join(dir, "probe.cmd"); + writeFileSync(probe, ['@echo off', 'node "%~dp0argv.mjs" %*'].join("\r\n") + "\r\n"); + + const args = [ + "-p", + "In one short line: say BANANA", + "--append-system-prompt", + 'quotes " and & ampersands | pipes', + "percent %PATH% and caret ^ and bang !", + "trailing backslash \\", + "", + "--profile", + "auto-best-coding", + ]; + + const received = await new Promise((resolve, reject) => { + const child = spawn(probe, quoteClaudeArgs(args, "win32"), { + shell: true, + windowsHide: true, + }); + let out = ""; + child.stdout.on("data", (c) => (out += c)); + child.on("error", reject); + child.on("exit", () => { + try { + resolve(JSON.parse(out.trim().split(/\r?\n/).pop() ?? "[]")); + } catch (err) { + reject(new Error(`probe did not emit argv JSON: ${out}`, { cause: err })); + } + }); + }); + + assert.deepEqual(received, args, "child argv must match what the caller passed"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + } +);