The access log records the proxy's address, never the client's, so abuse cannot be traced from webhooker's own logs #270

Open
opened 2026-08-24 02:11:49 +02:00 by clawbot · 0 comments
Collaborator

Found by a TLS deployment audit running real nginx in front of the app.

With TRUSTED_PROXIES=127.0.0.1/32 correctly set, and the rate limiter demonstrably keying on the true client address, every access log line still reads "remoteIP":"127.0.0.1".

Verified: two genuinely distinct clients (172.17.0.2 and 172.17.0.3, separate containers) hammered the receiver into 429. grep '"remoteIP"' | sort | uniq -c returned 15 "remoteIP":"127.0.0.1" — the proxy, every time. Rate-limit rejection lines carry no client identity at all, and the CSRF-failure WARN's remote_addr is likewise the proxy.

So the app already computes the real client address for rate limiting and then throws it away before logging. In the deployed shape, abuse forensics from webhooker's own logs are impossible: an operator seeing a flood of 429s cannot tell whether it is one attacker or a thousand.

Mitigated because nginx's own access log has the client IP — but correlating the two requires the operator to configure and log X-Request-Id, which the README never mentions. Not milestoned: the information exists at the proxy, so this is a convenience and forensics gap rather than a hole.

Definition of done:

  • The access log carries the client address the rate limiter already computed, not the peer address, whenever a trusted proxy supplied it. Keep the peer address available too — distinguishing "who connected" from "who the request is attributed to" is the point, and collapsing them loses information.
  • Rate-limit rejection lines carry the same client identity, since those are the lines an operator investigating abuse reads first.
  • The CSRF-failure WARN likewise.
  • Document the field's meaning, including that it is only trustworthy when TRUSTED_PROXIES is set correctly — an attributed address derived from an untrusted X-Forwarded-For is attacker-controlled and must not be presented as authoritative.
Found by a TLS deployment audit running real nginx in front of the app. With `TRUSTED_PROXIES=127.0.0.1/32` correctly set, and the rate limiter demonstrably keying on the true client address, every access log line still reads `"remoteIP":"127.0.0.1"`. Verified: two genuinely distinct clients (`172.17.0.2` and `172.17.0.3`, separate containers) hammered the receiver into `429`. `grep '"remoteIP"' | sort | uniq -c` returned `15 "remoteIP":"127.0.0.1"` — the proxy, every time. Rate-limit rejection lines carry no client identity at all, and the CSRF-failure WARN's `remote_addr` is likewise the proxy. So the app already computes the real client address for rate limiting and then throws it away before logging. In the deployed shape, abuse forensics from webhooker's own logs are impossible: an operator seeing a flood of 429s cannot tell whether it is one attacker or a thousand. Mitigated because nginx's own access log has the client IP — but correlating the two requires the operator to configure and log `X-Request-Id`, which the README never mentions. Not milestoned: the information exists at the proxy, so this is a convenience and forensics gap rather than a hole. Definition of done: - The access log carries the client address the rate limiter already computed, not the peer address, whenever a trusted proxy supplied it. Keep the peer address available too — distinguishing "who connected" from "who the request is attributed to" is the point, and collapsing them loses information. - Rate-limit rejection lines carry the same client identity, since those are the lines an operator investigating abuse reads first. - The CSRF-failure WARN likewise. - Document the field's meaning, including that it is only trustworthy when `TRUSTED_PROXIES` is set correctly — an attributed address derived from an untrusted `X-Forwarded-For` is attacker-controlled and must not be presented as authoritative.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#270