From ca2fdcc45f36335c1c5719fb699f4f90b351bfc2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Apr 2026 17:46:15 +0900 Subject: [PATCH] fix: enforce node pairing approval scopes end-to-end (#60461) (thanks @eleqtrizit) --- CHANGELOG.md | 1 + src/agents/tools/nodes-tool.test.ts | 11 +++- src/agents/tools/nodes-tool.ts | 17 +----- src/gateway/server.node-pairing-authz.test.ts | 58 ++++++++++++++++++- src/infra/node-pairing.test.ts | 47 ++++++++++++++- src/infra/node-pairing.ts | 57 +++++++++--------- 6 files changed, 143 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fd57da3a68..14f16a99f33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -165,6 +165,7 @@ Docs: https://docs.openclaw.ai - TUI/chat: keep pending local sends visible and reconciled across history reloads, make busy/error recovery clearer through fallback and terminal-error paths, and reclaim transcript width for long links and paths. (#59800) Thanks @vincentkoc. - Exec approvals/channels: decouple initiating-surface approval availability from native delivery enablement so Telegram, Slack, and Discord still expose approvals when approvers exist and native target routing is configured separately. (#59776) Thanks @joelnishanth. - Agents/logging: keep orphaned-user transcript repair warnings focused on interactive runs, and downgrade background-trigger repairs (`heartbeat`, `cron`, `memory`, `overflow`) to debug logs to reduce false-alarm gateway noise. +- Gateway/node pairing: require `operator.pairing` for node approvals end-to-end, while still requiring `operator.write` or `operator.admin` when the pending node commands need those higher scopes. (#60461) Thanks @eleqtrizit. ## 2026.4.1 diff --git a/src/agents/tools/nodes-tool.test.ts b/src/agents/tools/nodes-tool.test.ts index 213c53d2d86..e8d5868157f 100644 --- a/src/agents/tools/nodes-tool.test.ts +++ b/src/agents/tools/nodes-tool.test.ts @@ -240,7 +240,7 @@ describe("createNodesTool screen_record duration guardrails", () => { "node.pair.list", {}, {}, - { scopes: ["operator.pairing", "operator.write"] }, + { scopes: ["operator.pairing"] }, ); expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith( 2, @@ -280,7 +280,7 @@ describe("createNodesTool screen_record duration guardrails", () => { "node.pair.list", {}, {}, - { scopes: ["operator.pairing", "operator.write"] }, + { scopes: ["operator.pairing"] }, ); expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith( 2, @@ -314,6 +314,13 @@ describe("createNodesTool screen_record duration guardrails", () => { requestId: "req-1", }); + expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith( + 1, + "node.pair.list", + {}, + {}, + { scopes: ["operator.pairing"] }, + ); expect(gatewayMocks.callGatewayTool).toHaveBeenNthCalledWith( 2, "node.pair.approve", diff --git a/src/agents/tools/nodes-tool.ts b/src/agents/tools/nodes-tool.ts index ace7c328b6c..326cc12d02d 100644 --- a/src/agents/tools/nodes-tool.ts +++ b/src/agents/tools/nodes-tool.ts @@ -2,7 +2,7 @@ import crypto from "node:crypto"; import { Type } from "@sinclair/typebox"; import type { OpenClawConfig } from "../../config/config.js"; import type { OperatorScope } from "../../gateway/method-scopes.js"; -import { NODE_SYSTEM_RUN_COMMANDS } from "../../infra/node-commands.js"; +import { resolveNodePairApprovalScopes } from "../../infra/node-pairing.js"; import type { GatewayMessageChannel } from "../../utils/message-channel.js"; import { resolveSessionAgentId } from "../agent-scope.js"; import { resolveImageSanitizationLimits } from "../image-sanitization.js"; @@ -43,18 +43,7 @@ const LOCATION_ACCURACY = ["coarse", "balanced", "precise"] as const; type GatewayCallOptions = ReturnType; function resolveApproveScopes(commands: unknown): OperatorScope[] { - const normalized = Array.isArray(commands) - ? commands.filter((value): value is string => typeof value === "string") - : []; - if ( - normalized.some((command) => NODE_SYSTEM_RUN_COMMANDS.some((allowed) => allowed === command)) - ) { - return ["operator.pairing", "operator.admin"]; - } - if (normalized.length > 0) { - return ["operator.pairing", "operator.write"]; - } - return ["operator.pairing"]; + return resolveNodePairApprovalScopes(commands) as OperatorScope[]; } async function resolveNodePairApproveScopes( @@ -63,7 +52,7 @@ async function resolveNodePairApproveScopes( ): Promise { const pairing = await callGatewayTool<{ pending?: Array<{ requestId?: string; commands?: unknown }>; - }>("node.pair.list", gatewayOpts, {}, { scopes: ["operator.pairing", "operator.write"] }); + }>("node.pair.list", gatewayOpts, {}, { scopes: ["operator.pairing"] }); const pending = Array.isArray(pairing?.pending) ? pairing.pending : []; const match = pending.find((entry) => entry?.requestId === requestId); return resolveApproveScopes(match?.commands); diff --git a/src/gateway/server.node-pairing-authz.test.ts b/src/gateway/server.node-pairing-authz.test.ts index bb4b73b217d..cf6c1c5e267 100644 --- a/src/gateway/server.node-pairing-authz.test.ts +++ b/src/gateway/server.node-pairing-authz.test.ts @@ -126,6 +126,56 @@ describe("gateway node pairing authorization", () => { } }); + test("allows pairing-only operators to approve commandless node requests", async () => { + const started = await startServerWithClient("secret"); + const approver = await issueOperatorToken({ + name: "node-pair-approve-commandless", + approvedScopes: ["operator.admin"], + tokenScopes: ["operator.pairing"], + clientId: GATEWAY_CLIENT_NAMES.TEST, + clientMode: GATEWAY_CLIENT_MODES.TEST, + }); + + let pairingWs: WebSocket | undefined; + try { + const request = await requestNodePairing({ + nodeId: "node-approve-target", + platform: "darwin", + }); + + pairingWs = await openTrackedWs(started.port); + await connectOk(pairingWs, { + skipDefaultAuth: true, + deviceToken: approver.token, + deviceIdentityPath: approver.identityPath, + scopes: ["operator.pairing"], + }); + + const approve = await rpcReq<{ + requestId?: string; + node?: { nodeId?: string }; + }>(pairingWs, "node.pair.approve", { + requestId: request.request.requestId, + }); + expect(approve.ok).toBe(true); + expect(approve.payload?.requestId).toBe(request.request.requestId); + expect(approve.payload?.node?.nodeId).toBe("node-approve-target"); + + await expect( + import("../infra/node-pairing.js").then((m) => m.getPairedNode("node-approve-target")), + ).resolves.toEqual( + expect.objectContaining({ + nodeId: "node-approve-target", + }), + ); + } finally { + pairingWs?.close(); + started.ws.close(); + await started.server.close(); + started.envSnapshot.restore(); + } + }); + test("does not pin connected node commands to the approved pairing record", async () => { const started = await startServerWithClient("secret"); const pairedNode = await pairDeviceIdentity({ @@ -155,7 +205,9 @@ describe("gateway node pairing authorization", () => { platform: "darwin", commands: ["canvas.snapshot"], }); - await approveNodePairing(request.request.requestId); + await approveNodePairing(request.request.requestId, { + callerScopes: ["operator.pairing", "operator.write"], + }); nodeClient = await connectNodeClient({ port: started.port, @@ -217,7 +269,9 @@ describe("gateway node pairing authorization", () => { nodeId: pairedNode.identity.deviceId, platform: "darwin", }); - await approveNodePairing(initialApproval.request.requestId); + await approveNodePairing(initialApproval.request.requestId, { + callerScopes: ["operator.pairing"], + }); nodeClient = await connectNodeClient({ port: started.port, diff --git a/src/infra/node-pairing.test.ts b/src/infra/node-pairing.test.ts index 6103252c975..603f0edbe67 100644 --- a/src/infra/node-pairing.test.ts +++ b/src/infra/node-pairing.test.ts @@ -18,7 +18,11 @@ async function setupPairedNode(baseDir: string): Promise { }, baseDir, ); - await approveNodePairing(request.request.requestId, baseDir); + await approveNodePairing( + request.request.requestId, + { callerScopes: ["operator.pairing", "operator.admin"] }, + baseDir, + ); const paired = await getPairedNode("node-1", baseDir); expect(paired).not.toBeNull(); if (!paired) { @@ -165,6 +169,16 @@ describe("node pairing tokens", () => { }); await expect( approveNodePairing(request.request.requestId, { callerScopes: ["operator.write"] }, baseDir), + ).resolves.toEqual({ + status: "forbidden", + missingScope: "operator.pairing", + }); + await expect( + approveNodePairing( + request.request.requestId, + { callerScopes: ["operator.pairing", "operator.write"] }, + baseDir, + ), ).resolves.toEqual({ requestId: request.request.requestId, node: expect.objectContaining({ @@ -173,4 +187,35 @@ describe("node pairing tokens", () => { }), }); }); + + test("requires operator.pairing to approve commandless node requests", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-node-pairing-")); + const request = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + }, + baseDir, + ); + + await expect( + approveNodePairing(request.request.requestId, { callerScopes: [] }, baseDir), + ).resolves.toEqual({ + status: "forbidden", + missingScope: "operator.pairing", + }); + await expect( + approveNodePairing( + request.request.requestId, + { callerScopes: ["operator.pairing"] }, + baseDir, + ), + ).resolves.toEqual({ + requestId: request.request.requestId, + node: expect.objectContaining({ + nodeId: "node-1", + commands: undefined, + }), + }); + }); }); diff --git a/src/infra/node-pairing.ts b/src/infra/node-pairing.ts index 327266c7867..4b538fff275 100644 --- a/src/infra/node-pairing.ts +++ b/src/infra/node-pairing.ts @@ -59,6 +59,7 @@ type NodePairingStateFile = { const PENDING_TTL_MS = 5 * 60 * 1000; const OPERATOR_ROLE = "operator"; +const OPERATOR_PAIRING_SCOPE = "operator.pairing"; const OPERATOR_WRITE_SCOPE = "operator.write"; const OPERATOR_ADMIN_SCOPE = "operator.admin"; @@ -118,15 +119,24 @@ function refreshPendingNodePairingRequest( }; } -function resolveNodeApprovalRequiredScope(pending: NodePairingPendingRequest): string | null { +export function resolveNodePairApprovalScopes(commands: unknown): string[] { + const normalized = Array.isArray(commands) + ? commands.filter((command): command is string => typeof command === "string") + : []; + if ( + normalized.some((command) => NODE_SYSTEM_RUN_COMMANDS.some((allowed) => allowed === command)) + ) { + return [OPERATOR_PAIRING_SCOPE, OPERATOR_ADMIN_SCOPE]; + } + if (normalized.length > 0) { + return [OPERATOR_PAIRING_SCOPE, OPERATOR_WRITE_SCOPE]; + } + return [OPERATOR_PAIRING_SCOPE]; +} + +function resolveNodeApprovalRequiredScopes(pending: NodePairingPendingRequest): string[] { const commands = Array.isArray(pending.commands) ? pending.commands : []; - if (commands.some((command) => NODE_SYSTEM_RUN_COMMANDS.some((allowed) => allowed === command))) { - return OPERATOR_ADMIN_SCOPE; - } - if (commands.length > 0) { - return OPERATOR_WRITE_SCOPE; - } - return null; + return resolveNodePairApprovalScopes(commands); } type ApprovedNodePairingResult = { requestId: string; node: NodePairingPairedNode }; @@ -220,10 +230,6 @@ export async function requestNodePairing( }); } -export async function approveNodePairing( - requestId: string, - baseDir?: string, -): Promise; export async function approveNodePairing( requestId: string, options: { callerScopes?: readonly string[] }, @@ -231,30 +237,23 @@ export async function approveNodePairing( ): Promise; export async function approveNodePairing( requestId: string, - optionsOrBaseDir?: { callerScopes?: readonly string[] } | string, - maybeBaseDir?: string, + options: { callerScopes?: readonly string[] }, + baseDir?: string, ): Promise { - const options = - typeof optionsOrBaseDir === "string" || optionsOrBaseDir === undefined - ? undefined - : optionsOrBaseDir; - const baseDir = typeof optionsOrBaseDir === "string" ? optionsOrBaseDir : maybeBaseDir; return await withLock(async () => { const state = await loadState(baseDir); const pending = state.pendingById[requestId]; if (!pending) { return null; } - const requiredScope = resolveNodeApprovalRequiredScope(pending); - if (requiredScope && options !== undefined) { - const missingScope = resolveMissingRequestedScope({ - role: OPERATOR_ROLE, - requestedScopes: [requiredScope], - allowedScopes: options.callerScopes ?? [], - }); - if (missingScope) { - return { status: "forbidden", missingScope }; - } + const requiredScopes = resolveNodeApprovalRequiredScopes(pending); + const missingScope = resolveMissingRequestedScope({ + role: OPERATOR_ROLE, + requestedScopes: requiredScopes, + allowedScopes: options.callerScopes ?? [], + }); + if (missingScope) { + return { status: "forbidden", missingScope }; } const now = Date.now();