From 7cda9df4cb2791fb25ec37f0471ee78971b312c5 Mon Sep 17 00:00:00 2001 From: Agustin Rivera <31522568+eleqtrizit@users.noreply.github.com> Date: Fri, 3 Apr 2026 18:06:35 +0000 Subject: [PATCH] fix(device): reject unapproved token roles --- src/gateway/server-methods/devices.ts | 18 +++++++- .../server.device-token-rotate-authz.test.ts | 37 +++++++++++++++ src/infra/device-pairing.test.ts | 46 +++++++++++++++++++ src/infra/device-pairing.ts | 14 +++++- 4 files changed, 112 insertions(+), 3 deletions(-) diff --git a/src/gateway/server-methods/devices.ts b/src/gateway/server-methods/devices.ts index ab8607fbf6b..bb6a0213ef2 100644 --- a/src/gateway/server-methods/devices.ts +++ b/src/gateway/server-methods/devices.ts @@ -1,6 +1,7 @@ import { approveDevicePairing, getPairedDevice, + listApprovedPairedDeviceRoles, listDevicePairing, removePairedDevice, type DeviceAuthToken, @@ -196,6 +197,7 @@ export const deviceHandlers: GatewayRequestHandlers = { role: string; scopes?: string[]; }; + const normalizedRole = role.trim(); const pairedDevice = await getPairedDevice(deviceId); if (!pairedDevice) { logDeviceTokenRotationDenied({ @@ -211,9 +213,23 @@ export const deviceHandlers: GatewayRequestHandlers = { ); return; } + if (!listApprovedPairedDeviceRoles(pairedDevice).includes(normalizedRole)) { + logDeviceTokenRotationDenied({ + log: context.logGateway, + deviceId, + role, + reason: "unknown-device-or-role", + }); + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_ROTATION_DENIED_MESSAGE), + ); + return; + } const callerScopes = Array.isArray(client?.connect?.scopes) ? client.connect.scopes : []; const requestedScopes = normalizeDeviceAuthScopes( - scopes ?? pairedDevice.tokens?.[role.trim()]?.scopes ?? pairedDevice.scopes, + scopes ?? pairedDevice.tokens?.[normalizedRole]?.scopes ?? pairedDevice.scopes, ); const missingScope = resolveMissingRequestedScope({ role, diff --git a/src/gateway/server.device-token-rotate-authz.test.ts b/src/gateway/server.device-token-rotate-authz.test.ts index e47f968b042..4ae55502679 100644 --- a/src/gateway/server.device-token-rotate-authz.test.ts +++ b/src/gateway/server.device-token-rotate-authz.test.ts @@ -239,4 +239,41 @@ describe("gateway device.token.rotate caller scope guard", () => { started.envSnapshot.restore(); } }); + + test("rejects rotating a token for an unapproved role on an existing paired device", async () => { + const started = await startServerWithClient("secret"); + const attacker = await issueOperatorToken({ + name: "rotate-unapproved-role", + approvedScopes: ["operator.pairing"], + tokenScopes: ["operator.pairing"], + clientId: GATEWAY_CLIENT_NAMES.TEST, + clientMode: GATEWAY_CLIENT_MODES.TEST, + }); + + let pairingWs: WebSocket | undefined; + try { + pairingWs = await connectPairingScopedOperator({ + port: started.port, + identityPath: attacker.identityPath, + deviceToken: attacker.token, + }); + + const rotate = await rpcReq(pairingWs, "device.token.rotate", { + deviceId: attacker.deviceId, + role: "node", + }); + + expect(rotate.ok).toBe(false); + expect(rotate.error?.message).toBe("device token rotation denied"); + + const paired = await getPairedDevice(attacker.deviceId); + expect(paired?.tokens?.node).toBeUndefined(); + expect(paired?.tokens?.operator?.scopes).toEqual(["operator.pairing"]); + } finally { + pairingWs?.close(); + started.ws.close(); + await started.server.close(); + started.envSnapshot.restore(); + } + }); }); diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts index 63b04886c27..0aced1b8513 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -803,6 +803,52 @@ describe("device pairing tokens", () => { expect(hasEffectivePairedDeviceRole(device, "operator")).toBe(true); }); + test("filters active token roles to the approved pairing role set", async () => { + const now = Date.now(); + const device: PairedDevice = { + deviceId: "device-filtered", + publicKey: "pk-filtered", + role: "operator", + roles: ["operator"], + tokens: { + node: { + token: "forged-node-token", + role: "node", + scopes: [], + createdAtMs: now, + }, + operator: { + token: "real-operator-token", + role: "operator", + scopes: ["operator.read"], + createdAtMs: now, + }, + }, + createdAtMs: now, + approvedAtMs: now, + }; + + expect(listEffectivePairedDeviceRoles(device)).toEqual(["operator"]); + expect(hasEffectivePairedDeviceRole(device, "node")).toBe(false); + }); + + test("rejects rotating a token for a role that was never approved", async () => { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); + await setupPairedOperatorDevice(baseDir, ["operator.pairing"]); + + await expect( + rotateDeviceToken({ + deviceId: "device-1", + role: "node", + baseDir, + }), + ).resolves.toEqual({ ok: false, reason: "unknown-device-or-role" }); + + const paired = await getPairedDevice("device-1", baseDir); + expect(paired?.tokens?.node).toBeUndefined(); + expect(paired && listEffectivePairedDeviceRoles(paired)).toEqual(["operator"]); + }); + test("removes paired devices by device id", async () => { const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); await setupPairedOperatorDevice(baseDir, ["operator.read"]); diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index 6ed5a1830d7..69868afbd57 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -168,12 +168,19 @@ function listActiveTokenRoles( ); } +export function listApprovedPairedDeviceRoles( + device: Pick, +): string[] { + return mergeRoles(device.roles, device.role) ?? []; +} + export function listEffectivePairedDeviceRoles( device: Pick, ): string[] { const activeTokenRoles = listActiveTokenRoles(device.tokens); if (activeTokenRoles && activeTokenRoles.length > 0) { - return activeTokenRoles; + const approvedRoles = new Set(listApprovedPairedDeviceRoles(device)); + return activeTokenRoles.filter((role) => approvedRoles.has(role)); } // Only fall back to legacy role fields when the tokens map is absent // or has no entries at all (empty object from a fresh pairing record). @@ -182,7 +189,7 @@ export function listEffectivePairedDeviceRoles( if (device.tokens && Object.keys(device.tokens).length > 0) { return []; } - return mergeRoles(device.roles, device.role) ?? []; + return listApprovedPairedDeviceRoles(device); } export function hasEffectivePairedDeviceRole( @@ -873,6 +880,9 @@ function resolveDeviceTokenUpdateContext(params: { if (!role) { return null; } + if (!listApprovedPairedDeviceRoles(device).includes(role)) { + return null; + } const tokens = cloneDeviceTokens(device); const existing = tokens[role]; return { device, role, tokens, existing };