Skip to content

fix(v2): fail fast on unreadable routed agent tasks - #283

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
MathiasHeinke:codex/v2-encrypted-task-failfast
Jul 22, 2026
Merged

fix(v2): fail fast on unreadable routed agent tasks#283
lidge-jun merged 1 commit into
lidge-jun:devfrom
MathiasHeinke:codex/v2-encrypted-task-failfast

Conversation

@MathiasHeinke

Copy link
Copy Markdown
Contributor

Summary

Fail fast when a Codex V2 agent_message contains a native ChatGPT Fernet task token but no provider-readable task payload and the selected route is external.

Instead of forwarding an empty task to Grok, Claude, Kimi, or another routed provider, OpenCodex now returns a deterministic, non-retryable HTTP 400 before auth resolution, adapter construction, or provider dispatch. The error directs the caller to plaintext V2 agent-message delivery or a native ChatGPT model.

This is a companion safety fix for #92. It does not claim to recover the missing task body.

Root cause and scope

The native ChatGPT backend can decrypt backend-minted Fernet tokens. An external routed provider cannot. By the time OpenCodex receives this request, the plaintext task is absent, so the proxy has no cryptographically valid way to reconstruct it.

This PR intentionally does not attempt to decrypt or reinterpret that ciphertext. Doing so would require keys OpenCodex does not own and would blur the trust boundary. The actual end-to-end fix remains plaintext V2 task delivery in the Codex client.

The guard is deliberately narrow:

  • only V2 agent_message input is inspected
  • only an exact Fernet-shaped encrypted_content task with no readable payload is rejected
  • native canonical ChatGPT forwarding remains byte-preserving so the backend can decrypt it
  • readable plaintext task content still routes normally
  • the plaintext-in-encrypted_content compatibility path from related PR fix: preserve routed sub-agent task payloads by sanitizing before parse #94 remains intact
  • unrelated encrypted reasoning and compaction content is ignored
  • the response never echoes ciphertext or request-derived content

Why fail fast

Without this guard, the model override can look successful while the routed child receives no actionable task. That can produce misleading worker output and repeated provider-cost attempts. A clear 400 makes the delivery mismatch observable and non-retryable until the upstream client emits plaintext task content.

Verification

  • bun test --isolate tests/multi-agent-compat.test.ts tests/v2-agent-message-failfast.test.ts — 31 pass, 0 fail
  • bun run typecheck — pass
  • bun run privacy:scan — pass
  • git diff --check origin/dev...HEAD — pass

Refs #92. Related: #94.

@lidge-jun

Copy link
Copy Markdown
Owner

Draft review — the fail-fast direction is right: current dev forwards genuine Fernet ciphertext that a non-ChatGPT provider can never decrypt, so the routed child proceeds with only the envelope and no actual task. A pre-network 400 is cheaper and more honest. Guard placement (after model routing, before auth/adapter dispatch, native ChatGPT path untouched) and the test coverage (zero external dispatch, non-retryable 400, no ciphertext reflection, native byte-preserving forward) both look good.

Feedback before marking ready:

  1. readableAgentMessagePayload() joins text parts and discards everything before the first Message Type: line. Please add fixture evidence (real codex-rs captures) that there's no legitimate shape where meaningful task text precedes the envelope — otherwise this could false-reject.
  2. The exact Fernet regex + envelope parser are capture-dependent. Pin the real captures as fixtures (or test comments), and make the intent explicit for array-vs-string content variants and payload-on-same-line cases, so upstream wire-shape drift fails loudly instead of silently not detecting.
  3. Only enforce-target has run — needs the full CI matrix before ready-for-review.

With those locked down this is a good candidate for the #92 mitigation path.

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fail-fast direction is valuable, and pure Fernet-only routed task slots correctly stop with a non-retryable 400 before provider dispatch. However, the current guard still fails open for the mixed shape that existing compatibility logic explicitly produces: plaintext hook preamble plus an encrypted actual task slot.

readablePayloadAfterMarker() counts any text after Payload: as provider-readable. With a normal NEW_TASK envelope followed by [CXC-LEAF-GUARD] ... plaintext and then a Fernet slot, sanitization removes the ciphertext, the guard sees the leaf-guard preamble as readable payload, and the routed provider is called with HTTP 200 even though the actual task body is gone. The captured provider message contains only the envelope and leaf guard. This preserves the core #92 failure mode and contradicts the PR guarantee that unreadable routed tasks stop before dispatch.

Please classify/strip known compatibility preambles before deciding that a task payload is readable, and add the existing mixed-slot fixture as a fail-fast regression. Also pin a real fixture for any plaintext allowed before Message Type:; the current helper can false-reject meaningful pre-envelope text.

Full local current-dev merge verification otherwise passed (3,512 tests, typecheck, privacy scan, GUI lint/build). No malware, backdoor, ciphertext reflection, unsafe I/O, or dependency concern was found.

@lidge-jun
lidge-jun marked this pull request as ready for review July 22, 2026 23:31
@lidge-jun
lidge-jun self-requested a review as a code owner July 22, 2026 23:31
@lidge-jun
lidge-jun merged commit c55633b into lidge-jun:dev Jul 22, 2026
8 checks passed
@lidge-jun

Copy link
Copy Markdown
Owner

Post-merge Sol review note: This PR was merged then reverted from dev after Sol review found a critical bypass:

  • Mixed encrypted slots (e.g. [CXC-LEAF-GUARD] + Fernet token) bypass the guard entirely
  • FERNET_TOKEN_EXACT regex accepts structurally impossible tokens
  • Request-wide scanning can block readable turns due to encrypted history

The fail-fast concept is important for security and will be rebuilt with proper mixed-slot handling and structural Fernet validation.

lidge-jun added a commit that referenced this pull request Jul 23, 2026
1 high blocker: mixed encrypted slots bypass the guard. 4 medium issues
including loose Fernet validation and history-scanning scope.
lidge-jun added a commit that referenced this pull request Jul 23, 2026
Sol reviewed all 10 PRs. Results:
- Merged: #286 (500→400), #296 (Cursor Router), #302 (Kiro hardening)
- Rebuild-on-dev: #255, #283, #293, #301, #298, #299
- Keep open: #249 (Cloudflare tunnel)

Typecheck + full test suite pass (3574/3574) after all merges.
lidge-jun added a commit that referenced this pull request Jul 23, 2026
Detect V2 Fernet-encrypted agent tasks that cannot be decrypted by
non-ChatGPT providers and return 400 early, preventing cost storms.

Fixes the mixed-slot bypass found by Sol review: control preambles
like [CXC-LEAF-GUARD] are no longer treated as actionable task text.

Co-authored-by: Mathias <66031173+MathiasHeinke@users.noreply.github.com>
Based-on: PR #283
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants