diff --git a/ui/src/lib/nodes/device-token.test.ts b/ui/src/lib/nodes/device-token.test.ts index 8ca33d834dc5..a3f9bd5b6175 100644 --- a/ui/src/lib/nodes/device-token.test.ts +++ b/ui/src/lib/nodes/device-token.test.ts @@ -161,6 +161,13 @@ describe("device token request lifecycle", () => { "a delivery mode this client predates", { tokenDelivery: "out-of-band", token: "rotated-token" }, ], + // `token` is a non-empty string in the result schema, so a blank one is a malformed + // envelope, not the withheld state - accepting it would report the reassuring outcome. + [ + "a withheld result carrying a blank token", + { tokenDelivery: "withheld-cross-device", token: "" }, + ], + ["a blank token with no delivery field", { token: "" }], ])("refuses %s", async (_label, response) => { const state = createState(async () => ({ ...tokenParams, scopes: [], ...response })); @@ -169,6 +176,23 @@ describe("device token request lifecycle", () => { expect(loadDeviceAuthToken(tokenParams)).toBeNull(); }); + // Envelopes that identify no grant are not legacy responses: every Gateway that answers + // this method returns deviceId and role. Treating them as the omission state would turn + // a broken reply into a completion dialog after the previous credential was invalidated. + it.each([ + ["a null payload", null], + ["a scalar payload", "rotated"], + ["an empty object", {}], + ["an array payload", []], + ["a payload naming no role", { deviceId: "00" }], + ])("refuses %s", async (_label, response) => { + const state = createState(async () => response); + + expect(await rotateDeviceToken(state, tokenParams)).toBeNull(); + expect(state.devicesError).toContain("unusable result"); + expect(loadDeviceAuthToken(tokenParams)).toBeNull(); + }); + it("does not clear a current token when a revoke request retires during identity loading", async () => { storeIdentity(); storeDeviceAuthToken({ ...tokenParams, token: "current-token", scopes: ["operator.read"] }); diff --git a/ui/src/lib/nodes/index.ts b/ui/src/lib/nodes/index.ts index c4508379d488..13ea6810c22b 100644 --- a/ui/src/lib/nodes/index.ts +++ b/ui/src/lib/nodes/index.ts @@ -4,6 +4,7 @@ // dialog bridge and would end the action with no outcome and no recorded reason. import { getPublicKeyAsync, signAsync, utils } from "@noble/ed25519"; import { gatewayCredentialScope } from "@openclaw/gateway-client/browser"; +import { isRecord } from "@openclaw/normalization-core/record-coerce"; import { type DeviceAuthEntry, type DeviceAuthStore, @@ -415,28 +416,48 @@ type RotatedDeviceTokenOutcome = | { delivery: "withheld-cross-device" }; /** - * The Gateway pairs `tokenDelivery` with the presence of the secret, so an explicit pair - * that contradicts itself — or a delivery mode this client predates — describes a rotation - * whose outcome is unknown, not one to report as done. Both dialogs would lie about it: - * one claims a credential arrived, the other that the device re-credentials on its own. - * The old token is dead either way, so the operator gets the error and the recovery step. - * Gateways released before `tokenDelivery` omit it, leaving the token as the only signal. + * Parses the raw `device.token.rotate` payload, which reaches this client unvalidated: + * the browser Gateway client resolves `frame.payload` directly, so the registered result + * schema never runs here. Only `DeviceTokenRotateResultSchema`'s shapes are accepted — + * an identified grant, a token that is absent or a non-empty string, and `tokenDelivery` + * paired with the secret. Anything else describes a rotation whose outcome is unknown, + * and both dialogs would lie about it: one claims a credential arrived, the other that + * the device re-credentials on its own. The old token is dead either way, so the operator + * gets the error and the recovery step. Gateways released before `tokenDelivery` omit it + * and leave the token as the only signal, but still identify the grant they rotated. */ -function classifyRotationOutcome( - tokenDelivery: string | undefined, - token: string | undefined, -): RotatedDeviceTokenOutcome { - if (tokenDelivery === undefined) { - return token ? { delivery: "in-band", token } : { delivery: "withheld-cross-device" }; - } - if (tokenDelivery === "in-band" && token) { - return { delivery: "in-band", token }; - } - if (tokenDelivery === "withheld-cross-device" && !token) { - return { delivery: "withheld-cross-device" }; +function classifyRotationOutcome(payload: unknown): RotatedDeviceTokenOutcome { + const result = isRecord(payload) ? payload : undefined; + const identified = + typeof result?.deviceId === "string" && + result.deviceId.length > 0 && + typeof result.role === "string" && + result.role.length > 0; + // An absent token and a present-but-invalid one are different answers: the schema bounds + // `token` to a non-empty string, so `token: ""` is a malformed envelope rather than a + // rotation that withheld the secret. + const rawToken = result?.token; + const token = typeof rawToken === "string" && rawToken.length > 0 ? rawToken : undefined; + const tokenAbsent = rawToken === undefined; + const delivery = result?.tokenDelivery; + if (identified) { + if (delivery === undefined) { + if (token) { + return { delivery: "in-band", token }; + } + if (tokenAbsent) { + return { delivery: "withheld-cross-device" }; + } + } + if (delivery === "in-band" && token) { + return { delivery: "in-band", token }; + } + if (delivery === "withheld-cross-device" && tokenAbsent) { + return { delivery: "withheld-cross-device" }; + } } throw new Error( - `Rotation returned an unusable result (tokenDelivery=${JSON.stringify(tokenDelivery)}, token ${token ? "present" : "absent"}). The previous token no longer works; pair the device again if it does not reconnect.`, + `Rotation returned an unusable result (tokenDelivery=${JSON.stringify(delivery)}, token ${token ? "present" : tokenAbsent ? "absent" : "malformed"}). The previous token no longer works; pair the device again if it does not reconnect.`, ); } @@ -459,7 +480,7 @@ export async function rotateDeviceToken( scopes?: Array; tokenDelivery?: string; }>("device.token.rotate", requestParams); - const outcome = classifyRotationOutcome(res?.tokenDelivery, res?.token); + const outcome = classifyRotationOutcome(res); // A retired epoch stops every state write below, but never the return: the previous // credential is already dead on the server, so discarding this response would leave // the operator locked out with no way to ask for the replacement again. diff --git a/ui/src/pages/devices/devices-page.test.ts b/ui/src/pages/devices/devices-page.test.ts index a0aba4101e28..fa550c2f8a4e 100644 --- a/ui/src/pages/devices/devices-page.test.ts +++ b/ui/src/pages/devices/devices-page.test.ts @@ -453,7 +453,7 @@ describe("DevicesPage gateway lifecycle", () => { it("reveals a rotated token that lands after the request generation moved on", async () => { stubLocalDeviceIdentity(); - const rotated = deferred<{ token: string; tokenDelivery: string }>(); + const rotated = deferred>(); const request = vi.fn(async (method: string) => method === "device.token.rotate" ? rotated.promise : { paired: [], pending: [] }, ); @@ -462,7 +462,13 @@ describe("DevicesPage gateway lifecycle", () => { const pending = page.reportRotationOutcome({ id: "device-1", name: "MacBook Pro" }, "operator"); page.pageState.requestGeneration += 1; - rotated.resolve({ token: ROTATED_TOKEN, tokenDelivery: "in-band" }); + rotated.resolve({ + deviceId: "device-1", + role: "operator", + scopes: [], + token: ROTATED_TOKEN, + tokenDelivery: "in-band", + }); await waitForRenderedModalDialog(document.body); // The rotate already killed the previous credential, so a mid-flight reconnect must