Secure Codex V2 continuity and portable ingress - #71
Conversation
mentatzoe
left a comment
There was a problem hiding this comment.
Reviewed exact head 2d65765 (Claude, non-author of this surface). One finding worth closing while this is still a draft, two smaller observations, and a lot that is genuinely right. Not approving only because the finding below deserves a disposition first; nothing here is architectural.
Reviewer scope: deterministic verification only. No real codex binary, no live adapter, and no installed-wheel run were exercised here — consistent with the PR's own boundaries. Independently reproduced at this head from a clean git archive: full suite 618 OK, 4 documented skips; the new ingress, continuity, and identity paths run under the stubs in test_surfaces.py.
Finding 1 — MEDIUM: the ingress authenticator proves origin but not freshness, and the residual window is the documented recovery path
ChannelIngressAuthenticator.unwrap MACs "nunchi.channel.ingress.v1" || source_id || payload_sha256. There is no nonce, no timestamp, and no one-use tracking in the authorization itself — a captured envelope re-authenticates forever.
I traced where that actually lands before calling it a defect, because most of it is absorbed downstream:
- An exact replay into a live runtime is neutralized by the observation layer:
observe()refuses a seendelivery_idorevent_idasexact-duplicate, and both sets are rebuilt from the durable audit at startup, so the block survives restart. - A signed envelope cannot be retargeted:
room_idsits inside the MAC'd payload, andsource_idis bound to the trusted config.
What remains is precise: operator recovery. docs/platform-v2.md names "a new state directory" as the recovery path for corrupt state. A fresh state directory has an empty dedup index, so every envelope an attacker captured during the previous life re-authenticates and re-enters as a live fact — old instructions, old questions, old room content replayed into a runtime that has just lost its memory of having seen them, at exactly the moment an operator is least suspicious of odd traffic. Freshness is the one property the downstream dedup cannot supply, because recovery deletes the dedup.
The repo's own bar makes this a gap rather than a preference: the outbound Discord HMAC lane requires short-lived, exact-operation authorization with nonces fsynced before dispatch and replay-blocked across restart. The new inbound lane is billed the same way ("HMAC-authenticated ingress") but carries none of that.
Smallest fix: add issued_at and nonce to the authorization object, include both in the MAC material, enforce a bounded freshness window, and keep a durable nonce set within the window — the ack/nonce journal machinery this repo already uses for outputs is the right shape. That closes the recovery-window replay without changing the envelope's outer shape version… though bumping schema_version to 2 would be the honest move since the material changes.
Finding 2 — LOW: _codex_auth_mode infers identity from human-facing CLI wording
Substring-matching "chatgpt" / "api key" / "access token" in codex login status output is fragile across Codex releases and locales. The failure direction is correct — an unrecognized wording maps to unknown, mismatches the pinned auth_mode, and the runtime refuses — so this is availability risk, not integrity risk. But the failure will present as "identity changed" when nothing changed except Codex's copywriting, on the same day an operator upgrades Codex for unrelated reasons. If Codex offers any structured status output (--json or an exit-code contract), prefer it; if not, a comment in the code naming this failure mode will save the operator who hits it an hour.
Observation — the session_mode default flip is right, but its migration story is one sentence short
Defaulting persistent → fresh is the correct direction: persistent continuity now carries binding obligations (exact model, pinned runtime_identity) that a default cannot satisfy, and old explicit persistent configs fail closed with a clear error, which forces the pin — good. The quiet case is the operator whose old config never named session_mode: they silently drop from persistent to fresh on upgrade, their committed session file is ignored without any error, and the only tell is the probe. The README covers the new modes; one explicit line — "configs that omitted session_mode were persistent before this change and are fresh after it; existing committed tasks are not resumed" — would close the surprise.
What I verified and want on the record as sound
- Continuity commits ride host-acceptance receipts and nothing else.
CodexTaskReceiptJournalcommits a staged task only on a durably appendedtransportrecord or aparticipant-hostsilent— the exact invariant shape that survived five adversarial rounds on the Claude Code surface, including the bounded staging (cap 8, oldest-evicted) and the commit-after-super().append()ordering that keeps the pin downstream of durability. - Consume-before-resume is the right answer to a non-transactional resume seam. Moving the committed pin to
inflightbefore Codex can mutate task history means crash, cancellation, malformed output, expiry, and rejection all reset rather than resume a task whose failed turn may have already advanced its internal state. Thereset-requiredstatus with a stated reason and repair is honest operator surface. - Identity is re-attested immediately before every execution, not just at construction: binary path + SHA-256, version, auth mode, and credential binding are all recomputed and compared inside the turn lock. A swapped binary or rotated credential between turns refuses rather than executes.
- The credential binding stores no secret material. The account-id path hashes only non-secret identifiers; the fallback double-hashes. The
auth.jsonread isO_NOFOLLOW, regular-file-checked, and size-bounded. - The backend split keeps exactly one participant per runtime (
len(participant_backends) != 1refuses both-or-neither), native adapters refuseingress_auth, and the generic channel requires it — no unauthenticated generic path survives. - The
UserPromptSubmithook is not a V1 revival. It renders no verdicts and derives no facts; it only refuses raw<channelmarkup and names the authenticated alternative. Its regex is deliberately narrow (<channel+ space/>), which is fine because it is not the boundary — the HMAC requirement is; worth keeping that framing if anyone proposes making the regex cleverer. capability_modelocked to"reduced"with the limitation stated in the probe, rather than implied parity — matches the repo's truthfulness posture.
Boundaries
This review adds no installed, live, or real-runtime evidence; the PR's own "remaining boundaries" section stands. Draft status and merge timing stay with the author and Zoe.
Generated by Claude Code
Outcome
Implements safe Codex task continuity for #62 and a shared authenticated adapter path for #63. It also makes #60's reduced-capability limitation explicit instead of claiming normal Codex parity.
What changed
CODEX_HOME, provider, account, credential scope, authentication mode, and continuity generation.<channel>prompt markup with a usable native-adapter alternative.Why
The previous runner persisted Codex task identity before downstream acceptance and did not bind the authenticated runtime strongly enough. It was also Discord-specific, while generic prompt markup could not establish trustworthy native identity or continuity facts.
Verification
python3 -m unittest— 618 tests passed, 4 skipped.python3 -m evals.verdict_suite.runner --list— all 11 lifecycle scenarios enumerated.git diff --checkpassed.Remaining boundaries
This PR does not claim installed, live, integration, release, or final product acceptance.