Repository navigation
docs(#1237): the first x-forwarded-for entry is safe because Supabase's edge sets it - #1293
Merged
Merged
Conversation
…'s edge sets it Measured on production today, as part of #1245 stage A3's deploy: a create-lead request sent with `X-Forwarded-For: 203.0.113.9` (TEST-NET) was keyed by the rate limiter on the sender's real public IPv4, not on the spoofed value and not on any internal or shared address. The platform puts the true client address first. So trusting entry [0] is correct here, and #1237's second defect does not reproduce. What was wrong was the reasoning written beside it in both functions and the unit test: "taking the last would let a caller prepend their own header". A client can always write the leftmost entry of a header it sends; [0] is trustworthy only because the edge overwrites it. The comments now say that, record how it was measured, and say to re-measure before trusting [0] behind any other proxy. Comments only; no behaviour change. deno check clean (18 functions). Deployed after merge so the live functions match main. Closes #1237 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This finishes #1237. Both of its defects are now settled on production.
Defect 1, the check-then-record race, is fixed and live, through #1283 (SQL) and #1291 (Edge Functions). This morning's concurrent probe on production proved it: after one earlier attempt, 8 simultaneous
create-leadPOSTs answered exactly 4×409 then 4×429.Defect 2, trusting the first
x-forwarded-forentry, does not reproduce. I sent the same probe withX-Forwarded-For: 203.0.113.9(TEST-NET). The limiter keyed it on the sender's real public IPv4 — not on the spoofed value, and not on any internal or shared address. Supabase's edge puts the true client address first.What was wrong is the reasoning beside the code, in both functions and in the unit test: "taking the last would let a caller prepend their own header". A client can always write the leftmost entry of a header it sends. Entry
[0]is trustworthy only because the edge overwrites it. The comments now:[0]behind any other proxy.This is comments only, with no behaviour change.
deno checkis clean across all 18 functions, the unit tests pass (37), and gitleaks is clean. After merge I redeploy both functions so what is live matches main, verified the same way as A3.Closes #1237
🤖 Generated with Claude Code