diff --git a/CHANGELOG.md b/CHANGELOG.md index fb93b2f6f..e431f59b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Docs: https://docs.openclaw.ai - Media understanding/audio transcription guard: skip tiny/empty audio files (<1024 bytes) before provider/CLI transcription to avoid noisy invalid-audio failures and preserve clean fallback behavior. (#8388) Thanks @Glucksberg. - OpenAI media capabilities: include `audio` in the OpenAI provider capability list so audio transcription models are eligible in media-understanding provider selection. (#12717) Thanks @openjay. - Security/Node exec approvals: preserve shell/dispatch-wrapper argv semantics during approval hardening so approved wrapper commands (for example `env sh -c ...`) cannot drift into a different runtime command shape, and add regression coverage for both approval-plan generation and approved runtime execution paths. Thanks @tdjackey for reporting. +- Security/Node exec approvals: revalidate approval-bound `cwd` identity immediately before execution/forwarding and fail closed with an explicit denial when `cwd` drifts after approval hardening. - Security/ACP sandbox inheritance: enforce fail-closed runtime guardrails for `sessions_spawn` with `runtime="acp"` by rejecting ACP spawns from sandboxed requester sessions and rejecting `sandbox="require"` for ACP runtime, preventing sandbox-boundary bypass via host-side ACP initialization. (#32254) Thanks @tdjackey for reporting, and @dutifulbob for the fix. - Browser/Security output boundary hardening: replace check-then-rename output commits with root-bound fd-verified writes, unify install/skills canonical path-boundary checks, and add regression coverage for symlink-rebind race paths across browser output and shared fs-safe write flows. Thanks @tdjackey for reporting. - Security/fs-safe write hardening: make `writeFileWithinRoot` use same-directory temp writes plus atomic rename, add post-write inode/hardlink revalidation with security warnings on boundary drift, and avoid truncating existing targets when final rename fails. diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index c7f63d453..a09477804 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -576,6 +576,55 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { } }); + it("denies approval-based execution when cwd identity drifts before execution", async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-cwd-drift-")); + const fallback = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-cwd-drift-alt-")); + const script = path.join(tmp, "run.sh"); + fs.writeFileSync(script, "#!/bin/sh\necho SAFE\n"); + fs.chmodSync(script, 0o755); + const canonicalCwd = fs.realpathSync(tmp); + const realStatSync = fs.statSync.bind(fs); + let targetStatCalls = 0; + const driftStat = realStatSync(fallback); + const statSpy = vi.spyOn(fs, "statSync").mockImplementation((...args) => { + const [target] = args; + const resolvedTarget = + typeof target === "string" + ? path.resolve(target) + : Buffer.isBuffer(target) + ? path.resolve(target.toString()) + : target instanceof URL + ? path.resolve(target.pathname) + : path.resolve(String(target)); + if (resolvedTarget === canonicalCwd) { + targetStatCalls += 1; + if (targetStatCalls >= 3) { + return driftStat; + } + } + return realStatSync(...args); + }); + try { + const { runCommand, sendInvokeResult } = await runSystemInvoke({ + preferMacAppExecHost: false, + command: ["./run.sh"], + cwd: tmp, + approved: true, + security: "full", + ask: "off", + }); + expect(runCommand).not.toHaveBeenCalled(); + expectInvokeErrorMessage(sendInvokeResult, { + message: "SYSTEM_RUN_DENIED: approval cwd changed before execution", + exact: true, + }); + } finally { + statSpy.mockRestore(); + fs.rmSync(tmp, { recursive: true, force: true }); + fs.rmSync(fallback, { recursive: true, force: true }); + } + }); + it("denies ./sh wrapper spoof in allowlist on-miss mode before execution", async () => { const marker = path.join(os.tmpdir(), `openclaw-wrapper-spoof-${process.pid}-${Date.now()}`); const runCommand = vi.fn(async () => { diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 78e5d589f..9f6976df3 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -1,4 +1,5 @@ import crypto from "node:crypto"; +import fs from "node:fs"; import { resolveAgentConfig } from "../agents/agent-scope.js"; import { loadConfig } from "../config/config.js"; import type { GatewayClient } from "../gateway/client.js"; @@ -14,6 +15,7 @@ import { } from "../infra/exec-approvals.js"; import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js"; import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; +import { sameFileIdentity } from "../infra/file-identity.js"; import { sanitizeSystemRunEnvOverrides } from "../infra/host-env-security.js"; import { resolveSystemRunCommand } from "../infra/system-run-command.js"; import { evaluateSystemRunPolicy, resolveExecApprovalDecision } from "./exec-policy.js"; @@ -81,9 +83,12 @@ type SystemRunPolicyPhase = SystemRunParsePhase & { segments: ExecCommandSegment[]; plannedAllowlistArgv: string[] | undefined; isWindows: boolean; + approvedCwdStat: fs.Stats | undefined; }; const safeBinTrustedDirWarningCache = new Set(); +const APPROVAL_CWD_DRIFT_DENIED_MESSAGE = + "SYSTEM_RUN_DENIED: approval cwd changed before execution"; function warnWritableTrustedDirOnce(message: string): void { if (safeBinTrustedDirWarningCache.has(message)) { @@ -107,6 +112,36 @@ function normalizeDeniedReason(reason: string | null | undefined): SystemRunDeni } } +function revalidateApprovedCwdBeforeExecution( + phase: SystemRunPolicyPhase, +): { ok: true } | { ok: false } { + if (!phase.policy.approvedByAsk || !phase.cwd || !phase.approvedCwdStat) { + return { ok: true }; + } + const hardened = hardenApprovedExecutionPaths({ + approvedByAsk: true, + argv: [], + shellCommand: null, + cwd: phase.cwd, + }); + if (!hardened.ok || hardened.cwd !== phase.cwd) { + return { ok: false }; + } + let currentCwdStat: fs.Stats; + try { + currentCwdStat = fs.statSync(phase.cwd); + } catch { + return { ok: false }; + } + if (!currentCwdStat.isDirectory()) { + return { ok: false }; + } + if (!sameFileIdentity(phase.approvedCwdStat, currentCwdStat)) { + return { ok: false }; + } + return { ok: true }; +} + export type HandleSystemRunInvokeOptions = { client: GatewayClient; params: SystemRunParams; @@ -299,6 +334,25 @@ async function evaluateSystemRunPolicyPhase( }); return null; } + let approvedCwdStat: fs.Stats | undefined; + if (policy.approvedByAsk && hardenedPaths.cwd) { + try { + approvedCwdStat = fs.statSync(hardenedPaths.cwd); + } catch { + await sendSystemRunDenied(opts, parsed.execution, { + reason: "approval-required", + message: APPROVAL_CWD_DRIFT_DENIED_MESSAGE, + }); + return null; + } + if (!approvedCwdStat.isDirectory()) { + await sendSystemRunDenied(opts, parsed.execution, { + reason: "approval-required", + message: APPROVAL_CWD_DRIFT_DENIED_MESSAGE, + }); + return null; + } + } const plannedAllowlistArgv = resolvePlannedAllowlistArgv({ security, @@ -326,6 +380,7 @@ async function evaluateSystemRunPolicyPhase( segments, plannedAllowlistArgv: plannedAllowlistArgv ?? undefined, isWindows, + approvedCwdStat, }; } @@ -333,6 +388,18 @@ async function executeSystemRunPhase( opts: HandleSystemRunInvokeOptions, phase: SystemRunPolicyPhase, ): Promise { + const cwdRevalidation = revalidateApprovedCwdBeforeExecution(phase); + if (!cwdRevalidation.ok) { + console.warn( + `[security] system.run approval cwd drift blocked: runId=${phase.runId} cwd=${phase.cwd ?? ""}`, + ); + await sendSystemRunDenied(opts, phase.execution, { + reason: "approval-required", + message: APPROVAL_CWD_DRIFT_DENIED_MESSAGE, + }); + return; + } + const useMacAppExec = opts.preferMacAppExecHost; if (useMacAppExec) { const execRequest: ExecHostRequest = {