Skip to content

Harden redirect handler: buffer content, filter sensitive headers#1055

Open
Chris0Jeky wants to merge 3 commits intomainfrom
fix/redirect-content-and-header-safety
Open

Harden redirect handler: buffer content, filter sensitive headers#1055
Chris0Jeky wants to merge 3 commits intomainfrom
fix/redirect-content-and-header-safety

Conversation

@Chris0Jeky
Copy link
Copy Markdown
Owner

Summary

  • Content buffering: Read request body into byte array before first SendAsync so 307/308 redirects create fresh ByteArrayContent instead of reusing a potentially consumed/non-seekable stream
  • Host header stripping: Always excluded on redirects since it must match the new destination
  • Authorization header filtering: Stripped on cross-host redirects to prevent credential leakage; preserved on same-host redirects

Addresses Gemini HIGH findings on PR #1054.

Test plan

  • SendAsync_307Redirect_PreservesContentAndSafeHeaders — verifies body is replayed and custom headers forwarded
  • SendAsync_307CrossHostRedirect_StripsAuthorizationHeader — verifies auth stripped on cross-host
  • SendAsync_307SameHostRedirect_PreservesAuthorizationHeader — verifies auth kept on same-host
  • All 14 EgressEnvelopeHandler tests pass

- 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
Copilot AI review requested due to automatic review settings May 9, 2026 09:57
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

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.

Comment thread backend/src/Taskdeck.Application/Services/EgressEnvelopeHandler.cs Outdated
Comment thread backend/src/Taskdeck.Application/Services/EgressEnvelopeHandler.cs Outdated
Comment thread backend/src/Taskdeck.Application/Services/EgressEnvelopeHandler.cs Outdated
Comment thread backend/src/Taskdeck.Application/Services/EgressEnvelopeHandler.cs Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 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".

Comment thread backend/src/Taskdeck.Application/Services/EgressEnvelopeHandler.cs Outdated
Comment thread backend/src/Taskdeck.Application/Services/EgressEnvelopeHandler.cs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 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".

Comment thread backend/src/Taskdeck.Application/Services/EgressEnvelopeHandler.cs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending

Development

Successfully merging this pull request may close these issues.

2 participants