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
Moved INTO the 1.0.0 milestone, 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 Use registration 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.
Moved INTO the `1.0.0` milestone, 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 `Use` registration 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.
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.
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