fix(codex): verify auth.json identity, not just content, before publishing

src/codex/main-account.ts

@@ -1,5 +1,5 @@
 import { createHash } from "node:crypto";
-import { readFileSync } from "node:fs";
+import { readFileSync, statSync } from "node:fs";
 import { join } from "node:path";
 import { readCodexTokens } from "./auth-collision";
 import {
@@ -37,6 +37,16 @@ let beforeMainAuthJsonRenameForTests: (() => void) | null = null;
 type MainAuthJsonCredential = {
   path: string;
   rawSha256: string;
+  /**
+   * Filesystem identity of the file the hash was taken from (#2999).
+   *
+   * A content hash cannot tell "unchanged" from "replaced with a file that happens to
+   * hash the same", and more importantly it is read at a different instant than the
+   * rename. Carrying dev+ino lets the pre-rename guard ask the sharper question: is this
+   * still the same file, not merely one with the same bytes. `null` when the target could
+   * not be stat'ed, which is treated as "cannot prove identity" rather than "matches".
+   */
+  identity: { dev: number; ino: number } | null;
   root: Record<string, unknown>;
   tokens: Record<string, unknown>;
   accessToken?: string;
@@ -73,6 +83,22 @@ function sha256(value: string): string {
   return createHash("sha256").update(value).digest("hex");
 }
 
+/**
+ * Filesystem identity of a path, or null when it cannot be read.
+ *
+ * Null is deliberately NOT "matches anything": a caller that cannot prove identity must
+ * fail closed, because the whole point here is refusing to overwrite a file we can no
+ * longer vouch for.
+ */
+function statIdentity(path: string): { dev: number; ino: number } | null {
+  try {
+    const stat = statSync(path);
+    return { dev: Number(stat.dev), ino: Number(stat.ino) };
+  } catch {
+    return null;
+  }
+}
+
 function readMainAuthJsonCredential(): MainAuthJsonCredential | null {
   const path = resolveWriteTarget(join(resolveCodexHomeDir(), "auth.json"));
   let raw: string;
@@ -98,6 +124,7 @@ function readMainAuthJsonCredential(): MainAuthJsonCredential | null {
     return {
       path,
       rawSha256: sha256(raw),
+      identity: statIdentity(path),
       root,
       tokens,
       ...(accessToken ? { accessToken } : {}),
@@ -131,6 +158,21 @@ function assertMainAuthJsonSnapshotUnchanged(expected: MainAuthJsonCredential):
   if (!current || current.path !== expected.path || current.rawSha256 !== expected.rawSha256) {
     throw new MainAuthJsonChangedDuringRefreshError();
   }
+  // Identity, not just content (#2999). A writer can land between this check and the
+  // rename, and rename(2) replaces unconditionally - so the narrower the question asked
+  // here, the smaller the window where a Codex login gets silently overwritten. An
+  // unreadable identity on either side fails closed: unprovable is not the same as equal.
+  assertMainAuthJsonIdentityUnchanged(expected);
+}
+
+function assertMainAuthJsonIdentityUnchanged(expected: MainAuthJsonCredential): void {
+  const identity = statIdentity(expected.path);
+  if (!identity
+    || !expected.identity
+    || identity.dev !== expected.identity.dev
+    || identity.ino !== expected.identity.ino) {
+    throw new MainAuthJsonChangedDuringRefreshError();
+  }
 }
 
 function persistRefreshedMainAuthJson(
@@ -160,6 +202,9 @@ function persistRefreshedMainAuthJson(
         beforeMainAuthJsonRenameForTests = null;
         hook?.();
       },
+      // Runs immediately before rename(2), after the test hook has had its chance to
+      // simulate an external writer. Full snapshot check (content AND identity): this is
+      // the last look we get, so it asks everything it can rather than the cheap question.
       validateBeforeRename: () => assertMainAuthJsonSnapshotUnchanged(expected),
     },
   );

tests/codex-main-account-refresh.test.ts

@@ -1,5 +1,5 @@
 import { afterEach, beforeEach, describe, expect, test } from "bun:test";
-import { mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs";
+import { existsSync, mkdtempSync, readFileSync, readdirSync, renameSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import {
@@ -247,3 +247,84 @@ describe("native main token refresh", () => {
     }
   });
 });
+
+describe("publication never overwrites an external Codex writer (#2999)", () => {
+  const refreshOk = async () => ({
+    access: "ocx-staged-access",
+    refresh: "ocx-staged-refresh",
+    expires: Date.now() + 3_600_000,
+    accountId: "account-main",
+  });
+
+  function seedExpired(authPath: string): void {
+    writeFileSync(authPath, JSON.stringify({
+      auth_mode: "chatgpt",
+      tokens: { access_token: expiredJwt(), refresh_token: "old-refresh", account_id: "account-main" },
+    }));
+  }
+
+  test("a writer landing at the rename boundary is preserved byte-for-byte", async () => {
+    // Issue reproduction step 5: replace auth.json from a simulated Codex writer at the
+    // final pre-rename hook, then let the publisher resume. Before this guard the staged
+    // credential won and the user's own `codex login` result was silently replaced.
+    const authPath = join(home, "auth.json");
+    seedExpired(authPath);
+    const external = JSON.stringify({
+      auth_mode: "chatgpt",
+      tokens: { access_token: "codex-cli-wrote-this", refresh_token: "codex-refresh", account_id: "account-main" },
+    });
+    setMainAuthJsonBeforeRenameHookForTests(() => { writeFileSync(authPath, external); });
+
+    // Refusal surfaces as MainAuthJsonChangedDuringRefreshError, the existing signal for
+    // "the file moved under us" - the caller retries against the new state rather than
+    // proceeding with a credential it no longer owns.
+    await expect(getValidMainAccountToken({ refreshToken: refreshOk })).rejects.toThrow();
+
+    expect(readFileSync(authPath, "utf8")).toBe(external);
+    expect(readFileSync(authPath, "utf8")).not.toContain("ocx-staged-access");
+  });
+
+  test("a same-bytes replacement with a new inode is still refused", async () => {
+    // The case a content hash cannot see. rename(2) replaces unconditionally, so the
+    // question that matters at the boundary is "is this the same FILE", not "does it hash
+    // the same" - an external writer that rewrote identical bytes still owns the target.
+    const authPath = join(home, "auth.json");
+    seedExpired(authPath);
+    const identical = readFileSync(authPath, "utf8");
+    setMainAuthJsonBeforeRenameHookForTests(() => {
+      // Replace via a distinct file so the inode changes while the bytes do not.
+      const swap = join(home, "swap.json");
+      writeFileSync(swap, identical);
+      renameSync(swap, authPath);
+    });
+
+    await expect(getValidMainAccountToken({ refreshToken: refreshOk })).rejects.toThrow();
+
+    expect(readFileSync(authPath, "utf8")).toBe(identical);
+    expect(readFileSync(authPath, "utf8")).not.toContain("ocx-staged-access");
+  });
+
+  test("the canonical target survives a refused publication", async () => {
+    // A refusal must never leave the credential missing: losing auth.json is worse than
+    // losing the refresh, because Codex CLI then has nothing to authenticate with.
+    const authPath = join(home, "auth.json");
+    seedExpired(authPath);
+    setMainAuthJsonBeforeRenameHookForTests(() => {
+      writeFileSync(authPath, JSON.stringify({ auth_mode: "chatgpt", tokens: { access_token: "other" } }));
+    });
+
+    await expect(getValidMainAccountToken({ refreshToken: refreshOk })).rejects.toThrow();
+
+    expect(existsSync(authPath)).toBe(true);
+    expect(readdirSync(home).filter(name => name.startsWith("auth.json.")).length).toBe(0);
+  });
+
+  test("an uncontested publication still succeeds", async () => {
+    // The guard must not make the ordinary path fail closed.
+    const authPath = join(home, "auth.json");
+    seedExpired(authPath);
+    const token = await getValidMainAccountToken({ refreshToken: refreshOk });
+    expect(token?.accessToken).toBe("ocx-staged-access");
+    expect(readFileSync(authPath, "utf8")).toContain("ocx-staged-access");
+  });
+});