fix(pairing): preserve corrupt pairing stores

steipete · steipete · commit ec56dd311620 · 2026-04-26T02:50:19.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -75,6 +75,9 @@ Docs: https://docs.openclaw.ai
 - Diagnostics/OTEL: treat normal early model stream cleanup as a completed
   model call instead of exporting a misleading `StreamAbandoned` error span.
   Thanks @vincentkoc.
+- Gateway/pairing: stop corrupt or unreadable device/node pairing stores from
+  being treated as empty state, preserving `paired.json` for repair instead of
+  overwriting approved pairings. Fixes #71873. Thanks @iret77.
 - ACP: wait for the configured runtime backend to become healthy before startup
   identity reconciliation, avoiding transient acpx warnings during Gateway boot.
   Fixes #40566.
diff --git a/src/commands/doctor-device-pairing.test.ts b/src/commands/doctor-device-pairing.test.ts
@@ -104,6 +104,34 @@ describe("noteDevicePairingHealth", () => {
     });
   });
 
+  it("warns when local pairing state is corrupt instead of treating it as empty", async () => {
+    await withTempDir("openclaw-doctor-device-pairing-", async (stateDir) => {
+      await withEnvAsync(
+        {
+          OPENCLAW_STATE_DIR: stateDir,
+          OPENCLAW_TEST_FAST: "1",
+        },
+        async () => {
+          const pairedPath = path.join(stateDir, "devices", "paired.json");
+          await fs.mkdir(path.dirname(pairedPath), { recursive: true });
+          await fs.writeFile(pairedPath, "{not-json}", "utf8");
+
+          await noteDevicePairingHealth({
+            cfg: { gateway: { mode: "local" } },
+            healthOk: false,
+          });
+
+          expect(noteMock).toHaveBeenCalledTimes(1);
+          const message = String(noteMock.mock.calls[0]?.[0] ?? "");
+          expect(noteMock.mock.calls[0]?.[1]).toBe("Device pairing");
+          expect(message).toContain("paired.json");
+          expect(message).toContain("refused to treat it as empty");
+          expect(await fs.readFile(pairedPath, "utf8")).toBe("{not-json}");
+        },
+      );
+    });
+  });
+
   it("warns when the local cached device token predates the gateway rotation", async () => {
     await withApprovedOperatorPairing(async ({ stateDir, identity }) => {
       storeDeviceAuthToken({
diff --git a/src/commands/doctor-device-pairing.ts b/src/commands/doctor-device-pairing.ts
@@ -12,6 +12,7 @@ import {
   type DevicePairingPendingRequest,
   type PairedDevice,
 } from "../infra/device-pairing.js";
+import { JsonFileReadError } from "../infra/json-files.js";
 import type { DeviceAuthStore } from "../shared/device-auth.js";
 import { normalizeDeviceAuthScopes } from "../shared/device-auth.js";
 import { roleScopesAllow } from "../shared/operator-scope-compat.js";
@@ -510,11 +511,25 @@ function collectLocalDeviceAuthIssues(snapshot: DoctorPairingSnapshot): string[]
   return lines;
 }
 
+function formatPairingStoreReadIssue(error: JsonFileReadError): string {
+  const problem = error.reason === "parse" ? "contains invalid JSON" : "could not be read";
+  return `- Device pairing store ${error.filePath} ${problem}. OpenClaw refused to treat it as empty to avoid overwriting approved pairings. Fix the JSON or file permissions, or move it aside and re-pair devices.`;
+}
+
 export async function noteDevicePairingHealth(params: {
   cfg: OpenClawConfig;
   healthOk: boolean;
 }): Promise<void> {
-  const snapshot = await loadDoctorPairingSnapshot(params);
+  let snapshot: DoctorPairingSnapshot | null;
+  try {
+    snapshot = await loadDoctorPairingSnapshot(params);
+  } catch (error) {
+    if (error instanceof JsonFileReadError) {
+      note(formatPairingStoreReadIssue(error), "Device pairing");
+      return;
+    }
+    throw error;
+  }
   if (!snapshot) {
     return;
   }
diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts
@@ -1053,6 +1053,26 @@ describe("device pairing tokens", () => {
     await expect(getPairedDevice("device-1", baseDir)).resolves.toBeNull();
   });
 
+  test("refuses to overwrite corrupt paired device state", async () => {
+    const baseDir = await makeDevicePairingDir();
+    const request = await requestDevicePairing(
+      {
+        deviceId: "device-1",
+        publicKey: "public-key-1",
+        role: "node",
+        scopes: [],
+      },
+      baseDir,
+    );
+    const { pairedPath } = resolvePairingPaths(baseDir, "devices");
+    await writeFile(pairedPath, "{not-json}", "utf8");
+
+    await expect(
+      approveDevicePairing(request.request.requestId, { callerScopes: [] }, baseDir),
+    ).rejects.toThrow(/paired\.json/);
+    await expect(readFile(pairedPath, "utf8")).resolves.toBe("{not-json}");
+  });
+
   test("clears paired device state by device id", async () => {
     const baseDir = await makeDevicePairingDir();
     await setupPairedOperatorDevice(baseDir, ["operator.read"]);
diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts
@@ -12,7 +12,7 @@ import {
 import {
   createAsyncLock,
   pruneExpiredPending,
-  readJsonFile,
+  readDurableJsonFile,
   reconcilePendingPairingRequests,
   resolvePairingPaths,
   writeJsonAtomic,
@@ -143,8 +143,8 @@ export function formatDevicePairingForbiddenMessage(result: DevicePairingForbidd
 async function loadState(baseDir?: string): Promise<DevicePairingStateFile> {
   const { pendingPath, pairedPath } = resolvePairingPaths(baseDir, "devices");
   const [pending, paired] = await Promise.all([
-    readJsonFile<Record<string, DevicePairingPendingRequest>>(pendingPath),
-    readJsonFile<Record<string, PairedDevice>>(pairedPath),
+    readDurableJsonFile<Record<string, DevicePairingPendingRequest>>(pendingPath),
+    readDurableJsonFile<Record<string, PairedDevice>>(pairedPath),
   ]);
   const state: DevicePairingStateFile = {
     pendingById: pending ?? {},
diff --git a/src/infra/json-files.test.ts b/src/infra/json-files.test.ts
@@ -3,7 +3,14 @@ import path from "node:path";
 import { setTimeout as sleep } from "node:timers/promises";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { withTempDir } from "../test-helpers/temp-dir.js";
-import { createAsyncLock, readJsonFile, writeJsonAtomic, writeTextAtomic } from "./json-files.js";
+import {
+  JsonFileReadError,
+  createAsyncLock,
+  readDurableJsonFile,
+  readJsonFile,
+  writeJsonAtomic,
+  writeTextAtomic,
+} from "./json-files.js";
 
 const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform");
 
@@ -45,6 +52,23 @@ describe("json file helpers", () => {
     });
   });
 
+  it("reads durable json strictly while allowing missing files", async () => {
+    await withTempDir({ prefix: "openclaw-json-files-" }, async (base) => {
+      const validPath = path.join(base, "valid.json");
+      const invalidPath = path.join(base, "invalid.json");
+      const missingPath = path.join(base, "missing.json");
+      await fs.writeFile(validPath, '{"ok":true}', "utf8");
+      await fs.writeFile(invalidPath, "{not-json}", "utf8");
+
+      await expect(readDurableJsonFile(validPath)).resolves.toEqual({ ok: true });
+      await expect(readDurableJsonFile(missingPath)).resolves.toBeNull();
+      await expect(readDurableJsonFile(invalidPath)).rejects.toMatchObject({
+        filePath: invalidPath,
+        reason: "parse",
+      } satisfies Partial<JsonFileReadError>);
+    });
+  });
+
   it("writes json atomically with pretty formatting and optional trailing newline", async () => {
     await withTempDir({ prefix: "openclaw-json-files-" }, async (base) => {
       const filePath = path.join(base, "nested", "config.json");
diff --git a/src/infra/json-files.ts b/src/infra/json-files.ts
@@ -7,6 +7,18 @@ function getErrorCode(err: unknown): string | undefined {
   return err instanceof Error ? (err as NodeJS.ErrnoException).code : undefined;
 }
 
+export class JsonFileReadError extends Error {
+  readonly filePath: string;
+  readonly reason: "read" | "parse";
+
+  constructor(filePath: string, reason: "read" | "parse", cause: unknown) {
+    super(`Failed to ${reason} JSON file: ${filePath}`, { cause });
+    this.name = "JsonFileReadError";
+    this.filePath = filePath;
+    this.reason = reason;
+  }
+}
+
 async function replaceFileWithWindowsFallback(tempPath: string, filePath: string, mode: number) {
   try {
     await fs.rename(tempPath, filePath);
@@ -43,6 +55,23 @@ export async function readJsonFile<T>(filePath: string): Promise<T | null> {
   }
 }
 
+export async function readDurableJsonFile<T>(filePath: string): Promise<T | null> {
+  let raw: string;
+  try {
+    raw = await fs.readFile(filePath, "utf8");
+  } catch (err) {
+    if (getErrorCode(err) === "ENOENT") {
+      return null;
+    }
+    throw new JsonFileReadError(filePath, "read", err);
+  }
+  try {
+    return JSON.parse(raw) as T;
+  } catch (err) {
+    throw new JsonFileReadError(filePath, "parse", err);
+  }
+}
+
 export function readJsonFileSync(filePath: string): unknown {
   try {
     const raw = readFileSync(filePath, "utf8");
diff --git a/src/infra/node-pairing.test.ts b/src/infra/node-pairing.test.ts
@@ -1,3 +1,4 @@
+import fs from "node:fs/promises";
 import { afterAll, beforeAll, describe, expect, test } from "vitest";
 import { createSuiteTempRootTracker } from "../test-helpers/temp-dir.js";
 import {
@@ -7,6 +8,7 @@ import {
   requestNodePairing,
   verifyNodeToken,
 } from "./node-pairing.js";
+import { resolvePairingPaths } from "./pairing-files.js";
 
 async function setupPairedNode(baseDir: string): Promise<string> {
   const request = await requestNodePairing(
@@ -202,4 +204,23 @@ describe("node pairing tokens", () => {
       });
     });
   });
+
+  test("refuses to overwrite corrupt paired node state when requesting pairing", async () => {
+    await withNodePairingDir(async (baseDir) => {
+      const { dir, pairedPath } = resolvePairingPaths(baseDir, "nodes");
+      await fs.mkdir(dir, { recursive: true });
+      await fs.writeFile(pairedPath, "{not-json}", "utf8");
+
+      await expect(
+        requestNodePairing(
+          {
+            nodeId: "node-1",
+            platform: "darwin",
+          },
+          baseDir,
+        ),
+      ).rejects.toThrow(/paired\.json/);
+      await expect(fs.readFile(pairedPath, "utf8")).resolves.toBe("{not-json}");
+    });
+  });
 });
diff --git a/src/infra/node-pairing.ts b/src/infra/node-pairing.ts
@@ -5,7 +5,7 @@ import { type NodeApprovalScope, resolveNodePairApprovalScopes } from "./node-pa
 import {
   createAsyncLock,
   pruneExpiredPending,
-  readJsonFile,
+  readDurableJsonFile,
   reconcilePendingPairingRequests,
   resolvePairingPaths,
   writeJsonAtomic,
@@ -134,8 +134,8 @@ type ApproveNodePairingResult = ApprovedNodePairingResult | ForbiddenNodePairing
 async function loadState(baseDir?: string): Promise<NodePairingStateFile> {
   const { pendingPath, pairedPath } = resolvePairingPaths(baseDir, "nodes");
   const [pending, paired] = await Promise.all([
-    readJsonFile<Record<string, NodePairingPendingRequest>>(pendingPath),
-    readJsonFile<Record<string, NodePairingPairedNode>>(pairedPath),
+    readDurableJsonFile<Record<string, NodePairingPendingRequest>>(pendingPath),
+    readDurableJsonFile<Record<string, NodePairingPairedNode>>(pairedPath),
   ]);
   const state: NodePairingStateFile = {
     pendingById: pending ?? {},
diff --git a/src/infra/pairing-files.ts b/src/infra/pairing-files.ts
@@ -1,7 +1,12 @@
 import path from "node:path";
 import { resolveStateDir } from "../config/paths.js";
 
-export { createAsyncLock, readJsonFile, writeJsonAtomic } from "./json-files.js";
+export {
+  createAsyncLock,
+  readDurableJsonFile,
+  readJsonFile,
+  writeJsonAtomic,
+} from "./json-files.js";
 
 export function resolvePairingPaths(baseDir: string | undefined, subdir: string) {
   const root = baseDir ?? resolveStateDir();
