Add Codex ChatGPT account auth

[evandowning]

Summary

• Add [codex] auth = "chatgpt" for Codex account/workspace authentication.
• Install guest Secret Service support and a codex-account wrapper for D-Bus/GNOME Keyring.
• Suppress OPENAI_API_KEY forwarding and auth.json copying in ChatGPT auth mode.
• Document setup, billing behavior, and the proxy incompatibility.

Validation

• rustfmt --edition 2024 src/lib.rs src/backend.rs src/commands/lifecycle.rs src/config.rs src/guest.rs src/lima.rs src/setup.rs
• shfmt -w scripts/guest/codex-account.sh
• shellcheck scripts/guest/codex-account.sh
• git diff --check

Not run here:

• cargo test --lib codex: blocked by crates.io DNS and missing cached anyhow 1.0.104
• taplo format --check: taplo unavailable

[hbrodin]

Review: Codex ChatGPT account auth

Reviewed 9dd65df...ffb362e (1776 lines, 27 files). Eight review passes, then every finding through two adversarial falsification passes in an isolated worktree — 27 raw findings down to 6 inline plus the nits below. Where I could execute something rather than reason about it, I did: the wrapper was run under a real pty in ubuntu:24.04 containers, and each test claim below was mutation-checked by hand.

The gates all pass on ffb362e: cargo fmt --check, cargo clippy --all-targets --all-features -D warnings, taplo format --check, cargo test (1072 lib tests), and shellcheck on the new script. Worth stating because CI could not verify these here.

One finding blocks the feature. scripts/guest/codex-account.sh:136-141 starts the keyring daemon before unlocking it, and the unlock is a no-op once a daemon owns the bus — so auth = "chatgpt" fails on every invocation, on both a fresh and an existing keyring. It is a two-line reorder. Details and the reproduction are inline. Everything else here is a P2 test gap and four P3s.

I have left this as a comment rather than a formal request for changes — the call on the P1 is yours, but I would not expect the feature to work as merged.

Design note

The shape of this is right, and a couple of decisions are better than the obvious alternatives: gating the wrapper on the guest config rather than on an env var keeps codex-yolo working from inside the guest, and routing login/logout bare rather than special-casing them in the wrapper keeps the argv construction in one place. The keyring store with no auth.json fallback is the correct choice for a mode whose whole point is not writing a plaintext token.

Nits — no action required

1. src/guest.rs:497 and :501 (codex_account_script_uses_secret_service) — two of the five substring assertions cannot fail. /usr/local/bin/codex-account (:497) also appears in the script's own cat > and chmod 755 lines, and the bare "dbus-run-session" (:501) also appears in the command -v guard and its die message, so both match regardless of what the wrapper actually does. I confirmed by deleting the real exec dbus-run-session line: all three tests stayed green. The other three (gnome-keyring-daemon --unlock, secret-tool store, secret-tool clear) are unique and do have teeth. Anchoring the weak two on surrounding syntax — exec dbus-run-session -- and cat >/usr/local/bin/codex-account — would fix it.

2. src/backend.rs:1366-1374 — the else { "" } arm at :1372-1374 is dead. Both reads of openai_suppression_reason are already guarded by the same condition: :1398 sits inside if suppress_openai_key, and :1428 is inside a closure whose OPENAI_API_KEY branch is reachable only because that name is pushed to suppressed under the same if. So the sentinel is never observed. Dropping the outer if is a three-line simplification. (No new type needed — docs/code-style.md:111 explicitly says not to demand a newtype for every primitive, and this one is a local string.)

3. src/backend.rs:1763 — "then recreate or restart the VM" overstates what restart does. Only create_and_start copies the template; Lima instances get no provision block from generate_start_template (src/lima.rs:1292-1339 — the only provision: in the file, at :1397, belongs to the throwaway builder VM), so start_existing is a bare limactl start and cannot pick up new guest binaries. Suggest dropping "or restart". Relatedly, none of docs/codex-integration.md, docs/commands.md, or CHANGELOG.md:16 mentions that an existing VM must be destroyed and recreated, not just that the image must be rebuilt — that is the step a user upgrading in place will miss.

4. Two comments still describe the pre-diff condition set. src/backend.rs:2586-2590 documents three rewrite conditions while the body now has four (|| auth.uses_chatgpt_account()). src/commands/lifecycle.rs:1613-1615 says the key "must not re-enter the guest — the host-side proxy holds it", which now also covers ChatGPT mode, where there is no proxy and no host-side holder.

Checked and dismissed

Recording these so they do not resurface in another review: the stale guest auth.json (pre-existing from #411, copy_staged_to_guest is not in this diff, and inert in keyring mode which has no auth.json fallback); the un-exported keyring env (the real daemon prints nothing when XDG_RUNTIME_DIR is set, so there is no value to export); the seven positional parameters on stage_codex_files (already six at merge base, eight functions in src/ already exceed five, and clippy's default threshold is above both); coop setup --rebuild being redundant alongside --image (redundant is not wrong, and it matches existing docs); the || die after --unlock (it fires on any nonzero and is what keeps set -e from aborting silently); run_interactive's exit-code handling (untouched, and shared with coop claude/shell/exec); and libsecret-tools needing universe — the firecracker-ci squashfs has Components: main universe restricted multiverse, so it resolves fine.

[evandowning]

Thanks — this was a genuinely useful review, and the P1 was real. Pushed 09c7944; replies are on each thread. Summary of what changed and the two places I diverged from the suggested fix.

The P1

Reproduced before touching anything, in ubuntu:24.04 under a real pty with the wrapper installed byte-for-byte: both a fresh and an existing keyring exit 1, with gcr-prompter: cannot open display making the mechanism visible.

The suggested two-line reorder does not fix it. I tried it as written and it still fails everywhere, because with start_keyring gone the probe D-Bus-activates the secrets daemon itself — so the unlock is handed to gcr-prompter one step later. The unlock has to happen before anything touches the bus at all. Since dbus-run-session mints a fresh private bus per invocation, there is never a pre-unlocked daemon for a probe to find, so the probe-first check has no case left to serve and the unlock is now unconditional. Verified: fresh keyring, existing keyring, wrong password, and api_key passthrough all behave correctly.

The nits

1. Weak assertions — fixed, anchored on exec dbus-run-session -- and cat >/usr/local/bin/codex-account. Confirmed both now fail when the corresponding line is deleted. Added codex_account_script_unlocks_before_touching_the_bus to pin the P1 ordering (compares call-site indices rather than requiring adjacency) and codex_account_script_captures_probe_stderr to pin the 2>&1 >/dev/null order, which is load-bearing and easy to "tidy" into always-empty.
2. Dead else { "" } — dropped, exactly as described.
3. "or restart" — fixed, and this turned out bigger than the wording. coop restore <vm> --image <image> replaces a stopped instance's disk in place and keeps the name, index, IP, and workspace association, so recommending destroy as the only remedy was both overstated and needlessly destructive (destroy_instance rm -rfs the instance dir). The error message and the docs now lead with restore. The "existing VM needs more than a rebuild" point is now in codex-integration.md, getting-started.md, and the CHANGELOG.
4. Two stale comments — both updated.

One thing you did not flag, which your P3 led me to

Recording codex_keyring_materialized before copy_codex_config would have persisted it even when ensure_codex_account_guest_support bailed — permanently forcing a full guest config rewrite on every later boot of a VM the key never reached, including the read_codex_plugin_state round-trip whose warn-and-continue drops the guest's own plugin tables. It is now written only after the guest config lands.

Verification

cargo fmt --check, cargo clippy --all-targets --all-features -D warnings, cargo test (1084 lib tests, up from 1072), and shellcheck all pass. Every new test was hand-mutated to confirm it fails when the behavior is removed — including the one that matters most for your P2: the guard's four original tests all drove the conflict through cfg.proxy.openai, which CoopConfig::validate already rejects, so none of them exercised the per-VM proxy.json override the guard actually exists for. There is now a test that writes ProxyState to the instance dir with config [proxy] empty; it dies if the resolution is reduced to cfg.proxy.openai.is_some().

taplo format --check was not run (not installed here, and no TOML changed). The integration suite has not run on either backend.

Follow-ups I deliberately did not do here

• No integration coverage for keyring mode. test_codex_account_auth_support still exercises only the api_key passthrough branch, and nothing end-to-end covers the chatgpt→api_key rewrite. Both are straightforward in the existing phase shapes, but I can't run tests/integration.sh from here and would rather not add unverified phases.
• stage_codex_files is now at eight parameters with an #[expect(clippy::too_many_arguments)], three of them bools. It shares five parameters with codex_config_needs_rewrite under a comment saying they must stay in sync, which a shared inputs struct would make structural. That is a refactor, so per the one-logical-change rule it does not belong in this PR.
• The flag is never cleared, so a VM that has ever used chatgpt rewrites its Codex config on every boot thereafter. Same tradeoff codex_materialized already makes; flagging it as deliberate rather than overlooked.

[evandowning]

Rebased onto main (clean, no conflicts) and did a second full review pass over the whole PR rather than just the last diff. Force-pushed as 03675ae. Three commits of fixes on top of the rebase; everything below was reproduced or mutation-checked before being changed.

Three ways a plaintext token could still reach the guest disk

The mode exists to keep a plaintext ChatGPT credential off the guest disk. All three of these defeated that.

1. coop codex never checked the guest was actually in keyring mode. It verified the Secret Service packages but not that cli_auth_credentials_store = "keyring" had reached the guest ~/.codex/config.toml. The wrapper gates on that file, so enabling the mode against an already-running VM — where agent bootstrap has not re-run — meant the wrapper passed straight through to plain Codex and codex login wrote the token in the clear, with no warning on any path. ensure_codex_keyring_configured now fails closed and names the fix.

2. A stale guest auth.json was never removed. Dropping it from the staged file set only stops coop copying a new one — copy_staged_to_guest is mkdir -p + scp and deletes nothing. I grepped every auth.json reference to confirm. A VM that had run in api_key mode kept its previously-copied plaintext credential indefinitely after switching. Each ChatGPT-mode bootstrap now removes it, which also covers the file a --no-agents login leaves behind.

   (@hbrodin — this is the one you checked and dismissed as "inert". That is right about Codex's read precedence, but the file stays readable on the guest disk, which is the actual delta this mode sells. Flagging since it reverses your call.)

3. api_key mode dropped the keyring key only incidentally, by rebuilding from the host base. A host config.toml that itself sets cli_auth_credentials_store was copied straight back, leaving the guest demanding a password it no longer needed. Now removed explicitly.

Two bugs in the wrapper, both reproduced in containers

4. Nested invocation was broken — a regression from my previous pass. COOP_CODEX_ACCOUNT_DBUS=1 is exported and never unset, so an in-guest agent shelling out to codex-account/codex-yolo skipped the D-Bus re-exec and hit the unlock on a bus that already had a daemon: the same gcr-prompter trap. Before the unlock-first change the probe absorbed this; afterwards nested calls always failed. A second marker lets a nested call reuse the session — exit 1 → exit 0.

5. The wrapper eval'd the daemon's stdout verbatim, so a non-assignment line ran as a command. A GLib diagnostic killed it with eval: unexpected EOF, exit 2. Only NAME=value lines are taken now — and exported, which they were not, so they never reached Codex anyway.

Rewrite churn this mode forces

6. codex_keyring_materialized is now cleared as well as set. Latching it made every boot after a switch back to api_key pay a full config rewrite plus the read_codex_plugin_state round-trip — precisely the hazard the comment justifying the deferred set warns about.

7. [projects.*] joins marketplaces/plugins in the preserved set. ChatGPT mode rewrites the guest config.toml every boot and only those two tables were carried across, so Codex's workspace-trust records were wiped on each restart — users would have re-approved trust after every coop stop/start. model/model_provider are deliberately still not preserved, since local-model and proxy routing overwrite them on purpose.

Smaller

8. The keyring unlock is now timeout-bounded like the probe already was — it runs in a command substitution, which waits for EOF rather than exit.
9. --no-agents read model.json eagerly through ? (also mine), so an unreadable file aborted a start that never opens it. Now lazy, behind the cheap terms.
10. The lifecycle comment no longer claims the VM still starts when the proxy/auth conflict fires. Agent bootstrap is one of the entry points that fails hard, so coop start aborts too — only --no-agents brings it up.
11. Docs: coop exec never routes through the wrapper on its own, so the TTY example described a path that does not exist.

Tests

A verified cargo-mutants survivor at src/config.rs:2068: && → || passed the entire suite while making both plain auth = "chatgpt" and a plain [proxy.openai] hard config errors — the two commonest configurations this feature touches. Two tests; the sweep over validate is now clean apart from one pre-existing miss from #212.

Guest-script assertions for the D-Bus recursion guard (breaking it fork-bombs the guest, and nothing noticed), inherited-session reuse, both probe timeouts and the unlock timeout, and the TTY guard.

Integration now enters keyring mode — via CODEX_HOME, needing no reconfigured VM — instead of only asserting the passthrough branch. Previously the cli_auth_credentials_store regex was never executed on any layer, so a regex matching = "file" would have shipped green. Plus a pre-VM check that validate rejects chatgpt + [proxy.openai]. Every helper used was verified to exist first, since that file has no shellcheck gate.

Gates: cargo fmt --check, cargo clippy --all-targets --all-features -D warnings, cargo test (1106, up from 1072 at the start of review), taplo format --check, shellcheck, bash -n. The wrapper was exercised in ubuntu:24.04 across fresh keyring, existing keyring, wrong password, empty password, mismatched confirmation, missing binary, non-TTY, exit-code propagation, nested invocation, and codex-yolo login against the real codex-cli 0.152.0. The integration suite has not been run on either backend — that still needs Lima and Firecracker.

One thing I did not change, for you to decide

Against the threat model in docs/trust-model.md the keyring buys less than it appears. The password is typed through the untrusted guest on every launch — dbus-run-session mints a fresh bus each time, so it can never be amortized, and I confirmed it prompts on every invocation. A compromised guest harvests it on the first run. What the mode genuinely buys is encryption at rest on the guest disk, which is worth having but is a narrower claim than "keeps credentials safe from the agent". Worth stating explicitly in the PR description so the per-launch prompt and the fleet-wide image rebuild are a recorded trade rather than an implicit one.

[evandowning]

Re-rebased onto main after #440 landed, now at f1826ed. Worth calling out one thing, because a file quietly left this PR's diff.

#440 rewrote the README down to four sections and removed the feature list this PR had been editing, so both of my README hunks conflicted. I resolved by taking main's README verbatim — it is now byte-identical to main and no longer appears in this PR's diff at all. Re-adding the ChatGPT blurb would have fought the intent of a commit that had just landed, and main's Documentation section already links Codex integration. The mode stays documented in docs/getting-started.md and docs/codex-integration.md, both of which this PR still edits. If you would rather it appear in the README, it is a one-line addition to Usage.

Also worth recording, since it was invisible: the previous force-push produced no CI run at all. check, deny, taplo and zizmor were absent from the checks list rather than failing — GitHub could not compute a merge ref while the PR was conflicting, so the pull_request-triggered CI never fired, and only the push-triggered CodeQL workflow ran. A green-looking checks list with the important jobs simply missing is an easy thing to read as "passing". They are all present and passing on f1826ed.

Still outstanding: the integration suite has not been run on either backend.

[hbrodin]

Approving. Thank you for this — the revision work here was genuinely excellent, and unusually rigorous.

I reviewed the ~900 lines added since my pass at ffb362e (a16cc7c, 6ef374f, f1826ed), which had had no independent review, then put every finding I raised through an adversarial falsification pass. Most of mine did not survive it, so rather than post a long list I want to record what I checked and what actually stands.

Your fixes hold up

All six findings from the first pass are properly resolved, and several went further than I suggested. You were right that my two-line reorder for the P1 did not fix it — the probe D-Bus-activates the daemon itself, so the unlock has to precede anything touching the bus. Making it unconditional and deleting start_keyring/eval_env_output is the correct shape. Turning the --no-agents check into no_agents_skips_codex_keyring with a lazy closure is better than the inline warning I proposed. And you were right to reverse my "inert" dismissal of the stale guest auth.json: read precedence was not the point, on-disk readability was, and that is the delta this mode sells.

I verified the delta by executing the wrapper rather than reading it — extracted from the heredoc, stubbed CODEX_BIN/secret-tool/gnome-keyring-daemon, driven under a real pty. Passthrough gating is correct (a commented-out key and = "file" both stay in passthrough), exit codes propagate through both execs, the TTY and missing-tool guards fire, the nested/inherited-session path does not re-prompt, fresh-vs-existing keyring prompting is right, and the daemon env now genuinely reaches Codex. I also confirmed the eval removal is a real fix by feeding the stub daemon a touch command on stdout — it was not executed.

Also checked and clean: .cargo/mutants.toml correctly needs no update (every new shell-out landed in the already-excluded src/backend.rs, and the one new scoped-module helper is pure and tested); TEMPLATE_VERSION 1→2 and required_guest_binaries 4→8 are the right cross-file sync; codex-yolo is rerouted on both backends with assertions on each; config.example.toml, docs/images-and-profiles.md, and the CHANGELOG are all in step. Leaving the README byte-identical to main post-#440 was the right call.

One follow-up, filed as #441

CODEX_HOME diverges between the wrapper (${CODEX_HOME:-$HOME/.codex}) and the two new host-side halves, which hardcode ~/.codex — so the guard can pass while the wrapper falls through to plain Codex and codex login writes a plaintext token. Filed rather than requested here, to keep this PR to one logical change.

Worth flagging one thing from that investigation: aligning the host grep to ${CODEX_HOME:-$HOME/.codex} does not fix it, because exec_ok builds a bare ssh with no session env while run_interactive uses .envs(...) + SendEnv. The fix belongs in the wrapper. Details and a suggested six-line patch are in the issue.

That issue also notes the docs/trust-model.md sentence asserting coop codex "refuses to launch when the guest config does not actually select the keyring store" is not true under CODEX_HOME — worth either implementing the guard or softening the claim.

Findings I withdrew

Recording these so they do not resurface:

• Wrapper aborting on a daemon diagnostic. I believed the [A-Za-z_]*=* filter could match a GLib line containing = and kill the wrapper via set -e after the password was typed. It cannot with the real daemon: gnome-keyring's stdout is env-lines-only by upstream design (its own test says so), GLib warnings go to stderr, and the actual format leads with (pid): so the glob never fires. Upstream's own PAM module applies a weaker filter to the same stream. Your filter is fine.
• codex_keyring_materialized stale across coop restore — codex_bootstrap_needed is unconditionally true under chatgpt auth, so a normal start re-materializes the key regardless, and cmd_restore is not touched by this PR.
• restore vs destroy wording in codex-integration.md — I misread it; the preceding sentence already establishes that replacing the disk is the point, and the contrast is instance identity.
• stage_codex_files_drops_keyring_store_when_switching_back_to_api_key — the drop is pinned with teeth by the sibling host-supplied-config test; the tautological assertion is redundant, not a gap.
• Coverage of the new needs_codex term — I applied a standard docs/testing.md explicitly waives for backend.rs.
• ensure_codex_keyring_configured conflating SSH failure with "not configured" — pre-existing house pattern, same as the codex_bin/claude_bin checks on main.
• COOP_CODEX_ACCOUNT_UNLOCKED surviving the re-exec — mechanism is real but unreachable; DBUS=1 is always exported first.

Nits, none blocking

• src/backend.rs — the comment introducing the suppressed collection still opens "In proxy mode…", but suppress_openai_key now covers ChatGPT mode too; the suppression_reason closure right below exists because there are two reasons. The two sibling comments were updated, this one was missed.
• codex_keyring_not_configured_message is the only pub fn in backend.rs with no cross-module caller; its direct sibling codex_missing_guest_cli_message is private. No impact (mod backend; is private), just consistency.
• docs/configuration.md — the golden-image Secret Service paragraph sits inside the ## proxy section, where "this mode" reads as the proxy. docs/images-and-profiles.md in this same PR already carries the same explanation, correctly placed.
• docs/getting-started.md — the inline coop restore sentence is missing the stop/start either side, which codex-integration.md gets right. Prose rather than a fenced recipe, so minor, but a reader who pastes it hits as_topped-style errors.
• In codex_keyring_not_configured_message_names_the_recovery, the contains("cli_auth_credentials_store") disjunct is dead — the message never names the key. Naming it would help a user inspecting the file by hand.

One request

Per CONTRIBUTING.md step 3, please run ./tests/run-integration.sh and --remote on the other backend when you have a machine for it. tests/integration.sh is not run in CI, so test_codex_account_auth_support has not executed anywhere yet, and the wrapper is the riskiest surface in this change. Not gating on it — #403 and #249 both merged with the same disclosure — but it is worth doing before the next release.

Finally: your point about the mode's security value is right and I would take you up on it. The keyring password is typed through the untrusted guest on every launch and dbus-run-session mints a fresh bus each time, so it is never amortized. What this buys is encryption at rest on the guest disk plus avoiding API-key billing, not protection from a compromised guest. docs/trust-model.md is already candid about the refresh token; the only line I would soften is "Two guardrails close the gaps that leaves," since only coop codex can fail closed — codex-account/codex-yolo structurally cannot.

Nice work. Merging.