Found by the independent review of #174 while sweeping for siblings of the credential leak. Pre-existing, and outside that PR's scope.
internal/middleware/middleware.go:616-622 — the MaxBodySize rejection path logs r.URL.Pathuntruncated, at Warn level, so it is on by default. And internal/server/routes.go:128-131 places MaxBodySizeahead ofRequireAuth.
So an unauthenticated POST /source/<8 KB of client-chosen text>/edit carrying an oversize declared Content-Length writes an arbitrary-length attacker-chosen path into the operator's log — bypassing the 2,560-byte per-line budget that #146 just established, because that budget lives in the access-log field capping and this is a separate log call.
Milestoned 1.0.0 for consistency: this is exactly the defect class #146 was moved into the milestone for — an unauthenticated client on the public internet writing unbounded attacker-chosen text into the operator's logs. That fix is incomplete while this path exists, and leaving it would mean the stated per-line bound is true of one log call and false of another, which is worse than not stating it.
Note it does NOT reopen #160: there is no query string involved, only the path.
Definition of done
The path in this log line is bounded, using the same field-capping mechanism #146 introduced rather than a second ad-hoc truncation — one budget, one implementation.
Audit every other log call for the same shape. This one was found by accident; the access-log sweep did not cover it because it is not the access log. Any slog call reaching a client-controlled value — path, header, form field, URL — before or independently of the access-log capping needs the same treatment or an explicit reason. List what you checked.
Consider whether MaxBodySize should sit behind RequireAuth instead. It probably should not — an oversize body ought to be rejected before any session work — so if you leave the ordering, say why in a comment, since the ordering is what makes this reachable unauthenticated.
A test that a flood of oversize requests with long client-chosen paths does not produce log lines that grow with the input.
Implementation requirements
Branch from next, PR based on next, single commit, title ending (closes #N).
Run make bootstrap in a fresh clone before gating — browser assets are fetched at build time, and make lint now needs Docker.
Gate on make check plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.
Found by the independent review of https://git.eeqj.de/sneak/webhooker/pulls/174 while sweeping for siblings of the credential leak. Pre-existing, and outside that PR's scope.
`internal/middleware/middleware.go:616-622` — the `MaxBodySize` rejection path logs `r.URL.Path` **untruncated**, at `Warn` level, so it is on by default. And `internal/server/routes.go:128-131` places `MaxBodySize` **ahead of** `RequireAuth`.
So an unauthenticated `POST /source/<8 KB of client-chosen text>/edit` carrying an oversize declared `Content-Length` writes an arbitrary-length attacker-chosen path into the operator's log — bypassing the 2,560-byte per-line budget that https://git.eeqj.de/sneak/webhooker/issues/146 just established, because that budget lives in the access-log field capping and this is a separate log call.
Milestoned `1.0.0` for consistency: this is exactly the defect class https://git.eeqj.de/sneak/webhooker/issues/146 was moved into the milestone for — an unauthenticated client on the public internet writing unbounded attacker-chosen text into the operator's logs. That fix is incomplete while this path exists, and leaving it would mean the stated per-line bound is true of one log call and false of another, which is worse than not stating it.
Note it does NOT reopen https://git.eeqj.de/sneak/webhooker/issues/160: there is no query string involved, only the path.
## Definition of done
- The path in this log line is bounded, using the same field-capping mechanism https://git.eeqj.de/sneak/webhooker/issues/146 introduced rather than a second ad-hoc truncation — one budget, one implementation.
- **Audit every other log call for the same shape.** This one was found by accident; the access-log sweep did not cover it because it is not the access log. Any `slog` call reaching a client-controlled value — path, header, form field, URL — before or independently of the access-log capping needs the same treatment or an explicit reason. List what you checked.
- Consider whether `MaxBodySize` should sit behind `RequireAuth` instead. It probably should not — an oversize body ought to be rejected before any session work — so if you leave the ordering, say why in a comment, since the ordering is what makes this reachable unauthenticated.
- A test that a flood of oversize requests with long client-chosen paths does not produce log lines that grow with the input.
## Implementation requirements
- Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`.
- Do not modify `TODO.md` (see https://git.eeqj.de/sneak/webhooker/issues/112).
- Run `make bootstrap` in a fresh clone before gating — browser assets are fetched at build time, and `make lint` now needs Docker.
- Gate on `make check` plus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.
clawbot
added this to the 1.0.0 milestone 2026-08-18 01:09:18 +02:00
clawbot
self-assigned this 2026-08-18 01:09:18 +02:00
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.
Found by the independent review of #174 while sweeping for siblings of the credential leak. Pre-existing, and outside that PR's scope.
internal/middleware/middleware.go:616-622— theMaxBodySizerejection path logsr.URL.Pathuntruncated, atWarnlevel, so it is on by default. Andinternal/server/routes.go:128-131placesMaxBodySizeahead ofRequireAuth.So an unauthenticated
POST /source/<8 KB of client-chosen text>/editcarrying an oversize declaredContent-Lengthwrites an arbitrary-length attacker-chosen path into the operator's log — bypassing the 2,560-byte per-line budget that #146 just established, because that budget lives in the access-log field capping and this is a separate log call.Milestoned
1.0.0for consistency: this is exactly the defect class #146 was moved into the milestone for — an unauthenticated client on the public internet writing unbounded attacker-chosen text into the operator's logs. That fix is incomplete while this path exists, and leaving it would mean the stated per-line bound is true of one log call and false of another, which is worse than not stating it.Note it does NOT reopen #160: there is no query string involved, only the path.
Definition of done
slogcall reaching a client-controlled value — path, header, form field, URL — before or independently of the access-log capping needs the same treatment or an explicit reason. List what you checked.MaxBodySizeshould sit behindRequireAuthinstead. It probably should not — an oversize body ought to be rejected before any session work — so if you leave the ordering, say why in a comment, since the ordering is what makes this reachable unauthenticated.Implementation requirements
next, PR based onnext, single commit, title ending(closes #N).TODO.md(see #112).make bootstrapin a fresh clone before gating — browser assets are fetched at build time, andmake lintnow needs Docker.make checkplus the Docker lint path with the cache defeated. All linting runs in Docker, never on the host.clawbot referenced this issue2026-08-18 04:10:16 +02:00