Skip to content

fix(responses): reject Fernet-shaped agent plaintext - #5239

Merged
lidge-jun merged 1 commit into
devfrom
fix/5236-fernet-shaped-agent-plaintext
Sep 20, 2026
Merged

lidge-jun merged 1 commit into
devfrom
fix/5236-fernet-shaped-agent-plaintext

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Sep 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • require canonical key-independent Fernet structure before unknown full-history encrypted_content is trusted on the final ChatGPT-native route
  • preserve the existing third-party confidentiality boundary: ambiguous encoded slots stay opaque until routing, then noncanonical routes omit them rather than exposing possible truncated ciphertext
  • recognize the exact decrypt/decode rejection when WebSocket/SSE emits it through a detail envelope, and route it through the existing single-shot, request-budgeted sanitize-and-rebuild path
  • document the transport contract and decision record

Fixes #5236.

Why this shape

The reported 195-character gAAAA... model output matched the old length/alphabet heuristic but is not even a canonical Fernet token. Applying the strict rule before routing would be unsafe in the other direction: a malformed or truncated real ciphertext slot could then be forwarded as plaintext to a third-party provider. This change therefore keeps the conservative pre-route hold, applies strict classification only after the canonical native destination is known, and leaves routed-provider omission behavior intact.

The recovery guard remains single-shot across the rebuilt send. A repeated rejection is surfaced; it cannot loop.

Verification

  • focused response/agent suites: 283 pass, 0 fail
  • bun run typecheck
  • bun run structure:check
  • file-size ratchet: 9 pass, 0 fail
  • protected local Codex/OpenCodex/Paseo config hashes unchanged

Tests used fresh temporary HOME, CODEX_HOME, and OPENCODEX_HOME only.

Integration note

This PR targets dev and changes the Bun/TypeScript Responses path. The merging maintainer should record whether the current Go line needs an equivalent port before closeout.

@lidge-jun this is a verified replay-poisoning fix; please review the route-specific trust boundary and single-shot recovery behavior before merge.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of encrypted function and agent output during replay.
    • Prevented base64-like text from being incorrectly treated as backend ciphertext.
    • Added recovery for streamed responses when encrypted content is rejected, allowing successful completion after a single retry.
    • Preserved unknown opaque content until the final destination is determined, while rewriting invalid encrypted content when required for native replay.
  • Documentation

    • Documented encrypted-content recovery and routing behavior for Responses HTTP/SSE.

@Ingwannu
Ingwannu requested a review from lidge-jun as a code owner September 20, 2026 02:43
@Ingwannu Ingwannu added bug Something isn't working proxy HTTP proxy, routing, reverse-proxy / management auth streaming SSE, WebSocket, terminal stream frames labels Sep 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8711e219-eaf6-4813-98e3-e6556f122f0d

📥 Commits

Reviewing files that changed from the base of the PR and between 9164338 and 2371783.

📒 Files selected for processing (8)
  • src/server/responses/core-opaque-recovery.ts
  • src/server/responses/encrypted-payload.ts
  • src/server/responses/passthrough-dispatch.ts
  • src/server/responses/request-prepare.ts
  • structure/decisions/ADR-5236-responses-http-sse.md
  • structure/transports/responses.md
  • tests/responses/responses-compaction-routing.test.ts
  • tests/responses/responses-opaque-blob-recovery.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change requires structural Fernet validation, preserves ambiguous encrypted slots until route selection, rewrites invalid slots for canonical native replay, and enables one bounded recovery for recognized encrypted-output rejection responses.

Changes

Responses opaque recovery

Layer / File(s) Summary
Payload classification and route handling
src/server/responses/encrypted-payload.ts, src/server/responses/request-prepare.ts, tests/responses/*
looksLikeBackendCiphertext now checks Fernet structure. Early sanitization preserves unknown opaque slots. Canonical OpenAI forwarding rewrites non-Fernet slots. Tests cover canonical Fernet data and Fernet-shaped plaintext.
Rejection detection and bounded recovery
src/server/responses/core-opaque-recovery.ts, src/server/responses/passthrough-dispatch.ts, structure/decisions/ADR-5236-responses-http-sse.md, structure/transports/responses.md, tests/responses/responses-opaque-blob-recovery.test.ts
Rejection detection now recognizes upstream error messages and top-level detail payloads. Streamed preflight records the exact rejection so the existing single-shot sanitize-and-retry path can run. Documentation records the recovery behavior, and tests cover the retry flow.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant NativeReplay
  participant requestPrepare
  participant sanitizeEncryptedContentInPlace
  participant CanonicalOpenAI
  participant passthroughDispatch
  NativeReplay->>requestPrepare: Send responses history
  requestPrepare->>sanitizeEncryptedContentInPlace: Preserve unknown opaque slots
  requestPrepare->>sanitizeEncryptedContentInPlace: Rewrite non-Fernet slots for canonical replay
  requestPrepare->>CanonicalOpenAI: Forward sanitized request
  CanonicalOpenAI-->>passthroughDispatch: Encrypted output rejection
  passthroughDispatch->>passthroughDispatch: Detect rejection and record upstreamError
  passthroughDispatch->>sanitizeEncryptedContentInPlace: Run single-shot sanitize-and-rebuild recovery
  passthroughDispatch->>CanonicalOpenAI: Retry request
Loading

Possibly related PRs

  • lidge-jun/opencodex#3753: Added the earlier opaque-output recovery path that this change extends with structural Fernet validation and additional rejection shapes.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 6 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: rejecting agent-generated plaintext that resembles Fernet ciphertext in Responses handling.
Linked Issues check ✅ Passed The PR satisfies the coding requirements in [#5236]. src/server/responses/encrypted-payload.ts replaces length-and-character-set classification with key-independent canonical Fernet structure valida…
Out of Scope Changes check ✅ Passed The changed files remain within [#5236]. The source changes modify ciphertext classification, route-dependent sanitization, decrypt-rejection detection, and bounded streamed recovery. The tests in `te…
Full details: Docstring Coverage

Explanation

Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 6 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 72 / 80

이 PR은 #5236을 고칩니다. 예전에는 encrypted_content 값이 길이 64 이상이고 Base64처럼 보이기만 하면 “진짜 암호문”으로 취급했습니다. 그래서 모델이 만든 gAAAA... 같은 일반 글자도 네이티브 ChatGPT로 다시 보내졌고, 백엔드가 decrypt 거부를 내면서 스레드가 막혔습니다. 지금은 looksLikeBackendCiphertext가 Fernet 토큰의 실제 구조(버전 바이트, 길이, 블록 정렬)를 검사합니다. 라우트가 정해지기 전에는 애매한 슬롯을 잠시 불투명하게 두고, 정식 OpenAI 경로로 갈 때만 엄격히 다시 씁니다. 제3자 경로에서는 예전에 하던 대로 슬롯을 빼서 잘린 암호문이 밖으로 새지 않게 합니다. WebSocket/SSE가 detail로 decrypt 거부를 보내도 기존 한 번짜리 sanitize-and-rebuild 복구로 들어가게 했고, ADR·transport 문서도 같이 넣었습니다. base는 dev라서 방향도 맞습니다.

라인 - tests/codex-integration/multi-agent-compat.test.ts (sanitizeEncryptedContentInPlace > plaintext parked in encrypted slots becomes input_text; real blobs survive): CI test 2/4가 여기서 빨갛습니다. blob = "gAAAAAB".padEnd(120, "Qw1_-=")는 예전 길이/문자 휴리스틱으로는 “진짜 블롭”이었지만, 새 Fernet 구조 검사에서는 가짜라서 rewrite됩니다. 그래서 expect(rewritten).toBe(2)가 3을 받고, 뒤의 “blob은 그대로 encrypted_content” 단언도 깨집니다. 같은 describe에 이미 fernetFixture()가 있는데 이 테스트가 그걸 안 씁니다. responses 쪽 픽스처는 고쳤고 여기만 빠졌습니다.
라인 - src/server/responses/passthrough-dispatch.ts (streamed decrypt 판별): isEncryptedFunctionOutputRejection(JSON.stringify(payload))는 객체를 다시 문자열로 만들었다가 파싱합니다. 동작은 맞지만 불필요한 왕복입니다. detail/message 경로를 객체에서 직접 보거나, helper에 object overload를 두는 편이 더 깔끔합니다. 머지 차단 이슈는 아닙니다.
라인 - 구조적으로만 맞는 Fernet 모양 문자열: 구조 ≠ 인증이므로, 우연히 형식이 맞는 가짜도 한 번은 네이티브로 갈 수 있습니다. PR이 말하는 대로 그 경우는 한 번짜리 복구에 맡깁니다. 문서·ADR에 이미 적혀 있어 설계 선택은 분명합니다.

메인테이너의 판단이 필요한 지점

PR 본문이 물은 대로, 지금 Go 라인에도 같은 분류/복구를 이식해야 하는지 머지 전에 기록할지 정해야 합니다. Bun/TS Responses만 고친 상태입니다. 또한 CI 빨간 픽스처를 “의도된 동작 변경으로 테스트 갱신”으로 볼지(권장), 아니면 옛 휴리스틱 호환을 어딘가에 남길지 한 줄로 확정하면 좋습니다.

너의 추천

방향은 맞고 #5236 재현(195자 gAAAA... plaintext)도 테스트로 잡았습니다. 머지 전에 multi-agent-compat.test.ts의 생존 픽스처를 fernetFixture()(또는 동등한 구조 유효 토큰)로 바꾸고 CI test 2/4를 초록으로 만든 뒤 합치세요. Go 이식 여부는 closeout 메모만 남겨도 됩니다. preview deploy 이야기는 불필요합니다.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun
lidge-jun merged commit 12cb129 into dev Sep 20, 2026
36 of 40 checks passed
@lidge-jun
lidge-jun deleted the fix/5236-fernet-shaped-agent-plaintext branch September 20, 2026 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working proxy HTTP proxy, routing, reverse-proxy / management auth streaming SSE, WebSocket, terminal stream frames

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants