From 68a8a98ab761cee8a22839a6a447253e735d0493 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 01:31:23 +0000 Subject: [PATCH] fix(acpx): default strict windows wrapper policy on windows --- CHANGELOG.md | 2 +- extensions/acpx/openclaw.plugin.json | 2 +- extensions/acpx/src/config.test.ts | 2 +- extensions/acpx/src/config.ts | 4 +- .../src/runtime-internals/process.test.ts | 25 +++++++ .../acpx/src/runtime-internals/process.ts | 61 +++++++++++++--- .../src/runtime-internals/test-fixtures.ts | 2 +- extensions/acpx/src/runtime.test.ts | 29 +++++++- extensions/acpx/src/runtime.ts | 16 +++++ extensions/lobster/src/windows-spawn.ts | 11 ++- src/plugin-sdk/index.ts | 5 ++ src/plugin-sdk/windows-spawn.ts | 70 +++++++++++++++---- 12 files changed, 195 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eba3269dc65..908d9660c76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,7 +124,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 compatibility fallback for unknown custom wrappers by default and adding an opt-in strict mode (`strictWindowsCmdWrapper`) to fail closed for unresolvable wrappers. This ships in the next npm release. Thanks @tdjackey for reporting. +- Security/ACPX Windows spawn hardening: resolve `.cmd/.bat` wrappers via PATH/PATHEXT and execute unwrapped Node/EXE entrypoints without shell parsing when possible, and enable strict fail-closed handling (`strictWindowsCmdWrapper`) by default for unresolvable wrappers on Windows (with explicit opt-out for compatibility). This ships in the next npm release. Thanks @tdjackey for reporting. - 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. - Security/Zalo webhook memory hardening: bound webhook security tracking state and normalize security keying to matched webhook paths (excluding attacker query-string churn) to prevent unauthenticated memory growth pressure on reachable webhook endpoints. Thanks @Somet2mes. - Security/Web search citation redirects: enforce strict SSRF defaults for Gemini citation redirect resolution so redirects to localhost/private/internal targets are blocked. Thanks @tdjackey for reporting. diff --git a/extensions/acpx/openclaw.plugin.json b/extensions/acpx/openclaw.plugin.json index e7280550301..49412b66b51 100644 --- a/extensions/acpx/openclaw.plugin.json +++ b/extensions/acpx/openclaw.plugin.json @@ -60,7 +60,7 @@ }, "strictWindowsCmdWrapper": { "label": "Strict Windows cmd Wrapper", - "help": "When enabled on Windows, reject unresolved .cmd/.bat wrappers instead of shell fallback. Hardening-only; can break non-standard wrappers.", + "help": "Enabled by default. On Windows, reject unresolved .cmd/.bat wrappers instead of shell fallback. Disable only for compatibility with non-standard wrappers.", "advanced": true }, "timeoutSeconds": { diff --git a/extensions/acpx/src/config.test.ts b/extensions/acpx/src/config.test.ts index 3c1d1361b1e..149fb52ba85 100644 --- a/extensions/acpx/src/config.test.ts +++ b/extensions/acpx/src/config.test.ts @@ -20,7 +20,7 @@ describe("acpx plugin config parsing", () => { expect(resolved.expectedVersion).toBe(ACPX_PINNED_VERSION); expect(resolved.allowPluginLocalInstall).toBe(true); expect(resolved.cwd).toBe(path.resolve("/tmp/workspace")); - expect(resolved.strictWindowsCmdWrapper).toBe(false); + expect(resolved.strictWindowsCmdWrapper).toBe(true); }); it("accepts command override and disables plugin-local auto-install", () => { diff --git a/extensions/acpx/src/config.ts b/extensions/acpx/src/config.ts index 51508867f59..a5441423c5e 100644 --- a/extensions/acpx/src/config.ts +++ b/extensions/acpx/src/config.ts @@ -45,6 +45,7 @@ export type ResolvedAcpxPluginConfig = { const DEFAULT_PERMISSION_MODE: AcpxPermissionMode = "approve-reads"; const DEFAULT_NON_INTERACTIVE_POLICY: AcpxNonInteractivePermissionPolicy = "fail"; const DEFAULT_QUEUE_OWNER_TTL_SECONDS = 0.1; +const DEFAULT_STRICT_WINDOWS_CMD_WRAPPER = true; type ParseResult = | { ok: true; value: AcpxPluginConfig | undefined } @@ -255,7 +256,8 @@ export function resolveAcpxPluginConfig(params: { permissionMode: normalized.permissionMode ?? DEFAULT_PERMISSION_MODE, nonInteractivePermissions: normalized.nonInteractivePermissions ?? DEFAULT_NON_INTERACTIVE_POLICY, - strictWindowsCmdWrapper: normalized.strictWindowsCmdWrapper ?? false, + strictWindowsCmdWrapper: + normalized.strictWindowsCmdWrapper ?? DEFAULT_STRICT_WINDOWS_CMD_WRAPPER, timeoutSeconds: normalized.timeoutSeconds, queueOwnerTtlSeconds: normalized.queueOwnerTtlSeconds ?? DEFAULT_QUEUE_OWNER_TTL_SECONDS, }; diff --git a/extensions/acpx/src/runtime-internals/process.test.ts b/extensions/acpx/src/runtime-internals/process.test.ts index 6a60ed7839a..85a72a13398 100644 --- a/extensions/acpx/src/runtime-internals/process.test.ts +++ b/extensions/acpx/src/runtime-internals/process.test.ts @@ -164,6 +164,31 @@ describe("resolveSpawnCommand", () => { ).toThrow(/without shell execution/); }); + it("fails closed for wrapper fallback when args include a malicious cwd payload", async () => { + const dir = await createTempDir(); + const wrapperPath = path.join(dir, "strict-wrapper.cmd"); + await writeFile(wrapperPath, "@ECHO off\r\necho wrapper\r\n", "utf8"); + const payload = "C:\\safe & calc.exe"; + const events: Array<{ resolution: string }> = []; + + expect(() => + resolveSpawnCommand( + { + command: wrapperPath, + args: ["--cwd", payload, "agent", "status"], + }, + { + strictWindowsCmdWrapper: true, + onResolved: (event) => { + events.push({ resolution: event.resolution }); + }, + }, + winRuntime({}), + ), + ).toThrow(/without shell execution/); + expect(events).toEqual([{ resolution: "unresolved-wrapper" }]); + }); + it("reuses resolved command when cache is provided", async () => { const dir = await createTempDir(); const wrapperPath = path.join(dir, "acpx.cmd"); diff --git a/extensions/acpx/src/runtime-internals/process.ts b/extensions/acpx/src/runtime-internals/process.ts index fb92a95572f..3032393df92 100644 --- a/extensions/acpx/src/runtime-internals/process.ts +++ b/extensions/acpx/src/runtime-internals/process.ts @@ -1,7 +1,15 @@ import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; import { existsSync } from "node:fs"; -import type { WindowsSpawnProgram } from "openclaw/plugin-sdk"; -import { materializeWindowsSpawnProgram, resolveWindowsSpawnProgram } from "openclaw/plugin-sdk"; +import type { + WindowsSpawnProgram, + WindowsSpawnProgramCandidate, + WindowsSpawnResolution, +} from "openclaw/plugin-sdk"; +import { + applyWindowsSpawnProgramPolicy, + materializeWindowsSpawnProgram, + resolveWindowsSpawnProgramCandidate, +} from "openclaw/plugin-sdk"; export type SpawnExit = { code: number | null; @@ -24,12 +32,21 @@ type SpawnRuntime = { export type SpawnCommandCache = { key?: string; - program?: WindowsSpawnProgram; + candidate?: WindowsSpawnProgramCandidate; +}; + +export type SpawnResolution = WindowsSpawnResolution | "unresolved-wrapper"; +export type SpawnResolutionEvent = { + command: string; + cacheHit: boolean; + strictWindowsCmdWrapper: boolean; + resolution: SpawnResolution; }; export type SpawnCommandOptions = { strictWindowsCmdWrapper?: boolean; cache?: SpawnCommandCache; + onResolved?: (event: SpawnResolutionEvent) => void; }; const DEFAULT_RUNTIME: SpawnRuntime = { @@ -44,27 +61,51 @@ export function resolveSpawnCommand( runtime: SpawnRuntime = DEFAULT_RUNTIME, ): ResolvedSpawnCommand { const strictWindowsCmdWrapper = options?.strictWindowsCmdWrapper === true; - const cacheKey = `${params.command}::${strictWindowsCmdWrapper ? "strict" : "compat"}`; + const cacheKey = params.command; const cachedProgram = options?.cache; - let program = - cachedProgram?.key === cacheKey && cachedProgram.program ? cachedProgram.program : undefined; - if (!program) { - program = resolveWindowsSpawnProgram({ + const cacheHit = cachedProgram?.key === cacheKey && cachedProgram.candidate != null; + let candidate = + cachedProgram?.key === cacheKey && cachedProgram.candidate + ? cachedProgram.candidate + : undefined; + if (!candidate) { + candidate = resolveWindowsSpawnProgramCandidate({ command: params.command, platform: runtime.platform, env: runtime.env, execPath: runtime.execPath, packageName: "acpx", - allowShellFallback: !strictWindowsCmdWrapper, }); if (cachedProgram) { cachedProgram.key = cacheKey; - cachedProgram.program = program; + cachedProgram.candidate = candidate; } } + let program: WindowsSpawnProgram; + try { + program = applyWindowsSpawnProgramPolicy({ + candidate, + allowShellFallback: !strictWindowsCmdWrapper, + }); + } catch (error) { + options?.onResolved?.({ + command: params.command, + cacheHit, + strictWindowsCmdWrapper, + resolution: candidate.resolution, + }); + throw error; + } + const resolved = materializeWindowsSpawnProgram(program, params.args); + options?.onResolved?.({ + command: params.command, + cacheHit, + strictWindowsCmdWrapper, + resolution: resolved.resolution, + }); return { command: resolved.command, args: resolved.argv, diff --git a/extensions/acpx/src/runtime-internals/test-fixtures.ts b/extensions/acpx/src/runtime-internals/test-fixtures.ts index be56f9a1bda..4aba03fa09d 100644 --- a/extensions/acpx/src/runtime-internals/test-fixtures.ts +++ b/extensions/acpx/src/runtime-internals/test-fixtures.ts @@ -272,7 +272,7 @@ export async function createMockRuntimeFixture(params?: { cwd: dir, permissionMode: params?.permissionMode ?? "approve-all", nonInteractivePermissions: "fail", - strictWindowsCmdWrapper: false, + strictWindowsCmdWrapper: true, queueOwnerTtlSeconds: params?.queueOwnerTtlSeconds ?? 0.1, }; diff --git a/extensions/acpx/src/runtime.test.ts b/extensions/acpx/src/runtime.test.ts index 2e078688752..5af339bf7f8 100644 --- a/extensions/acpx/src/runtime.test.ts +++ b/extensions/acpx/src/runtime.test.ts @@ -325,7 +325,7 @@ describe("AcpxRuntime", () => { cwd: process.cwd(), permissionMode: "approve-reads", nonInteractivePermissions: "fail", - strictWindowsCmdWrapper: false, + strictWindowsCmdWrapper: true, queueOwnerTtlSeconds: 0.1, }, { logger: NOOP_LOGGER }, @@ -341,6 +341,31 @@ describe("AcpxRuntime", () => { expect(runtime.isHealthy()).toBe(true); }); + it("logs ACPX spawn resolution once per command policy", async () => { + const { config } = await createMockRuntimeFixture(); + const debugLogs: string[] = []; + const runtime = new AcpxRuntime( + { + ...config, + strictWindowsCmdWrapper: true, + }, + { + logger: { + ...NOOP_LOGGER, + debug: (message: string) => { + debugLogs.push(message); + }, + }, + }, + ); + + await runtime.probeAvailability(); + + const spawnLogs = debugLogs.filter((entry) => entry.startsWith("acpx spawn resolver:")); + expect(spawnLogs.length).toBe(1); + expect(spawnLogs[0]).toContain("mode=strict"); + }); + it("returns doctor report for missing command", async () => { const runtime = new AcpxRuntime( { @@ -350,7 +375,7 @@ describe("AcpxRuntime", () => { cwd: process.cwd(), permissionMode: "approve-reads", nonInteractivePermissions: "fail", - strictWindowsCmdWrapper: false, + strictWindowsCmdWrapper: true, queueOwnerTtlSeconds: 0.1, }, { logger: NOOP_LOGGER }, diff --git a/extensions/acpx/src/runtime.ts b/extensions/acpx/src/runtime.ts index 87369515e20..0d9973afe70 100644 --- a/extensions/acpx/src/runtime.ts +++ b/extensions/acpx/src/runtime.ts @@ -23,6 +23,7 @@ import { resolveSpawnFailure, type SpawnCommandCache, type SpawnCommandOptions, + type SpawnResolutionEvent, spawnAndCollect, spawnWithResolvedCommand, waitForExit, @@ -98,6 +99,7 @@ export class AcpxRuntime implements AcpRuntime { private readonly queueOwnerTtlSeconds: number; private readonly spawnCommandCache: SpawnCommandCache = {}; private readonly spawnCommandOptions: SpawnCommandOptions; + private readonly loggedSpawnResolutions = new Set(); constructor( private readonly config: ResolvedAcpxPluginConfig, @@ -117,6 +119,9 @@ export class AcpxRuntime implements AcpRuntime { this.spawnCommandOptions = { strictWindowsCmdWrapper: this.config.strictWindowsCmdWrapper, cache: this.spawnCommandCache, + onResolved: (event) => { + this.logSpawnResolution(event); + }, }; } @@ -124,6 +129,17 @@ export class AcpxRuntime implements AcpRuntime { return this.healthy; } + private logSpawnResolution(event: SpawnResolutionEvent): void { + const key = `${event.command}::${event.strictWindowsCmdWrapper ? "strict" : "compat"}::${event.resolution}`; + if (event.cacheHit || this.loggedSpawnResolutions.has(key)) { + return; + } + this.loggedSpawnResolutions.add(key); + this.logger?.debug?.( + `acpx spawn resolver: command=${event.command} mode=${event.strictWindowsCmdWrapper ? "strict" : "compat"} resolution=${event.resolution}`, + ); + } + async probeAvailability(): Promise { const versionCheck = await checkAcpxVersion({ command: this.config.command, diff --git a/extensions/lobster/src/windows-spawn.ts b/extensions/lobster/src/windows-spawn.ts index 187e71e3521..6e42dfec41c 100644 --- a/extensions/lobster/src/windows-spawn.ts +++ b/extensions/lobster/src/windows-spawn.ts @@ -1,4 +1,8 @@ -import { materializeWindowsSpawnProgram, resolveWindowsSpawnProgram } from "openclaw/plugin-sdk"; +import { + applyWindowsSpawnProgramPolicy, + materializeWindowsSpawnProgram, + resolveWindowsSpawnProgramCandidate, +} from "openclaw/plugin-sdk"; type SpawnTarget = { command: string; @@ -11,10 +15,13 @@ export function resolveWindowsLobsterSpawn( argv: string[], env: NodeJS.ProcessEnv, ): SpawnTarget { - const program = resolveWindowsSpawnProgram({ + const candidate = resolveWindowsSpawnProgramCandidate({ command: execPath, env, packageName: "lobster", + }); + const program = applyWindowsSpawnProgramPolicy({ + candidate, allowShellFallback: false, }); const resolved = materializeWindowsSpawnProgram(program, argv); diff --git a/src/plugin-sdk/index.ts b/src/plugin-sdk/index.ts index dde00254bab..aeadabb5e0e 100644 --- a/src/plugin-sdk/index.ts +++ b/src/plugin-sdk/index.ts @@ -242,13 +242,18 @@ export { chunkTextForOutbound } from "./text-chunking.js"; export { readJsonFileWithFallback, writeJsonFileAtomically } from "./json-store.js"; export { buildRandomTempFilePath, withTempDownloadPath } from "./temp-path.js"; export { + applyWindowsSpawnProgramPolicy, materializeWindowsSpawnProgram, resolveWindowsExecutablePath, + resolveWindowsSpawnProgramCandidate, resolveWindowsSpawnProgram, } from "./windows-spawn.js"; export type { + ResolveWindowsSpawnProgramCandidateParams, ResolveWindowsSpawnProgramParams, + WindowsSpawnCandidateResolution, WindowsSpawnInvocation, + WindowsSpawnProgramCandidate, WindowsSpawnProgram, WindowsSpawnResolution, } from "./windows-spawn.js"; diff --git a/src/plugin-sdk/windows-spawn.ts b/src/plugin-sdk/windows-spawn.ts index 87bc2da6858..16d24de629a 100644 --- a/src/plugin-sdk/windows-spawn.ts +++ b/src/plugin-sdk/windows-spawn.ts @@ -7,6 +7,14 @@ export type WindowsSpawnResolution = | "exe-entrypoint" | "shell-fallback"; +export type WindowsSpawnCandidateResolution = Exclude; +export type WindowsSpawnProgramCandidate = { + command: string; + leadingArgv: string[]; + resolution: WindowsSpawnCandidateResolution | "unresolved-wrapper"; + windowsHide?: boolean; +}; + export type WindowsSpawnProgram = { command: string; leadingArgv: string[]; @@ -31,6 +39,10 @@ export type ResolveWindowsSpawnProgramParams = { packageName?: string; allowShellFallback?: boolean; }; +export type ResolveWindowsSpawnProgramCandidateParams = Omit< + ResolveWindowsSpawnProgramParams, + "allowShellFallback" +>; function isFilePath(candidate: string): boolean { try { @@ -176,9 +188,9 @@ function resolveEntrypointFromPackageJson( return null; } -export function resolveWindowsSpawnProgram( - params: ResolveWindowsSpawnProgramParams, -): WindowsSpawnProgram { +export function resolveWindowsSpawnProgramCandidate( + params: ResolveWindowsSpawnProgramCandidateParams, +): WindowsSpawnProgramCandidate { const platform = params.platform ?? process.platform; const env = params.env ?? process.env; const execPath = params.execPath ?? process.execPath; @@ -224,18 +236,11 @@ export function resolveWindowsSpawnProgram( }; } - if (params.allowShellFallback !== false) { - return { - command: resolvedCommand, - leadingArgv: [], - resolution: "shell-fallback", - shell: true, - }; - } - - throw new Error( - `${path.basename(resolvedCommand)} wrapper resolved, but no executable/Node entrypoint could be resolved without shell execution.`, - ); + return { + command: resolvedCommand, + leadingArgv: [], + resolution: "unresolved-wrapper", + }; } return { @@ -245,6 +250,41 @@ export function resolveWindowsSpawnProgram( }; } +export function applyWindowsSpawnProgramPolicy(params: { + candidate: WindowsSpawnProgramCandidate; + allowShellFallback?: boolean; +}): WindowsSpawnProgram { + if (params.candidate.resolution !== "unresolved-wrapper") { + return { + command: params.candidate.command, + leadingArgv: params.candidate.leadingArgv, + resolution: params.candidate.resolution, + windowsHide: params.candidate.windowsHide, + }; + } + if (params.allowShellFallback !== false) { + return { + command: params.candidate.command, + leadingArgv: [], + resolution: "shell-fallback", + shell: true, + }; + } + throw new Error( + `${path.basename(params.candidate.command)} wrapper resolved, but no executable/Node entrypoint could be resolved without shell execution.`, + ); +} + +export function resolveWindowsSpawnProgram( + params: ResolveWindowsSpawnProgramParams, +): WindowsSpawnProgram { + const candidate = resolveWindowsSpawnProgramCandidate(params); + return applyWindowsSpawnProgramPolicy({ + candidate, + allowShellFallback: params.allowShellFallback, + }); +} + export function materializeWindowsSpawnProgram( program: WindowsSpawnProgram, argv: string[],