mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(ui): reject malformed device-token rotation envelopes
The parser trusted the envelope around the fields it read. A null, scalar, or empty payload carries neither tokenDelivery nor token, so it matched the legacy omission state and produced the reassuring completion dialog after the previous credential had already been invalidated. A blank token did the same, though the result schema bounds token to a non-empty string. Only DeviceTokenRotateResultSchema's shapes are accepted now: the payload must be a record that identifies the grant it rotated - every Gateway answering this method returns deviceId and role, before and after tokenDelivery existed - and token must be either absent or a non-empty string. This has to happen here because the browser Gateway client resolves frame.payload directly, so the registered result schema never runs on the client. The mid-flight-reconnect reveal test asserted on a two-field stub no Gateway sends; it now uses the real response shape.
This commit is contained in:
@@ -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"] });
|
||||
|
||||
+41
-20
@@ -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<string>;
|
||||
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.
|
||||
|
||||
@@ -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<Record<string, unknown>>();
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user