ZBBS-WORK-394: request_log takes the last X-Forwarded-For hop, not the first#232
Merged
Merged
Conversation
…e first The first XFF entry is client-controlled: nginx appends the address it actually saw after any client-supplied entries, so a scanner sending 'X-Forwarded-For: 127.0.0.1' (localhost-trust probe) was logged as 127.0.0.1 in the admin Recent API Activity panel (observed 2026-06-10, ~1,640-request wordlist burst). We sit exactly one trusted proxy deep, so the last entry is always the real peer. Display-only fix — nothing in the app makes trust decisions off this field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jeffdafoe
added a commit
that referenced
this pull request
Jun 16, 2026
…e first (#232) The first XFF entry is client-controlled: nginx appends the address it actually saw after any client-supplied entries, so a scanner sending 'X-Forwarded-For: 127.0.0.1' (localhost-trust probe) was logged as 127.0.0.1 in the admin Recent API Activity panel (observed 2026-06-10, ~1,640-request wordlist burst). We sit exactly one trusted proxy deep, so the last entry is always the real peer. Display-only fix — nothing in the app makes trust decisions off this field. Co-authored-by: Claude Fable 5 <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.
Problem
The Recent API Activity panel logged today's ~1,640-request scanner wordlist burst as source
127.0.0.1— the scanner sentX-Forwarded-For: 127.0.0.1(localhost-trust probe) and the middleware takes the first XFF entry, which is client-controlled. nginx ($proxy_add_x_forwarded_for) appends the address it actually saw after client-supplied entries, so the last hop is the trustworthy one in our exactly-one-proxy deployment.Display-integrity only — verified nothing in the app makes trust decisions off this field.
Fix
Take the last XFF hop; junk/missing header falls back to
req.ip. Spoofed list → real IP; single hop → unchanged; direct local connection → socket address.code_review: no blocking issues. Its optional array-header guard was deliberately skipped — Node joins duplicate headers into a single string for all non-
set-cookieheaders, so string-or-undefined is the platform contract.— Work
🤖 Generated with Claude Code