diff --git a/CHANGELOG.md b/CHANGELOG.md index e787efb34ba..31c1f6a7966 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Security/Windows: avoid shell invocation when spawning child processes to prevent cmd.exe metacharacter injection via untrusted CLI arguments (e.g. agent prompt text). - Agents: deliver tool result media (screenshots, images, audio) to channels regardless of verbose level. (#11735) Thanks @strelov1. - Telegram: when `channels.telegram.commands.native` is `false`, exclude plugin commands from `setMyCommands` menu registration while keeping plugin slash handlers callable. (#15132) Thanks @Glucksberg. - LINE: return 200 OK for Developers Console "Verify" requests (`{"events":[]}`) without `X-Line-Signature`, while still requiring signatures for real deliveries. (#16582) Thanks @arosstale. diff --git a/scripts/test-parallel.mjs b/scripts/test-parallel.mjs index 140a4c6c927..4f23a556da7 100644 --- a/scripts/test-parallel.mjs +++ b/scripts/test-parallel.mjs @@ -223,7 +223,6 @@ const runOnce = (entry, extraArgs = []) => const child = spawn(pnpm, args, { stdio: "inherit", env: { ...process.env, VITEST_GROUP: entry.name, NODE_OPTIONS: nextNodeOptions }, - shell: process.platform === "win32", }); children.add(child); child.on("exit", (code, signal) => { @@ -277,7 +276,6 @@ if (passthroughArgs.length > 0) { const child = spawn(pnpm, args, { stdio: "inherit", env: { ...process.env, NODE_OPTIONS: nextNodeOptions }, - shell: process.platform === "win32", }); children.add(child); child.on("exit", (exitCode, signal) => { diff --git a/scripts/ui.js b/scripts/ui.js index 66c1ffe1468..5f6c753f4e2 100644 --- a/scripts/ui.js +++ b/scripts/ui.js @@ -55,7 +55,6 @@ function run(cmd, args) { cwd: uiDir, stdio: "inherit", env: process.env, - shell: process.platform === "win32", }); child.on("exit", (code, signal) => { if (signal) { @@ -70,7 +69,6 @@ function runSync(cmd, args, envOverride) { cwd: uiDir, stdio: "inherit", env: envOverride ?? process.env, - shell: process.platform === "win32", }); if (result.signal) { process.exit(1); diff --git a/src/daemon/launchd.ts b/src/daemon/launchd.ts index 56da87df332..85d632d106b 100644 --- a/src/daemon/launchd.ts +++ b/src/daemon/launchd.ts @@ -107,7 +107,6 @@ async function execLaunchctl( try { const { stdout, stderr } = await execFileAsync("launchctl", args, { encoding: "utf8", - shell: process.platform === "win32", }); return { stdout: String(stdout ?? ""), diff --git a/src/process/exec.test.ts b/src/process/exec.test.ts index ae8a865ad18..7e1dd1f0232 100644 --- a/src/process/exec.test.ts +++ b/src/process/exec.test.ts @@ -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 ?? "")'], diff --git a/src/process/exec.ts b/src/process/exec.ts index 64d217c1034..0cbd77263ac 100644 --- a/src/process/exec.ts +++ b/src/process/exec.ts @@ -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) => { diff --git a/src/tui/tui-local-shell.ts b/src/tui/tui-local-shell.ts index 436a833ccf5..58f01ba78f3 100644 --- a/src/tui/tui-local-shell.ts +++ b/src/tui/tui-local-shell.ts @@ -107,6 +107,8 @@ export function createLocalShellRunner(deps: LocalShellDeps) { await new Promise((resolve) => { const child = spawnCommand(cmd, { + // Intentionally a shell: this is an operator-only local TUI feature (prefixed with `!`) + // and is gated behind an explicit in-session approval prompt. shell: true, cwd: getCwd(), env,