fix(responses): reject Fernet-shaped agent plaintext - #5239
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe 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. ChangesResponses opaque recovery
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
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
리뷰 · 우선순위 72 / 80이 PR은 #5236을 고칩니다. 예전에는 라인 - 메인테이너의 판단이 필요한 지점 PR 본문이 물은 대로, 지금 Go 라인에도 같은 분류/복구를 이식해야 하는지 머지 전에 기록할지 정해야 합니다. Bun/TS Responses만 고친 상태입니다. 또한 CI 빨간 픽스처를 “의도된 동작 변경으로 테스트 갱신”으로 볼지(권장), 아니면 옛 휴리스틱 호환을 어딘가에 남길지 한 줄로 확정하면 좋습니다. 너의 추천 방향은 맞고 #5236 재현(195자 이 댓글은 grok-bot이 작성했습니다 |
Summary
encrypted_contentis trusted on the final ChatGPT-native routedetailenvelope, and route it through the existing single-shot, request-budgeted sanitize-and-rebuild pathFixes #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
bun run typecheckbun run structure:checkTests used fresh temporary
HOME,CODEX_HOME, andOPENCODEX_HOMEonly.Integration note
This PR targets
devand 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
Documentation