diff --git a/SECURITY.md b/SECURITY.md index 5f1e8f0cb9e..204dadbf36d 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -125,6 +125,7 @@ Plugins/extensions are part of OpenClaw's trusted computing base for a gateway. - Any report whose only claim is that an operator-enabled `dangerous*`/`dangerously*` config option weakens defaults (these are explicit break-glass tradeoffs by design) - Reports that depend on trusted operator-supplied configuration values to trigger availability impact (for example custom regex patterns). These may still be fixed as defense-in-depth hardening, but are not security-boundary bypasses. - Reports whose only claim is heuristic/parity drift in command-risk detection (for example obfuscation-pattern checks) across exec surfaces, without a demonstrated trust-boundary bypass. These are hardening-only findings and are not vulnerabilities; triage may close them as `invalid`/`no-action` or track them separately as low/informational hardening. +- Reports whose only claim is that exec approvals do not semantically model every interpreter/runtime loader form, subcommand, flag combination, package script, or transitive module/config import. Exec approvals bind exact request context and best-effort direct local file operands; they are not a complete semantic model of everything a runtime may load. - Exposed secrets that are third-party/user-controlled credentials (not OpenClaw-owned and not granting access to OpenClaw-operated infrastructure/services) without demonstrated OpenClaw impact - Reports whose only claim is host-side exec when sandbox runtime is disabled/unavailable (documented default behavior in the trusted-operator model), without a boundary bypass. - Reports whose only claim is that a platform-provided upload destination URL is untrusted (for example Microsoft Teams `fileConsent/invoke` `uploadInfo.uploadUrl`) without proving attacker control in an authenticated production flow. @@ -165,6 +166,7 @@ OpenClaw separates routing from execution, but both remain inside the same opera - **Gateway** is the control plane. If a caller passes Gateway auth, they are treated as a trusted operator for that Gateway. - **Node** is an execution extension of the Gateway. Pairing a node grants operator-level remote capability on that node. - **Exec approvals** (allowlist/ask UI) are operator guardrails to reduce accidental command execution, not a multi-tenant authorization boundary. +- Exec approvals bind exact command/cwd/env context and, when OpenClaw can identify one concrete local script/file operand, that file snapshot too. This is best-effort integrity hardening, not a complete semantic model of every interpreter/runtime loader path. - Differences in command-risk warning heuristics between exec surfaces (`gateway`, `node`, `sandbox`) do not, by themselves, constitute a security-boundary bypass. - For untrusted-user isolation, split by trust boundary: separate gateways and separate OS users/hosts per boundary. diff --git a/docs/gateway/security/index.md b/docs/gateway/security/index.md index 8b790f4ded6..f4cf08b73c6 100644 --- a/docs/gateway/security/index.md +++ b/docs/gateway/security/index.md @@ -104,6 +104,7 @@ Treat Gateway and node as one operator trust domain, with different roles: - A caller authenticated to the Gateway is trusted at Gateway scope. After pairing, node actions are trusted operator actions on that node. - `sessionKey` is routing/context selection, not per-user auth. - Exec approvals (allowlist + ask) are guardrails for operator intent, not hostile multi-tenant isolation. +- Exec approvals bind exact request context and best-effort direct local file operands; they do not semantically model every runtime/interpreter loader path. Use sandboxing and host isolation for strong boundaries. If you need hostile-user isolation, split trust boundaries by OS user/host and run separate gateways. @@ -370,6 +371,7 @@ If a macOS node is paired, the Gateway can invoke `system.run` on that node. Thi - Requires node pairing (approval + token). - Controlled on the Mac via **Settings → Exec approvals** (security + ask + allowlist). +- Approval mode binds exact request context and, when possible, one concrete local script/file operand. If OpenClaw cannot identify exactly one direct local file for an interpreter/runtime command, approval-backed execution is denied rather than promising full semantic coverage. - If you don’t want remote execution, set security to **deny** and remove node pairing for that Mac. ## Dynamic skills (watcher / remote nodes) diff --git a/docs/nodes/index.md b/docs/nodes/index.md index 1b9b2bfaea2..69bdeb2c4c9 100644 --- a/docs/nodes/index.md +++ b/docs/nodes/index.md @@ -54,6 +54,15 @@ forwards `exec` calls to the **node host** when `host=node` is selected. - **Node host**: executes `system.run`/`system.which` on the node machine. - **Approvals**: enforced on the node host via `~/.openclaw/exec-approvals.json`. +Approval note: + +- Approval-backed node runs bind exact request context. +- For direct shell/runtime file executions, OpenClaw also best-effort binds one concrete local + file operand and denies the run if that file changes before execution. +- If OpenClaw cannot identify exactly one concrete local file for an interpreter/runtime command, + approval-backed execution is denied instead of pretending full runtime coverage. Use sandboxing, + separate hosts, or an explicit trusted allowlist/full workflow for broader interpreter semantics. + ### Start a node host (foreground) On the node machine: diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 91fdff80650..0bca1dee488 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -30,9 +30,14 @@ Trust model note: - Gateway-authenticated callers are trusted operators for that Gateway. - Paired nodes extend that trusted operator capability onto the node host. - Exec approvals reduce accidental execution risk, but are not a per-user auth boundary. -- Approved node-host runs also bind canonical execution context: canonical cwd, pinned executable - path when applicable, and interpreter-style script operands. If a bound script changes after - approval but before execution, the run is denied instead of executing drifted content. +- Approved node-host runs bind canonical execution context: canonical cwd, exact argv, env + binding when present, and pinned executable path when applicable. +- For shell scripts and direct interpreter/runtime file invocations, OpenClaw also tries to bind + one concrete local file operand. If that bound file changes after approval but before execution, + the run is denied instead of executing drifted content. +- This file binding is intentionally best-effort, not a complete semantic model of every + interpreter/runtime loader path. If approval mode cannot identify exactly one concrete local + file to bind, it refuses to mint an approval-backed run instead of pretending full coverage. macOS split: @@ -259,6 +264,20 @@ For `host=node`, approval requests include a canonical `systemRunPlan` payload. that plan as the authoritative command/cwd/session context when forwarding approved `system.run` requests. +## Interpreter/runtime commands + +Approval-backed interpreter/runtime runs are intentionally conservative: + +- Exact argv/cwd/env context is always bound. +- Direct shell script and direct runtime file forms are best-effort bound to one concrete local + file snapshot. +- If OpenClaw cannot identify exactly one concrete local file for an interpreter/runtime command + (for example package scripts, eval forms, runtime-specific loader chains, or ambiguous multi-file + forms), approval-backed execution is denied instead of claiming semantic coverage it does not + have. +- For those workflows, prefer sandboxing, a separate host boundary, or an explicit trusted + allowlist/full workflow where the operator accepts the broader runtime semantics. + When approvals are required, the exec tool returns immediately with an approval id. Use that id to correlate later system events (`Exec finished` / `Exec denied`). If no decision arrives before the timeout, the request is treated as an approval timeout and surfaced as a denial reason. diff --git a/src/node-host/invoke-system-run-plan.test.ts b/src/node-host/invoke-system-run-plan.test.ts index c7ba0657df1..c192509197e 100644 --- a/src/node-host/invoke-system-run-plan.test.ts +++ b/src/node-host/invoke-system-run-plan.test.ts @@ -214,6 +214,38 @@ describe("hardenApprovedExecutionPaths", () => { } const mutableOperandCases: RuntimeFixture[] = [ + { + name: "python flagged file", + binName: "python3", + argv: ["python3", "-B", "./run.py"], + scriptName: "run.py", + initialBody: 'print("SAFE")\n', + expectedArgvIndex: 2, + }, + { + name: "lua direct file", + binName: "lua", + argv: ["lua", "./run.lua"], + scriptName: "run.lua", + initialBody: 'print("SAFE")\n', + expectedArgvIndex: 1, + }, + { + name: "pypy direct file", + binName: "pypy", + argv: ["pypy", "./run.py"], + scriptName: "run.py", + initialBody: 'print("SAFE")\n', + expectedArgvIndex: 1, + }, + { + name: "versioned node alias file", + binName: "node20", + argv: ["node20", "./run.js"], + scriptName: "run.js", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 1, + }, { name: "bun direct file", binName: "bun", @@ -238,6 +270,22 @@ describe("hardenApprovedExecutionPaths", () => { initialBody: 'console.log("SAFE");\n', expectedArgvIndex: 5, }, + { + name: "bun test file", + binName: "bun", + argv: ["bun", "test", "./run.test.ts"], + scriptName: "run.test.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 2, + }, + { + name: "deno test file", + binName: "deno", + argv: ["deno", "test", "./run.test.ts"], + scriptName: "run.test.ts", + initialBody: 'console.log("SAFE");\n', + expectedArgvIndex: 2, + }, ]; for (const runtimeCase of mutableOperandCases) { @@ -296,7 +344,7 @@ describe("hardenApprovedExecutionPaths", () => { } }); - it("does not snapshot bun package script names", () => { + it("rejects bun package script names that do not bind a concrete file", () => { withFakeRuntimeBin({ binName: "bun", run: () => { @@ -306,11 +354,11 @@ describe("hardenApprovedExecutionPaths", () => { command: ["bun", "run", "dev"], cwd: tmp, }); - expect(prepared.ok).toBe(true); - if (!prepared.ok) { - throw new Error("unreachable"); - } - expect(prepared.plan.mutableFileOperand).toBeUndefined(); + expect(prepared).toEqual({ + ok: false, + message: + "SYSTEM_RUN_DENIED: approval cannot safely bind this interpreter/runtime command", + }); } finally { fs.rmSync(tmp, { recursive: true, force: true }); } @@ -318,7 +366,7 @@ describe("hardenApprovedExecutionPaths", () => { }); }); - it("does not snapshot deno eval invocations", () => { + it("rejects deno eval invocations that do not bind a concrete file", () => { withFakeRuntimeBin({ binName: "deno", run: () => { @@ -328,11 +376,11 @@ describe("hardenApprovedExecutionPaths", () => { command: ["deno", "eval", "console.log('SAFE')"], cwd: tmp, }); - expect(prepared.ok).toBe(true); - if (!prepared.ok) { - throw new Error("unreachable"); - } - expect(prepared.plan.mutableFileOperand).toBeUndefined(); + expect(prepared).toEqual({ + ok: false, + message: + "SYSTEM_RUN_DENIED: approval cannot safely bind this interpreter/runtime command", + }); } finally { fs.rmSync(tmp, { recursive: true, force: true }); } diff --git a/src/node-host/invoke-system-run-plan.ts b/src/node-host/invoke-system-run-plan.ts index e58738c9680..bc782d9c71c 100644 --- a/src/node-host/invoke-system-run-plan.ts +++ b/src/node-host/invoke-system-run-plan.ts @@ -6,6 +6,7 @@ import type { SystemRunApprovalPlan, } from "../infra/exec-approvals.js"; import { resolveCommandResolutionFromArgv } from "../infra/exec-command-resolution.js"; +import { isInterpreterLikeSafeBin } from "../infra/exec-safe-bin-runtime-policy.js"; import { POSIX_SHELL_WRAPPERS, normalizeExecutableToken, @@ -316,6 +317,53 @@ function resolveOptionFilteredPositionalIndex(params: { return null; } +function collectExistingFileOperandIndexes(params: { + argv: string[]; + startIndex: number; + cwd: string | undefined; +}): number[] { + let afterDoubleDash = false; + const hits: number[] = []; + for (let i = params.startIndex; i < params.argv.length; i += 1) { + const token = params.argv[i]?.trim() ?? ""; + if (!token) { + continue; + } + if (afterDoubleDash) { + if (resolvesToExistingFileSync(token, params.cwd)) { + hits.push(i); + } + continue; + } + if (token === "--") { + afterDoubleDash = true; + continue; + } + if (token === "-") { + return []; + } + if (token.startsWith("-")) { + continue; + } + if (resolvesToExistingFileSync(token, params.cwd)) { + hits.push(i); + } + } + return hits; +} + +function resolveGenericInterpreterScriptOperandIndex(params: { + argv: string[]; + cwd: string | undefined; +}): number | null { + const hits = collectExistingFileOperandIndexes({ + argv: params.argv, + startIndex: 1, + cwd: params.cwd, + }); + return hits.length === 1 ? hits[0] : null; +} + function resolveBunScriptOperandIndex(params: { argv: string[]; cwd: string | undefined; @@ -371,36 +419,76 @@ function resolveMutableFileOperandIndex(argv: string[], cwd: string | undefined) const shellIndex = resolvePosixShellScriptOperandIndex(unwrapped.argv); return shellIndex === null ? null : unwrapped.baseIndex + shellIndex; } - if (!MUTABLE_ARGV1_INTERPRETER_PATTERNS.some((pattern) => pattern.test(executable))) { - if (executable === "bun") { - const bunIndex = resolveBunScriptOperandIndex({ - argv: unwrapped.argv, - cwd, - }); - return bunIndex === null ? null : unwrapped.baseIndex + bunIndex; + if (MUTABLE_ARGV1_INTERPRETER_PATTERNS.some((pattern) => pattern.test(executable))) { + const operand = unwrapped.argv[1]?.trim() ?? ""; + if (operand && operand !== "-" && !operand.startsWith("-")) { + return unwrapped.baseIndex + 1; } - if (executable === "deno") { - const denoIndex = resolveDenoRunScriptOperandIndex({ - argv: unwrapped.argv, - cwd, - }); - return denoIndex === null ? null : unwrapped.baseIndex + denoIndex; + } + if (executable === "bun") { + const bunIndex = resolveBunScriptOperandIndex({ + argv: unwrapped.argv, + cwd, + }); + if (bunIndex !== null) { + return unwrapped.baseIndex + bunIndex; } + } + if (executable === "deno") { + const denoIndex = resolveDenoRunScriptOperandIndex({ + argv: unwrapped.argv, + cwd, + }); + if (denoIndex !== null) { + return unwrapped.baseIndex + denoIndex; + } + } + if (!isInterpreterLikeSafeBin(executable)) { return null; } - const operand = unwrapped.argv[1]?.trim() ?? ""; - if (!operand || operand === "-" || operand.startsWith("-")) { - return null; + const genericIndex = resolveGenericInterpreterScriptOperandIndex({ + argv: unwrapped.argv, + cwd, + }); + return genericIndex === null ? null : unwrapped.baseIndex + genericIndex; +} + +function requiresStableInterpreterApprovalBindingWithShellCommand(params: { + argv: string[]; + shellCommand: string | null; +}): boolean { + if (params.shellCommand !== null) { + return false; } - return unwrapped.baseIndex + 1; + const unwrapped = unwrapArgvForMutableOperand(params.argv); + const executable = normalizeExecutableToken(unwrapped.argv[0] ?? ""); + if (!executable) { + return false; + } + if ((POSIX_SHELL_WRAPPERS as ReadonlySet).has(executable)) { + return false; + } + return isInterpreterLikeSafeBin(executable); } function resolveMutableFileOperandSnapshotSync(params: { argv: string[]; cwd: string | undefined; + shellCommand: string | null; }): { ok: true; snapshot: SystemRunApprovalFileOperand | null } | { ok: false; message: string } { const argvIndex = resolveMutableFileOperandIndex(params.argv, params.cwd); if (argvIndex === null) { + if ( + requiresStableInterpreterApprovalBindingWithShellCommand({ + argv: params.argv, + shellCommand: params.shellCommand, + }) + ) { + return { + ok: false, + message: "SYSTEM_RUN_DENIED: approval cannot safely bind this interpreter/runtime command", + }; + } return { ok: true, snapshot: null }; } const rawOperand = params.argv[argvIndex]?.trim(); @@ -658,6 +746,7 @@ export function buildSystemRunApprovalPlan(params: { const mutableFileOperand = resolveMutableFileOperandSnapshotSync({ argv: hardening.argv, cwd: hardening.cwd, + shellCommand: command.shellCommand, }); if (!mutableFileOperand.ok) { return { ok: false, message: mutableFileOperand.message };