From 7c0a849ed7c48c199b508721a881ffefafcd46fb Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 15 Mar 2026 09:01:53 -0700 Subject: [PATCH] fix: harden device token rotation denial paths --- CHANGELOG.md | 1 + src/gateway/server-methods/devices.ts | 54 +++++++++++++++++-- .../server.auth.compat-baseline.test.ts | 6 ++- .../server.device-token-rotate-authz.test.ts | 45 ++++++++++++++-- src/infra/device-pairing.test.ts | 24 ++++++--- src/infra/device-pairing.ts | 20 +++++-- 6 files changed, 129 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1fd84a09ba..95a68bc92cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Docs: https://docs.openclaw.ai - WhatsApp/reconnect: restore the append recency filter in the extension inbox monitor and handle protobuf `Long` timestamps correctly, so fresh post-reconnect append messages are processed while stale history sync stays suppressed. (#42588) thanks @MonkeyLeeT. - WhatsApp/login: wait for pending creds writes before reopening after Baileys `515` pairing restarts in both QR login and `channels login` flows, and keep the restart coverage pinned to the real wrapped error shape plus per-account creds queues. (#27910) Thanks @asyncjason. - Agents/openai-compatible tool calls: deduplicate repeated tool call ids across live assistant messages and replayed history so OpenAI-compatible backends no longer reject duplicate `tool_call_id` values with HTTP 400. (#40996) Thanks @xaeon2026. +- Security/device pairing: harden `device.token.rotate` deny handling by keeping public failures generic while logging internal deny reasons and preserving approved-baseline enforcement. (`GHSA-7jrw-x62h-64p8`) ### Fixes diff --git a/src/gateway/server-methods/devices.ts b/src/gateway/server-methods/devices.ts index a068b2dfac5..4becd52edcc 100644 --- a/src/gateway/server-methods/devices.ts +++ b/src/gateway/server-methods/devices.ts @@ -4,6 +4,7 @@ import { listDevicePairing, removePairedDevice, type DeviceAuthToken, + type RotateDeviceTokenDenyReason, rejectDevicePairing, revokeDeviceToken, rotateDeviceToken, @@ -24,6 +25,8 @@ import { } from "../protocol/index.js"; import type { GatewayRequestHandlers } from "./types.js"; +const DEVICE_TOKEN_ROTATION_DENIED_MESSAGE = "device token rotation denied"; + function redactPairedDevice( device: { tokens?: Record } & Record, ) { @@ -53,6 +56,19 @@ function resolveMissingRequestedScope(params: { return null; } +function logDeviceTokenRotationDenied(params: { + log: { warn: (message: string) => void }; + deviceId: string; + role: string; + reason: RotateDeviceTokenDenyReason | "caller-missing-scope" | "unknown-device-or-role"; + scope?: string | null; +}) { + const suffix = params.scope ? ` scope=${params.scope}` : ""; + params.log.warn( + `device token rotation denied device=${params.deviceId} role=${params.role} reason=${params.reason}${suffix}`, + ); +} + export const deviceHandlers: GatewayRequestHandlers = { "device.pair.list": async ({ params, respond }) => { if (!validateDevicePairListParams(params)) { @@ -189,7 +205,17 @@ export const deviceHandlers: GatewayRequestHandlers = { }; const pairedDevice = await getPairedDevice(deviceId); if (!pairedDevice) { - respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId/role")); + 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 : []; @@ -202,18 +228,36 @@ export const deviceHandlers: GatewayRequestHandlers = { callerScopes, }); if (missingScope) { + logDeviceTokenRotationDenied({ + log: context.logGateway, + deviceId, + role, + reason: "caller-missing-scope", + scope: missingScope, + }); respond( false, undefined, - errorShape(ErrorCodes.INVALID_REQUEST, `missing scope: ${missingScope}`), + errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_ROTATION_DENIED_MESSAGE), ); return; } - const entry = await rotateDeviceToken({ deviceId, role, scopes }); - if (!entry) { - respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown deviceId/role")); + const rotated = await rotateDeviceToken({ deviceId, role, scopes }); + if (!rotated.ok) { + logDeviceTokenRotationDenied({ + log: context.logGateway, + deviceId, + role, + reason: rotated.reason, + }); + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, DEVICE_TOKEN_ROTATION_DENIED_MESSAGE), + ); return; } + const entry = rotated.entry; context.logGateway.info( `device token rotated device=${deviceId} role=${entry.role} scopes=${entry.scopes.join(",")}`, ); diff --git a/src/gateway/server.auth.compat-baseline.test.ts b/src/gateway/server.auth.compat-baseline.test.ts index 27fc4abc72d..630e53de84f 100644 --- a/src/gateway/server.auth.compat-baseline.test.ts +++ b/src/gateway/server.auth.compat-baseline.test.ts @@ -174,7 +174,9 @@ describe("gateway auth compatibility baseline", () => { role: "operator", scopes: ["operator.admin"], }); - expect(rotated?.token).toBeTruthy(); + expect(rotated.ok).toBe(true); + const rotatedToken = rotated.ok ? rotated.entry.token : ""; + expect(rotatedToken).toBeTruthy(); const ws = await openWs(port); try { @@ -182,7 +184,7 @@ describe("gateway auth compatibility baseline", () => { skipDefaultAuth: true, client: { ...BACKEND_GATEWAY_CLIENT }, deviceIdentityPath: identityPath, - deviceToken: String(rotated?.token ?? ""), + deviceToken: rotatedToken, scopes: ["operator.admin"], }); expect(res.ok).toBe(true); diff --git a/src/gateway/server.device-token-rotate-authz.test.ts b/src/gateway/server.device-token-rotate-authz.test.ts index 9f3ecdaf719..efb4d5e44b1 100644 --- a/src/gateway/server.device-token-rotate-authz.test.ts +++ b/src/gateway/server.device-token-rotate-authz.test.ts @@ -87,11 +87,13 @@ async function issuePairingScopedTokenForAdminApprovedDevice(name: string): Prom role: "operator", scopes: ["operator.pairing"], }); - expect(rotated?.token).toBeTruthy(); + expect(rotated.ok).toBe(true); + const pairingToken = rotated.ok ? rotated.entry.token : ""; + expect(pairingToken).toBeTruthy(); return { deviceId: paired.identity.deviceId, identityPath: paired.identityPath, - pairingToken: String(rotated?.token ?? ""), + pairingToken, }; } @@ -221,7 +223,7 @@ describe("gateway device.token.rotate caller scope guard", () => { scopes: ["operator.admin"], }); expect(rotate.ok).toBe(false); - expect(rotate.error?.message).toBe("missing scope: operator.admin"); + expect(rotate.error?.message).toBe("device token rotation denied"); const paired = await getPairedDevice(attacker.deviceId); expect(paired?.tokens?.operator?.scopes).toEqual(["operator.pairing"]); @@ -266,7 +268,7 @@ describe("gateway device.token.rotate caller scope guard", () => { }); expect(rotate.ok).toBe(false); - expect(rotate.error?.message).toBe("missing scope: operator.admin"); + expect(rotate.error?.message).toBe("device token rotation denied"); await waitForMacrotasks(); expect(sawInvoke).toBe(false); @@ -281,4 +283,39 @@ describe("gateway device.token.rotate caller scope guard", () => { started.envSnapshot.restore(); } }); + + test("returns the same public deny for unknown devices and caller scope failures", async () => { + const started = await startServerWithClient("secret"); + const attacker = await issuePairingScopedTokenForAdminApprovedDevice("rotate-deny-shape"); + + let pairingWs: WebSocket | undefined; + try { + pairingWs = await connectPairingScopedOperator({ + port: started.port, + identityPath: attacker.identityPath, + deviceToken: attacker.pairingToken, + }); + + const missingScope = await rpcReq(pairingWs, "device.token.rotate", { + deviceId: attacker.deviceId, + role: "operator", + scopes: ["operator.admin"], + }); + const unknownDevice = await rpcReq(pairingWs, "device.token.rotate", { + deviceId: "missing-device", + role: "operator", + scopes: ["operator.pairing"], + }); + + expect(missingScope.ok).toBe(false); + expect(unknownDevice.ok).toBe(false); + expect(missingScope.error?.message).toBe("device token rotation denied"); + expect(unknownDevice.error?.message).toBe("device token rotation denied"); + } 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 ddf0826d048..4deb04a8912 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -13,6 +13,7 @@ import { rotateDeviceToken, verifyDeviceToken, type PairedDevice, + type RotateDeviceTokenResult, } from "./device-pairing.js"; import { resolvePairingPaths } from "./pairing-files.js"; @@ -55,6 +56,14 @@ function requireToken(token: string | undefined): string { return token; } +function requireRotatedEntry(result: RotateDeviceTokenResult) { + expect(result.ok).toBe(true); + if (!result.ok) { + throw new Error(`expected rotated token entry, got ${result.reason}`); + } + return result.entry; +} + async function overwritePairedOperatorTokenScopes(baseDir: string, scopes: string[]) { const { pairedPath } = resolvePairingPaths(baseDir, "devices"); const pairedByDeviceId = JSON.parse(await readFile(pairedPath, "utf8")) as Record< @@ -204,22 +213,24 @@ describe("device pairing tokens", () => { const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); await setupPairedOperatorDevice(baseDir, ["operator.admin"]); - await rotateDeviceToken({ + const downscoped = await rotateDeviceToken({ deviceId: "device-1", role: "operator", scopes: ["operator.read"], baseDir, }); + expect(downscoped.ok).toBe(true); let paired = await getPairedDevice("device-1", baseDir); expect(paired?.tokens?.operator?.scopes).toEqual(["operator.read"]); expect(paired?.scopes).toEqual(["operator.admin"]); expect(paired?.approvedScopes).toEqual(["operator.admin"]); - await rotateDeviceToken({ + const reused = await rotateDeviceToken({ deviceId: "device-1", role: "operator", baseDir, }); + expect(reused.ok).toBe(true); paired = await getPairedDevice("device-1", baseDir); expect(paired?.tokens?.operator?.scopes).toEqual(["operator.read"]); }); @@ -255,7 +266,7 @@ describe("device pairing tokens", () => { scopes: ["operator.admin"], baseDir, }); - expect(rotated).toBeNull(); + expect(rotated).toEqual({ ok: false, reason: "scope-outside-approved-baseline" }); const after = await getPairedDevice("device-1", baseDir); expect(after?.tokens?.operator?.token).toEqual(before?.tokens?.operator?.token); @@ -357,12 +368,13 @@ describe("device pairing tokens", () => { scopes: ["operator.talk.secrets"], baseDir, }); - expect(rotated?.scopes).toEqual(["operator.talk.secrets"]); + const entry = requireRotatedEntry(rotated); + expect(entry.scopes).toEqual(["operator.talk.secrets"]); await expect( verifyOperatorToken({ baseDir, - token: requireToken(rotated?.token), + token: requireToken(entry.token), scopes: ["operator.talk.secrets"], }), ).resolves.toEqual({ ok: true }); @@ -395,7 +407,7 @@ describe("device pairing tokens", () => { scopes: ["operator.admin"], baseDir, }), - ).resolves.toBeNull(); + ).resolves.toEqual({ ok: false, reason: "missing-approved-scope-baseline" }); }); test("treats multibyte same-length token input as mismatch without throwing", async () => { diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index 5bd2909a56e..d16cd06f0cc 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -48,6 +48,15 @@ export type DeviceAuthTokenSummary = { lastUsedAtMs?: number; }; +export type RotateDeviceTokenDenyReason = + | "unknown-device-or-role" + | "missing-approved-scope-baseline" + | "scope-outside-approved-baseline"; + +export type RotateDeviceTokenResult = + | { ok: true; entry: DeviceAuthToken } + | { ok: false; reason: RotateDeviceTokenDenyReason }; + export type PairedDevice = { deviceId: string; publicKey: string; @@ -587,7 +596,7 @@ export async function rotateDeviceToken(params: { role: string; scopes?: string[]; baseDir?: string; -}): Promise { +}): Promise { return await withLock(async () => { const state = await loadState(params.baseDir); const context = resolveDeviceTokenUpdateContext({ @@ -596,13 +605,16 @@ export async function rotateDeviceToken(params: { role: params.role, }); if (!context) { - return null; + return { ok: false, reason: "unknown-device-or-role" }; } const { device, role, tokens, existing } = context; const requestedScopes = normalizeDeviceAuthScopes( params.scopes ?? existing?.scopes ?? device.scopes, ); const approvedScopes = resolveApprovedDeviceScopeBaseline(device); + if (!approvedScopes) { + return { ok: false, reason: "missing-approved-scope-baseline" }; + } if ( !scopesWithinApprovedDeviceBaseline({ role, @@ -610,7 +622,7 @@ export async function rotateDeviceToken(params: { approvedScopes, }) ) { - return null; + return { ok: false, reason: "scope-outside-approved-baseline" }; } const now = Date.now(); const next = buildDeviceAuthToken({ @@ -624,7 +636,7 @@ export async function rotateDeviceToken(params: { device.tokens = tokens; state.pairedByDeviceId[device.deviceId] = device; await persistState(state, params.baseDir); - return next; + return { ok: true, entry: next }; }); }