diff --git a/src/gateway/node-connect-reconcile.test.ts b/src/gateway/node-connect-reconcile.test.ts index bb04723fa63c..c4e9b496d529 100644 --- a/src/gateway/node-connect-reconcile.test.ts +++ b/src/gateway/node-connect-reconcile.test.ts @@ -267,15 +267,40 @@ describe("reconcileNodePairingOnConnect", () => { expect(result.shouldClearPendingPairings).toBe(true); }); - it("requires a fresh pairing request when paired node permissions change", async () => { + it("requires a fresh pairing request when paired node permissions widen", async () => { const requestPairing = makePendingPairingRequest("req-permissions"); const result = await reconcileNodePairingOnConnect({ cfg: {} as never, connectParams: makeNodeConnectParams({ + commands: [], + permissions: { camera: true, notifications: true }, + }), + pairedNode: makePairedNode({ commands: [], permissions: { camera: true, notifications: false }, }), + requestPairing, + }); + + expectNodePairingRequest(requestPairing, { + commands: [], + permissions: { camera: true, notifications: true }, + }); + expect(result.effectiveCommands).toEqual([]); + expect(result.effectivePermissions).toEqual({ camera: true, notifications: false }); + expect(result.pendingPairing?.request.requestId).toBe("req-permissions"); + }); + + it("accepts false-only permission metadata without reapproval", async () => { + const requestPairing = vi.fn(); + + const result = await reconcileNodePairingOnConnect({ + cfg: {} as never, + connectParams: makeNodeConnectParams({ + commands: [], + permissions: { camera: true, notifications: false, watchReachable: false }, + }), pairedNode: makePairedNode({ commands: [], permissions: { camera: true }, @@ -283,17 +308,17 @@ describe("reconcileNodePairingOnConnect", () => { requestPairing, }); - expectNodePairingRequest(requestPairing, { - commands: [], - permissions: { camera: true, notifications: false }, + expect(requestPairing).not.toHaveBeenCalled(); + expect(result.effectivePermissions).toEqual({ + camera: true, + notifications: false, + watchReachable: false, }); - expect(result.effectiveCommands).toEqual([]); - expect(result.effectivePermissions).toEqual({ camera: true, notifications: false }); - expect(result.pendingPairing?.request.requestId).toBe("req-permissions"); + expect(result.shouldClearPendingPairings).toBe(true); }); - it("applies declared capability and permission downgrades to the live surface", async () => { - const requestPairing = makePendingPairingRequest("req-downgrade"); + it("applies declared capability and permission downgrades without reapproval", async () => { + const requestPairing = vi.fn(); const result = await reconcileNodePairingOnConnect({ cfg: {} as never, @@ -310,14 +335,11 @@ describe("reconcileNodePairingOnConnect", () => { requestPairing, }); - expectNodePairingRequest(requestPairing, { - caps: ["camera"], - commands: [], - permissions: { camera: false }, - }); + expect(requestPairing).not.toHaveBeenCalled(); expect(result.effectiveCaps).toEqual(["camera"]); expect(result.effectiveCommands).toEqual([]); expect(result.effectivePermissions).toEqual({ camera: false }); - expect(result.pendingPairing?.request.requestId).toBe("req-downgrade"); + expect(result.pendingPairing).toBeUndefined(); + expect(result.shouldClearPendingPairings).toBe(true); }); }); diff --git a/src/gateway/node-connect-reconcile.ts b/src/gateway/node-connect-reconcile.ts index 14d8b0f29df2..6044763c7a62 100644 --- a/src/gateway/node-connect-reconcile.ts +++ b/src/gateway/node-connect-reconcile.ts @@ -2,11 +2,7 @@ // Computes approved runtime surfaces and pending pairing upgrades on reconnect. import type { ConnectParams } from "../../packages/gateway-protocol/src/index.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; -import { - normalizeNodeApprovalSurfaceList, - sameNodeApprovalSurfaceSet, - sameNodePermissionSurface, -} from "../infra/node-pairing-surface.js"; +import { normalizeNodeApprovalSurfaceList } from "../infra/node-pairing-surface.js"; import type { NodePairingPairedNode, NodePairingRequestInput, @@ -87,6 +83,15 @@ function intersectPermissionSurface(params: { return entries.length > 0 ? Object.fromEntries(entries) : undefined; } +function hasPermissionUpgrade(params: { + approved: Record | undefined; + declared: Record | undefined; +}): boolean { + return Object.entries(params.declared ?? {}).some( + ([key, declaredValue]) => declaredValue && params.approved?.[key] !== true, + ); +} + function buildNodePairingRequestInput(params: { nodeId: string; connectParams: ConnectParams; @@ -171,11 +176,13 @@ export async function reconcileNodePairingOnConnect(params: { const approvedCaps = normalizeNodeApprovalSurfaceList(params.pairedNode.caps); const approvedPermissions = normalizePermissionMap(params.pairedNode.permissions); const hasCommandUpgrade = declared.some((command) => !approvedCommands.includes(command)); - const hasCapabilityChange = !sameNodeApprovalSurfaceSet(params.pairedNode.caps, declaredCaps); - const hasPermissionChange = !sameNodePermissionSurface( - params.pairedNode.permissions, - declaredPermissions, + const hasCapabilityUpgrade = declaredCaps.some( + (capability) => !approvedCaps.includes(capability), ); + const permissionUpgrade = hasPermissionUpgrade({ + approved: approvedPermissions, + declared: declaredPermissions, + }); const effectiveApprovedDeclaredCaps = intersectApprovalSurfaceList({ approved: approvedCaps, declared: declaredCaps, @@ -189,16 +196,16 @@ export async function reconcileNodePairingOnConnect(params: { declared: declaredPermissions, }); - // A reconnect may use only the intersection of old approval and new - // declaration until the upgraded caps/commands/permissions are approved. - if (hasCommandUpgrade || hasCapabilityChange || hasPermissionChange) { + // Availability and permission loss only narrow the live surface. Reapproval + // is required when a reconnect widens authority beyond the durable approval. + if (hasCommandUpgrade || hasCapabilityUpgrade || permissionUpgrade) { const pendingPairing = await params.requestPairing( buildNodePairingRequestInput({ nodeId, connectParams: params.connectParams, caps: declaredCaps, commands: declared, - permissions: declaredPermissions ?? (hasPermissionChange ? {} : undefined), + permissions: declaredPermissions ?? (permissionUpgrade ? {} : undefined), remoteIp: params.reportedClientIp, }), ); diff --git a/src/gateway/server.node-pairing-authz.test.ts b/src/gateway/server.node-pairing-authz.test.ts index 5d2d83a38ee9..a80d1f1a4a86 100644 --- a/src/gateway/server.node-pairing-authz.test.ts +++ b/src/gateway/server.node-pairing-authz.test.ts @@ -529,6 +529,52 @@ describe("gateway node pairing authorization", () => { }); describeWithGatewayServer("paired node reconnects", (getStarted) => { + test("keeps iOS approval when a transient permission becomes unavailable", async () => { + const pairedNode = await pairDeviceIdentity({ + name: "ios-transient-permission", + role: "node", + scopes: [], + clientId: GATEWAY_CLIENT_NAMES.IOS_APP, + clientMode: GATEWAY_CLIENT_MODES.NODE, + }); + const initialPermissions = { camera: true, watchReachable: true }; + const initial = await requestNodePairing({ + nodeId: pairedNode.identity.deviceId, + platform: "ios", + deviceFamily: "iPhone", + commands: [], + permissions: initialPermissions, + }); + await approveNodePairing(initial.request.requestId, { + callerScopes: ["operator.pairing", "operator.write"], + }); + + const nodeClient = await connectGatewayClient({ + url: `ws://127.0.0.1:${getStarted().port}`, + token: "secret", + role: "node", + clientName: GATEWAY_CLIENT_NAMES.IOS_APP, + clientDisplayName: "iPhone", + clientVersion: "1.0.0", + platform: "ios", + deviceFamily: "iPhone", + mode: GATEWAY_CLIENT_MODES.NODE, + scopes: [], + commands: [], + permissions: { camera: true, watchReachable: false }, + deviceIdentity: pairedNode.identity, + }); + await nodeClient.stopAndWait(); + + const pairing = await listNodePairing(); + expect(pairing.pending.some((entry) => entry.nodeId === pairedNode.identity.deviceId)).toBe( + false, + ); + await expect(findPairedNode(pairedNode.identity.deviceId)).resolves.toMatchObject({ + permissions: initialPermissions, + }); + }); + test("clears stale reapproval when a node returns to its approved surface", async () => { const pairedNode = await pairDeviceIdentity({ name: "node-reverted-reapproval", diff --git a/src/gateway/test-helpers.e2e.ts b/src/gateway/test-helpers.e2e.ts index a07ccffe233c..ba3e55cf575e 100644 --- a/src/gateway/test-helpers.e2e.ts +++ b/src/gateway/test-helpers.e2e.ts @@ -46,6 +46,7 @@ export async function connectGatewayClient(params: { scopes?: string[]; caps?: string[]; commands?: string[]; + permissions?: Record; instanceId?: string; deviceIdentity?: DeviceIdentity; onEvent?: (evt: { event?: string; payload?: unknown }) => void; @@ -108,6 +109,7 @@ export async function connectGatewayClient(params: { scopes, caps: params.caps, commands: params.commands, + permissions: params.permissions, instanceId: params.instanceId, deviceIdentity, onEvent: params.onEvent,