From 6e008e93beeb6ece160eca88d0f0c612bba2e792 Mon Sep 17 00:00:00 2001 From: icesword0760 <25030760@qq.com> Date: Mon, 2 Mar 2026 21:49:54 +0800 Subject: [PATCH] Process: fix Windows .cmd spawn EINVAL (openclaw#29759) thanks @icesword0760 Verified: - pnpm vitest run src/process/exec.test.ts src/process/exec.windows.test.ts Co-authored-by: icesword0760 <123886211+icesword0760@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> --- src/process/exec.ts | 63 ++++++++++++++++--- src/process/exec.windows.test.ts | 104 +++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 10 deletions(-) create mode 100644 src/process/exec.windows.test.ts diff --git a/src/process/exec.ts b/src/process/exec.ts index f27889985a3..d3801e486af 100644 --- a/src/process/exec.ts +++ b/src/process/exec.ts @@ -9,6 +9,35 @@ import { resolveCommandStdio } from "./spawn-utils.js"; const execFileAsync = promisify(execFile); +const WINDOWS_UNSAFE_CMD_CHARS_RE = /[&|<>^%\r\n]/; + +function isWindowsBatchCommand(resolvedCommand: string): boolean { + if (process.platform !== "win32") { + return false; + } + const ext = path.extname(resolvedCommand).toLowerCase(); + return ext === ".cmd" || ext === ".bat"; +} + +function escapeForCmdExe(arg: string): string { + // Reject cmd metacharacters to avoid injection when we must pass a single command line. + if (WINDOWS_UNSAFE_CMD_CHARS_RE.test(arg)) { + throw new Error( + `Unsafe Windows cmd.exe argument detected: ${JSON.stringify(arg)}. ` + + "Pass an explicit shell-wrapper argv at the call site instead.", + ); + } + // Quote when needed; double inner quotes for cmd parsing. + if (!arg.includes(" ") && !arg.includes('"')) { + return arg; + } + return `"${arg.replace(/"/g, '""')}"`; +} + +function buildCmdExeCommandLine(resolvedCommand: string, args: string[]): string { + return [escapeForCmdExe(resolvedCommand), ...args.map(escapeForCmdExe)].join(" "); +} + /** * On Windows, Node 18.20.2+ (CVE-2024-27980) rejects spawning .cmd/.bat directly * without shell, causing EINVAL. Resolve npm/npx to node + cli script so we @@ -100,7 +129,14 @@ export async function runExec( execCommand = resolveCommand(command); execArgs = args; } - const { stdout, stderr } = await execFileAsync(execCommand, execArgs, options); + const useCmdWrapper = isWindowsBatchCommand(execCommand); + const { stdout, stderr } = useCmdWrapper + ? await execFileAsync( + process.env.ComSpec ?? "cmd.exe", + ["/d", "/s", "/c", buildCmdExeCommandLine(execCommand, execArgs)], + { ...options, windowsVerbatimArguments: true }, + ) + : await execFileAsync(execCommand, execArgs, options); if (shouldLogVerbose()) { if (stdout.trim()) { logDebug(stdout.trim()); @@ -178,15 +214,22 @@ export async function runCommandWithTimeout( const stdio = resolveCommandStdio({ hasInput, preferInherit: true }); const finalArgv = process.platform === "win32" ? (resolveNpmArgvForWindows(argv) ?? argv) : argv; const resolvedCommand = finalArgv !== argv ? (finalArgv[0] ?? "") : resolveCommand(argv[0] ?? ""); - const child = spawn(resolvedCommand, finalArgv.slice(1), { - stdio, - cwd, - env: resolvedEnv, - windowsVerbatimArguments, - ...(shouldSpawnWithShell({ resolvedCommand, platform: process.platform }) - ? { shell: true } - : {}), - }); + const useCmdWrapper = isWindowsBatchCommand(resolvedCommand); + const child = spawn( + useCmdWrapper ? (process.env.ComSpec ?? "cmd.exe") : resolvedCommand, + useCmdWrapper + ? ["/d", "/s", "/c", buildCmdExeCommandLine(resolvedCommand, finalArgv.slice(1))] + : finalArgv.slice(1), + { + stdio, + cwd, + env: resolvedEnv, + windowsVerbatimArguments: useCmdWrapper ? true : windowsVerbatimArguments, + ...(shouldSpawnWithShell({ resolvedCommand, platform: process.platform }) + ? { shell: true } + : {}), + }, + ); // Spawn with inherited stdin (TTY) so tools like `pi` stay interactive when needed. return await new Promise((resolve, reject) => { let stdout = ""; diff --git a/src/process/exec.windows.test.ts b/src/process/exec.windows.test.ts new file mode 100644 index 00000000000..8b171ffebf0 --- /dev/null +++ b/src/process/exec.windows.test.ts @@ -0,0 +1,104 @@ +import { EventEmitter } from "node:events"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +const spawnMock = vi.hoisted(() => vi.fn()); +const execFileMock = vi.hoisted(() => vi.fn()); + +vi.mock("node:child_process", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + spawn: spawnMock, + execFile: execFileMock, + }; +}); + +import { runCommandWithTimeout, runExec } from "./exec.js"; + +type MockChild = EventEmitter & { + stdout: EventEmitter; + stderr: EventEmitter; + stdin: { write: ReturnType; end: ReturnType }; + kill: ReturnType; + pid?: number; + killed?: boolean; +}; + +function createMockChild(params?: { code?: number; signal?: NodeJS.Signals | null }): MockChild { + const child = new EventEmitter() as MockChild; + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.stdin = { + write: vi.fn(), + end: vi.fn(), + }; + child.kill = vi.fn(() => true); + child.pid = 1234; + child.killed = false; + queueMicrotask(() => { + child.emit("close", params?.code ?? 0, params?.signal ?? null); + }); + return child; +} + +describe("windows command wrapper behavior", () => { + afterEach(() => { + spawnMock.mockReset(); + execFileMock.mockReset(); + vi.restoreAllMocks(); + }); + + it("wraps .cmd commands via cmd.exe in runCommandWithTimeout", async () => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const expectedComSpec = process.env.ComSpec ?? "cmd.exe"; + let captured: { command: string; args: string[]; options: Record } | null = + null; + + spawnMock.mockImplementation( + (command: string, args: string[], options: Record) => { + captured = { command, args, options }; + return createMockChild(); + }, + ); + + try { + const result = await runCommandWithTimeout(["pnpm", "--version"], { timeoutMs: 1000 }); + expect(result.code).toBe(0); + expect(captured?.command).toBe(expectedComSpec); + expect(captured?.args.slice(0, 3)).toEqual(["/d", "/s", "/c"]); + expect(captured?.args[3]).toContain("pnpm.cmd --version"); + expect(captured?.options.windowsVerbatimArguments).toBe(true); + } finally { + platformSpy.mockRestore(); + } + }); + + it("uses cmd.exe wrapper with windowsVerbatimArguments in runExec for .cmd shims", async () => { + const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32"); + const expectedComSpec = process.env.ComSpec ?? "cmd.exe"; + let captured: { command: string; args: string[]; options: Record } | null = + null; + + execFileMock.mockImplementation( + ( + command: string, + args: string[], + options: Record, + cb: (err: Error | null, stdout: string, stderr: string) => void, + ) => { + captured = { command, args, options }; + cb(null, "ok", ""); + }, + ); + + try { + await runExec("pnpm", ["--version"], 1000); + expect(captured?.command).toBe(expectedComSpec); + expect(captured?.args.slice(0, 3)).toEqual(["/d", "/s", "/c"]); + expect(captured?.args[3]).toContain("pnpm.cmd --version"); + expect(captured?.options.windowsVerbatimArguments).toBe(true); + } finally { + platformSpy.mockRestore(); + } + }); +});