diff --git a/CHANGELOG.md b/CHANGELOG.md index 62aca46ec4..bc80fb92af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Docs: https://docs.openclaw.ai - Security: enforce access-group gating for Slack slash commands when channel type lookup fails. - Security: require validated shared-secret auth before skipping device identity on gateway connect. - Security: guard skill installer downloads with SSRF checks (block private/localhost URLs). +- Security: harden Windows exec allowlist; block cmd.exe bypass via single &. Thanks @simecek. - Media understanding: apply SSRF guardrails to provider fetches; allow private baseUrl overrides explicitly. - Tests: stub SSRF DNS pinning in web auto-reply + Gemini video coverage. (#6619) Thanks @joshp123. - fix(voice-call): harden inbound allowlist; reject anonymous callers; require Telnyx publicKey for allowlist; token-gate Twilio media streams; cap webhook body size (thanks @simecek) diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 2ac3dc541c..a771f85879 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -1046,6 +1046,7 @@ export function createExecTool( safeBins: new Set(), cwd: workdir, env, + platform: nodeInfo?.platform, }); let analysisOk = baseAllowlistEval.analysisOk; let allowlistSatisfied = false; @@ -1073,6 +1074,7 @@ export function createExecTool( safeBins: new Set(), cwd: workdir, env, + platform: nodeInfo?.platform, }); allowlistSatisfied = allowlistEval.allowlistSatisfied; analysisOk = allowlistEval.analysisOk; @@ -1282,6 +1284,7 @@ export function createExecTool( safeBins, cwd: workdir, env, + platform: process.platform, }); const allowlistMatches = allowlistEval.allowlistMatches; const analysisOk = allowlistEval.analysisOk; diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 635ba97113..6970b8e83b 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -161,6 +161,24 @@ describe("exec approvals shell parsing", () => { expect(res.ok).toBe(true); expect(res.segments[0]?.argv[0]).toBe("echo"); }); + + it("rejects windows shell metacharacters", () => { + const res = analyzeShellCommand({ + command: "ping 127.0.0.1 -n 1 & whoami", + platform: "win32", + }); + expect(res.ok).toBe(false); + expect(res.reason).toBe("unsupported windows shell token: &"); + }); + + it("parses windows quoted executables", () => { + const res = analyzeShellCommand({ + command: '"C:\\Program Files\\Tool\\tool.exe" --version', + platform: "win32", + }); + expect(res.ok).toBe(true); + expect(res.segments[0]?.argv).toEqual(["C:\\Program Files\\Tool\\tool.exe", "--version"]); + }); }); describe("exec approvals shell allowlist (chained commands)", () => { @@ -227,6 +245,19 @@ describe("exec approvals shell allowlist (chained commands)", () => { expect(result.analysisOk).toBe(true); expect(result.allowlistSatisfied).toBe(true); }); + + it("rejects windows chain separators for allowlist analysis", () => { + const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/ping" }]; + const result = evaluateShellAllowlist({ + command: "ping 127.0.0.1 -n 1 & whoami", + allowlist, + safeBins: new Set(), + cwd: "/tmp", + platform: "win32", + }); + expect(result.analysisOk).toBe(false); + expect(result.allowlistSatisfied).toBe(false); + }); }); describe("exec approvals safe bins", () => { diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index a510bd72ab..2d167631cd 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -586,6 +586,19 @@ export type ExecCommandAnalysis = { const DISALLOWED_PIPELINE_TOKENS = new Set([">", "<", "`", "\n", "\r", "(", ")"]); const DOUBLE_QUOTE_ESCAPES = new Set(["\\", '"', "$", "`", "\n", "\r"]); +const WINDOWS_UNSUPPORTED_TOKENS = new Set([ + "&", + "|", + "<", + ">", + "^", + "(", + ")", + "%", + "!", + "\n", + "\r", +]); function isDoubleQuoteEscape(next: string | undefined): next is string { return Boolean(next && DOUBLE_QUOTE_ESCAPES.has(next)); @@ -735,6 +748,86 @@ function splitShellPipeline(command: string): { ok: boolean; reason?: string; se return { ok: true, segments: result.parts }; } +function findWindowsUnsupportedToken(command: string): string | null { + for (const ch of command) { + if (WINDOWS_UNSUPPORTED_TOKENS.has(ch)) { + if (ch === "\n" || ch === "\r") { + return "newline"; + } + return ch; + } + } + return null; +} + +function tokenizeWindowsSegment(segment: string): string[] | null { + const tokens: string[] = []; + let buf = ""; + let inDouble = false; + + const pushToken = () => { + if (buf.length > 0) { + tokens.push(buf); + buf = ""; + } + }; + + for (let i = 0; i < segment.length; i += 1) { + const ch = segment[i]; + if (ch === '"') { + inDouble = !inDouble; + continue; + } + if (!inDouble && /\s/.test(ch)) { + pushToken(); + continue; + } + buf += ch; + } + + if (inDouble) { + return null; + } + pushToken(); + return tokens.length > 0 ? tokens : null; +} + +function analyzeWindowsShellCommand(params: { + command: string; + cwd?: string; + env?: NodeJS.ProcessEnv; +}): ExecCommandAnalysis { + const unsupported = findWindowsUnsupportedToken(params.command); + if (unsupported) { + return { + ok: false, + reason: `unsupported windows shell token: ${unsupported}`, + segments: [], + }; + } + const argv = tokenizeWindowsSegment(params.command); + if (!argv || argv.length === 0) { + return { ok: false, reason: "unable to parse windows command", segments: [] }; + } + return { + ok: true, + segments: [ + { + raw: params.command, + argv, + resolution: resolveCommandResolutionFromArgv(argv, params.cwd, params.env), + }, + ], + }; +} + +function isWindowsPlatform(platform?: string | null): boolean { + const normalized = String(platform ?? "") + .trim() + .toLowerCase(); + return normalized.startsWith("win"); +} + function tokenizeShellSegment(segment: string): string[] | null { const tokens: string[] = []; let buf = ""; @@ -828,7 +921,11 @@ export function analyzeShellCommand(params: { command: string; cwd?: string; env?: NodeJS.ProcessEnv; + platform?: string | null; }): ExecCommandAnalysis { + if (isWindowsPlatform(params.platform)) { + return analyzeWindowsShellCommand(params); + } // First try splitting by chain operators (&&, ||, ;) const chainParts = splitCommandChain(params.command); if (chainParts) { @@ -1190,13 +1287,15 @@ export function evaluateShellAllowlist(params: { env?: NodeJS.ProcessEnv; skillBins?: Set; autoAllowSkills?: boolean; + platform?: string | null; }): ExecAllowlistAnalysis { - const chainParts = splitCommandChain(params.command); + const chainParts = isWindowsPlatform(params.platform) ? null : splitCommandChain(params.command); if (!chainParts) { const analysis = analyzeShellCommand({ command: params.command, cwd: params.cwd, env: params.env, + platform: params.platform, }); if (!analysis.ok) { return { @@ -1230,6 +1329,7 @@ export function evaluateShellAllowlist(params: { command: part, cwd: params.cwd, env: params.env, + platform: params.platform, }); if (!analysis.ok) { return { diff --git a/src/node-host/runner.ts b/src/node-host/runner.ts index e0a425c6b6..15e9fdde79 100644 --- a/src/node-host/runner.ts +++ b/src/node-host/runner.ts @@ -119,6 +119,15 @@ function resolveExecSecurity(value?: string): ExecSecurity { return value === "deny" || value === "allowlist" || value === "full" ? value : "allowlist"; } +function isCmdExeInvocation(argv: string[]): boolean { + const token = argv[0]?.trim(); + if (!token) { + return false; + } + const base = path.win32.basename(token).toLowerCase(); + return base === "cmd.exe" || base === "cmd"; +} + function resolveExecAsk(value?: string): ExecAsk { return value === "off" || value === "on-miss" || value === "always" ? value : "on-miss"; } @@ -906,6 +915,7 @@ async function handleInvoke( env, skillBins: bins, autoAllowSkills, + platform: process.platform, }); analysisOk = allowlistEval.analysisOk; allowlistMatches = allowlistEval.allowlistMatches; @@ -928,6 +938,14 @@ async function handleInvoke( security === "allowlist" && analysisOk ? allowlistEval.allowlistSatisfied : false; segments = analysis.segments; } + const isWindows = process.platform === "win32"; + const cmdInvocation = rawCommand + ? isCmdExeInvocation(segments[0]?.argv ?? []) + : isCmdExeInvocation(argv); + if (security === "allowlist" && isWindows && cmdInvocation) { + analysisOk = false; + allowlistSatisfied = false; + } const useMacAppExec = process.platform === "darwin"; if (useMacAppExec) { @@ -1127,8 +1145,23 @@ async function handleInvoke( return; } + let execArgv = argv; + if ( + security === "allowlist" && + isWindows && + !approvedByAsk && + rawCommand && + analysisOk && + allowlistSatisfied && + segments.length === 1 && + segments[0]?.argv.length > 0 + ) { + // Avoid cmd.exe in allowlist mode on Windows; run the parsed argv directly. + execArgv = segments[0].argv; + } + const result = await runCommand( - argv, + execArgv, params.cwd?.trim() || undefined, env, params.timeoutMs ?? undefined,