From 9e6e7a3d69958da000df3690c7cbb186bb1825e8 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 1 Mar 2026 23:44:31 +0000 Subject: [PATCH] fix(acpx): harden windows cmd wrapper spawning --- CHANGELOG.md | 1 + .../src/runtime-internals/process.test.ts | 146 ++++++++++++++++++ .../acpx/src/runtime-internals/process.ts | 129 +++++++++++++++- 3 files changed, 268 insertions(+), 8 deletions(-) create mode 100644 extensions/acpx/src/runtime-internals/process.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ab5b828ec1..03bd82a8ec3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,7 @@ Docs: https://docs.openclaw.ai - ACP/Harness thread spawn routing: force ACP harness thread creation through `sessions_spawn` (`runtime: "acp"`, `thread: true`) and explicitly forbid `message action=thread-create` for ACP harness requests, avoiding misrouted `Unknown channel` errors. (#30957) Thanks @dutifulbob. - Docs/ACP permissions: document the correct `permissionMode` default (`approve-reads`) and clarify non-interactive permission failure behavior/troubleshooting guidance. (#31044) Thanks @barronlroth. - Security/Logging utility hardening: remove `eval`-based command execution from `scripts/clawlog.sh`, switch to argv-safe command construction, and escape predicate literals for user-supplied search/category filters to block local command/predicate injection paths. +- Security/ACPX Windows spawn hardening: resolve `.cmd/.bat` wrappers via PATH/PATHEXT and execute unwrapped Node/EXE entrypoints without shell parsing when possible, while preserving shell fallback for unknown custom wrappers to keep compatibility. - Security/Inbound metadata stripping: tighten sentinel matching and JSON-fence validation for inbound metadata stripping so user-authored lookalike lines no longer trigger unintended metadata removal. - Channels/Command parsing parity: align command-body parsing fields with channel command-gating text for Slack, Signal, Microsoft Teams, Mattermost, and BlueBubbles to avoid mention-strip mismatches and inconsistent command detection. - CLI/Startup (Raspberry Pi + small hosts): speed up startup by avoiding unnecessary plugin preload on fast routes, adding root `--version` fast-path bootstrap bypass, parallelizing status JSON/non-JSON scans where safe, and enabling Node compile cache at startup with env override compatibility (`NODE_COMPILE_CACHE`, `NODE_DISABLE_COMPILE_CACHE`). (#5871) Thanks @BookCatKid and @vincentkoc for raising startup reports, and @lupuletic for related startup work in #27973. diff --git a/extensions/acpx/src/runtime-internals/process.test.ts b/extensions/acpx/src/runtime-internals/process.test.ts new file mode 100644 index 00000000000..161debb4718 --- /dev/null +++ b/extensions/acpx/src/runtime-internals/process.test.ts @@ -0,0 +1,146 @@ +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { resolveSpawnCommand } from "./process.js"; + +const tempDirs: string[] = []; + +function winRuntime(env: NodeJS.ProcessEnv) { + return { + platform: "win32" as const, + env, + execPath: "C:\\node\\node.exe", + }; +} + +async function createTempDir(): Promise { + const dir = await mkdtemp(path.join(tmpdir(), "openclaw-acpx-process-test-")); + tempDirs.push(dir); + return dir; +} + +afterEach(async () => { + while (tempDirs.length > 0) { + const dir = tempDirs.pop(); + if (!dir) { + continue; + } + await rm(dir, { + recursive: true, + force: true, + maxRetries: 8, + retryDelay: 8, + }); + } +}); + +describe("resolveSpawnCommand", () => { + it("keeps non-windows spawns unchanged", () => { + const resolved = resolveSpawnCommand( + { + command: "acpx", + args: ["--help"], + }, + { + platform: "darwin", + env: {}, + execPath: "/usr/bin/node", + }, + ); + + expect(resolved).toEqual({ + command: "acpx", + args: ["--help"], + }); + }); + + it("routes .js command execution through node on windows", () => { + const resolved = resolveSpawnCommand( + { + command: "C:/tools/acpx/cli.js", + args: ["--help"], + }, + winRuntime({}), + ); + + expect(resolved.command).toBe("C:\\node\\node.exe"); + expect(resolved.args).toEqual(["C:/tools/acpx/cli.js", "--help"]); + expect(resolved.shell).toBeUndefined(); + expect(resolved.windowsHide).toBe(true); + }); + + it("resolves a .cmd wrapper from PATH and unwraps shim entrypoint", async () => { + const dir = await createTempDir(); + const binDir = path.join(dir, "bin"); + const scriptPath = path.join(dir, "acpx", "dist", "index.js"); + await mkdir(path.dirname(scriptPath), { recursive: true }); + await mkdir(binDir, { recursive: true }); + await writeFile(scriptPath, "console.log('ok');", "utf8"); + const shimPath = path.join(binDir, "acpx.cmd"); + await writeFile( + shimPath, + ["@ECHO off", '"%~dp0\\..\\acpx\\dist\\index.js" %*', ""].join("\r\n"), + "utf8", + ); + + const resolved = resolveSpawnCommand( + { + command: "acpx", + args: ["--format", "json", "agent", "status"], + }, + winRuntime({ + PATH: binDir, + PATHEXT: ".CMD;.EXE;.BAT", + }), + ); + + expect(resolved.command).toBe("C:\\node\\node.exe"); + expect(resolved.args[0]).toBe(scriptPath); + expect(resolved.args.slice(1)).toEqual(["--format", "json", "agent", "status"]); + expect(resolved.shell).toBeUndefined(); + expect(resolved.windowsHide).toBe(true); + }); + + it("prefers executable shim targets without shell", async () => { + const dir = await createTempDir(); + const wrapperPath = path.join(dir, "acpx.cmd"); + const exePath = path.join(dir, "acpx.exe"); + await writeFile(exePath, "", "utf8"); + await writeFile(wrapperPath, ["@ECHO off", '"%~dp0\\acpx.exe" %*', ""].join("\r\n"), "utf8"); + + const resolved = resolveSpawnCommand( + { + command: wrapperPath, + args: ["--help"], + }, + winRuntime({}), + ); + + expect(resolved).toEqual({ + command: exePath, + args: ["--help"], + windowsHide: true, + }); + }); + + it("falls back to shell mode when wrapper cannot be safely unwrapped", async () => { + const dir = await createTempDir(); + const wrapperPath = path.join(dir, "custom-wrapper.cmd"); + await writeFile(wrapperPath, "@ECHO off\r\necho wrapper\r\n", "utf8"); + + const resolved = resolveSpawnCommand( + { + command: wrapperPath, + args: ["--arg", "value"], + }, + winRuntime({}), + ); + + expect(resolved).toEqual({ + command: wrapperPath, + args: ["--arg", "value"], + shell: true, + }); + }); +}); diff --git a/extensions/acpx/src/runtime-internals/process.ts b/extensions/acpx/src/runtime-internals/process.ts index 752b48835ec..a9fb1761bf1 100644 --- a/extensions/acpx/src/runtime-internals/process.ts +++ b/extensions/acpx/src/runtime-internals/process.ts @@ -1,5 +1,5 @@ import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; -import { existsSync } from "node:fs"; +import { existsSync, readFileSync, statSync } from "node:fs"; import path from "node:path"; export type SpawnExit = { @@ -12,31 +12,143 @@ type ResolvedSpawnCommand = { command: string; args: string[]; shell?: boolean; + windowsHide?: boolean; }; -function resolveSpawnCommand(params: { command: string; args: string[] }): ResolvedSpawnCommand { - if (process.platform !== "win32") { +type SpawnRuntime = { + platform: NodeJS.Platform; + env: NodeJS.ProcessEnv; + execPath: string; +}; + +const DEFAULT_RUNTIME: SpawnRuntime = { + platform: process.platform, + env: process.env, + execPath: process.execPath, +}; + +function isFilePath(candidate: string): boolean { + try { + return statSync(candidate).isFile(); + } catch { + return false; + } +} + +function resolveWindowsExecutablePath(command: string, env: NodeJS.ProcessEnv): string { + if (command.includes("/") || command.includes("\\") || path.isAbsolute(command)) { + return command; + } + + const pathValue = env.PATH ?? env.Path ?? process.env.PATH ?? process.env.Path ?? ""; + const pathEntries = pathValue + .split(";") + .map((entry) => entry.trim()) + .filter(Boolean); + const hasExtension = path.extname(command).length > 0; + const pathExtRaw = + env.PATHEXT ?? + env.Pathext ?? + process.env.PATHEXT ?? + process.env.Pathext ?? + ".EXE;.CMD;.BAT;.COM"; + const pathExt = hasExtension + ? [""] + : pathExtRaw + .split(";") + .map((ext) => ext.trim()) + .filter(Boolean) + .map((ext) => (ext.startsWith(".") ? ext : `.${ext}`)); + + for (const dir of pathEntries) { + for (const ext of pathExt) { + for (const candidateExt of [ext, ext.toLowerCase(), ext.toUpperCase()]) { + const candidate = path.join(dir, `${command}${candidateExt}`); + if (isFilePath(candidate)) { + return candidate; + } + } + } + } + + return command; +} + +function resolveNodeEntrypointFromCmdShim(wrapperPath: string): string | null { + if (!isFilePath(wrapperPath)) { + return null; + } + try { + const content = readFileSync(wrapperPath, "utf8"); + const candidates: string[] = []; + for (const match of content.matchAll(/"([^"\r\n]*)"/g)) { + const token = match[1] ?? ""; + const relMatch = token.match(/%~?dp0%?\s*[\\/]*(.*)$/i); + const relative = relMatch?.[1]?.trim(); + if (!relative) { + continue; + } + const normalizedRelative = relative.replace(/[\\/]+/g, path.sep).replace(/^[\\/]+/, ""); + const candidate = path.resolve(path.dirname(wrapperPath), normalizedRelative); + if (isFilePath(candidate)) { + candidates.push(candidate); + } + } + const nonNode = candidates.find((candidate) => { + const base = path.basename(candidate).toLowerCase(); + return base !== "node.exe" && base !== "node"; + }); + return nonNode ?? null; + } catch { + return null; + } +} + +export function resolveSpawnCommand( + params: { command: string; args: string[] }, + runtime: SpawnRuntime = DEFAULT_RUNTIME, +): ResolvedSpawnCommand { + if (runtime.platform !== "win32") { return { command: params.command, args: params.args }; } - const extension = path.extname(params.command).toLowerCase(); + const resolvedCommand = resolveWindowsExecutablePath(params.command, runtime.env); + const extension = path.extname(resolvedCommand).toLowerCase(); if (extension === ".js" || extension === ".cjs" || extension === ".mjs") { return { - command: process.execPath, - args: [params.command, ...params.args], + command: runtime.execPath, + args: [resolvedCommand, ...params.args], + windowsHide: true, }; } if (extension === ".cmd" || extension === ".bat") { + const entrypoint = resolveNodeEntrypointFromCmdShim(resolvedCommand); + if (entrypoint) { + const entryExt = path.extname(entrypoint).toLowerCase(); + if (entryExt === ".exe") { + return { + command: entrypoint, + args: params.args, + windowsHide: true, + }; + } + return { + command: runtime.execPath, + args: [entrypoint, ...params.args], + windowsHide: true, + }; + } + // Preserve compatibility for non-npm wrappers we cannot safely unwrap. return { - command: params.command, + command: resolvedCommand, args: params.args, shell: true, }; } return { - command: params.command, + command: resolvedCommand, args: params.args, }; } @@ -56,6 +168,7 @@ export function spawnWithResolvedCommand(params: { env: process.env, stdio: ["pipe", "pipe", "pipe"], shell: resolved.shell, + windowsHide: resolved.windowsHide, }); }