fix(codex): keep two host-lane records that a failed read used to destroy (STA-4735) (#15289)

`snapshotCodexRuntimeHookTrustProvenance` rebuilt `.orca-hook-trust-provenance.json`
from the current `config.toml` on every install and refresh, including when the
existing record could not be read. That record is the only thing separating a
trust entry Orca wrote from one the user approved inside Codex, so rewriting it
after a failed read stamps the approval as Orca-written — and
`promoteCodexRuntimeHookApprovalsToSystem`, which runs earlier in the same pass
and had already bailed on the same unreadable file, then skips it on every
later pass too. One denied read, permanent loss.

Only the unreadable case is preserved. A malformed or absent record is still
rebuilt, because resetting those IS the intent; conflating the two would wedge a
user on a corrupt file forever.

`fileContentsEqual` returned `false` from a bare `catch`, so "I could not read
this" reached `writeRuntimeAuth` as "these differ" and sent it to the
unconditional write below — replacing a refresh token Codex may have rotated a
moment earlier with Orca's stale copy. It now reports the difference only when
the bytes were actually compared, and the caller refuses instead.

`fileContentsMatchExpected`'s `!existsSync` has the same collapse but is left
alone deliberately: the write it guards is `writeFileAtomicallyIfUnchanged`,
whose rename-and-compare re-checks the real file and refuses on its own, so
classifying there would add a guard no test can drive. Noted in a comment.

The three `writeRuntimeAuthAtPath` call sites have the same overwrite shape but
are all on the WSL lane, which STA-4606 restructures; refusing there without its
lane bookkeeping would set a baseline for a write that never happened.

src/main/codex-accounts/runtime-home-service.ts

@@ -16,6 +16,7 @@ import {
   symlinkSync,
   unlinkSync
 } from 'node:fs'
+import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence'
 import { execFileSync } from 'node:child_process'
 import {
   dirname,
@@ -2011,7 +2012,15 @@ export class CodexRuntimeHomeService {
     }
     const provenance: CodexSharedRuntimeAuthProvenance =
       owner.owner === 'system-default' ? { owner: 'system-default', authJson: contents } : owner
-    const runtimeAuthAlreadyMatches = this.fileContentsEqual(runtimeAuthPath, contents)
+    const runtimeAuthComparison = this.compareFileContents(runtimeAuthPath, contents)
+    if (runtimeAuthComparison === null) {
+      // Why: an unreadable runtime auth.json may hold a token Codex rotated a
+      // moment ago. Treating "could not read" as "differs" sent execution to the
+      // unconditional write below, consuming that rotation and logging the user
+      // out for good. Refuse; the next sync retries.
+      return false
+    }
+    const runtimeAuthAlreadyMatches = runtimeAuthComparison
     if (
       runtimeAuthAlreadyMatches &&
       this.sharedRuntimeAuthProvenanceMatches(
@@ -2059,16 +2068,31 @@ export class CodexRuntimeHomeService {
     writeFileAtomically(authPath, contents, { mode: 0o600 })
   }
 
-  private fileContentsEqual(targetPath: string, contents: string): boolean {
+  /**
+   * `true`/`false` only when the bytes were actually read; `null` when the file
+   * could not be read at all. The old `catch { return false }` reported "these
+   * differ" for a file nobody could open, and every caller reads that as
+   * permission to write.
+   */
+  private compareFileContents(targetPath: string, contents: string): boolean | null {
     try {
-      return existsSync(targetPath) && readFileSync(targetPath, 'utf-8') === contents
-    } catch {
-      return false
+      return readFileSync(targetPath, 'utf-8') === contents
+    } catch (error) {
+      return isDefinitiveAbsence(error) ? false : null
     }
   }
 
+  private fileContentsEqual(targetPath: string, contents: string): boolean {
+    return this.compareFileContents(targetPath, contents) === true
+  }
+
   private fileContentsMatchExpected(targetPath: string, expectedContents: string | null): boolean {
     if (expectedContents === null) {
+      // Why: `!existsSync` does report `true` for a locked file, but this branch
+      // is not where that matters — the write it guards is
+      // `writeFileAtomicallyIfUnchanged`, whose rename-and-compare re-checks the
+      // real file and refuses on its own. Classifying here would be a guard no
+      // test can drive.
       return !existsSync(targetPath)
     }
     return this.fileContentsEqual(targetPath, expectedContents)

src/main/codex-accounts/sta-4735-unreadable-host-lane-writes.test.ts

@@ -0,0 +1,160 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type * as NodeFs from 'node:fs'
+import { join } from 'node:path'
+import { createSettings } from './runtime-home-settings-test-fixtures'
+import {
+  createCodexAccountRecord,
+  createCodexAuthJson,
+  createManagedAuth,
+  createStore,
+  getRuntimeCodexAuthPath,
+  setupRuntimeHomeTest,
+  teardownRuntimeHomeTest,
+  testState
+} from './runtime-home-service-test-harness'
+
+// STA-4735: two host-lane reads still answered "absent" from a read that had
+// only failed, and both answers authorised a write over live data.
+
+const denials = vi.hoisted(() => {
+  const state = {
+    paths: new Set<string>(),
+    reads: new Map<string, number>(),
+    deny(path: string): void {
+      state.paths.add(path)
+    },
+    release(path: string): void {
+      state.paths.delete(path)
+    },
+    readsFor(path: string): number {
+      return state.reads.get(path) ?? 0
+    },
+    reset(): void {
+      state.paths.clear()
+      state.reads.clear()
+    },
+    check(target: unknown, syscall: string): void {
+      if (typeof target !== 'string' || !state.paths.has(target)) {
+        return
+      }
+      state.reads.set(target, (state.reads.get(target) ?? 0) + 1)
+      const error: NodeJS.ErrnoException = new Error(
+        `EPERM: operation not permitted, ${syscall} '${target}'`
+      )
+      error.code = 'EPERM'
+      error.errno = -4048
+      error.syscall = syscall
+      error.path = target
+      throw error
+    }
+  }
+  return state
+})
+
+vi.mock('node:fs', async (importOriginal) => {
+  const actual = await importOriginal<typeof NodeFs>()
+  const guard = (fn: unknown, syscall: string): unknown => {
+    const original = fn as (...args: unknown[]) => unknown
+    const wrapped = (...args: unknown[]): unknown => {
+      denials.check(args[0], syscall)
+      return original(...args)
+    }
+    return Object.assign(wrapped, original)
+  }
+  const patched: Record<string, unknown> = {
+    ...actual,
+    readFileSync: guard(actual.readFileSync, 'read'),
+    lstatSync: guard(actual.lstatSync, 'lstat'),
+    statSync: guard(actual.statSync, 'stat'),
+    existsSync: Object.assign(
+      (...args: unknown[]): boolean =>
+        typeof args[0] === 'string' && denials.paths.has(args[0])
+          ? false
+          : actual.existsSync(args[0] as string),
+      actual.existsSync
+    )
+  }
+  return { ...patched, default: patched }
+})
+
+vi.mock('electron', () => ({ app: { getPath: () => testState.userDataDir } }))
+
+vi.mock('node:os', async () => {
+  const actual = await vi.importActual<typeof import('node:os')>('node:os') // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.importActual requires inline import()
+  return { ...actual, homedir: () => testState.fakeHomeDir }
+})
+
+const realFs = await vi.importActual<typeof NodeFs>('node:fs')
+
+type RuntimeAuthWriter = {
+  writeRuntimeAuth(
+    contents: string,
+    owner: { owner: 'system-default' } | { owner: 'managed'; accountId: string },
+    options?: { expectedContents: string | null }
+  ): boolean
+}
+
+describe('STA-4735 an unreadable runtime auth.json must not be written over', () => {
+  beforeEach(() => {
+    denials.reset()
+    setupRuntimeHomeTest()
+  })
+
+  afterEach(() => {
+    denials.reset()
+    teardownRuntimeHomeTest()
+  })
+
+  async function createService(): Promise<RuntimeAuthWriter> {
+    const managedHomePath = createManagedAuth(
+      testState.userDataDir,
+      'account-1',
+      createCodexAuthJson('user@example.com', 'acct-1', 'refresh-1')
+    )
+    const store = createStore(
+      createSettings({
+        codexManagedAccounts: [
+          createCodexAccountRecord('account-1', 'user@example.com', 'acct-1', managedHomePath)
+        ],
+        activeCodexManagedAccountId: 'account-1',
+        activeCodexManagedAccountIdsByRuntime: { host: 'account-1', wsl: {} }
+      })
+    )
+    const { CodexRuntimeHomeService } = await import('./runtime-home-service')
+    return new CodexRuntimeHomeService(store as never) as unknown as RuntimeAuthWriter
+  }
+
+  it('refuses the write instead of overwriting a token it could not read', async () => {
+    const service = await createService()
+    const runtimeAuthPath = getRuntimeCodexAuthPath()
+    const ROTATED = createCodexAuthJson('user@example.com', 'acct-1', 'refresh-rotated-by-codex')
+    const STALE = createCodexAuthJson('user@example.com', 'acct-1', 'refresh-stale-orca-copy')
+    realFs.mkdirSync(join(runtimeAuthPath, '..'), { recursive: true })
+    realFs.writeFileSync(runtimeAuthPath, ROTATED, 'utf-8')
+
+    denials.deny(runtimeAuthPath)
+    const wrote = service.writeRuntimeAuth(STALE, { owner: 'managed', accountId: 'account-1' })
+
+    // The fault really was consumed by the code under test.
+    expect(denials.readsFor(runtimeAuthPath)).toBeGreaterThan(0)
+    // THE FIX. Before it, "could not read" counted as "differs" and the write
+    // below replaced a freshly rotated refresh token with Orca's stale copy.
+    expect(wrote).toBe(false)
+    expect(realFs.readFileSync(runtimeAuthPath, 'utf-8')).toBe(ROTATED)
+  })
+
+  it('still writes once the file is readable again', async () => {
+    const service = await createService()
+    const runtimeAuthPath = getRuntimeCodexAuthPath()
+    const NEXT = createCodexAuthJson('user@example.com', 'acct-1', 'refresh-2')
+    realFs.mkdirSync(join(runtimeAuthPath, '..'), { recursive: true })
+    realFs.writeFileSync(runtimeAuthPath, 'stale\n', 'utf-8')
+
+    // Why: the anchor. A fix that simply stopped writing would pass the test
+    // above and break every credential sync.
+    const wrote = service.writeRuntimeAuth(NEXT, { owner: 'managed', accountId: 'account-1' })
+
+    expect(wrote).toBe(true)
+    expect(realFs.readFileSync(runtimeAuthPath, 'utf-8')).toBe(NEXT)
+  })
+})

src/main/codex/hook-trust-promotion.ts

@@ -1,5 +1,6 @@
 import { existsSync, readFileSync, writeFileSync } from 'node:fs'
 import { join } from 'node:path'
+import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence'
 import {
   createManagedCommandMatcher,
   readHooksJson,
@@ -43,15 +44,23 @@ function getProvenancePath(runtimeHomePath: string): string {
   return join(runtimeHomePath, '.orca-hook-trust-provenance.json')
 }
 
+/**
+ * `null` means "no usable provenance": genuinely absent, or present but
+ * malformed, where rebuilding it IS the intent. It deliberately does NOT cover
+ * a file that could not be read — see `provenanceIsUnreadable`.
+ */
 function readHookTrustProvenance(
   runtimeHomePath: string
 ): Map<string, HookTrustProvenanceEntry> | null {
   const provenancePath = getProvenancePath(runtimeHomePath)
-  if (!existsSync(provenancePath)) {
+  let rawProvenance: string
+  try {
+    rawProvenance = readFileSync(provenancePath, 'utf-8')
+  } catch {
     return null
   }
   try {
-    const parsed: unknown = JSON.parse(readFileSync(provenancePath, 'utf-8'))
+    const parsed: unknown = JSON.parse(rawProvenance)
     if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
       return null
     }
@@ -79,9 +88,29 @@ function readHookTrustProvenance(
  * install/refresh, so the next launch can tell "entry Orca wrote" apart from
  * "entry Codex wrote after a user approval". Call after all trust writes.
  */
+/**
+ * Why: this record is the only thing that tells a later launch which
+ * `config.toml` trust entries Orca wrote apart from which the user approved
+ * inside Codex. Overwriting it from the current config state after a failed
+ * read stamps the user's approval as Orca-written, and promotion then skips it
+ * forever — a permanent loss from one unreadable file. Keep the old record and
+ * let the next pass, which can read it, do the comparison.
+ */
+function provenanceIsUnreadable(provenancePath: string): boolean {
+  try {
+    readFileSync(provenancePath, 'utf-8')
+    return false
+  } catch (error) {
+    return !isDefinitiveAbsence(error)
+  }
+}
+
 export function snapshotCodexRuntimeHookTrustProvenance(
   runtimeHomePath: string = getOrcaManagedCodexHomePath()
 ): void {
+  if (provenanceIsUnreadable(getProvenancePath(runtimeHomePath))) {
+    return
+  }
   try {
     const runtimeHooksPath = join(runtimeHomePath, 'hooks.json')
     const canonicalRuntimeHooksPath = getCodexExplicitHomeHookSourcePath(runtimeHooksPath)

src/main/codex/sta-4735-hook-trust-provenance-overwrite.test.ts

@@ -0,0 +1,151 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type * as NodeFs from 'node:fs'
+import type * as NodeOs from 'node:os'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+
+// STA-4735: the hook-trust provenance file is the only record of which
+// config.toml trust entries Orca wrote versus which the user approved inside
+// Codex. An unreadable one was rebuilt from the current config on the same
+// pass, stamping the user's approval as Orca-written — after which promotion
+// skips it forever.
+
+const denials = vi.hoisted(() => {
+  const state = {
+    paths: new Set<string>(),
+    deny(path: string): void {
+      state.paths.add(path)
+    },
+    reset(): void {
+      state.paths.clear()
+    },
+    check(target: unknown, syscall: string): void {
+      if (typeof target !== 'string' || !state.paths.has(target)) {
+        return
+      }
+      const error: NodeJS.ErrnoException = new Error(
+        `EPERM: operation not permitted, ${syscall} '${target}'`
+      )
+      error.code = 'EPERM'
+      error.errno = -4048
+      error.syscall = syscall
+      error.path = target
+      throw error
+    }
+  }
+  return state
+})
+
+vi.mock('node:fs', async (importOriginal) => {
+  const actual = await importOriginal<typeof NodeFs>()
+  const guard = (fn: unknown, syscall: string): unknown => {
+    const original = fn as (...args: unknown[]) => unknown
+    const wrapped = (...args: unknown[]): unknown => {
+      denials.check(args[0], syscall)
+      return original(...args)
+    }
+    return Object.assign(wrapped, original)
+  }
+  const patched: Record<string, unknown> = {
+    ...actual,
+    readFileSync: guard(actual.readFileSync, 'read'),
+    existsSync: Object.assign(
+      (...args: unknown[]): boolean =>
+        typeof args[0] === 'string' && denials.paths.has(args[0])
+          ? false
+          : actual.existsSync(args[0] as string),
+      actual.existsSync
+    )
+  }
+  return { ...patched, default: patched }
+})
+
+const { getPathMock, homedirMock } = vi.hoisted(() => ({
+  getPathMock: vi.fn<(name: string) => string>(),
+  homedirMock: vi.fn<() => string>()
+}))
+
+vi.mock('electron', () => ({ app: { getPath: getPathMock } }))
+
+vi.mock('node:os', async () => {
+  const actual = await vi.importActual<typeof NodeOs>('node:os')
+  return { ...actual, homedir: homedirMock }
+})
+
+const realFs = await vi.importActual<typeof NodeFs>('node:fs')
+const { snapshotCodexRuntimeHookTrustProvenance } = await import('./hook-trust-promotion')
+
+const PROVENANCE_ENTRY = 'orca-hooks:stop:0:0'
+let fakeHomeDir: string
+let userDataDir: string
+let runtimeHomePath: string
+
+const provenancePath = (): string => join(runtimeHomePath, '.orca-hook-trust-provenance.json')
+
+function seedRecordedProvenance(): string {
+  const contents = `${JSON.stringify(
+    { version: 1, entries: { [PROVENANCE_ENTRY]: { trustedHash: 'sha256:orca', enabled: true } } },
+    null,
+    2
+  )}\n`
+  realFs.writeFileSync(provenancePath(), contents, 'utf-8')
+  return contents
+}
+
+beforeEach(() => {
+  denials.reset()
+  fakeHomeDir = realFs.mkdtempSync(join(tmpdir(), 'orca-sta4735-home-'))
+  userDataDir = realFs.mkdtempSync(join(tmpdir(), 'orca-sta4735-data-'))
+  runtimeHomePath = join(userDataDir, 'codex-runtime-home', 'home')
+  homedirMock.mockReturnValue(fakeHomeDir)
+  getPathMock.mockImplementation((name: string) => {
+    if (name === 'userData') {
+      return userDataDir
+    }
+    throw new Error(`unexpected app.getPath(${name})`)
+  })
+  realFs.mkdirSync(runtimeHomePath, { recursive: true })
+  realFs.mkdirSync(join(fakeHomeDir, '.codex'), { recursive: true })
+  realFs.writeFileSync(join(runtimeHomePath, 'config.toml'), 'model = "m"\n', 'utf-8')
+})
+
+afterEach(() => {
+  denials.reset()
+  realFs.rmSync(fakeHomeDir, { recursive: true, force: true })
+  realFs.rmSync(userDataDir, { recursive: true, force: true })
+  vi.clearAllMocks()
+})
+
+describe('STA-4735 snapshotCodexRuntimeHookTrustProvenance', () => {
+  it('leaves a provenance record it could not read exactly as it found it', () => {
+    const recorded = seedRecordedProvenance()
+    denials.deny(provenancePath())
+
+    snapshotCodexRuntimeHookTrustProvenance(runtimeHomePath)
+
+    // Before the fix this rewrote the file from the current config.toml, which
+    // holds no record of what Orca wrote — so a user approval made since the
+    // last pass was permanently reclassified as Orca's own write.
+    expect(realFs.readFileSync(provenancePath(), 'utf-8')).toBe(recorded)
+  })
+
+  it('still records a provenance file that does not exist yet', () => {
+    // Why: writing the first snapshot is this function's whole job. A fix that
+    // refused whenever the read failed would never seed one.
+    snapshotCodexRuntimeHookTrustProvenance(runtimeHomePath)
+
+    expect(realFs.existsSync(provenancePath())).toBe(true)
+    expect(JSON.parse(realFs.readFileSync(provenancePath(), 'utf-8'))).toMatchObject({ version: 1 })
+  })
+
+  it('still replaces a provenance file that is present but malformed', () => {
+    // Why: a corrupt record carries no information, and resetting it IS the
+    // intent. Only the unreadable case may be preserved — conflating the two
+    // would wedge a user on a broken file forever.
+    realFs.writeFileSync(provenancePath(), '{ not json', 'utf-8')
+
+    snapshotCodexRuntimeHookTrustProvenance(runtimeHomePath)
+
+    expect(JSON.parse(realFs.readFileSync(provenancePath(), 'utf-8'))).toMatchObject({ version: 1 })
+  })
+})