The access log writes one INFO line with the full attacker-controlled URL per request, including rejected ones #146
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Disclosed by the rework of #143, which deliberately did not fix it.
NOT milestoned 1.0.0. It is pre-existing, it is what essentially every HTTP access log does, and #143 halved the volume rather than adding to it. Filed so the remaining amplification is written down rather than discovered later.
internal/middleware/middleware.go:121logs one INFO line per request containingr.URL.String()— the full URL including query. It is registered withUse, so it runs before the route limiter and records rejected requests too.Net: a client flooding
/webhook/with invented paths still writes O(requests) INFO lines carrying attacker-chosen text, on the unauthenticated receiver. #139 bounded the database work and removed the second log line; this is the one that remains.Bounding it is genuinely a design decision, not a bug fix, which is why it was correctly left out of #139: any change here alters observability for every route in the service, not just the receiver.
The decision to make
/webhook/*./webhook/{uuid}) rather than the concrete URL for 4xx responses, keeping the line but dropping the attacker-controlled portion.Whichever is chosen, the reasoning should end up in the README's logging section so an operator sizing log storage knows what an unauthenticated flood costs them.
Definition of done
Implementation requirements
next, PR based onnext, single commit, title ending(closes #N).TODO.md.make checkplus the Docker lint path with the cache defeated (#119).Moved INTO the
1.0.0milestone, reversing the "NOT milestoned" line in the body above.Reason: the 1.0 bar is what an unauthenticated attacker on the public internet can do. Here that is write unbounded attacker-chosen text into the operator's logs, at one INFO line per request, on the one endpoint with no auth in front of it — and
Useregistration means rejected requests count too. Log volume is an operator's disk and bill. "Every access log does this" is true of authenticated admin surfaces; it is not an answer for an internet-exposed receiver that 404s invented paths for free.Decision on the three options in the body: take option 2 — for 4xx responses log the chi ROUTE PATTERN, not the concrete URL. It keeps one line per request (so real traffic stays observable and rate accounting still works) while removing the attacker-controlled portion, and unlike sampling it does not silently drop evidence during an actual incident. Option 3 is rejected: rotation bounds disk but not the injection of arbitrary text into logs an operator greps.
clawbot referenced this issue2026-08-17 23:50:11 +02:00