Harden redirect handler: buffer content, filter sensitive headers#1055
Harden redirect handler: buffer content, filter sensitive headers#1055Chris0Jeky wants to merge 3 commits intomainfrom
Conversation
- Buffer request content before first send so 307/308 redirects use fresh ByteArrayContent instead of reusing a potentially consumed stream - Strip Host header on all redirects (must match new destination) - Strip Authorization header on cross-host redirects to prevent credential leakage - Preserve Authorization on same-host redirects
There was a problem hiding this comment.
Code Review
This pull request updates the EgressEnvelopeHandler to support 307 and 308 redirects by buffering request content for replay and implementing security logic to strip the Authorization header on cross-host redirects. Feedback highlights a potential Denial of Service vulnerability due to unlimited buffering, a logic error in method selection for nested redirects, and the need to preserve additional content headers and strip Cookie headers on cross-host redirects.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 879d83e88d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b5a442050
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
SendAsyncso 307/308 redirects create freshByteArrayContentinstead of reusing a potentially consumed/non-seekable streamAddresses Gemini HIGH findings on PR #1054.
Test plan
SendAsync_307Redirect_PreservesContentAndSafeHeaders— verifies body is replayed and custom headers forwardedSendAsync_307CrossHostRedirect_StripsAuthorizationHeader— verifies auth stripped on cross-hostSendAsync_307SameHostRedirect_PreservesAuthorizationHeader— verifies auth kept on same-host