fix(email): reject XOAUTH2 SASL delimiter injection - #1340
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe email client now rejects SASL delimiter characters in XOAUTH2 usernames and access tokens. Parameterized tests verify the ChangesXOAUTH2 delimiter validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR adds focused rejection of XOAUTH2 delimiter injection while preserving the ordinary payload contract; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
|
PR governance metadata gate update for PR governance metadata gate is ready; all current-head requirements passed. |
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@opencode-agent review current exact head |
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@opencode-agent review current exact head |
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Security defect
generate_oauth2_string()interpolated the caller-provided user identity and OAuth access token directly into the SASL XOAUTH2 attribute stream, whose fields are delimited by0x01. If either value itself contained0x01, the generated authentication payload acquired caller-controlled field boundaries instead of representing exactly oneuserattribute and oneauth=Bearerattribute.TDD evidence
The initial exact head
4baff93834cb59e992c9bfd1fe1a7b944ef21377added only two hostile-input regressions:Production was unchanged on that head. Executing the exact pre-fix function against those cases produced no
ValueError; decoding the resulting bytes showed the injected field boundaries, establishing the intended RED at the construction boundary. Predecessor-head workflow results are diagnostic history only.Bounded GREEN implementation
Current exact head:
333285f877595cf0f23d32791af8874fb6b8af05.Current protected base:
develop@83ce2561e6566bc29a7abdecad6cd0a2e4ceb2a8.The production helper rejects Control-A in either caller-controlled XOAUTH2 field before constructing or base64-encoding the SASL response. The ordinary payload contract remains unchanged. No credential logging, protocol fallback, IMAP/SMTP destination-policy expansion, token-policy change, or provider-specific authentication behavior was added.
The branch also adds
docs/research/email-authentication-xoauth2/README.md, grounding the structural delimiter rule in RFC 7628 and Google's official Gmail XOAUTH2 wire-format documentation with APA 7 references and an explicit claim boundary.Current exact-head gate state
On exact head
333285f877595cf0f23d32791af8874fb6b8af05:backend (Python 3.14): success;strix: success;opencode-review: success;coverage-evidence: success;A fresh CodeRabbit review request was rate-limited and produced no formal review. Rate-limit text is not approval evidence. No current formal review is therefore counted as a qualifying independent approval.
Replacement boundary
This current-
developlane carries the intended security behavior from stale PR #1293 without its unrelated historical drift. #1293 is now closed and no old-head check or review evidence is reused here.Merge gate
Do not merge until the unchanged current head has qualifying independent current-head review evidence and every live required context remains terminal-success under normal protected-branch rules. Queued, predecessor-head, stale, rate-limited, status-only, or model-only evidence is not success.
Summary by CodeRabbit
Security
Bug Fixes
Documentation
Tests