Serve an event's full stored body over HTTP (closes #157)
Some checks failed
check / check (push) Has been cancelled

Capping the event log page at 8 KB of body per event left no
in-app way to see a larger one: storage keeps it, but no route
served it, so a payload over the cap was reachable only by an
operator with filesystem access. GitHub pull_request and
multi-commit push payloads, expanded Stripe events and Shopify
orders all routinely clear 8 KB, which is exactly when the tool
is supposed to be useful.

GET /source/{sourceID}/logs/{eventID}/body now serves one whole
body, and the truncation marker links to it when — and only
when — a body was actually cut.

The response is deliberately inert. Its bytes are chosen by
whoever can reach the public receiver and it hands them back
inside the operator's own authenticated origin, so it goes out
as application/octet-stream with Content-Disposition: attachment
and nosniff, and the filename is built from a parsed uuid rather
than from anything in the request. The application CSP is no
help on this path: script-src allows 'unsafe-inline' from
'self', so a document served from this origin could run its own
script.

The body is read in one query and held whole while it is
written. There is no cheaper bound to take. database/sql
exposes no incremental handle on a SQLite blob, and reading
byte ranges with substr does not avoid the cost either: SQLite
materialises the entire column value to evaluate each substr
call, so range reads pay for the whole body once per range
rather than once per download. Measured over a 1 MiB body,
64 KiB ranges cost 11-15x a single read to move the same bytes.
The bound is therefore the one the issue asks for: the route is
owner-authenticated and ingest is capped at 1 MB, so peak is
one body per concurrent download. Nothing goes through
renderTemplate, which buffers a whole response before writing
it.

Reading the body before the first header is written also means
an event reaped mid-request cannot produce a torn response: it
is either served whole or 404s cleanly, and both are tested.

The ownership check the log page applies is extracted as
ownedWebhook and shared with the download, so the two cannot
drift apart. A webhook owned by someone else and one that does
not exist are the same 404.

The route registration and the link the template emits are
covered end to end through the production router, so a typo in
either fails the suite rather than leaving the feature dead
behind green handler tests.
This commit is contained in:
clawbot
2026-08-17 21:13:18 +00:00
parent c378690977
commit 1ec8856bce
7 changed files with 904 additions and 28 deletions

View File

@@ -0,0 +1,192 @@
package handlers
import (
"database/sql"
"errors"
"net/http"
"strconv"
"github.com/go-chi/chi"
"github.com/google/uuid"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
)
// eventBodyQuery reads one event's stored body as bytes. The cast
// to blob is what makes the driver hand back the stored bytes
// rather than a string conversion, so Content-Length taken from
// the result matches what goes on the wire. The soft-delete
// predicate is spelled out because Raw bypasses GORM's default
// scope, and it is what stops a reaped event still being
// downloadable.
const eventBodyQuery = "SELECT cast(body as blob) " +
"FROM events WHERE id = ? AND webhook_id = ? AND deleted_at IS NULL"
// HandleEventBodyDownload serves one event's stored body in
// full, which the event log page cannot: it caps each rendered
// body at maxRenderedBodyBytes.
//
// The bytes are attacker-supplied — anyone who can reach the
// public receiver chooses them — and this route hands them back
// inside the operator's own authenticated origin, so the
// response is deliberately not renderable. Content-Disposition
// makes the browser download rather than display it, and the
// octet-stream type plus nosniff stop it being interpreted as
// HTML or script. Without those a stored payload would execute
// as the logged-in operator. The application's CSP does not
// help here: script-src allows 'unsafe-inline' from 'self', so
// a document served from this origin could run its own inline
// script.
func (h *Handlers) HandleEventBodyDownload() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
webhook, ok := h.ownedWebhook(w, r)
if !ok {
return
}
// Parsing the id before use serves two purposes: a
// malformed id can never reach the SQL or the response
// header, and the canonical form below is drawn from
// uuid's own fixed alphabet rather than from the
// request, so the Content-Disposition value cannot be
// steered by a client.
eventID, err := uuid.Parse(chi.URLParam(r, "eventID"))
if err != nil {
http.NotFound(w, r)
return
}
h.serveEventBody(w, r, webhook, eventID.String())
}
}
// serveEventBody writes the named event's stored body to w.
//
// The event must belong to webhook, which is what keeps this
// route from reading any event in the system by id alone. Two
// things enforce that and they are not equally strong. The
// operative one is that events live in a per-webhook SQLite
// file, so a sibling webhook's event is not in the database
// being queried at all. The webhook_id predicate on the query
// below is the second guard, and it is currently redundant
// against that isolation; it is there so the scoping survives
// any future change that puts more than one webhook's events in
// one file.
//
// The body is read in one query and held whole in memory while
// it is written. That is the bound: one body per concurrent
// download, and a body is capped at 1 MB when it is ingested,
// so a download cannot cost more than that. There is no
// cheaper bound available — database/sql exposes no incremental
// handle on a SQLite BLOB, and reading byte ranges with substr
// does not avoid the cost either, because SQLite materialises
// the whole column value to evaluate each substr call. Range
// reads only pay for that materialisation once per range.
//
// One consequence is worth keeping in view: the read finishes
// before the client is written to, so no read lock is held for
// the length of a slow download. These per-webhook databases
// run in SQLite's default journal mode rather than WAL, so a
// lock held that long would block the receiver from recording
// new events.
func (h *Handlers) serveEventBody(
w http.ResponseWriter,
r *http.Request,
webhook database.Webhook,
eventID string,
) {
if !h.dbMgr.DBExists(webhook.ID) {
http.NotFound(w, r)
return
}
webhookDB, err := h.dbMgr.GetDB(webhook.ID)
if err != nil {
h.serverError(w, "failed to get webhook database", err)
return
}
body, found, err := eventBody(webhookDB, webhook.ID, eventID)
if err != nil {
h.serverError(w, "failed to read event body", err)
return
}
// A miss is a 404 whether the event belongs to another
// webhook or does not exist at all, so the response does
// not report which. Reading the body before any header is
// written is also what keeps an event reaped mid-request
// from producing a torn response: either the read finds the
// row and the whole body is served, or it does not and the
// response is a clean 404.
if !found {
http.NotFound(w, r)
return
}
setEventBodyHeaders(w, eventID, int64(len(body)))
_, err = w.Write(body)
if err != nil {
// The status and Content-Length are already committed,
// so the client sees a short download. There is no way
// to report a 500 from here; the log is the record.
h.log.Error(
"failed to write event body",
"webhook_id", webhook.ID,
"event_id", eventID,
"error", err,
)
}
}
// eventBody returns an event's stored body and whether the event
// exists within the webhook.
func eventBody(
webhookDB *gorm.DB,
webhookID, eventID string,
) ([]byte, bool, error) {
var body []byte
err := webhookDB.Raw(
eventBodyQuery, eventID, webhookID,
).Row().Scan(&body)
if errors.Is(err, sql.ErrNoRows) {
return nil, false, nil
}
if err != nil {
return nil, false, err
}
return body, true, nil
}
// setEventBodyHeaders applies the response headers that make
// this route safe to hand attacker-supplied bytes through. See
// HandleEventBodyDownload for why they are a security control
// and not a formatting choice.
//
// nosniff is also set by the global SecurityHeaders middleware.
// It is repeated here so the guarantee belongs to the route
// that needs it rather than to a middleware someone could
// reorder or scope away.
func setEventBodyHeaders(
w http.ResponseWriter,
eventID string,
size int64,
) {
w.Header().Set("Content-Type", "application/octet-stream")
w.Header().Set("X-Content-Type-Options", "nosniff")
w.Header().Set(
"Content-Disposition",
`attachment; filename="webhooker-event-`+eventID+`.bin"`,
)
w.Header().Set("Content-Length", strconv.FormatInt(size, 10))
}