Unset or empty, TRUSTED_PROXIES now defaults to 10.0.0.0/8, 172.16.0.0/12 and 192.168.0.0/16, so a reverse proxy reaching webhooker from one of those ranges gets per-client rate-limit buckets with nothing set. A set value replaces the default entirely; an unparseable one still fails startup.
envPrefixList takes a default like the other env* helpers; ALLOWED_EGRESS_CIDRS passes an empty one and behaves as before.
The startup warning for an empty list is removed, with its test hook and test.
README: the Configuration table, "Trusted proxies", "Running under upaas" and step 3 of the reverse-proxy checklist give the default, one rule (if any client can reach webhooker, or the proxy in front of it, from an RFC 1918 source address, set the list to the proxy's address alone), and where to find that address (the remoteIP field of the http request log line). Rate Limiting, the login endpoint, Architecture and Security passages that described the old default or the warning are updated to match, as are code comments that said the list was empty by default.
Tests: the default when unset or blank, a set value replacing it, invalid values failing startup.
Disclosures:
Judgement call: a set value that names nothing (TRUSTED_PROXIES=,) still yields an empty list and trusts nobody, since a set value replaces the default.
Loopback is not in the default, per the issue; a proxy reaching the binary over loopback still needs TRUSTED_PROXIES=127.0.0.1, as the README's nginx example already sets.
Unset or empty, `TRUSTED_PROXIES` now defaults to `10.0.0.0/8`, `172.16.0.0/12` and `192.168.0.0/16`, so a reverse proxy reaching webhooker from one of those ranges gets per-client rate-limit buckets with nothing set. A set value replaces the default entirely; an unparseable one still fails startup.
- `envPrefixList` takes a default like the other `env*` helpers; `ALLOWED_EGRESS_CIDRS` passes an empty one and behaves as before.
- The startup warning for an empty list is removed, with its test hook and test.
- README: the Configuration table, "Trusted proxies", "Running under upaas" and step 3 of the reverse-proxy checklist give the default, one rule (if any client can reach webhooker, or the proxy in front of it, from an RFC 1918 source address, set the list to the proxy's address alone), and where to find that address (the `remoteIP` field of the `http request` log line). Rate Limiting, the login endpoint, Architecture and Security passages that described the old default or the warning are updated to match, as are code comments that said the list was empty by default.
- Tests: the default when unset or blank, a set value replacing it, invalid values failing startup.
Disclosures:
- Judgement call: a set value that names nothing (`TRUSTED_PROXIES=,`) still yields an empty list and trusts nobody, since a set value replaces the default.
- Loopback is not in the default, per the issue; a proxy reaching the binary over loopback still needs `TRUSTED_PROXIES=127.0.0.1`, as the README's nginx example already sets.
Closes https://git.eeqj.de/sneak/webhooker/issues/333
Model: opus-5-5
README.md, the TRUSTED_PROXIES row of the Configuration table, the first two bullets under "Trusted proxies", and the TRUSTED_PROXIES bullet under "Running under upaas": they say to narrow the list only when clients connect directly from private addresses (upaas: "other than through the proxy"), and describe a private-addressed client behind the proxy as merely sharing the proxy's bucket. That is wrong. Such a client's own address is skipped as a trusted hop, so if it sends its own X-Forwarded-For, the entry it wrote is taken as the client and it chooses its own rate-limit key, the same as a direct peer. An operator whose LAN clients reach webhooker through the proxy is told the default is safe when every limit, including the webhook receiver's, is bypassable by those clients. Acceptable: state that under the default any client with a private address, whether it connects directly or through the proxy, can choose its own rate-limit key, and that an operator with any clients on private addresses must set the list to the proxy's address alone; drop the "directly" and "other than through the proxy" qualifiers.
README.md, "The login endpoint": "Setting TRUSTED_PROXIES does not stop the saturation, but it makes the source visible in the failure logs" still assumes the old default, where leaving the list unset hid the client. Under the new default a proxy on a private network is already covered and the source is already visible. Acceptable: say the source is visible in the failure logs when TRUSTED_PROXIES covers the proxy, which the default does for one on a private network.
Model: opus-5-5
1. `README.md`, the `TRUSTED_PROXIES` row of the Configuration table, the first two bullets under "Trusted proxies", and the `TRUSTED_PROXIES` bullet under "Running under upaas": they say to narrow the list only when clients connect *directly* from private addresses (upaas: "other than through the proxy"), and describe a private-addressed client behind the proxy as merely sharing the proxy's bucket. That is wrong. Such a client's own address is skipped as a trusted hop, so if it sends its own `X-Forwarded-For`, the entry it wrote is taken as the client and it chooses its own rate-limit key, the same as a direct peer. An operator whose LAN clients reach webhooker through the proxy is told the default is safe when every limit, including the webhook receiver's, is bypassable by those clients. Acceptable: state that under the default any client with a private address, whether it connects directly or through the proxy, can choose its own rate-limit key, and that an operator with any clients on private addresses must set the list to the proxy's address alone; drop the "directly" and "other than through the proxy" qualifiers.
2. `README.md`, "The login endpoint": "Setting `TRUSTED_PROXIES` does not stop the saturation, but it makes the source visible in the failure logs" still assumes the old default, where leaving the list unset hid the client. Under the new default a proxy on a private network is already covered and the source is already visible. Acceptable: say the source is visible in the failure logs when `TRUSTED_PROXIES` covers the proxy, which the default does for one on a private network.
Model: opus-5-5
clawbot
changed title from Trust the RFC 1918 ranges as proxies when TRUSTED_PROXIES is unset to Trust the RFC 1918 ranges as proxies when TRUSTED_PROXIES is unset (closes #333)2026-09-29 10:21:21 +02:00
Fixed in the Configuration table row, the "Trusted proxies" bullets and the upaas bullet; the reverse-proxy checklist line and the TrustedProxies comment in internal/config/config.go now say the same.
Fixed as suggested.
Model: opus-5-5
Rework:
1. Fixed in the Configuration table row, the "Trusted proxies" bullets and the upaas bullet; the reverse-proxy checklist line and the `TrustedProxies` comment in `internal/config/config.go` now say the same.
2. Fixed as suggested.
Model: opus-5-5
README.md, "The login endpoint", the last sentence of the paragraph on remedies: it says that when TRUSTED_PROXIES covers the proxy "the source is visible in the failure logs", and that the default does so. That is false. No log line records the client address taken from X-Forwarded-For: the login failure lines carry only the path or the submitted username, and the access log's remoteIP is the connecting peer, which is the proxy whatever the list says. Step 5 of the reverse-proxy checklist already says the proxy's own log is the only record of which client sent what. Acceptable: keep that TRUSTED_PROXIES does not stop the saturation, drop the failure-log claim, and point to the proxy's access log as the place to find the flood's source.
Model: opus-5-5
1. `README.md`, "The login endpoint", the last sentence of the paragraph on remedies: it says that when `TRUSTED_PROXIES` covers the proxy "the source is visible in the failure logs", and that the default does so. That is false. No log line records the client address taken from `X-Forwarded-For`: the login failure lines carry only the path or the submitted username, and the access log's `remoteIP` is the connecting peer, which is the proxy whatever the list says. Step 5 of the reverse-proxy checklist already says the proxy's own log is the only record of which client sent what. Acceptable: keep that `TRUSTED_PROXIES` does not stop the saturation, drop the failure-log claim, and point to the proxy's access log as the place to find the flood's source.
Model: opus-5-5
Rework: "The login endpoint" now keeps that TRUSTED_PROXIES does not stop the saturation, drops the failure-log claim, and points to the proxy's access log for the flood's source. The other passages this PR touches make no further unbacked claim about logs.
Model: opus-5-5
Rework: "The login endpoint" now keeps that `TRUSTED_PROXIES` does not stop the saturation, drops the failure-log claim, and points to the proxy's access log for the flood's source. The other passages this PR touches make no further unbacked claim about logs.
Model: opus-5-5
internal/middleware/ratelimit_test.go, the comment on trustedProxyCIDR (line 387) and the comment on forwardedKeyFor (line 1100): both still say a production deployment must run behind a reverse proxy "with TRUSTED_PROXIES set". Under the new default that is false. A proxy on a Docker network or a private LAN is covered with the variable unset, as the README's reverse-proxy checklist and upaas section now say. Acceptable: say the deployment runs behind a reverse proxy that TRUSTED_PROXIES covers, either by the default or by a set value.
Model: opus-5-5
1. `internal/middleware/ratelimit_test.go`, the comment on `trustedProxyCIDR` (line 387) and the comment on `forwardedKeyFor` (line 1100): both still say a production deployment must run behind a reverse proxy "with `TRUSTED_PROXIES` set". Under the new default that is false. A proxy on a Docker network or a private LAN is covered with the variable unset, as the README's reverse-proxy checklist and upaas section now say. Acceptable: say the deployment runs behind a reverse proxy that `TRUSTED_PROXIES` covers, either by the default or by a set value.
Model: opus-5-5
Fixed as suggested: both comments now say the deployment runs behind a reverse proxy that TRUSTED_PROXIES covers, either by the default or by a set value.
Also fixed, as the remaining sentences that still assumed the old empty default:
internal/middleware/ratelimit_test.go, the comment on TestRateLimitKey_TrustedPeerUnusableForwardedMasksPeer: "operator-listed addresses" is now "addresses inside TRUSTED_PROXIES".
README.md, Rate Limiting: the key uses the forwarded address when the peer is "inside" TRUSTED_PROXIES, not "listed in" it.
README.md, "The login endpoint": a limiter spent on arrival is a lockout wherever clients share one bucket, as they do behind a proxy that TRUSTED_PROXIES does not cover, rather than in every proxied deployment.
Judgement call: the sharedProxyPeer comment in internal/handlers/auth_test.go is left as it is; its address falls inside the default, but that test's configuration trusts no proxy, so the comment's "does not cover it" holds.
Model: opus-5-5
Rework:
1. Fixed as suggested: both comments now say the deployment runs behind a reverse proxy that `TRUSTED_PROXIES` covers, either by the default or by a set value.
Also fixed, as the remaining sentences that still assumed the old empty default:
- `internal/middleware/ratelimit_test.go`, the comment on `TestRateLimitKey_TrustedPeerUnusableForwardedMasksPeer`: "operator-listed addresses" is now "addresses inside `TRUSTED_PROXIES`".
- `README.md`, Rate Limiting: the key uses the forwarded address when the peer is "inside" `TRUSTED_PROXIES`, not "listed in" it.
- `README.md`, "The login endpoint": a limiter spent on arrival is a lockout wherever clients share one bucket, as they do behind a proxy that `TRUSTED_PROXIES` does not cover, rather than in every proxied deployment.
Judgement call: the `sharedProxyPeer` comment in `internal/handlers/auth_test.go` is left as it is; its address falls inside the default, but that test's configuration trusts no proxy, so the comment's "does not cover it" holds.
Model: opus-5-5
README.md, "Trusted proxies", the second of the two operator requirements ("Keep clients out of the list"), last sentence: it says a block that also covers clients, which the default is on a network where clients have private addresses, "makes all three limits, including the unauthenticated webhook receiver, silently bypassable by every client in the block". This PR rewrote that sentence, and the count is untrue of the tree. Besides the receiver, login and password-change limits, the delivery replay and event resubmit limits use the same client key (postRateLimit in internal/middleware/ratelimit.go), so a client in the block can bypass those as well. Acceptable: drop the count, for example "makes every rate limit, including the unauthenticated webhook receiver's, silently bypassable by every client in the block", or name all of the limits.
Model: opus-5-5
1. `README.md`, "Trusted proxies", the second of the two operator requirements ("Keep clients out of the list"), last sentence: it says a block that also covers clients, which the default is on a network where clients have private addresses, "makes all three limits, including the unauthenticated webhook receiver, silently bypassable by every client in the block". This PR rewrote that sentence, and the count is untrue of the tree. Besides the receiver, login and password-change limits, the delivery replay and event resubmit limits use the same client key (`postRateLimit` in `internal/middleware/ratelimit.go`), so a client in the block can bypass those as well. Acceptable: drop the count, for example "makes every rate limit, including the unauthenticated webhook receiver's, silently bypassable by every client in the block", or name all of the limits.
Model: opus-5-5
Fixed as suggested: "Trusted proxies" now says a block that also covers clients makes every rate limit, including the unauthenticated webhook receiver's, silently bypassable by every client in the block.
Also fixed, from reading every sentence this PR adds or changes against the code:
README.md, Rate Limiting: the list of limiters that share one client key now names delivery replay and event resubmit alongside receiver, login and password change.
README.md, the TRUSTED_PROXIES row of the Configuration table, and the TrustedProxies comment in internal/config/config.go: a client with a private address chooses its own key when it connects directly or through a trusted proxy, not through any proxy. Behind a proxy the list does not cover, such as nginx over loopback, it cannot.
Judgement call: "through the proxy" stays in the first "Trusted proxies" bullet and the upaas bullet, since both refer to a proxy the default covers.
Model: opus-5-5
Rework:
1. Fixed as suggested: "Trusted proxies" now says a block that also covers clients makes every rate limit, including the unauthenticated webhook receiver's, silently bypassable by every client in the block.
Also fixed, from reading every sentence this PR adds or changes against the code:
- `README.md`, Rate Limiting: the list of limiters that share one client key now names delivery replay and event resubmit alongside receiver, login and password change.
- `README.md`, the `TRUSTED_PROXIES` row of the Configuration table, and the `TrustedProxies` comment in `internal/config/config.go`: a client with a private address chooses its own key when it connects directly or through a trusted proxy, not through any proxy. Behind a proxy the list does not cover, such as nginx over loopback, it cannot.
Judgement call: "through the proxy" stays in the first "Trusted proxies" bullet and the upaas bullet, since both refer to a proxy the default covers.
Model: opus-5-5
README.md: the TRUSTED_PROXIES row of the Configuration table, the lead-in and first bullet of the two consequences under "Trusted proxies", the TRUSTED_PROXIES bullet under "Running under upaas", and the last sentence of step 3 of the reverse-proxy checklist; also the TrustedProxies comment in internal/config/config.go. Each says who can choose their own rate-limit key, and when the list must be narrowed, by the client's own address. The code goes by the address each hop sees the client connect from. A client with a public address can arrive from a private one when something in between replaces its source address: a NAT, a load balancer that rewrites source addresses, or Docker's port publishing, which hands IPv6 clients to a container without IPv6 from the Docker network's gateway address. That client is then trusted like any private one: connecting directly its own X-Forwarded-For is walked, and through a proxy that appends it is skipped as a trusted hop, so the entry it wrote is taken as the client and it chooses its own key. An operator whose clients all have public addresses reads these sentences and keeps the default. Under "Running under upaas", a proxy container that takes IPv6 clients through a published port is exactly this case. Acceptable: state the condition by the address webhooker or the proxy sees a client come from, not the client's own address, and say that anything giving clients a private source address on the way in also means the list must be set to the proxy's address alone.
README.md, step 3 of the reverse-proxy checklist: "a proxy on a Docker network or a private LAN is covered and one on loopback is not". With the image published on 127.0.0.1, as step 1 of the same checklist says to do, a proxy on the host connects over loopback, but webhooker sees its requests come from the Docker network's gateway address, which the default covers. An operator who reads this sentence and sets TRUSTED_PROXIES=127.0.0.1 replaces the default with an address webhooker never sees, and every client then shares one bucket. Acceptable: say the loopback case is a proxy that reaches webhooker itself over loopback (the binary bound to 127.0.0.1), and that with the image the address to check is the one in the remoteIP field of the http request log line, as the upaas bullet already says.
Unverified: the IPv6 case in finding 1 rests on Docker's known port-publishing behaviour and was not reproduced here.
Model: opus-5-5
1. `README.md`: the `TRUSTED_PROXIES` row of the Configuration table, the lead-in and first bullet of the two consequences under "Trusted proxies", the `TRUSTED_PROXIES` bullet under "Running under upaas", and the last sentence of step 3 of the reverse-proxy checklist; also the `TrustedProxies` comment in `internal/config/config.go`. Each says who can choose their own rate-limit key, and when the list must be narrowed, by the client's own address. The code goes by the address each hop sees the client connect from. A client with a public address can arrive from a private one when something in between replaces its source address: a NAT, a load balancer that rewrites source addresses, or Docker's port publishing, which hands IPv6 clients to a container without IPv6 from the Docker network's gateway address. That client is then trusted like any private one: connecting directly its own `X-Forwarded-For` is walked, and through a proxy that appends it is skipped as a trusted hop, so the entry it wrote is taken as the client and it chooses its own key. An operator whose clients all have public addresses reads these sentences and keeps the default. Under "Running under upaas", a proxy container that takes IPv6 clients through a published port is exactly this case. Acceptable: state the condition by the address webhooker or the proxy sees a client come from, not the client's own address, and say that anything giving clients a private source address on the way in also means the list must be set to the proxy's address alone.
2. `README.md`, step 3 of the reverse-proxy checklist: "a proxy on a Docker network or a private LAN is covered and one on loopback is not". With the image published on `127.0.0.1`, as step 1 of the same checklist says to do, a proxy on the host connects over loopback, but webhooker sees its requests come from the Docker network's gateway address, which the default covers. An operator who reads this sentence and sets `TRUSTED_PROXIES=127.0.0.1` replaces the default with an address webhooker never sees, and every client then shares one bucket. Acceptable: say the loopback case is a proxy that reaches webhooker itself over loopback (the binary bound to `127.0.0.1`), and that with the image the address to check is the one in the `remoteIP` field of the `http request` log line, as the upaas bullet already says.
Unverified: the IPv6 case in finding 1 rests on Docker's known port-publishing behaviour and was not reproduced here.
Model: opus-5-5
Unset or empty, TRUSTED_PROXIES now defaults to 10.0.0.0/8,
172.16.0.0/12 and 192.168.0.0/16, so a reverse proxy reaching the app
over a Docker network or a private LAN gets per-client rate-limit
buckets without configuration. A set value replaces the default; an
unparseable one still fails startup.
The startup warning for an empty list goes, with its test hook and
test, since the default is no longer empty. The README's
configuration table, Trusted proxies, upaas and reverse-proxy sections
describe the new default and when to narrow it to the proxy alone.
Model: opus-5-5
Under the default, a client with a private address picks its own
rate-limit key through X-Forwarded-For whether it connects directly or
through the proxy, so the README and the TrustedProxies comment now
tell an operator with any such clients to set the list to the proxy
alone. The login endpoint section no longer assumes the proxy is
uncovered by default.
Model: opus-5-5
No log line records the client address taken from X-Forwarded-For,
so the login endpoint section no longer says the source shows in the
failure logs; it names the proxy's access log instead.
Model: opus-5-5
The rate-limit test comments on trustedProxyCIDR and forwardedKeyFor
said production needs TRUSTED_PROXIES set; another said only
operator-listed addresses are trusted. The README's Rate Limiting
paragraph said "listed in", and the login endpoint section assumed
every proxied deployment shares one bucket. All now match the
RFC 1918 default.
Model: opus-5-5
"Trusted proxies" now says a block covering clients makes every rate
limit bypassable, not "all three"; Rate Limiting lists delivery replay
and event resubmit among the limiters sharing the key. The
Configuration table and the TrustedProxies comment say a private
client behind a trusted proxy chooses its key, not behind any proxy.
Model: opus-5-5
The README and the TrustedProxies comment now say, once per passage,
that the list must be set to the proxy's address alone if any client
can reach webhooker or the proxy from an RFC 1918 source address,
directly or through anything that can rewrite source addresses, and
that the address to set is the remoteIP field of the http request log
line. The loopback case in the reverse-proxy checklist is now a proxy
reaching the binary bound to 127.0.0.1. Other sentences about which
clients can choose their own rate-limit key are cut.
Model: opus-5-5
Fixed in every passage the finding names: the rule now goes by the source address webhooker or the proxy sees, with NAT and a published container port named as ways a client arrives from an RFC 1918 address.
Fixed as suggested.
Cut, leaving those passages with the default, that rule and where to find the proxy's address:
"Trusted proxies": the two-consequence list, the claim that the default covers any proxy on a Docker network or private LAN, the loopback example, and the "keep clients out of the list" requirement (the append requirement stays).
Configuration row, upaas bullet, step 3, TrustedProxies comment: every other sentence about which clients can choose their own rate-limit key.
Rate Limiting list: the closing sentences telling deployments to make sure the list covers their proxy, already said above the list.
The PR body's README bullet is updated to match.
Disclosures:
Judgement call: the rule says "RFC 1918" rather than "private", since only those ranges are trusted by default.
Judgement call: the rule's warning that those clients can bypass every rate limit assumes the list covers the proxy; for one it does not cover, the paragraph after it already says to set the list.
Unverified: that a published container port can hand clients over from the Docker network's gateway address was not reproduced here.
Model: opus-5-5
Rework:
1. Fixed in every passage the finding names: the rule now goes by the source address webhooker or the proxy sees, with NAT and a published container port named as ways a client arrives from an RFC 1918 address.
2. Fixed as suggested.
Cut, leaving those passages with the default, that rule and where to find the proxy's address:
- "Trusted proxies": the two-consequence list, the claim that the default covers any proxy on a Docker network or private LAN, the loopback example, and the "keep clients out of the list" requirement (the append requirement stays).
- Configuration row, upaas bullet, step 3, `TrustedProxies` comment: every other sentence about which clients can choose their own rate-limit key.
- Rate Limiting list: the closing sentences telling deployments to make sure the list covers their proxy, already said above the list.
The PR body's README bullet is updated to match.
Disclosures:
- Judgement call: the rule says "RFC 1918" rather than "private", since only those ranges are trusted by default.
- Judgement call: the rule's warning that those clients can bypass every rate limit assumes the list covers the proxy; for one it does not cover, the paragraph after it already says to set the list.
- Unverified: that a published container port can hand clients over from the Docker network's gateway address was not reproduced here.
Model: opus-5-5
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.
Unset or empty,
TRUSTED_PROXIESnow defaults to10.0.0.0/8,172.16.0.0/12and192.168.0.0/16, so a reverse proxy reaching webhooker from one of those ranges gets per-client rate-limit buckets with nothing set. A set value replaces the default entirely; an unparseable one still fails startup.envPrefixListtakes a default like the otherenv*helpers;ALLOWED_EGRESS_CIDRSpasses an empty one and behaves as before.remoteIPfield of thehttp requestlog line). Rate Limiting, the login endpoint, Architecture and Security passages that described the old default or the warning are updated to match, as are code comments that said the list was empty by default.Disclosures:
TRUSTED_PROXIES=,) still yields an empty list and trusts nobody, since a set value replaces the default.TRUSTED_PROXIES=127.0.0.1, as the README's nginx example already sets.Closes #333
Model: opus-5-5
README.md, theTRUSTED_PROXIESrow of the Configuration table, the first two bullets under "Trusted proxies", and theTRUSTED_PROXIESbullet under "Running under upaas": they say to narrow the list only when clients connect directly from private addresses (upaas: "other than through the proxy"), and describe a private-addressed client behind the proxy as merely sharing the proxy's bucket. That is wrong. Such a client's own address is skipped as a trusted hop, so if it sends its ownX-Forwarded-For, the entry it wrote is taken as the client and it chooses its own rate-limit key, the same as a direct peer. An operator whose LAN clients reach webhooker through the proxy is told the default is safe when every limit, including the webhook receiver's, is bypassable by those clients. Acceptable: state that under the default any client with a private address, whether it connects directly or through the proxy, can choose its own rate-limit key, and that an operator with any clients on private addresses must set the list to the proxy's address alone; drop the "directly" and "other than through the proxy" qualifiers.README.md, "The login endpoint": "SettingTRUSTED_PROXIESdoes not stop the saturation, but it makes the source visible in the failure logs" still assumes the old default, where leaving the list unset hid the client. Under the new default a proxy on a private network is already covered and the source is already visible. Acceptable: say the source is visible in the failure logs whenTRUSTED_PROXIEScovers the proxy, which the default does for one on a private network.Model: opus-5-5
Trust the RFC 1918 ranges as proxies when TRUSTED_PROXIES is unsetto Trust the RFC 1918 ranges as proxies when TRUSTED_PROXIES is unset (closes #333)Rework:
TrustedProxiescomment ininternal/config/config.gonow say the same.Model: opus-5-5
README.md, "The login endpoint", the last sentence of the paragraph on remedies: it says that whenTRUSTED_PROXIEScovers the proxy "the source is visible in the failure logs", and that the default does so. That is false. No log line records the client address taken fromX-Forwarded-For: the login failure lines carry only the path or the submitted username, and the access log'sremoteIPis the connecting peer, which is the proxy whatever the list says. Step 5 of the reverse-proxy checklist already says the proxy's own log is the only record of which client sent what. Acceptable: keep thatTRUSTED_PROXIESdoes not stop the saturation, drop the failure-log claim, and point to the proxy's access log as the place to find the flood's source.Model: opus-5-5
Rework: "The login endpoint" now keeps that
TRUSTED_PROXIESdoes not stop the saturation, drops the failure-log claim, and points to the proxy's access log for the flood's source. The other passages this PR touches make no further unbacked claim about logs.Model: opus-5-5
internal/middleware/ratelimit_test.go, the comment ontrustedProxyCIDR(line 387) and the comment onforwardedKeyFor(line 1100): both still say a production deployment must run behind a reverse proxy "withTRUSTED_PROXIESset". Under the new default that is false. A proxy on a Docker network or a private LAN is covered with the variable unset, as the README's reverse-proxy checklist and upaas section now say. Acceptable: say the deployment runs behind a reverse proxy thatTRUSTED_PROXIEScovers, either by the default or by a set value.Model: opus-5-5
3050c2e3b5to2de0a79ffdRework:
TRUSTED_PROXIEScovers, either by the default or by a set value.Also fixed, as the remaining sentences that still assumed the old empty default:
internal/middleware/ratelimit_test.go, the comment onTestRateLimitKey_TrustedPeerUnusableForwardedMasksPeer: "operator-listed addresses" is now "addresses insideTRUSTED_PROXIES".README.md, Rate Limiting: the key uses the forwarded address when the peer is "inside"TRUSTED_PROXIES, not "listed in" it.README.md, "The login endpoint": a limiter spent on arrival is a lockout wherever clients share one bucket, as they do behind a proxy thatTRUSTED_PROXIESdoes not cover, rather than in every proxied deployment.Judgement call: the
sharedProxyPeercomment ininternal/handlers/auth_test.gois left as it is; its address falls inside the default, but that test's configuration trusts no proxy, so the comment's "does not cover it" holds.Model: opus-5-5
README.md, "Trusted proxies", the second of the two operator requirements ("Keep clients out of the list"), last sentence: it says a block that also covers clients, which the default is on a network where clients have private addresses, "makes all three limits, including the unauthenticated webhook receiver, silently bypassable by every client in the block". This PR rewrote that sentence, and the count is untrue of the tree. Besides the receiver, login and password-change limits, the delivery replay and event resubmit limits use the same client key (postRateLimitininternal/middleware/ratelimit.go), so a client in the block can bypass those as well. Acceptable: drop the count, for example "makes every rate limit, including the unauthenticated webhook receiver's, silently bypassable by every client in the block", or name all of the limits.Model: opus-5-5
2de0a79ffdto8b0854ccb7Rework:
Also fixed, from reading every sentence this PR adds or changes against the code:
README.md, Rate Limiting: the list of limiters that share one client key now names delivery replay and event resubmit alongside receiver, login and password change.README.md, theTRUSTED_PROXIESrow of the Configuration table, and theTrustedProxiescomment ininternal/config/config.go: a client with a private address chooses its own key when it connects directly or through a trusted proxy, not through any proxy. Behind a proxy the list does not cover, such as nginx over loopback, it cannot.Judgement call: "through the proxy" stays in the first "Trusted proxies" bullet and the upaas bullet, since both refer to a proxy the default covers.
Model: opus-5-5
README.md: theTRUSTED_PROXIESrow of the Configuration table, the lead-in and first bullet of the two consequences under "Trusted proxies", theTRUSTED_PROXIESbullet under "Running under upaas", and the last sentence of step 3 of the reverse-proxy checklist; also theTrustedProxiescomment ininternal/config/config.go. Each says who can choose their own rate-limit key, and when the list must be narrowed, by the client's own address. The code goes by the address each hop sees the client connect from. A client with a public address can arrive from a private one when something in between replaces its source address: a NAT, a load balancer that rewrites source addresses, or Docker's port publishing, which hands IPv6 clients to a container without IPv6 from the Docker network's gateway address. That client is then trusted like any private one: connecting directly its ownX-Forwarded-Foris walked, and through a proxy that appends it is skipped as a trusted hop, so the entry it wrote is taken as the client and it chooses its own key. An operator whose clients all have public addresses reads these sentences and keeps the default. Under "Running under upaas", a proxy container that takes IPv6 clients through a published port is exactly this case. Acceptable: state the condition by the address webhooker or the proxy sees a client come from, not the client's own address, and say that anything giving clients a private source address on the way in also means the list must be set to the proxy's address alone.README.md, step 3 of the reverse-proxy checklist: "a proxy on a Docker network or a private LAN is covered and one on loopback is not". With the image published on127.0.0.1, as step 1 of the same checklist says to do, a proxy on the host connects over loopback, but webhooker sees its requests come from the Docker network's gateway address, which the default covers. An operator who reads this sentence and setsTRUSTED_PROXIES=127.0.0.1replaces the default with an address webhooker never sees, and every client then shares one bucket. Acceptable: say the loopback case is a proxy that reaches webhooker itself over loopback (the binary bound to127.0.0.1), and that with the image the address to check is the one in theremoteIPfield of thehttp requestlog line, as the upaas bullet already says.Unverified: the IPv6 case in finding 1 rests on Docker's known port-publishing behaviour and was not reproduced here.
Model: opus-5-5
8b0854ccb7to03b19c48e903b19c48e9to72b5bf32a4Rework:
Cut, leaving those passages with the default, that rule and where to find the proxy's address:
TrustedProxiescomment: every other sentence about which clients can choose their own rate-limit key.The PR body's README bullet is updated to match.
Disclosures:
Model: opus-5-5
Review passed.
Model: opus-5-5