From 300fb36879c2efbe93c98d36067aacf4cdabd818 Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Fri, 3 Apr 2026 20:21:44 -0400 Subject: [PATCH] infra: atomically replace sync JSON writes (#60589) Merged via squash. Prepared head SHA: cb8ed7704992fe7ea94a6ef16ee1638f3f650a23 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + src/infra/json-file.test.ts | 119 +++++++++++++++++++++++++++++++--- src/infra/json-file.ts | 124 +++++++++++++++++++++++++++++++++--- 3 files changed, 227 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d5beaabba4..955d642e81e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -104,6 +104,7 @@ Docs: https://docs.openclaw.ai - WhatsApp/watchdog: reset watchdog timeout after reconnect so quiet channels no longer enter a tight reconnect loop from stale message timestamps carried across connection runs. (#60007) Thanks @MonkeyLeeT. - Agents/fallback: persist selected fallback overrides before retry attempts start, prefer persisted overrides during live-session reconciliation, and keep provider-scoped auth-profile failover from snapping retries back to stale primary selections. - Agents/MCP: sort MCP tools deterministically by name so the tools block in API requests is stable across turns, preventing unnecessary prompt-cache busting from non-deterministic `listTools()` order. (#58037) Thanks @bcherny. +- Infra/json-file: preserve symlink-backed JSON stores and Windows overwrite fallback when atomically saving small sync JSON state files. (#60589) Thanks @gumadeiras. ## 2026.4.2 diff --git a/src/infra/json-file.test.ts b/src/infra/json-file.test.ts index 10f344d4902..d26fd94a2e9 100644 --- a/src/infra/json-file.test.ts +++ b/src/infra/json-file.test.ts @@ -1,9 +1,20 @@ import fs from "node:fs"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { withTempDir } from "../test-helpers/temp-dir.js"; import { loadJsonFile, saveJsonFile } from "./json-file.js"; +const SAVED_PAYLOAD = { enabled: true, count: 2 }; +const PREVIOUS_JSON = '{"enabled":false}\n'; + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +function writeExistingJson(pathname: string) { + fs.writeFileSync(pathname, PREVIOUS_JSON, "utf8"); +} + async function withJsonPath( run: (params: { root: string; pathname: string }) => Promise | T, ): Promise { @@ -13,6 +24,10 @@ async function withJsonPath( } describe("json-file helpers", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + it.each([ { name: "missing files", @@ -40,11 +55,11 @@ describe("json-file helpers", () => { it("creates parent dirs, writes a trailing newline, and loads the saved object", async () => { await withTempDir({ prefix: "openclaw-json-file-" }, async (root) => { const pathname = path.join(root, "nested", "config.json"); - saveJsonFile(pathname, { enabled: true, count: 2 }); + saveJsonFile(pathname, SAVED_PAYLOAD); const raw = fs.readFileSync(pathname, "utf8"); expect(raw.endsWith("\n")).toBe(true); - expect(loadJsonFile(pathname)).toEqual({ enabled: true, count: 2 }); + expect(loadJsonFile(pathname)).toEqual(SAVED_PAYLOAD); const fileMode = fs.statSync(pathname).mode & 0o777; const dirMode = fs.statSync(path.dirname(pathname)).mode & 0o777; @@ -64,15 +79,103 @@ describe("json-file helpers", () => { }, { name: "existing JSON files", - setup: (pathname: string) => { - fs.writeFileSync(pathname, '{"enabled":false}\n', "utf8"); - }, + setup: writeExistingJson, }, ])("writes the latest payload for $name", async ({ setup }) => { await withJsonPath(({ pathname }) => { setup(pathname); - saveJsonFile(pathname, { enabled: true, count: 2 }); - expect(loadJsonFile(pathname)).toEqual({ enabled: true, count: 2 }); + saveJsonFile(pathname, SAVED_PAYLOAD); + expect(loadJsonFile(pathname)).toEqual(SAVED_PAYLOAD); + }); + }); + + it("writes through a sibling temp file before replacing the destination", async () => { + await withJsonPath(({ pathname }) => { + writeExistingJson(pathname); + const renameSpy = vi.spyOn(fs, "renameSync"); + + saveJsonFile(pathname, SAVED_PAYLOAD); + + const renameCall = renameSpy.mock.calls.find(([, target]) => target === pathname); + expect(renameCall?.[0]).toMatch(new RegExp(`^${escapeRegExp(pathname)}\\..+\\.tmp$`)); + expect(renameSpy).toHaveBeenCalledWith(renameCall?.[0], pathname); + expect(loadJsonFile(pathname)).toEqual(SAVED_PAYLOAD); + }); + }); + + it.runIf(process.platform !== "win32")( + "preserves symlink destinations when replacing existing JSON files", + async () => { + await withTempDir({ prefix: "openclaw-json-file-" }, async (root) => { + const targetDir = path.join(root, "target"); + const targetPath = path.join(targetDir, "config.json"); + const linkPath = path.join(root, "config-link.json"); + fs.mkdirSync(targetDir, { recursive: true }); + writeExistingJson(targetPath); + fs.symlinkSync(targetPath, linkPath); + + saveJsonFile(linkPath, SAVED_PAYLOAD); + + expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true); + expect(loadJsonFile(targetPath)).toEqual(SAVED_PAYLOAD); + expect(loadJsonFile(linkPath)).toEqual(SAVED_PAYLOAD); + }); + }, + ); + + it.runIf(process.platform !== "win32")( + "creates a missing target file through an existing symlink", + async () => { + await withTempDir({ prefix: "openclaw-json-file-" }, async (root) => { + const targetDir = path.join(root, "target"); + const targetPath = path.join(targetDir, "config.json"); + const linkPath = path.join(root, "config-link.json"); + fs.mkdirSync(targetDir, { recursive: true }); + fs.symlinkSync(targetPath, linkPath); + + saveJsonFile(linkPath, SAVED_PAYLOAD); + + expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true); + expect(loadJsonFile(targetPath)).toEqual(SAVED_PAYLOAD); + expect(loadJsonFile(linkPath)).toEqual(SAVED_PAYLOAD); + }); + }, + ); + + it.runIf(process.platform !== "win32")( + "does not create missing target directories through an existing symlink", + async () => { + await withTempDir({ prefix: "openclaw-json-file-" }, async (root) => { + const missingTargetDir = path.join(root, "missing-target"); + const targetPath = path.join(missingTargetDir, "config.json"); + const linkPath = path.join(root, "config-link.json"); + fs.symlinkSync(targetPath, linkPath); + + expect(() => saveJsonFile(linkPath, SAVED_PAYLOAD)).toThrow( + expect.objectContaining({ code: "ENOENT" }), + ); + expect(fs.existsSync(missingTargetDir)).toBe(false); + expect(fs.lstatSync(linkPath).isSymbolicLink()).toBe(true); + }); + }, + ); + + it("falls back to copy when rename-based overwrite fails", async () => { + await withJsonPath(({ root, pathname }) => { + writeExistingJson(pathname); + const copySpy = vi.spyOn(fs, "copyFileSync"); + const renameSpy = vi.spyOn(fs, "renameSync").mockImplementationOnce(() => { + const err = new Error("EPERM") as NodeJS.ErrnoException; + err.code = "EPERM"; + throw err; + }); + + saveJsonFile(pathname, SAVED_PAYLOAD); + + expect(renameSpy).toHaveBeenCalledOnce(); + expect(copySpy).toHaveBeenCalledOnce(); + expect(loadJsonFile(pathname)).toEqual(SAVED_PAYLOAD); + expect(fs.readdirSync(root)).toEqual(["config.json"]); }); }); }); diff --git a/src/infra/json-file.ts b/src/infra/json-file.ts index f34259df2ec..01602efb430 100644 --- a/src/infra/json-file.ts +++ b/src/infra/json-file.ts @@ -1,23 +1,129 @@ +import { randomUUID } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; -export function loadJsonFile(pathname: string): unknown { +const JSON_FILE_MODE = 0o600; +const JSON_DIR_MODE = 0o700; + +function trySetSecureMode(pathname: string) { try { - if (!fs.existsSync(pathname)) { - return undefined; + fs.chmodSync(pathname, JSON_FILE_MODE); + } catch { + // best-effort on platforms without chmod support + } +} + +function trySyncDirectory(pathname: string) { + let fd: number | undefined; + try { + fd = fs.openSync(path.dirname(pathname), "r"); + fs.fsyncSync(fd); + } catch { + // best-effort; some platforms/filesystems do not support syncing directories. + } finally { + if (fd !== undefined) { + try { + fs.closeSync(fd); + } catch { + // best-effort cleanup + } } + } +} + +function readSymlinkTargetPath(linkPath: string): string { + const target = fs.readlinkSync(linkPath); + return path.resolve(path.dirname(linkPath), target); +} + +function resolveJsonWriteTarget(pathname: string): { targetPath: string; followsSymlink: boolean } { + let currentPath = pathname; + const visited = new Set(); + let followsSymlink = false; + + for (;;) { + let stat: fs.Stats; + try { + stat = fs.lstatSync(currentPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } + return { targetPath: currentPath, followsSymlink }; + } + + if (!stat.isSymbolicLink()) { + return { targetPath: currentPath, followsSymlink }; + } + + if (visited.has(currentPath)) { + const err = new Error( + `Too many symlink levels while resolving ${pathname}`, + ) as NodeJS.ErrnoException; + err.code = "ELOOP"; + throw err; + } + + visited.add(currentPath); + followsSymlink = true; + currentPath = readSymlinkTargetPath(currentPath); + } +} + +function renameJsonFileWithFallback(tmpPath: string, pathname: string) { + try { + fs.renameSync(tmpPath, pathname); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + // Windows does not reliably support rename-based overwrite for existing files. + if (code === "EPERM" || code === "EEXIST") { + fs.copyFileSync(tmpPath, pathname); + fs.rmSync(tmpPath, { force: true }); + return; + } + throw error; + } +} + +function writeTempJsonFile(pathname: string, payload: string) { + const fd = fs.openSync(pathname, "w", JSON_FILE_MODE); + try { + fs.writeFileSync(fd, payload, "utf8"); + fs.fsyncSync(fd); + } finally { + fs.closeSync(fd); + } +} + +export function loadJsonFile(pathname: string): T | undefined { + try { const raw = fs.readFileSync(pathname, "utf8"); - return JSON.parse(raw) as unknown; + return JSON.parse(raw) as T; } catch { return undefined; } } export function saveJsonFile(pathname: string, data: unknown) { - const dir = path.dirname(pathname); - if (!fs.existsSync(dir)) { - fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + const { targetPath, followsSymlink } = resolveJsonWriteTarget(pathname); + const tmpPath = `${targetPath}.${randomUUID()}.tmp`; + const payload = `${JSON.stringify(data, null, 2)}\n`; + + if (!followsSymlink) { + fs.mkdirSync(path.dirname(targetPath), { recursive: true, mode: JSON_DIR_MODE }); + } + try { + writeTempJsonFile(tmpPath, payload); + trySetSecureMode(tmpPath); + renameJsonFileWithFallback(tmpPath, targetPath); + trySetSecureMode(targetPath); + trySyncDirectory(targetPath); + } finally { + try { + fs.rmSync(tmpPath, { force: true }); + } catch { + // best-effort cleanup when rename does not happen + } } - fs.writeFileSync(pathname, `${JSON.stringify(data, null, 2)}\n`, "utf8"); - fs.chmodSync(pathname, 0o600); }