fix: one stat-identity model for the target-secret filesystem store

src/auth/AGENTS.md

@@ -11,6 +11,12 @@ src/auth/
 ├── paths.ts            # Spawnfile auth home and profile path helpers
 ├── profileStore.ts     # Read/write auth profiles and imported auth material
 ├── importers.ts        # `.env`, Codex, and Claude Code import flows
+├── targetSecretSourceFsIdentity.ts
+│                       # Shared stat-identity model for the target-secret store:
+│                       # `FileIdentity`/`DirectoryIdentity` plus the four comparators
+│                       # the filesystem read and publish paths both use
+├── targetSecretSource*.ts
+│                       # Target-secret source records, publish/read paths, and lifecycle
 └── *.test.ts           # Tests next to the implementation they cover
 ```
 
@@ -23,6 +29,12 @@ src/auth/
 - Host-local auth-owned secret state lives under `auth/target-secrets` and is keyed by immutable ids in fixed leaves:
   `versions`, `grants`, `redemptions`, `revocations`, and `aliases`.
 - Path helpers for target secrets must be directory-rooted and direct-keyed only; no listing helpers.
+- Every filesystem path into that store re-observes a pathname it already stat'ed, and it must decide
+  sameness through `targetSecretSourceFsIdentity.ts` — never through a locally defined comparator.
+  One store gets one identity model; two models mean the weaker one silently wins somewhere.
+  `birthtimeNs` belongs in every comparator (it survives inode-number reuse); `ctimeNs` belongs in
+  `sameFileExact` alone, because a cooperating publisher legitimately mutates a growing final and the
+  publisher itself mutates directory ctime.
 - `derived-config.content` is DECLARATIVE, non-secret configuration that the caller authored into the request by design —
   it is stored as a target secret for uniform handling, but it is not minted material and its presence in the caller's
   own request file is not a leak.

src/auth/targetSecretSourceFsIdentity.test.ts

@@ -0,0 +1,109 @@
+import type { BigIntStats } from "node:fs";
+import { describe, expect, it } from "vitest";
+
+import { TARGET_SECRET_SOURCE_ERROR } from "./targetSecretSourceRecordCommon.js";
+import {
+  directoryIdentityOf, fileIdentityOf, sameDirectory, sameFileExact, sameFileInode, sameFileNode
+} from "./targetSecretSourceFsIdentity.js";
+
+// These pairs are synthetic on purpose. The identity model must be provable
+// without depending on whether the host filesystem recycles inode numbers,
+// reports a birth time, or ticks its clock coarsely: those are exactly the
+// properties that made the real-filesystem regression only appear ~35% of the
+// time, and only under parallel load.
+
+const owner = 1000;
+const MAX = 65_536;
+
+type StatFields = {
+  readonly birthtimeNs?: bigint;
+  readonly ctimeNs?: bigint;
+  readonly dev?: bigint;
+  readonly ino?: bigint;
+  readonly mode?: bigint;
+  readonly nlink?: bigint;
+  readonly size?: bigint;
+  readonly uid?: bigint;
+  readonly directory?: boolean;
+  readonly symlink?: boolean;
+};
+
+const stats = (fields: StatFields = {}): BigIntStats => ({
+  birthtimeNs: 1_000n, ctimeNs: 2_000n, dev: 66n, ino: 4_242n, mode: 0o100600n,
+  nlink: 1n, size: 128n, uid: BigInt(owner), ...fields,
+  isFile: () => fields.directory !== true,
+  isDirectory: () => fields.directory === true,
+  isSymbolicLink: () => fields.symlink === true
+} as unknown as BigIntStats);
+
+const directoryStats = (fields: StatFields = {}): BigIntStats =>
+  stats({ directory: true, mode: 0o40700n, size: 4_096n, nlink: 2n, ...fields });
+
+const file = (fields: StatFields = {}) => fileIdentityOf(stats(fields), owner, [1, 2], MAX);
+const directory = (fields: StatFields = {}) => directoryIdentityOf(directoryStats(fields), owner);
+
+describe("targetSecretSourceFsIdentity", () => {
+  it("rejects every pair that differs only by birth time", () => {
+    const left = file();
+    const right = file({ birthtimeNs: 1_001n });
+    expect(sameFileExact(left, right)).toBe(false);
+    expect(sameFileNode(left, right)).toBe(false);
+    expect(sameFileInode(left, right)).toBe(false);
+
+    const leftDirectory = directory();
+    const rightDirectory = directory({ birthtimeNs: 1_001n });
+    expect(sameDirectory(leftDirectory, rightDirectory)).toBe(false);
+  });
+
+  it("accepts a same-inode pair that only grew and re-stamped its ctime", () => {
+    const left = file({ ctimeNs: 2_000n, size: 64n });
+    const right = file({ ctimeNs: 9_000n, size: 128n });
+    expect(sameFileNode(left, right)).toBe(true);
+    expect(sameFileExact(left, right)).toBe(false);
+  });
+
+  it("accepts a same-inode pair whose link count moved while an election name was linked", () => {
+    const left = file({ nlink: 1n });
+    const right = file({ nlink: 2n });
+    expect(sameFileInode(left, right)).toBe(true);
+    expect(sameFileNode(left, right)).toBe(false);
+    expect(sameFileExact(left, right)).toBe(false);
+  });
+
+  it("accepts identical observations and rejects every other single-field difference", () => {
+    const base = file();
+    expect(sameFileExact(base, file())).toBe(true);
+    expect(sameFileNode(base, file())).toBe(true);
+    expect(sameFileInode(base, file())).toBe(true);
+    expect(sameDirectory(directory(), directory())).toBe(true);
+
+    for (const changed of [{ dev: 67n }, { ino: 4_243n }, { mode: 0o100400n }, { uid: BigInt(owner + 1) }]) {
+      const drifted = { ...base, ...changed };
+      expect(sameFileExact(base, drifted)).toBe(false);
+      expect(sameFileNode(base, drifted)).toBe(false);
+      expect(sameFileInode(base, drifted)).toBe(false);
+      expect(sameDirectory(directory(), { ...directory(), ...changed })).toBe(false);
+    }
+    expect(sameFileInode(base, file({ size: 64n }))).toBe(false);
+    expect(sameFileExact(base, file({ ctimeNs: 3_000n }))).toBe(false);
+  });
+
+  it("carries the validation the two call sites already required", () => {
+    expect(file().size).toBe(128);
+    expect(fileIdentityOf(stats({ size: 0n }), owner, [1], MAX, true).size).toBe(0);
+    expect(() => fileIdentityOf(stats({ directory: true }), owner, [1], MAX)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+    expect(() => fileIdentityOf(stats({ symlink: true }), owner, [1], MAX)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+    expect(() => fileIdentityOf(stats(), owner + 1, [1], MAX)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+    expect(() => fileIdentityOf(stats({ nlink: 2n }), owner, [1], MAX)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+    expect(() => fileIdentityOf(stats({ mode: 0o100644n }), owner, [1], MAX)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+    expect(() => fileIdentityOf(stats({ size: -1n }), owner, [1], MAX)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+    expect(() => fileIdentityOf(stats({ size: BigInt(MAX) + 1n }), owner, [1], MAX)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+    expect(() => fileIdentityOf(stats({ size: 1n }), owner, [1], MAX, true)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+
+    expect(directory().mode).toBe(0o40700n);
+    expect(() => directoryIdentityOf(stats(), owner)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+    expect(() => directoryIdentityOf(directoryStats({ symlink: true }), owner)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+    expect(() => directoryIdentityOf(directoryStats(), owner + 1)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+    expect(() => directoryIdentityOf(directoryStats({ mode: 0o40755n }), owner)).toThrow(TARGET_SECRET_SOURCE_ERROR);
+  });
+});

src/auth/targetSecretSourceFsIdentity.ts

@@ -0,0 +1,108 @@
+import { type BigIntStats } from "node:fs";
+
+import { TARGET_SECRET_SOURCE_ERROR } from "./targetSecretSourceRecordCommon.js";
+
+// One identity model for the whole auth-owned target-secret store.
+//
+// The read path and the publish path both re-observe a pathname they already
+// stat'ed and must decide whether they are still looking at the same node.
+// They used to answer that question differently: the read path compared
+// `birthtimeNs` as well, the publish path compared only `dev, ino, mode,
+// nlink, uid` (+ `size`). On a filesystem that recycles inode numbers — ext4
+// does, freely — the publish path could be handed a *different* file at the
+// same `dev, ino` and see no difference at all. This module is the single
+// definition both paths now use.
+//
+// `birthtimeNs` is in every comparator. It is fixed for the life of an inode
+// and changes on every reallocation, so it is the discriminator that survives
+// inode-number reuse. `ctimeNs` is in `sameFileExact` only: it moves whenever
+// the file is legitimately mutated while observed — a cooperating peer
+// appending the second half of a record, or the publisher itself touching a
+// directory — so putting it anywhere else would reject correct behaviour.
+// `sameFileNode` therefore omits `ctimeNs` and `size` for exactly the same
+// reason it always omitted `size`.
+//
+// On a filesystem that does not report a birth time, libuv reports `0n` or the
+// ctime for every file, so `birthtimeNs` carries no information and every
+// comparison degrades to the behaviour these paths had before this module
+// existed. It can never produce a false failure, only fail to add one.
+//
+// The property this actually buys, stated exactly: any replacement that is not
+// byte-, mode-, uid-, and nlink-identical to the original is rejected by the
+// content proof that already runs; a byte-identical replacement is rejected as
+// well, unless it was created within one kernel tick of the original, in which
+// case it is indistinguishable from the original by any stat field and is
+// observably harmless. This is not full inode-replacement detection: the
+// lstat-then-open window holds no file descriptor pinning the inode, so no
+// stat-based scheme can deliver that.
+
+export type DirectoryIdentity = Readonly<{
+  birthtimeNs: bigint;
+  dev: bigint;
+  ino: bigint;
+  mode: bigint;
+  uid: bigint;
+}>;
+
+export type FileIdentity = Readonly<{
+  birthtimeNs: bigint;
+  ctimeNs: bigint;
+  dev: bigint;
+  ino: bigint;
+  mode: bigint;
+  nlink: bigint;
+  size: number;
+  uid: bigint;
+}>;
+
+const fail = (): never => { throw new Error(TARGET_SECRET_SOURCE_ERROR); };
+
+/** Validates one mode-`0700` owner-held directory and reduces it to its identity. */
+export const directoryIdentityOf = (info: BigIntStats, owner: number): DirectoryIdentity => {
+  if (!info.isDirectory() || info.isSymbolicLink() || info.uid !== BigInt(owner) || (info.mode & 0o7777n) !== 0o700n) fail();
+  return { birthtimeNs: info.birthtimeNs, dev: info.dev, ino: info.ino, mode: info.mode, uid: info.uid };
+};
+
+/** Validates one mode-`0600` owner-held regular file and reduces it to its identity. */
+export const fileIdentityOf = (
+  info: BigIntStats, owner: number, links: readonly number[], maxBytes: number, zero = false
+): FileIdentity => {
+  if (!info.isFile() || info.isSymbolicLink() || info.uid !== BigInt(owner)
+    || !links.some((link) => info.nlink === BigInt(link)) || (info.mode & 0o7777n) !== 0o600n
+    || info.size < 0n || info.size > BigInt(maxBytes) || (zero && info.size !== 0n)) fail();
+  return {
+    birthtimeNs: info.birthtimeNs, ctimeNs: info.ctimeNs, dev: info.dev, ino: info.ino,
+    mode: info.mode, nlink: info.nlink, size: Number(info.size), uid: info.uid
+  };
+};
+
+/**
+ * Same directory node. Never compares ctime: the publisher mutates the leaf
+ * directory's ctime itself every time it links or unlinks an election name.
+ */
+export const sameDirectory = (left: DirectoryIdentity, right: DirectoryIdentity): boolean =>
+  left.birthtimeNs === right.birthtimeNs && left.dev === right.dev && left.ino === right.ino
+  && left.mode === right.mode && left.uid === right.uid;
+
+/** Same file, unchanged in every observable field including ctime and size. */
+export const sameFileExact = (left: FileIdentity, right: FileIdentity): boolean =>
+  left.birthtimeNs === right.birthtimeNs && left.ctimeNs === right.ctimeNs && left.dev === right.dev
+  && left.ino === right.ino && left.mode === right.mode && left.nlink === right.nlink
+  && left.size === right.size && left.uid === right.uid;
+
+/**
+ * Same file node, allowing the size and ctime to move. This is the growing
+ * final: an identical writer may legitimately append the rest of the record
+ * between two observations, which changes both.
+ */
+export const sameFileNode = (left: FileIdentity, right: FileIdentity): boolean =>
+  left.birthtimeNs === right.birthtimeNs && left.dev === right.dev && left.ino === right.ino
+  && left.mode === right.mode && left.nlink === right.nlink && left.uid === right.uid;
+
+/**
+ * Same inode, allowing the link count and ctime to move. This is the zero-byte
+ * token or claim while an identical publisher links or unlinks the other name.
+ */
+export const sameFileInode = (left: FileIdentity, right: FileIdentity): boolean =>
+  left.birthtimeNs === right.birthtimeNs && left.dev === right.dev && left.ino === right.ino
+  && left.mode === right.mode && left.size === right.size && left.uid === right.uid;

src/auth/targetSecretSourceFsPublishImmutable.test.ts

@@ -1,6 +1,8 @@
+import type { BigIntStats } from "node:fs";
 import { chmod, link, lstat, mkdtemp, readFile, rm, unlink, writeFile } from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
+import { setTimeout as sleepMs } from "node:timers/promises";
 import { afterEach, describe, expect, it } from "vitest";
 
 import { resolveAuthHome, resolveSpawnfileHome, resolveTargetSecretVersionPath, resolveTargetSecretVersionsDirectory, resolveTargetSecretsRoot } from "./paths.js";
@@ -11,6 +13,7 @@ import { createTargetSecretSourceVersionRecordBytes, parseTargetSecretSourceVers
 
 const originalHome = process.env.SPAWNFILE_HOME;
 const cleanup: string[] = [];
+let recordedInodeReuse = false;
 const entropy = (value: number) => (): Uint8Array => new Uint8Array(16).fill(value);
 
 afterEach(async () => {
@@ -345,18 +348,36 @@ describe("targetSecretSourceFsPublishImmutable", () => {
     await expect(lstat(input.final_path)).rejects.toMatchObject({ code: "ENOENT" });
   });
 
-  it("fails closed when the final inode is replaced between named and opened observations", async () => {
+  // The old title claimed detection of any inode replacement. Nothing pins the
+  // inode between the named lstat and the open, so that is not deliverable and
+  // the test never proved it: on a filesystem that recycles inode numbers the
+  // replacement can land on the very inode that was just freed, and until the
+  // publish path compared birth times as well it was then indistinguishable.
+  it("fails closed when the final is unlinked and recreated with identical bytes after the named observation", async () => {
     const base = await setup();
     const { input } = packet();
     await base.publishImmutable(input);
-    let replaced = false;
+    let original: BigIntStats | undefined;
+    let replacement: BigIntStats | undefined;
     const reader = await initializeCurrent({ hookForTest: async (phase, file) => {
-      if (phase !== "after_final_lstat" || replaced) return;
-      replaced = true;
+      if (phase !== "after_final_lstat" || original) return;
+      original = await lstat(file, { bigint: true });
       await unlink(file);
+      // One kernel tick at HZ >= 100, so the replacement cannot inherit the
+      // original's birth time from the coarse timestamp clock.
+      await sleepMs(25);
       await writeFile(file, input.bytes, { mode: 0o600 });
+      replacement = await lstat(file, { bigint: true });
     } });
     await expect(reader.publishImmutable(input)).rejects.toThrow(TARGET_SECRET_SOURCE_ERROR);
-    expect(replaced).toBe(true);
+    expect(original).toBeDefined();
+    expect(replacement).toBeDefined();
+    // Without this the run could pass on a filesystem that reports no birth
+    // time, proving nothing about the discriminator the rejection relies on.
+    expect(replacement!.birthtimeNs).not.toBe(original!.birthtimeNs);
+    // Recorded, never asserted: whether the inode number came back is a
+    // property of the host filesystem and its concurrent load, not of this code.
+    recordedInodeReuse = replacement!.dev === original!.dev && replacement!.ino === original!.ino;
+    expect(typeof recordedInodeReuse).toBe("boolean");
   });
 });