Skip to content

codex install: failed install can strand a live credential when Codex concurrently rewrites config.toml #40

Description

@jarodtaylor

Source: U24 (#24) ce-code-review — cross-model Codex adversarial peer, P1@conf75. Disposition: DEFER to the decision-#16 config-write-robustness cluster — but this one warrants a CEO scope call (see options), because it works against #24's own acceptance criterion "a failed install leaves no live credential behind."

Mechanism (verified against undo.ts)

  1. Fresh install mints token T, writes config.toml (write a succeeds, journaled).
  2. A concurrent Codex process rewrites config.toml for its own reasons (settings/trust hashes), preserving our entry — this changes the whole-file hash.
  3. A later install write (b hooks.json / c AGENTS.md) fails (symlink/permission/corruption).
  4. Rollback calls the journal undo, whose identity check refuses on the diverged file — src/configwrite/undo.ts:117 if (currentHash !== entry.postHash) throw … refusing to clobber a later write. rollback catches+logs it best-effort, so our entry (with T) survives.
  5. U8's second backstop (revokeMintedToken deleting codex.token) was removed by U8 follow-up: single-source the Codex credential — gate reads the token from config.toml, retire codex.token #24, so nothing else neutralizes T. readCodexToken still extracts T → the gate keeps accepting a credential from a failed install.

The non-concurrent case is safe and tested (rollback undoes the config write when it is NOT diverged). The gap is specifically failed-install × concurrent-Codex-rewrite.

Why deferred (reachability)

Triple coincidence in a ~ms window: fresh mint + concurrent Codex rewrite between writes a and b/c + a b/c write fault. Manual single-user CLI, pre-U15 no always-on server; a re-run of install (idempotent) or uninstall (targeted removal) cleans it. Same class as the documented first-write journal window already deferred to #16 (src/install/codex.ts:210-219). The in-process Opus adversarial reviewer put it below the merge bar.

Scope options (CEO call — Jarod)

  • A. Defer to the Capture: cold-start tail reads a whole file in one allocation #16 cluster (this issue as-is). Cleanest; keeps the unit subtractive.
  • B. Harden now, small. It does not require reintroducing the deleted apparatus. One boolean mintedFresh = extractCodexToken(existingConfig) === null, and in the rollback catch, when mintedFresh, do a targeted removeKeysIfPresent('mcp_servers.agent-os') gated on the embedded token still equaling T (compare-and-remove) — reusing the uninstaller's own divergence-robust helper. Larger than one line (fully hardening its own failure path overlaps the Capture: cold-start tail reads a whole file in one allocation #16 engine fix), smaller than "re-add provenance."

Not a merge blocker for #24 — but flag for Jarod's decision on A vs B.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    deferredParked work with a promotion trigger

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions