Skip to content

Commit 205d8d4

Browse files
steipetesteipete
committed
fix(pairing): recover malformed pairing state files
1 parent aa1834a commit 205d8d4

5 files changed

Lines changed: 89 additions & 9 deletions

File tree

‎src/infra/device-pairing.test.ts‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { readFile, writeFile } from "node:fs/promises";
1+
import { mkdir, readFile, writeFile } from "node:fs/promises";
22
import { afterAll, beforeAll, describe, expect, test } from "vitest";
33
import { PAIRING_SETUP_BOOTSTRAP_PROFILE } from "../shared/device-bootstrap-profile.js";
44
import { createSuiteTempRootTracker } from "../test-helpers/temp-dir.js";
@@ -161,6 +161,42 @@ describe("device pairing tokens", () => {
161161
expect(second.request.requestId).toBe(first.request.requestId);
162162
});
163163

164+
test("recovers when pairing state files were written as arrays", async () => {
165+
const baseDir = await makeDevicePairingDir();
166+
const paths = resolvePairingPaths(baseDir, "devices");
167+
await mkdir(paths.dir, { recursive: true });
168+
await writeFile(paths.pendingPath, "[]", "utf8");
169+
await writeFile(paths.pairedPath, "[]", "utf8");
170+
171+
const pending = await requestDevicePairing(
172+
{
173+
deviceId: "device-array-state",
174+
publicKey: "public-key-array-state",
175+
role: "operator",
176+
scopes: ["operator.read"],
177+
},
178+
baseDir,
179+
);
180+
const approved = await approveDevicePairing(
181+
pending.request.requestId,
182+
{ callerScopes: ["operator.read"] },
183+
baseDir,
184+
);
185+
186+
expect(approved).toEqual(
187+
expect.objectContaining({
188+
status: "approved",
189+
device: expect.objectContaining({ deviceId: "device-array-state" }),
190+
}),
191+
);
192+
expect(Array.isArray(JSON.parse(await readFile(paths.pendingPath, "utf8")))).toBe(false);
193+
expect(JSON.parse(await readFile(paths.pairedPath, "utf8"))).toEqual(
194+
expect.objectContaining({
195+
"device-array-state": expect.objectContaining({ deviceId: "device-array-state" }),
196+
}),
197+
);
198+
});
199+
164200
test("re-requesting with identical params preserves the original ts to prevent queue-jumping", async () => {
165201
// Regression: refreshPendingDevicePairingRequest must not bump ts to Date.now().
166202
// An attacker who reconnects with the same key/role/scopes could otherwise

‎src/infra/device-pairing.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
pruneExpiredPending,
1515
readDurableJsonFile,
1616
reconcilePendingPairingRequests,
17+
coercePairingStateRecord,
1718
resolvePairingPaths,
1819
writeJsonAtomic,
1920
} from "./pairing-files.js";
@@ -152,12 +153,12 @@ export function formatDevicePairingForbiddenMessage(result: DevicePairingForbidd
152153
async function loadState(baseDir?: string): Promise<DevicePairingStateFile> {
153154
const { pendingPath, pairedPath } = resolvePairingPaths(baseDir, "devices");
154155
const [pending, paired] = await Promise.all([
155-
readDurableJsonFile<Record<string, DevicePairingPendingRequest>>(pendingPath),
156-
readDurableJsonFile<Record<string, PairedDevice>>(pairedPath),
156+
readDurableJsonFile<unknown>(pendingPath),
157+
readDurableJsonFile<unknown>(pairedPath),
157158
]);
158159
const state: DevicePairingStateFile = {
159-
pendingById: pending ?? {},
160-
pairedByDeviceId: paired ?? {},
160+
pendingById: coercePairingStateRecord<DevicePairingPendingRequest>(pending),
161+
pairedByDeviceId: coercePairingStateRecord<PairedDevice>(paired),
161162
};
162163
pruneExpiredPending(state.pendingById, Date.now(), PENDING_TTL_MS);
163164
return state;

‎src/infra/node-pairing.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,41 @@ describe("node pairing tokens", () => {
131131
});
132132
});
133133

134+
test("recovers when pairing state files were written as arrays", async () => {
135+
await withNodePairingDir(async (baseDir) => {
136+
const paths = resolvePairingPaths(baseDir, "nodes");
137+
await fs.mkdir(paths.dir, { recursive: true });
138+
await fs.writeFile(paths.pendingPath, "[]", "utf8");
139+
await fs.writeFile(paths.pairedPath, "[]", "utf8");
140+
141+
const pending = await requestNodePairing(
142+
{
143+
nodeId: "node-array-state",
144+
platform: "darwin",
145+
commands: ["system.run"],
146+
},
147+
baseDir,
148+
);
149+
const approved = await approveNodePairing(
150+
pending.request.requestId,
151+
{ callerScopes: ["operator.pairing", "operator.admin"] },
152+
baseDir,
153+
);
154+
155+
expect(approved).toEqual(
156+
expect.objectContaining({
157+
node: expect.objectContaining({ nodeId: "node-array-state" }),
158+
}),
159+
);
160+
expect(Array.isArray(JSON.parse(await fs.readFile(paths.pendingPath, "utf8")))).toBe(false);
161+
expect(JSON.parse(await fs.readFile(paths.pairedPath, "utf8"))).toEqual(
162+
expect.objectContaining({
163+
"node-array-state": expect.objectContaining({ nodeId: "node-array-state" }),
164+
}),
165+
);
166+
});
167+
});
168+
134169
test("generates base64url node tokens and rejects mismatches", async () => {
135170
await withNodePairingDir(async (baseDir) => {
136171
const token = await setupPairedNode(baseDir);

‎src/infra/node-pairing.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
pruneExpiredPending,
88
readDurableJsonFile,
99
reconcilePendingPairingRequests,
10+
coercePairingStateRecord,
1011
resolvePairingPaths,
1112
writeJsonAtomic,
1213
} from "./pairing-files.js";
@@ -136,12 +137,12 @@ type ApproveNodePairingResult = ApprovedNodePairingResult | ForbiddenNodePairing
136137
async function loadState(baseDir?: string): Promise<NodePairingStateFile> {
137138
const { pendingPath, pairedPath } = resolvePairingPaths(baseDir, "nodes");
138139
const [pending, paired] = await Promise.all([
139-
readDurableJsonFile<Record<string, NodePairingPendingRequest>>(pendingPath),
140-
readDurableJsonFile<Record<string, NodePairingPairedNode>>(pairedPath),
140+
readDurableJsonFile<unknown>(pendingPath),
141+
readDurableJsonFile<unknown>(pairedPath),
141142
]);
142143
const state: NodePairingStateFile = {
143-
pendingById: pending ?? {},
144-
pairedByNodeId: paired ?? {},
144+
pendingById: coercePairingStateRecord<NodePairingPendingRequest>(pending),
145+
pairedByNodeId: coercePairingStateRecord<NodePairingPairedNode>(paired),
145146
};
146147
pruneExpiredPending(state.pendingById, Date.now(), PENDING_TTL_MS);
147148
return state;

‎src/infra/pairing-files.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@ export function resolvePairingPaths(baseDir: string | undefined, subdir: string)
1818
};
1919
}
2020

21+
export function coercePairingStateRecord<T>(value: unknown): Record<string, T> {
22+
if (!value || typeof value !== "object" || Array.isArray(value)) {
23+
return {};
24+
}
25+
return value as Record<string, T>;
26+
}
27+
2128
export function pruneExpiredPending<T extends { ts: number }>(
2229
pendingById: Record<string, T>,
2330
nowMs: number,

0 commit comments

Comments
 (0)