diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bfe19d53bd..5f41716a587 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -299,6 +299,7 @@ Docs: https://docs.openclaw.ai - Agents/OpenAI-responses compatibility: strip unsupported `store` payload fields when `supportsStore=false` (including OpenAI-compatible non-OpenAI providers) while preserving server-compaction payload behavior. (#39219) Thanks @ademczuk. - Agents/model fallback visibility: warn when configured model IDs cannot be resolved and fallback is applied, with log-safe sanitization of model text to prevent control-sequence injection in warning output. (#39215) Thanks @ademczuk. - Outbound delivery replay safety: use two-phase delivery ACK markers (`.json` -> `.delivered` -> unlink) and startup marker cleanup so crash windows between send and cleanup do not replay already-delivered messages. (#38668) Thanks @Gundam98. +- Nodes/system.run approval binding: carry prepared approval plans through gateway forwarding and bind interpreter-style script operands across approval to execution, so post-approval script rewrites are denied while unchanged approved script runs keep working. Thanks @tdjackey for reporting. ## 2026.3.2 diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 45141e6d735..d538e411093 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -30,6 +30,9 @@ 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. macOS split: diff --git a/src/gateway/node-invoke-system-run-approval.test.ts b/src/gateway/node-invoke-system-run-approval.test.ts index 63f750de889..31dbdede846 100644 --- a/src/gateway/node-invoke-system-run-approval.test.ts +++ b/src/gateway/node-invoke-system-run-approval.test.ts @@ -278,6 +278,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { const forwarded = result.params as Record; expect(forwarded.command).toEqual(["/usr/bin/echo", "SAFE"]); expect(forwarded.rawCommand).toBe("/usr/bin/echo SAFE"); + expect(forwarded.systemRunPlan).toEqual(record.request.systemRunPlan); expect(forwarded.cwd).toBe("/real/cwd"); expect(forwarded.agentId).toBe("main"); expect(forwarded.sessionKey).toBe("agent:main:main"); diff --git a/src/gateway/node-invoke-system-run-approval.ts b/src/gateway/node-invoke-system-run-approval.ts index cf182559b9d..1099896f6c8 100644 --- a/src/gateway/node-invoke-system-run-approval.ts +++ b/src/gateway/node-invoke-system-run-approval.ts @@ -13,6 +13,7 @@ import { type SystemRunParamsLike = { command?: unknown; rawCommand?: unknown; + systemRunPlan?: unknown; cwd?: unknown; env?: unknown; timeoutMs?: unknown; @@ -69,6 +70,7 @@ function pickSystemRunParams(raw: Record): Record; + const argvIndex = + typeof candidate.argvIndex === "number" && + Number.isInteger(candidate.argvIndex) && + candidate.argvIndex >= 0 + ? candidate.argvIndex + : null; + const filePath = normalizeNonEmptyString(candidate.path); + const sha256 = normalizeNonEmptyString(candidate.sha256); + if (argvIndex === null || !filePath || !sha256) { + return null; + } + return { + argvIndex, + path: filePath, + sha256, + }; +} + export function normalizeSystemRunApprovalPlan(value: unknown): SystemRunApprovalPlan | null { if (!value || typeof value !== "object" || Array.isArray(value)) { return null; @@ -14,12 +46,17 @@ export function normalizeSystemRunApprovalPlan(value: unknown): SystemRunApprova if (argv.length === 0) { return null; } + const mutableFileOperand = normalizeSystemRunApprovalFileOperand(candidate.mutableFileOperand); + if (candidate.mutableFileOperand !== undefined && mutableFileOperand === null) { + return null; + } return { argv, cwd: normalizeNonEmptyString(candidate.cwd), rawCommand: normalizeNonEmptyString(candidate.rawCommand), agentId: normalizeNonEmptyString(candidate.agentId), sessionKey: normalizeNonEmptyString(candidate.sessionKey), + mutableFileOperand: mutableFileOperand ?? undefined, }; } diff --git a/src/node-host/invoke-system-run-plan.test.ts b/src/node-host/invoke-system-run-plan.test.ts index 484eca04757..a0d1dcbe3e4 100644 --- a/src/node-host/invoke-system-run-plan.test.ts +++ b/src/node-host/invoke-system-run-plan.test.ts @@ -128,4 +128,28 @@ describe("hardenApprovedExecutionPaths", () => { } }); } + + it("captures mutable shell script operands in approval plans", () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-script-plan-")); + const script = path.join(tmp, "run.sh"); + fs.writeFileSync(script, "#!/bin/sh\necho SAFE\n"); + fs.chmodSync(script, 0o755); + try { + const prepared = buildSystemRunApprovalPlan({ + command: ["/bin/sh", "./run.sh"], + cwd: tmp, + }); + expect(prepared.ok).toBe(true); + if (!prepared.ok) { + throw new Error("unreachable"); + } + expect(prepared.plan.mutableFileOperand).toEqual({ + argvIndex: 1, + path: fs.realpathSync(script), + sha256: expect.any(String), + }); + } 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 b434175a3d8..c35bf748667 100644 --- a/src/node-host/invoke-system-run-plan.ts +++ b/src/node-host/invoke-system-run-plan.ts @@ -1,8 +1,22 @@ +import crypto from "node:crypto"; import fs from "node:fs"; import path from "node:path"; -import type { SystemRunApprovalPlan } from "../infra/exec-approvals.js"; +import type { + SystemRunApprovalFileOperand, + SystemRunApprovalPlan, +} from "../infra/exec-approvals.js"; import { resolveCommandResolutionFromArgv } from "../infra/exec-command-resolution.js"; +import { + POSIX_SHELL_WRAPPERS, + normalizeExecutableToken, + unwrapKnownDispatchWrapperInvocation, + unwrapKnownShellMultiplexerInvocation, +} from "../infra/exec-wrapper-resolution.js"; import { sameFileIdentity } from "../infra/file-identity.js"; +import { + POSIX_INLINE_COMMAND_FLAGS, + resolveInlineCommandMatch, +} from "../infra/shell-inline-command.js"; import { formatExecCommand, resolveSystemRunCommand } from "../infra/system-run-command.js"; export type ApprovedCwdSnapshot = { @@ -10,6 +24,14 @@ export type ApprovedCwdSnapshot = { stat: fs.Stats; }; +const MUTABLE_ARGV1_INTERPRETER_PATTERNS = [ + /^(?:node|nodejs)$/, + /^perl$/, + /^php$/, + /^python(?:\d+(?:\.\d+)*)?$/, + /^ruby$/, +] as const; + function normalizeString(value: unknown): string | null { if (typeof value !== "string") { return null; @@ -68,6 +90,125 @@ function shouldPinExecutableForApproval(params: { return (params.wrapperChain?.length ?? 0) === 0; } +function hashFileContentsSync(filePath: string): string { + return crypto.createHash("sha256").update(fs.readFileSync(filePath)).digest("hex"); +} + +function unwrapArgvForMutableOperand(argv: string[]): { argv: string[]; baseIndex: number } { + let current = argv; + let baseIndex = 0; + while (true) { + const dispatchUnwrap = unwrapKnownDispatchWrapperInvocation(current); + if (dispatchUnwrap.kind === "unwrapped") { + baseIndex += current.length - dispatchUnwrap.argv.length; + current = dispatchUnwrap.argv; + continue; + } + const shellMultiplexerUnwrap = unwrapKnownShellMultiplexerInvocation(current); + if (shellMultiplexerUnwrap.kind === "unwrapped") { + baseIndex += current.length - shellMultiplexerUnwrap.argv.length; + current = shellMultiplexerUnwrap.argv; + continue; + } + return { argv: current, baseIndex }; + } +} + +function resolvePosixShellScriptOperandIndex(argv: string[]): number | null { + if ( + resolveInlineCommandMatch(argv, POSIX_INLINE_COMMAND_FLAGS, { + allowCombinedC: true, + }).valueTokenIndex !== null + ) { + return null; + } + let afterDoubleDash = false; + for (let i = 1; i < argv.length; i += 1) { + const token = argv[i]?.trim() ?? ""; + if (!token) { + continue; + } + if (token === "-") { + return null; + } + if (!afterDoubleDash && token === "--") { + afterDoubleDash = true; + continue; + } + if (!afterDoubleDash && token === "-s") { + return null; + } + if (!afterDoubleDash && token.startsWith("-")) { + continue; + } + return i; + } + return null; +} + +function resolveMutableFileOperandIndex(argv: string[]): number | null { + const unwrapped = unwrapArgvForMutableOperand(argv); + const executable = normalizeExecutableToken(unwrapped.argv[0] ?? ""); + if (!executable) { + return null; + } + if ((POSIX_SHELL_WRAPPERS as ReadonlySet).has(executable)) { + const shellIndex = resolvePosixShellScriptOperandIndex(unwrapped.argv); + return shellIndex === null ? null : unwrapped.baseIndex + shellIndex; + } + if (!MUTABLE_ARGV1_INTERPRETER_PATTERNS.some((pattern) => pattern.test(executable))) { + return null; + } + const operand = unwrapped.argv[1]?.trim() ?? ""; + if (!operand || operand === "-" || operand.startsWith("-")) { + return null; + } + return unwrapped.baseIndex + 1; +} + +function resolveMutableFileOperandSnapshotSync(params: { + argv: string[]; + cwd: string | undefined; +}): { ok: true; snapshot: SystemRunApprovalFileOperand | null } | { ok: false; message: string } { + const argvIndex = resolveMutableFileOperandIndex(params.argv); + if (argvIndex === null) { + return { ok: true, snapshot: null }; + } + const rawOperand = params.argv[argvIndex]?.trim(); + if (!rawOperand) { + return { + ok: false, + message: "SYSTEM_RUN_DENIED: approval requires a stable script operand", + }; + } + const resolvedPath = path.resolve(params.cwd ?? process.cwd(), rawOperand); + let realPath: string; + let stat: fs.Stats; + try { + realPath = fs.realpathSync(resolvedPath); + stat = fs.statSync(realPath); + } catch { + return { + ok: false, + message: "SYSTEM_RUN_DENIED: approval requires an existing script operand", + }; + } + if (!stat.isFile()) { + return { + ok: false, + message: "SYSTEM_RUN_DENIED: approval requires a file script operand", + }; + } + return { + ok: true, + snapshot: { + argvIndex, + path: realPath, + sha256: hashFileContentsSync(realPath), + }, + }; +} + function resolveCanonicalApprovalCwdSync(cwd: string): | { ok: true; @@ -135,6 +276,32 @@ export function revalidateApprovedCwdSnapshot(params: { snapshot: ApprovedCwdSna return sameFileIdentity(params.snapshot.stat, current.snapshot.stat); } +export function revalidateApprovedMutableFileOperand(params: { + snapshot: SystemRunApprovalFileOperand; + argv: string[]; + cwd: string | undefined; +}): boolean { + const operand = params.argv[params.snapshot.argvIndex]?.trim(); + if (!operand) { + return false; + } + const resolvedPath = path.resolve(params.cwd ?? process.cwd(), operand); + let realPath: string; + try { + realPath = fs.realpathSync(resolvedPath); + } catch { + return false; + } + if (realPath !== params.snapshot.path) { + return false; + } + try { + return hashFileContentsSync(realPath) === params.snapshot.sha256; + } catch { + return false; + } +} + export function hardenApprovedExecutionPaths(params: { approvedByAsk: boolean; argv: string[]; @@ -257,6 +424,13 @@ export function buildSystemRunApprovalPlan(params: { const rawCommand = hardening.argvChanged ? formatExecCommand(hardening.argv) || null : command.cmdText.trim() || null; + const mutableFileOperand = resolveMutableFileOperandSnapshotSync({ + argv: hardening.argv, + cwd: hardening.cwd, + }); + if (!mutableFileOperand.ok) { + return { ok: false, message: mutableFileOperand.message }; + } return { ok: true, plan: { @@ -265,7 +439,8 @@ export function buildSystemRunApprovalPlan(params: { rawCommand, agentId: normalizeString(params.agentId), sessionKey: normalizeString(params.sessionKey), + mutableFileOperand: mutableFileOperand.snapshot ?? undefined, }, - cmdText: command.cmdText, + cmdText: rawCommand ?? formatExecCommand(hardening.argv), }; } diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index b0952fb7eff..75f894acaa9 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { describe, expect, it, type Mock, vi } from "vitest"; +import type { SystemRunApprovalPlan } from "../infra/exec-approvals.js"; import { saveExecApprovals } from "../infra/exec-approvals.js"; import type { ExecHostResponse } from "../infra/exec-host.js"; import { buildSystemRunApprovalPlan } from "./invoke-system-run-plan.js"; @@ -235,6 +236,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { runViaResponse?: ExecHostResponse | null; command?: string[]; rawCommand?: string | null; + systemRunPlan?: SystemRunApprovalPlan | null; cwd?: string; security?: "full" | "allowlist"; ask?: "off" | "on-miss" | "always"; @@ -289,6 +291,7 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { params: { command: params.command ?? ["echo", "ok"], rawCommand: params.rawCommand, + systemRunPlan: params.systemRunPlan, cwd: params.cwd, approved: params.approved ?? false, sessionKey: "agent:main:main", @@ -687,6 +690,76 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { } }); + it("denies approval-based execution when a script operand changes after approval", async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-script-drift-")); + const script = path.join(tmp, "run.sh"); + fs.writeFileSync(script, "#!/bin/sh\necho SAFE\n"); + fs.chmodSync(script, 0o755); + try { + const prepared = buildSystemRunApprovalPlan({ + command: ["/bin/sh", "./run.sh"], + cwd: tmp, + }); + expect(prepared.ok).toBe(true); + if (!prepared.ok) { + throw new Error("unreachable"); + } + + fs.writeFileSync(script, "#!/bin/sh\necho PWNED\n"); + const { runCommand, sendInvokeResult } = await runSystemInvoke({ + preferMacAppExecHost: false, + command: prepared.plan.argv, + rawCommand: prepared.plan.rawCommand, + systemRunPlan: prepared.plan, + cwd: prepared.plan.cwd ?? tmp, + approved: true, + security: "full", + ask: "off", + }); + + expect(runCommand).not.toHaveBeenCalled(); + expectInvokeErrorMessage(sendInvokeResult, { + message: "SYSTEM_RUN_DENIED: approval script operand changed before execution", + exact: true, + }); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + it("keeps approved shell script execution working when the script is unchanged", async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-script-stable-")); + const script = path.join(tmp, "run.sh"); + fs.writeFileSync(script, "#!/bin/sh\necho SAFE\n"); + fs.chmodSync(script, 0o755); + try { + const prepared = buildSystemRunApprovalPlan({ + command: ["/bin/sh", "./run.sh"], + cwd: tmp, + }); + expect(prepared.ok).toBe(true); + if (!prepared.ok) { + throw new Error("unreachable"); + } + + const { runCommand, sendInvokeResult } = await runSystemInvoke({ + preferMacAppExecHost: false, + command: prepared.plan.argv, + rawCommand: prepared.plan.rawCommand, + systemRunPlan: prepared.plan, + cwd: prepared.plan.cwd ?? tmp, + approved: true, + security: "full", + ask: "off", + }); + + expect(runCommand).toHaveBeenCalledTimes(1); + expectInvokeOk(sendInvokeResult); + } finally { + fs.rmSync(tmp, { 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 6eed9ae3d7c..5fb737930a8 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -15,6 +15,7 @@ import { import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js"; import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; import { sanitizeSystemRunEnvOverrides } from "../infra/host-env-security.js"; +import { normalizeSystemRunApprovalPlan } from "../infra/system-run-approval-binding.js"; import { resolveSystemRunCommand } from "../infra/system-run-command.js"; import { logWarn } from "../logger.js"; import { evaluateSystemRunPolicy, resolveExecApprovalDecision } from "./exec-policy.js"; @@ -27,6 +28,7 @@ import { import { hardenApprovedExecutionPaths, revalidateApprovedCwdSnapshot, + revalidateApprovedMutableFileOperand, type ApprovedCwdSnapshot, } from "./invoke-system-run-plan.js"; import type { @@ -63,6 +65,7 @@ type SystemRunParsePhase = { argv: string[]; shellCommand: string | null; cmdText: string; + approvalPlan: import("../infra/exec-approvals.js").SystemRunApprovalPlan | null; agentId: string | undefined; sessionKey: string; runId: string; @@ -92,6 +95,8 @@ type SystemRunPolicyPhase = SystemRunParsePhase & { const safeBinTrustedDirWarningCache = new Set(); const APPROVAL_CWD_DRIFT_DENIED_MESSAGE = "SYSTEM_RUN_DENIED: approval cwd changed before execution"; +const APPROVAL_SCRIPT_OPERAND_DRIFT_DENIED_MESSAGE = + "SYSTEM_RUN_DENIED: approval script operand changed before execution"; function warnWritableTrustedDirOnce(message: string): void { if (safeBinTrustedDirWarningCache.has(message)) { @@ -197,6 +202,17 @@ async function parseSystemRunPhase( const shellCommand = command.shellCommand; const cmdText = command.cmdText; + const approvalPlan = + opts.params.systemRunPlan === undefined + ? null + : normalizeSystemRunApprovalPlan(opts.params.systemRunPlan); + if (opts.params.systemRunPlan !== undefined && !approvalPlan) { + await opts.sendInvokeResult({ + ok: false, + error: { code: "INVALID_REQUEST", message: "systemRunPlan invalid" }, + }); + return null; + } const agentId = opts.params.agentId?.trim() || undefined; const sessionKey = opts.params.sessionKey?.trim() || "node"; const runId = opts.params.runId?.trim() || crypto.randomUUID(); @@ -208,6 +224,7 @@ async function parseSystemRunPhase( argv: command.argv, shellCommand, cmdText, + approvalPlan, agentId, sessionKey, runId, @@ -361,6 +378,21 @@ async function executeSystemRunPhase( }); return; } + if ( + phase.approvalPlan?.mutableFileOperand && + !revalidateApprovedMutableFileOperand({ + snapshot: phase.approvalPlan.mutableFileOperand, + argv: phase.argv, + cwd: phase.cwd, + }) + ) { + logWarn(`security: system.run approval script drift blocked (runId=${phase.runId})`); + await sendSystemRunDenied(opts, phase.execution, { + reason: "approval-required", + message: APPROVAL_SCRIPT_OPERAND_DRIFT_DENIED_MESSAGE, + }); + return; + } const useMacAppExec = opts.preferMacAppExecHost; if (useMacAppExec) { diff --git a/src/node-host/invoke-types.ts b/src/node-host/invoke-types.ts index 72ffe75c2d7..619f86c84ff 100644 --- a/src/node-host/invoke-types.ts +++ b/src/node-host/invoke-types.ts @@ -1,8 +1,9 @@ -import type { SkillBinTrustEntry } from "../infra/exec-approvals.js"; +import type { SkillBinTrustEntry, SystemRunApprovalPlan } from "../infra/exec-approvals.js"; export type SystemRunParams = { command: string[]; rawCommand?: string | null; + systemRunPlan?: SystemRunApprovalPlan | null; cwd?: string | null; env?: Record; timeoutMs?: number | null;