diff --git a/CHANGELOG.md b/CHANGELOG.md index 798a08d6993..2202be77ac6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Docs: https://docs.openclaw.ai - Security/Workspace FS boundary aliases: harden canonical boundary resolution for non-existent-leaf symlink aliases while preserving valid in-root aliases, preventing first-write workspace escapes via out-of-root symlink targets. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting. - Security/Config includes: harden `$include` file loading with verified-open reads, reject hardlinked include aliases, and enforce include file-size guardrails so config include resolution remains bounded to trusted in-root files. This ships in the next npm release (`2026.2.26`). Thanks @zpbrent for reporting. - Security/Node exec approvals: require structured `commandArgv` approvals for `host=node`, enforce versioned `systemRunBindingV1` matching for argv/cwd/session/agent/env context with fail-closed behavior on missing/mismatched bindings, and add `GIT_EXTERNAL_DIFF` to blocked host env keys. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting. +- Security/Node exec approvals hardening: freeze immutable approval-time execution plans (`argv`/`cwd`/`agentId`/`sessionKey`) via `system.run.prepare`, enforce those canonical plan values during approval forwarding/execution, and reject mutable parent-symlink cwd paths during approval-plan building to prevent approval bypass via symlink rebind. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting. - Security/Microsoft Teams media fetch: route Graph message/hosted-content/attachment fetches and auth-scope fallback attachment downloads through shared SSRF-guarded fetch paths, and centralize hostname-suffix allowlist policy helpers in the plugin SDK to remove channel/plugin drift. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting. - Security/Voice Call (Twilio): bind webhook replay + manager dedupe identity to authenticated request material, remove unsigned `i-twilio-idempotency-token` trust from replay/dedupe keys, and thread verified request identity through provider parse flow to harden cross-provider event dedupe. This ships in the next npm release (`2026.2.26`). Thanks @tdjackey for reporting. - Microsoft Teams/File uploads: acknowledge `fileConsent/invoke` immediately (`invokeResponse` before upload + file card send) so Teams no longer shows false "Something went wrong" timeout banners while upload completion continues asynchronously; includes updated async regression coverage. Landed from contributor PR #27641 by @scz2011. diff --git a/src/cli/nodes-cli.coverage.test.ts b/src/cli/nodes-cli.coverage.test.ts index cd3fe62857d..2670586662a 100644 --- a/src/cli/nodes-cli.coverage.test.ts +++ b/src/cli/nodes-cli.coverage.test.ts @@ -28,6 +28,35 @@ const callGateway = vi.fn(async (opts: NodeInvokeCall) => { }; } if (opts.method === "node.invoke") { + const command = opts.params?.command; + if (command === "system.run.prepare") { + const params = (opts.params?.params ?? {}) as { + command?: unknown[]; + rawCommand?: unknown; + cwd?: unknown; + agentId?: unknown; + }; + const argv = Array.isArray(params.command) + ? params.command.map((entry) => String(entry)) + : []; + const rawCommand = + typeof params.rawCommand === "string" && params.rawCommand.trim().length > 0 + ? params.rawCommand + : null; + return { + payload: { + cmdText: rawCommand ?? argv.join(" "), + plan: { + version: 2, + argv, + cwd: typeof params.cwd === "string" ? params.cwd : null, + rawCommand, + agentId: typeof params.agentId === "string" ? params.agentId : null, + sessionKey: null, + }, + }, + }; + } return { payload: { stdout: "", @@ -80,8 +109,16 @@ vi.mock("../config/config.js", () => ({ describe("nodes-cli coverage", () => { let registerNodesCli: (program: Command) => void; - const getNodeInvokeCall = () => - callGateway.mock.calls.find((call) => call[0]?.method === "node.invoke")?.[0] as NodeInvokeCall; + const getNodeInvokeCall = () => { + const nodeInvokeCalls = callGateway.mock.calls + .map((call) => call[0]) + .filter((entry): entry is NodeInvokeCall => entry?.method === "node.invoke"); + const last = nodeInvokeCalls.at(-1); + if (!last) { + throw new Error("expected node.invoke call"); + } + return last; + }; const getApprovalRequestCall = () => callGateway.mock.calls.find((call) => call[0]?.method === "exec.approval.request")?.[0] as { @@ -135,6 +172,7 @@ describe("nodes-cli coverage", () => { expect(invoke?.params?.command).toBe("system.run"); expect(invoke?.params?.params).toEqual({ command: ["echo", "hi"], + rawCommand: null, cwd: "/tmp", env: { FOO: "bar" }, timeoutMs: 1200, @@ -147,6 +185,14 @@ describe("nodes-cli coverage", () => { expect(invoke?.params?.timeoutMs).toBe(5000); const approval = getApprovalRequestCall(); expect(approval?.params?.["commandArgv"]).toEqual(["echo", "hi"]); + expect(approval?.params?.["systemRunPlanV2"]).toEqual({ + version: 2, + argv: ["echo", "hi"], + cwd: "/tmp", + rawCommand: null, + agentId: "main", + sessionKey: null, + }); }); it("invokes system.run with raw command", async () => { @@ -174,6 +220,14 @@ describe("nodes-cli coverage", () => { }); const approval = getApprovalRequestCall(); expect(approval?.params?.["commandArgv"]).toEqual(["/bin/sh", "-lc", "echo hi"]); + expect(approval?.params?.["systemRunPlanV2"]).toEqual({ + version: 2, + argv: ["/bin/sh", "-lc", "echo hi"], + cwd: null, + rawCommand: "echo hi", + agentId: "main", + sessionKey: null, + }); }); it("invokes system.notify with provided fields", async () => { diff --git a/src/cli/nodes-cli/register.invoke.ts b/src/cli/nodes-cli/register.invoke.ts index e644d754d12..caf9ae02c4e 100644 --- a/src/cli/nodes-cli/register.invoke.ts +++ b/src/cli/nodes-cli/register.invoke.ts @@ -7,12 +7,14 @@ import { type ExecApprovalsFile, type ExecAsk, type ExecSecurity, + type SystemRunApprovalPlanV2, maxAsk, minSecurity, resolveExecApprovalsFromFile, } from "../../infra/exec-approvals.js"; import { buildNodeShellCommand } from "../../infra/node-shell.js"; import { applyPathPrepend } from "../../infra/path-prepend.js"; +import { normalizeSystemRunApprovalPlanV2 } from "../../infra/system-run-approval-binding.js"; import { defaultRuntime } from "../../runtime.js"; import { parseEnvPairs, parseTimeoutMs } from "../nodes-run.js"; import { getNodesTheme, runNodesCommand } from "./cli-utils.js"; @@ -42,6 +44,22 @@ type ExecDefaults = { safeBins?: string[]; }; +function parsePreparedRunPlan(payload: unknown): { + cmdText: string; + plan: SystemRunApprovalPlanV2; +} { + if (!payload || typeof payload !== "object" || Array.isArray(payload)) { + throw new Error("invalid system.run.prepare response"); + } + const raw = payload as { cmdText?: unknown; plan?: unknown }; + const cmdText = typeof raw.cmdText === "string" ? raw.cmdText.trim() : ""; + const plan = normalizeSystemRunApprovalPlanV2(raw.plan); + if (!cmdText || !plan) { + throw new Error("invalid system.run.prepare response"); + } + return { cmdText, plan }; +} + function normalizeExecSecurity(value?: string | null): ExecSecurity | null { const normalized = value?.trim().toLowerCase(); if (normalized === "deny" || normalized === "allowlist" || normalized === "full") { @@ -192,6 +210,20 @@ export function registerNodesInvokeCommands(nodes: Command) { applyPathPrepend(nodeEnv, execDefaults?.pathPrepend, { requireExisting: true }); } + const prepareResponse = (await callGatewayCli("node.invoke", opts, { + nodeId, + command: "system.run.prepare", + params: { + command: argv, + rawCommand, + cwd: opts.cwd, + agentId, + }, + idempotencyKey: `prepare-${randomIdempotencyKey()}`, + })) as { payload?: unknown } | null; + const prepared = parsePreparedRunPlan(prepareResponse?.payload); + const approvalPlan = prepared.plan; + let approvedByAsk = false; let approvalDecision: "allow-once" | "allow-always" | null = null; const configuredSecurity = normalizeExecSecurity(execDefaults?.security) ?? "allowlist"; @@ -251,16 +283,17 @@ export function registerNodesInvokeCommands(nodes: Command) { opts, { id: approvalId, - command: rawCommand ?? argv.join(" "), - commandArgv: argv, - cwd: opts.cwd, + command: prepared.cmdText, + commandArgv: approvalPlan.argv, + systemRunPlanV2: approvalPlan, + cwd: approvalPlan.cwd, nodeId, host: "node", security: hostSecurity, ask: hostAsk, - agentId, + agentId: approvalPlan.agentId ?? agentId, resolvedPath: undefined, - sessionKey: undefined, + sessionKey: approvalPlan.sessionKey ?? undefined, timeoutMs: approvalTimeoutMs, }, { transportTimeoutMs }, @@ -296,19 +329,21 @@ export function registerNodesInvokeCommands(nodes: Command) { nodeId, command: "system.run", params: { - command: argv, - cwd: opts.cwd, + command: approvalPlan.argv, + rawCommand: approvalPlan.rawCommand, + cwd: approvalPlan.cwd, env: nodeEnv, timeoutMs, needsScreenRecording: opts.needsScreenRecording === true, }, idempotencyKey: String(opts.idempotencyKey ?? randomIdempotencyKey()), }; - if (agentId) { - (invokeParams.params as Record).agentId = agentId; + if (approvalPlan.agentId ?? agentId) { + (invokeParams.params as Record).agentId = + approvalPlan.agentId ?? agentId; } - if (rawCommand) { - (invokeParams.params as Record).rawCommand = rawCommand; + if (approvalPlan.sessionKey) { + (invokeParams.params as Record).sessionKey = approvalPlan.sessionKey; } (invokeParams.params as Record).approved = approvedByAsk; if (approvalDecision) { diff --git a/src/gateway/node-command-policy.ts b/src/gateway/node-command-policy.ts index 68eb8bb2835..ba32b0d7747 100644 --- a/src/gateway/node-command-policy.ts +++ b/src/gateway/node-command-policy.ts @@ -40,7 +40,13 @@ const SMS_DANGEROUS_COMMANDS = ["sms.send"]; // iOS nodes don't implement system.run/which, but they do support notifications. const IOS_SYSTEM_COMMANDS = ["system.notify"]; -const SYSTEM_COMMANDS = ["system.run", "system.which", "system.notify", "browser.proxy"]; +const SYSTEM_COMMANDS = [ + "system.run.prepare", + "system.run", + "system.which", + "system.notify", + "browser.proxy", +]; // "High risk" node commands. These can be enabled by explicitly adding them to // `gateway.nodes.allowCommands` (and ensuring they're not blocked by denyCommands). diff --git a/src/gateway/node-invoke-system-run-approval.test.ts b/src/gateway/node-invoke-system-run-approval.test.ts index 1336e0fe009..50798323a3b 100644 --- a/src/gateway/node-invoke-system-run-approval.test.ts +++ b/src/gateway/node-invoke-system-run-approval.test.ts @@ -229,6 +229,50 @@ describe("sanitizeSystemRunParamsForForwarding", () => { expectAllowOnceForwardingResult(result); }); + test("uses systemRunPlanV2 for forwarded command context and ignores caller tampering", () => { + const record = makeRecord("echo SAFE", ["echo", "SAFE"]); + record.request.systemRunPlanV2 = { + version: 2, + argv: ["/usr/bin/echo", "SAFE"], + cwd: "/real/cwd", + rawCommand: "/usr/bin/echo SAFE", + agentId: "main", + sessionKey: "agent:main:main", + }; + record.request.systemRunBindingV1 = buildSystemRunApprovalBindingV1({ + argv: ["/usr/bin/echo", "SAFE"], + cwd: "/real/cwd", + agentId: "main", + sessionKey: "agent:main:main", + }).binding; + const result = sanitizeSystemRunParamsForForwarding({ + rawParams: { + command: ["echo", "PWNED"], + rawCommand: "echo PWNED", + cwd: "/tmp/attacker-link/sub", + agentId: "attacker", + sessionKey: "agent:attacker:main", + runId: "approval-1", + approved: true, + approvalDecision: "allow-once", + }, + nodeId: "node-1", + client, + execApprovalManager: manager(record), + nowMs: now, + }); + expectAllowOnceForwardingResult(result); + if (!result.ok) { + throw new Error("unreachable"); + } + const forwarded = result.params as Record; + expect(forwarded.command).toEqual(["/usr/bin/echo", "SAFE"]); + expect(forwarded.rawCommand).toBe("/usr/bin/echo SAFE"); + expect(forwarded.cwd).toBe("/real/cwd"); + expect(forwarded.agentId).toBe("main"); + expect(forwarded.sessionKey).toBe("agent:main:main"); + }); + test("rejects env overrides when approval record lacks env binding", () => { const result = sanitizeSystemRunParamsForForwarding({ rawParams: { diff --git a/src/gateway/node-invoke-system-run-approval.ts b/src/gateway/node-invoke-system-run-approval.ts index fffca68574f..a9d572c9763 100644 --- a/src/gateway/node-invoke-system-run-approval.ts +++ b/src/gateway/node-invoke-system-run-approval.ts @@ -1,3 +1,4 @@ +import { normalizeSystemRunApprovalPlanV2 } from "../infra/system-run-approval-binding.js"; import { resolveSystemRunCommand } from "../infra/system-run-command.js"; import type { ExecApprovalRecord } from "./exec-approval-manager.js"; import { @@ -99,18 +100,6 @@ export function sanitizeSystemRunParamsForForwarding(opts: { } const p = obj as SystemRunParamsLike; - const cmdTextResolution = resolveSystemRunCommand({ - command: p.command, - rawCommand: p.rawCommand, - }); - if (!cmdTextResolution.ok) { - return { - ok: false, - message: cmdTextResolution.message, - details: cmdTextResolution.details, - }; - } - const approved = p.approved === true; const requestedDecision = normalizeApprovalDecision(p.approvalDecision); const wantsApprovalOverride = approved || requestedDecision !== null; @@ -120,6 +109,17 @@ export function sanitizeSystemRunParamsForForwarding(opts: { const next: Record = pickSystemRunParams(obj); if (!wantsApprovalOverride) { + const cmdTextResolution = resolveSystemRunCommand({ + command: p.command, + rawCommand: p.rawCommand, + }); + if (!cmdTextResolution.ok) { + return { + ok: false, + message: cmdTextResolution.message, + details: cmdTextResolution.details, + }; + } return { ok: true, params: next }; } @@ -206,13 +206,62 @@ export function sanitizeSystemRunParamsForForwarding(opts: { }; } + const planV2 = normalizeSystemRunApprovalPlanV2(snapshot.request.systemRunPlanV2 ?? null); + let approvalArgv: string[]; + let approvalCwd: string | null; + let approvalAgentId: string | null; + let approvalSessionKey: string | null; + if (planV2) { + approvalArgv = [...planV2.argv]; + approvalCwd = planV2.cwd; + approvalAgentId = planV2.agentId; + approvalSessionKey = planV2.sessionKey; + next.command = [...planV2.argv]; + if (planV2.rawCommand) { + next.rawCommand = planV2.rawCommand; + } else { + delete next.rawCommand; + } + if (planV2.cwd) { + next.cwd = planV2.cwd; + } else { + delete next.cwd; + } + if (planV2.agentId) { + next.agentId = planV2.agentId; + } else { + delete next.agentId; + } + if (planV2.sessionKey) { + next.sessionKey = planV2.sessionKey; + } else { + delete next.sessionKey; + } + } else { + const cmdTextResolution = resolveSystemRunCommand({ + command: p.command, + rawCommand: p.rawCommand, + }); + if (!cmdTextResolution.ok) { + return { + ok: false, + message: cmdTextResolution.message, + details: cmdTextResolution.details, + }; + } + approvalArgv = cmdTextResolution.argv; + approvalCwd = normalizeString(p.cwd) ?? null; + approvalAgentId = normalizeString(p.agentId) ?? null; + approvalSessionKey = normalizeString(p.sessionKey) ?? null; + } + const approvalMatch = evaluateSystemRunApprovalMatch({ - argv: cmdTextResolution.argv, + argv: approvalArgv, request: snapshot.request, binding: { - cwd: normalizeString(p.cwd) ?? null, - agentId: normalizeString(p.agentId) ?? null, - sessionKey: normalizeString(p.sessionKey) ?? null, + cwd: approvalCwd, + agentId: approvalAgentId, + sessionKey: approvalSessionKey, env: p.env, }, }); diff --git a/src/gateway/protocol/schema/exec-approvals.ts b/src/gateway/protocol/schema/exec-approvals.ts index 44ed1544385..0358cde48fe 100644 --- a/src/gateway/protocol/schema/exec-approvals.ts +++ b/src/gateway/protocol/schema/exec-approvals.ts @@ -90,6 +90,19 @@ export const ExecApprovalRequestParamsSchema = Type.Object( id: Type.Optional(NonEmptyString), command: NonEmptyString, commandArgv: Type.Optional(Type.Array(Type.String())), + systemRunPlanV2: Type.Optional( + Type.Object( + { + version: Type.Literal(2), + argv: Type.Array(Type.String()), + cwd: Type.Union([Type.String(), Type.Null()]), + rawCommand: Type.Union([Type.String(), Type.Null()]), + agentId: Type.Union([Type.String(), Type.Null()]), + sessionKey: Type.Union([Type.String(), Type.Null()]), + }, + { additionalProperties: false }, + ), + ), env: Type.Optional(Type.Record(NonEmptyString, Type.String())), cwd: Type.Optional(Type.Union([Type.String(), Type.Null()])), nodeId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index 76806fb265d..707686539c8 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -3,7 +3,11 @@ import { DEFAULT_EXEC_APPROVAL_TIMEOUT_MS, type ExecApprovalDecision, } from "../../infra/exec-approvals.js"; -import { buildSystemRunApprovalBindingV1 } from "../../infra/system-run-approval-binding.js"; +import { + buildSystemRunApprovalBindingV1, + normalizeSystemRunApprovalPlanV2, +} from "../../infra/system-run-approval-binding.js"; +import { formatExecCommand } from "../../infra/system-run-command.js"; import type { ExecApprovalManager } from "../exec-approval-manager.js"; import { ErrorCodes, @@ -47,6 +51,7 @@ export function createExecApprovalHandlers( commandArgv?: string[]; env?: Record; cwd?: string; + systemRunPlanV2?: unknown; nodeId?: string; host?: string; security?: string; @@ -70,6 +75,18 @@ export function createExecApprovalHandlers( const commandArgv = Array.isArray(p.commandArgv) ? p.commandArgv.map((entry) => String(entry)) : undefined; + const systemRunPlanV2 = + host === "node" ? normalizeSystemRunApprovalPlanV2(p.systemRunPlanV2) : null; + const effectiveCommandArgv = systemRunPlanV2?.argv ?? commandArgv; + const effectiveCwd = systemRunPlanV2?.cwd ?? p.cwd; + const effectiveAgentId = systemRunPlanV2?.agentId ?? p.agentId; + const effectiveSessionKey = systemRunPlanV2?.sessionKey ?? p.sessionKey; + const effectiveCommandText = (() => { + if (!systemRunPlanV2) { + return p.command; + } + return systemRunPlanV2.rawCommand ?? formatExecCommand(systemRunPlanV2.argv); + })(); if (host === "node" && !nodeId) { respond( false, @@ -78,7 +95,10 @@ export function createExecApprovalHandlers( ); return; } - if (host === "node" && (!Array.isArray(commandArgv) || commandArgv.length === 0)) { + if ( + host === "node" && + (!Array.isArray(effectiveCommandArgv) || effectiveCommandArgv.length === 0) + ) { respond( false, undefined, @@ -89,10 +109,10 @@ export function createExecApprovalHandlers( const systemRunBindingV1 = host === "node" ? buildSystemRunApprovalBindingV1({ - argv: commandArgv, - cwd: p.cwd, - agentId: p.agentId, - sessionKey: p.sessionKey, + argv: effectiveCommandArgv, + cwd: effectiveCwd, + agentId: effectiveAgentId, + sessionKey: effectiveSessionKey, env: p.env, }) : null; @@ -105,18 +125,19 @@ export function createExecApprovalHandlers( return; } const request = { - command: p.command, - commandArgv, + command: effectiveCommandText, + commandArgv: effectiveCommandArgv, envKeys: systemRunBindingV1?.envKeys?.length ? systemRunBindingV1.envKeys : undefined, systemRunBindingV1: systemRunBindingV1?.binding ?? null, - cwd: p.cwd ?? null, + systemRunPlanV2: systemRunPlanV2, + cwd: effectiveCwd ?? null, nodeId: host === "node" ? nodeId : null, host: host || null, security: p.security ?? null, ask: p.ask ?? null, - agentId: p.agentId ?? null, + agentId: effectiveAgentId ?? null, resolvedPath: p.resolvedPath ?? null, - sessionKey: p.sessionKey ?? null, + sessionKey: effectiveSessionKey ?? null, turnSourceChannel: typeof p.turnSourceChannel === "string" ? p.turnSourceChannel.trim() || null : null, turnSourceTo: typeof p.turnSourceTo === "string" ? p.turnSourceTo.trim() || null : null, diff --git a/src/gateway/server-methods/server-methods.test.ts b/src/gateway/server-methods/server-methods.test.ts index c43d9a5cdf5..c6db927093a 100644 --- a/src/gateway/server-methods/server-methods.test.ts +++ b/src/gateway/server-methods/server-methods.test.ts @@ -471,6 +471,44 @@ describe("exec approval handlers", () => { ); }); + it("prefers systemRunPlanV2 canonical command/cwd when present", async () => { + const { handlers, broadcasts, respond, context } = createExecApprovalFixture(); + await requestExecApproval({ + handlers, + respond, + context, + params: { + command: "echo stale", + commandArgv: ["echo", "stale"], + cwd: "/tmp/link/sub", + systemRunPlanV2: { + version: 2, + argv: ["/usr/bin/echo", "ok"], + cwd: "/real/cwd", + rawCommand: "/usr/bin/echo ok", + agentId: "main", + sessionKey: "agent:main:main", + }, + }, + }); + const requested = broadcasts.find((entry) => entry.event === "exec.approval.requested"); + expect(requested).toBeTruthy(); + const request = (requested?.payload as { request?: Record })?.request ?? {}; + expect(request["command"]).toBe("/usr/bin/echo ok"); + expect(request["commandArgv"]).toEqual(["/usr/bin/echo", "ok"]); + expect(request["cwd"]).toBe("/real/cwd"); + expect(request["agentId"]).toBe("main"); + expect(request["sessionKey"]).toBe("agent:main:main"); + expect(request["systemRunPlanV2"]).toEqual({ + version: 2, + argv: ["/usr/bin/echo", "ok"], + cwd: "/real/cwd", + rawCommand: "/usr/bin/echo ok", + agentId: "main", + sessionKey: "agent:main:main", + }); + }); + it("accepts resolve during broadcast", async () => { const manager = new ExecApprovalManager(); const handlers = createExecApprovalHandlers(manager); diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index f08998fc756..b48a65e02ca 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -20,12 +20,22 @@ export type SystemRunApprovalBindingV1 = { envHash: string | null; }; +export type SystemRunApprovalPlanV2 = { + version: 2; + argv: string[]; + cwd: string | null; + rawCommand: string | null; + agentId: string | null; + sessionKey: string | null; +}; + export type ExecApprovalRequestPayload = { command: string; commandArgv?: string[]; // Optional UI-safe env key preview for approval prompts. envKeys?: string[]; systemRunBindingV1?: SystemRunApprovalBindingV1 | null; + systemRunPlanV2?: SystemRunApprovalPlanV2 | null; cwd?: string | null; nodeId?: string | null; host?: string | null; diff --git a/src/infra/system-run-approval-binding.ts b/src/infra/system-run-approval-binding.ts index fbfb390167a..a760f4948ef 100644 --- a/src/infra/system-run-approval-binding.ts +++ b/src/infra/system-run-approval-binding.ts @@ -1,5 +1,5 @@ import crypto from "node:crypto"; -import type { SystemRunApprovalBindingV1 } from "./exec-approvals.js"; +import type { SystemRunApprovalBindingV1, SystemRunApprovalPlanV2 } from "./exec-approvals.js"; import { normalizeEnvVarKey } from "./host-env-security.js"; type NormalizedSystemRunEnvEntry = [key: string, value: string]; @@ -16,6 +16,28 @@ function normalizeStringArray(value: unknown): string[] { return Array.isArray(value) ? value.map((entry) => String(entry)) : []; } +export function normalizeSystemRunApprovalPlanV2(value: unknown): SystemRunApprovalPlanV2 | null { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return null; + } + const candidate = value as Record; + if (candidate.version !== 2) { + return null; + } + const argv = normalizeStringArray(candidate.argv); + if (argv.length === 0) { + return null; + } + return { + version: 2, + argv, + cwd: normalizeString(candidate.cwd), + rawCommand: normalizeString(candidate.rawCommand), + agentId: normalizeString(candidate.agentId), + sessionKey: normalizeString(candidate.sessionKey), + }; +} + function normalizeSystemRunEnvEntries(env: unknown): NormalizedSystemRunEnvEntry[] { if (!env || typeof env !== "object" || Array.isArray(env)) { return []; diff --git a/src/node-host/invoke-system-run.test.ts b/src/node-host/invoke-system-run.test.ts index 2682edd2423..1ad04cc4b38 100644 --- a/src/node-host/invoke-system-run.test.ts +++ b/src/node-host/invoke-system-run.test.ts @@ -252,6 +252,39 @@ describe("handleSystemRunInvoke mac app exec host routing", () => { }, ); + it.runIf(process.platform !== "win32")( + "denies approval-based execution when cwd contains a symlink parent component", + async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-cwd-parent-link-")); + const safeRoot = path.join(tmp, "safe-root"); + const safeSub = path.join(safeRoot, "sub"); + const linkRoot = path.join(tmp, "approved-link"); + fs.mkdirSync(safeSub, { recursive: true }); + fs.symlinkSync(safeRoot, linkRoot, "dir"); + try { + const { runCommand, sendInvokeResult } = await runSystemInvoke({ + preferMacAppExecHost: false, + command: ["./run.sh"], + cwd: path.join(linkRoot, "sub"), + approved: true, + security: "full", + ask: "off", + }); + expect(runCommand).not.toHaveBeenCalled(); + expect(sendInvokeResult).toHaveBeenCalledWith( + expect.objectContaining({ + ok: false, + error: expect.objectContaining({ + message: expect.stringContaining("no symlink path components"), + }), + }), + ); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }, + ); + it("uses canonical executable path for approval-based relative command execution", async () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-cwd-real-")); const script = path.join(tmp, "run.sh"); diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 93edb85e0b7..5f358131dc0 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -16,6 +16,7 @@ import { type ExecAsk, type ExecCommandSegment, type ExecSecurity, + type SystemRunApprovalPlanV2, type SkillBinTrustEntry, } from "../infra/exec-approvals.js"; import type { ExecHostRequest, ExecHostResponse, ExecHostRunResult } from "../infra/exec-host.js"; @@ -113,6 +114,14 @@ function normalizeDeniedReason(reason: string | null | undefined): SystemRunDeni } } +function normalizeString(value: unknown): string | null { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + return trimmed ? trimmed : null; +} + function isPathLikeExecutableToken(value: string): boolean { if (!value) { return false; @@ -129,6 +138,46 @@ function isPathLikeExecutableToken(value: string): boolean { return false; } +function pathComponentsFromRootSync(targetPath: string): string[] { + const absolute = path.resolve(targetPath); + const parts: string[] = []; + let cursor = absolute; + while (true) { + parts.unshift(cursor); + const parent = path.dirname(cursor); + if (parent === cursor) { + return parts; + } + cursor = parent; + } +} + +function isWritableByCurrentProcessSync(candidate: string): boolean { + try { + fs.accessSync(candidate, fs.constants.W_OK); + return true; + } catch { + return false; + } +} + +function hasMutableSymlinkPathComponentSync(targetPath: string): boolean { + for (const component of pathComponentsFromRootSync(targetPath)) { + try { + if (!fs.lstatSync(component).isSymbolicLink()) { + continue; + } + const parentDir = path.dirname(component); + if (isWritableByCurrentProcessSync(parentDir)) { + return true; + } + } catch { + return true; + } + } + return false; +} + function hardenApprovedExecutionPaths(params: { approvedByAsk: boolean; argv: string[]; @@ -163,6 +212,12 @@ function hardenApprovedExecutionPaths(params: { message: "SYSTEM_RUN_DENIED: approval requires cwd to be a directory", }; } + if (hasMutableSymlinkPathComponentSync(requestedCwd)) { + return { + ok: false, + message: "SYSTEM_RUN_DENIED: approval requires canonical cwd (no symlink path components)", + }; + } if (cwdLstat.isSymbolicLink()) { return { ok: false, @@ -207,6 +262,46 @@ function hardenApprovedExecutionPaths(params: { return { ok: true, argv, cwd: hardenedCwd }; } +export function buildSystemRunApprovalPlanV2(params: { + command?: unknown; + rawCommand?: unknown; + cwd?: unknown; + agentId?: unknown; + sessionKey?: unknown; +}): { ok: true; plan: SystemRunApprovalPlanV2; cmdText: string } | { ok: false; message: string } { + const command = resolveSystemRunCommand({ + command: params.command, + rawCommand: params.rawCommand, + }); + if (!command.ok) { + return { ok: false, message: command.message }; + } + if (command.argv.length === 0) { + return { ok: false, message: "command required" }; + } + const hardening = hardenApprovedExecutionPaths({ + approvedByAsk: true, + argv: command.argv, + shellCommand: command.shellCommand, + cwd: normalizeString(params.cwd) ?? undefined, + }); + if (!hardening.ok) { + return { ok: false, message: hardening.message }; + } + return { + ok: true, + plan: { + version: 2, + argv: hardening.argv, + cwd: hardening.cwd ?? null, + rawCommand: command.cmdText.trim() || null, + agentId: normalizeString(params.agentId), + sessionKey: normalizeString(params.sessionKey), + }, + cmdText: command.cmdText, + }; +} + export type HandleSystemRunInvokeOptions = { client: GatewayClient; params: SystemRunParams; diff --git a/src/node-host/invoke.ts b/src/node-host/invoke.ts index c6d5d2ccc8a..7d7b21ad474 100644 --- a/src/node-host/invoke.ts +++ b/src/node-host/invoke.ts @@ -20,7 +20,7 @@ import { } from "../infra/exec-host.js"; import { sanitizeHostExecEnv } from "../infra/host-env-security.js"; import { runBrowserProxyCommand } from "./invoke-browser.js"; -import { handleSystemRunInvoke } from "./invoke-system-run.js"; +import { buildSystemRunApprovalPlanV2, handleSystemRunInvoke } from "./invoke-system-run.js"; import type { ExecEventPayload, RunResult, @@ -420,6 +420,30 @@ export async function handleInvoke( return; } + if (command === "system.run.prepare") { + try { + const params = decodeParams<{ + command?: unknown; + rawCommand?: unknown; + cwd?: unknown; + agentId?: unknown; + sessionKey?: unknown; + }>(frame.paramsJSON); + const prepared = buildSystemRunApprovalPlanV2(params); + if (!prepared.ok) { + await sendErrorResult(client, frame, "INVALID_REQUEST", prepared.message); + return; + } + await sendJsonPayloadResult(client, frame, { + cmdText: prepared.cmdText, + plan: prepared.plan, + }); + } catch (err) { + await sendInvalidRequestResult(client, frame, err); + } + return; + } + if (command !== "system.run") { await sendErrorResult(client, frame, "UNAVAILABLE", "command not supported"); return; diff --git a/src/node-host/runner.ts b/src/node-host/runner.ts index edf2cc12215..1b4114071bd 100644 --- a/src/node-host/runner.ts +++ b/src/node-host/runner.ts @@ -189,6 +189,7 @@ export async function runNodeHost(opts: NodeHostRunOptions): Promise { scopes: [], caps: ["system", ...(browserProxyEnabled ? ["browser"] : [])], commands: [ + "system.run.prepare", "system.run", "system.which", "system.execApprovals.get",