internal/middleware/middleware.go lines 136 to 169 (clientIP, firstForwardedFor), at v1.0.0-rc1: when the direct peer is in TRUSTED_PROXIES, the client address is the leftmost valid entry of X-Forwarded-For. Every ordinary reverse proxy appends the peer it saw to the list the client sent, so the leftmost entry is whatever the client wrote. With the default TRUSTED_PROXIES (the RFC 1918 ranges) and the backend run on its own behind such a proxy (make run, or the binary outside the image), any client can choose the address the rate limit counts and the request log records, and so get a fresh REPORTS_PER_MINUTE allowance on every request. The image is not affected: its nginx replaces X-Forwarded-For with the one address it resolved (configs/nginx.conf lines 47 to 52 say so), and the backend trusts only 127.0.0.1/32 there.
How to see it: start the backend on its own with the default settings, and send POST /api/v1/reports from a peer in 10.0.0.0/8 with X-Forwarded-For: 203.0.113.9, 10.0.0.5: the request log says remote_ip203.0.113.9. Send 61 such requests, each with a different first entry: none is refused.
What is right: REPO_POLICIES.md line 580 to 583: "The application must accept forwarded headers only from a configured set of trusted proxy addresses — never trust X-Forwarded-For unconditionally." The entries a trusted proxy added are the ones on the right; the client address is the rightmost entry that is not itself a trusted proxy (what nginx's real_ip_recursive on does). README.md Settings describes TRUSTED_PROXIES as the set "whose X-Forwarded-For / X-Real-IP headers are trusted", with no warning that the client controls the value.
Severity: low. The shipped image is unaffected; it matters only for a standalone backend behind a proxy, which the README allows.
Definition of done
clientIP walks X-Forwarded-For from the right, skipping addresses in TRUSTED_PROXIES, and takes the first address that is not a trusted proxy; with none left it uses the peer. X-Real-IP is used only when X-Forwarded-For gives nothing.
Tests in internal/middleware/middleware_test.go: from a trusted peer, X-Forwarded-For: 203.0.113.9, 198.51.100.7 resolves to 198.51.100.7; 203.0.113.9, 10.0.0.5 with 10.0.0.0/8 trusted resolves to 203.0.113.9; a list of only trusted addresses resolves to the peer; TestRateLimitIsPerForwardedClient and TestRateLimitIgnoresForwardedForFromUntrustedPeer still pass.
README.md Settings says which entry is taken.
TODO.md updated in the same commit; commit title ends (closes #N) with this issue's number.
Model: fable-5-1
`internal/middleware/middleware.go` lines 136 to 169 (`clientIP`, `firstForwardedFor`), at `v1.0.0-rc1`: when the direct peer is in `TRUSTED_PROXIES`, the client address is the leftmost valid entry of `X-Forwarded-For`. Every ordinary reverse proxy appends the peer it saw to the list the client sent, so the leftmost entry is whatever the client wrote. With the default `TRUSTED_PROXIES` (the RFC 1918 ranges) and the backend run on its own behind such a proxy (`make run`, or the binary outside the image), any client can choose the address the rate limit counts and the request log records, and so get a fresh `REPORTS_PER_MINUTE` allowance on every request. The image is not affected: its nginx replaces `X-Forwarded-For` with the one address it resolved (`configs/nginx.conf` lines 47 to 52 say so), and the backend trusts only `127.0.0.1/32` there.
**How to see it**: start the backend on its own with the default settings, and send `POST /api/v1/reports` from a peer in `10.0.0.0/8` with `X-Forwarded-For: 203.0.113.9, 10.0.0.5`: the request log says `remote_ip` `203.0.113.9`. Send 61 such requests, each with a different first entry: none is refused.
**What is right**: `REPO_POLICIES.md` line 580 to 583: "The application must accept forwarded headers only from a configured set of trusted proxy addresses — never trust `X-Forwarded-For` unconditionally." The entries a trusted proxy added are the ones on the right; the client address is the rightmost entry that is not itself a trusted proxy (what nginx's `real_ip_recursive on` does). `README.md` Settings describes `TRUSTED_PROXIES` as the set "whose `X-Forwarded-For` / `X-Real-IP` headers are trusted", with no warning that the client controls the value.
**Severity**: low. The shipped image is unaffected; it matters only for a standalone backend behind a proxy, which the README allows.
## Definition of done
- [ ] `clientIP` walks `X-Forwarded-For` from the right, skipping addresses in `TRUSTED_PROXIES`, and takes the first address that is not a trusted proxy; with none left it uses the peer. `X-Real-IP` is used only when `X-Forwarded-For` gives nothing.
- [ ] Tests in `internal/middleware/middleware_test.go`: from a trusted peer, `X-Forwarded-For: 203.0.113.9, 198.51.100.7` resolves to `198.51.100.7`; `203.0.113.9, 10.0.0.5` with `10.0.0.0/8` trusted resolves to `203.0.113.9`; a list of only trusted addresses resolves to the peer; `TestRateLimitIsPerForwardedClient` and `TestRateLimitIgnoresForwardedForFromUntrustedPeer` still pass.
- [ ] `README.md` Settings says which entry is taken.
- [ ] `TODO.md` updated in the same commit; commit title ends ` (closes #N)` with this issue's number.
Model: fable-5-1
clawbot
self-assigned this 2026-10-14 16:08:46 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
internal/middleware/middleware.golines 136 to 169 (clientIP,firstForwardedFor), atv1.0.0-rc1: when the direct peer is inTRUSTED_PROXIES, the client address is the leftmost valid entry ofX-Forwarded-For. Every ordinary reverse proxy appends the peer it saw to the list the client sent, so the leftmost entry is whatever the client wrote. With the defaultTRUSTED_PROXIES(the RFC 1918 ranges) and the backend run on its own behind such a proxy (make run, or the binary outside the image), any client can choose the address the rate limit counts and the request log records, and so get a freshREPORTS_PER_MINUTEallowance on every request. The image is not affected: its nginx replacesX-Forwarded-Forwith the one address it resolved (configs/nginx.conflines 47 to 52 say so), and the backend trusts only127.0.0.1/32there.How to see it: start the backend on its own with the default settings, and send
POST /api/v1/reportsfrom a peer in10.0.0.0/8withX-Forwarded-For: 203.0.113.9, 10.0.0.5: the request log saysremote_ip203.0.113.9. Send 61 such requests, each with a different first entry: none is refused.What is right:
REPO_POLICIES.mdline 580 to 583: "The application must accept forwarded headers only from a configured set of trusted proxy addresses — never trustX-Forwarded-Forunconditionally." The entries a trusted proxy added are the ones on the right; the client address is the rightmost entry that is not itself a trusted proxy (what nginx'sreal_ip_recursive ondoes).README.mdSettings describesTRUSTED_PROXIESas the set "whoseX-Forwarded-For/X-Real-IPheaders are trusted", with no warning that the client controls the value.Severity: low. The shipped image is unaffected; it matters only for a standalone backend behind a proxy, which the README allows.
Definition of done
clientIPwalksX-Forwarded-Forfrom the right, skipping addresses inTRUSTED_PROXIES, and takes the first address that is not a trusted proxy; with none left it uses the peer.X-Real-IPis used only whenX-Forwarded-Forgives nothing.internal/middleware/middleware_test.go: from a trusted peer,X-Forwarded-For: 203.0.113.9, 198.51.100.7resolves to198.51.100.7;203.0.113.9, 10.0.0.5with10.0.0.0/8trusted resolves to203.0.113.9; a list of only trusted addresses resolves to the peer;TestRateLimitIsPerForwardedClientandTestRateLimitIgnoresForwardedForFromUntrustedPeerstill pass.README.mdSettings says which entry is taken.TODO.mdupdated in the same commit; commit title ends(closes #N)with this issue's number.Model: fable-5-1