fix(security): harden Windows child process spawning

This commit is contained in:
Peter Steinberger
2026-02-15 03:24:21 +01:00
parent 7b697d6128
commit a7eb0dd9a5
7 changed files with 29 additions and 9 deletions

View File

@@ -1,7 +1,16 @@
import { describe, expect, it } from "vitest";
import { runCommandWithTimeout } from "./exec.js";
import { runCommandWithTimeout, shouldSpawnWithShell } from "./exec.js";
describe("runCommandWithTimeout", () => {
it("never enables shell execution (Windows cmd.exe injection hardening)", () => {
expect(
shouldSpawnWithShell({
resolvedCommand: "npm.cmd",
platform: "win32",
}),
).toBe(false);
});
it("passes env overrides to child", async () => {
const result = await runCommandWithTimeout(
[process.execPath, "-e", 'process.stdout.write(process.env.OPENCLAW_TEST_ENV ?? "")'],

View File

@@ -29,6 +29,19 @@ function resolveCommand(command: string): string {
return command;
}
export function shouldSpawnWithShell(params: {
resolvedCommand: string;
platform: NodeJS.Platform;
}): boolean {
// SECURITY: never enable `shell` for argv-based execution.
// `shell` routes through cmd.exe on Windows, which turns untrusted argv values
// (like chat prompts passed as CLI args) into command-injection primitives.
// If you need a shell, use an explicit shell-wrapper argv (e.g. `cmd.exe /c ...`)
// and validate/escape at the call site.
void params;
return false;
}
// Simple promise-wrapped execFile with optional verbosity logging.
export async function runExec(
command: string,
@@ -117,14 +130,14 @@ export async function runCommandWithTimeout(
const stdio = resolveCommandStdio({ hasInput, preferInherit: true });
const resolvedCommand = resolveCommand(argv[0] ?? "");
const commandExt = path.extname(resolvedCommand).toLowerCase();
const useShell = process.platform === "win32" && commandExt !== ".exe";
const child = spawn(resolvedCommand, argv.slice(1), {
stdio,
cwd,
env: resolvedEnv,
windowsVerbatimArguments,
shell: useShell,
...(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) => {