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
This commit is contained in:
parent
ff61343d76
commit
ec2c6d83b9
@ -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)
|
||||
|
||||
@ -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<string>;
|
||||
};
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
loadConfig: vi.fn(() => ({})),
|
||||
resolveNodeCommandAllowlist: vi.fn(() => []),
|
||||
isNodeCommandAllowed: vi.fn(() => ({ ok: true })),
|
||||
resolveNodeCommandAllowlist: vi.fn<() => Set<string>>(() => 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);
|
||||
|
||||
|
||||
@ -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,
|
||||
{
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user