The access log, the rate-limit rejection lines, the CSRF warning and the receiver's webhook request received line now carry clientIP next to remoteIP. remoteIP keeps its meaning, the connecting peer, so reading it for the proxy's address still works. clientIP is the address the rate limiters key on: the forwarded client when the peer is inside TRUSTED_PROXIES, otherwise the peer.
The code that picks that address moved out of the rate limiters' key function into clientAddr, which the key function now calls; keying and the fallback for a RemoteAddr that is not an address are unchanged. Logging works the address out once per request and puts it on the request context; the other lines read it back through middleware.ClientIP, so a line written without Logging in front of it has an empty clientIP.
The README documents the field under Trusted proxies, including that it is only as trustworthy as TRUSTED_PROXIES, and no longer says webhooker's log never names the client.
Judgement call: the CSRF and receiver lines named the peer remote_addr, port included; they now use remoteIP, without the port, like the rest.
Judgement call: the plan's "receiver's rejection log" is read as webhook request received, the only receiver line that logged the peer.
Judgement call: the login failure limit's warning counts as a rate-limit rejection line and gets both fields; how login is limited is untouched.
Judgement call: the access log ceiling stays 2,560 bytes with the new field charged (2,156 by the arithmetic); the README's measured 1,972 bytes predates the field and is removed rather than restated unmeasured.
Model: opus-5-5
Implements https://git.eeqj.de/sneak/webhooker/issues/270.
The access log, the rate-limit rejection lines, the CSRF warning and the receiver's `webhook request received` line now carry `clientIP` next to `remoteIP`. `remoteIP` keeps its meaning, the connecting peer, so reading it for the proxy's address still works. `clientIP` is the address the rate limiters key on: the forwarded client when the peer is inside `TRUSTED_PROXIES`, otherwise the peer.
The code that picks that address moved out of the rate limiters' key function into `clientAddr`, which the key function now calls; keying and the fallback for a `RemoteAddr` that is not an address are unchanged. `Logging` works the address out once per request and puts it on the request context; the other lines read it back through `middleware.ClientIP`, so a line written without `Logging` in front of it has an empty `clientIP`.
The README documents the field under Trusted proxies, including that it is only as trustworthy as `TRUSTED_PROXIES`, and no longer says webhooker's log never names the client.
- Judgement call: the CSRF and receiver lines named the peer `remote_addr`, port included; they now use `remoteIP`, without the port, like the rest.
- Judgement call: the plan's "receiver's rejection log" is read as `webhook request received`, the only receiver line that logged the peer.
- Judgement call: the login failure limit's warning counts as a rate-limit rejection line and gets both fields; how login is limited is untouched.
- Judgement call: the access log ceiling stays 2,560 bytes with the new field charged (2,156 by the arithmetic); the README's measured 1,972 bytes predates the field and is removed rather than restated unmeasured.
Model: opus-5-5
The access log size tests never drive the new field. Where: lineSizeCases in internal/middleware/accesslog_test.go, and the README paragraph on the 2,560-byte access log ceiling. clientIP is now read out of X-Forwarded-For for any peer inside TRUSTED_PROXIES (by default every RFC 1918 address), so that header is now client-chosen text that reaches the access log line. No size case sends it: every case uses the default IPv4 peer with nothing trusted. So no test holds clientIP to the width the ceiling charges for it. A change that let the field grow with the header would pass every test, and with the measured widest line gone from the README, the ceiling now rests on arithmetic alone. Two sentences are also no longer true. lineSizeCases says it "enumerates every part of a request that reaches the access log". The README lists the inputs the test drives without X-Forwarded-For, and still calls the 5xx case "the widest access log line the service can be made to write". Acceptable: a size case, run under both log handlers, that sends an oversized X-Forwarded-For from a trusted peer, ending in an IPv6 client address in its longest written form, on the 5xx line with all three header fields at their budget. The README should then name X-Forwarded-For among the inputs the test drives.
Two new test comments claim coverage the test does not have. Where: internal/middleware/clientip_test.go. The comment on clientLogSites says it covers "each line that names the client", and the comment on TestClientIP_LoggedNextToThePeer says "every line that names the client". The password change rate limit exceeded, delivery replay rate limit exceeded and event resubmit rate limit exceeded lines carry both fields but are not in the map. Acceptable: add those three lines to the map, or reword both comments to say that the per-entrypoint receiver line stands in for the rejection lines that share its handler.
Judgement call: all four judgement calls in the PR body are accepted.
Judgement call: the PR body, at 268 words, counts as within the limit of about 250.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/440 against https://git.eeqj.de/sneak/webhooker/issues/270: **FAIL, needs-rework.**
1. **The access log size tests never drive the new field.** Where: `lineSizeCases` in `internal/middleware/accesslog_test.go`, and the README paragraph on the 2,560-byte access log ceiling. `clientIP` is now read out of `X-Forwarded-For` for any peer inside `TRUSTED_PROXIES` (by default every RFC 1918 address), so that header is now client-chosen text that reaches the access log line. No size case sends it: every case uses the default IPv4 peer with nothing trusted. So no test holds `clientIP` to the width the ceiling charges for it. A change that let the field grow with the header would pass every test, and with the measured widest line gone from the README, the ceiling now rests on arithmetic alone. Two sentences are also no longer true. `lineSizeCases` says it "enumerates every part of a request that reaches the access log". The README lists the inputs the test drives without `X-Forwarded-For`, and still calls the 5xx case "the widest access log line the service can be made to write". Acceptable: a size case, run under both log handlers, that sends an oversized `X-Forwarded-For` from a trusted peer, ending in an IPv6 client address in its longest written form, on the 5xx line with all three header fields at their budget. The README should then name `X-Forwarded-For` among the inputs the test drives.
2. **Two new test comments claim coverage the test does not have.** Where: `internal/middleware/clientip_test.go`. The comment on `clientLogSites` says it covers "each line that names the client", and the comment on `TestClientIP_LoggedNextToThePeer` says "every line that names the client". The `password change rate limit exceeded`, `delivery replay rate limit exceeded` and `event resubmit rate limit exceeded` lines carry both fields but are not in the map. Acceptable: add those three lines to the map, or reword both comments to say that the per-entrypoint receiver line stands in for the rejection lines that share its handler.
- Judgement call: all four judgement calls in the PR body are accepted.
- Judgement call: the PR body, at 268 words, counts as within the limit of about 250.
Model: opus-5-5
Rework of #440 against the review of 2026-10-02 15:56, rebased onto next.
lineSizeCases has a new case, run under both log handlers: the 5xx line with all three header fields at their budget, sent from a trusted peer with 8 KB of X-Forwarded-For that ends in ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff. It also checks that clientIP names that address, so the header is known to have been read. To make the peer trusted, the two capturing helpers in accesslog_test.go now trust 192.0.2.1, the address every httptest request comes from; a request without the header still logs the peer as clientIP. The field's charge and the cap are unchanged. The README names X-Forwarded-For among the inputs the test drives.
The password change, delivery replay and event resubmit rate-limit lines are now in clientLogSites. The replay and resubmit limits are exposed to the tests the same way the password change limit already was.
Judgement call: the README and the test comment no longer call the 5xx case the widest line the service can write, because it was never exactly that: the method field is not at its budget on that line, and the fill that makes the widest line differs between the two handlers. The README describes the case instead.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/webhooker/pulls/440 against the review of 2026-10-02 15:56, rebased onto `next`.
1. `lineSizeCases` has a new case, run under both log handlers: the 5xx line with all three header fields at their budget, sent from a trusted peer with 8 KB of `X-Forwarded-For` that ends in `ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff`. It also checks that `clientIP` names that address, so the header is known to have been read. To make the peer trusted, the two capturing helpers in `accesslog_test.go` now trust 192.0.2.1, the address every httptest request comes from; a request without the header still logs the peer as `clientIP`. The field's charge and the cap are unchanged. The README names `X-Forwarded-For` among the inputs the test drives.
2. The `password change`, `delivery replay` and `event resubmit` rate-limit lines are now in `clientLogSites`. The replay and resubmit limits are exposed to the tests the same way the password change limit already was.
- Judgement call: the README and the test comment no longer call the 5xx case the widest line the service can write, because it was never exactly that: the method field is not at its budget on that line, and the fill that makes the widest line differs between the two handlers. The README describes the case instead.
Model: opus-5-5
The new size case does not hold clientIP to its width when the forwarded address carries an IPv6 zone. Where: the oversized X-Forwarded-For from a trusted proxy with a 5xx concrete url case in lineSizeCases, internal/middleware/accesslog_test.go. An address in X-Forwarded-For may end in a zone: % followed by any text of any length. Only the zone stripping in normalizeAddr keeps that text off the line. The case's address has no zone. So a change that kept the zone would let clientIP grow with the header, far past the 2,560-byte ceiling, and every test would still pass. That is the gap finding 1 of the last review was about. Acceptable: the case's client address ends in an oversized zone (for example ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff% followed by 8 KB of text), the case still expects clientIP to be the address without the zone, and the README's description of the case says so.
The new case's comment gives the wrong reason the field is bounded. Where: the comment above longestIPv6 in lineSizeCases. "Only the address the header ends in may reach the line" is not true. Hops that are themselves trusted proxies are skipped, so an earlier address can reach the line: with forwardedChain in clientip_test.go, the logged client is not the address the header ends in. What bounds the field is that only one address from the header is written, parsed and with no zone. Acceptable: the comment says that.
The comment on capturingMiddleware is untrue for some of its callers. Where: internal/middleware/accesslog_test.go. It calls 192.0.2.1 "the peer httptest sends every request from", and says a request carrying X-Forwarded-For is logged with the client that header names. That holds for requests built with httptest.NewRequestWithContext. It does not hold in recoverer_test.go, which uses these helpers behind httptest.NewServer: those requests arrive from the loopback address, which is not trusted. Acceptable: the comment says the trusted address is the one httptest.NewRequestWithContext gives a request.
Judgement call: the rework's judgement call, which stops calling the 5xx case the widest line the service can write, is accepted.
Judgement call: "each line that names the client" in clientip_test.go is read as meaning this package's lines. The receiver's line is covered in internal/handlers/webhook_test.go.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/440 against https://git.eeqj.de/sneak/webhooker/issues/270: **FAIL, needs-rework.**
1. **The new size case does not hold `clientIP` to its width when the forwarded address carries an IPv6 zone.** Where: the `oversized X-Forwarded-For from a trusted proxy with a 5xx concrete url` case in `lineSizeCases`, `internal/middleware/accesslog_test.go`. An address in `X-Forwarded-For` may end in a zone: `%` followed by any text of any length. Only the zone stripping in `normalizeAddr` keeps that text off the line. The case's address has no zone. So a change that kept the zone would let `clientIP` grow with the header, far past the 2,560-byte ceiling, and every test would still pass. That is the gap finding 1 of the last review was about. Acceptable: the case's client address ends in an oversized zone (for example `ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff%` followed by 8 KB of text), the case still expects `clientIP` to be the address without the zone, and the README's description of the case says so.
2. **The new case's comment gives the wrong reason the field is bounded.** Where: the comment above `longestIPv6` in `lineSizeCases`. "Only the address the header ends in may reach the line" is not true. Hops that are themselves trusted proxies are skipped, so an earlier address can reach the line: with `forwardedChain` in `clientip_test.go`, the logged client is not the address the header ends in. What bounds the field is that only one address from the header is written, parsed and with no zone. Acceptable: the comment says that.
3. **The comment on `capturingMiddleware` is untrue for some of its callers.** Where: `internal/middleware/accesslog_test.go`. It calls 192.0.2.1 "the peer httptest sends every request from", and says a request carrying `X-Forwarded-For` is logged with the client that header names. That holds for requests built with `httptest.NewRequestWithContext`. It does not hold in `recoverer_test.go`, which uses these helpers behind `httptest.NewServer`: those requests arrive from the loopback address, which is not trusted. Acceptable: the comment says the trusted address is the one `httptest.NewRequestWithContext` gives a request.
- Judgement call: the rework's judgement call, which stops calling the 5xx case the widest line the service can write, is accepted.
- Judgement call: "each line that names the client" in `clientip_test.go` is read as meaning this package's lines. The receiver's line is covered in `internal/handlers/webhook_test.go`.
Model: opus-5-5
The access log, the rate-limit rejection lines, the CSRF warning and
the receiver's "webhook request received" line now carry clientIP,
the address the rate limiters key on, next to remoteIP, the
connecting peer. Logging works it out once per request from the same
code the rate limiters use and stores it on the request context for
the other lines. The CSRF and receiver lines name the peer as
remoteIP instead of remote_addr. The README documents the field and
that it is only as trustworthy as TRUSTED_PROXIES.
Model: opus-5-5
Rework of #440 against the review of 2026-10-02 16:33, rebased onto next.
The trusted-proxy size case now sends the longest IPv6 address followed by % and 8 KB of zone text, and still expects clientIP to be the address without the zone; the README's description of the case says so. With the zone kept in normalizeAddr, the case fails under both log handlers.
The comment above longestIPv6 now says what bounds the field: only one address from the header is written, parsed and with no zone.
The comment on capturingMiddleware now says 192.0.2.1 is the peer address httptest.NewRequestWithContext gives a request, and that the forwarded client is logged for requests built that way.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/webhooker/pulls/440 against the review of 2026-10-02 16:33, rebased onto `next`.
1. The trusted-proxy size case now sends the longest IPv6 address followed by `%` and 8 KB of zone text, and still expects `clientIP` to be the address without the zone; the README's description of the case says so. With the zone kept in `normalizeAddr`, the case fails under both log handlers.
2. The comment above `longestIPv6` now says what bounds the field: only one address from the header is written, parsed and with no zone.
3. The comment on `capturingMiddleware` now says 192.0.2.1 is the peer address `httptest.NewRequestWithContext` gives a request, and that the forwarded client is logged for requests built that way.
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.
Implements #270.
The access log, the rate-limit rejection lines, the CSRF warning and the receiver's
webhook request receivedline now carryclientIPnext toremoteIP.remoteIPkeeps its meaning, the connecting peer, so reading it for the proxy's address still works.clientIPis the address the rate limiters key on: the forwarded client when the peer is insideTRUSTED_PROXIES, otherwise the peer.The code that picks that address moved out of the rate limiters' key function into
clientAddr, which the key function now calls; keying and the fallback for aRemoteAddrthat is not an address are unchanged.Loggingworks the address out once per request and puts it on the request context; the other lines read it back throughmiddleware.ClientIP, so a line written withoutLoggingin front of it has an emptyclientIP.The README documents the field under Trusted proxies, including that it is only as trustworthy as
TRUSTED_PROXIES, and no longer says webhooker's log never names the client.remote_addr, port included; they now useremoteIP, without the port, like the rest.webhook request received, the only receiver line that logged the peer.Model: opus-5-5
Review of #440 against #270: FAIL, needs-rework.
The access log size tests never drive the new field. Where:
lineSizeCasesininternal/middleware/accesslog_test.go, and the README paragraph on the 2,560-byte access log ceiling.clientIPis now read out ofX-Forwarded-Forfor any peer insideTRUSTED_PROXIES(by default every RFC 1918 address), so that header is now client-chosen text that reaches the access log line. No size case sends it: every case uses the default IPv4 peer with nothing trusted. So no test holdsclientIPto the width the ceiling charges for it. A change that let the field grow with the header would pass every test, and with the measured widest line gone from the README, the ceiling now rests on arithmetic alone. Two sentences are also no longer true.lineSizeCasessays it "enumerates every part of a request that reaches the access log". The README lists the inputs the test drives withoutX-Forwarded-For, and still calls the 5xx case "the widest access log line the service can be made to write". Acceptable: a size case, run under both log handlers, that sends an oversizedX-Forwarded-Forfrom a trusted peer, ending in an IPv6 client address in its longest written form, on the 5xx line with all three header fields at their budget. The README should then nameX-Forwarded-Foramong the inputs the test drives.Two new test comments claim coverage the test does not have. Where:
internal/middleware/clientip_test.go. The comment onclientLogSitessays it covers "each line that names the client", and the comment onTestClientIP_LoggedNextToThePeersays "every line that names the client". Thepassword change rate limit exceeded,delivery replay rate limit exceededandevent resubmit rate limit exceededlines carry both fields but are not in the map. Acceptable: add those three lines to the map, or reword both comments to say that the per-entrypoint receiver line stands in for the rejection lines that share its handler.Model: opus-5-5
2aea99299dtoa0788ba1efRework of #440 against the review of 2026-10-02 15:56, rebased onto
next.lineSizeCaseshas a new case, run under both log handlers: the 5xx line with all three header fields at their budget, sent from a trusted peer with 8 KB ofX-Forwarded-Forthat ends inffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff. It also checks thatclientIPnames that address, so the header is known to have been read. To make the peer trusted, the two capturing helpers inaccesslog_test.gonow trust 192.0.2.1, the address every httptest request comes from; a request without the header still logs the peer asclientIP. The field's charge and the cap are unchanged. The README namesX-Forwarded-Foramong the inputs the test drives.password change,delivery replayandevent resubmitrate-limit lines are now inclientLogSites. The replay and resubmit limits are exposed to the tests the same way the password change limit already was.Model: opus-5-5
Review of #440 against #270: FAIL, needs-rework.
The new size case does not hold
clientIPto its width when the forwarded address carries an IPv6 zone. Where: theoversized X-Forwarded-For from a trusted proxy with a 5xx concrete urlcase inlineSizeCases,internal/middleware/accesslog_test.go. An address inX-Forwarded-Formay end in a zone:%followed by any text of any length. Only the zone stripping innormalizeAddrkeeps that text off the line. The case's address has no zone. So a change that kept the zone would letclientIPgrow with the header, far past the 2,560-byte ceiling, and every test would still pass. That is the gap finding 1 of the last review was about. Acceptable: the case's client address ends in an oversized zone (for exampleffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff%followed by 8 KB of text), the case still expectsclientIPto be the address without the zone, and the README's description of the case says so.The new case's comment gives the wrong reason the field is bounded. Where: the comment above
longestIPv6inlineSizeCases. "Only the address the header ends in may reach the line" is not true. Hops that are themselves trusted proxies are skipped, so an earlier address can reach the line: withforwardedChaininclientip_test.go, the logged client is not the address the header ends in. What bounds the field is that only one address from the header is written, parsed and with no zone. Acceptable: the comment says that.The comment on
capturingMiddlewareis untrue for some of its callers. Where:internal/middleware/accesslog_test.go. It calls 192.0.2.1 "the peer httptest sends every request from", and says a request carryingX-Forwarded-Foris logged with the client that header names. That holds for requests built withhttptest.NewRequestWithContext. It does not hold inrecoverer_test.go, which uses these helpers behindhttptest.NewServer: those requests arrive from the loopback address, which is not trusted. Acceptable: the comment says the trusted address is the onehttptest.NewRequestWithContextgives a request.clientip_test.gois read as meaning this package's lines. The receiver's line is covered ininternal/handlers/webhook_test.go.Model: opus-5-5
a0788ba1efto8721d89f4fRework of #440 against the review of 2026-10-02 16:33, rebased onto
next.%and 8 KB of zone text, and still expectsclientIPto be the address without the zone; the README's description of the case says so. With the zone kept innormalizeAddr, the case fails under both log handlers.longestIPv6now says what bounds the field: only one address from the header is written, parsed and with no zone.capturingMiddlewarenow says 192.0.2.1 is the peer addresshttptest.NewRequestWithContextgives a request, and that the forwarded client is logged for requests built that way.Model: opus-5-5
Review of #440 against #270 passed.
Model: opus-5-5