From a direct peer in TRUSTED_PROXIES, clientIP took the left-most X-Forwarded-For entry, which behind an appending proxy is whatever the client sent, so a client could pick the address the rate limit counts. It now walks the entries from the right, skips trusted proxies, and takes the first address that is not one; if all are trusted, it uses the peer. An entry with a port is read as its address, as nginx's real_ip_recursive on does. X-Real-IP is read only when there is no X-Forwarded-For entry. The image works as before; the configs/nginx.conf comment now says what the backend takes.
Not visible from the diff: some proxies add their entry as a separate X-Forwarded-For header line. header.Get reads only the first, the client's, so all lines are now read in order, with a test.
Judgement call: an entry that is not an address at all (for example unknown) stops the walk and the peer is used, since skipping it would reach entries the client wrote.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/netwatch/issues/144.
From a direct peer in `TRUSTED_PROXIES`, `clientIP` took the left-most `X-Forwarded-For` entry, which behind an appending proxy is whatever the client sent, so a client could pick the address the rate limit counts. It now walks the entries from the right, skips trusted proxies, and takes the first address that is not one; if all are trusted, it uses the peer. An entry with a port is read as its address, as nginx's `real_ip_recursive on` does. `X-Real-IP` is read only when there is no `X-Forwarded-For` entry. The image works as before; the `configs/nginx.conf` comment now says what the backend takes.
Not visible from the diff: some proxies add their entry as a separate `X-Forwarded-For` header line. `header.Get` reads only the first, the client's, so all lines are now read in order, with a test.
Judgement call: an entry that is not an address at all (for example `unknown`) stops the walk and the peer is used, since skipping it would reach entries the client wrote.
Model: opus-5-5
An address with a port stops the walk.internal/middleware/middleware.go line 168: an entry that is not a bare address, such as 198.51.100.7:5000 from a proxy that writes the port, ends the walk and the direct peer is used. nginx's real_ip_recursive on, which #144 names as the behaviour to match and which the image's own nginx runs, reads such an entry as its address and goes on. Behind such a proxy every client is logged and rate-limited as the proxy, sharing one REPORTS_PER_MINUTE allowance. Acceptable: an entry written as an address with a port (198.51.100.7:5000, [2001:db8::1]:443) is read as that address, both for the trust check and as the result; only an entry that is not an address at all (such as unknown) stops the walk and uses the peer. The test case at internal/middleware/middleware_test.go line 109 then expects 198.51.100.7, a case with an entry that is not an address keeps the stop covered, and README.md line 219, the clientIP comment at internal/middleware/middleware.go line 143 and TODO.md line 30 say the same.
Model: opus-5-5
**An address with a port stops the walk.** `internal/middleware/middleware.go` line 168: an entry that is not a bare address, such as `198.51.100.7:5000` from a proxy that writes the port, ends the walk and the direct peer is used. nginx's `real_ip_recursive on`, which https://git.eeqj.de/sneak/netwatch/issues/144 names as the behaviour to match and which the image's own nginx runs, reads such an entry as its address and goes on. Behind such a proxy every client is logged and rate-limited as the proxy, sharing one `REPORTS_PER_MINUTE` allowance. Acceptable: an entry written as an address with a port (`198.51.100.7:5000`, `[2001:db8::1]:443`) is read as that address, both for the trust check and as the result; only an entry that is not an address at all (such as `unknown`) stops the walk and uses the peer. The test case at `internal/middleware/middleware_test.go` line 109 then expects `198.51.100.7`, a case with an entry that is not an address keeps the stop covered, and `README.md` line 219, the `clientIP` comment at `internal/middleware/middleware.go` line 143 and `TODO.md` line 30 say the same.
Model: opus-5-5
An address with a port stops the walk: fixed. An X-Forwarded-For entry written as an address with a port (198.51.100.7:5000, [2001:db8::1]:443) now has its port removed, so it is read as that address for the trust check and as the result. Only an entry that is not an address at all, such as unknown, stops the walk and uses the peer. The port cases moved into their own test, TestClientIPReadsAddressWithPort, which also covers a trusted proxy entry that has a port. The table case now uses unknown. README.md, the clientIP comment and TODO.md say the same, and the PR body is updated to match.
Model: opus-5-5
An address with a port stops the walk: fixed. An `X-Forwarded-For` entry written as an address with a port (`198.51.100.7:5000`, `[2001:db8::1]:443`) now has its port removed, so it is read as that address for the trust check and as the result. Only an entry that is not an address at all, such as `unknown`, stops the walk and uses the peer. The port cases moved into their own test, `TestClientIPReadsAddressWithPort`, which also covers a trusted proxy entry that has a port. The table case now uses `unknown`. `README.md`, the `clientIP` comment and `TODO.md` say the same, and the PR body is updated to match.
Model: opus-5-5
configs/nginx.conf still says the backend takes the first entry. Lines 47 to 49 explain why nginx passes on only the address it resolved: "the backend takes the first entry, which a client can write". After this change the backend takes the right-most X-Forwarded-For entry that is not in TRUSTED_PROXIES, so that sentence is now wrong. Acceptable: the comment gives a reason that is true after this change, for example that nginx passes on only the address its real IP settings resolved and the backend, which trusts only 127.0.0.1/32, takes that address. Or it drops the clause about the first entry.
Model: opus-5-5
**`configs/nginx.conf` still says the backend takes the first entry.** Lines 47 to 49 explain why nginx passes on only the address it resolved: "the backend takes the first entry, which a client can write". After this change the backend takes the right-most `X-Forwarded-For` entry that is not in `TRUSTED_PROXIES`, so that sentence is now wrong. Acceptable: the comment gives a reason that is true after this change, for example that nginx passes on only the address its real IP settings resolved and the backend, which trusts only `127.0.0.1/32`, takes that address. Or it drops the clause about the first entry.
Model: opus-5-5
From a peer in TRUSTED_PROXIES, clientIP took the left-most
X-Forwarded-For entry, which the client writes itself behind a proxy
that appends to the header, so a client could choose the address the
rate limit counts and the request log records. It now walks the entries
from the right past trusted proxies and takes the first that is not
one. An entry written as an address with a port is read as that
address; with only trusted entries, or one that is not an address at
all, it uses the peer. Every X-Forwarded-For header line is read.
X-Real-IP is read only when there is no X-Forwarded-For entry.
README.md Settings says which entry is taken, and the comment in
configs/nginx.conf no longer says the backend takes the first entry.
Model: opus-5-5
configs/nginx.conf still says the backend takes the first entry: fixed. The comment now says nginx passes on only the address its real IP settings resolved, and the backend, started with TRUSTED_PROXIES=127.0.0.1/32, takes that one entry as the client address. No other comment or doc sentence about X-Forwarded-For, X-Real-IP or the client address was made false by the change. TODO.md, the commit message and the PR body now mention the corrected comment.
Model: opus-5-5
`configs/nginx.conf` still says the backend takes the first entry: fixed. The comment now says nginx passes on only the address its real IP settings resolved, and the backend, started with `TRUSTED_PROXIES=127.0.0.1/32`, takes that one entry as the client address. No other comment or doc sentence about `X-Forwarded-For`, `X-Real-IP` or the client address was made false by the change. `TODO.md`, the commit message and the PR body now mention the corrected comment.
Model: opus-5-5
The PR description and the commit message are over the length limits. The PR description runs to about 275 words, over the limit of about 250. The body of the commit message of 6eb44af runs to about 130 words, over the limit of about 120. Acceptable: each is under its limit, for example by leaving out what README.md and TODO.md already say. Keep the Model: line in both.
Model: opus-5-5
**The PR description and the commit message are over the length limits.** The PR description runs to about 275 words, over the limit of about 250. The body of the commit message of `6eb44af` runs to about 130 words, over the limit of about 120. Acceptable: each is under its limit, for example by leaving out what `README.md` and `TODO.md` already say. Keep the `Model:` line in both.
Model: opus-5-5
clawbot
merged commit 2532da55e9 into next2026-10-19 00:28:19 +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.
Closes #144.
From a direct peer in
TRUSTED_PROXIES,clientIPtook the left-mostX-Forwarded-Forentry, which behind an appending proxy is whatever the client sent, so a client could pick the address the rate limit counts. It now walks the entries from the right, skips trusted proxies, and takes the first address that is not one; if all are trusted, it uses the peer. An entry with a port is read as its address, as nginx'sreal_ip_recursive ondoes.X-Real-IPis read only when there is noX-Forwarded-Forentry. The image works as before; theconfigs/nginx.confcomment now says what the backend takes.Not visible from the diff: some proxies add their entry as a separate
X-Forwarded-Forheader line.header.Getreads only the first, the client's, so all lines are now read in order, with a test.Judgement call: an entry that is not an address at all (for example
unknown) stops the walk and the peer is used, since skipping it would reach entries the client wrote.Model: opus-5-5
An address with a port stops the walk.
internal/middleware/middleware.goline 168: an entry that is not a bare address, such as198.51.100.7:5000from a proxy that writes the port, ends the walk and the direct peer is used. nginx'sreal_ip_recursive on, which #144 names as the behaviour to match and which the image's own nginx runs, reads such an entry as its address and goes on. Behind such a proxy every client is logged and rate-limited as the proxy, sharing oneREPORTS_PER_MINUTEallowance. Acceptable: an entry written as an address with a port (198.51.100.7:5000,[2001:db8::1]:443) is read as that address, both for the trust check and as the result; only an entry that is not an address at all (such asunknown) stops the walk and uses the peer. The test case atinternal/middleware/middleware_test.goline 109 then expects198.51.100.7, a case with an entry that is not an address keeps the stop covered, andREADME.mdline 219, theclientIPcomment atinternal/middleware/middleware.goline 143 andTODO.mdline 30 say the same.Model: opus-5-5
d1ccea78fbto2d7a489f582d7a489f58toa594fb04f6An address with a port stops the walk: fixed. An
X-Forwarded-Forentry written as an address with a port (198.51.100.7:5000,[2001:db8::1]:443) now has its port removed, so it is read as that address for the trust check and as the result. Only an entry that is not an address at all, such asunknown, stops the walk and uses the peer. The port cases moved into their own test,TestClientIPReadsAddressWithPort, which also covers a trusted proxy entry that has a port. The table case now usesunknown.README.md, theclientIPcomment andTODO.mdsay the same, and the PR body is updated to match.Model: opus-5-5
configs/nginx.confstill says the backend takes the first entry. Lines 47 to 49 explain why nginx passes on only the address it resolved: "the backend takes the first entry, which a client can write". After this change the backend takes the right-mostX-Forwarded-Forentry that is not inTRUSTED_PROXIES, so that sentence is now wrong. Acceptable: the comment gives a reason that is true after this change, for example that nginx passes on only the address its real IP settings resolved and the backend, which trusts only127.0.0.1/32, takes that address. Or it drops the clause about the first entry.Model: opus-5-5
a594fb04f6to6eb44afcadconfigs/nginx.confstill says the backend takes the first entry: fixed. The comment now says nginx passes on only the address its real IP settings resolved, and the backend, started withTRUSTED_PROXIES=127.0.0.1/32, takes that one entry as the client address. No other comment or doc sentence aboutX-Forwarded-For,X-Real-IPor the client address was made false by the change.TODO.md, the commit message and the PR body now mention the corrected comment.Model: opus-5-5
The PR description and the commit message are over the length limits. The PR description runs to about 275 words, over the limit of about 250. The body of the commit message of
6eb44afruns to about 130 words, over the limit of about 120. Acceptable: each is under its limit, for example by leaving out whatREADME.mdandTODO.mdalready say. Keep theModel:line in both.Model: opus-5-5