fix: device pairing floods stacked approval alerts for a retrying device and stale approvals no-op (#100976)

* fix(gateway): stop device pairing approval alert floods from retrying devices

A device retrying with a broken token minted a new pending requestId (and
approval alert broadcast) every 5-minute TTL window because refresh preserved
ts and expiry keyed on ts. Pending requests now stay alive via an internal
refreshedAtMs keepalive (ts still owns ordering/--latest), superseded device
requests broadcast device.pair.resolved like node pairing already did, and the
Mac app keeps one alert per device, closes superseded visible alerts, and
resyncs its queue when approving a stale request.

Closes #100974

* chore: satisfy swiftformat and refresh native i18n inventory for pairing prompter changes
This commit is contained in:
Peter Steinberger 2026-07-06 17:20:06 +01:00 committed by GitHub
parent 5e22787ba1
commit 0a488bd30b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 200 additions and 31 deletions

View file

@ -16299,7 +16299,7 @@
},
{
"kind": "conditional-branch",
"line": 239,
"line": 263,
"path": "apps/macos/Sources/OpenClaw/DevicePairingApprovalPrompter.swift",
"source": "New Mac wants to connect",
"surface": "apple",
@ -16307,7 +16307,7 @@
},
{
"kind": "conditional-branch",
"line": 239,
"line": 263,
"path": "apps/macos/Sources/OpenClaw/DevicePairingApprovalPrompter.swift",
"source": "New device wants to connect",
"surface": "apple",
@ -16315,7 +16315,7 @@
},
{
"kind": "conditional-branch",
"line": 243,
"line": 267,
"path": "apps/macos/Sources/OpenClaw/DevicePairingApprovalPrompter.swift",
"source": "this Mac app",
"surface": "apple",
@ -16323,7 +16323,7 @@
},
{
"kind": "conditional-branch",
"line": 243,
"line": 267,
"path": "apps/macos/Sources/OpenClaw/DevicePairingApprovalPrompter.swift",
"source": "this device",
"surface": "apple",
@ -16331,7 +16331,7 @@
},
{
"kind": "conditional-branch",
"line": 248,
"line": 272,
"path": "apps/macos/Sources/OpenClaw/DevicePairingApprovalPrompter.swift",
"source": "Approve Device",
"surface": "apple",
@ -16339,7 +16339,7 @@
},
{
"kind": "conditional-branch",
"line": 248,
"line": 272,
"path": "apps/macos/Sources/OpenClaw/DevicePairingApprovalPrompter.swift",
"source": "Approve Mac",
"surface": "apple",
@ -16347,7 +16347,7 @@
},
{
"kind": "conditional-branch",
"line": 284,
"line": 308,
"path": "apps/macos/Sources/OpenClaw/DevicePairingApprovalPrompter.swift",
"source": "New device",
"surface": "apple",
@ -16355,7 +16355,7 @@
},
{
"kind": "conditional-branch",
"line": 284,
"line": 308,
"path": "apps/macos/Sources/OpenClaw/DevicePairingApprovalPrompter.swift",
"source": "OpenClaw Mac app",
"surface": "apple",
@ -17995,7 +17995,7 @@
},
{
"kind": "conditional-branch",
"line": 395,
"line": 399,
"path": "apps/macos/Sources/OpenClaw/NodePairingApprovalPrompter.swift",
"source": "Node pairing approved",
"surface": "apple",
@ -18003,7 +18003,7 @@
},
{
"kind": "conditional-branch",
"line": 395,
"line": 399,
"path": "apps/macos/Sources/OpenClaw/NodePairingApprovalPrompter.swift",
"source": "Node pairing rejected",
"surface": "apple",

View file

@ -146,7 +146,11 @@ final class DevicePairingApprovalPrompter {
switch response {
case .alertFirstButtonReturn:
_ = await self.approve(requestId: request.requestId)
if await !(self.approve(requestId: request.requestId)) {
// Stale request (expired or superseded on the gateway): re-sync the
// queue with gateway truth so accumulated stale alerts collapse at once.
await self.loadPendingRequestsFromGateway()
}
case .alertSecondButtonReturn:
shouldRemove = false
if let idx = self.queue.firstIndex(of: request) {
@ -211,9 +215,29 @@ final class DevicePairingApprovalPrompter {
}
}
/// The gateway keeps at most one live pending request per device, so a new
/// requestId for the same device supersedes anything still queued for it.
/// Without this, missed/dropped resolve pushes pile up as alerts whose
/// approval can no longer succeed. Returns nil when the request is already queued.
static func coalescedQueue(_ queue: [PendingRequest], adding req: PendingRequest) -> [PendingRequest]? {
guard !queue.contains(where: { $0.requestId == req.requestId }) else { return nil }
return queue.filter { $0.deviceId != req.deviceId } + [req]
}
private func enqueue(_ req: PendingRequest) {
guard !self.queue.contains(req) else { return }
self.queue.append(req)
guard let next = Self.coalescedQueue(self.queue, adding: req) else { return }
let supersededActiveId = self.alertState.activeRequestId.flatMap { activeId in
self.queue.contains(where: {
$0.requestId == activeId && $0.deviceId == req.deviceId
}) ? activeId : nil
}
self.queue = next
if let supersededActiveId {
// The visible alert is for a superseded requestId; close it so the
// fresh request presents instead of an approve that would no-op.
self.resolvedByRequestId.insert(supersededActiveId)
self.endActiveAlert()
}
self.updatePendingCounts()
self.presentNextIfNeeded()
}

View file

@ -265,7 +265,11 @@ final class NodePairingApprovalPrompter {
}
private func enqueue(_ req: PendingRequest) {
if self.queue.contains(req) { return }
if self.queue.contains(where: { $0.requestId == req.requestId }) { return }
// The gateway keeps at most one live pending request per node; a newer
// request supersedes queued ones so missed resolve pushes cannot stack
// stale alerts. The actively presented request stays owned by its alert.
self.queue.removeAll { $0.nodeId == req.nodeId && $0.requestId != self.alertState.activeRequestId }
self.queue.append(req)
self.updatePendingCounts()
self.presentNextIfNeeded()

View file

@ -91,4 +91,37 @@ struct NodePairingApprovalPrompterTests {
#expect(accessory.frame.width >= 380)
#expect(accessory.frame.height > 80)
}
@Test func `a newer device pairing request supersedes queued requests for the same device`() {
func request(_ requestId: String, deviceId: String, ts: Double) -> DevicePairingApprovalPrompter
.PendingRequest
{
DevicePairingApprovalPrompter.PendingRequest(
requestId: requestId,
deviceId: deviceId,
publicKey: "pub",
displayName: nil,
platform: "MacIntel",
clientId: nil,
clientMode: nil,
role: "node",
scopes: nil,
remoteIp: nil,
silent: nil,
isRepair: true,
ts: ts)
}
let stale1 = request("req-1", deviceId: "device-a", ts: 1)
let stale2 = request("req-2", deviceId: "device-a", ts: 2)
let other = request("req-3", deviceId: "device-b", ts: 3)
let fresh = request("req-4", deviceId: "device-a", ts: 4)
// Stale requests for the same device collapse; other devices are untouched.
let coalesced = DevicePairingApprovalPrompter.coalescedQueue([stale1, stale2, other], adding: fresh)
#expect(coalesced?.map(\.requestId) == ["req-3", "req-4"])
// Re-delivery of an already queued requestId is a no-op.
#expect(DevicePairingApprovalPrompter.coalescedQueue([stale1, other], adding: stale1) == nil)
}
}

View file

@ -30,7 +30,9 @@ handshake. Only clients that explicitly call `node.pair.*` use this flow.
4. On approval, the Gateway issues a **new token** (tokens rotate on re-pair).
5. The node reconnects using the token and is now paired.
Pending requests expire automatically after **5 minutes**.
Pending requests expire automatically **5 minutes after the node's last
retry** — an actively reconnecting node keeps its one pending request alive
rather than generating a fresh request (and approval prompt) per attempt.
## CLI workflow (headless friendly)

View file

@ -29,7 +29,7 @@ openclaw nodes status
openclaw nodes describe --node <idOrNameOrIp>
```
Pending pairing requests expire after 5 minutes; see [Gateway-owned pairing](/gateway/pairing) for the full request/approve/token lifecycle. If a node retries with changed auth details (role/scopes/public key), the prior pending request is superseded and a new `requestId` is created — re-run `openclaw devices list` before approving.
Pending pairing requests expire 5 minutes after the device's last retry — a device that keeps reconnecting keeps its one pending request (and `requestId`) alive instead of minting a new prompt every few minutes; see [Gateway-owned pairing](/gateway/pairing) for the full request/approve/token lifecycle. If a node retries with changed auth details (role/scopes/public key), the prior pending request is superseded and a new `requestId` is created — clients get a `device.pair.resolved` event for the superseded request, and you should re-run `openclaw devices list` before approving.
- `nodes status` marks a node as **paired** when its device pairing role includes `node`.
- The device pairing record is the durable approved-role contract. Token rotation stays inside that contract; it cannot upgrade a paired node into a role that pairing approval never granted.

View file

@ -1443,6 +1443,21 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar
allowSetupCodeMobileBootstrapPairing,
});
const context = buildRequestContext();
// A replacement request obsoletes older pending requestIds; tell approval
// UIs so they drop the stale prompts instead of stacking alerts forever.
const supersededResolvedAt = Date.now();
for (const superseded of pairing.superseded ?? []) {
context.broadcast(
"device.pair.resolved",
{
requestId: superseded.requestId,
deviceId: superseded.deviceId,
decision: "rejected",
ts: supersededResolvedAt,
},
{ dropIfSlow: true },
);
}
let approved: Awaited<ReturnType<typeof approveDevicePairing>> | undefined;
let resolvedByConcurrentApproval = false;
let recoveryRequestId: string | undefined;

View file

@ -92,6 +92,9 @@ describe("device pairing requestId churn", () => {
expect(approveReconnect.created).toBe(true);
expect(approveReconnect.request.requestId).not.toBe(readRepair.request.requestId);
expect(approveReconnect.superseded).toEqual([
{ requestId: readRepair.request.requestId, deviceId: DEVICE_ID },
]);
const staleApprove = await approveDevicePairing(
readRepair.request.requestId,

View file

@ -293,6 +293,50 @@ describe("device pairing tokens", () => {
expect(second.request.ts).toBe(originalTs);
});
test("re-requests keep one pending request alive past the pending TTL without churning requestIds", async () => {
// Regression: a device retrying all night must not mint a new requestId (and a
// new approval prompt broadcast) every TTL window. Refreshes stamp refreshedAtMs
// as a TTL keepalive while ts stays the creation time for approval ordering.
const baseDir = await makeDevicePairingDir();
const req = {
deviceId: "device-1",
publicKey: "public-key-1",
role: "operator" as const,
scopes: ["operator.read"],
};
const first = await requestDevicePairing(req, baseDir);
const refreshed = await requestDevicePairing(req, baseDir);
expect(refreshed.created).toBe(false);
// Simulate hours of aging since creation while retries kept the keepalive fresh.
const paths = resolvePairingPaths(baseDir, "devices");
const pendingById = JSON.parse(await readFile(paths.pendingPath, "utf8")) as Record<
string,
{ ts: number; refreshedAtMs?: number }
>;
const pending = requireValue(
pendingById[first.request.requestId],
"expected pending pairing request",
);
expect(pending.refreshedAtMs).toBeGreaterThanOrEqual(pending.ts);
const createdTs = Date.now() - 60 * 60 * 1000;
pending.ts = createdTs;
await writeFile(paths.pendingPath, JSON.stringify(pendingById, null, 2));
const third = await requestDevicePairing(req, baseDir);
expect(third.created).toBe(false);
expect(third.request.requestId).toBe(first.request.requestId);
expect(third.request.ts).toBe(createdTs);
// The keepalive is store-internal; it must not leak into protocol payloads.
expect("refreshedAtMs" in third.request).toBe(false);
// A stale keepalive still expires the request.
pending.refreshedAtMs = createdTs;
pendingById[first.request.requestId] = pending;
await writeFile(paths.pendingPath, JSON.stringify(pendingById, null, 2));
expect((await listDevicePairing(baseDir)).pending).toHaveLength(0);
});
test("supersedes pending requests when requested roles/scopes change", async () => {
const baseDir = await makeDevicePairingDir();
const first = await requestDevicePairing(
@ -316,6 +360,9 @@ describe("device pairing tokens", () => {
expect(second.created).toBe(true);
expect(second.request.requestId).not.toBe(first.request.requestId);
expect(second.superseded).toEqual([
{ requestId: first.request.requestId, deviceId: "device-1" },
]);
expect(second.request.role).toBe("operator");
expectArrayIncludesAll(second.request.roles, ["node", "operator"], "request roles");
expectArrayIncludesAll(
@ -1934,5 +1981,4 @@ describe("device pairing tokens", () => {
).rejects.toThrow(/paired\.json/);
await expect(readFile(pairedPath, "utf8")).resolves.toBe("{not-json}");
});
});

View file

@ -43,6 +43,26 @@ export type DevicePairingPendingRequest = {
ts: number;
};
// Internal pending record. refreshedAtMs is a TTL keepalive stamped on refresh so an
// actively retrying device keeps one pending request (and requestId) alive instead of
// minting a new request every TTL window and flooding operator approval UIs. It never
// crosses the protocol boundary, and ordering/--latest still use ts.
type DevicePairingPendingRecord = DevicePairingPendingRequest & { refreshedAtMs?: number };
/** Pending request summary returned when a replacement supersedes older requests. */
export type DevicePairingSupersededRequest = Pick<
DevicePairingPendingRequest,
"requestId" | "deviceId"
>;
/** Result for creating or refreshing a pending device pairing request. */
export type RequestDevicePairingResult = {
status: "pending";
request: DevicePairingPendingRequest;
created: boolean;
superseded?: DevicePairingSupersededRequest[];
};
/** Bearer token issued to one paired device role. */
export type DeviceAuthToken = {
token: string;
@ -155,7 +175,7 @@ export type ApproveDevicePairingResult =
| null;
type DevicePairingStateFile = {
pendingById: Record<string, DevicePairingPendingRequest>;
pendingById: Record<string, DevicePairingPendingRecord>;
pairedByDeviceId: Record<string, PairedDevice>;
};
@ -192,7 +212,7 @@ async function loadState(baseDir?: string): Promise<DevicePairingStateFile> {
readJsonIfExists<unknown>(pairedPath),
]);
const state: DevicePairingStateFile = {
pendingById: coercePairingStateRecord<DevicePairingPendingRequest>(pending),
pendingById: coercePairingStateRecord<DevicePairingPendingRecord>(pending),
pairedByDeviceId: coercePairingStateRecord<PairedDevice>(paired),
};
pruneExpiredPending(state.pendingById, Date.now(), PENDING_TTL_MS);
@ -395,10 +415,10 @@ function incomingApprovalCoveredByExisting(
}
function refreshPendingDevicePairingRequest(
existing: DevicePairingPendingRequest,
existing: DevicePairingPendingRecord,
incoming: Omit<DevicePairingPendingRequest, "requestId" | "ts" | "isRepair">,
isRepair: boolean,
): DevicePairingPendingRequest {
): DevicePairingPendingRecord {
return {
...existing,
publicKey: incoming.publicKey,
@ -415,6 +435,8 @@ function refreshPendingDevicePairingRequest(
// request's queue position. Using Date.now() here would let an attacker silently
// refresh recency and win the implicit --latest approval race.
ts: existing.ts,
// Keepalive for the pending TTL only (see pruneExpiredPending); never affects ordering.
refreshedAtMs: Date.now(),
};
}
@ -427,6 +449,13 @@ function resolveSupersededPendingSilent(params: {
);
}
function toPublicPendingDevicePairingRequest(
pending: DevicePairingPendingRecord,
): DevicePairingPendingRequest {
const { refreshedAtMs: _refreshedAtMs, ...request } = pending;
return request;
}
function buildPendingDevicePairingRequest(params: {
requestId?: string;
deviceId: string;
@ -613,7 +642,9 @@ function scopesWithinApprovedDeviceBaseline(params: {
export async function listDevicePairing(baseDir?: string): Promise<DevicePairingList> {
const state = await loadState(baseDir);
const pending = Object.values(state.pendingById).toSorted((a, b) => b.ts - a.ts);
const pending = Object.values(state.pendingById)
.map(toPublicPendingDevicePairingRequest)
.toSorted((a, b) => b.ts - a.ts);
const paired = Object.values(state.pairedByDeviceId).toSorted(
(a, b) => b.approvedAtMs - a.approvedAtMs,
);
@ -635,18 +666,15 @@ export async function getPendingDevicePairing(
baseDir?: string,
): Promise<DevicePairingPendingRequest | null> {
const state = await loadState(baseDir);
return state.pendingById[requestId] ?? null;
const pending = state.pendingById[requestId];
return pending ? toPublicPendingDevicePairingRequest(pending) : null;
}
/** Create or refresh a pending device pairing request for owner approval. */
export async function requestDevicePairing(
req: Omit<DevicePairingPendingRequest, "requestId" | "ts" | "isRepair">,
baseDir?: string,
): Promise<{
status: "pending";
request: DevicePairingPendingRequest;
created: boolean;
}> {
): Promise<RequestDevicePairingResult> {
return await withLock(async () => {
const state = await loadState(baseDir);
const deviceId = normalizeDeviceId(req.deviceId);
@ -657,7 +685,7 @@ export async function requestDevicePairing(
const pendingForDevice = Object.values(state.pendingById)
.filter((pending) => pending.deviceId === deviceId)
.toSorted((left, right) => right.ts - left.ts);
return await reconcilePendingPairingRequests({
const result = await reconcilePendingPairingRequests({
pendingById: state.pendingById,
existing: pendingForDevice,
incoming: req,
@ -696,6 +724,18 @@ export async function requestDevicePairing(
},
persist: async () => await persistState(state, baseDir, "pending"),
});
// Surface superseded requestIds so callers can broadcast their resolution;
// clients otherwise keep prompting for requests that can no longer be approved.
const superseded = result.created
? pendingForDevice
.filter((pending) => pending.requestId !== result.request.requestId)
.map((pending) => ({ requestId: pending.requestId, deviceId: pending.deviceId }))
: [];
const publicResult = {
...result,
request: toPublicPendingDevicePairingRequest(result.request),
};
return superseded.length > 0 ? { ...publicResult, superseded } : publicResult;
});
}

View file

@ -24,13 +24,15 @@ export function coercePairingStateRecord<T>(value: unknown): Record<string, T> {
}
/** Remove pending requests older than the caller's pairing TTL. */
export function pruneExpiredPending<T extends { ts: number }>(
export function pruneExpiredPending<T extends { ts: number; refreshedAtMs?: number }>(
pendingById: Record<string, T>,
nowMs: number,
ttlMs: number,
) {
for (const [id, req] of Object.entries(pendingById)) {
if (nowMs - req.ts > ttlMs) {
// refreshedAtMs is a TTL keepalive: expiry counts from the device's last
// re-request, while ts stays the creation time for approval ordering.
if (nowMs - (req.refreshedAtMs ?? req.ts) > ttlMs) {
delete pendingById[id];
}
}