mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 20:35:39 -06:00
fix(gateway): avoid reapproval on node permission loss (#102661)
This commit is contained in:
committed by
GitHub
parent
b8c1b220e4
commit
5b0e970dc7
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string, boolean> | undefined;
|
||||
declared: Record<string, boolean> | 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,
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -46,6 +46,7 @@ export async function connectGatewayClient(params: {
|
||||
scopes?: string[];
|
||||
caps?: string[];
|
||||
commands?: string[];
|
||||
permissions?: Record<string, boolean>;
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user