mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
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>
This commit is contained in:
@@ -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 = "";
|
||||
|
||||
104
src/process/exec.windows.test.ts
Normal file
104
src/process/exec.windows.test.ts
Normal file
@@ -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<typeof import("node:child_process")>();
|
||||
return {
|
||||
...actual,
|
||||
spawn: spawnMock,
|
||||
execFile: execFileMock,
|
||||
};
|
||||
});
|
||||
|
||||
import { runCommandWithTimeout, runExec } from "./exec.js";
|
||||
|
||||
type MockChild = EventEmitter & {
|
||||
stdout: EventEmitter;
|
||||
stderr: EventEmitter;
|
||||
stdin: { write: ReturnType<typeof vi.fn>; end: ReturnType<typeof vi.fn> };
|
||||
kill: ReturnType<typeof vi.fn>;
|
||||
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<string, unknown> } | null =
|
||||
null;
|
||||
|
||||
spawnMock.mockImplementation(
|
||||
(command: string, args: string[], options: Record<string, unknown>) => {
|
||||
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<string, unknown> } | null =
|
||||
null;
|
||||
|
||||
execFileMock.mockImplementation(
|
||||
(
|
||||
command: string,
|
||||
args: string[],
|
||||
options: Record<string, unknown>,
|
||||
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();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user