The access log writes one INFO line with the full attacker-controlled URL per request, including rejected ones #146

New Issue

clawbot · 2026-08-12T12:57:46+02:00

clawbot commented

2026-08-12 12:57:46 +02:00

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:121 logs one INFO line per request containing r.URL.String() — the full URL including query. It is registered with Use, 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

Sample or suppress access-log lines for rejected requests, at least on /webhook/*.
Or log the route pattern (/webhook/{uuid}) rather than the concrete URL for 4xx responses, keeping the line but dropping the attacker-controlled portion.
Or accept it and rely on log rotation, which is the status quo and a legitimate answer.

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

A path-varying flood produces a bounded number of log lines, or an explicit documented decision that it does not need to.
If bounded: a test asserting the line count does not grow linearly with a flood.
No loss of observability for legitimate traffic — the access log must still record real requests usefully.

Implementation requirements

Branch from next, PR based on next, single commit, title ending (closes #N).
Do not modify TODO.md.
Gate on make check plus the Docker lint path with the cache defeated (#119).

Disclosed by the rework of https://git.eeqj.de/sneak/webhooker/pulls/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 https://git.eeqj.de/sneak/webhooker/pulls/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:121` logs one INFO line per request containing `r.URL.String()` — the full URL including query. It is registered with `Use`, 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. https://git.eeqj.de/sneak/webhooker/issues/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 https://git.eeqj.de/sneak/webhooker/issues/139: any change here alters observability for every route in the service, not just the receiver. ## The decision to make - Sample or suppress access-log lines for rejected requests, at least on `/webhook/*`. - Or log the route pattern (`/webhook/{uuid}`) rather than the concrete URL for 4xx responses, keeping the line but dropping the attacker-controlled portion. - Or accept it and rely on log rotation, which is the status quo and a legitimate answer. 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 - A path-varying flood produces a bounded number of log lines, or an explicit documented decision that it does not need to. - If bounded: a test asserting the line count does not grow linearly with a flood. - No loss of observability for legitimate traffic — the access log must still record real requests usefully. ## Implementation requirements - Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`. - Do not modify `TODO.md`. - Gate on `make check` plus the Docker lint path with the cache defeated (https://git.eeqj.de/sneak/webhooker/issues/119).

clawbot self-assigned this 2026-08-12 12:57:46 +02:00

clawbot referenced this issue

2026-08-12 13:04:22 +02:00

Bound the receiver rate limit per client IP across /webhook/* (closes #139) #143

clawbot referenced this issue

2026-08-12 13:13:47 +02:00

Bound the receiver rate limit per client IP across /webhook/* (closes #139) #143

Sign in to join this conversation.