diff --git a/changelog.d/machines-state-crash-recovery.md b/changelog.d/machines-state-crash-recovery.md new file mode 100644 index 0000000000..1f32af9d7c --- /dev/null +++ b/changelog.d/machines-state-crash-recovery.md @@ -0,0 +1,6 @@ +### Fixed +- A crash or power loss no longer leaves the app unable to start because its saved remote PCs + cannot be read. The damaged file is kept beside it under a new name, the app starts without + those PCs, and Settings > Remote PCs names the file so you can add them again. The saved PCs + and herdr's machine approvals are now written so that a crash cannot leave them unreadable, + and only when they changed instead of on every refresh. diff --git a/server/api.contract.test.ts b/server/api.contract.test.ts index 595499c9e8..a55befe2be 100644 --- a/server/api.contract.test.ts +++ b/server/api.contract.test.ts @@ -31,7 +31,7 @@ import { testShellEnv } from "../scripts/test-shell.ts"; import { PendingInputError, PendingInputs } from "./pending-input.ts"; import { loginPathProbe, ShellAgentLocator, type LoginPathProbe } from "./shell-agent.ts"; import { DeviceStore } from "./devices.ts"; -import type { MachineEvent } from "../shared/machines.ts"; +import type { MachineEvent, MachineSettings } from "../shared/machines.ts"; import { MAX_MUTED_PANES, type SharedPreferences } from "../shared/preferences.ts"; /** @@ -1308,6 +1308,27 @@ describe("agent rename", () => { } }); + // #2044: a crash left machines.json its old length in NULs, and the server would not start + it("starts on saved PCs that are not JSON and names the file it set aside in the PC settings, read only", async () => { + const state = mkdtempSync(join(tmpdir(), "ovis-machines-set-aside-")); + writeFileSync(join(state, "machines.json"), Buffer.alloc(353)); + const error = spyOn(console, "error").mockImplementation(() => {}); + let app: ReturnType | undefined; + try { + app = createServer({ port: 0, hostname: "127.0.0.1", token: "", stateDir: state, tailscaleOwner: null }); + const url = `http://127.0.0.1:${app.port}/api/machines/settings`; + const settings = await (await fetch(url)).json() as MachineSettings; + expect(settings).toEqual({ auto_update_bridges: true, set_aside: [expect.stringMatching(/^machines\.json\.corrupt-\S+$/)] }); + expect(readdirSync(state)).toContain(settings.set_aside![0]!); + const patched = await fetch(url, { method: "PATCH", headers: { "content-type": "application/json", "x-herdr-machine": "1" }, body: JSON.stringify({ auto_update_bridges: false, set_aside: [] }) }); + expect(patched.status).toBe(200); + expect(await patched.json()).toEqual({ auto_update_bridges: false, set_aside: settings.set_aside }); + } finally { + app?.stop(); error.mockRestore(); + rmSync(state, { recursive: true, force: true }); + } + }); + it("validates the body before asking herdr", async () => { for (const [body, code] of [ [{}, "missing_pane_id"], diff --git a/server/herdr-profile-state.test.ts b/server/herdr-profile-state.test.ts index d6e74932f9..29054bf72b 100644 --- a/server/herdr-profile-state.test.ts +++ b/server/herdr-profile-state.test.ts @@ -1,8 +1,9 @@ -import { afterEach, expect, it } from "bun:test"; -import { mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { afterEach, expect, it, spyOn } from "bun:test"; +import { existsSync, mkdtempSync, openSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { HerdrProfileState } from "./herdr-profile-state.ts"; +import { privateStateFs } from "./private-state.ts"; import type { SessionSnapshot } from "../shared/protocol.ts"; const dirs: string[] = []; @@ -80,3 +81,62 @@ it("retries a failed key removal when the old target returns, after a restart an readded.flush(); expect(readFileSync(join(dir, "herdr-profile-state.json"), "utf8")).not.toContain("retiring"); }); +it("sets aside approvals that are not JSON, bytes untouched, and starts without them", () => { + const dir = directory(), path = join(dir, "herdr-profile-state.json"); + const zeros = Buffer.alloc(120); + writeFileSync(path, zeros); + const error = spyOn(console, "error").mockImplementation(() => {}); + try { + const store = new HerdrProfileState(dir); + expect(store.setAside).toMatch(/herdr-profile-state\.json\.corrupt-[^:]+$/); + expect(readFileSync(store.setAside!)).toEqual(zeros); + expect(existsSync(path)).toBe(false); + expect(store.approved("id", { destination: "BuildBox" })).toBe(false); + store.approve("id", { destination: "BuildBox" }); + store.flush(); + expect(new HerdrProfileState(dir).approved("id", { destination: "BuildBox" })).toBe(true); + } finally { error.mockRestore(); } +}); +it("writes unchanged approvals again when their file was deleted or replaced on disk", () => { + const dir = directory(), path = join(dir, "herdr-profile-state.json"); + const store = new HerdrProfileState(dir); + store.approve("id", { destination: "BuildBox" }); + store.flush(); + const saved = readFileSync(path, "utf8"); + rmSync(path); + store.approve("id", { destination: "BuildBox" }); + store.flush(); + expect(readFileSync(path, "utf8")).toBe(saved); + writeFileSync(path, "[]"); + store.approve("id", { destination: "BuildBox" }); + store.flush(); + expect(readFileSync(path, "utf8")).toBe(saved); + // the same length, other bytes + writeFileSync(path, saved.replace('"id"', '"xx"')); + store.approve("id", { destination: "BuildBox" }); + store.flush(); + expect(readFileSync(path, "utf8")).toBe(saved); +}); +it("writes durably, and only when the document changed", () => { + const dir = directory(); + const temporaries: string[] = []; + const open = spyOn(privateStateFs, "openSync").mockImplementation(((path: string, flags: string, mode?: number) => { + if (flags === "wx") temporaries.push(path); + return openSync(path, flags, mode); + }) as typeof openSync); + try { + const store = new HerdrProfileState(dir); + store.approve("id", { destination: "BuildBox" }); + store.saveSnapshot("id", { panes: [{ pane_id: "test-pane" }] } as SessionSnapshot); + store.flush(); + expect(temporaries).toHaveLength(1); + // every refresh brings a new snapshot object: the same panes write nothing + store.saveSnapshot("id", { panes: [{ pane_id: "test-pane" }] } as SessionSnapshot); + store.flush(); + expect(temporaries).toHaveLength(1); + store.saveSnapshot("id", { panes: [{ pane_id: "other-pane" }] } as SessionSnapshot); + store.flush(); + expect(temporaries).toHaveLength(2); + expect(new HerdrProfileState(dir).snapshot("id")?.panes[0]?.pane_id).toBe("other-pane"); + } finally { open.mockRestore(); } +}); diff --git a/server/herdr-profile-state.ts b/server/herdr-profile-state.ts index a287d9870a..c7894aff98 100644 --- a/server/herdr-profile-state.ts +++ b/server/herdr-profile-state.ts @@ -1,8 +1,9 @@ import { createHash } from "node:crypto"; -import { chmodSync, existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import type { SshTarget } from "../shared/machines.ts"; import type { SessionSnapshot } from "../shared/protocol.ts"; +import { writePrivateState } from "./private-state.ts"; +import { holdsDocument, readStateDocument, setAsideDamaged } from "./state-document.ts"; /** `retiring`: this binding's key removal was started and has not succeeded yet. */ interface ProfileState { binding: string; approved: boolean; connectionApproved: boolean; snapshot: SessionSnapshot | null; retiring?: true } @@ -21,10 +22,21 @@ export class HerdrProfileState { private entries = new Map(); private dirty = false; private path: string; - constructor(private directory: string) { + /** the document this store last wrote: a flush that would write the same bytes writes nothing + * while the file still holds them */ + private written: string | undefined; + /** where a file that was not JSON was moved at startup (setAsideDamaged), its approvals with it */ + readonly setAside: string | null = null; + constructor(directory: string) { this.path = join(directory, "herdr-profile-state.json"); - if (!existsSync(this.path)) return; - const saved: unknown = JSON.parse(readFileSync(this.path, "utf8")); + const read = readStateDocument(this.path); + if (read.kind === "missing") return; + if (read.kind === "malformed") { + this.setAside = setAsideDamaged(this.path); + console.error(`${this.path} is not JSON; it was set aside as ${this.setAside}, and herdr's saved machines need approving again`); + return; + } + const saved = read.value; if (!Array.isArray(saved)) throw new Error("Invalid herdr-profile-state.json; approvals were preserved"); for (const item of saved) { if (!item || typeof item.profile_id !== "string" || !item.profile_id || this.entries.has(item.profile_id) @@ -82,11 +94,17 @@ export class HerdrProfileState { if (errors.length) throw new AggregateError(errors, "Some saved machine keys could not be removed; cleanup will retry"); return removed; } + /** + * Writes the approvals durably (writePrivateState): a crash must not leave the file unreadable. A + * refresh hands over a new snapshot object each time, so an unchanged one still marks the state + * dirty; only a document that differs from the one on disk pays for the write and its fsync. + */ flush(): void { if (!this.dirty) return; - mkdirSync(this.directory, { recursive: true, mode: 0o700 }); - const tmp = this.path + ".tmp"; - writeFileSync(tmp, JSON.stringify([...this.entries].map(([profile_id, state]) => ({ profile_id, ...state }))), { mode: 0o600 }); - chmodSync(tmp, 0o600); renameSync(tmp, this.path); this.dirty = false; + const document = [...this.entries].map(([profile_id, state]) => ({ profile_id, ...state })); + const text = JSON.stringify(document); + // the file on disk may have been deleted or replaced since this store wrote it + if (text !== this.written || !holdsDocument(this.path, text)) { writePrivateState(this.path, document); this.written = text; } + this.dirty = false; } } diff --git a/server/machines-refresh.test.ts b/server/machines-refresh.test.ts index 8cb898e36d..52612f7eb8 100644 --- a/server/machines-refresh.test.ts +++ b/server/machines-refresh.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, spyOn } from "bun:test"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, openSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { createServer, type Socket } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -8,6 +8,7 @@ import type { ServerMessage, SessionSnapshot } from "../shared/protocol.ts"; import { CompletionTracker } from "./completion.ts"; import { handleMachineRequest } from "./machine-api.ts"; import { bridgeAgentNews, MachineManager, SOURCE_CHECK_INTERVAL_MS } from "./machines.ts"; +import { privateStateFs } from "./private-state.ts"; import type { PushService } from "./push.ts"; import { REMOTE_LIMITS, RemoteResponseTooLarge } from "./remote-json.ts"; @@ -1021,3 +1022,41 @@ describe("workspace order evidence through the manager", () => { expect(JSON.stringify(await (await ask("")).json())).not.toContain("fixture-token"); }); }); + +describe("saving a remote PC's snapshot", () => { + it("rewrites machines.json durably, and only when a refresh changed what it holds", async () => { + const { manager, drive, remote, runtime } = await remoteFixture(); + const internals = manager as unknown as { stateDir: string; saveTimer?: unknown }; + // each durable write creates its temporary file exclusively ("wx") and fsyncs it + const temporaries: string[] = []; + const open = spyOn(privateStateFs, "openSync"); + const fsync = spyOn(privateStateFs, "fsyncSync"); + cleanups.push(() => { open.mockRestore(); fsync.mockRestore(); }); + open.mockImplementation(((path: string, flags: string, mode?: number) => { + if (flags === "wx" && path.includes("machines.json")) temporaries.push(path); + return openSync(path, flags, mode); + }) as typeof openSync); + // a refresh saves after a short wait (saveSoon): a save is done once its timer is gone + const refreshed = async () => { + await drive.refresh(runtime); + for (let attempt = 0; attempt < 200 && internals.saveTimer !== undefined; attempt++) await Bun.sleep(25); + expect(internals.saveTimer).toBeUndefined(); + }; + const saved = () => JSON.parse(readFileSync(join(internals.stateDir, "machines.json"), "utf8")) as { snapshot: SessionSnapshot | null }[]; + + await refreshed(); + expect(temporaries).toHaveLength(1); + expect(fsync).toHaveBeenCalled(); + expect(saved()[0]!.snapshot?.panes[0]?.agent_status).toBe("working"); + + // the same panes again, in a new object as every read brings: nothing to write + await refreshed(); + await refreshed(); + expect(temporaries).toHaveLength(1); + + remote.set(snapshot("done")); + await refreshed(); + expect(temporaries).toHaveLength(2); + expect(saved()[0]!.snapshot?.panes[0]?.agent_status).toBe("done"); + }); +}); diff --git a/server/machines.test.ts b/server/machines.test.ts index e5a8afae2d..5f493f44e1 100644 --- a/server/machines.test.ts +++ b/server/machines.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, spyOn } from "bun:test"; -import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, win32 } from "node:path"; import { describeProgress } from "../src/lib/bridgeProgress.ts"; @@ -1049,3 +1049,73 @@ describe("bridge verification failures", () => { expect((await unanswered({ host: "203.0.113.99" })).forwards).toEqual([[18472]]); }); }); + +// A crash can leave machines.json its old length in NULs (#2044): the app must still start. +describe("saved PCs that cannot be read", () => { + const offline = async () => { throw new Error("local offline"); }; + const dirs: string[] = []; + afterEach(() => { for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); }); + const directory = () => { const dir = mkdtempSync(join(tmpdir(), "herdr-machines-state-")); dirs.push(dir); return dir; }; + + it("sets a file of NULs aside and starts without the PCs it held", () => { + const dir = directory(); + const zeros = Buffer.alloc(353); + writeFileSync(join(dir, "machines.json"), zeros); + const error = spyOn(console, "error").mockImplementation(() => {}); + let manager: MachineManager | undefined; + try { + manager = new MachineManager(dir, {} as PushService, new CompletionTracker(null), offline); + const aside = readdirSync(dir).filter((name) => name.startsWith("machines.json.corrupt-")); + expect(aside).toHaveLength(1); + expect(aside[0]).not.toContain(":"); + expect(readFileSync(join(dir, aside[0]!))).toEqual(zeros); + expect(existsSync(join(dir, "machines.json"))).toBe(false); + expect(manager.list().map((machine) => machine.kind)).toEqual(["local"]); + expect(manager.settings()).toEqual({ auto_update_bridges: true, set_aside: aside }); + expect(String(error.mock.calls[0]?.[0])).toContain(aside[0]!); + // the next save makes a readable file again + (manager as unknown as { persist(): void }).persist(); + expect(JSON.parse(readFileSync(join(dir, "machines.json"), "utf8"))).toEqual([]); + } finally { manager?.stop(); error.mockRestore(); } + }); + + it("sets aside herdr's approvals that are not JSON, and says so with the PCs' settings", () => { + const dir = directory(); + writeFileSync(join(dir, "herdr-profile-state.json"), Buffer.alloc(120)); + const error = spyOn(console, "error").mockImplementation(() => {}); + let manager: MachineManager | undefined; + try { + manager = new MachineManager(dir, {} as PushService, new CompletionTracker(null), offline); + expect(manager.settings().set_aside).toEqual([expect.stringMatching(/^herdr-profile-state\.json\.corrupt-/)]); + } finally { manager?.stop(); error.mockRestore(); } + }); + + it("writes an unchanged list again when its file was deleted or replaced on disk", () => { + const dir = directory(), path = join(dir, "machines.json"); + writeFileSync(path, JSON.stringify([{ id: "11111111-1111-1111-1111-111111111111", name: "Saved PC", target: { destination: "BuildBox" }, enabled: false, snapshot: null }])); + const manager = new MachineManager(dir, {} as PushService, new CompletionTracker(null), offline); + try { + const persist = () => (manager as unknown as { persist(): void }).persist(); + persist(); + const saved = readFileSync(path, "utf8"); + rmSync(path); + persist(); + expect(readFileSync(path, "utf8")).toBe(saved); + writeFileSync(path, "[]"); + persist(); + expect(readFileSync(path, "utf8")).toBe(saved); + // the same length, other bytes + writeFileSync(path, saved.replace("Saved PC", "Other PC")); + persist(); + expect(readFileSync(path, "utf8")).toBe(saved); + } finally { manager.stop(); } + }); + + it("still refuses to start on JSON that is not a list of PCs, leaving it in place", () => { + const dir = directory(); + writeFileSync(join(dir, "machines.json"), "{}"); + expect(() => new MachineManager(dir, {} as PushService, new CompletionTracker(null), offline)).toThrow("Invalid machines.json"); + expect(readFileSync(join(dir, "machines.json"), "utf8")).toBe("{}"); + expect(readdirSync(dir).filter((name) => name.includes("corrupt"))).toEqual([]); + }); +}); diff --git a/server/machines.ts b/server/machines.ts index ed519d6313..3fa87e52d4 100644 --- a/server/machines.ts +++ b/server/machines.ts @@ -3,7 +3,7 @@ import { randomUUID } from "node:crypto"; import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, renameSync, rmSync, unlinkSync, writeFileSync } from "node:fs"; import { createServer as tcpServer } from "node:net"; import { hostname } from "node:os"; -import { join } from "node:path"; +import { basename, join } from "node:path"; import { BRIDGE_PROTOCOL, LOCAL_MACHINE, REMOTE_BUNDLE_VERSION, type BridgeIdentity, type Machine, type MachineAction, type MachineEvent, type MachineSettings, type SetupAction, type SetupJob, type SetupProgress, type SetupRequest, type SshTarget } from "../shared/machines.ts"; import type { ServerMessage, SessionSnapshot, HerdrPane } from "../shared/protocol.ts"; import type { PushService } from "./push.ts"; @@ -16,6 +16,8 @@ import type { CompletionTracker } from "./completion.ts"; import { HerdrProfileState, herdrMachineId } from "./herdr-profile-state.ts"; import { sameSshSession, type HerdrMachineProfile } from "./herdr-profiles.ts"; import { validateTarget } from "./machine-security.ts"; +import { writePrivateState } from "./private-state.ts"; +import { holdsDocument, readStateDocument, setAsideDamaged } from "./state-document.ts"; import { detectHost, HERDR_INSTALL_CMD, installLockOwner } from "./remote-host.ts"; import { discard, readCappedJson, REMOTE_LIMITS } from "./remote-json.ts"; import { SshConnection, sshClient } from "./ssh.ts"; @@ -202,6 +204,10 @@ export class MachineManager { private saveTimer?: ReturnType; private profileState: HerdrProfileState; private statePath: string; + /** machines.json as this manager last wrote it (persist) */ + private written: string | undefined; + /** the state files that were not JSON at startup, by the name they were moved to (settings) */ + private setAside: string[] = []; private sshDir: string; private local: Machine = { id: LOCAL_MACHINE, name: hostname(), kind: "local", enabled: true, state: "connecting", error: null, snapshot: null }; private localDiagnostics = new ConnectionDiagnostics("machine.local"); @@ -239,8 +245,17 @@ export class MachineManager { : error instanceof Error && "code" in error && typeof error.code === "string" ? error.code : "read failed"; if (cause !== "ENOENT") console.error(`Cannot read ${this.settingsPath}; remote PC settings start empty`, cause); } - if (existsSync(this.statePath)) { - const saved: unknown = JSON.parse(readFileSync(this.statePath, "utf8")); + if (this.profileState.setAside !== null) this.setAside.push(basename(this.profileState.setAside)); + // bytes that are not JSON (a crash can leave the file all NULs) are moved aside and the app starts + // without saved PCs; JSON that is not a list of PCs still refuses to start, so nothing replaces it + const read = readStateDocument(this.statePath); + if (read.kind === "malformed") { + const aside = setAsideDamaged(this.statePath); + this.setAside.push(basename(aside)); + console.error(`${this.statePath} is not JSON; it was set aside as ${aside}, and the app starts without the PCs added here`); + } + if (read.kind === "json") { + const saved = read.value; if (!Array.isArray(saved)) throw new Error("Invalid machines.json; registrations were preserved"); for (const item of saved as StoredMachine[]) { if (!/^[a-f0-9-]{36}$/.test(item.id) || typeof item.name !== "string" || typeof item.enabled !== "boolean") throw new Error("Invalid machine registration"); @@ -429,17 +444,22 @@ export class MachineManager { this.saveTimer = setTimeout(() => { this.saveTimer = undefined; if (!this.stopped) { try { this.persist(); } catch (e) { console.error("PC state could not be saved:", e instanceof Error ? e.message : "write failed"); } } }, 1000); this.saveTimer.unref(); } + /** + * The registrations cannot be recomputed, so they are written durably (writePrivateState): a + * crash after a plain rename can leave the file its old length in NULs. Every refresh of a remote + * PC saves soon, mostly with the same snapshot, so only a document that differs from the one on + * disk pays for the write and its fsync. + */ private persist(): void { - mkdirSync(this.stateDir, { recursive: true, mode: 0o700 }); const data: StoredMachine[] = [...this.machines.values()].filter(({ machine }) => !machine.herdr_profile_id).map(({ machine: m }) => ({ id: m.id, name: m.name, target: m.target!, enabled: m.enabled, snapshot: m.snapshot })); - const tmp = this.statePath + ".tmp"; - writeFileSync(tmp, JSON.stringify(data), { mode: 0o600 }); - chmodSync(tmp, 0o600); renameSync(tmp, this.statePath); + const text = JSON.stringify(data); + // the file on disk may have been deleted or replaced since this manager wrote it + if (text !== this.written || !holdsDocument(this.statePath, text)) { writePrivateState(this.statePath, data); this.written = text; } for (const { machine } of this.machines.values()) if (machine.herdr_profile_id) this.profileState.saveSnapshot(machine.herdr_profile_id, machine.snapshot); this.profileState.flush(); } job(id: string): SetupJob | undefined { return this.jobs.get(id)?.public; } - settings(): MachineSettings { return { ...this.preferences }; } + settings(): MachineSettings { return { ...this.preferences, ...(this.setAside.length > 0 ? { set_aside: [...this.setAside] } : {}) }; } updateSettings(patch: Record): MachineSettings { if (patch["auto_update_bridges"] !== undefined) { if (typeof patch["auto_update_bridges"] !== "boolean") throw new Error("auto_update_bridges must be boolean"); diff --git a/server/private-state.ts b/server/private-state.ts index 9b4311bc71..fb265a6ce3 100644 --- a/server/private-state.ts +++ b/server/private-state.ts @@ -84,7 +84,7 @@ export function preparePrivateState(path: string, value: unknown, platform: Node } /** Records the rename in `directory` (best effort, see the top of this file). */ -function syncDirectory(directory: string, platform: NodeJS.Platform): PrivateStateWrite { +export function syncDirectory(directory: string, platform: NodeJS.Platform): PrivateStateWrite { const fs = privateStateFs; if (platform === "win32") return { directorySynced: false }; let directoryFd: number | null = null; diff --git a/server/state-document.test.ts b/server/state-document.test.ts new file mode 100644 index 0000000000..3963c2fae5 --- /dev/null +++ b/server/state-document.test.ts @@ -0,0 +1,98 @@ +import { afterEach, describe, expect, it, spyOn } from "bun:test"; +import * as fs from "node:fs"; +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { basename, join } from "node:path"; +import { readStateDocument, setAsideDamaged } from "./state-document.ts"; + +const dirs: string[] = []; +afterEach(() => { for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); }); +const directory = () => { const dir = mkdtempSync(join(tmpdir(), "herdr-state-document-")); dirs.push(dir); return dir; }; + +describe("setAsideDamaged", () => { + it("moves a file of NULs aside, bytes untouched, under a name a Windows file may have", () => { + const dir = directory(), path = join(dir, "machines.json"); + const zeros = Buffer.alloc(353); + writeFileSync(path, zeros); + expect(readStateDocument(path)).toEqual({ kind: "malformed" }); + const aside = setAsideDamaged(path, new Date("2026-10-26T07:06:23.123Z")); + expect(basename(aside)).toBe("machines.json.corrupt-2026-10-26T07-06-23-123Z"); + expect(readFileSync(aside)).toEqual(zeros); + expect(readStateDocument(path)).toEqual({ kind: "missing" }); + }); + + it("keeps every damaged copy: a later one never replaces an earlier one", () => { + const dir = directory(), path = join(dir, "machines.json"); + writeFileSync(path, "first"); + setAsideDamaged(path, new Date("2026-10-24T01:00:00.000Z")); + writeFileSync(path, "second"); + setAsideDamaged(path, new Date("2026-10-26T01:00:00.000Z")); + const kept = readdirSync(dir).sort(); + expect(kept).toHaveLength(2); + expect(kept.map((name) => readFileSync(join(dir, name), "utf8"))).toEqual(["first", "second"]); + }); + + it("never replaces an earlier copy set aside at the same moment", () => { + const dir = directory(), path = join(dir, "machines.json"); + const now = new Date("2026-10-26T07:06:23.123Z"); + writeFileSync(path, "first"); + const first = setAsideDamaged(path, now); + writeFileSync(path, "second"); + const second = setAsideDamaged(path, now); + expect(second).not.toBe(first); + expect(readFileSync(first, "utf8")).toBe("first"); + expect(readFileSync(second, "utf8")).toBe("second"); + expect(readStateDocument(path)).toEqual({ kind: "missing" }); + }); + + // a file system without hard links: the copy is made with copyFileSync instead + const withoutLinks = () => spyOn(fs, "linkSync").mockImplementation(() => { throw Object.assign(new Error("no links"), { code: "EPERM" }); }); + + it("flushes a copy to disk before the damaged original goes", () => { + const dir = directory(), path = join(dir, "machines.json"); + writeFileSync(path, "damaged"); + const steps: string[] = []; + const links = withoutLinks(); + const fsync = spyOn(fs, "fsyncSync").mockImplementation(() => { steps.push("fsync"); }); + const unlink = spyOn(fs, "unlinkSync"); + unlink.mockImplementation(((target: fs.PathLike) => { steps.push(`unlink ${basename(String(target))}`); unlink.mockRestore(); fs.unlinkSync(target); }) as typeof fs.unlinkSync); + try { + const aside = setAsideDamaged(path, new Date("2026-10-26T07:06:23.123Z")); + expect(readFileSync(aside, "utf8")).toBe("damaged"); + expect(steps.indexOf("fsync")).toBeGreaterThanOrEqual(0); + expect(steps.indexOf("fsync")).toBeLessThan(steps.indexOf("unlink machines.json")); + } finally { links.mockRestore(); fsync.mockRestore(); unlink.mockRestore(); } + }); + + it("removes a copy that cannot be made private, leaving the original where it was", () => { + const dir = directory(), path = join(dir, "machines.json"); + writeFileSync(path, "damaged"); + const links = withoutLinks(); + const chmod = spyOn(fs, "chmodSync"); + chmod.mockImplementation(((target: fs.PathLike, mode: fs.Mode) => { + if (String(target).includes(".corrupt-")) throw Object.assign(new Error("denied"), { code: "EACCES" }); + }) as typeof fs.chmodSync); + try { + expect(() => setAsideDamaged(path)).toThrow(/\(EACCES\)$/); + expect(readdirSync(dir)).toEqual(["machines.json"]); + expect(readFileSync(path, "utf8")).toBe("damaged"); + } finally { links.mockRestore(); chmod.mockRestore(); } + }); + + it("reports why the original could not be removed, even when the spare copy cannot be either", () => { + const dir = directory(), path = join(dir, "machines.json"); + writeFileSync(path, "damaged"); + const unlink = spyOn(fs, "unlinkSync").mockImplementation(() => { throw Object.assign(new Error("busy"), { code: "EBUSY" }); }); + const rm = spyOn(fs, "rmSync").mockImplementation(() => { throw Object.assign(new Error("denied"), { code: "EACCES" }); }); + try { + expect(() => setAsideDamaged(path)).toThrow(/\(EBUSY\)$/); + expect(readFileSync(path, "utf8")).toBe("damaged"); + } finally { unlink.mockRestore(); rm.mockRestore(); } + }); + + it("throws, naming the file and why, when it cannot be moved", () => { + const dir = directory(); + expect(() => setAsideDamaged(join(dir, "machines.json"))).toThrow(/^machines\.json is not JSON and cannot be set aside as machines\.json\.corrupt-\S+ \(ENOENT\)$/); + expect(readdirSync(dir)).toEqual([]); + }); +}); diff --git a/server/state-document.ts b/server/state-document.ts index 543017853d..68c78bfca6 100644 --- a/server/state-document.ts +++ b/server/state-document.ts @@ -1,4 +1,5 @@ -import { chmodSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, closeSync, constants, copyFileSync, fsyncSync, linkSync, mkdirSync, openSync, readFileSync, renameSync, rmSync, unlinkSync, writeFileSync } from "node:fs"; +import { syncDirectory } from "./private-state.ts"; import { basename, dirname } from "node:path"; /** @@ -21,6 +22,73 @@ export function readStateDocument(path: string): StateDocument { /** The name a damaged document is kept under until the next damaged one replaces it. */ export const damagedCopy = (path: string): string => `${path}.bad`; +/** + * Moves a document that is not JSON out of the way at once, bytes untouched, to + * `.corrupt-