Follow-up to #135, whose definition of done asks for this to be filed if the untruncated body is not reachable.
The fix for #135 caps the event log page at 8192 bytes of body per event. Nothing is lost from storage: the full body stays in the per-webhook SQLite database, and the database archive target keeps its own full copy (internal/delivery/target_database.go). But no HTTP route serves it — /api/v1 is still an empty route group (internal/server/routes.go) — so a body over the cap is now reachable only out of band, by an operator with filesystem access to the data directory.
Before the cap the event log page itself showed the whole body, so this is a gap the cap introduces for the app's own users.
Definition of done
An authenticated owner of the webhook can fetch one event's stored body in full, e.g. GET /source/{sourceID}/logs/{eventID}/body, served with a non-renderable content type (application/octet-stream) and Content-Disposition: attachment so an HTML or script payload cannot execute in the admin's origin.
The response streams from the row rather than buffering it, so the 1 MB ingest cap is the only bound needed and executeTemplate's buffering is not involved.
The event log's truncation marker links to it.
Authorization is the same ownership check the log page applies, with a test that another user's event 404s.
Follow-up to https://git.eeqj.de/sneak/webhooker/issues/135, whose definition of done asks for this to be filed if the untruncated body is not reachable.
The fix for https://git.eeqj.de/sneak/webhooker/issues/135 caps the event log page at 8192 bytes of body per event. Nothing is lost from storage: the full body stays in the per-webhook SQLite database, and the database archive target keeps its own full copy (`internal/delivery/target_database.go`). But no HTTP route serves it — `/api/v1` is still an empty route group (`internal/server/routes.go`) — so a body over the cap is now reachable only out of band, by an operator with filesystem access to the data directory.
Before the cap the event log page itself showed the whole body, so this is a gap the cap introduces for the app's own users.
## Definition of done
- An authenticated owner of the webhook can fetch one event's stored body in full, e.g. `GET /source/{sourceID}/logs/{eventID}/body`, served with a non-renderable content type (`application/octet-stream`) and `Content-Disposition: attachment` so an HTML or script payload cannot execute in the admin's origin.
- The response streams from the row rather than buffering it, so the 1 MB ingest cap is the only bound needed and `executeTemplate`'s buffering is not involved.
- The event log's truncation marker links to it.
- Authorization is the same ownership check the log page applies, with a test that another user's event 404s.
Reason: #135's definition of done did permit filing this rather than fixing it inline, and #158 was right to take that option — but permitted is not the same as acceptable to ship. webhooker exists to durably store webhooks so they can be inspected and replayed. After the cap lands, a body over 8 KB cannot be seen through the product at all, only by an operator with filesystem access to the SQLite files. Debugging a large payload is precisely when the tool is supposed to earn its keep, so tagging 1.0 in that state means shipping a regression we introduced ourselves.
It is small and the definition of done above is already complete, so it costs little to close the gap in the same milestone rather than after it.
Two things the implementer must not drop, both already in the DoD and both load-bearing rather than stylistic: the response is attacker-controlled bytes being handed to the admin's own origin, so application/octet-stream plus Content-Disposition: attachment is what stops a stored HTML or script payload executing as the logged-in operator — this route would otherwise be a stored-XSS delivery mechanism. And it must stream from the row rather than buffer, or it reintroduces the resident-memory cost #135 just removed, on a route that serves the largest bodies in the system.
Sequencing: this touches internal/handlers/source_management.go, internal/server/routes.go and templates/source_logs.html, all of which #158 is changing. It gets dispatched once that squash-merges into next, not before.
Added to the `1.0.0` milestone.
Reason: https://git.eeqj.de/sneak/webhooker/issues/135's definition of done did permit filing this rather than fixing it inline, and https://git.eeqj.de/sneak/webhooker/pulls/158 was right to take that option — but permitted is not the same as acceptable to ship. webhooker exists to durably store webhooks so they can be inspected and replayed. After the cap lands, a body over 8 KB cannot be seen through the product at all, only by an operator with filesystem access to the SQLite files. Debugging a large payload is precisely when the tool is supposed to earn its keep, so tagging 1.0 in that state means shipping a regression we introduced ourselves.
It is small and the definition of done above is already complete, so it costs little to close the gap in the same milestone rather than after it.
Two things the implementer must not drop, both already in the DoD and both load-bearing rather than stylistic: the response is attacker-controlled bytes being handed to the admin's own origin, so `application/octet-stream` plus `Content-Disposition: attachment` is what stops a stored HTML or script payload executing as the logged-in operator — this route would otherwise be a stored-XSS delivery mechanism. And it must stream from the row rather than buffer, or it reintroduces the resident-memory cost https://git.eeqj.de/sneak/webhooker/issues/135 just removed, on a route that serves the largest bodies in the system.
Sequencing: this touches `internal/handlers/source_management.go`, `internal/server/routes.go` and `templates/source_logs.html`, all of which https://git.eeqj.de/sneak/webhooker/pulls/158 is changing. It gets dispatched once that squash-merges into `next`, not before.
Route GET /source/{sourceID}/logs/{eventID}/body, registered inside the existing /source/{sourceID} group so it inherits RequireAuth, NoCache and the rest.
Authorization: the id = ? AND user_id = ? lookup is currently copy-pasted per handler. I extract it once as ownedWebhook and call it from both HandleSourceLogs and the new handler, so the download cannot drift from the page it belongs to. Event lookup is then scoped id = ? AND webhook_id = ?, so an event id from another webhook is a miss, not an IDOR. Both misses are http.NotFound.
Streaming: modernc.org/sqlite reaches us through database/sql, which exposes no incremental BLOB handle, so there is no true row stream to take — scanning the column materialises it whole whatever wrapper is used. Instead I read the row in fixed 64 KiB chunks with substr(cast(body as blob), ?, ?) (the same byte-wise cast the #135 projection relies on) and write each chunk straight to the ResponseWriter, no renderTemplate. Peak resident body bytes is the chunk, not the payload. Deliberately no wrapping read transaction: these per-webhook databases are not in WAL mode, so holding a read lock across a slow client would block the receiver from writing new events. I will state the bound as what it is in the PR body rather than calling it streaming from the row.
Headers: application/octet-stream, Content-Disposition: attachment, Content-Length from a length(cast(body as blob)) query, and X-Content-Type-Options: nosniff set on the response itself — the global SecurityHeaders middleware does set it, but the route should not depend on a distant middleware for its own security control. The filename is built from the event id only after uuid.Parse accepts it, so nothing client-supplied reaches the header. The existing CSP does not help here (script-src 'self' 'unsafe-inline' would permit an inline script in a document served from our own origin); the disposition is the control.
Template: the #135 truncation marker gains a download link, rendered only under {{if .BodyTruncated}}.
Tests: owner fetches an over-cap body byte-identical; another user's event 404s; an event id belonging to a different webhook 404s; the disposition and content type are asserted; a <script> body is served as an attachment and unaltered.
Plan.
Route `GET /source/{sourceID}/logs/{eventID}/body`, registered inside the existing `/source/{sourceID}` group so it inherits `RequireAuth`, `NoCache` and the rest.
Authorization: the `id = ? AND user_id = ?` lookup is currently copy-pasted per handler. I extract it once as `ownedWebhook` and call it from both `HandleSourceLogs` and the new handler, so the download cannot drift from the page it belongs to. Event lookup is then scoped `id = ? AND webhook_id = ?`, so an event id from another webhook is a miss, not an IDOR. Both misses are `http.NotFound`.
Streaming: `modernc.org/sqlite` reaches us through `database/sql`, which exposes no incremental BLOB handle, so there is no true row stream to take — scanning the column materialises it whole whatever wrapper is used. Instead I read the row in fixed 64 KiB chunks with `substr(cast(body as blob), ?, ?)` (the same byte-wise cast the `#135` projection relies on) and write each chunk straight to the `ResponseWriter`, no `renderTemplate`. Peak resident body bytes is the chunk, not the payload. Deliberately no wrapping read transaction: these per-webhook databases are not in WAL mode, so holding a read lock across a slow client would block the receiver from writing new events. I will state the bound as what it is in the PR body rather than calling it streaming from the row.
Headers: `application/octet-stream`, `Content-Disposition: attachment`, `Content-Length` from a `length(cast(body as blob))` query, and `X-Content-Type-Options: nosniff` set on the response itself — the global `SecurityHeaders` middleware does set it, but the route should not depend on a distant middleware for its own security control. The filename is built from the event id only after `uuid.Parse` accepts it, so nothing client-supplied reaches the header. The existing CSP does not help here (`script-src 'self' 'unsafe-inline'` would permit an inline script in a document served from our own origin); the disposition is the control.
Template: the `#135` truncation marker gains a download link, rendered only under `{{if .BodyTruncated}}`.
Tests: owner fetches an over-cap body byte-identical; another user's event 404s; an event id belonging to a different webhook 404s; the disposition and content type are asserted; a `<script>` body is served as an attachment and unaltered.
Built as planned above; #167 carries the detail and the gate evidence. One correction to the plan, since it changes what a reader should believe about the route:
The webhook_id = ? predicate I described as closing the IDOR is not what closes it. Events live in a per-webhook SQLite file, so a sibling webhook's event is not in the database being queried at all. Mutating both queries to drop that predicate leaves the cross-webhook test green. The predicate stays as a second guard and the code says which mechanism is load-bearing. The ownership mutation does fail as expected, serving the other user's payload with a 200.
Verified with make check (exit 0) and docker build --no-cache-filter=lint --no-cache-filter=builder . (exit 0, 0 issues. from make lint, no cached test packages), both re-run after rebasing onto current next.
Built as planned above; https://git.eeqj.de/sneak/webhooker/pulls/167 carries the detail and the gate evidence. One correction to the plan, since it changes what a reader should believe about the route:
The `webhook_id = ?` predicate I described as closing the IDOR is not what closes it. Events live in a per-webhook SQLite file, so a sibling webhook's event is not in the database being queried at all. Mutating both queries to drop that predicate leaves the cross-webhook test green. The predicate stays as a second guard and the code says which mechanism is load-bearing. The ownership mutation does fail as expected, serving the other user's payload with a 200.
Verified with `make check` (exit 0) and `docker build --no-cache-filter=lint --no-cache-filter=builder .` (exit 0, `0 issues.` from `make lint`, no cached test packages), both re-run after rebasing onto current `next`.
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.
Follow-up to #135, whose definition of done asks for this to be filed if the untruncated body is not reachable.
The fix for #135 caps the event log page at 8192 bytes of body per event. Nothing is lost from storage: the full body stays in the per-webhook SQLite database, and the database archive target keeps its own full copy (
internal/delivery/target_database.go). But no HTTP route serves it —/api/v1is still an empty route group (internal/server/routes.go) — so a body over the cap is now reachable only out of band, by an operator with filesystem access to the data directory.Before the cap the event log page itself showed the whole body, so this is a gap the cap introduces for the app's own users.
Definition of done
GET /source/{sourceID}/logs/{eventID}/body, served with a non-renderable content type (application/octet-stream) andContent-Disposition: attachmentso an HTML or script payload cannot execute in the admin's origin.executeTemplate's buffering is not involved.Added to the
1.0.0milestone.Reason: #135's definition of done did permit filing this rather than fixing it inline, and #158 was right to take that option — but permitted is not the same as acceptable to ship. webhooker exists to durably store webhooks so they can be inspected and replayed. After the cap lands, a body over 8 KB cannot be seen through the product at all, only by an operator with filesystem access to the SQLite files. Debugging a large payload is precisely when the tool is supposed to earn its keep, so tagging 1.0 in that state means shipping a regression we introduced ourselves.
It is small and the definition of done above is already complete, so it costs little to close the gap in the same milestone rather than after it.
Two things the implementer must not drop, both already in the DoD and both load-bearing rather than stylistic: the response is attacker-controlled bytes being handed to the admin's own origin, so
application/octet-streamplusContent-Disposition: attachmentis what stops a stored HTML or script payload executing as the logged-in operator — this route would otherwise be a stored-XSS delivery mechanism. And it must stream from the row rather than buffer, or it reintroduces the resident-memory cost #135 just removed, on a route that serves the largest bodies in the system.Sequencing: this touches
internal/handlers/source_management.go,internal/server/routes.goandtemplates/source_logs.html, all of which #158 is changing. It gets dispatched once that squash-merges intonext, not before.Plan.
Route
GET /source/{sourceID}/logs/{eventID}/body, registered inside the existing/source/{sourceID}group so it inheritsRequireAuth,NoCacheand the rest.Authorization: the
id = ? AND user_id = ?lookup is currently copy-pasted per handler. I extract it once asownedWebhookand call it from bothHandleSourceLogsand the new handler, so the download cannot drift from the page it belongs to. Event lookup is then scopedid = ? AND webhook_id = ?, so an event id from another webhook is a miss, not an IDOR. Both misses arehttp.NotFound.Streaming:
modernc.org/sqlitereaches us throughdatabase/sql, which exposes no incremental BLOB handle, so there is no true row stream to take — scanning the column materialises it whole whatever wrapper is used. Instead I read the row in fixed 64 KiB chunks withsubstr(cast(body as blob), ?, ?)(the same byte-wise cast the#135projection relies on) and write each chunk straight to theResponseWriter, norenderTemplate. Peak resident body bytes is the chunk, not the payload. Deliberately no wrapping read transaction: these per-webhook databases are not in WAL mode, so holding a read lock across a slow client would block the receiver from writing new events. I will state the bound as what it is in the PR body rather than calling it streaming from the row.Headers:
application/octet-stream,Content-Disposition: attachment,Content-Lengthfrom alength(cast(body as blob))query, andX-Content-Type-Options: nosniffset on the response itself — the globalSecurityHeadersmiddleware does set it, but the route should not depend on a distant middleware for its own security control. The filename is built from the event id only afteruuid.Parseaccepts it, so nothing client-supplied reaches the header. The existing CSP does not help here (script-src 'self' 'unsafe-inline'would permit an inline script in a document served from our own origin); the disposition is the control.Template: the
#135truncation marker gains a download link, rendered only under{{if .BodyTruncated}}.Tests: owner fetches an over-cap body byte-identical; another user's event 404s; an event id belonging to a different webhook 404s; the disposition and content type are asserted; a
<script>body is served as an attachment and unaltered.Built as planned above; #167 carries the detail and the gate evidence. One correction to the plan, since it changes what a reader should believe about the route:
The
webhook_id = ?predicate I described as closing the IDOR is not what closes it. Events live in a per-webhook SQLite file, so a sibling webhook's event is not in the database being queried at all. Mutating both queries to drop that predicate leaves the cross-webhook test green. The predicate stays as a second guard and the code says which mechanism is load-bearing. The ownership mutation does fail as expected, serving the other user's payload with a 200.Verified with
make check(exit 0) anddocker build --no-cache-filter=lint --no-cache-filter=builder .(exit 0,0 issues.frommake lint, no cached test packages), both re-run after rebasing onto currentnext.clawbot referenced this issue2026-08-17 23:50:11 +02:00