From ec2c6d83b9f5f91d6d9094842e0f19b88e63e3e2 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 15 Mar 2026 08:47:17 -0700 Subject: [PATCH] Nodes: recheck queued actions before delivery (#46815) * Nodes: recheck queued actions before delivery * Nodes tests: cover pull-time policy recheck * Nodes tests: type node policy mocks explicitly --- CHANGELOG.md | 1 + .../server-methods/nodes.invoke-wake.test.ts | 64 ++++++++++++++++++- src/gateway/server-methods/nodes.ts | 35 +++++++++- 3 files changed, 97 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fa449296ea..a1fd84a09ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Docs: https://docs.openclaw.ai - Docs/Mintlify: fix MDX marker syntax on Perplexity, Model Providers, Moonshot, and exec approvals pages so local docs preview no longer breaks rendering or leaves stale pages unpublished. (#46695) Thanks @velvet-shark. - Email/webhook wrapping: sanitize sender and subject metadata before external-content wrapping so metadata fields cannot break the wrapper structure. Thanks @vincentkoc. - Node/startup: remove leftover debug `console.log("node host PATH: ...")` that printed the resolved PATH on every `openclaw node run` invocation. (#46411) +- Nodes/pending actions: re-check queued foreground actions against the current node command policy before returning them to the node. Thanks @vincentkoc. - ACP/approvals: use canonical tool identity for prompting decisions and fail closed when conflicting tool identity hints are present. Thanks @vincentkoc. - Telegram/message send: forward `--force-document` through the `sendPayload` path as well as `sendMedia`, so Telegram payload sends with `channelData` keep uploading images as documents instead of silently falling back to compressed photo sends. (#47119) Thanks @thepagent. - Telegram/message chunking: preserve spaces, paragraph separators, and word boundaries when HTML overflow rechunking splits formatted replies. (#47274) diff --git a/src/gateway/server-methods/nodes.invoke-wake.test.ts b/src/gateway/server-methods/nodes.invoke-wake.test.ts index fc01f718bbb..ea29384698c 100644 --- a/src/gateway/server-methods/nodes.invoke-wake.test.ts +++ b/src/gateway/server-methods/nodes.invoke-wake.test.ts @@ -2,10 +2,18 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ErrorCodes } from "../protocol/index.js"; import { maybeWakeNodeWithApns, nodeHandlers } from "./nodes.js"; +type MockNodeCommandPolicyParams = { + command: string; + declaredCommands?: string[]; + allowlist: Set; +}; + const mocks = vi.hoisted(() => ({ loadConfig: vi.fn(() => ({})), - resolveNodeCommandAllowlist: vi.fn(() => []), - isNodeCommandAllowed: vi.fn(() => ({ ok: true })), + resolveNodeCommandAllowlist: vi.fn<() => Set>(() => new Set()), + isNodeCommandAllowed: vi.fn< + (params: MockNodeCommandPolicyParams) => { ok: true } | { ok: false; reason: string } + >(() => ({ ok: true })), sanitizeNodeInvokeParamsForForwarding: vi.fn(({ rawParams }: { rawParams: unknown }) => ({ ok: true, params: rawParams, @@ -518,6 +526,58 @@ describe("node.invoke APNs wake path", () => { }); }); + it("drops queued actions that are no longer allowed at pull time", async () => { + mocks.loadApnsRegistration.mockResolvedValue(null); + const allowlistedCommands = new Set(["camera.snap", "canvas.navigate"]); + mocks.resolveNodeCommandAllowlist.mockImplementation(() => new Set(allowlistedCommands)); + mocks.isNodeCommandAllowed.mockImplementation( + ({ command, declaredCommands, allowlist }: MockNodeCommandPolicyParams) => { + if (!allowlist.has(command)) { + return { ok: false, reason: "command not allowlisted" }; + } + if (!declaredCommands.includes(command)) { + return { ok: false, reason: "command not declared by node" }; + } + return { ok: true }; + }, + ); + + const nodeRegistry = { + get: vi.fn(() => ({ + nodeId: "ios-node-policy", + commands: ["camera.snap", "canvas.navigate"], + platform: "iOS 26.4.0", + })), + invoke: vi.fn().mockResolvedValue({ + ok: false, + error: { + code: "NODE_BACKGROUND_UNAVAILABLE", + message: "NODE_BACKGROUND_UNAVAILABLE: canvas/camera/screen commands require foreground", + }, + }), + }; + + await invokeNode({ + nodeRegistry, + requestParams: { + nodeId: "ios-node-policy", + command: "camera.snap", + params: { facing: "front" }, + idempotencyKey: "idem-policy", + }, + }); + + allowlistedCommands.delete("camera.snap"); + + const pullRespond = await pullPending("ios-node-policy"); + const pullCall = pullRespond.mock.calls[0] as RespondCall | undefined; + expect(pullCall?.[0]).toBe(true); + expect(pullCall?.[1]).toMatchObject({ + nodeId: "ios-node-policy", + actions: [], + }); + }); + it("dedupes queued foreground actions by idempotency key", async () => { mocks.loadApnsRegistration.mockResolvedValue(null); diff --git a/src/gateway/server-methods/nodes.ts b/src/gateway/server-methods/nodes.ts index 7f78809abbb..ae6c8090b6c 100644 --- a/src/gateway/server-methods/nodes.ts +++ b/src/gateway/server-methods/nodes.ts @@ -26,6 +26,7 @@ import { import { isNodeCommandAllowed, resolveNodeCommandAllowlist } from "../node-command-policy.js"; import { sanitizeNodeInvokeParamsForForwarding } from "../node-invoke-sanitize.js"; import { + type ConnectParams, ErrorCodes, errorShape, validateNodeDescribeParams, @@ -218,6 +219,38 @@ function listPendingNodeActions(nodeId: string): PendingNodeAction[] { return prunePendingNodeActions(nodeId, Date.now()); } +function resolveAllowedPendingNodeActions(params: { + nodeId: string; + client: { connect?: ConnectParams | null } | null; +}): PendingNodeAction[] { + const pending = listPendingNodeActions(params.nodeId); + if (pending.length === 0) { + return pending; + } + const connect = params.client?.connect; + const declaredCommands = Array.isArray(connect?.commands) ? connect.commands : []; + const allowlist = resolveNodeCommandAllowlist(loadConfig(), { + platform: connect?.client?.platform, + deviceFamily: connect?.client?.deviceFamily, + }); + const allowed = pending.filter((entry) => { + const result = isNodeCommandAllowed({ + command: entry.command, + declaredCommands, + allowlist, + }); + return result.ok; + }); + if (allowed.length !== pending.length) { + if (allowed.length === 0) { + pendingNodeActionsById.delete(params.nodeId); + } else { + pendingNodeActionsById.set(params.nodeId, allowed); + } + } + return allowed; +} + function ackPendingNodeActions(nodeId: string, ids: string[]): PendingNodeAction[] { if (ids.length === 0) { return listPendingNodeActions(nodeId); @@ -805,7 +838,7 @@ export const nodeHandlers: GatewayRequestHandlers = { return; } - const pending = listPendingNodeActions(trimmedNodeId); + const pending = resolveAllowedPendingNodeActions({ nodeId: trimmedNodeId, client }); respond( true, {