diff --git a/CHANGELOG.md b/CHANGELOG.md index addeec3606a..3800ba43189 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,6 +124,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Exec approvals/node: let trusted backend node invokes complete no-device Control UI approvals after the original request connection changes, while keeping node, command, cwd, env, and allow-once replay bindings enforced. Fixes #78569. Thanks @naturedogdog. - CLI/completion: guard the shell-profile source line written by `openclaw completion --install` with a file existence check (`[ -f ... ] && source ...` for bash/zsh, `test -f ...; and source ...` for fish) so uninstalling OpenClaw no longer makes new login shells error on a missing completion cache. (#78659) Thanks @sjf. - Cron/doctor: repair persisted cron jobs whose `payload.model` was stored as `"default"`, `"null"`, blank, or JSON `null` by removing the bad override during `openclaw doctor --fix` while keeping cron runtime model validation strict. Fixes #78549. Thanks @bizzle12368239. - Telegram: honor `accessGroup:*` sender allowlists for DMs, groups, native commands, and callback authorization before applying Telegram's numeric sender-ID checks. Fixes #78660. Thanks @manugc. diff --git a/src/agents/bash-tools.exec-host-node.test.ts b/src/agents/bash-tools.exec-host-node.test.ts index 8e58c4593ba..08be30eb17e 100644 --- a/src/agents/bash-tools.exec-host-node.test.ts +++ b/src/agents/bash-tools.exec-host-node.test.ts @@ -297,6 +297,7 @@ describe("executeNodeHostCommand", () => { timeoutMs: 30_000, }), }), + { scopes: ["operator.write", "operator.approvals"] }, ); }); @@ -355,6 +356,7 @@ describe("executeNodeHostCommand", () => { systemRunPlan: expectedPlan, }), }), + { scopes: ["operator.write", "operator.approvals"] }, ); }); }); diff --git a/src/agents/bash-tools.exec-host-node.ts b/src/agents/bash-tools.exec-host-node.ts index 52ddd97fcb2..dbf5f5f225c 100644 --- a/src/agents/bash-tools.exec-host-node.ts +++ b/src/agents/bash-tools.exec-host-node.ts @@ -1,4 +1,5 @@ import type { AgentToolResult } from "@mariozechner/pi-agent-core"; +import { APPROVALS_SCOPE, WRITE_SCOPE } from "../gateway/operator-scopes.js"; import { requiresExecApproval, resolveExecApprovalAllowedDecisions, @@ -29,6 +30,8 @@ import { callGatewayTool } from "./tools/gateway.js"; export type { ExecuteNodeHostCommandParams } from "./bash-tools.exec-host-node.types.js"; +const APPROVED_NODE_INVOKE_SCOPES = [WRITE_SCOPE, APPROVALS_SCOPE]; + export async function executeNodeHostCommand( params: ExecuteNodeHostCommandParams, ): Promise> { @@ -225,6 +228,7 @@ export async function executeNodeHostCommand( notifyOnExit: params.notifyOnExit, systemRunPlan: prepared.plan, }), + { scopes: APPROVED_NODE_INVOKE_SCOPES }, ); const payload = raw?.payload && typeof raw.payload === "object" @@ -271,22 +275,24 @@ export async function executeNodeHostCommand( } const startedAt = Date.now(); - const raw = await callGatewayTool( - "node.invoke", - { timeoutMs: target.invokeTimeoutMs }, - buildNodeSystemRunInvoke({ - target, - command: prepared.argv, - rawCommand: prepared.rawCommand, - cwd: prepared.cwd, - agentId: prepared.agentId, - sessionKey: prepared.sessionKey, - approved: inlineApprovedByAsk, - approvalDecision: inlineApprovalDecision, - runId: inlineApprovalId, - notifyOnExit: params.notifyOnExit, - systemRunPlan: prepared.plan, - }), - ); + const invoke = buildNodeSystemRunInvoke({ + target, + command: prepared.argv, + rawCommand: prepared.rawCommand, + cwd: prepared.cwd, + agentId: prepared.agentId, + sessionKey: prepared.sessionKey, + approved: inlineApprovedByAsk, + approvalDecision: inlineApprovalDecision, + runId: inlineApprovalId, + notifyOnExit: params.notifyOnExit, + systemRunPlan: prepared.plan, + }); + const raw = + inlineApprovedByAsk && inlineApprovalId + ? await callGatewayTool("node.invoke", { timeoutMs: target.invokeTimeoutMs }, invoke, { + scopes: APPROVED_NODE_INVOKE_SCOPES, + }) + : await callGatewayTool("node.invoke", { timeoutMs: target.invokeTimeoutMs }, invoke); return formatNodeRunToolResult({ raw, startedAt, cwd: params.workdir }); } diff --git a/src/gateway/exec-approval-manager.ts b/src/gateway/exec-approval-manager.ts index 197fd0a670a..697a2288d11 100644 --- a/src/gateway/exec-approval-manager.ts +++ b/src/gateway/exec-approval-manager.ts @@ -31,6 +31,7 @@ export type ExecApprovalRecord = { requestedByConnId?: string | null; requestedByDeviceId?: string | null; requestedByClientId?: string | null; + requestedByDeviceTokenAuth?: boolean; resolvedAtMs?: number; decision?: ExecApprovalDecision; consumedDecision?: ExecApprovalDecision; diff --git a/src/gateway/node-invoke-system-run-approval.test.ts b/src/gateway/node-invoke-system-run-approval.test.ts index 496635f0cf2..bef7564a46e 100644 --- a/src/gateway/node-invoke-system-run-approval.test.ts +++ b/src/gateway/node-invoke-system-run-approval.test.ts @@ -12,8 +12,16 @@ describe("sanitizeSystemRunParamsForForwarding", () => { connId: "conn-1", connect: { scopes: ["operator.write", "operator.approvals"], + client: { id: "cli-1", mode: "cli" }, device: { id: "dev-1" }, - client: { id: "cli-1" }, + }, + }; + const trustedBackendClient = { + connId: "backend-conn", + connect: { + scopes: ["operator.write", "operator.approvals"], + client: { id: "gateway-client", mode: "backend" }, + device: null, }, }; @@ -45,6 +53,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { requestedByConnId: "conn-1", requestedByDeviceId: "dev-1", requestedByClientId: "cli-1", + requestedByDeviceTokenAuth: false, resolvedAtMs: now - 500, decision: "allow-once", resolvedBy: "operator", @@ -361,6 +370,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { record.requestedByConnId = "conn-1"; record.requestedByDeviceId = "dev-1"; record.requestedByClientId = "cli-1"; + record.requestedByDeviceTokenAuth = false; const decisionPromise = approvalManager.register(record, 60_000); approvalManager.resolve(runId, "allow-once", "operator"); @@ -426,4 +436,113 @@ describe("sanitizeSystemRunParamsForForwarding", () => { }); expectRejectedForwardingResult(result, "APPROVAL_NODE_MISMATCH", "not valid for this node"); }); + + test("accepts trusted backend replay for no-device approval after the request connection changes", () => { + const record = makeRecord("echo SAFE", ["echo", "SAFE"]); + record.requestedByConnId = "control-ui-conn"; + record.requestedByDeviceId = null; + record.requestedByClientId = "openclaw-control-ui"; + record.requestedByDeviceTokenAuth = false; + + const result = sanitizeSystemRunParamsForForwarding({ + rawParams: { + command: ["echo", "SAFE"], + rawCommand: "echo SAFE", + runId: "approval-1", + approved: true, + approvalDecision: "allow-once", + }, + nodeId: "node-1", + client: trustedBackendClient, + execApprovalManager: manager(record), + nowMs: now, + }); + + expectAllowOnceForwardingResult(result); + }); + + test("rejects no-device approval replay from a backend client without approval scope", () => { + const record = makeRecord("echo SAFE", ["echo", "SAFE"]); + record.requestedByConnId = "control-ui-conn"; + record.requestedByDeviceId = null; + record.requestedByClientId = "openclaw-control-ui"; + record.requestedByDeviceTokenAuth = false; + + const result = sanitizeSystemRunParamsForForwarding({ + rawParams: { + command: ["echo", "SAFE"], + rawCommand: "echo SAFE", + runId: "approval-1", + approved: true, + approvalDecision: "allow-once", + }, + nodeId: "node-1", + client: { + ...trustedBackendClient, + connect: { + ...trustedBackendClient.connect, + scopes: ["operator.write"], + }, + }, + execApprovalManager: manager(record), + nowMs: now, + }); + + expectRejectedForwardingResult(result, "APPROVAL_CLIENT_MISMATCH", "not valid for this client"); + }); + + test("rejects no-device approval replay from a non-backend client on a different connection", () => { + const record = makeRecord("echo SAFE", ["echo", "SAFE"]); + record.requestedByConnId = "control-ui-conn"; + record.requestedByDeviceId = null; + record.requestedByClientId = "openclaw-control-ui"; + record.requestedByDeviceTokenAuth = false; + + const result = sanitizeSystemRunParamsForForwarding({ + rawParams: { + command: ["echo", "SAFE"], + rawCommand: "echo SAFE", + runId: "approval-1", + approved: true, + approvalDecision: "allow-once", + }, + nodeId: "node-1", + client: { + connId: "other-control-ui-conn", + connect: { + scopes: ["operator.write", "operator.approvals"], + client: { id: "openclaw-control-ui", mode: "ui" }, + device: null, + }, + }, + execApprovalManager: manager(record), + nowMs: now, + }); + + expectRejectedForwardingResult(result, "APPROVAL_CLIENT_MISMATCH", "not valid for this client"); + }); + + test("rejects no-device approval replay when the original request used device-token auth", () => { + const record = makeRecord("echo SAFE", ["echo", "SAFE"]); + record.requestedByConnId = "control-ui-conn"; + record.requestedByDeviceId = null; + record.requestedByClientId = "openclaw-control-ui"; + record.requestedByDeviceTokenAuth = true; + + const result = sanitizeSystemRunParamsForForwarding({ + rawParams: { + command: ["echo", "SAFE"], + rawCommand: "echo SAFE", + runId: "approval-1", + approved: true, + approvalDecision: "allow-once", + }, + nodeId: "node-1", + client: trustedBackendClient, + execApprovalManager: manager(record), + nowMs: now, + }); + + expectRejectedForwardingResult(result, "APPROVAL_CLIENT_MISMATCH", "not valid for this client"); + }); }); diff --git a/src/gateway/node-invoke-system-run-approval.ts b/src/gateway/node-invoke-system-run-approval.ts index 72f064877e0..41bb259e10c 100644 --- a/src/gateway/node-invoke-system-run-approval.ts +++ b/src/gateway/node-invoke-system-run-approval.ts @@ -11,6 +11,7 @@ import { evaluateSystemRunApprovalMatch, toSystemRunApprovalMismatchError, } from "./node-invoke-system-run-approval-match.js"; +import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "./protocol/client-info.js"; type SystemRunParamsLike = { command?: unknown; @@ -35,12 +36,21 @@ type ApprovalLookup = { type ApprovalClient = { connId?: string | null; + isDeviceTokenAuth?: boolean; connect?: { scopes?: unknown; + client?: { id?: string | null; mode?: string | null } | null; device?: { id?: string | null } | null; } | null; }; +const BACKEND_BRIDGEABLE_NO_DEVICE_REQUEST_CLIENT_IDS = new Set([ + GATEWAY_CLIENT_NAMES.CONTROL_UI, + GATEWAY_CLIENT_NAMES.WEBCHAT_UI, + GATEWAY_CLIENT_NAMES.WEBCHAT, + GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT, +]); + function normalizeApprovalDecision(value: unknown): "allow-once" | "allow-always" | null { const s = normalizeNullableString(value); return s === "allow-once" || s === "allow-always" ? s : null; @@ -51,6 +61,29 @@ function clientHasApprovals(client: ApprovalClient | null): boolean { return scopes.includes("operator.admin") || scopes.includes("operator.approvals"); } +function isTrustedBackendApprovalClient(client: ApprovalClient | null): boolean { + return ( + clientHasApprovals(client) && + client?.connect?.client?.id === GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT && + client.connect.client.mode === GATEWAY_CLIENT_MODES.BACKEND && + client.isDeviceTokenAuth !== true + ); +} + +function canBridgeNoDeviceApprovalFromBackend(params: { + snapshot: ExecApprovalRecord; + client: ApprovalClient | null; +}): boolean { + const requestedByClientId = normalizeNullableString(params.snapshot.requestedByClientId); + return ( + params.snapshot.requestedByDeviceId == null && + params.snapshot.requestedByDeviceTokenAuth !== true && + requestedByClientId !== null && + BACKEND_BRIDGEABLE_NO_DEVICE_REQUEST_CLIENT_IDS.has(requestedByClientId) && + isTrustedBackendApprovalClient(params.client) + ); +} + function pickSystemRunParams(raw: Record): Record { // Defensive allowlist: only forward fields that the node-host `system.run` handler understands. // This prevents future internal control fields from being smuggled through the gateway. @@ -190,7 +223,8 @@ export function sanitizeSystemRunParamsForForwarding(opts: { } } else if ( snapshot.requestedByConnId && - snapshot.requestedByConnId !== (opts.client?.connId ?? null) + snapshot.requestedByConnId !== (opts.client?.connId ?? null) && + !canBridgeNoDeviceApprovalFromBackend({ snapshot, client: opts.client }) ) { return systemRunApprovalGuardError({ code: "APPROVAL_CLIENT_MISMATCH", diff --git a/src/gateway/server-methods/exec-approval.ts b/src/gateway/server-methods/exec-approval.ts index 5bafbb2d8b9..1e33610da67 100644 --- a/src/gateway/server-methods/exec-approval.ts +++ b/src/gateway/server-methods/exec-approval.ts @@ -263,6 +263,7 @@ export function createExecApprovalHandlers( record.requestedByConnId = client?.connId ?? null; record.requestedByDeviceId = client?.connect?.device?.id ?? null; record.requestedByClientId = client?.connect?.client?.id ?? null; + record.requestedByDeviceTokenAuth = client?.isDeviceTokenAuth === true; // Use register() to synchronously add to pending map before sending any response. // This ensures the approval ID is valid immediately after the "accepted" response. let decisionPromise: Promise<