Serve an event's full stored body over HTTP (closes #157)

The 8 KB render cap from #135 left storage untouched but no route served
the rest, so a body over the cap was reachable only with filesystem
access to the SQLite files — in a product whose purpose is storing
webhooks so they can be inspected.

GET /source/{sourceID}/logs/{eventID}/body serves the whole body to the
webhook's owner, as application/octet-stream with an attachment
disposition and nosniff. Those are a security control, not formatting:
the bytes come from the public receiver and are handed back inside the
operator's authenticated origin, and the existing CSP would not stop a
stored HTML payload executing there. The truncation marker links to it
only when a body was actually cut.

Accepted deviation, documented rather than glossed: #157's definition of
done asks the route to stream from the row. It buffers whole instead,
because database/sql exposes no incremental handle on a SQLite BLOB and
substr range reads re-materialise the entire column per call — an
earlier revision chunked at 64 KiB and was 11-15x slower for a worse
bound. Three independent reviewers confirmed no streaming path exists.

Independently reviewed three times. Two earlier revisions each asserted
a memory bound the code did not have; the final reviewer measured
2.057x at the ingest cap and pinned the two overlapping allocations from
source — the driver's column buffer and database/sql's convertAssign
clone — confirming the stated "roughly two bodies, and 2x is a floor
not a ceiling" is now accurate, since SQLite's own materialisation sits
outside the Go heap.

internal/handlers/event_body_test.go

@@ -0,0 +1,506 @@
+package handlers_test
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/go-chi/chi"
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gorm.io/gorm/clause"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/handlers"
+	"sneak.berlin/go/webhooker/internal/session"
+)
+
+// paramEventID is the chi URL parameter the body download
+// handler reads.
+const paramEventID = "eventID"
+
+// otherTestUserID owns webhooks the session user must not be
+// able to read.
+const otherTestUserID = "other-user-id"
+
+// seedWebhookFor inserts a webhook owned by the given user.
+func seedWebhookFor(
+	t *testing.T,
+	db *database.Database,
+	userID string,
+) *database.Webhook {
+	t.Helper()
+
+	wh := &database.Webhook{
+		UserID: userID,
+		Name:   "wh-" + userID,
+	}
+
+	require.NoError(
+		t,
+		db.DB().Omit(clause.Associations).Create(wh).Error,
+	)
+
+	return wh
+}
+
+// fetchEventBody runs the real download handler as the test user
+// for the given source and event ids.
+func fetchEventBody(
+	t *testing.T,
+	h *handlers.Handlers,
+	sess *session.Session,
+	sourceID, eventID string,
+) *httptest.ResponseRecorder {
+	t.Helper()
+
+	// The path is escaped and the raw id goes in the route
+	// context, which is what chi hands a handler: the param is
+	// already percent-decoded by the time it is read.
+	req := httptest.NewRequestWithContext(
+		context.Background(),
+		http.MethodGet,
+		"/source/"+url.PathEscape(sourceID)+
+			"/logs/"+url.PathEscape(eventID)+"/body",
+		nil,
+	)
+
+	for _, c := range authenticatedCookies(
+		t, sess, deleteTestUserID, deleteTestUsername,
+	) {
+		req.AddCookie(c)
+	}
+
+	rctx := chi.NewRouteContext()
+	rctx.URLParams.Add(paramSourceID, sourceID)
+	rctx.URLParams.Add(paramEventID, eventID)
+
+	req = req.WithContext(
+		context.WithValue(
+			req.Context(), chi.RouteCtxKey, rctx,
+		),
+	)
+
+	w := httptest.NewRecorder()
+	h.HandleEventBodyDownload().ServeHTTP(w, req)
+
+	return w
+}
+
+// TestHandleEventBodyDownload_ServesOversizeBodyInFull is the
+// capability the render cap took away: a body far above what the
+// event log page will show comes back whole and byte-identical,
+// with the headers that keep it from being rendered.
+func TestHandleEventBodyDownload_ServesOversizeBodyInFull(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var (
+		h     *handlers.Handlers
+		sess  *session.Session
+		db    *database.Database
+		dbMgr *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &dbMgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	// Far above the render cap, with multibyte runes and a
+	// distinctive tail, so a body that the log page can only
+	// show a slice of comes back whole and in order.
+	const sentinel = "TAIL-SENTINEL-1f4a9c"
+
+	stored := strings.Repeat("A", 200*1024) +
+		strings.Repeat(snowman, 1000) + sentinel
+
+	wh := seedWebhook(t, db)
+	evt := seedEventWithBody(t, dbMgr, wh.ID, stored)
+
+	w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
+
+	require.Equal(t, http.StatusOK, w.Code)
+	assert.Greater(t, len(stored), bodyCap)
+	assert.Equal(t, stored, w.Body.String())
+	assert.Equal(
+		t, strconv.Itoa(len(stored)),
+		w.Header().Get("Content-Length"),
+	)
+}
+
+// TestHandleEventBodyDownload_BodiesRoundTripByteIdentical
+// covers the sizes and byte values a stored body can actually
+// take: empty, one byte, either side of the render cap, and
+// bytes that are not text at all. Content-Length has to equal
+// the bytes written in every case, since it is derived from the
+// same read that produces them.
+func TestHandleEventBodyDownload_BodiesRoundTripByteIdentical(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	// A NUL, invalid UTF-8 and a multibyte rune, so nothing on
+	// the path can be treating the body as text.
+	binary := "\x00\x01\xff\xfe" + snowman + "\x00tail"
+
+	cases := map[string]string{
+		"empty":         "",
+		"single byte":   "x",
+		"one below cap": strings.Repeat("b", bodyCap-1),
+		"exactly cap":   strings.Repeat("c", bodyCap),
+		"one above cap": strings.Repeat("d", bodyCap+1),
+		"binary":        binary,
+	}
+
+	for name, stored := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			var (
+				h     *handlers.Handlers
+				sess  *session.Session
+				db    *database.Database
+				dbMgr *database.WebhookDBManager
+			)
+
+			app := newTestApp(t, &h, &sess, &db, &dbMgr)
+			app.RequireStart()
+
+			t.Cleanup(app.RequireStop)
+
+			wh := seedWebhook(t, db)
+			evt := seedEventWithBody(t, dbMgr, wh.ID, stored)
+
+			w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
+
+			require.Equal(t, http.StatusOK, w.Code)
+			assert.Equal(t, stored, w.Body.String())
+			assert.Equal(
+				t, strconv.Itoa(len(stored)),
+				w.Header().Get("Content-Length"),
+			)
+			assert.Equal(
+				t, len(stored), w.Body.Len(),
+				"Content-Length must equal bytes written",
+			)
+		})
+	}
+}
+
+// TestHandleEventBodyDownload_HeadersAreNotRenderable pins the
+// response headers that stop attacker-supplied bytes executing
+// in the operator's own origin. They are a security control, not
+// presentation.
+func TestHandleEventBodyDownload_HeadersAreNotRenderable(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var (
+		h     *handlers.Handlers
+		sess  *session.Session
+		db    *database.Database
+		dbMgr *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &dbMgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	wh := seedWebhook(t, db)
+	evt := seedEventWithBody(t, dbMgr, wh.ID, `{"small":true}`)
+
+	w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
+
+	require.Equal(t, http.StatusOK, w.Code)
+	assert.Equal(
+		t, "application/octet-stream",
+		w.Header().Get("Content-Type"),
+	)
+	assert.Equal(
+		t, "nosniff",
+		w.Header().Get("X-Content-Type-Options"),
+	)
+
+	disposition := w.Header().Get("Content-Disposition")
+	assert.Equal(
+		t,
+		`attachment; filename="webhooker-event-`+evt.ID+`.bin"`,
+		disposition,
+	)
+}
+
+// TestHandleEventBodyDownload_ScriptBodyStaysInert proves a
+// stored HTML payload is handed back as an attachment of opaque
+// bytes rather than as anything a browser will execute. The
+// bytes themselves are unaltered: this route reports what was
+// delivered.
+func TestHandleEventBodyDownload_ScriptBodyStaysInert(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var (
+		h     *handlers.Handlers
+		sess  *session.Session
+		db    *database.Database
+		dbMgr *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &dbMgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	const payload = `<html><script>alert(document.cookie)` +
+		`</script></html>`
+
+	wh := seedWebhook(t, db)
+	evt := seedEventWithBody(t, dbMgr, wh.ID, payload)
+
+	w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
+
+	require.Equal(t, http.StatusOK, w.Code)
+	assert.Equal(t, payload, w.Body.String())
+
+	contentType := w.Header().Get("Content-Type")
+	assert.Equal(t, "application/octet-stream", contentType)
+	assert.NotContains(t, contentType, "html")
+	assert.NotContains(t, contentType, "xml")
+	assert.NotContains(t, contentType, "javascript")
+	assert.Contains(
+		t, w.Header().Get("Content-Disposition"), "attachment",
+	)
+	assert.Equal(
+		t, "nosniff",
+		w.Header().Get("X-Content-Type-Options"),
+	)
+}
+
+// TestHandleEventBodyDownload_OtherUsersEvent404s is the
+// authorization test the definition of done asks for: an event
+// stored under a webhook the session user does not own is not
+// readable, and the miss does not distinguish itself from a
+// nonexistent one.
+func TestHandleEventBodyDownload_OtherUsersEvent404s(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var (
+		h     *handlers.Handlers
+		sess  *session.Session
+		db    *database.Database
+		dbMgr *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &dbMgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	const theirPayload = "OTHER-USERS-PAYLOAD-8b1d"
+
+	theirs := seedWebhookFor(t, db, otherTestUserID)
+	evt := seedEventWithBody(t, dbMgr, theirs.ID, theirPayload)
+
+	w := fetchEventBody(t, h, sess, theirs.ID, evt.ID)
+
+	assert.Equal(t, http.StatusNotFound, w.Code)
+	assert.NotContains(t, w.Body.String(), theirPayload)
+}
+
+// TestHandleEventBodyDownload_EventOfAnotherWebhook404s pins
+// that holding a valid event id is not enough: the event has to
+// belong to the webhook in the path. Both webhooks here are the
+// session user's and both have event databases, so the
+// ownership check cannot be what produces the 404.
+//
+// What does produce it is the per-webhook database file rather
+// than the webhook_id predicate on the query — removing that
+// predicate leaves this test green, because the sibling's event
+// is in a different file. The test is kept as the behavioural
+// guard the route owes; see serveEventBody for which mechanism
+// is load-bearing.
+func TestHandleEventBodyDownload_EventOfAnotherWebhook404s(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var (
+		h     *handlers.Handlers
+		sess  *session.Session
+		db    *database.Database
+		dbMgr *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &dbMgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	const other = "BELONGS-TO-THE-OTHER-WEBHOOK-3c7e"
+
+	mine := seedWebhook(t, db)
+	seedEventWithBody(t, dbMgr, mine.ID, `{"mine":true}`)
+
+	sibling := seedWebhook(t, db)
+	evt := seedEventWithBody(t, dbMgr, sibling.ID, other)
+
+	w := fetchEventBody(t, h, sess, mine.ID, evt.ID)
+
+	assert.Equal(t, http.StatusNotFound, w.Code)
+	assert.NotContains(t, w.Body.String(), other)
+}
+
+// TestHandleEventBodyDownload_UnknownEvent404s covers the plain
+// miss, including an id that is not a uuid at all and so never
+// reaches the query or the response header.
+func TestHandleEventBodyDownload_UnknownEvent404s(t *testing.T) {
+	t.Parallel()
+
+	var (
+		h     *handlers.Handlers
+		sess  *session.Session
+		db    *database.Database
+		dbMgr *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &dbMgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	wh := seedWebhook(t, db)
+	seedEventWithBody(t, dbMgr, wh.ID, `{"mine":true}`)
+
+	for _, id := range []string{
+		uuid.New().String(),
+		`../../etc/passwd`,
+		"not-a-uuid",
+		`x"; rm -rf /`,
+	} {
+		w := fetchEventBody(t, h, sess, wh.ID, id)
+
+		assert.Equal(
+			t, http.StatusNotFound, w.Code,
+			"event id %q", id,
+		)
+		assert.Empty(
+			t, w.Header().Get("Content-Disposition"),
+			"event id %q must not reach a header", id,
+		)
+	}
+}
+
+// TestHandleEventBodyDownload_ReapedEvent404s pins what happens
+// when the retention reaper takes an event out from under this
+// route. The body is read in one query before any header is
+// written, so a reaped event cannot produce a partial download:
+// it is a clean 404 with no Content-Length and no
+// Content-Disposition. Both removals the codebase performs are
+// covered — the reaper hard-deletes, and a soft-deleted row is
+// excluded by the query's own deleted_at predicate rather than
+// by GORM's default scope, which Raw bypasses.
+func TestHandleEventBodyDownload_ReapedEvent404s(t *testing.T) {
+	t.Parallel()
+
+	for name, hard := range map[string]bool{
+		"soft deleted": false,
+		"hard deleted": true,
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			var (
+				h     *handlers.Handlers
+				sess  *session.Session
+				db    *database.Database
+				dbMgr *database.WebhookDBManager
+			)
+
+			app := newTestApp(t, &h, &sess, &db, &dbMgr)
+			app.RequireStart()
+
+			t.Cleanup(app.RequireStop)
+
+			const payload = "REAPED-PAYLOAD-4d2a"
+
+			wh := seedWebhook(t, db)
+			evt := seedEventWithBody(t, dbMgr, wh.ID, payload)
+
+			webhookDB, err := dbMgr.GetDB(wh.ID)
+			require.NoError(t, err)
+
+			del := webhookDB
+			if hard {
+				del = del.Unscoped()
+			}
+
+			require.NoError(
+				t,
+				del.Delete(&database.Event{}, "id = ?", evt.ID).
+					Error,
+			)
+
+			w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
+
+			assert.Equal(t, http.StatusNotFound, w.Code)
+			assert.NotContains(t, w.Body.String(), payload)
+			assert.Empty(t, w.Header().Get("Content-Length"))
+			assert.Empty(
+				t, w.Header().Get("Content-Disposition"),
+			)
+		})
+	}
+}
+
+// TestHandleSourceLogs_TruncationMarkerLinksToDownload proves
+// the page tells the reader where the rest of the body is, and
+// only when there is a rest to fetch.
+func TestHandleSourceLogs_TruncationMarkerLinksToDownload(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var (
+		h     *handlers.Handlers
+		sess  *session.Session
+		db    *database.Database
+		dbMgr *database.WebhookDBManager
+	)
+
+	app := newTestApp(t, &h, &sess, &db, &dbMgr)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	big := seedWebhook(t, db)
+	bigEvt := seedEventWithBody(
+		t, dbMgr, big.ID, strings.Repeat("A", 4*bodyCap),
+	)
+
+	page := renderSourceLogsPage(t, h, sess, big.ID)
+	assert.Contains(
+		t, page,
+		"/source/"+big.ID+"/logs/"+bigEvt.ID+"/body",
+	)
+
+	small := seedWebhook(t, db)
+	smallEvt := seedEventWithBody(
+		t, dbMgr, small.ID, `{"kept":"whole"}`,
+	)
+
+	page = renderSourceLogsPage(t, h, sess, small.ID)
+	assert.NotContains(
+		t, page,
+		"/source/"+small.ID+"/logs/"+smallEvt.ID+"/body",
+	)
+}