From 205d8d4994c5ff7ccbea710eb2047c657aeac1e2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 28 Apr 2026 11:37:07 +0100 Subject: [PATCH] fix(pairing): recover malformed pairing state files --- src/infra/device-pairing.test.ts | 38 +++++++++++++++++++++++++++++++- src/infra/device-pairing.ts | 9 ++++---- src/infra/node-pairing.test.ts | 35 +++++++++++++++++++++++++++++ src/infra/node-pairing.ts | 9 ++++---- src/infra/pairing-files.ts | 7 ++++++ 5 files changed, 89 insertions(+), 9 deletions(-) diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts index 166424cb08b..b0bbac3c0ab 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -1,4 +1,4 @@ -import { readFile, writeFile } from "node:fs/promises"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; import { afterAll, beforeAll, describe, expect, test } from "vitest"; import { PAIRING_SETUP_BOOTSTRAP_PROFILE } from "../shared/device-bootstrap-profile.js"; import { createSuiteTempRootTracker } from "../test-helpers/temp-dir.js"; @@ -161,6 +161,42 @@ describe("device pairing tokens", () => { expect(second.request.requestId).toBe(first.request.requestId); }); + test("recovers when pairing state files were written as arrays", async () => { + const baseDir = await makeDevicePairingDir(); + const paths = resolvePairingPaths(baseDir, "devices"); + await mkdir(paths.dir, { recursive: true }); + await writeFile(paths.pendingPath, "[]", "utf8"); + await writeFile(paths.pairedPath, "[]", "utf8"); + + const pending = await requestDevicePairing( + { + deviceId: "device-array-state", + publicKey: "public-key-array-state", + role: "operator", + scopes: ["operator.read"], + }, + baseDir, + ); + const approved = await approveDevicePairing( + pending.request.requestId, + { callerScopes: ["operator.read"] }, + baseDir, + ); + + expect(approved).toEqual( + expect.objectContaining({ + status: "approved", + device: expect.objectContaining({ deviceId: "device-array-state" }), + }), + ); + expect(Array.isArray(JSON.parse(await readFile(paths.pendingPath, "utf8")))).toBe(false); + expect(JSON.parse(await readFile(paths.pairedPath, "utf8"))).toEqual( + expect.objectContaining({ + "device-array-state": expect.objectContaining({ deviceId: "device-array-state" }), + }), + ); + }); + test("re-requesting with identical params preserves the original ts to prevent queue-jumping", async () => { // Regression: refreshPendingDevicePairingRequest must not bump ts to Date.now(). // An attacker who reconnects with the same key/role/scopes could otherwise diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index bcc4ddb4560..6e283a97c90 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -14,6 +14,7 @@ import { pruneExpiredPending, readDurableJsonFile, reconcilePendingPairingRequests, + coercePairingStateRecord, resolvePairingPaths, writeJsonAtomic, } from "./pairing-files.js"; @@ -152,12 +153,12 @@ export function formatDevicePairingForbiddenMessage(result: DevicePairingForbidd async function loadState(baseDir?: string): Promise { const { pendingPath, pairedPath } = resolvePairingPaths(baseDir, "devices"); const [pending, paired] = await Promise.all([ - readDurableJsonFile>(pendingPath), - readDurableJsonFile>(pairedPath), + readDurableJsonFile(pendingPath), + readDurableJsonFile(pairedPath), ]); const state: DevicePairingStateFile = { - pendingById: pending ?? {}, - pairedByDeviceId: paired ?? {}, + pendingById: coercePairingStateRecord(pending), + pairedByDeviceId: coercePairingStateRecord(paired), }; pruneExpiredPending(state.pendingById, Date.now(), PENDING_TTL_MS); return state; diff --git a/src/infra/node-pairing.test.ts b/src/infra/node-pairing.test.ts index 15635e0650b..eb074a85297 100644 --- a/src/infra/node-pairing.test.ts +++ b/src/infra/node-pairing.test.ts @@ -131,6 +131,41 @@ describe("node pairing tokens", () => { }); }); + test("recovers when pairing state files were written as arrays", async () => { + await withNodePairingDir(async (baseDir) => { + const paths = resolvePairingPaths(baseDir, "nodes"); + await fs.mkdir(paths.dir, { recursive: true }); + await fs.writeFile(paths.pendingPath, "[]", "utf8"); + await fs.writeFile(paths.pairedPath, "[]", "utf8"); + + const pending = await requestNodePairing( + { + nodeId: "node-array-state", + platform: "darwin", + commands: ["system.run"], + }, + baseDir, + ); + const approved = await approveNodePairing( + pending.request.requestId, + { callerScopes: ["operator.pairing", "operator.admin"] }, + baseDir, + ); + + expect(approved).toEqual( + expect.objectContaining({ + node: expect.objectContaining({ nodeId: "node-array-state" }), + }), + ); + expect(Array.isArray(JSON.parse(await fs.readFile(paths.pendingPath, "utf8")))).toBe(false); + expect(JSON.parse(await fs.readFile(paths.pairedPath, "utf8"))).toEqual( + expect.objectContaining({ + "node-array-state": expect.objectContaining({ nodeId: "node-array-state" }), + }), + ); + }); + }); + test("generates base64url node tokens and rejects mismatches", async () => { await withNodePairingDir(async (baseDir) => { const token = await setupPairedNode(baseDir); diff --git a/src/infra/node-pairing.ts b/src/infra/node-pairing.ts index 447248f3ed5..df076dbf257 100644 --- a/src/infra/node-pairing.ts +++ b/src/infra/node-pairing.ts @@ -7,6 +7,7 @@ import { pruneExpiredPending, readDurableJsonFile, reconcilePendingPairingRequests, + coercePairingStateRecord, resolvePairingPaths, writeJsonAtomic, } from "./pairing-files.js"; @@ -136,12 +137,12 @@ type ApproveNodePairingResult = ApprovedNodePairingResult | ForbiddenNodePairing async function loadState(baseDir?: string): Promise { const { pendingPath, pairedPath } = resolvePairingPaths(baseDir, "nodes"); const [pending, paired] = await Promise.all([ - readDurableJsonFile>(pendingPath), - readDurableJsonFile>(pairedPath), + readDurableJsonFile(pendingPath), + readDurableJsonFile(pairedPath), ]); const state: NodePairingStateFile = { - pendingById: pending ?? {}, - pairedByNodeId: paired ?? {}, + pendingById: coercePairingStateRecord(pending), + pairedByNodeId: coercePairingStateRecord(paired), }; pruneExpiredPending(state.pendingById, Date.now(), PENDING_TTL_MS); return state; diff --git a/src/infra/pairing-files.ts b/src/infra/pairing-files.ts index a7f38b43517..bed3f735701 100644 --- a/src/infra/pairing-files.ts +++ b/src/infra/pairing-files.ts @@ -18,6 +18,13 @@ export function resolvePairingPaths(baseDir: string | undefined, subdir: string) }; } +export function coercePairingStateRecord(value: unknown): Record { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return {}; + } + return value as Record; +} + export function pruneExpiredPending( pendingById: Record, nowMs: number,