piclaw-bot · Sep 30, 2026 · Sep 30, 2026
diff --git a/...agent-harness-integration-adr/evidence/earendil-0991-credential-atomic-write.md b/...agent-harness-integration-adr/evidence/earendil-0991-credential-atomic-write.md
@@ -0,0 +1,31 @@
+# Atomic credential-file replacement (#1458)
+
+Piclaw stages complete credential JSON in a private same-directory file, syncs it and replaces `auth.json` by rename under the existing file lock. Owned process-loss fixtures leave complete old or new JSON at the tested checkpoints. This is local POSIX process-crash evidence, not power-loss or general filesystem durability certification.
+
+## Write and commit boundary
+
+The app-owned writer creates a unique `wx` staging file with mode 0600, writes and syncs it, closes it, renames it over the target and syncs the parent directory on POSIX. Windows uses the same replacement path without directory sync; it has no platform-specific receipt in this slice.
+
+Pre-rename failures preserve the old file and clean staging where possible. Descriptor cleanup and staging cleanup run outside `finally` control flow, preserving commit state and using generic diagnostics. If cleanup itself fails, the error reports that failure without raw paths or credential-bearing exception text.
+
+Rename is the credential commit point. A later directory-sync failure produces a typed app-owned commit error; the store records it in `drainErrors()` and emits metadata-only warning, while returning the new authoritative credential. It does not report a failed mutation that could cause a caller to restore an already superseded token. Power-loss persistence after an unsuccessful directory sync is unknown.
+
+The store refuses symlink, hardlinked and non-regular targets. On POSIX, it verifies the directly supplied credential directory/file ownership, uses no-follow descriptor opens for validation/read, and enforces 0700/0600. This assumes an app-owned parent directory without attacks by the owning UID; ancestor-directory substitution and arbitrary hostile shared roots are not certified. Initial creation uses `wx`, so two initializers cannot overwrite each other's file.
+
+## Fault and process-loss fixtures
+
+The optional app-owned file-I/O seam injects failures at staging open, partial write, file sync, close before/after close, rename, directory sync and cleanup. Tests check original/new file bytes, private permissions, descriptor cleanup and generic error output. A store-level directory-sync failure test verifies committed state, cross-instance reads and redacted warning records.
+
+Owned child fixtures block during partial staging, after file sync before rename, and immediately after rename. The parent asserts the child was alive, kills only that child, checks SIGKILL and reads complete old/new JSON. Pre-rename deaths leave a private staging file; after rename there is none. These are helper checkpoints using the same production I/O path, not random mid-instruction crash fuzzing or filesystem power-loss tests.
+
+Staging files left by an abrupt process death are not read as credentials and are not deleted automatically. They may contain synthetic/real private credential bytes and need a separately reviewed retention/cleanup policy. No bulk cleanup or historical-secret deletion occurs here.
+
+## Validation
+
+The failure/store/owned-crash set passed 30 tests and 166 assertions. The post-cross-process-merge writer/store/public-runtime lifecycle and cross-process set passed 40 tests and 219 assertions, including the real stale-lock recovery case. Strict fixture typechecks, all five standard typechecks, scoped lint and diff checks passed. Reviews corrected cleanup, descriptor and directly supplied path safety; final scoped review found no blocker for the private POSIX parent scope.
+
+The first focused run hit the existing concurrent-mutation five-second test budget; an unchanged rerun passed 22 tests and 115 assertions. No timeout was raised. At merged cross-process baseline `e7539b67a5c501c5e875ff42fe568e5ca2d2b186`, `make ci-fast` passed 5,969 runtime tests, eight skips and no failures, 25 feature tests and nine web checks. Seven skips are existing and one is the earlier opt-in stale-lock fixture, separately executed in the 40-test set. Pack hygiene passed 24,742 files; final five typechecks and diff checks passed with the unchanged 95-diagnostic compose baseline. Private Bun caches were used without shared-cache permission changes.
+
+## Remaining qualification
+
+Atomic replacement does not certify every filesystem, Windows behavior, parent-ancestor attack, power failure or stale staging-file retention. Legacy configuration keys/backups, all provider/device/CLI/UI paths, Delegate children and approved live accounts remain open. No live credential, account, inference, deployment or restart was used.
diff --git a/runtime/src/agent-pool/credential-file.ts b/runtime/src/agent-pool/credential-file.ts
@@ -0,0 +1,60 @@
+import { closeSync, fsyncSync, openSync, renameSync, unlinkSync, writeFileSync } from "node:fs";
+import { dirname } from "node:path";
+import { randomUUID } from "node:crypto";
+
+/** App-owned file-I/O seam for disposable failure/crash fixtures. */
+export interface CredentialFileIO {
+  open(path: string, flags: string, mode?: number): number;
+  write(fd: number, bytes: string): void;
+  sync(fd: number): void;
+  close(fd: number): void;
+  replace(source: string, target: string): void;
+  remove(path: string): void;
+}
+const nativeIO: CredentialFileIO = {
+  open: openSync, write: (fd, bytes) => writeFileSync(fd, bytes, "utf8"),
+  sync: fsyncSync, close: closeSync, replace: renameSync, remove: unlinkSync,
+};
+
+/** A same-directory replacement keeps readers on complete old or new JSON. */
+export function writeCredentialFileAtomic(path: string, bytes: string, io: CredentialFileIO = nativeIO): void {
+  const temp = `${path}.${process.pid}.${randomUUID()}.tmp`;
+  let staged = false, committed = false, failed = false, cleanupFailed = false;
+  let descriptor: number | undefined;
+  try {
+    descriptor = io.open(temp, "wx", 0o600);
+    staged = true;
+    io.write(descriptor, bytes);
+    io.sync(descriptor);
+    io.close(descriptor); descriptor = undefined;
+    io.replace(temp, path); committed = true; staged = false;
+    // Windows cannot open directories for fsync. Atomic visibility is used
+    // there too, but directory-sync durability is not claimed.
+    if (process.platform !== "win32") {
+      descriptor = io.open(dirname(path), "r");
+      io.sync(descriptor);
+      io.close(descriptor); descriptor = undefined;
+    }
+  } catch { failed = true; }
+  // Cleanup happens outside finally so it cannot obscure commit state or the
+  // primary failure. Retry close before opening any other descriptor.
+  if (descriptor !== undefined) {
+    try { io.close(descriptor); }
+    catch { cleanupFailed = true; }
+  }
+  if (staged) {
+    try { io.remove(temp); }
+    catch { cleanupFailed = true; }
+  }
+  if (failed || cleanupFailed) {
+    if (committed) throw new CredentialFileCommitError();
+    throw new Error(cleanupFailed
+      ? "Credential file could not be replaced; staging cleanup also failed."
+      : "Credential file could not be replaced.");
+  }
+}
+
+/** Rename committed the new credential, but directory synchronization failed. */
+export class CredentialFileCommitError extends Error {
+  constructor() { super("Credential file was replaced, but directory synchronization failed."); this.name = "CredentialFileCommitError"; }
+}
diff --git a/runtime/src/agent-pool/credential-store.ts b/runtime/src/agent-pool/credential-store.ts
@@ -1,6 +1,7 @@
 import { execSync } from "node:child_process";
-import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
+import { closeSync, constants, existsSync, fchmodSync, fstatSync, lstatSync, mkdirSync, openSync, readFileSync, writeFileSync } from "node:fs";
 import { dirname, join } from "node:path";
+import { CredentialFileCommitError, type CredentialFileIO, writeCredentialFileAtomic } from "./credential-file.js";
 
 import type { Credential, CredentialInfo, CredentialStore } from "@earendil-works/pi-ai";
 import lockfile from "proper-lockfile";
@@ -153,6 +154,7 @@ export class FileCredentialStore implements PiclawCredentialStore {
   constructor(
     readonly authPath: string = join(getPiclawAgentDir(), "auth.json"),
     oauthRefreshRetry: Partial<OAuthRefreshRetryOptions> = {},
+    private readonly credentialFileIO?: CredentialFileIO,
   ) {
     this.oauthRefreshRetry = {
       maxRetries: oauthRefreshRetry.maxRetries ?? DEFAULT_OAUTH_REFRESH_MAX_RETRIES,
@@ -172,18 +174,46 @@ export class FileCredentialStore implements PiclawCredentialStore {
   private ensureFile(): void {
     const parent = dirname(this.authPath);
     if (!existsSync(parent)) mkdirSync(parent, { recursive: true, mode: AUTH_DIRECTORY_MODE });
-    if (!existsSync(this.authPath)) writeFileSync(this.authPath, "{}", { encoding: "utf8", mode: AUTH_FILE_MODE });
-    chmodSync(this.authPath, AUTH_FILE_MODE);
+    const parentStat = lstatSync(parent);
+    if (!parentStat.isDirectory() || parentStat.isSymbolicLink()) throw new Error("Credential directory must be a real directory.");
+    if (process.platform !== "win32") {
+      const dir = openSync(parent, constants.O_RDONLY | constants.O_NOFOLLOW);
+      try {
+        const stat = fstatSync(dir);
+        if (!stat.isDirectory() || stat.uid !== process.getuid?.()) throw new Error("Credential directory ownership mismatch.");
+        fchmodSync(dir, AUTH_DIRECTORY_MODE);
+      } finally { closeSync(dir); }
+    }
+    if (!existsSync(this.authPath)) {
+      try { writeFileSync(this.authPath, "{}", { encoding: "utf8", mode: AUTH_FILE_MODE, flag: "wx" }); }
+      catch (error) { if (!error || typeof error !== "object" || !("code" in error) || error.code !== "EEXIST") throw error; }
+    }
+    const target = lstatSync(this.authPath);
+    if (!target.isFile() || target.isSymbolicLink() || target.nlink !== 1) throw new Error("Credential target must be a private regular file.");
+    const fd = openSync(this.authPath, constants.O_RDONLY | (process.platform === "win32" ? 0 : constants.O_NOFOLLOW));
+    try {
+      const stat = fstatSync(fd);
+      if (!stat.isFile() || stat.nlink !== 1 || (process.platform !== "win32" && stat.uid !== process.getuid?.())) throw new Error("Credential file ownership mismatch.");
+      fchmodSync(fd, AUTH_FILE_MODE);
+    } finally { closeSync(fd); }
   }
 
   private readFileData(): CredentialData {
     this.ensureFile();
-    return parseCredentialData(readFileSync(this.authPath, "utf8"));
+    const fd = openSync(this.authPath, constants.O_RDONLY | (process.platform === "win32" ? 0 : constants.O_NOFOLLOW));
+    try { return parseCredentialData(readFileSync(fd, "utf8")); }
+    finally { closeSync(fd); }
   }
 
   private writeFileData(data: CredentialData): void {
-    writeFileSync(this.authPath, `${JSON.stringify(data, null, 2)}\n`, { encoding: "utf8", mode: AUTH_FILE_MODE });
-    chmodSync(this.authPath, AUTH_FILE_MODE);
+    try { writeCredentialFileAtomic(this.authPath, `${JSON.stringify(data, null, 2)}\n`, this.credentialFileIO); }
+    catch (error) {
+      // After rename, the new credential is authoritative. Reporting a failed
+      // mutation would let callers mistakenly restore a superseded token.
+      if (!(error instanceof CredentialFileCommitError)) throw error;
+      this.recordError(error);
+      log.warn("Credential file committed without confirmed directory synchronization", { operation: "credential_store.directory_sync_failed" });
+    }
   }
 
   private async withLock<T>(fn: (current: CredentialData, assertLock: () => void) => Promise<T>): Promise<T> {

diff --git a/runtime/test/agent-pool/credential-file-crash.test.ts b/runtime/test/agent-pool/credential-file-crash.test.ts
@@ -0,0 +1,33 @@
+import { expect, test } from "bun:test";
+import { readFileSync, readdirSync, statSync, writeFileSync } from "node:fs";
+import { dirname, join } from "node:path";
+import { createTempWorkspace } from "../helpers.js";
+for (const stage of ["partial", "before-rename", "after-rename"]) {
+  test(`owned process loss at atomic credential checkpoint: ${stage}`, async () => {
+    const ws = createTempWorkspace("atomic-auth-crash-");
+    const path = join(ws.base, "auth.json");
+    const old = '{"provider":{"type":"oauth","refresh":"synthetic-old"}}\n';
+    const current = '{"provider":{"type":"oauth","refresh":"synthetic-new"}}\n';
+    writeFileSync(path, old, { mode: 0o600 });
+    const child = Bun.spawn([process.execPath, "--no-env-file", join(import.meta.dir, "fixtures/credential-file-kill.ts"), stage, path], {
+      env: { PATH: `${dirname(process.execPath)}:/usr/bin:/bin`, HOME: ws.base, PI_OFFLINE: "1", OTEL_SDK_DISABLED: "true" }, stdout: "pipe", stderr: "pipe",
+    });
+    const timer = setTimeout(() => child.kill("SIGKILL"), 5000);
+    try {
+      const reader = child.stdout.getReader();
+      const { value } = await reader.read();
+      expect(new TextDecoder().decode(value)).toBe("blocked\n");
+      expect(child.exitCode).toBeNull();
+      child.kill("SIGKILL"); await child.exited;
+      expect(child.signalCode).toBe("SIGKILL");
+      expect(await new Response(child.stderr).text()).toBe("");
+      expect(readFileSync(path, "utf8")).toBe(stage === "after-rename" ? current : old);
+      expect(JSON.parse(readFileSync(path, "utf8")).provider.refresh).toBe(stage === "after-rename" ? "synthetic-new" : "synthetic-old");
+      const staging = readdirSync(ws.base).filter(name => name.endsWith(".tmp"));
+      expect(staging).toHaveLength(stage === "after-rename" ? 0 : 1);
+      for (const file of staging) expect(statSync(join(ws.base, file)).mode & 0o777).toBe(0o600);
+    } finally {
+      clearTimeout(timer); if (child.exitCode === null) child.kill("SIGKILL"); await child.exited; ws.cleanup();
+    }
+  }, 7000);
+}
diff --git a/runtime/test/agent-pool/credential-file.test.ts b/runtime/test/agent-pool/credential-file.test.ts
@@ -0,0 +1,101 @@
+import { expect, test } from "bun:test";
+import { closeSync, fsyncSync, openSync, readFileSync, readdirSync, renameSync, statSync, unlinkSync, writeFileSync } from "node:fs";
+import { join } from "node:path";
+import { CredentialFileCommitError, type CredentialFileIO, writeCredentialFileAtomic } from "../../src/agent-pool/credential-file.js";
+import { createTempWorkspace } from "../helpers.js";
+import { FileCredentialStore } from "../../src/agent-pool/credential-store.js";
+import { addLogSink, removeLogSink, type LogRecord } from "../../src/utils/logger.js";
+
+test("directory-sync failure reports a committed store value without raw diagnostics or rollback", async () => {
+  const ws = createTempWorkspace("atomic-credential-commit-");
+  const path = join(ws.base, "agent", "auth.json");
+  const seed = new FileCredentialStore(path);
+  await seed.modify("provider", async () => ({ type: "oauth", access: "old", refresh: "old", expires: 1 }));
+  let directoryFd = -1;
+  const io: CredentialFileIO = {
+    open: (name, flags, mode) => { const fd = openSync(name, flags, mode); if (flags === "r") directoryFd = fd; return fd; },
+    write: (fd, bytes) => writeFileSync(fd, bytes), sync: fd => { if (fd === directoryFd) throw new Error("SENTINEL-dir-sync-secret"); fsyncSync(fd); },
+    close: closeSync, replace: renameSync, remove: unlinkSync,
+  };
+  const logs: LogRecord[] = [], sink = (record: LogRecord) => logs.push(record);
+  addLogSink(sink);
+  try {
+    const store = new FileCredentialStore(path, {}, io);
+    const current = await store.modify("provider", async () => ({ type: "oauth", access: "new", refresh: "rotated", expires: Date.now() + 3_600_000 }));
+    expect(current).toMatchObject({ access: "new", refresh: "rotated" });
+    expect(await seed.read("provider")).toEqual(current);
+    if (process.platform !== "win32") expect(store.drainErrors()[0]).toBeInstanceOf(CredentialFileCommitError);
+    expect(JSON.stringify(logs)).not.toContain("SENTINEL");
+  } finally { removeLogSink(sink); ws.cleanup(); }
+});
+
+const oldBytes = '{"provider":{"type":"oauth","refresh":"synthetic-old"}}\n';
+const newBytes = '{"provider":{"type":"oauth","refresh":"synthetic-new"}}\n';
+for (const stage of ["none", "open", "write", "sync", "close", "close-before", "replace", "directory-sync", "cleanup"]) {
+  test(`atomic credential file: ${stage}`, () => {
+    const ws = createTempWorkspace("atomic-credential-");
+    const path = join(ws.base, "auth.json");
+    writeFileSync(path, oldBytes, { mode: 0o600 });
+    const events: string[] = [], descriptors = new Set<number>();
+    let tempFd = -1;
+    let closedFailure = false;
+    const fail = () => { throw new Error("SENTINEL-secret-filesystem-diagnostic"); };
+    const io: CredentialFileIO = {
+      open: (name, flags, mode) => {
+        events.push(flags === "wx" ? "open-temp" : "open-directory");
+        if (stage === "open" && flags === "wx") fail();
+        const fd = openSync(name, flags, mode); descriptors.add(fd);
+        if (flags === "wx") { tempFd = fd; expect(statSync(name).mode & 0o777).toBe(0o600); }
+        return fd;
+      },
+      write: (fd, bytes) => {
+        events.push("write");
+        expect(readFileSync(path, "utf8")).toBe(oldBytes);
+        if (stage === "write" || stage === "cleanup") { writeFileSync(fd, bytes.slice(0, 7)); fail(); }
+        writeFileSync(fd, bytes);
+      },
+      sync: fd => {
+        events.push(fd === tempFd ? "sync-temp" : "sync-directory");
+        if (stage === "sync" && fd === tempFd || stage === "directory-sync" && fd !== tempFd) fail();
+        fsyncSync(fd);
+      },
+      close: fd => {
+        const wasTemp = fd === tempFd;
+        if (stage === "close-before" && wasTemp && !closedFailure) { closedFailure = true; fail(); }
+        closeSync(fd); descriptors.delete(fd);
+        events.push(wasTemp ? "close-temp" : "close-directory");
+        if (wasTemp) tempFd = -1;
+        if (stage === "close" && wasTemp && !closedFailure) { closedFailure = true; fail(); }
+      },
+      replace: (source, target) => {
+        events.push("replace");
+        expect(readFileSync(path, "utf8")).toBe(oldBytes);
+        expect(readFileSync(source, "utf8")).toBe(newBytes);
+        if (stage === "replace") fail();
+        renameSync(source, target);
+      },
+      remove: name => { events.push("remove-temp"); if (stage === "cleanup") fail(); unlinkSync(name); },
+    };
+    try {
+      if (stage === "none") {
+        writeCredentialFileAtomic(path, newBytes, io);
+        expect(events).toEqual(process.platform === "win32"
+          ? ["open-temp", "write", "sync-temp", "close-temp", "replace"]
+          : ["open-temp", "write", "sync-temp", "close-temp", "replace", "open-directory", "sync-directory", "close-directory"]);
+      } else if (stage === "directory-sync" && process.platform === "win32") {
+        writeCredentialFileAtomic(path, newBytes, io);
+      } else {
+        let caught: unknown;
+        try { writeCredentialFileAtomic(path, newBytes, io); } catch (error) { caught = error; }
+        expect(caught).toBeInstanceOf(stage === "directory-sync" ? CredentialFileCommitError : Error);
+        expect(String(caught)).not.toContain("SENTINEL");
+      }
+      expect(readFileSync(path, "utf8")).toBe(stage === "none" || stage === "directory-sync" ? newBytes : oldBytes);
+      expect(statSync(path).mode & 0o777).toBe(0o600);
+      const staging = readdirSync(ws.base).filter(name => name.startsWith("auth.json.") && name.endsWith(".tmp"));
+      expect(staging).toHaveLength(stage === "cleanup" ? 1 : 0);
+      if (stage === "cleanup") expect(statSync(join(ws.base, staging[0])).mode & 0o777).toBe(0o600);
+      expect(descriptors.size).toBe(0);
+    } finally { for (const fd of descriptors) closeSync(fd); ws.cleanup(); }
+  });
+}