mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 12:10:42 +00:00
fix(pairing): preserve corrupt pairing stores
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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"]);
|
||||
|
||||
@@ -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 ?? {},
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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}");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 ?? {},
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user