Repository navigation
fix(shared): strip credentials and request bodies when safeFetch follows redirects - #1137
Conversation
…ows redirects safeFetch re-sent the original headers and body to every redirect target, so a webhook configured with Authorization or secret headers leaked them to whatever host the endpoint redirected to. Cross-origin hops now keep only benign headers, and 301/302/303 redirects of a POST become a body-less GET as in the Fetch spec.
|
@Adityakk9031 is attempting to deploy a commit to the Databuddy OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
 [Critical impact] Modifies how the fetch wrapper handles HTTP redirects. The PR appears safe to merge, with non-blocking fixes recommended for redirected FindingsSummaryAdds
Reviews (1) · Last reviewed commit: "fix(shared): strip credentials and reque..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const CROSS_ORIGIN_REDIRECT_HEADERS = new Set([ | ||
| "accept", | ||
| "accept-language", | ||
| "content-type", | ||
| "user-agent", | ||
| ]); |
There was a problem hiding this comment.
The allowlist drops accept-encoding, including the identity value required by fetchRobotsTxt. If a website redirects to another origin that sends compressed content, fetchRobotsTxt rejects the response. checkAiRobots then reports no robots.txt and treats blocked crawlers as allowed. Preserve accept-encoding across redirects; it does not carry credentials.
| const CROSS_ORIGIN_REDIRECT_HEADERS = new Set([ | |
| "accept", | |
| "accept-language", | |
| "content-type", | |
| "user-agent", | |
| ]); | |
| const CROSS_ORIGIN_REDIRECT_HEADERS = new Set([ | |
| "accept", | |
| "accept-encoding", | |
| "accept-language", | |
| "content-type", | |
| "user-agent", | |
| ]); |
| }); | ||
| }); | ||
|
|
||
| describe("redirectInit", () => { |
There was a problem hiding this comment.
The new tests call only redirectInit; none exercise the changed safeFetch loop. Removing the assignment to fetchInit would leave every new test passing while restoring the original behavior.
Add a test with multiple redirects that checks the outgoing requests. Verify that stripped credentials stay stripped and that a POST changed to GET does not regain its body.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This branch has not been deployed
1 participant
Problem
safeFetchre-sends the original headers and body to every redirect target. A webhook configured withAuthorizationor a secret header (WebhookProvider) leaks them to whatever host the endpoint redirects to, and a 301/302/303 redirect of a POST is replayed as a POST with the body.Fix
New exported
redirectInitapplied on each hop:accept,accept-language,content-type,user-agentcontent-*headers (as in the Fetch spec)Tests
Added
redirectInitcases tossrf-guard.test.ts.Summary by cubic
Fixes
safeFetchso redirects no longer forward credentials or request bodies to the redirect target.accept,accept-language,content-type, anduser-agentheaders, droppingAuthorizationand secret headers.content-*headers, matching the Fetch spec.redirectInitexport.Written for commit bb46799. Summary will update on new commits.