diff --git a/src/cli/nodes-cli/register.status.ts b/src/cli/nodes-cli/register.status.ts index b45f5096b8f..9709ea65958 100644 --- a/src/cli/nodes-cli/register.status.ts +++ b/src/cli/nodes-cli/register.status.ts @@ -11,14 +11,23 @@ import { formatErrorMessage } from "../../infra/errors.js"; import { formatTimeAgo } from "../../infra/format-time/format-relative.ts"; import { defaultRuntime } from "../../runtime.js"; import { shortenHomeInString } from "../../utils.js"; +import { formatCliCommand } from "../command-format.js"; import { parseDurationMs } from "../parse-duration.js"; import { getNodesTheme, runNodesCommand } from "./cli-utils.js"; import { formatPermissions, parseNodeList, parsePairingList } from "./format.js"; import { renderPendingPairingRequestsTable } from "./pairing-render.js"; -import { callGatewayCli, nodesCallOpts, resolveNodeId } from "./rpc.js"; +import { + callGatewayCli, + callNodeDiagnosticsGatewayCli, + nodesCallOpts, + resolveNodeDiagnosticsId, +} from "./rpc.js"; import type { NodeListNode, NodesRpcOpts, PairedNode } from "./types.js"; type PairedNodeListRow = PairedNode & Partial; +type NodeApprovalState = NonNullable; + +const DEFAULT_NODES_RPC_TIMEOUT_MS = 10_000; function formatVersionLabel(raw: string) { const trimmed = raw.trim(); @@ -98,6 +107,61 @@ function formatNodeTerminalLabel(node: { nodeId: string; displayName?: string }) return sanitizeTerminalText(label); } +function formatNodeApprovalState(raw: unknown): NodeApprovalState | null { + return raw === "approved" || + raw === "pending-approval" || + raw === "pending-reapproval" || + raw === "unapproved" + ? raw + : null; +} + +function formatApprovalStateLabel(state: NodeApprovalState): string { + if (state === "pending-approval") { + return "approval pending"; + } + if (state === "pending-reapproval") { + return "reapproval pending"; + } + return state; +} + +function isPendingApprovalState( + state: NodeApprovalState | null, +): state is "pending-approval" | "pending-reapproval" { + return state === "pending-approval" || state === "pending-reapproval"; +} + +function quoteCliArg(value: string): string { + if (/^[A-Za-z0-9_/:=.,@%+-]+$/.test(value)) { + return value; + } + return `'${value.replaceAll("'", "'\\''")}'`; +} + +function formatPendingApprovalCommand(raw: unknown, opts: NodesRpcOpts): string | null { + const requestId = normalizeOptionalString(raw); + if (!requestId) { + return null; + } + const args = ["openclaw", "nodes", "approve", requestId]; + const timeout = normalizeOptionalString(opts.timeout); + if (timeout && timeout !== String(DEFAULT_NODES_RPC_TIMEOUT_MS)) { + args.push("--timeout", timeout); + } + return formatCliCommand(args.map(quoteCliArg).join(" ")); +} + +function formatConnectionFlagReminder(opts: NodesRpcOpts): string | null { + const flags = [ + normalizeOptionalString(opts.url) ? "--url" : null, + normalizeOptionalString(opts.token) ? "--token" : null, + ].filter((flag) => flag !== null); + return flags.length > 0 + ? `Reuse the same ${flags.join("/")} option${flags.length === 1 ? "" : "s"} when rerunning.` + : null; +} + function parseSinceMs(raw: unknown, label: string): number | undefined { if (raw === undefined || raw === null) { return undefined; @@ -194,7 +258,7 @@ export function registerNodesStatusCommands(nodes: Command) { await runNodesCommand("status", async () => { const connectedOnly = Boolean(opts.connected); const sinceMs = parseSinceMs(opts.lastConnected, "Invalid --last-connected"); - const result = await callGatewayCli("node.list", opts, {}); + const result = await callNodeDiagnosticsGatewayCli("node.list", opts, {}); const obj: Record = typeof result === "object" && result !== null ? result : {}; const { ok, warn, muted } = getNodesTheme(); @@ -268,6 +332,15 @@ export function registerNodesStatusCommands(nodes: Command) { : "?"; const paired = n.paired ? ok("paired") : warn("unpaired"); const connected = n.connected ? ok("connected") : muted("disconnected"); + const approvalState = formatNodeApprovalState(n.approvalState); + const approval = + approvalState === "approved" + ? ok("approved") + : isPendingApprovalState(approvalState) + ? warn(formatApprovalStateLabel(approvalState)) + : approvalState === "unapproved" + ? warn("unapproved") + : null; const since = typeof n.connectedAtMs === "number" ? ` (${formatTimeAgo(Math.max(0, now - n.connectedAtMs))})` @@ -278,7 +351,7 @@ export function registerNodesStatusCommands(nodes: Command) { ID: sanitizeTerminalText(n.nodeId), IP: sanitizeTerminalText(n.remoteIp ?? ""), Detail: detailParts.join(" · "), - Status: `${paired} · ${connected}${since}`, + Status: `${paired} · ${connected}${since}${approval ? ` · ${approval}` : ""}`, Caps: caps, }; }); @@ -297,6 +370,22 @@ export function registerNodesStatusCommands(nodes: Command) { rows, }).trimEnd(), ); + for (const node of filtered) { + const approvalState = formatNodeApprovalState(node.approvalState); + const approveCommand = formatPendingApprovalCommand(node.pendingRequestId, opts); + if (isPendingApprovalState(approvalState) && approveCommand) { + const action = approvalState === "pending-reapproval" ? "Reapproval" : "Approval"; + defaultRuntime.log( + warn( + `${action} pending for ${formatNodeTerminalLabel(node)}. Run ${sanitizeTerminalText(approveCommand)}`, + ), + ); + const connectionReminder = formatConnectionFlagReminder(opts); + if (connectionReminder) { + defaultRuntime.log(warn(connectionReminder)); + } + } + } }); }), ); @@ -308,8 +397,8 @@ export function registerNodesStatusCommands(nodes: Command) { .requiredOption("--node ", "Node id, name, or IP") .action(async (opts: NodesRpcOpts) => { await runNodesCommand("describe", async () => { - const nodeId = await resolveNodeId(opts, opts.node ?? ""); - const result = await callGatewayCli("node.describe", opts, { + const nodeId = await resolveNodeDiagnosticsId(opts, opts.node ?? ""); + const result = await callNodeDiagnosticsGatewayCli("node.describe", opts, { nodeId, }); if (opts.json) { @@ -329,6 +418,19 @@ export function registerNodesStatusCommands(nodes: Command) { ? obj.commands.map(String).filter(Boolean).toSorted() : []; const perms = formatPermissions(obj.permissions); + const approvalState = formatNodeApprovalState(obj.approvalState); + const pendingRequestId = normalizeOptionalString(obj.pendingRequestId); + const pendingCaps = Array.isArray(obj.pendingDeclaredCaps) + ? obj.pendingDeclaredCaps.map(String).filter(Boolean).toSorted() + : null; + const pendingCommands = Array.isArray(obj.pendingDeclaredCommands) + ? obj.pendingDeclaredCommands.map(String).filter(Boolean).toSorted() + : []; + const pendingPerms = formatPermissions(obj.pendingDeclaredPermissions); + const approveCommand = isPendingApprovalState(approvalState) + ? formatPendingApprovalCommand(pendingRequestId, opts) + : null; + const connectionReminder = approveCommand ? formatConnectionFlagReminder(opts) : null; const family = typeof obj.deviceFamily === "string" ? obj.deviceFamily : null; const model = typeof obj.modelIdentifier === "string" ? obj.modelIdentifier : null; const client = formatClientLabel(obj as { clientId?: string; clientMode?: string }); @@ -359,6 +461,27 @@ export function registerNodesStatusCommands(nodes: Command) { versions ? { Field: "Version", Value: sanitizeTerminalText(versions) } : null, pathEnv ? { Field: "PATH", Value: sanitizeTerminalText(pathEnv) } : null, { Field: "Status", Value: status }, + approvalState + ? { Field: "Approval", Value: formatApprovalStateLabel(approvalState) } + : null, + pendingRequestId + ? { Field: "Pending request", Value: sanitizeTerminalText(pendingRequestId) } + : null, + pendingCaps + ? { Field: "Pending caps", Value: sanitizeTerminalText(pendingCaps.join(", ")) } + : null, + pendingPerms + ? { Field: "Pending perms", Value: sanitizeTerminalText(pendingPerms) } + : null, + approveCommand + ? { + Field: approvalState === "pending-reapproval" ? "Reapprove" : "Approve", + Value: sanitizeTerminalText(approveCommand), + } + : null, + approveCommand && connectionReminder + ? { Field: "Connection reminder", Value: connectionReminder } + : null, { Field: "Caps", Value: caps ? sanitizeTerminalText(caps.join(", ")) : "?" }, ].filter(Boolean) as Array<{ Field: string; Value: string }>; @@ -376,11 +499,18 @@ export function registerNodesStatusCommands(nodes: Command) { defaultRuntime.log(""); defaultRuntime.log(heading("Commands")); if (commands.length === 0) { - defaultRuntime.log(muted("- (none reported)")); - return; + defaultRuntime.log(muted("- (none effective)")); + } else { + for (const c of commands) { + defaultRuntime.log(`- ${c}`); + } } - for (const c of commands) { - defaultRuntime.log(`- ${c}`); + if (pendingCommands.length > 0) { + defaultRuntime.log(""); + defaultRuntime.log(heading("Pending commands")); + for (const command of pendingCommands) { + defaultRuntime.log(`- ${sanitizeTerminalText(command)}`); + } } }); }), diff --git a/src/cli/nodes-cli/rpc.runtime.ts b/src/cli/nodes-cli/rpc.runtime.ts index 4ab56a82feb..bc61bd300de 100644 --- a/src/cli/nodes-cli/rpc.runtime.ts +++ b/src/cli/nodes-cli/rpc.runtime.ts @@ -20,7 +20,13 @@ export async function callGatewayCliRuntime( method: string, opts: NodesRpcOpts, params?: unknown, - callOpts?: { transportTimeoutMs?: number }, + callOpts?: { + scopes?: OperatorScope[]; + transportTimeoutMs?: number; + useStoredDeviceAuth?: boolean; + requiredStoredDeviceAuthScopes?: OperatorScope[]; + useLocalBackendSharedAuth?: boolean; + }, ) { // Progress is suppressed for JSON callers so stdout remains structured. return await withProgress( @@ -35,9 +41,17 @@ export async function callGatewayCliRuntime( token: opts.token, method, params, + scopes: callOpts?.scopes, + useStoredDeviceAuth: callOpts?.useStoredDeviceAuth, + requiredStoredDeviceAuthScopes: callOpts?.requiredStoredDeviceAuthScopes, + requireLocalBackendSharedAuth: callOpts?.useLocalBackendSharedAuth, timeoutMs: resolveNodesTransportTimeoutMs(opts, callOpts?.transportTimeoutMs), - clientName: GATEWAY_CLIENT_NAMES.CLI, - mode: GATEWAY_CLIENT_MODES.CLI, + clientName: callOpts?.useLocalBackendSharedAuth + ? GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT + : GATEWAY_CLIENT_NAMES.CLI, + mode: callOpts?.useLocalBackendSharedAuth + ? GATEWAY_CLIENT_MODES.BACKEND + : GATEWAY_CLIENT_MODES.CLI, }), ); } diff --git a/src/cli/nodes-cli/rpc.ts b/src/cli/nodes-cli/rpc.ts index a17e3338218..482c941750d 100644 --- a/src/cli/nodes-cli/rpc.ts +++ b/src/cli/nodes-cli/rpc.ts @@ -23,6 +23,57 @@ async function loadNodesCliRpcRuntime(): Promise { return nodesCliRpcRuntimeLoader.load(); } +const STORED_DEVICE_AUTH_FALLBACK_DETAIL_CODES = new Set([ + "AUTH_REQUIRED", + "AUTH_UNAUTHORIZED", + "AUTH_TOKEN_MISMATCH", + "AUTH_DEVICE_TOKEN_MISMATCH", + "AUTH_SCOPE_MISMATCH", + "PAIRING_REQUIRED", +]); + +function readGatewayClientRequestDetailCode(value: unknown): string | null { + if (!(value instanceof Error) || value.name !== "GatewayClientRequestError") { + return null; + } + const details = (value as Error & { details?: unknown }).details; + if (!details || typeof details !== "object") { + return null; + } + const code = (details as { code?: unknown }).code; + return typeof code === "string" ? code : null; +} + +function isDiagnosticsAuthFallbackError(value: unknown): value is Error { + if ( + value instanceof Error && + (value.name === "GatewayCredentialsRequiredError" || + value.name === "GatewayStoredDeviceAuthUnavailableError" || + value.name === "GatewayLocalBackendSharedAuthUnavailableError") + ) { + return true; + } + const detailCode = readGatewayClientRequestDetailCode(value); + if (detailCode !== null && STORED_DEVICE_AUTH_FALLBACK_DETAIL_CODES.has(detailCode)) { + return true; + } + return ( + value instanceof Error && + value.name === "GatewayClientRequestError" && + (value as Error & { gatewayCode?: unknown }).gatewayCode === "INVALID_REQUEST" && + value.message.includes("missing scope: operator.read") + ); +} + +function isUnknownGatewayMethodError(value: unknown, method: string): value is Error { + return ( + value instanceof Error && + value.name === "GatewayClientRequestError" && + (value as Error & { gatewayCode?: unknown }).gatewayCode === "INVALID_REQUEST" && + value.message.includes(`unknown method: ${method}`) + ); +} + /** Attach shared Gateway connection/json options to a node command. */ export const nodesCallOpts = (cmd: Command, defaults?: { timeoutMs?: number }) => cmd @@ -36,12 +87,47 @@ export const callGatewayCli = async ( method: string, opts: NodesRpcOpts, params?: unknown, - callOpts?: { transportTimeoutMs?: number }, + callOpts?: { + scopes?: OperatorScope[]; + transportTimeoutMs?: number; + useStoredDeviceAuth?: boolean; + requiredStoredDeviceAuthScopes?: OperatorScope[]; + useLocalBackendSharedAuth?: boolean; + }, ) => { const runtime = await loadNodesCliRpcRuntime(); return await runtime.callGatewayCliRuntime(method, opts, params, callOpts); }; +/** Read node diagnostics with pairing details when authorized, otherwise keep read-only access. */ +export const callNodeDiagnosticsGatewayCli = async ( + method: "node.list" | "node.describe", + opts: NodesRpcOpts, + params?: unknown, +) => { + try { + return await callGatewayCli(method, opts, params, { + useStoredDeviceAuth: true, + requiredStoredDeviceAuthScopes: ["operator.read", "operator.pairing"], + }); + } catch (error) { + if (!isDiagnosticsAuthFallbackError(error)) { + throw error; + } + } + try { + return await callGatewayCli(method, opts, params, { + scopes: ["operator.read", "operator.pairing"], + useLocalBackendSharedAuth: true, + }); + } catch (error) { + if (!isDiagnosticsAuthFallbackError(error)) { + throw error; + } + } + return await callGatewayCli(method, opts, params); +}; + /** Call pairing approval methods with explicit operator scopes. */ export const callNodePairApprovalGatewayCli = async ( method: "node.pair.list" | "node.pair.approve", @@ -155,6 +241,19 @@ export async function resolveNodeId(opts: NodesRpcOpts, query: string) { return (await resolveNode(opts, query)).nodeId; } +/** Resolve a node through the pairing-aware diagnostics view when available. */ +export async function resolveNodeDiagnosticsId(opts: NodesRpcOpts, query: string) { + try { + const res = await callNodeDiagnosticsGatewayCli("node.list", opts, {}); + return resolveNodeFromNodeList(parseNodeList(res), query).nodeId; + } catch (error) { + if (!isUnknownGatewayMethodError(error, "node.list")) { + throw error; + } + return await resolveNodeId(opts, query); + } +} + /** Resolve a node query to the best available node record. */ export async function resolveNode(opts: NodesRpcOpts, query: string): Promise { let nodes: NodeListNode[]; diff --git a/src/cli/program.nodes-basic.e2e.test.ts b/src/cli/program.nodes-basic.e2e.test.ts index 767d86c7723..7b6143b9f4c 100644 --- a/src/cli/program.nodes-basic.e2e.test.ts +++ b/src/cli/program.nodes-basic.e2e.test.ts @@ -14,6 +14,9 @@ type GatewayCallRequest = { mode?: string; params?: unknown; scopes?: unknown; + useStoredDeviceAuth?: boolean; + requiredStoredDeviceAuthScopes?: unknown; + requireLocalBackendSharedAuth?: boolean; }; function formatRuntimeLogCallArg(value: unknown): string { @@ -410,6 +413,48 @@ describe("cli program (nodes basics)", () => { "canvas", ], }, + { + label: "pending first node approval", + node: { + nodeId: "pending-node", + displayName: "Pending Node", + caps: [], + commands: [], + approvalState: "pending-approval", + pendingRequestId: "request-approval", + pendingDeclaredCaps: ["system"], + pendingDeclaredCommands: ["system.run"], + paired: true, + connected: true, + }, + expectedOutput: [ + "Pending Node", + "approval pending", + "Approval pending for Pending Node", + "openclaw nodes approve request-approval", + ], + }, + { + label: "pending node reapproval", + node: { + nodeId: "pending-reapproval-node", + displayName: "Pending Reapproval Node", + caps: ["camera"], + commands: ["camera.snap"], + approvalState: "pending-reapproval", + pendingRequestId: "request-reapproval", + pendingDeclaredCaps: ["camera", "system"], + pendingDeclaredCommands: ["camera.snap", "system.run"], + paired: true, + connected: true, + }, + expectedOutput: [ + "Pending Reapproval Node", + "reapproval pending", + "Reapproval pending for Pending Reapproval Node", + "openclaw nodes approve request-reapproval", + ], + }, ])("runs nodes status and renders $label", async ({ node, expectedOutput }) => { callGateway.mockResolvedValue({ ts: Date.now(), @@ -423,6 +468,30 @@ describe("cli program (nodes basics)", () => { for (const expected of expectedOutput) { expect(output).toContain(expected); } + expect( + gatewayRequests().find((request) => request.method === "node.list")?.useStoredDeviceAuth, + ).toBe(true); + }); + + it("keeps connection age adjacent to connection status before pending approval", async () => { + callGateway.mockResolvedValue({ + ts: Date.now(), + nodes: [ + { + nodeId: "pending-reapproval-node", + displayName: "Pending Reapproval Node", + approvalState: "pending-reapproval", + pendingRequestId: "request-reapproval", + paired: true, + connected: true, + connectedAtMs: Date.now() - 60_000, + }, + ], + }); + + await runProgram(["nodes", "status"]); + + expect(getRuntimeOutput()).toMatch(/connected \([^)]* ago\) · reapproval pending/); }); it("runs nodes describe and calls node.describe", async () => { @@ -430,8 +499,13 @@ describe("cli program (nodes basics)", () => { ts: Date.now(), nodeId: "ios-node", displayName: "iOS Node", - caps: ["canvas", "camera"], - commands: ["canvas.eval", "canvas.snapshot", "camera.snap"], + caps: ["camera"], + commands: ["camera.snap"], + approvalState: "pending-reapproval", + pendingRequestId: "request-approval", + pendingDeclaredCaps: ["camera", "canvas"], + pendingDeclaredCommands: ["camera.snap", "canvas.eval\u001b[2K", "canvas.snapshot"], + pendingDeclaredPermissions: { camera: true }, connected: true, }); @@ -444,10 +518,345 @@ describe("cli program (nodes basics)", () => { ); expect(describeRequest?.clientName).toBe("cli"); expect(describeRequest?.mode).toBe("cli"); + expect(describeRequest?.useStoredDeviceAuth).toBe(true); const out = getRuntimeOutput(); expect(out).toContain("Commands"); + expect(out).toContain("camera.snap"); + expect(out).toContain("Approval"); + expect(out).toContain("reapproval pending"); + expect(out).toContain("Pending request"); + expect(out).toContain("request-approval"); + expect(out).toContain("Pending caps"); + expect(out).toContain("canvas"); + expect(out).toContain("Pending commands"); expect(out).toContain("canvas.eval"); + expect(out).toContain("openclaw nodes approve request-approval"); + expect(out).not.toContain("\u001b"); + expect(out).not.toContain("[2K"); + }); + + it("keeps explicit gateway options in node reapproval guidance without leaking auth", async () => { + callGateway.mockResolvedValue({ + ts: Date.now(), + nodes: [ + { + nodeId: "pending-node", + displayName: "Pending Node", + approvalState: "pending-reapproval", + pendingRequestId: "request-reapproval", + paired: true, + connected: true, + }, + ], + }); + + await runProgram([ + "nodes", + "status", + "--url", + "ws://gateway-user:url-secret@gateway.example:18789/openclaw?cluster=qa", + "--timeout", + "3000", + "--token", + "secret-token", + ]); + + const output = getRuntimeOutput(); + expect(output).toContain("openclaw nodes approve request-reapproval --timeout 3000"); + expect(output).toContain("Reuse the same --url/--token options when rerunning."); + expect(output).not.toContain("gateway-user"); + expect(output).not.toContain("url-secret"); + expect(output).not.toContain("gateway.example"); + expect(output).not.toContain("secret-token"); + }); + + it("falls back to read-only node status when pairing diagnostics are unavailable", async () => { + callGateway.mockImplementation(async (...args: unknown[]) => { + const opts = (args[0] ?? {}) as { + method?: string; + scopes?: string[]; + useStoredDeviceAuth?: boolean; + }; + if (opts.method === "node.list" && opts.useStoredDeviceAuth) { + throw Object.assign(new Error("stored device auth unavailable"), { + name: "GatewayCredentialsRequiredError", + }); + } + if (opts.method === "node.list" && opts.scopes?.includes("operator.pairing")) { + throw Object.assign(new Error("unauthorized: pairing scope unavailable"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + details: { code: "AUTH_SCOPE_MISMATCH" }, + }); + } + if (opts.method === "node.list") { + return { + ts: Date.now(), + nodes: [ + { + nodeId: "read-only-node", + displayName: "Read Only Node", + approvalState: "approved", + paired: true, + connected: false, + }, + ], + }; + } + return { ok: true }; + }); + + await runProgram(["nodes", "status"]); + + const requests = gatewayRequests().filter((request) => request.method === "node.list"); + expect(requests).toHaveLength(3); + expect(requests[0]?.useStoredDeviceAuth).toBe(true); + expect(requests[0]?.requiredStoredDeviceAuthScopes).toEqual([ + "operator.read", + "operator.pairing", + ]); + expect(requests[1]?.scopes).toEqual(["operator.read", "operator.pairing"]); + expect(requests[1]?.clientName).toBe("gateway-client"); + expect(requests[1]?.mode).toBe("backend"); + expect(requests[1]?.requireLocalBackendSharedAuth).toBe(true); + expect(requests[2]?.useStoredDeviceAuth).toBeUndefined(); + expect(requests[2]?.scopes).toBeUndefined(); + expect(getRuntimeOutput()).toContain("Read Only Node"); + }); + + it("keeps remote explicit diagnostic credentials on the read-only path", async () => { + callGateway.mockImplementation(async (...args: unknown[]) => { + const opts = (args[0] ?? {}) as { + method?: string; + requireLocalBackendSharedAuth?: boolean; + useStoredDeviceAuth?: boolean; + }; + if (opts.method === "node.list" && opts.useStoredDeviceAuth) { + throw Object.assign(new Error("stored device auth disabled for explicit credentials"), { + name: "GatewayStoredDeviceAuthUnavailableError", + }); + } + if (opts.method === "node.list" && opts.requireLocalBackendSharedAuth) { + throw Object.assign(new Error("local backend shared auth unavailable for remote target"), { + name: "GatewayLocalBackendSharedAuthUnavailableError", + }); + } + return { + nodes: [ + { + nodeId: "remote-read-only-node", + displayName: "Remote Read Only Node", + paired: true, + connected: false, + }, + ], + }; + }); + + await runProgram([ + "nodes", + "status", + "--url", + "wss://gateway.example.test", + "--token", + "explicit-token", + ]); + + const requests = gatewayRequests().filter((request) => request.method === "node.list"); + expect(requests).toHaveLength(3); + expect(requests[0]?.useStoredDeviceAuth).toBe(true); + expect(requests[0]?.requiredStoredDeviceAuthScopes).toEqual([ + "operator.read", + "operator.pairing", + ]); + expect(requests[1]?.scopes).toEqual(["operator.read", "operator.pairing"]); + expect(requests[1]?.clientName).toBe("gateway-client"); + expect(requests[1]?.mode).toBe("backend"); + expect(requests[1]?.requireLocalBackendSharedAuth).toBe(true); + expect(requests[2]?.scopes).toBeUndefined(); + expect(getRuntimeOutput()).toContain("Remote Read Only Node"); + }); + + it("does not retry node diagnostics after a transport failure", async () => { + callGateway.mockRejectedValue(new Error("gateway timed out")); + + await expect(runProgram(["nodes", "status"])).rejects.toThrow("exit"); + + const requests = gatewayRequests().filter((request) => request.method === "node.list"); + expect(requests).toHaveLength(1); + expect(requests[0]?.useStoredDeviceAuth).toBe(true); + }); + + it("falls back to configured auth after stored device auth is rejected", async () => { + callGateway.mockImplementation(async (...args: unknown[]) => { + const opts = (args[0] ?? {}) as { method?: string; useStoredDeviceAuth?: boolean }; + if (opts.method === "node.list" && opts.useStoredDeviceAuth) { + throw Object.assign(new Error("unauthorized: device token mismatch"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + details: { code: "AUTH_DEVICE_TOKEN_MISMATCH" }, + }); + } + if (opts.method === "node.list") { + return { + nodes: [ + { + nodeId: "configured-auth-node", + displayName: "Configured Auth Node", + paired: true, + connected: false, + }, + ], + }; + } + return { ok: true }; + }); + + await runProgram(["nodes", "status"]); + + const requests = gatewayRequests().filter((request) => request.method === "node.list"); + expect(requests).toHaveLength(2); + expect(requests[0]?.useStoredDeviceAuth).toBe(true); + expect(requests[1]?.useStoredDeviceAuth).toBeUndefined(); + expect(getRuntimeOutput()).toContain("Configured Auth Node"); + }); + + it("falls back to configured auth when stored device auth lacks read scope", async () => { + callGateway.mockImplementation(async (...args: unknown[]) => { + const opts = (args[0] ?? {}) as { + method?: string; + scopes?: string[]; + useStoredDeviceAuth?: boolean; + }; + if (opts.method === "node.list" && opts.useStoredDeviceAuth) { + throw Object.assign(new Error("missing scope: operator.read"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + }); + } + if (opts.method === "node.list" && opts.scopes?.includes("operator.pairing")) { + return { + nodes: [ + { + nodeId: "shared-auth-node", + displayName: "Shared Auth Node", + paired: true, + connected: false, + }, + ], + }; + } + return { nodes: [] }; + }); + + await runProgram(["nodes", "status"]); + + const requests = gatewayRequests().filter((request) => request.method === "node.list"); + expect(requests).toHaveLength(2); + expect(requests[1]?.scopes).toEqual(["operator.read", "operator.pairing"]); + expect(requests[1]?.clientName).toBe("gateway-client"); + expect(requests[1]?.mode).toBe("backend"); + expect(requests[1]?.requireLocalBackendSharedAuth).toBe(true); + expect(getRuntimeOutput()).toContain("Shared Auth Node"); + }); + + it("describes pending-only nodes through the pairing diagnostics view", async () => { + callGateway.mockImplementation(async (...args: unknown[]) => { + const opts = (args[0] ?? {}) as { + method?: string; + params?: { nodeId?: string }; + useStoredDeviceAuth?: boolean; + }; + if (opts.method === "node.list") { + return opts.useStoredDeviceAuth + ? { + nodes: [ + { + nodeId: "pending-only-node", + displayName: "Pending Only Node", + approvalState: "pending-approval", + pendingRequestId: "pending-only-request", + paired: false, + connected: false, + }, + ], + } + : { nodes: [] }; + } + if (opts.method === "node.describe" && opts.params?.nodeId === "pending-only-node") { + return { + nodeId: "pending-only-node", + displayName: "Pending Only Node", + approvalState: "pending-approval", + pendingRequestId: "pending-only-request", + paired: false, + connected: false, + }; + } + return { ok: true }; + }); + + await runProgram(["nodes", "describe", "--node", "pending-only-node"]); + + const describeRequest = gatewayRequests().find((request) => request.method === "node.describe"); + expect(describeRequest?.params).toEqual({ nodeId: "pending-only-node" }); + expect(describeRequest?.useStoredDeviceAuth).toBe(true); + expect(getRuntimeOutput()).toContain("pending-only-request"); + }); + + it("describes nodes through the paired-node fallback on older gateways", async () => { + callGateway.mockImplementation(async (...args: unknown[]) => { + const opts = (args[0] ?? {}) as { + method?: string; + params?: { nodeId?: string }; + }; + if (opts.method === "node.list") { + throw Object.assign(new Error("unknown method: node.list"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + }); + } + if (opts.method === "node.pair.list") { + return { + pending: [], + paired: [{ nodeId: "legacy-node", displayName: "Legacy Node" }], + }; + } + if (opts.method === "node.describe" && opts.params?.nodeId === "legacy-node") { + return { + nodeId: "legacy-node", + displayName: "Legacy Node", + paired: true, + connected: false, + }; + } + return { ok: true }; + }); + + await runProgram(["nodes", "describe", "--node", "legacy-node"]); + + expectGatewayRequest("node.pair.list", {}); + expectGatewayRequest("node.describe", { nodeId: "legacy-node" }); + expect(getRuntimeOutput()).toContain("Legacy Node"); + }); + + it("does not recommend approval from a stale pending request id alone", async () => { + mockGatewayWithIosNodeListAnd("node.describe", { + nodeId: "ios-node", + displayName: "iOS Node", + approvalState: "approved", + pendingRequestId: "stale-request", + connected: true, + }); + + await runProgram(["nodes", "describe", "--node", "ios-node", "--token", "secret-token"]); + + const output = getRuntimeOutput(); + expect(output).toContain("stale-request"); + expect(output).not.toContain("openclaw nodes approve stale-request"); + expect(output).not.toContain("Reuse the same --token option when rerunning."); + expect(output).not.toContain("secret-token"); }); it("runs nodes approve with the pending request approval scopes", async () => { diff --git a/src/gateway/auth-rate-limit.test.ts b/src/gateway/auth-rate-limit.test.ts index 0774588ab99..4d4da3034d0 100644 --- a/src/gateway/auth-rate-limit.test.ts +++ b/src/gateway/auth-rate-limit.test.ts @@ -6,6 +6,7 @@ import { AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN, AUTH_RATE_LIMIT_SCOPE_HOOK_AUTH, AUTH_RATE_LIMIT_SCOPE_SHARED_SECRET, + buildRateLimitIdentityKey, createAuthRateLimiter, type AuthRateLimiter, } from "./auth-rate-limit.js"; @@ -200,6 +201,13 @@ describe("auth rate limiter", () => { expect(limiter.check("127.0.0.1").allowed).toBe(false); }); + it("does not exempt opaque identity keys", () => { + limiter = createAuthRateLimiter({ maxAttempts: 1, windowMs: 60_000, lockoutMs: 60_000 }); + const key = buildRateLimitIdentityKey("node", "node-1"); + limiter.recordFailure(key); + expect(limiter.check(key).allowed).toBe(false); + }); + // ---------- reset ---------- it("clears tracking state when reset is called", () => { diff --git a/src/gateway/auth-rate-limit.ts b/src/gateway/auth-rate-limit.ts index 3e7a8d0d91e..89672f5c7bf 100644 --- a/src/gateway/auth-rate-limit.ts +++ b/src/gateway/auth-rate-limit.ts @@ -43,6 +43,9 @@ export const AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN = "device-token"; // The request path enters the node-pairing storage lock, so bursts must be // throttled before they queue behind that lock and delay operator actions. export const AUTH_RATE_LIMIT_SCOPE_NODE_PAIRING = "node-pairing"; +// Paired-node approval-surface changes use a dedicated limiter so reconnect +// storms cannot queue unbounded writes behind the shared pairing-state lock. +export const AUTH_RATE_LIMIT_SCOPE_NODE_REAPPROVAL = "node-reapproval"; // Per-IP gate for the pre-auth bootstrap-token verify path. // `verifyDeviceBootstrapToken` is `withLock`-serialized in // `device-bootstrap.ts` and runs fs read + fs write on every attempt; @@ -52,6 +55,7 @@ export const AUTH_RATE_LIMIT_SCOPE_NODE_PAIRING = "node-pairing"; export const AUTH_RATE_LIMIT_SCOPE_BOOTSTRAP_TOKEN = "bootstrap-token"; export const AUTH_RATE_LIMIT_SCOPE_HOOK_AUTH = "hook-auth"; const BROWSER_ORIGIN_RATE_LIMIT_KEY_PREFIX = "browser-origin:"; +const IDENTITY_RATE_LIMIT_KEY_PREFIX = "identity:"; interface RateLimitEntry { /** Timestamps (epoch ms) of recent failed attempts inside the window. */ @@ -102,12 +106,21 @@ const PRUNE_INTERVAL_MS = 60_000; // prune stale entries every minute * share one representation (including IPv4-mapped IPv6 forms). */ export function normalizeRateLimitClientIp(ip: string | undefined): string { - if (typeof ip === "string" && ip.startsWith(BROWSER_ORIGIN_RATE_LIMIT_KEY_PREFIX)) { + if ( + typeof ip === "string" && + (ip.startsWith(BROWSER_ORIGIN_RATE_LIMIT_KEY_PREFIX) || + ip.startsWith(IDENTITY_RATE_LIMIT_KEY_PREFIX)) + ) { return ip; } return resolveClientIp({ remoteAddr: ip }) ?? "unknown"; } +/** Build an opaque limiter identity that is not subject to loopback IP exemptions. */ +export function buildRateLimitIdentityKey(namespace: string, identity: string): string { + return `${IDENTITY_RATE_LIMIT_KEY_PREFIX}${namespace}:${identity}`; +} + function resolvePruneIntervalMs(value: number | undefined): number { if (value === undefined) { return PRUNE_INTERVAL_MS; diff --git a/src/gateway/call.test.ts b/src/gateway/call.test.ts index e0627f4b056..d650009a37b 100644 --- a/src/gateway/call.test.ts +++ b/src/gateway/call.test.ts @@ -94,6 +94,7 @@ let startCalls = 0; let closeCode = 1006; let closeReason = ""; let helloMethods: string[] | undefined = ["health", "secrets.resolve"]; +let connectError: Error | null = null; vi.mock("./client.js", () => ({ describeGatewayCloseCode: (code: number) => { @@ -147,7 +148,7 @@ vi.mock("./client.js", () => ({ }); } else if (startMode === "connect-error") { lastClientOptions?.onConnectError?.( - connectAssemblyErrorState.create("device private key invalid"), + connectError ?? connectAssemblyErrorState.create("device private key invalid"), ); } else if (startMode === "close") { lastClientOptions?.onClose?.(closeCode, closeReason); @@ -223,7 +224,7 @@ class StubGatewayClient { }); } else if (startMode === "connect-error") { lastClientOptions?.onConnectError?.( - connectAssemblyErrorState.create("device private key invalid"), + connectError ?? connectAssemblyErrorState.create("device private key invalid"), ); } else if (startMode === "close") { lastClientOptions?.onClose?.(closeCode, closeReason); @@ -254,6 +255,7 @@ function resetGatewayCallMocks() { closeCode = 1006; closeReason = ""; helloMethods = ["health", "secrets.resolve"]; + connectError = null; const loadConfigForTests = getRuntimeConfig as unknown as () => OpenClawConfig; const resolveGatewayPortForTests = resolveGatewayPort as unknown as ( cfg?: OpenClawConfig, @@ -746,6 +748,210 @@ describe("callGateway url resolution", () => { expect(lastClientOptions?.scopes).toStrictEqual([]); }); + it("reuses stored device auth without requesting stronger scopes", async () => { + setLocalLoopbackGatewayConfig(); + loadDeviceAuthTokenMock.mockReturnValue({ + token: "paired-device-token", + role: "operator", + scopes: ["operator.read", "operator.pairing"], + updatedAtMs: 123, + }); + + await callGatewayCli({ method: "node.list", useStoredDeviceAuth: true }); + + expect(lastClientOptions?.token).toBeUndefined(); + expect(lastClientOptions?.password).toBeUndefined(); + expect(lastClientOptions?.scopes).toBeUndefined(); + expect(lastClientOptions?.deviceIdentity).toEqual(deviceIdentityState.value); + }); + + it("does not replace explicit credentials with stored device auth", async () => { + setLocalLoopbackGatewayConfig(); + + await expect( + callGatewayCli({ + method: "node.list", + token: "explicit-token", + useStoredDeviceAuth: true, + }), + ).rejects.toMatchObject({ name: "GatewayStoredDeviceAuthUnavailableError" }); + + expect(lastClientOptions).toBeNull(); + }); + + it("prefers stored device auth over configured local credentials", async () => { + getRuntimeConfig.mockReturnValue({ + gateway: { + mode: "local", + bind: "loopback", + auth: { mode: "token", token: "configured-token" }, + }, + }); + setGatewayNetworkDefaults(); + + await callGatewayCli({ method: "node.list", useStoredDeviceAuth: true }); + + expect(lastClientOptions?.token).toBeUndefined(); + expect(lastClientOptions?.scopes).toBeUndefined(); + }); + + it("does not resolve configured local SecretRefs when using stored device auth", async () => { + getRuntimeConfig.mockReturnValue({ + gateway: { + mode: "local", + bind: "loopback", + auth: { + mode: "password", + password: { source: "env", provider: "default", id: "MISSING_LOCAL_PASSWORD" }, + }, + }, + secrets: { + providers: { + default: { source: "env" }, + }, + }, + } as unknown as OpenClawConfig); + setGatewayNetworkDefaults(); + + await callGatewayCli({ method: "node.list", useStoredDeviceAuth: true }); + + expect(lastClientOptions?.password).toBeUndefined(); + expect(lastClientOptions?.deviceIdentity).toEqual(deviceIdentityState.value); + }); + + it("rejects stored device auth that lacks caller-required scopes", async () => { + setLocalLoopbackGatewayConfig(); + loadDeviceAuthTokenMock.mockReturnValue({ + token: "paired-device-token", + role: "operator", + scopes: ["operator.read"], + updatedAtMs: 123, + }); + + await expect( + callGatewayCli({ + method: "node.list", + useStoredDeviceAuth: true, + requiredStoredDeviceAuthScopes: ["operator.read", "operator.pairing"], + }), + ).rejects.toMatchObject({ name: "GatewayStoredDeviceAuthUnavailableError" }); + + expect(lastClientOptions).toBeNull(); + }); + + it("does not send stored device auth to configured remote gateways", async () => { + getRuntimeConfig.mockReturnValue(makeRemotePasswordGatewayConfig("remote-password")); + setGatewayNetworkDefaults(); + + await expect( + callGatewayCli({ method: "node.list", useStoredDeviceAuth: true }), + ).rejects.toMatchObject({ name: "GatewayStoredDeviceAuthUnavailableError" }); + + expect(lastClientOptions).toBeNull(); + }); + + it("fails before connecting when stored device auth is unavailable", async () => { + getRuntimeConfig.mockReturnValue({ + gateway: { mode: "local", bind: "loopback", auth: { mode: "none" } }, + }); + setGatewayNetworkDefaults(); + loadDeviceAuthTokenMock.mockReturnValue(null); + + await expect( + callGatewayCli({ method: "node.list", useStoredDeviceAuth: true }), + ).rejects.toThrow("requires credentials before opening a websocket"); + + expect(lastClientOptions).toBeNull(); + expect(startCalls).toBe(0); + }); + + it("uses local backend shared auth without a device identity when required", async () => { + setLocalLoopbackGatewayConfig(); + + await callGateway({ + method: "node.list", + token: "explicit-token", + scopes: ["operator.read", "operator.pairing"], + requireLocalBackendSharedAuth: true, + }); + + expect(lastClientOptions?.clientName).toBe(GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT); + expect(lastClientOptions?.mode).toBe(GATEWAY_CLIENT_MODES.BACKEND); + expect(lastClientOptions?.scopes).toEqual(["operator.read", "operator.pairing"]); + expect(lastClientOptions?.deviceIdentity).toBeNull(); + }); + + it("uses local backend auth-none without a device identity when required", async () => { + getRuntimeConfig.mockReturnValue({ + gateway: { mode: "local", bind: "loopback", auth: { mode: "none" } }, + }); + setGatewayNetworkDefaults(); + + await callGateway({ + method: "node.list", + scopes: ["operator.read", "operator.pairing"], + requireLocalBackendSharedAuth: true, + }); + + expect(lastClientOptions?.clientName).toBe(GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT); + expect(lastClientOptions?.mode).toBe(GATEWAY_CLIENT_MODES.BACKEND); + expect(lastClientOptions?.scopes).toEqual(["operator.read", "operator.pairing"]); + expect(lastClientOptions?.token).toBeUndefined(); + expect(lastClientOptions?.password).toBeUndefined(); + expect(lastClientOptions?.deviceIdentity).toBeNull(); + }); + + it("rejects required local backend shared auth for remote targets", async () => { + await expect( + callGateway({ + method: "node.list", + url: "wss://remote.example.test", + token: "explicit-token", + scopes: ["operator.read", "operator.pairing"], + requireLocalBackendSharedAuth: true, + }), + ).rejects.toMatchObject({ name: "GatewayLocalBackendSharedAuthUnavailableError" }); + + expect(lastClientOptions).toBeNull(); + }); + + it("rejects required local backend shared auth for loopback URL overrides", async () => { + await expect( + callGateway({ + method: "node.list", + url: "ws://127.0.0.1:18789", + token: "explicit-token", + scopes: ["operator.read", "operator.pairing"], + requireLocalBackendSharedAuth: true, + }), + ).rejects.toMatchObject({ name: "GatewayLocalBackendSharedAuthUnavailableError" }); + + expect(lastClientOptions).toBeNull(); + }); + + it("rejects required local backend shared auth for remote-mode loopback tunnels", async () => { + getRuntimeConfig.mockReturnValue({ + gateway: { + mode: "remote", + remote: { + url: "ws://127.0.0.1:18789", + token: "remote-token", + }, + }, + }); + setGatewayNetworkDefaults(); + + await expect( + callGateway({ + method: "node.list", + scopes: ["operator.read", "operator.pairing"], + requireLocalBackendSharedAuth: true, + }), + ).rejects.toMatchObject({ name: "GatewayLocalBackendSharedAuthUnavailableError" }); + + expect(lastClientOptions).toBeNull(); + }); + it("uses backend client metadata for explicit scoped default calls", async () => { setLocalLoopbackGatewayConfig(); @@ -1169,6 +1375,30 @@ describe("callGateway error details", () => { expect(lastRequestOptions).toBeNull(); }); + it("surfaces stored device auth handshake failures for credential fallback", async () => { + startMode = "connect-error"; + connectError = Object.assign(new Error("unauthorized: device token mismatch"), { + name: "GatewayClientRequestError", + gatewayCode: "INVALID_REQUEST", + details: { code: "AUTH_DEVICE_TOKEN_MISMATCH" }, + }); + setLocalLoopbackGatewayConfig(); + + vi.useFakeTimers(); + const promise = callGatewayCli({ + method: "node.list", + timeoutMs: 5, + useStoredDeviceAuth: true, + }); + const rejection = expect(promise).rejects.toMatchObject({ + name: "GatewayClientRequestError", + details: { code: "AUTH_DEVICE_TOKEN_MISMATCH" }, + }); + await vi.advanceTimersByTimeAsync(5); + + await rejection; + }); + it("includes connection details on timeout", async () => { startMode = "silent"; setLocalLoopbackGatewayConfig(); diff --git a/src/gateway/call.ts b/src/gateway/call.ts index 8ee1333306c..46114d5451f 100644 --- a/src/gateway/call.ts +++ b/src/gateway/call.ts @@ -24,6 +24,8 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import { loadDeviceAuthToken } from "../infra/device-auth-store.js"; import { loadOrCreateDeviceIdentity, type DeviceIdentity } from "../infra/device-identity.js"; import { loadGatewayTlsRuntime } from "../infra/tls/gateway.js"; +import type { DeviceAuthEntry } from "../shared/device-auth.js"; +import { roleScopesAllow } from "../shared/operator-scope-compat.js"; import { resolveSafeTimeoutDelayMs } from "../utils/timer-delay.js"; import { VERSION } from "../version.js"; import { resolveGatewayAuth } from "./auth-resolve.js"; @@ -82,6 +84,9 @@ type CallGatewayBaseOptions = { platform?: string; mode?: GatewayClientMode; approvalRuntimeToken?: string; + useStoredDeviceAuth?: boolean; + requiredStoredDeviceAuthScopes?: OperatorScope[]; + requireLocalBackendSharedAuth?: boolean; deviceIdentity?: DeviceIdentity | null; instanceId?: string; minProtocol?: number; @@ -164,6 +169,20 @@ export class GatewayExplicitAuthRequiredError extends Error { } } +export class GatewayStoredDeviceAuthUnavailableError extends Error { + constructor(message: string) { + super(message); + this.name = "GatewayStoredDeviceAuthUnavailableError"; + } +} + +export class GatewayLocalBackendSharedAuthUnavailableError extends Error { + constructor(message: string) { + super(message); + this.name = "GatewayLocalBackendSharedAuthUnavailableError"; + } +} + export type GatewayTransportErrorJson = { ok: false; error: { @@ -414,14 +433,16 @@ function shouldOmitDeviceIdentityForGatewayCall(params: { url: string; token?: string; password?: string; + allowAuthNone?: boolean; }): boolean { const mode = params.opts.mode ?? GATEWAY_CLIENT_MODES.CLI; const clientName = params.opts.clientName ?? GATEWAY_CLIENT_NAMES.CLI; - const hasSharedAuth = Boolean(params.token || params.password); + const hasDirectLocalBackendAuth = + Boolean(params.token || params.password) || params.allowAuthNone === true; return ( mode === GATEWAY_CLIENT_MODES.BACKEND && clientName === GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT && - hasSharedAuth && + hasDirectLocalBackendAuth && isLoopbackGatewayUrl(params.url) ); } @@ -444,23 +465,27 @@ function resolveDeviceIdentityForGatewayCall(params: { } } -function hasStoredOperatorDeviceAuthToken(deviceIdentity: DeviceIdentity | null): boolean { +function loadStoredOperatorDeviceAuthToken( + deviceIdentity: DeviceIdentity | null, +): DeviceAuthEntry | null { if (!deviceIdentity) { - return false; + return null; } try { - return Boolean( - gatewayCallDeps.loadDeviceAuthToken({ - deviceId: deviceIdentity.deviceId, - role: "operator", - env: process.env, - })?.token, - ); + return gatewayCallDeps.loadDeviceAuthToken({ + deviceId: deviceIdentity.deviceId, + role: "operator", + env: process.env, + }); } catch { - return false; + return null; } } +function hasStoredOperatorDeviceAuthToken(deviceIdentity: DeviceIdentity | null): boolean { + return Boolean(loadStoredOperatorDeviceAuthToken(deviceIdentity)?.token); +} + function resolveGatewayCallAuth(config: OpenClawConfig) { return resolveGatewayAuth({ authConfig: config.gateway?.auth, @@ -807,7 +832,7 @@ function ensureGatewaySupportsRequiredMethods(params: { async function executeGatewayRequestWithScopes(params: { opts: CallGatewayBaseOptions; - scopes: OperatorScope[]; + scopes: OperatorScope[] | undefined; url: string; token?: string; password?: string; @@ -817,6 +842,7 @@ async function executeGatewayRequestWithScopes(params: { safeTimerTimeoutMs: number; connectionDetails: GatewayConnectionDetails; deviceIdentity: DeviceIdentity | null; + surfaceGatewayClientRequestErrors: boolean; }): Promise { const { opts, @@ -829,6 +855,7 @@ async function executeGatewayRequestWithScopes(params: { timeoutMs, safeTimerTimeoutMs, deviceIdentity, + surfaceGatewayClientRequestErrors, } = params; return await new Promise((resolve, reject) => { if (opts.signal?.aborted) { @@ -910,7 +937,7 @@ async function executeGatewayRequestWithScopes(params: { mode: opts.mode ?? GATEWAY_CLIENT_MODES.CLI, ...(opts.approvalRuntimeToken ? { approvalRuntimeToken: opts.approvalRuntimeToken } : {}), role: "operator", - scopes, + ...(Array.isArray(scopes) ? { scopes } : {}), deviceIdentity, minProtocol: opts.minProtocol ?? MIN_CLIENT_PROTOCOL_VERSION, maxProtocol: opts.maxProtocol ?? PROTOCOL_VERSION, @@ -955,7 +982,11 @@ async function executeGatewayRequestWithScopes(params: { ); }, onConnectError: (err) => { - if (settled || !isGatewayConnectAssemblyError(err)) { + const isGatewayClientRequestError = err.name === "GatewayClientRequestError"; + const shouldSurface = + isGatewayConnectAssemblyError(err) || + (surfaceGatewayClientRequestErrors && isGatewayClientRequestError); + if (settled || !shouldSurface) { return; } ignoreClose = true; @@ -1001,14 +1032,31 @@ async function executeGatewayRequestWithScopes(params: { async function callGatewayWithScopes>( opts: CallGatewayBaseOptions, - scopes: OperatorScope[], + scopes: OperatorScope[] | undefined, ): Promise { const context = await resolveGatewayCallContext(opts); const { timeoutMs, safeTimerTimeoutMs } = resolveGatewayCallTimeout( opts.timeoutMs, context.config.gateway?.handshakeTimeoutMs, ); - const resolvedCredentials = await resolveGatewayCredentials(context); + if (opts.requireLocalBackendSharedAuth && (context.urlOverride || context.isRemoteMode)) { + throw new GatewayLocalBackendSharedAuthUnavailableError( + "local backend shared auth is limited to the configured local gateway", + ); + } + const useStoredDeviceAuth = opts.useStoredDeviceAuth === true; + if ( + useStoredDeviceAuth && + (context.urlOverride || + context.explicitAuth.token || + context.explicitAuth.password || + context.isRemoteMode) + ) { + throw new GatewayStoredDeviceAuthUnavailableError( + "stored device auth is limited to the configured local gateway", + ); + } + const resolvedCredentials = useStoredDeviceAuth ? {} : await resolveGatewayCredentials(context); ensureExplicitGatewayAuth({ urlOverride: context.urlOverride, urlOverrideSource: context.urlOverrideSource, @@ -1026,11 +1074,50 @@ async function callGatewayWithScopes>( }); const url = connectionDetails.url; const tlsFingerprint = await resolveGatewayTlsFingerprint({ opts, context, url }); - const { token, password } = resolvedCredentials; + const token = useStoredDeviceAuth ? undefined : resolvedCredentials.token; + const password = useStoredDeviceAuth ? undefined : resolvedCredentials.password; + const allowAuthNone = + opts.requireLocalBackendSharedAuth === true && + resolveGatewayCallAuth(context.config).mode === "none"; + const omitDeviceIdentity = shouldOmitDeviceIdentityForGatewayCall({ + opts, + url, + token, + password, + allowAuthNone, + }); + if (opts.requireLocalBackendSharedAuth && !omitDeviceIdentity) { + throw new GatewayLocalBackendSharedAuthUnavailableError( + "local backend shared auth requires a loopback gateway with token/password credentials or auth mode none", + ); + } const deviceIdentity = opts.deviceIdentity === undefined - ? resolveDeviceIdentityForGatewayCall({ opts, url, token, password }) + ? omitDeviceIdentity + ? null + : resolveDeviceIdentityForGatewayCall({ opts, url, token, password }) : opts.deviceIdentity; + if (useStoredDeviceAuth) { + const storedAuth = loadStoredOperatorDeviceAuthToken(deviceIdentity); + if (!storedAuth?.token) { + throw new GatewayCredentialsRequiredError({ + method: opts.method, + configPath: context.configPath, + }); + } + if ( + Array.isArray(opts.requiredStoredDeviceAuthScopes) && + !roleScopesAllow({ + role: "operator", + requestedScopes: opts.requiredStoredDeviceAuthScopes, + allowedScopes: storedAuth.scopes, + }) + ) { + throw new GatewayStoredDeviceAuthUnavailableError( + "stored device auth does not grant the required operator scopes", + ); + } + } ensureGatewayCallCanAuthenticate({ opts, context, @@ -1040,7 +1127,7 @@ async function callGatewayWithScopes>( }); return await executeGatewayRequestWithScopes({ opts, - scopes, + scopes: useStoredDeviceAuth ? undefined : scopes, url, token, password, @@ -1050,6 +1137,8 @@ async function callGatewayWithScopes>( safeTimerTimeoutMs, connectionDetails, deviceIdentity, + surfaceGatewayClientRequestErrors: + useStoredDeviceAuth || opts.requireLocalBackendSharedAuth === true, }); } diff --git a/src/gateway/node-catalog.test.ts b/src/gateway/node-catalog.test.ts index 4ce2b2a745f..f6e720225b5 100644 --- a/src/gateway/node-catalog.test.ts +++ b/src/gateway/node-catalog.test.ts @@ -12,6 +12,7 @@ import { type CatalogInput = Parameters[0]; type TestPairedDevice = CatalogInput["pairedDevices"][number]; type TestPairedNode = NonNullable[number]; +type TestPendingNode = NonNullable[number]; function pairedDevice(overrides: Partial = {}): TestPairedDevice { return { @@ -49,6 +50,19 @@ function pairedNode(overrides: Partial = {}): TestPairedNode { }; } +function pendingNode(overrides: Partial = {}): TestPendingNode { + return { + requestId: "request-1", + nodeId: "mac-1", + platform: "macos", + caps: ["camera", "screen"], + commands: ["screen.snapshot", "system.run"], + permissions: { camera: true, screen: true }, + ts: 200, + ...overrides, + }; +} + describe("gateway/node-catalog", () => { it("filters paired nodes by active node token instead of sticky historical roles", () => { const catalog = createKnownNodeCatalog({ @@ -233,6 +247,180 @@ describe("gateway/node-catalog", () => { expect(node?.connected).toBe(true); }); + it("reports pending first approval without making declarations effective", () => { + const catalog = createKnownNodeCatalog({ + pairedDevices: [pairedDevice({ deviceId: "new-node" })], + pairedNodes: [], + pendingNodes: [pendingNode({ nodeId: "new-node", displayName: "Pending Mac" })], + connectedNodes: [], + }); + + const node = getKnownNode(catalog, "new-node"); + expect(node?.displayName).toBe("Mac"); + expect(node?.approvalState).toBe("pending-approval"); + expect(node?.pendingRequestId).toBe("request-1"); + expect(node?.pendingDeclaredCaps).toEqual(["camera", "screen"]); + expect(node?.pendingDeclaredCommands).toEqual(["screen.snapshot", "system.run"]); + expect(node?.pendingDeclaredPermissions).toEqual({ camera: true, screen: true }); + expect(node?.caps).toEqual([]); + expect(node?.commands).toEqual([]); + expect(node?.permissions).toBeUndefined(); + }); + + it("uses pending request metadata as the final fallback for pending-only nodes", () => { + const catalog = createKnownNodeCatalog({ + pairedDevices: [], + pairedNodes: [], + pendingNodes: [ + pendingNode({ + nodeId: "new-node", + clientId: "openclaw-linux", + clientMode: "node", + displayName: "Pending Node", + platform: "linux", + version: "1.2.3", + coreVersion: "1.2.4", + uiVersion: "1.2.5", + deviceFamily: "desktop", + modelIdentifier: "x86_64", + remoteIp: "100.0.0.20", + }), + ], + connectedNodes: [], + }); + + expect(getKnownNode(catalog, "new-node")).toMatchObject({ + nodeId: "new-node", + clientId: "openclaw-linux", + clientMode: "node", + displayName: "Pending Node", + platform: "linux", + version: "1.2.3", + coreVersion: "1.2.4", + uiVersion: "1.2.5", + deviceFamily: "desktop", + modelIdentifier: "x86_64", + remoteIp: "100.0.0.20", + approvalState: "pending-approval", + pendingRequestId: "request-1", + caps: [], + commands: [], + paired: false, + connected: false, + }); + expect(getKnownNode(catalog, "new-node")?.permissions).toBeUndefined(); + }); + + it("preserves pending first approval when a metadata reconnect omits permissions", () => { + const catalog = createKnownNodeCatalog({ + pairedDevices: [pairedDevice({ deviceId: "new-node" })], + pairedNodes: [], + pendingNodes: [pendingNode({ nodeId: "new-node" })], + connectedNodes: [ + { + nodeId: "new-node", + connId: "conn-1", + client: {} as never, + displayName: "New Node", + platform: "macos", + declaredCaps: ["camera", "screen"], + caps: [], + declaredCommands: ["screen.snapshot", "system.run"], + commands: [], + connectedAtMs: 1, + }, + ], + }); + + const node = getKnownNode(catalog, "new-node"); + expect(node?.approvalState).toBe("pending-approval"); + expect(node?.pendingRequestId).toBe("request-1"); + expect(node?.pendingDeclaredPermissions).toEqual({ camera: true, screen: true }); + expect(node?.permissions).toBeUndefined(); + }); + + it("reports pending reapproval without making declarations effective", () => { + const catalog = createKnownNodeCatalog({ + pairedDevices: [pairedDevice()], + pairedNodes: [ + pairedNode({ + caps: ["camera"], + commands: ["screen.snapshot"], + permissions: { camera: true }, + }), + ], + pendingNodes: [pendingNode()], + connectedNodes: [ + { + nodeId: "mac-1", + connId: "conn-1", + client: {} as never, + displayName: "Mac", + platform: "macos", + declaredCaps: ["camera", "screen"], + caps: ["camera"], + declaredCommands: ["screen.snapshot", "system.run"], + commands: ["screen.snapshot"], + declaredPermissions: { camera: true, screen: true }, + permissions: { camera: true }, + connectedAtMs: 1, + }, + ], + }); + + const node = getKnownNode(catalog, "mac-1"); + expect(node?.approvalState).toBe("pending-reapproval"); + expect(node?.pendingRequestId).toBe("request-1"); + expect(node?.pendingDeclaredCaps).toEqual(["camera", "screen"]); + expect(node?.pendingDeclaredCommands).toEqual(["screen.snapshot", "system.run"]); + expect(node?.pendingDeclaredPermissions).toEqual({ camera: true, screen: true }); + expect(node?.caps).toEqual(["camera"]); + expect(node?.commands).toEqual(["screen.snapshot"]); + expect(node?.permissions).toEqual({ camera: true }); + }); + + it("ignores a pending reapproval that no longer matches the live declaration", () => { + const catalog = createKnownNodeCatalog({ + pairedDevices: [pairedDevice()], + pairedNodes: [ + pairedNode({ + caps: ["camera"], + commands: ["screen.snapshot"], + permissions: { camera: true }, + }), + ], + pendingNodes: [pendingNode()], + connectedNodes: [ + { + nodeId: "mac-1", + connId: "conn-1", + client: {} as never, + displayName: "Mac", + platform: "macos", + declaredCaps: ["camera"], + caps: ["camera"], + declaredCommands: ["screen.snapshot"], + commands: ["screen.snapshot"], + declaredPermissions: { camera: true }, + permissions: { camera: true }, + connectedAtMs: 1, + }, + ], + }); + + const entry = getKnownNodeEntry(catalog, "mac-1"); + expect(entry?.pendingNodePairing).toBeUndefined(); + const node = getKnownNode(catalog, "mac-1"); + expect(node?.approvalState).toBe("approved"); + expect(node?.pendingRequestId).toBeUndefined(); + expect(node?.pendingDeclaredCaps).toBeUndefined(); + expect(node?.pendingDeclaredCommands).toBeUndefined(); + expect(node?.pendingDeclaredPermissions).toBeUndefined(); + expect(node?.caps).toEqual(["camera"]); + expect(node?.commands).toEqual(["screen.snapshot"]); + expect(node?.permissions).toEqual({ camera: true }); + }); + it("ignores malformed node capability entries instead of throwing", () => { const catalog = createKnownNodeCatalog({ pairedDevices: [], diff --git a/src/gateway/node-catalog.ts b/src/gateway/node-catalog.ts index 0ba0bbb721f..309787fa576 100644 --- a/src/gateway/node-catalog.ts +++ b/src/gateway/node-catalog.ts @@ -3,7 +3,11 @@ import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce"; import { normalizeSortedUniqueTrimmedStringList } from "@openclaw/normalization-core/string-normalization"; import { hasEffectivePairedDeviceRole, type PairedDevice } from "../infra/device-pairing.js"; -import type { NodePairingPairedNode } from "../infra/node-pairing.js"; +import { + sameNodeApprovalSurfaceSet, + sameNodePermissionSurface, +} from "../infra/node-pairing-surface.js"; +import type { NodePairingPairedNode, NodePairingPendingRequest } from "../infra/node-pairing.js"; import type { NodeListNode } from "../shared/node-list-types.js"; import type { NodeSession } from "./node-registry.js"; @@ -38,10 +42,29 @@ type KnownNodeApprovedSource = { lastSeenReason?: string; }; +type KnownNodePendingSource = { + requestId: string; + nodeId: string; + displayName?: string; + platform?: string; + version?: string; + coreVersion?: string; + uiVersion?: string; + clientId?: string; + clientMode?: string; + remoteIp?: string; + deviceFamily?: string; + modelIdentifier?: string; + caps: string[]; + commands: string[]; + permissions?: Record; +}; + type KnownNodeEntry = { nodeId: string; devicePairing?: KnownNodeDevicePairingSource; nodePairing?: KnownNodeApprovedSource; + pendingNodePairing?: KnownNodePendingSource; live?: NodeSession; effective: NodeListNode; }; @@ -89,6 +112,46 @@ function buildApprovedNodeSource(entry: NodePairingPairedNode): KnownNodeApprove }; } +function buildPendingNodeSource(entry: NodePairingPendingRequest): KnownNodePendingSource { + return { + requestId: entry.requestId, + nodeId: entry.nodeId, + displayName: entry.displayName, + platform: entry.platform, + version: entry.version, + coreVersion: entry.coreVersion, + uiVersion: entry.uiVersion, + clientId: entry.clientId, + clientMode: entry.clientMode, + remoteIp: entry.remoteIp, + deviceFamily: entry.deviceFamily, + modelIdentifier: entry.modelIdentifier, + caps: uniqueSortedStrings(entry.caps), + commands: uniqueSortedStrings(entry.commands), + permissions: entry.permissions, + }; +} + +function resolveCurrentPendingNodePairing(params: { + pending?: KnownNodePendingSource; + nodePairing?: KnownNodeApprovedSource; + live?: NodeSession; +}): KnownNodePendingSource | undefined { + const { pending, nodePairing, live } = params; + if (!pending || !live) { + return pending; + } + const declaredPermissions = + !nodePairing && live.declaredPermissions === undefined + ? pending.permissions + : live.declaredPermissions; + return sameNodeApprovalSurfaceSet(pending.caps, live.declaredCaps) && + sameNodeApprovalSurfaceSet(pending.commands, live.declaredCommands) && + sameNodePermissionSurface(pending.permissions, declaredPermissions) + ? pending + : undefined; +} + function resolveEffectiveLastSeen(params: { live?: NodeSession; devicePairing?: KnownNodeDevicePairingSource; @@ -127,28 +190,54 @@ function buildEffectiveKnownNode(entry: { nodeId: string; devicePairing?: KnownNodeDevicePairingSource; nodePairing?: KnownNodeApprovedSource; + pendingNodePairing?: KnownNodePendingSource; live?: NodeSession; }): NodeListNode { - const { nodeId, devicePairing, nodePairing, live } = entry; + const { nodeId, devicePairing, nodePairing, pendingNodePairing, live } = entry; const lastSeen = resolveEffectiveLastSeen({ live, devicePairing, nodePairing }); return { nodeId, - displayName: live?.displayName ?? nodePairing?.displayName ?? devicePairing?.displayName, - platform: live?.platform ?? nodePairing?.platform ?? devicePairing?.platform, - version: live?.version ?? nodePairing?.version, - coreVersion: live?.coreVersion ?? nodePairing?.coreVersion, - uiVersion: live?.uiVersion ?? nodePairing?.uiVersion, - clientId: live?.clientId ?? devicePairing?.clientId, - clientMode: live?.clientMode ?? devicePairing?.clientMode, - deviceFamily: live?.deviceFamily ?? nodePairing?.deviceFamily, - modelIdentifier: live?.modelIdentifier ?? nodePairing?.modelIdentifier, - remoteIp: live?.remoteIp ?? nodePairing?.remoteIp ?? devicePairing?.remoteIp, + displayName: + live?.displayName ?? + nodePairing?.displayName ?? + devicePairing?.displayName ?? + pendingNodePairing?.displayName, + platform: + live?.platform ?? + nodePairing?.platform ?? + devicePairing?.platform ?? + pendingNodePairing?.platform, + version: live?.version ?? nodePairing?.version ?? pendingNodePairing?.version, + coreVersion: live?.coreVersion ?? nodePairing?.coreVersion ?? pendingNodePairing?.coreVersion, + uiVersion: live?.uiVersion ?? nodePairing?.uiVersion ?? pendingNodePairing?.uiVersion, + clientId: live?.clientId ?? devicePairing?.clientId ?? pendingNodePairing?.clientId, + clientMode: live?.clientMode ?? devicePairing?.clientMode ?? pendingNodePairing?.clientMode, + deviceFamily: + live?.deviceFamily ?? nodePairing?.deviceFamily ?? pendingNodePairing?.deviceFamily, + modelIdentifier: + live?.modelIdentifier ?? nodePairing?.modelIdentifier ?? pendingNodePairing?.modelIdentifier, + remoteIp: + live?.remoteIp ?? + nodePairing?.remoteIp ?? + devicePairing?.remoteIp ?? + pendingNodePairing?.remoteIp, caps: live ? uniqueSortedStrings(live.caps) : uniqueSortedStrings(nodePairing?.caps), commands: live ? uniqueSortedStrings(live.commands) : uniqueSortedStrings(nodePairing?.commands), pathEnv: live?.pathEnv, permissions: live?.permissions ?? nodePairing?.permissions, + approvalState: pendingNodePairing + ? nodePairing + ? "pending-reapproval" + : "pending-approval" + : nodePairing + ? "approved" + : "unapproved", + pendingRequestId: pendingNodePairing?.requestId, + pendingDeclaredCaps: pendingNodePairing?.caps, + pendingDeclaredCommands: pendingNodePairing?.commands, + pendingDeclaredPermissions: pendingNodePairing?.permissions, connectedAtMs: live?.connectedAtMs, lastSeenAtMs: lastSeen.lastSeenAtMs, lastSeenReason: lastSeen.lastSeenReason, @@ -177,6 +266,7 @@ function compareKnownNodes(left: NodeListNode, right: NodeListNode): number { export function createKnownNodeCatalog(params: { pairedDevices: readonly PairedDevice[]; pairedNodes?: readonly NodePairingPairedNode[]; + pendingNodes?: readonly NodePairingPendingRequest[]; connectedNodes: readonly NodeSession[]; }): KnownNodeCatalog { const devicePairingById = new Map( @@ -187,10 +277,18 @@ export function createKnownNodeCatalog(params: { const nodePairingById = new Map( (params.pairedNodes ?? []).map((entry) => [entry.nodeId, buildApprovedNodeSource(entry)]), ); + const pendingNodePairingById = new Map(); + // listNodePairing returns newest requests first; keep the current approval action per node. + for (const entry of params.pendingNodes ?? []) { + if (!pendingNodePairingById.has(entry.nodeId)) { + pendingNodePairingById.set(entry.nodeId, buildPendingNodeSource(entry)); + } + } const liveById = new Map(params.connectedNodes.map((entry) => [entry.nodeId, entry])); const nodeIds = new Set([ ...devicePairingById.keys(), ...nodePairingById.keys(), + ...pendingNodePairingById.keys(), ...liveById.keys(), ]); const entriesById = new Map(); @@ -198,15 +296,22 @@ export function createKnownNodeCatalog(params: { const devicePairing = devicePairingById.get(nodeId); const nodePairing = nodePairingById.get(nodeId); const live = liveById.get(nodeId); + const pendingNodePairing = resolveCurrentPendingNodePairing({ + pending: pendingNodePairingById.get(nodeId), + nodePairing, + live, + }); entriesById.set(nodeId, { nodeId, devicePairing, nodePairing, + pendingNodePairing, live, effective: buildEffectiveKnownNode({ nodeId, devicePairing, nodePairing, + pendingNodePairing, live, }), }); diff --git a/src/gateway/node-connect-reconcile.test.ts b/src/gateway/node-connect-reconcile.test.ts index 6913238e7e3..d0c16aae1f0 100644 --- a/src/gateway/node-connect-reconcile.test.ts +++ b/src/gateway/node-connect-reconcile.test.ts @@ -174,7 +174,7 @@ describe("reconcileNodePairingOnConnect", () => { expect(result.pendingPairing?.request.requestId).toBe("req-caps"); }); - it("preserves the approved surface when paired node upgrade pairing is throttled", async () => { + it("keeps the approved surface when paired-node reapproval is throttled", async () => { const requestPairing = vi.fn(async () => null); const result = await reconcileNodePairingOnConnect({ @@ -195,6 +195,27 @@ describe("reconcileNodePairingOnConnect", () => { expect(result.effectiveCommands).toEqual([]); expect(result.declaredCaps).toEqual(["camera", "screen"]); expect(result.pendingPairing).toBeUndefined(); + expect(result.shouldClearPendingPairings).toBeUndefined(); + }); + + it("defers stale pending reapproval cleanup when the node returns to its approved surface", async () => { + const requestPairing = makePendingPairingRequest("req-unused"); + + const result = await reconcileNodePairingOnConnect({ + cfg: {} as never, + connectParams: makeNodeConnectParams({ + caps: ["camera"], + commands: ["canvas.snapshot"], + }), + pairedNode: makePairedNode({ + caps: ["camera"], + commands: ["canvas.snapshot"], + }), + requestPairing, + }); + + expect(requestPairing).not.toHaveBeenCalled(); + expect(result.shouldClearPendingPairings).toBe(true); }); it("requires a fresh pairing request when paired node permissions change", async () => { diff --git a/src/gateway/node-connect-reconcile.ts b/src/gateway/node-connect-reconcile.ts index babd9f4ff52..c3c8bc91a36 100644 --- a/src/gateway/node-connect-reconcile.ts +++ b/src/gateway/node-connect-reconcile.ts @@ -30,6 +30,7 @@ export type NodeConnectPairingReconcileResult = { declaredPermissions?: Record; effectivePermissions?: Record; pendingPairing?: RequestNodePairingResult; + shouldClearPendingPairings?: boolean; }; function resolveApprovedReconnectCommands(params: { @@ -221,5 +222,6 @@ export async function reconcileNodePairingOnConnect(params: { effectiveCommands: declared, declaredPermissions, effectivePermissions: declaredPermissions, + shouldClearPendingPairings: true, }; } diff --git a/src/gateway/node-reapproval-coordinator.test.ts b/src/gateway/node-reapproval-coordinator.test.ts new file mode 100644 index 00000000000..77c44587087 --- /dev/null +++ b/src/gateway/node-reapproval-coordinator.test.ts @@ -0,0 +1,425 @@ +// Covers paired-node reapproval reuse and changed-surface write limits. +import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { + approveNodePairing, + beginNodePairingConnect, + listNodePairing, + releaseNodePairingCleanupClaim, + requestNodePairing, +} from "../infra/node-pairing.js"; +import { createSuiteTempRootTracker } from "../test-helpers/temp-dir.js"; +import { createNodeReapprovalCoordinator } from "./node-reapproval-coordinator.js"; + +const tempDirs = createSuiteTempRootTracker({ prefix: "openclaw-node-reapproval-" }); + +async function setupPairedNode(baseDir: string): Promise { + const request = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + caps: ["camera"], + }, + baseDir, + ); + await approveNodePairing( + request.request.requestId, + { callerScopes: ["operator.pairing"] }, + baseDir, + ); +} + +describe("node reapproval coordinator", () => { + beforeAll(async () => { + await tempDirs.setup(); + }); + + afterAll(async () => { + await tempDirs.cleanup(); + }); + + test("reuses identical pending state without consuming changed-surface quota", async () => { + const baseDir = await tempDirs.make("reuse"); + await setupPairedNode(baseDir); + const pending = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }, + baseDir, + ); + const coordinator = createNodeReapprovalCoordinator({ + maxAttempts: 1, + windowMs: 60_000, + lockoutMs: 60_000, + }); + + const matchingConnect = await beginNodePairingConnect("node-1", baseDir); + await expect( + coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }, + cleanupClaim: matchingConnect.cleanupClaim, + baseDir, + }), + ).resolves.toMatchObject({ + request: { requestId: pending.request.requestId }, + created: false, + }); + if (matchingConnect.cleanupClaim) { + await releaseNodePairingCleanupClaim(matchingConnect.cleanupClaim); + } + + const changedConnect = await beginNodePairingConnect("node-1", baseDir); + await expect( + coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "microphone"], + }, + cleanupClaim: changedConnect.cleanupClaim, + baseDir, + }), + ).resolves.toMatchObject({ + request: { caps: ["camera", "microphone"] }, + created: true, + }); + if (changedConnect.cleanupClaim) { + await releaseNodePairingCleanupClaim(changedConnect.cleanupClaim); + } + + await expect( + coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "location"], + }, + baseDir, + }), + ).resolves.toBeNull(); + expect((await listNodePairing(baseDir)).pending).toEqual([ + expect.objectContaining({ caps: ["camera", "microphone"] }), + ]); + + coordinator.dispose(); + }); + + test("stops accepting work after disposal", async () => { + const baseDir = await tempDirs.make("dispose"); + await setupPairedNode(baseDir); + const coordinator = createNodeReapprovalCoordinator(); + coordinator.dispose(); + + await expect( + coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }, + baseDir, + }), + ).resolves.toBeNull(); + expect((await listNodePairing(baseDir)).pending).toEqual([]); + }); + + test("bounds metadata refreshes while preserving the latest accepted values", async () => { + const baseDir = await tempDirs.make("metadata"); + await setupPairedNode(baseDir); + await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + displayName: "Old Name", + caps: ["camera", "screen"], + }, + baseDir, + ); + const coordinator = createNodeReapprovalCoordinator({ + maxAttempts: 1, + windowMs: 60_000, + lockoutMs: 60_000, + }); + + await expect( + coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + displayName: "New Name", + caps: ["camera", "screen"], + }, + baseDir, + }), + ).resolves.toMatchObject({ + request: { displayName: "New Name" }, + created: false, + }); + await expect( + coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + displayName: "Newest Name", + caps: ["camera", "screen"], + }, + baseDir, + }), + ).resolves.toBeNull(); + expect((await listNodePairing(baseDir)).pending).toEqual([ + expect.objectContaining({ displayName: "New Name" }), + ]); + + coordinator.dispose(); + }); + + test("coalesces concurrent reconnect work before pairing storage", async () => { + const baseDir = await tempDirs.make("concurrent"); + await setupPairedNode(baseDir); + const coordinator = createNodeReapprovalCoordinator({ + maxAttempts: 1, + windowMs: 60_000, + lockoutMs: 60_000, + }); + const input = { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }; + + const first = coordinator.request({ input, baseDir }); + const coalesced = coordinator.request({ input, baseDir }); + + await expect(first).resolves.toMatchObject({ + request: { caps: ["camera", "screen"] }, + }); + await expect(coalesced).resolves.toMatchObject({ + request: { caps: ["camera", "screen"] }, + created: false, + }); + expect((await listNodePairing(baseDir)).pending).toHaveLength(1); + + coordinator.dispose(); + }); + + test("queues one distinct concurrent declaration", async () => { + const baseDir = await tempDirs.make("distinct"); + await setupPairedNode(baseDir); + const coordinator = createNodeReapprovalCoordinator({ + maxAttempts: 2, + windowMs: 60_000, + lockoutMs: 60_000, + }); + + const first = coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }, + baseDir, + }); + const second = coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "microphone"], + }, + baseDir, + }); + + await expect(first).resolves.toMatchObject({ + request: { caps: ["camera", "screen"] }, + }); + await expect(second).resolves.toMatchObject({ + request: { caps: ["camera", "microphone"] }, + }); + expect((await listNodePairing(baseDir)).pending).toEqual([ + expect.objectContaining({ caps: ["camera", "microphone"] }), + ]); + + coordinator.dispose(); + }); + + test("keeps only the latest request waiting behind active work", async () => { + const baseDir = await tempDirs.make("latest"); + await setupPairedNode(baseDir); + const coordinator = createNodeReapprovalCoordinator({ + maxAttempts: 2, + windowMs: 60_000, + lockoutMs: 60_000, + }); + + const active = coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }, + baseDir, + }); + const superseded = coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "microphone"], + }, + baseDir, + }); + const latest = coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "location"], + }, + baseDir, + }); + + await expect(active).resolves.toMatchObject({ + request: { caps: ["camera", "screen"] }, + }); + await expect(superseded).resolves.toBeNull(); + await expect(latest).resolves.toMatchObject({ + request: { caps: ["camera", "location"] }, + }); + expect((await listNodePairing(baseDir)).pending).toEqual([ + expect.objectContaining({ caps: ["camera", "location"] }), + ]); + + coordinator.dispose(); + }); + + test("cancels queued work when the latest declaration matches active work", async () => { + const baseDir = await tempDirs.make("active-latest"); + await setupPairedNode(baseDir); + const coordinator = createNodeReapprovalCoordinator({ + maxAttempts: 2, + windowMs: 60_000, + lockoutMs: 60_000, + }); + const activeInput = { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }; + + const active = coordinator.request({ input: activeInput, baseDir }); + const stale = coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "microphone"], + }, + baseDir, + }); + const latest = coordinator.request({ input: activeInput, baseDir }); + + await expect(active).resolves.toMatchObject({ + request: { caps: ["camera", "screen"] }, + }); + await expect(stale).resolves.toBeNull(); + await expect(latest).resolves.toMatchObject({ + request: { caps: ["camera", "screen"] }, + created: false, + }); + expect((await listNodePairing(baseDir)).pending).toEqual([ + expect.objectContaining({ caps: ["camera", "screen"] }), + ]); + + coordinator.dispose(); + }); + + test("retains the newest cleanup claim across equivalent reconnects", async () => { + const baseDir = await tempDirs.make("cleanup-generation"); + await setupPairedNode(baseDir); + const pending = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }, + baseDir, + ); + const first = await beginNodePairingConnect("node-1", baseDir); + const staleCleanup = await beginNodePairingConnect("node-1", baseDir); + const latest = await beginNodePairingConnect("node-1", baseDir); + expect(first.cleanupClaim).toBeDefined(); + expect(staleCleanup.cleanupClaim).toBeDefined(); + expect(latest.cleanupClaim).toBeDefined(); + const coordinator = createNodeReapprovalCoordinator(); + const input = { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }; + + const firstReuse = coordinator.request({ + input, + cleanupClaim: first.cleanupClaim, + baseDir, + }); + const latestReuse = coordinator.request({ + input, + cleanupClaim: latest.cleanupClaim, + baseDir, + }); + const cleanup = coordinator.finalizeCleanup(staleCleanup.cleanupClaim!); + + await expect(firstReuse).resolves.toMatchObject({ + request: { requestId: pending.request.requestId }, + }); + await expect(latestReuse).resolves.toMatchObject({ + request: { requestId: pending.request.requestId }, + created: false, + }); + await expect(cleanup).resolves.toEqual([]); + expect((await listNodePairing(baseDir)).pending).toEqual([ + expect.objectContaining({ requestId: pending.request.requestId }), + ]); + + coordinator.dispose(); + }); + + test("serializes stale cleanup behind pending-request reuse", async () => { + const baseDir = await tempDirs.make("cleanup-order"); + await setupPairedNode(baseDir); + const pending = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }, + baseDir, + ); + const snapshot = await beginNodePairingConnect("node-1", baseDir); + expect(snapshot.cleanupClaim).toBeDefined(); + const coordinator = createNodeReapprovalCoordinator(); + + const reused = coordinator.request({ + input: { + nodeId: "node-1", + platform: "darwin", + caps: ["camera", "screen"], + }, + cleanupClaim: snapshot.cleanupClaim, + baseDir, + }); + const cleanup = coordinator.finalizeCleanup(snapshot.cleanupClaim!); + + await expect(reused).resolves.toMatchObject({ + request: { requestId: pending.request.requestId }, + created: false, + }); + await expect(cleanup).resolves.toEqual([]); + expect((await listNodePairing(baseDir)).pending).toEqual([ + expect.objectContaining({ requestId: pending.request.requestId }), + ]); + + coordinator.dispose(); + }); +}); diff --git a/src/gateway/node-reapproval-coordinator.ts b/src/gateway/node-reapproval-coordinator.ts new file mode 100644 index 00000000000..a8dd4d04a42 --- /dev/null +++ b/src/gateway/node-reapproval-coordinator.ts @@ -0,0 +1,240 @@ +// Coordinates paired-node reapproval requests before they enter pairing storage. +import { + finalizeNodePairingCleanupClaim, + requestNodePairing, + reusePendingNodePairingForReconnect, + type NodePairingCleanupClaim, + type NodePairingRequestInput, + type NodePairingSupersededRequest, + type RequestNodePairingResult, +} from "../infra/node-pairing.js"; +import { + AUTH_RATE_LIMIT_SCOPE_NODE_REAPPROVAL, + buildRateLimitIdentityKey, + createAuthRateLimiter, + type RateLimitConfig, +} from "./auth-rate-limit.js"; +import { withSerializedKeyedAttempt } from "./rate-limit-attempt-serialization.js"; + +type ReapprovalRequestParams = { + input: NodePairingRequestInput; + cleanupClaim?: NodePairingCleanupClaim; + baseDir?: string; +}; + +type DeferredResult = { + promise: Promise; + resolve: (result: RequestNodePairingResult | null) => void; + reject: (error: unknown) => void; +}; + +type QueuedRequest = { + fingerprint: string; + params: ReapprovalRequestParams; + deferred: DeferredResult; + followers: DeferredResult[]; +}; + +type NodeRequestState = { + activeFingerprint: string; + queued?: QueuedRequest; +}; + +export type NodeReapprovalCoordinator = { + request: (params: ReapprovalRequestParams) => Promise; + finalizeCleanup: (claim: NodePairingCleanupClaim) => Promise; + dispose: () => void; +}; + +function createDeferredResult(): DeferredResult { + let resolve!: DeferredResult["resolve"]; + let reject!: DeferredResult["reject"]; + const promise = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise; + reject = rejectPromise; + }); + return { promise, resolve, reject }; +} + +function normalizeFingerprintList(value: string[] | undefined): string[] | undefined { + return value + ? [ + ...new Set(value.map((entry) => entry.trim()).filter((entry) => entry.length > 0)), + ].toSorted() + : undefined; +} + +function buildRequestFingerprint(input: NodePairingRequestInput): string { + const permissions = input.permissions + ? Object.fromEntries( + Object.entries(input.permissions).toSorted(([left], [right]) => left.localeCompare(right)), + ) + : undefined; + return JSON.stringify({ + nodeId: input.nodeId.trim(), + clientId: input.clientId, + clientMode: input.clientMode, + displayName: input.displayName, + platform: input.platform, + version: input.version, + coreVersion: input.coreVersion, + uiVersion: input.uiVersion, + deviceFamily: input.deviceFamily, + modelIdentifier: input.modelIdentifier, + caps: normalizeFingerprintList(input.caps), + commands: normalizeFingerprintList(input.commands), + permissions, + remoteIp: input.remoteIp, + silent: Boolean(input.silent), + }); +} + +/** Creates the gateway-lifetime owner for paired-node reapproval write limits. */ +export function createNodeReapprovalCoordinator( + config?: RateLimitConfig, +): NodeReapprovalCoordinator { + const limiter = createAuthRateLimiter({ + ...config, + exemptLoopback: false, + }); + const requestStates = new Map(); + let disposed = false; + + const executeRequest = async ({ + input, + cleanupClaim, + baseDir, + }: ReapprovalRequestParams): Promise => { + if (disposed) { + return null; + } + const reused = await reusePendingNodePairingForReconnect(input, cleanupClaim, baseDir); + if (reused) { + return reused; + } + + const nodeId = input.nodeId.trim(); + const identityKey = buildRateLimitIdentityKey("node", nodeId); + const rateCheck = limiter.check(identityKey, AUTH_RATE_LIMIT_SCOPE_NODE_REAPPROVAL); + if (!rateCheck.allowed) { + return null; + } + const result = await requestNodePairing(input, baseDir); + limiter.recordFailure(identityKey, AUTH_RATE_LIMIT_SCOPE_NODE_REAPPROVAL); + return result; + }; + + const finishActiveRequest = (nodeId: string, state: NodeRequestState, fingerprint: string) => { + if (requestStates.get(nodeId) !== state || state.activeFingerprint !== fingerprint) { + return; + } + if (!state.queued) { + requestStates.delete(nodeId); + } + }; + + const startFirstRequest = ( + nodeId: string, + state: NodeRequestState, + request: QueuedRequest, + ): void => { + void withSerializedKeyedAttempt({ + key: `node-reapproval:${nodeId}`, + run: async () => { + try { + request.deferred.resolve(await executeRequest(request.params)); + } catch (error) { + request.deferred.reject(error); + } finally { + finishActiveRequest(nodeId, state, request.fingerprint); + } + }, + }); + }; + + const startQueuedRequest = (nodeId: string, state: NodeRequestState): void => { + void withSerializedKeyedAttempt({ + key: `node-reapproval:${nodeId}`, + run: async () => { + const queued = state.queued; + if (!queued) { + return; + } + state.queued = undefined; + state.activeFingerprint = queued.fingerprint; + try { + queued.deferred.resolve(await executeRequest(queued.params)); + for (const follower of queued.followers) { + follower.resolve(null); + } + } catch (error) { + queued.deferred.reject(error); + for (const follower of queued.followers) { + follower.reject(error); + } + } finally { + finishActiveRequest(nodeId, state, queued.fingerprint); + } + }, + }); + }; + + return { + request(params) { + if (disposed) { + return Promise.resolve(null); + } + const nodeId = params.input.nodeId.trim(); + const fingerprint = buildRequestFingerprint(params.input); + const state = requestStates.get(nodeId); + if (!state) { + const deferred = createDeferredResult(); + const nextState: NodeRequestState = { activeFingerprint: fingerprint }; + requestStates.set(nodeId, nextState); + startFirstRequest(nodeId, nextState, { + fingerprint, + params, + deferred, + followers: [], + }); + return deferred.promise; + } + if (state.queued?.fingerprint === fingerprint) { + const follower = createDeferredResult(); + state.queued.params = params; + state.queued.followers.push(follower); + return follower.promise; + } + + const deferred = createDeferredResult(); + if (state.queued) { + state.queued.deferred.resolve(null); + for (const follower of state.queued.followers) { + follower.resolve(null); + } + state.queued = { fingerprint, params, deferred, followers: [] }; + } else { + state.queued = { fingerprint, params, deferred, followers: [] }; + startQueuedRequest(nodeId, state); + } + return deferred.promise; + }, + async finalizeCleanup(claim) { + return await withSerializedKeyedAttempt({ + key: `node-reapproval:${claim.nodeId}`, + run: async () => await finalizeNodePairingCleanupClaim(claim), + }); + }, + dispose() { + disposed = true; + for (const state of requestStates.values()) { + state.queued?.deferred.resolve(null); + for (const follower of state.queued?.followers ?? []) { + follower.resolve(null); + } + } + requestStates.clear(); + limiter.dispose(); + }, + }; +} diff --git a/src/gateway/rate-limit-attempt-serialization.ts b/src/gateway/rate-limit-attempt-serialization.ts index fc62da0d921..e51484c2a07 100644 --- a/src/gateway/rate-limit-attempt-serialization.ts +++ b/src/gateway/rate-limit-attempt-serialization.ts @@ -12,13 +12,12 @@ function buildSerializationKey(ip: string | undefined, scope: string | undefined return `${normalizeScope(scope)}:${normalizeRateLimitClientIp(ip)}`; } -/** Runs one rate-limit attempt after prior attempts for the same IP/scope finish. */ -export async function withSerializedRateLimitAttempt(params: { - ip: string | undefined; - scope: string | undefined; +/** Runs one attempt after prior work for the same stable key finishes. */ +export async function withSerializedKeyedAttempt(params: { + key: string; run: () => Promise; }): Promise { - const key = buildSerializationKey(params.ip, params.scope); + const key = params.key; const previous = pendingAttempts.get(key) ?? Promise.resolve(); let releaseCurrent!: () => void; const current = new Promise((resolve) => { @@ -37,3 +36,15 @@ export async function withSerializedRateLimitAttempt(params: { } } } + +/** Runs one rate-limit attempt after prior attempts for the same IP/scope finish. */ +export async function withSerializedRateLimitAttempt(params: { + ip: string | undefined; + scope: string | undefined; + run: () => Promise; +}): Promise { + return await withSerializedKeyedAttempt({ + key: buildSerializationKey(params.ip, params.scope), + run: params.run, + }); +} diff --git a/src/gateway/server-methods/nodes.ts b/src/gateway/server-methods/nodes.ts index a5622dff03c..dafc03a709e 100644 --- a/src/gateway/server-methods/nodes.ts +++ b/src/gateway/server-methods/nodes.ts @@ -61,6 +61,7 @@ import { import { applyPluginNodeInvokePolicy } from "../node-invoke-plugin-policy.js"; import { sanitizeNodeInvokeParamsForForwarding } from "../node-invoke-sanitize.js"; import type { NodeSession } from "../node-registry.js"; +import { ADMIN_SCOPE, PAIRING_SCOPE } from "../operator-scopes.js"; import { refreshClientPluginNodeCapability } from "../plugin-node-capability.js"; import type { NodeEventContext } from "../server-node-events-types.js"; import { @@ -119,6 +120,11 @@ type PendingNodeAction = { const pendingNodeActionsById = new Map(); +function canReadPendingNodePairing(client: GatewayClient | null): boolean { + const scopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : []; + return scopes.includes(ADMIN_SCOPE) || scopes.includes(PAIRING_SCOPE); +} + function normalizeBrowserProxyPath(value: string): string { const trimmed = value.trim(); if (!trimmed) { @@ -932,7 +938,7 @@ export const nodeHandlers: GatewayRequestHandlers = { respond(true, { nodeId: updated.nodeId, displayName: updated.displayName }, undefined); }); }, - "node.list": async ({ params, respond, context }) => { + "node.list": async ({ params, respond, client, context }) => { if (!validateNodeListParams(params)) { respondInvalidParams({ respond, @@ -949,13 +955,14 @@ export const nodeHandlers: GatewayRequestHandlers = { const catalog = createKnownNodeCatalog({ pairedDevices: devicePairing.paired, pairedNodes: nodePairing.paired, + pendingNodes: canReadPendingNodePairing(client) ? nodePairing.pending : undefined, connectedNodes: context.nodeRegistry.listConnected(), }); const nodes = listKnownNodes(catalog); respond(true, { ts: Date.now(), nodes }, undefined); }); }, - "node.describe": async ({ params, respond, context }) => { + "node.describe": async ({ params, respond, client, context }) => { if (!validateNodeDescribeParams(params)) { respondInvalidParams({ respond, @@ -978,6 +985,7 @@ export const nodeHandlers: GatewayRequestHandlers = { const catalog = createKnownNodeCatalog({ pairedDevices: devicePairing.paired, pairedNodes: nodePairing.paired, + pendingNodes: canReadPendingNodePairing(client) ? nodePairing.pending : undefined, connectedNodes: context.nodeRegistry.listConnected(), }); const node = getKnownNode(catalog, id); diff --git a/src/gateway/server-ws-runtime.ts b/src/gateway/server-ws-runtime.ts index eacdc553707..8c86b2c58cf 100644 --- a/src/gateway/server-ws-runtime.ts +++ b/src/gateway/server-ws-runtime.ts @@ -31,6 +31,7 @@ export function attachGatewayWsHandlers(params: GatewayWsRuntimeParams) { getRequiredSharedGatewaySessionGeneration: params.getRequiredSharedGatewaySessionGeneration, rateLimiter: params.rateLimiter, browserRateLimiter: params.browserRateLimiter, + nodeReapprovalCoordinator: params.nodeReapprovalCoordinator, preauthHandshakeTimeoutMs: params.preauthHandshakeTimeoutMs, isStartupPending: params.isStartupPending, gatewayMethods: params.gatewayMethods, diff --git a/src/gateway/server.impl.ts b/src/gateway/server.impl.ts index 0db622eeebe..5e16103aa82 100644 --- a/src/gateway/server.impl.ts +++ b/src/gateway/server.impl.ts @@ -74,6 +74,7 @@ import { type GatewayMethodRegistry, } from "./methods/registry.js"; import { isLoopbackHost } from "./net.js"; +import { createNodeReapprovalCoordinator } from "./node-reapproval-coordinator.js"; import { listChannelPluginConfigTargetIds, pluginConfigTargetsChanged, @@ -819,6 +820,7 @@ export async function startGatewayServer( const rateLimitConfig = cfgAtStart.gateway?.auth?.rateLimit; const { rateLimiter: authRateLimiter, browserRateLimiter: browserAuthRateLimiter } = createGatewayAuthRateLimiters(rateLimitConfig); + const nodeReapprovalCoordinator = createNodeReapprovalCoordinator(rateLimitConfig); const controlUiRootState = await startupTrace.measure("control-ui.root", () => resolveGatewayControlUiRootState({ @@ -990,7 +992,10 @@ export async function startGatewayServer( runtimeState.skillsRefreshTimer = null; }, skillsChangeUnsub: runtimeState.skillsChangeUnsub, - disposeAuthRateLimiter: () => authRateLimiter.dispose(), + disposeAuthRateLimiter: () => { + authRateLimiter.dispose(); + nodeReapprovalCoordinator.dispose(); + }, disposeBrowserAuthRateLimiter: () => browserAuthRateLimiter.dispose(), stopModelPricingRefresh: runtimeState.stopModelPricingRefresh, stopChannelHealthMonitor: () => runtimeState?.channelHealthMonitor?.stop(), @@ -1531,6 +1536,7 @@ export async function startGatewayServer( getRequiredSharedGatewaySessionGeneration(sharedGatewaySessionGenerationState), rateLimiter: authRateLimiter, browserRateLimiter: browserAuthRateLimiter, + nodeReapprovalCoordinator, preauthHandshakeTimeoutMs, isStartupPending: isGatewayStartupPending, gatewayMethods: runtimeState.gatewayMethods, diff --git a/src/gateway/server.node-pairing-authz.test.ts b/src/gateway/server.node-pairing-authz.test.ts index 70707eaf006..b028c44d705 100644 --- a/src/gateway/server.node-pairing-authz.test.ts +++ b/src/gateway/server.node-pairing-authz.test.ts @@ -10,6 +10,7 @@ import { } from "../infra/node-pairing.js"; import { createSuiteTempRootTracker } from "../test-helpers/temp-dir.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js"; +import { callGateway } from "./call.js"; import { loadDeviceIdentity, openTrackedWs, @@ -111,10 +112,18 @@ async function expectRePairingRequest(params: { }); const connectedControlWs = controlWs; - let lastNodes: Array<{ nodeId: string; connected?: boolean; commands?: string[] }> = []; + type NodeDiagnostics = { + nodeId: string; + connected?: boolean; + commands?: string[]; + approvalState?: string; + pendingRequestId?: string; + pendingDeclaredCommands?: string[]; + }; + let lastNodes: NodeDiagnostics[] = []; await vi.waitFor(async () => { const list = await rpcReq<{ - nodes?: Array<{ nodeId: string; connected?: boolean; commands?: string[] }>; + nodes?: NodeDiagnostics[]; }>(connectedControlWs, "node.list", {}); lastNodes = list.payload?.nodes ?? []; const node = lastNodes.find( @@ -140,6 +149,23 @@ async function expectRePairingRequest(params: { const pending = pairing.pending?.find((entry) => entry.nodeId === pairedNode.identity.deviceId); expect(pending?.nodeId).toBe(pairedNode.identity.deviceId); expect(pending?.commands).toEqual(params.reconnectCommands); + const listedNode = lastNodes.find((entry) => entry.nodeId === pairedNode.identity.deviceId); + expect(listedNode).toMatchObject({ + approvalState: "pending-reapproval", + pendingRequestId: pending?.requestId, + pendingDeclaredCommands: params.reconnectCommands, + commands: params.expectedVisibleCommands, + }); + + const described = await rpcReq(connectedControlWs, "node.describe", { + nodeId: pairedNode.identity.deviceId, + }); + expect(described.payload).toMatchObject({ + approvalState: "pending-reapproval", + pendingRequestId: pending?.requestId, + pendingDeclaredCommands: params.reconnectCommands, + commands: params.expectedVisibleCommands, + }); } finally { controlWs?.close(); await firstClient?.stopAndWait(); @@ -314,7 +340,166 @@ describe("gateway node pairing authorization", () => { }); }); + describeWithGatewayServer("pending diagnostics scopes", (getStarted) => { + test("shows pending pairing records to direct-local backend shared-auth callers", async () => { + const pendingOnlyNodeId = "node-local-backend-pending"; + const pending = await requestNodePairing({ + nodeId: pendingOnlyNodeId, + platform: "macos", + commands: ["system.run"], + }); + + const listed = await callGateway<{ + nodes?: Array<{ + nodeId: string; + approvalState?: string; + pendingRequestId?: string; + }>; + }>({ + config: { + gateway: { + mode: "local", + bind: "loopback", + port: getStarted().port, + auth: { mode: "token", token: "secret" }, + }, + }, + method: "node.list", + scopes: ["operator.read", "operator.pairing"], + requireLocalBackendSharedAuth: true, + timeoutMs: 2_000, + }); + + expect(listed.nodes).toContainEqual( + expect.objectContaining({ + nodeId: pendingOnlyNodeId, + approvalState: "pending-approval", + pendingRequestId: pending.request.requestId, + }), + ); + }); + + test("hides pending pairing records from read-only callers", async () => { + const pairedNodeId = "node-read-only-paired"; + const pendingOnlyNodeId = "node-read-only-pending"; + const initial = await requestNodePairing({ + nodeId: pairedNodeId, + platform: "macos", + commands: ["screen.snapshot"], + }); + await approveNodePairing(initial.request.requestId, { + callerScopes: ["operator.pairing", "operator.write"], + }); + await requestNodePairing({ + nodeId: pairedNodeId, + platform: "macos", + commands: ["screen.snapshot", "system.run"], + }); + await requestNodePairing({ + nodeId: pendingOnlyNodeId, + platform: "macos", + commands: ["system.run"], + }); + + const ws = await openTrackedWs(getStarted().port); + try { + await connectOk(ws, { + token: "secret", + scopes: ["operator.read"], + deviceIdentityPath: `${await makeNodePairingStateDir()}/read-only.json`, + }); + + type NodeDiagnostics = { + nodeId: string; + approvalState?: string; + pendingRequestId?: string; + pendingDeclaredCommands?: string[]; + }; + const listed = await rpcReq<{ nodes?: NodeDiagnostics[] }>(ws, "node.list", {}); + expect(listed.ok).toBe(true); + const nodes = listed.payload?.nodes ?? []; + expect(nodes.some((node) => node.nodeId === pendingOnlyNodeId)).toBe(false); + expect(nodes.find((node) => node.nodeId === pairedNodeId)).toEqual( + expect.objectContaining({ + nodeId: pairedNodeId, + approvalState: "approved", + }), + ); + expect(nodes.find((node) => node.nodeId === pairedNodeId)).not.toHaveProperty( + "pendingRequestId", + ); + expect(nodes.find((node) => node.nodeId === pairedNodeId)).not.toHaveProperty( + "pendingDeclaredCommands", + ); + + const described = await rpcReq(ws, "node.describe", { + nodeId: pairedNodeId, + }); + expect(described.payload).toEqual( + expect.objectContaining({ + nodeId: pairedNodeId, + approvalState: "approved", + }), + ); + expect(described.payload).not.toHaveProperty("pendingRequestId"); + expect(described.payload).not.toHaveProperty("pendingDeclaredCommands"); + + const pendingOnly = await rpcReq(ws, "node.describe", { nodeId: pendingOnlyNodeId }); + expect(pendingOnly.ok).toBe(false); + expect(pendingOnly.error?.message).toContain("unknown nodeId"); + } finally { + ws.close(); + } + }); + }); + describeWithGatewayServer("paired node reconnects", (getStarted) => { + test("clears stale reapproval when a node returns to its approved surface", async () => { + const pairedNode = await pairDeviceIdentity({ + name: "node-reverted-reapproval", + role: "node", + scopes: [], + clientId: GATEWAY_CLIENT_NAMES.NODE_HOST, + clientMode: GATEWAY_CLIENT_MODES.NODE, + }); + const initial = await requestNodePairing({ + nodeId: pairedNode.identity.deviceId, + platform: "macos", + deviceFamily: "Mac", + commands: ["screen.snapshot"], + }); + await approveNodePairing(initial.request.requestId, { + callerScopes: ["operator.pairing", "operator.write"], + }); + + const upgraded = await connectNodeClient({ + port: getStarted().port, + deviceIdentity: pairedNode.identity, + commands: ["screen.snapshot", "system.run"], + }); + await upgraded.stopAndWait(); + expect( + (await listNodePairing()).pending.some( + (entry) => entry.nodeId === pairedNode.identity.deviceId, + ), + ).toBe(true); + + const reverted = await connectNodeClient({ + port: getStarted().port, + deviceIdentity: pairedNode.identity, + commands: ["screen.snapshot"], + }); + await reverted.stopAndWait(); + + await vi.waitFor(async () => { + expect( + (await listNodePairing()).pending.some( + (entry) => entry.nodeId === pairedNode.identity.deviceId, + ), + ).toBe(false); + }); + }); + test("requests re-pairing when a paired node reconnects with upgraded commands", async () => { await expectRePairingRequest({ started: getStarted(), diff --git a/src/gateway/server.node-pairing-rate-limit.test.ts b/src/gateway/server.node-pairing-rate-limit.test.ts index 43d5e8c0501..97af03aa87f 100644 --- a/src/gateway/server.node-pairing-rate-limit.test.ts +++ b/src/gateway/server.node-pairing-rate-limit.test.ts @@ -36,7 +36,11 @@ async function openWs(port: number) { return ws; } -async function attemptNodePairing(port: number, identityPath: string) { +async function attemptNodePairing( + port: number, + identityPath: string, + surface: { caps?: string[]; commands?: string[] } = {}, +) { const ws = await openWs(port); try { return await connectReq(ws, { @@ -44,8 +48,9 @@ async function attemptNodePairing(port: number, identityPath: string) { role: "node", scopes: [], client: NODE_CLIENT, - commands: ["system.run"], + commands: surface.commands ?? ["system.run"], deviceIdentityPath: identityPath, + ...(surface.caps ? { caps: surface.caps } : {}), }); } finally { ws.close(); @@ -110,7 +115,7 @@ describe("node pairing rate limit", () => { }); }); - test("keeps paired reconnects on the approved surface when upgrade pairing is limited", async () => { + test("records paired reconnect reapproval despite first-time pairing limits", async () => { testState.gatewayAuth = { mode: "token", token: "secret", @@ -127,7 +132,10 @@ describe("node pairing rate limit", () => { `openclaw-node-pairing-upgrade-${randomUUID()}`, ); const pairedIdentityPath = `${identityPrefix}-paired.json`; - await approveNodeIdentity({ identityPath: pairedIdentityPath, caps: ["camera"] }); + const pairedIdentity = await approveNodeIdentity({ + identityPath: pairedIdentityPath, + caps: ["camera"], + }); const firstTimeResponses = await Promise.all( Array.from( @@ -159,7 +167,70 @@ describe("node pairing rate limit", () => { }); } - expect((await listNodePairing()).pending).toHaveLength(3); + const pending = (await listNodePairing()).pending; + expect(pending).toHaveLength(4); + expect(pending.find((entry) => entry.nodeId === pairedIdentity.deviceId)?.caps).toEqual([ + "camera", + "screen", + ]); + }); + }); + + test("reuses identical paired reapproval without rejecting the node", async () => { + testState.gatewayAuth = { + mode: "token", + token: "secret", + rateLimit: { + maxAttempts: 1, + windowMs: 60_000, + lockoutMs: 60_000, + exemptLoopback: true, + }, + }; + await withGatewayServer(async ({ port }) => { + const identityPath = path.join(os.tmpdir(), `openclaw-node-reapproval-${randomUUID()}.json`); + const identity = await approveNodeIdentity({ identityPath, caps: ["camera"] }); + + const responses = await Promise.all( + Array.from( + { length: 20 }, + async () => + await attemptNodePairing(port, identityPath, { + caps: ["camera", "screen"], + commands: [], + }), + ), + ); + expect(responses.every((res) => res.ok)).toBe(true); + const pendingBeforeReuse = (await listNodePairing()).pending.find( + (entry) => entry.nodeId === identity.deviceId, + ); + expect(pendingBeforeReuse).toBeDefined(); + + await expect( + attemptNodePairing(port, identityPath, { + caps: ["camera", "screen"], + commands: [], + }), + ).resolves.toMatchObject({ ok: true }); + expect( + (await listNodePairing()).pending.find((entry) => entry.nodeId === identity.deviceId), + ).toMatchObject({ + requestId: pendingBeforeReuse!.requestId, + ts: pendingBeforeReuse!.ts, + }); + + const changedSurface = await attemptNodePairing(port, identityPath, { + caps: ["camera", "microphone"], + commands: [], + }); + expect(changedSurface.ok).toBe(true); + expect( + (await listNodePairing()).pending.find((entry) => entry.nodeId === identity.deviceId), + ).toMatchObject({ + requestId: pendingBeforeReuse!.requestId, + caps: ["camera", "screen"], + }); }); }); }); diff --git a/src/gateway/server.node-version-mismatch.test.ts b/src/gateway/server.node-version-mismatch.test.ts index ec8fd09f68b..c6cf1152db7 100644 --- a/src/gateway/server.node-version-mismatch.test.ts +++ b/src/gateway/server.node-version-mismatch.test.ts @@ -2,9 +2,12 @@ // gateway accepts matching node hosts and rejects incompatible local runtimes. import fs from "node:fs"; import path from "node:path"; -import { afterAll, beforeAll, describe, expect, test } from "vitest"; +import { afterAll, beforeAll, describe, expect, test, vi } from "vitest"; +import { WebSocket } from "ws"; +import { approveNodePairing, listNodePairing, requestNodePairing } from "../infra/node-pairing.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js"; import { resolveRuntimeServiceVersion } from "../version.js"; +import { pairDeviceIdentity } from "./device-authz.test-helpers.js"; import { connectGatewayClient } from "./test-helpers.e2e.js"; import { installGatewayTestHooks, startServer } from "./test-helpers.js"; @@ -75,6 +78,108 @@ describe("node host version mismatch guard", () => { ).rejects.toThrow(/client version mismatch|version mismatch/i); }); + test("rejected local reconnects preserve the active node pending reapproval", async () => { + const pairedNode = await pairDeviceIdentity({ + name: "node-version-mismatch-pending-reapproval", + role: "node", + scopes: [], + clientId: GATEWAY_CLIENT_NAMES.NODE_HOST, + clientMode: GATEWAY_CLIENT_MODES.NODE, + }); + const initial = await requestNodePairing({ + nodeId: pairedNode.identity.deviceId, + platform: "macos", + deviceFamily: "Mac", + commands: ["screen.snapshot"], + }); + await approveNodePairing(initial.request.requestId, { + callerScopes: ["operator.pairing", "operator.write"], + }); + + const upgraded = await connectGatewayClient({ + url: `ws://127.0.0.1:${port}`, + token: "secret", + role: "node", + clientName: GATEWAY_CLIENT_NAMES.NODE_HOST, + clientDisplayName: "test-node-upgraded", + clientVersion: gatewayVersion, + instanceId: TEST_LOCAL_NODE_ID, + platform: "macos", + deviceFamily: "Mac", + mode: GATEWAY_CLIENT_MODES.NODE, + scopes: [], + commands: ["screen.snapshot", "system.run"], + deviceIdentity: pairedNode.identity, + }); + try { + const connectReverted = async (clientVersion: string, clientDisplayName: string) => + await connectGatewayClient({ + url: `ws://127.0.0.1:${port}`, + token: "secret", + role: "node", + clientName: GATEWAY_CLIENT_NAMES.NODE_HOST, + clientDisplayName, + clientVersion, + instanceId: TEST_LOCAL_NODE_ID, + platform: "macos", + deviceFamily: "Mac", + mode: GATEWAY_CLIENT_MODES.NODE, + scopes: [], + commands: ["screen.snapshot"], + deviceIdentity: pairedNode.identity, + timeoutMs: 5_000, + timeoutMessage: "expected rejected reconnect", + }); + const pendingBefore = (await listNodePairing()).pending.find( + (entry) => entry.nodeId === pairedNode.identity.deviceId, + ); + expect(pendingBefore?.commands).toEqual(["screen.snapshot", "system.run"]); + + await expect(connectReverted("2020.1.1", "test-node-reverted-stale")).rejects.toThrow( + /client version mismatch|version mismatch/i, + ); + + const pendingAfterVersionMismatch = (await listNodePairing()).pending.find( + (entry) => entry.nodeId === pairedNode.identity.deviceId, + ); + expect(pendingAfterVersionMismatch?.requestId).toBe(pendingBefore?.requestId); + expect(pendingAfterVersionMismatch?.commands).toEqual(["screen.snapshot", "system.run"]); + + const originalSend = Reflect.get(WebSocket.prototype, "send"); + let failNextHelloOk = true; + const sendSpy = vi.spyOn(WebSocket.prototype, "send").mockImplementation(function ( + this: WebSocket, + ...args: Parameters + ) { + if (failNextHelloOk && typeof args[0] === "string" && args[0].includes('"hello-ok"')) { + failNextHelloOk = false; + const callback = args.findLast((arg) => typeof arg === "function"); + if (typeof callback === "function") { + callback(new Error("test hello-ok send failure")); + } + return; + } + Reflect.apply(originalSend, this, args); + }); + try { + await expect( + connectReverted(gatewayVersion, "test-node-reverted-hello-failure"), + ).rejects.toThrow(/gateway closed during connect/i); + expect(failNextHelloOk).toBe(false); + } finally { + sendSpy.mockRestore(); + } + + const pendingAfterHelloFailure = (await listNodePairing()).pending.find( + (entry) => entry.nodeId === pairedNode.identity.deviceId, + ); + expect(pendingAfterHelloFailure?.requestId).toBe(pendingBefore?.requestId); + expect(pendingAfterHelloFailure?.commands).toEqual(["screen.snapshot", "system.run"]); + } finally { + await upgraded.stopAndWait({ timeoutMs: 2_000 }); + } + }); + test("local node with dev/test version is allowed (not a released version)", async () => { // "dev" does not match YYYY.M.PATCH, so the guard skips const client = await connectGatewayClient({ diff --git a/src/gateway/server/ws-connection.ts b/src/gateway/server/ws-connection.ts index ec1e431a66d..81216bcf075 100644 --- a/src/gateway/server/ws-connection.ts +++ b/src/gateway/server/ws-connection.ts @@ -20,6 +20,7 @@ import { resolvePreauthHandshakeTimeoutMs } from "../handshake-timeouts.js"; import { resolveHostedPluginSurfaceUrl } from "../hosted-plugin-surface-url.js"; import type { GatewayMethodRegistry } from "../methods/registry.js"; import { isLoopbackAddress } from "../net.js"; +import type { NodeReapprovalCoordinator } from "../node-reapproval-coordinator.js"; import type { PluginNodeCapabilitySurface } from "../plugin-node-capability.js"; import { MAX_BUFFERED_BYTES, @@ -155,6 +156,7 @@ export type GatewayWsSharedHandlerParams = { rateLimiter?: AuthRateLimiter; /** Browser-origin fallback limiter (loopback is never exempt). */ browserRateLimiter?: AuthRateLimiter; + nodeReapprovalCoordinator?: NodeReapprovalCoordinator; preauthHandshakeTimeoutMs?: number; isStartupPending?: () => boolean; gatewayMethods: string[]; @@ -232,6 +234,7 @@ export function attachGatewayWsConnectionHandler(params: AttachGatewayWsConnecti ), rateLimiter, browserRateLimiter, + nodeReapprovalCoordinator, isStartupPending, gatewayMethods, events, @@ -528,6 +531,7 @@ export function attachGatewayWsConnectionHandler(params: AttachGatewayWsConnecti getRequiredSharedGatewaySessionGeneration, rateLimiter, browserRateLimiter, + nodeReapprovalCoordinator, isStartupPending, gatewayMethods, events, diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index c074dd3f1b5..31882fa6259 100644 --- a/src/gateway/server/ws-connection/message-handler.ts +++ b/src/gateway/server/ws-connection/message-handler.ts @@ -68,8 +68,12 @@ import { runWithDiagnosticTraceContext, } from "../../../infra/diagnostic-trace-context.js"; import { - getPairedNode, + beginNodePairingConnect, + finalizeNodePairingCleanupClaim, + releaseNodePairingCleanupClaim, requestNodePairing, + type NodePairingCleanupClaim, + type RequestNodePairingResult, updatePairedNodeMetadata, } from "../../../infra/node-pairing.js"; import { upsertPresence } from "../../../infra/system-presence.js"; @@ -110,6 +114,7 @@ import { resolveNodePairingClientIpSource, shouldAutoApproveNodePairingFromTrustedCidrs, } from "../../node-pairing-auto-approve.js"; +import type { NodeReapprovalCoordinator } from "../../node-reapproval-coordinator.js"; import { isOperatorApprovalRuntimeToken } from "../../operator-approval-runtime-token.js"; import { checkBrowserOrigin } from "../../origin-check.js"; import { @@ -210,7 +215,18 @@ async function requestNodePairingFromConnect(params: { input: Parameters[0]; rateLimiter?: AuthRateLimiter; clientIp?: string; -}): Promise>> { + pairedReconnect?: boolean; + cleanupClaim?: NodePairingCleanupClaim; + reapprovalCoordinator?: NodeReapprovalCoordinator; +}): Promise> | null> { + if (params.pairedReconnect) { + return params.reapprovalCoordinator + ? await params.reapprovalCoordinator.request({ + input: params.input, + cleanupClaim: params.cleanupClaim, + }) + : await requestNodePairing(params.input); + } if (!params.rateLimiter) { return await requestNodePairing(params.input); } @@ -362,6 +378,7 @@ export type GatewayWsMessageHandlerParams = { rateLimiter?: AuthRateLimiter; /** Browser-origin fallback limiter (loopback is never exempt). */ browserRateLimiter?: AuthRateLimiter; + nodeReapprovalCoordinator?: NodeReapprovalCoordinator; isStartupPending?: () => boolean; gatewayMethods: string[]; events: string[]; @@ -406,6 +423,7 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar getRequiredSharedGatewaySessionGeneration, rateLimiter, browserRateLimiter, + nodeReapprovalCoordinator, isStartupPending, gatewayMethods, events, @@ -539,6 +557,42 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar } const text = rawDataToString(data); + let pendingNodePairingCleanup: NodePairingCleanupClaim | undefined; + const broadcastNodePairingResult = (result: RequestNodePairingResult) => { + const context = buildRequestContext(); + const resolvedAt = Date.now(); + for (const superseded of result.created ? (result.superseded ?? []) : []) { + context.broadcast( + "node.pair.resolved", + { + requestId: superseded.requestId, + nodeId: superseded.nodeId, + decision: "rejected", + ts: resolvedAt, + }, + { dropIfSlow: true }, + ); + } + if (result.created) { + context.broadcast("node.pair.requested", result.request, { + dropIfSlow: true, + }); + } + }; + const releasePendingNodePairingCleanup = async () => { + const claim = pendingNodePairingCleanup; + pendingNodePairingCleanup = undefined; + if (!claim) { + return; + } + try { + await releaseNodePairingCleanupClaim(claim); + } catch (error) { + logGateway.warn( + `failed to release pending pairing cleanup for ${claim.nodeId}: ${formatForLog(error)}`, + ); + } + }; try { const parsed = JSON.parse(text); const frameType = @@ -1614,10 +1668,11 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar } } if (role === "node") { + const nodeId = connectParams.device?.id ?? connectParams.client.id; + const nodePairingSnapshot = await beginNodePairingConnect(nodeId); + const pairedNode = nodePairingSnapshot.pairedNode; + pendingNodePairingCleanup = nodePairingSnapshot.cleanupClaim; let reconciliation: Awaited>; - const pairedNode = await getPairedNode( - connectParams.device?.id ?? connectParams.client.id, - ); try { reconciliation = await reconcileNodePairingOnConnect({ cfg: getRuntimeConfig(), @@ -1625,23 +1680,18 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar pairedNode, reportedClientIp, requestPairing: async (input) => { - try { - return await requestNodePairingFromConnect({ - input, - rateLimiter: authRateLimiter, - clientIp: browserRateLimitClientIp, - }); - } catch (error) { - if (error instanceof NodePairingRateLimitError && pairedNode) { - // Paired upgrade reconnects can keep their approved surface; - // only the fresh pending request is throttled here. - return null; - } - throw error; - } + return await requestNodePairingFromConnect({ + input, + rateLimiter: authRateLimiter, + clientIp: browserRateLimitClientIp, + pairedReconnect: pairedNode !== null, + cleanupClaim: pendingNodePairingCleanup, + reapprovalCoordinator: nodeReapprovalCoordinator, + }); }, }); } catch (error) { + await releasePendingNodePairingCleanup(); if (error instanceof NodePairingRateLimitError) { rejectUnauthorized({ ok: false, @@ -1653,24 +1703,11 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar } throw error; } - if (reconciliation.pendingPairing?.created) { - const requestContext = buildRequestContext(); - const resolvedAt = Date.now(); - for (const superseded of reconciliation.pendingPairing.superseded ?? []) { - requestContext.broadcast( - "node.pair.resolved", - { - requestId: superseded.requestId, - nodeId: superseded.nodeId, - decision: "rejected", - ts: resolvedAt, - }, - { dropIfSlow: true }, - ); - } - requestContext.broadcast("node.pair.requested", reconciliation.pendingPairing.request, { - dropIfSlow: true, - }); + if (!reconciliation.shouldClearPendingPairings) { + await releasePendingNodePairingCleanup(); + } + if (reconciliation.pendingPairing) { + broadcastNodePairingResult(reconciliation.pendingPairing); } const nodeConnectParams = connectParams as ConnectParams & { declaredCaps?: string[]; @@ -1691,6 +1728,7 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar const presenceKey = shouldTrackPresence ? (device?.id ?? instanceId ?? connId) : undefined; if (isClosed()) { + await releasePendingNodePairingCleanup(); setCloseCause("connect-aborted-before-register", { ...clientMeta, auth: authMethod, @@ -1784,6 +1822,7 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar gatewayVersion, }, }); + await releasePendingNodePairingCleanup(); close(1008, "client version mismatch"); return; } @@ -1791,6 +1830,7 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar } if (!setClient(nextClient)) { + await releasePendingNodePairingCleanup(); setCloseCause("connect-aborted-before-register", { ...clientMeta, auth: authMethod, @@ -1974,10 +2014,38 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar ); } } + await releasePendingNodePairingCleanup(); setCloseCause("hello-send-failed", { error: formatForLog(err) }); close(); return; } + if (pendingNodePairingCleanup) { + const context = buildRequestContext(); + const cleanupClaim = pendingNodePairingCleanup; + pendingNodePairingCleanup = undefined; + try { + const resolvedPairings = nodeReapprovalCoordinator + ? await nodeReapprovalCoordinator.finalizeCleanup(cleanupClaim) + : await finalizeNodePairingCleanupClaim(cleanupClaim); + const resolvedAt = Date.now(); + for (const resolved of resolvedPairings) { + context.broadcast( + "node.pair.resolved", + { + requestId: resolved.requestId, + nodeId: resolved.nodeId, + decision: "rejected", + ts: resolvedAt, + }, + { dropIfSlow: true }, + ); + } + } catch (error) { + logGateway.warn( + `failed to clear stale pending pairings for ${cleanupClaim.nodeId}: ${formatForLog(error)}`, + ); + } + } logWs("out", "hello-ok", { connId, methods: gatewayMethods.length, @@ -2107,6 +2175,7 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar } void dispatch; } catch (err) { + await releasePendingNodePairingCleanup(); logGateway.error(`parse/handle error: ${String(err)}`); logWs("out", "parse-error", { connId, error: formatForLog(err) }); if (!getClient()) { diff --git a/src/infra/node-pairing.test.ts b/src/infra/node-pairing.test.ts index 79bc378c59f..fc002735ef0 100644 --- a/src/infra/node-pairing.test.ts +++ b/src/infra/node-pairing.test.ts @@ -4,10 +4,14 @@ import { afterAll, beforeAll, describe, expect, test } from "vitest"; import { createSuiteTempRootTracker } from "../test-helpers/temp-dir.js"; import { approveNodePairing, + beginNodePairingConnect, + finalizeNodePairingCleanupClaim, getPairedNode, listNodePairing, + releaseNodePairingCleanupClaim, removePairedNode, requestNodePairing, + reusePendingNodePairingForReconnect, updatePairedNodeMetadata, verifyNodeToken, } from "./node-pairing.js"; @@ -87,6 +91,8 @@ describe("node pairing tokens", () => { expect(first.created).toBe(true); expect(second.created).toBe(false); expect(second.request.requestId).toBe(first.request.requestId); + expect("revision" in first.request).toBe(false); + expect("revision" in second.request).toBe(false); const commandFirst = await requestNodePairing( { @@ -149,6 +155,7 @@ describe("node pairing tokens", () => { const pendingNode = findRecordByField(pairing.pending, "nodeId", "node-4"); expect(pendingNode.commands).toEqual(["canvas.present"]); expect(pendingNode.requiredApproveScopes).toEqual(["operator.pairing", "operator.write"]); + expect("revision" in pendingNode).toBe(false); expect(pairing.paired).toEqual([]); }); }); @@ -327,6 +334,243 @@ describe("node pairing tokens", () => { }); }); + test("rejects every pending request for one node without removing its approval", async () => { + await withNodePairingDir(async (baseDir) => { + await setupPairedNode(baseDir); + const pending = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run", "canvas.snapshot"], + }, + baseDir, + ); + const snapshot = await beginNodePairingConnect("node-1", baseDir); + expect(snapshot.cleanupClaim).toBeDefined(); + + await expect(finalizeNodePairingCleanupClaim(snapshot.cleanupClaim!)).resolves.toEqual([ + { requestId: pending.request.requestId, nodeId: "node-1" }, + ]); + await expect(finalizeNodePairingCleanupClaim(snapshot.cleanupClaim!)).resolves.toEqual([]); + + const pairing = await listNodePairing(baseDir); + expect(pairing.pending).toEqual([]); + expect(pairing.paired).toHaveLength(1); + expect(pairing.paired[0]?.nodeId).toBe("node-1"); + await expect(getPairedNode("node-1", baseDir)).resolves.toMatchObject({ + commands: ["system.run"], + }); + }); + }); + + test("preserves a pending request refreshed after the connect snapshot", async () => { + await withNodePairingDir(async (baseDir) => { + await setupPairedNode(baseDir); + const pending = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run", "canvas.snapshot"], + }, + baseDir, + ); + const snapshot = await beginNodePairingConnect("node-1", baseDir); + expect(snapshot.cleanupClaim).toBeDefined(); + const refreshed = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run", "canvas.snapshot"], + }, + baseDir, + ); + expect(refreshed.request.requestId).toBe(pending.request.requestId); + + await expect(finalizeNodePairingCleanupClaim(snapshot.cleanupClaim!)).resolves.toEqual([]); + expect((await listNodePairing(baseDir)).pending).toHaveLength(1); + }); + }); + + test("reuses an unchanged reconnect request without leaving stale cleanup ownership", async () => { + await withNodePairingDir(async (baseDir) => { + await setupPairedNode(baseDir); + const pending = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run", "canvas.snapshot"], + }, + baseDir, + ); + const snapshot = await beginNodePairingConnect("node-1", baseDir); + expect(snapshot.cleanupClaim).toBeDefined(); + + await expect( + reusePendingNodePairingForReconnect( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run", "canvas.snapshot"], + }, + snapshot.cleanupClaim, + baseDir, + ), + ).resolves.toMatchObject({ + request: { requestId: pending.request.requestId }, + created: false, + }); + await expect(finalizeNodePairingCleanupClaim(snapshot.cleanupClaim!)).resolves.toEqual([]); + await expect( + approveNodePairing( + pending.request.requestId, + { callerScopes: ["operator.pairing", "operator.admin"] }, + baseDir, + ), + ).resolves.toMatchObject({ requestId: pending.request.requestId }); + }); + }); + + test("does not reuse a reconnect request when pending metadata changed", async () => { + await withNodePairingDir(async (baseDir) => { + await setupPairedNode(baseDir); + await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + displayName: "Old Name", + commands: ["system.run", "canvas.snapshot"], + }, + baseDir, + ); + const snapshot = await beginNodePairingConnect("node-1", baseDir); + + await expect( + reusePendingNodePairingForReconnect( + { + nodeId: "node-1", + platform: "darwin", + displayName: "New Name", + commands: ["system.run", "canvas.snapshot"], + }, + snapshot.cleanupClaim, + baseDir, + ), + ).resolves.toBeNull(); + if (snapshot.cleanupClaim) { + await releaseNodePairingCleanupClaim(snapshot.cleanupClaim); + } + }); + }); + + test("preserves newer cleanup ownership after an older reconnect reuses pending state", async () => { + await withNodePairingDir(async (baseDir) => { + await setupPairedNode(baseDir); + const pending = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run", "canvas.snapshot"], + }, + baseDir, + ); + const matchingReconnect = await beginNodePairingConnect("node-1", baseDir); + const changedReconnect = await beginNodePairingConnect("node-1", baseDir); + expect(matchingReconnect.cleanupClaim).toBeDefined(); + expect(changedReconnect.cleanupClaim).toBeDefined(); + + await reusePendingNodePairingForReconnect( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run", "canvas.snapshot"], + }, + matchingReconnect.cleanupClaim, + baseDir, + ); + + await expect( + finalizeNodePairingCleanupClaim(changedReconnect.cleanupClaim!), + ).resolves.toEqual([{ requestId: pending.request.requestId, nodeId: "node-1" }]); + expect((await listNodePairing(baseDir)).pending).toEqual([]); + }); + }); + + test("preserves a replacement pending request created after the connect snapshot", async () => { + await withNodePairingDir(async (baseDir) => { + await setupPairedNode(baseDir); + const pending = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run", "canvas.snapshot"], + }, + baseDir, + ); + const snapshot = await beginNodePairingConnect("node-1", baseDir); + expect(snapshot.cleanupClaim).toBeDefined(); + const replacement = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run", "canvas.present"], + }, + baseDir, + ); + expect(replacement.request.requestId).not.toBe(pending.request.requestId); + + await expect(finalizeNodePairingCleanupClaim(snapshot.cleanupClaim!)).resolves.toEqual([]); + const remaining = (await listNodePairing(baseDir)).pending; + expect(remaining).toHaveLength(1); + expect(remaining[0]?.requestId).toBe(replacement.request.requestId); + }); + }); + + test("blocks approval until a reconnect cleanup claim is released", async () => { + await withNodePairingDir(async (baseDir) => { + await setupPairedNode(baseDir); + const pending = await requestNodePairing( + { + nodeId: "node-1", + platform: "darwin", + commands: ["system.run", "canvas.snapshot"], + }, + baseDir, + ); + const firstSnapshot = await beginNodePairingConnect("node-1", baseDir); + const secondSnapshot = await beginNodePairingConnect("node-1", baseDir); + expect(firstSnapshot.cleanupClaim).toBeDefined(); + expect(secondSnapshot.cleanupClaim?.generation).toBeGreaterThan( + firstSnapshot.cleanupClaim!.generation, + ); + + await expect( + approveNodePairing( + pending.request.requestId, + { callerScopes: ["operator.pairing", "operator.admin"] }, + baseDir, + ), + ).resolves.toBeNull(); + + await releaseNodePairingCleanupClaim(firstSnapshot.cleanupClaim!); + await expect( + approveNodePairing( + pending.request.requestId, + { callerScopes: ["operator.pairing", "operator.admin"] }, + baseDir, + ), + ).resolves.toBeNull(); + + await releaseNodePairingCleanupClaim(secondSnapshot.cleanupClaim!); + await expect( + approveNodePairing( + pending.request.requestId, + { callerScopes: ["operator.pairing", "operator.admin"] }, + baseDir, + ), + ).resolves.toMatchObject({ requestId: pending.request.requestId }); + }); + }); + test("requires the right scopes to approve node requests", async () => { await withNodePairingDir(async (baseDir) => { const systemRunRequest = await requestNodePairing( diff --git a/src/infra/node-pairing.ts b/src/infra/node-pairing.ts index 4340c01fdb7..f4ad1b11a78 100644 --- a/src/infra/node-pairing.ts +++ b/src/infra/node-pairing.ts @@ -47,6 +47,23 @@ export type NodePairingPendingRequest = NodePairingRequestInput & { ts: number; }; +type NodePairingPendingRecord = NodePairingPendingRequest & { + revision?: string; +}; + +export type NodePairingPendingSnapshot = Pick & { + revision?: string; +}; + +/** Opaque claim preventing approval while a reconnect resolves stale pending state. */ +export type NodePairingCleanupClaim = { + baseDir: string | undefined; + generation: number; + nodeId: string; + pendingPath: string; + observed: NodePairingPendingSnapshot[]; +}; + /** Pending request summary returned when a new approval surface supersedes older requests. */ export type NodePairingSupersededRequest = Pick; @@ -79,7 +96,7 @@ type NodePairingList = { }; type NodePairingStateFile = { - pendingById: Record; + pendingById: Record; pairedByNodeId: Record; }; @@ -87,13 +104,16 @@ const PENDING_TTL_MS = 5 * 60 * 1000; const OPERATOR_ROLE = "operator"; const withLock = createAsyncLock(); +const activeCleanupRevisionClaims = new Map>(); +let nextCleanupClaimGeneration = 0; function buildPendingNodePairingRequest(params: { requestId?: string; req: NodePairingRequestInput; -}): NodePairingPendingRequest { +}): NodePairingPendingRecord { return { requestId: params.requestId ?? randomUUID(), + revision: randomUUID(), nodeId: params.req.nodeId, clientId: params.req.clientId, clientMode: params.req.clientMode, @@ -114,11 +134,12 @@ function buildPendingNodePairingRequest(params: { } function refreshPendingNodePairingRequest( - existing: NodePairingPendingRequest, + existing: NodePairingPendingRecord, incoming: NodePairingRequestInput, -): NodePairingPendingRequest { +): NodePairingPendingRecord { return { ...existing, + revision: randomUUID(), clientId: incoming.clientId ?? existing.clientId, clientMode: incoming.clientMode ?? existing.clientMode, displayName: incoming.displayName ?? existing.displayName, @@ -139,7 +160,7 @@ function refreshPendingNodePairingRequest( } function samePendingApprovalSurface( - existing: NodePairingPendingRequest, + existing: NodePairingPendingRecord, incoming: NodePairingRequestInput, ): boolean { const incomingCaps = normalizeArrayBackedTrimmedStringList(incoming.caps) ?? existing.caps; @@ -154,8 +175,27 @@ function samePendingApprovalSurface( ); } +function samePendingReconnectMetadata( + existing: NodePairingPendingRecord, + incoming: NodePairingRequestInput, +): boolean { + return ( + (incoming.clientId ?? existing.clientId) === existing.clientId && + (incoming.clientMode ?? existing.clientMode) === existing.clientMode && + (incoming.displayName ?? existing.displayName) === existing.displayName && + (incoming.platform ?? existing.platform) === existing.platform && + (incoming.version ?? existing.version) === existing.version && + (incoming.coreVersion ?? existing.coreVersion) === existing.coreVersion && + (incoming.uiVersion ?? existing.uiVersion) === existing.uiVersion && + (incoming.deviceFamily ?? existing.deviceFamily) === existing.deviceFamily && + (incoming.modelIdentifier ?? existing.modelIdentifier) === existing.modelIdentifier && + (incoming.remoteIp ?? existing.remoteIp) === existing.remoteIp && + Boolean(existing.silent && incoming.silent) === Boolean(existing.silent) + ); +} + function mergeNodePairingReplacementInput(params: { - existing: readonly NodePairingPendingRequest[]; + existing: readonly NodePairingPendingRecord[]; incoming: NodePairingRequestInput; }): NodePairingRequestInput { const latest = params.existing[0]; @@ -180,16 +220,34 @@ function mergeNodePairingReplacementInput(params: { }; } -function resolveNodeApprovalRequiredScopes( - pending: NodePairingPendingRequest, -): NodeApprovalScope[] { +function resolveNodeApprovalRequiredScopes(pending: NodePairingPendingRecord): NodeApprovalScope[] { const commands = Array.isArray(pending.commands) ? pending.commands : []; return resolveNodePairApprovalScopes(commands); } -function toPendingNodePairingEntry(pending: NodePairingPendingRequest): NodePairingPendingEntry { +function toPublicPendingNodePairingRequest( + pending: NodePairingPendingRecord, +): NodePairingPendingRequest { + const { revision: _revision, ...request } = pending; + return request; +} + +function toPendingNodePairingSnapshot( + pending: NodePairingPendingRecord, +): NodePairingPendingSnapshot { + const snapshot: NodePairingPendingSnapshot = { + requestId: pending.requestId, + nodeId: pending.nodeId, + }; + if (pending.revision) { + snapshot.revision = pending.revision; + } + return snapshot; +} + +function toPendingNodePairingEntry(pending: NodePairingPendingRecord): NodePairingPendingEntry { return { - ...pending, + ...toPublicPendingNodePairingRequest(pending), requiredApproveScopes: resolveNodeApprovalRequiredScopes(pending), }; } @@ -205,7 +263,7 @@ async function loadState(baseDir?: string): Promise { readJsonIfExists(pairedPath), ]); const state: NodePairingStateFile = { - pendingById: coercePairingStateRecord(pending), + pendingById: coercePairingStateRecord(pending), pairedByNodeId: coercePairingStateRecord(paired), }; pruneExpiredPending(state.pendingById, Date.now(), PENDING_TTL_MS); @@ -224,6 +282,61 @@ function normalizeNodeId(nodeId: string) { return nodeId.trim(); } +function buildCleanupRevisionClaimKey( + pendingPath: string, + observed: NodePairingPendingSnapshot, +): string { + return `${pendingPath}\0${observed.requestId}\0${observed.revision ?? ""}`; +} + +function addCleanupClaim(claim: NodePairingCleanupClaim): void { + for (const observed of claim.observed) { + const key = buildCleanupRevisionClaimKey(claim.pendingPath, observed); + const generations = activeCleanupRevisionClaims.get(key) ?? new Set(); + generations.add(claim.generation); + activeCleanupRevisionClaims.set(key, generations); + } +} + +function cleanupClaimIsActive(claim: NodePairingCleanupClaim): boolean { + return claim.observed.some((observed) => { + const key = buildCleanupRevisionClaimKey(claim.pendingPath, observed); + return activeCleanupRevisionClaims.get(key)?.has(claim.generation) === true; + }); +} + +function removeCleanupClaim(claim: NodePairingCleanupClaim): void { + for (const observed of claim.observed) { + const key = buildCleanupRevisionClaimKey(claim.pendingPath, observed); + const generations = activeCleanupRevisionClaims.get(key); + generations?.delete(claim.generation); + if (!generations || generations.size === 0) { + activeCleanupRevisionClaims.delete(key); + } + } +} + +function invalidateCleanupClaimsThrough( + claim: NodePairingCleanupClaim, + pending: NodePairingPendingRecord, + baseDir: string | undefined, +): void { + const pendingPath = resolvePairingPaths(baseDir, "nodes").pendingPath; + const key = buildCleanupRevisionClaimKey(pendingPath, toPendingNodePairingSnapshot(pending)); + const generations = activeCleanupRevisionClaims.get(key); + if (!generations) { + return; + } + for (const generation of generations) { + if (generation <= claim.generation) { + generations.delete(generation); + } + } + if (generations.size === 0) { + activeCleanupRevisionClaims.delete(key); + } +} + function newToken() { return generatePairingToken(); } @@ -248,6 +361,93 @@ export async function getPairedNode( return state.pairedByNodeId[normalizeNodeId(nodeId)] ?? null; } +/** Snapshot pairing state and claim current pending revisions for one paired reconnect. */ +export async function beginNodePairingConnect( + nodeId: string, + baseDir?: string, +): Promise<{ + pairedNode: NodePairingPairedNode | null; + cleanupClaim?: NodePairingCleanupClaim; +}> { + return await withLock(async () => { + const state = await loadState(baseDir); + const normalized = normalizeNodeId(nodeId); + const pairedNode = state.pairedByNodeId[normalized] ?? null; + const observed = Object.values(state.pendingById) + .filter((entry) => entry.nodeId === normalized) + .map(toPendingNodePairingSnapshot); + if (!pairedNode || observed.length === 0) { + return { pairedNode }; + } + const pendingPath = resolvePairingPaths(baseDir, "nodes").pendingPath; + const claim: NodePairingCleanupClaim = { + baseDir, + generation: ++nextCleanupClaimGeneration, + nodeId: normalized, + pendingPath, + observed, + }; + addCleanupClaim(claim); + return { pairedNode, cleanupClaim: claim }; + }); +} + +function pendingHasActiveCleanupClaim( + pending: NodePairingPendingRecord, + baseDir: string | undefined, +): boolean { + const pendingPath = resolvePairingPaths(baseDir, "nodes").pendingPath; + const key = buildCleanupRevisionClaimKey(pendingPath, toPendingNodePairingSnapshot(pending)); + return (activeCleanupRevisionClaims.get(key)?.size ?? 0) > 0; +} + +/** Release a reconnect cleanup claim without changing pending pairing state. */ +export async function releaseNodePairingCleanupClaim( + claim: NodePairingCleanupClaim, +): Promise { + await withLock(async () => { + removeCleanupClaim(claim); + }); +} + +/** Delete pending revisions claimed by a reconnect after hello succeeds. */ +export async function finalizeNodePairingCleanupClaim( + claim: NodePairingCleanupClaim, +): Promise { + return await withLock(async () => { + if (!cleanupClaimIsActive(claim)) { + return []; + } + try { + const state = await loadState(claim.baseDir); + const observedById = new Map( + claim.observed + .filter((entry) => entry.nodeId === claim.nodeId) + .map((entry) => [entry.requestId, entry] as const), + ); + const rejected = Object.values(state.pendingById) + .filter((pending) => { + const observed = observedById.get(pending.requestId); + return observed !== undefined && observed.revision === pending.revision; + }) + .toSorted((left, right) => right.ts - left.ts); + if (rejected.length === 0) { + return []; + } + for (const pending of rejected) { + delete state.pendingById[pending.requestId]; + } + await persistState(state, claim.baseDir); + return rejected.map((pending) => ({ + requestId: pending.requestId, + nodeId: pending.nodeId, + })); + } finally { + removeCleanupClaim(claim); + } + }); +} + /** Create or refresh a pending node pairing request for operator approval. */ export async function requestNodePairing( req: NodePairingRequestInput, @@ -282,7 +482,44 @@ export async function requestNodePairing( .filter((pending) => pending.requestId !== result.request.requestId) .map((pending) => ({ requestId: pending.requestId, nodeId: pending.nodeId })) : []; - return superseded.length > 0 ? { ...result, superseded } : result; + const publicResult = { + ...result, + request: toPublicPendingNodePairingRequest(result.request), + }; + return superseded.length > 0 ? { ...publicResult, superseded } : publicResult; + }); +} + +/** Reuse an unchanged reconnect request without refreshing or writing pairing state. */ +export async function reusePendingNodePairingForReconnect( + req: NodePairingRequestInput, + cleanupClaim: NodePairingCleanupClaim | undefined, + baseDir?: string, +): Promise { + return await withLock(async () => { + const state = await loadState(baseDir); + const nodeId = normalizeNodeId(req.nodeId); + const pendingForNode = Object.values(state.pendingById) + .filter((pending) => pending.nodeId === nodeId) + .toSorted((left, right) => right.ts - left.ts); + if ( + pendingForNode.length === 1 && + samePendingApprovalSurface(pendingForNode[0], { ...req, nodeId }) && + samePendingReconnectMetadata(pendingForNode[0], req) + ) { + const pending = pendingForNode[0]; + // The unchanged reconnect supersedes older cleanup ownership without + // refreshing the request or writing pairing state. + if (cleanupClaim) { + invalidateCleanupClaimsThrough(cleanupClaim, pending, baseDir); + } + return { + status: "pending", + request: toPublicPendingNodePairingRequest(pending), + created: false, + }; + } + return null; }); } @@ -298,6 +535,11 @@ export async function approveNodePairing( if (!pending) { return null; } + // A paired reconnect has atomically observed this revision as stale. + // Approval can resume if the handshake fails and releases its claim. + if (pendingHasActiveCleanupClaim(pending, baseDir)) { + return null; + } const requiredScopes = resolveNodeApprovalRequiredScopes(pending); const missingScope = resolveMissingRequestedScope({ role: OPERATOR_ROLE, diff --git a/src/shared/node-list-types.ts b/src/shared/node-list-types.ts index d77add44f01..a6aec8de8d2 100644 --- a/src/shared/node-list-types.ts +++ b/src/shared/node-list-types.ts @@ -15,6 +15,11 @@ export type NodeListNode = { caps?: string[]; commands?: string[]; permissions?: Record; + approvalState?: "approved" | "pending-approval" | "pending-reapproval" | "unapproved"; + pendingRequestId?: string; + pendingDeclaredCaps?: string[]; + pendingDeclaredCommands?: string[]; + pendingDeclaredPermissions?: Record; paired?: boolean; connected?: boolean; connectedAtMs?: number;