Render admin page errors in the normal layout (closes #382)

Every 400, 403, 404 and 500 on an admin page now answers with an
error page in the normal layout: one fixed line for the status and a
link back to the webhook list, or to sign-in when nobody is signed
in. The router's handler for unknown paths and the CSRF middleware's
refusal use the same page. Status codes are unchanged. The receiver,
the healthcheck and /metrics keep their plain answers. If the error
page itself cannot render, the answer is the same status in plain
text.

Model: opus-5-5

.dockerignore

@@ -1,15 +1,14 @@
-# .git is deliberately NOT excluded: the build derives the version it stamps
-# into the binary from it (script/version). Nor is any tracked file: git in
-# the build would see it as deleted and mark the version -dirty. Only
-# untracked files belong here.
-#
 # .ci-fingerprint is deliberately NOT excluded: it is the CI cache barrier
 # that keeps the check stages from replaying a cached pass. See the lint
 # stage of the Dockerfile.
+.git/
 bin/
 # Extracted from 3p/ by `make assets` inside the build; a host copy is not
 # needed. The tarball in 3p/ must stay in the context.
 static/js/alpine.min.js
+*.md
+LICENSE
+.editorconfig
 .env
 .env.*
 *.db

.gitea/workflows/check.yml

@@ -28,11 +28,12 @@ jobs:
         run: script/ci-mark-superseded
 
       - name: Fingerprint the build context
-        # Every commit that changes more than docs writes a new fingerprint
-        # into the context, which invalidates the `COPY . .` layer of both
-        # check stages: a commit that was never linted, formatted-checked,
-        # tested and built cannot report success from cache. Docs-only
-        # commits rebuild too, since the context also carries `.git`.
+        # `.dockerignore` keeps docs out of the build context, so a docs-only
+        # commit legitimately replays the whole image from cache and stays
+        # cheap. Every other commit writes a new fingerprint into the context,
+        # which invalidates the `COPY . .` layer of both check stages: a
+        # commit that was never linted, formatted-checked, tested and built
+        # cannot report success from cache.
         run: |
           set -eu
           fp="$(git log -1 --format=%H -- . ':!*.md' ':!LICENSE' ':!.editorconfig')"

Dockerfile

@@ -38,8 +38,8 @@ FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a349228
 COPY --from=lint /src/go.sum /dev/null
 
 # jq is a runtime dependency of script/ci-mark-superseded, which the test
-# suite executes. git is what script/version derives the version with.
-RUN apt-get update && apt-get install -y --no-install-recommends make curl ca-certificates jq git && rm -rf /var/lib/apt/lists/*
+# suite executes.
+RUN apt-get update && apt-get install -y --no-install-recommends make curl ca-certificates jq && rm -rf /var/lib/apt/lists/*
 
 WORKDIR /build
 
@@ -55,22 +55,14 @@ COPY . .
 # from its tarball in 3p/.
 RUN make test
 
-# Version stamped into the binary: the VERSION build arg when one is
-# given, otherwise what script/version derives from the .git the build
-# context carries, so any `docker build .` of a clone stamps its commit.
-# With neither, as from a source tarball, it is "unknown".
+# Version stamped into the binary. .dockerignore excludes .git/, so
+# nothing in this stage can derive it: script/docker resolves it on the
+# host and passes it in. The default is what a bare `docker build .`
+# with no --build-arg gets, and it names no tag the tree may not be at.
 #
 # Declared here, below the test step, so a changed version does not
 # invalidate its cached layer.
-ARG VERSION
-
-# A context that carries .git must not stamp "unknown": that means git is
-# missing here or refused to read the checkout, and the image could not be
-# traced back to its commit.
-RUN if [ -d .git ] && [ "$(make version VERSION="$VERSION")" = unknown ]; then \
-        echo "version is unknown although the build context carries .git" >&2; \
-        exit 1; \
-    fi
+ARG VERSION=unknown
 
 RUN make build VERSION="$VERSION"
 

Makefile

@@ -4,12 +4,12 @@
 .DEFAULT_GOAL := check
 
 # Version stamped into the binary. Derived from git by script/version;
-# override it (`make build VERSION=v1.2.3`) to stamp a given value, which is
-# how the Dockerfile passes its build arg in.
+# override it (`make build VERSION=v1.2.3`) where git metadata is
+# unavailable, which is how the Dockerfile passes its build arg in.
 VERSION ?= $(shell script/version)
 
-# An empty override (`make build VERSION=`, or the Dockerfile's `make build
-# VERSION="$VERSION"` when no VERSION build arg was given) means unset,
+# An empty override (`make build VERSION=`, or a `--build-arg VERSION=`
+# landing on the Dockerfile's `make build VERSION="$VERSION"`) means unset,
 # exactly as it does in script/version -- stamping "" would leave the binary
 # reporting no version and the footer back on its "dev" fallback. `override`
 # is required: a plain assignment loses to the command-line definition it

README.md

@@ -1123,21 +1123,13 @@ build itself.
 | Uncommitted changes | the above with a `-dirty` suffix |
 | No git metadata | `unknown` |
 
-The image derives it the same way, from the `.git` that the build
-context carries, so any `docker build .` of a clone stamps the commit it
-was built from; a shallow clone of one branch has no tags and stamps the
-short SHA. `.dockerignore` must therefore leave out neither `.git` nor
-any tracked file, which git in the build would see as deleted, marking
-the version `-dirty`. A `VERSION` build arg (`--build-arg VERSION=...`)
-takes precedence; `script/docker` (and so `make docker`) passes the one
-`script/version` resolves on the host. The image build fails if its
-context carries `.git` and the version still comes out `unknown`, which
-means git in the build could not read the checkout.
-
-`unknown` is what a source tarball, or a `docker build` with no `.git`
-in its context and no `VERSION` build arg, reports. A build that reports
-`unknown` is a build nobody told what it was; it is not a failure, but
-it cannot be traced back to a commit.
+`unknown` is what a source tarball or a `docker build .` with no
+`--build-arg VERSION=...` reports. `.dockerignore` excludes `.git/`, so
+the build context carries no git metadata and the image cannot derive
+the version itself: `script/docker` (and so `make docker`) resolves it
+on the host and passes it in as the `VERSION` build arg. A build that
+reports `unknown` is a build nobody told what it was; it is not a
+failure, but it cannot be traced back to a commit.
 
 `make version` prints what the current checkout would stamp, and
 `make build VERSION=v1.2.3` overrides it. An empty override — from
@@ -1650,7 +1642,6 @@ data for auditing, for replay, and for resubmission.
 | `headers`      | JSON   | Complete request headers |
 | `body`         | text   | Raw request body |
 | `content_type` | string | Content-Type header value |
-| `body_bytes`   | integer | The body's size in bytes, recorded when the event is stored, on receipt and on resubmit |
 | `resubmitted_from_id` | UUID | The event this one was copied from by a resubmit (nullable; empty for an event that arrived on the receiver). Not a foreign key: the source event can be reaped by retention while its copies remain |
 
 **Relations:** Belongs to Webhook. Belongs to Entrypoint. Has many
@@ -3173,9 +3164,8 @@ version is fixed independently of the compiler's:
    rebuilds the binary with `CGO_ENABLED=1` and static linking so it
    runs on musl. Both builds go through `make build`, the relink adding
    its `-extldflags` via `GO_LDFLAGS`, so neither can drop the `-X` that
-   stamps the version. The version is the `VERSION` build arg if one is
-   given, otherwise derived from the `.git` in the context, and the
-   stage fails if a context with `.git` would stamp `unknown` (see
+   stamps the version. The version arrives as the `VERSION` build arg,
+   since the context has no `.git` (see
    [Version stamping](#version-stamping)).
 3. **Runtime stage** (`alpine:3.21`) — copies the static binary and
    `deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker`
@@ -3207,17 +3197,16 @@ A layer cache lets `docker build .` exit 0 in seconds with the lint and
 test stages replayed rather than executed, which would make a green
 check meaningless. The `check` workflow therefore writes
 `.ci-fingerprint` into the build context before building. Its value is
-the hash of the last commit that touched anything other than `*.md`,
-`LICENSE` and `.editorconfig`, so:
+the hash of the last commit that touched the build context, so:
 
 - Any commit that changes code (including a squash merge whose tree
   matches an already-built branch) gets a new fingerprint, invalidates
   the `COPY . .` layer of both check stages, and really runs
   `make fmt-check`, `golangci-lint`, `make test`, and `make build`. A
   run that reports success ran them.
-- A docs-only commit leaves the fingerprint unchanged, but it still
-  rebuilds in full: the context also carries `.git`, which changes with
-  every commit (see [Version stamping](#version-stamping)).
+- A docs-only commit leaves the fingerprint unchanged — `.dockerignore`
+  excludes `*.md`, `LICENSE` and `.editorconfig` from the context
+  anyway — so the image replays from cache and costs seconds.
 
 The module download layer sits above `COPY . .` and stays cached either
 way.

go.mod

@@ -4,7 +4,6 @@ go 1.26.1
 
 require (
 	github.com/99designs/basicauth-go v0.0.0-20230316000542-bf6f9cbbf0f8
-	github.com/dustin/go-humanize v1.0.1
 	github.com/getsentry/sentry-go v0.25.0
 	github.com/go-chi/chi v1.5.5
 	github.com/go-chi/cors v1.2.1
@@ -30,6 +29,7 @@ require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
+	github.com/dustin/go-humanize v1.0.1 // indirect
 	github.com/gorilla/securecookie v1.1.2 // indirect
 	github.com/jinzhu/inflection v1.0.0 // indirect
 	github.com/jinzhu/now v1.1.5 // indirect

internal/database/model_event.go

@@ -31,11 +31,6 @@ type Event struct {
 	Body        string `gorm:"type:text"   json:"body"`
 	ContentType string `json:"contentType"`
 
-	// BodyBytes is the size of Body in bytes, recorded when the event
-	// is stored so the recent events list can show it without reading
-	// the body.
-	BodyBytes int64 `gorm:"not null" json:"bodyBytes"`
-
 	// ResubmittedFromID names the event this one was copied from by
 	// an operator resubmit. It is nil for an event that arrived on
 	// the receiver, which is every event created before the column

internal/handlers/auth.go

@@ -36,7 +36,7 @@ func (h *Handlers) HandleLoginSubmit() http.HandlerFunc {
 		err := r.ParseForm()
 		if err != nil {
 			h.log.Error("failed to parse form", "error", err)
-			http.Error(w, "Bad request", http.StatusBadRequest)
+			h.renderError(w, r, http.StatusBadRequest)
 
 			return
 		}
@@ -165,11 +165,7 @@ func (h *Handlers) authenticateUser(
 
 	valid, err := database.VerifyPassword(password, user.Password)
 	if err != nil {
-		h.log.Error("failed to verify password", "error", err)
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
+		h.serverError(w, r, "failed to verify password", err)
 
 		return user, err
 	}
@@ -241,24 +237,14 @@ func (h *Handlers) createAuthenticatedSession(
 ) error {
 	oldSess, err := h.session.Get(r)
 	if err != nil {
-		h.log.Error("failed to get session", "error", err)
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
+		h.serverError(w, r, "failed to get session", err)
 
 		return err
 	}
 
 	sess, err := h.session.Regenerate(r, w, oldSess)
 	if err != nil {
-		h.log.Error(
-			"failed to regenerate session", "error", err,
-		)
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
+		h.serverError(w, r, "failed to regenerate session", err)
 
 		return err
 	}
@@ -267,11 +253,7 @@ func (h *Handlers) createAuthenticatedSession(
 
 	err = h.session.Save(r, w, sess)
 	if err != nil {
-		h.log.Error("failed to save session", "error", err)
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
+		h.serverError(w, r, "failed to save session", err)
 
 		return err
 	}

internal/handlers/delivery_replay.go

@@ -105,9 +105,7 @@ func (h *Handlers) HandleDeliveryReplay() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err := r.ParseForm()
 		if err != nil {
-			http.Error(
-				w, "Bad request", http.StatusBadRequest,
-			)
+			h.renderError(w, r, http.StatusBadRequest)
 
 			return
 		}
@@ -124,14 +122,14 @@ func (h *Handlers) replayDelivery(
 	webhook database.Webhook,
 ) {
 	if !h.dbMgr.DBExists(webhook.ID) {
-		http.NotFound(w, r)
+		h.renderError(w, r, http.StatusNotFound)
 
 		return
 	}
 
 	webhookDB, err := h.dbMgr.GetDB(webhook.ID)
 	if err != nil {
-		h.serverError(w, "failed to get webhook database", err)
+		h.serverError(w, r, "failed to get webhook database", err)
 
 		return
 	}
@@ -173,7 +171,7 @@ func (h *Handlers) loadReplaySource(
 			&original, "id = ?", chi.URLParam(r, "deliveryID"),
 		).Error
 	if err != nil {
-		http.NotFound(w, r)
+		h.renderError(w, r, http.StatusNotFound)
 
 		return nil, false
 	}
@@ -195,7 +193,7 @@ func (h *Handlers) queueReplay(
 	)
 	if err != nil {
 		h.serverError(
-			w, "failed to count in-flight deliveries", err,
+			w, r, "failed to count in-flight deliveries", err,
 		)
 
 		return
@@ -212,7 +210,7 @@ func (h *Handlers) queueReplay(
 	err = webhookDB.
 		First(&event, "id = ?", original.EventID).Error
 	if err != nil {
-		h.serverError(w, "failed to load event for replay", err)
+		h.serverError(w, r, "failed to load event for replay", err)
 
 		return
 	}
@@ -222,7 +220,7 @@ func (h *Handlers) queueReplay(
 	)
 	if err != nil {
 		h.serverError(
-			w, "failed to create replay delivery", err,
+			w, r, "failed to create replay delivery", err,
 		)
 
 		return

internal/handlers/error_page_test.go

@@ -0,0 +1,53 @@
+package handlers_test
+
+import (
+	"context"
+	"html/template"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"sneak.berlin/go/webhooker/internal/handlers"
+)
+
+// TestErrorPage_RenderFailureKeepsStatus proves that an error page
+// which cannot render answers with the status it was reporting, as
+// plain text, and is not attempted again: a page whose own render
+// fails reaches the error page, and the error page failing as well
+// ends there with the 500.
+func TestErrorPage_RenderFailureKeepsStatus(t *testing.T) {
+	t.Parallel()
+
+	var h *handlers.Handlers
+
+	app := newTestApp(t, &h)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	// .Status is an int, so asking it for a field fails the render.
+	failing := `{{.Status.Missing}}`
+	h.AddTemplateForTest("error.html", template.Must(
+		template.New("error").Parse(failing),
+	))
+	h.AddTemplateForTest("failing.html", template.Must(
+		template.New("failing").Parse(`{{.Data.Missing}}`),
+	))
+
+	req := httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet, "/", nil,
+	)
+
+	w := httptest.NewRecorder()
+	h.HandleErrorPage(http.StatusNotFound).ServeHTTP(w, req)
+
+	assert.Equal(t, http.StatusNotFound, w.Code)
+	assert.Equal(t, "Not Found\n", w.Body.String())
+
+	w = httptest.NewRecorder()
+	h.RenderTemplateForTest(w, req, "failing.html", 0)
+
+	assert.Equal(t, http.StatusInternalServerError, w.Code)
+	assert.Equal(t, "Internal Server Error\n", w.Body.String())
+}

internal/handlers/event_body.go

@@ -52,7 +52,7 @@ func (h *Handlers) HandleEventBodyDownload() http.HandlerFunc {
 		// steered by a client.
 		eventID, err := uuid.Parse(chi.URLParam(r, "eventID"))
 		if err != nil {
-			http.NotFound(w, r)
+			h.renderError(w, r, http.StatusNotFound)
 
 			return
 		}
@@ -103,21 +103,21 @@ func (h *Handlers) serveEventBody(
 	eventID string,
 ) {
 	if !h.dbMgr.DBExists(webhook.ID) {
-		http.NotFound(w, r)
+		h.renderError(w, r, http.StatusNotFound)
 
 		return
 	}
 
 	webhookDB, err := h.dbMgr.GetDB(webhook.ID)
 	if err != nil {
-		h.serverError(w, "failed to get webhook database", err)
+		h.serverError(w, r, "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)
+		h.serverError(w, r, "failed to read event body", err)
 
 		return
 	}
@@ -130,7 +130,7 @@ func (h *Handlers) serveEventBody(
 	// row and the whole body is served, or it does not and the
 	// response is a clean 404.
 	if !found {
-		http.NotFound(w, r)
+		h.renderError(w, r, http.StatusNotFound)
 
 		return
 	}

internal/handlers/event_resubmit.go

@@ -99,7 +99,7 @@ func (h *Handlers) HandleEventResubmit() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err := r.ParseForm()
 		if err != nil {
-			http.Error(w, "Bad request", http.StatusBadRequest)
+			h.renderError(w, r, http.StatusBadRequest)
 
 			return
 		}
@@ -120,20 +120,20 @@ func (h *Handlers) resubmitEvent(
 	// alphabet rather than from the request.
 	eventID, err := uuid.Parse(chi.URLParam(r, "eventID"))
 	if err != nil {
-		http.NotFound(w, r)
+		h.renderError(w, r, http.StatusNotFound)
 
 		return
 	}
 
 	if !h.dbMgr.DBExists(webhook.ID) {
-		http.NotFound(w, r)
+		h.renderError(w, r, http.StatusNotFound)
 
 		return
 	}
 
 	webhookDB, err := h.dbMgr.GetDB(webhook.ID)
 	if err != nil {
-		h.serverError(w, "failed to get webhook database", err)
+		h.serverError(w, r, "failed to get webhook database", err)
 
 		return
 	}
@@ -147,7 +147,7 @@ func (h *Handlers) resubmitEvent(
 		webhookDB, webhook.ID, eventID.String(),
 	)
 	if err != nil {
-		h.serverError(w, "failed to load event to resubmit", err)
+		h.serverError(w, r, "failed to load event to resubmit", err)
 
 		return
 	}
@@ -155,7 +155,7 @@ func (h *Handlers) resubmitEvent(
 	// A miss is a 404 whether the event was reaped, belongs to
 	// another webhook, or never existed.
 	if !found {
-		http.NotFound(w, r)
+		h.renderError(w, r, http.StatusNotFound)
 
 		return
 	}
@@ -207,7 +207,7 @@ func (h *Handlers) queueResubmit(
 	// inactive one is skipped rather than refused.
 	targets, err := h.loadActiveTargets(webhook.ID)
 	if err != nil {
-		h.serverError(w, "failed to query targets", err)
+		h.serverError(w, r, "failed to query targets", err)
 
 		return
 	}
@@ -225,7 +225,7 @@ func (h *Handlers) queueResubmit(
 		targets,
 	)
 	if err != nil {
-		h.serverError(w, "failed to store resubmitted event", err)
+		h.serverError(w, r, "failed to store resubmitted event", err)
 
 		return
 	}

internal/handlers/event_resubmit_test.go

@@ -204,7 +204,6 @@ func assertEventCopy(
 	assert.Equal(t, original.Method, fresh.Method)
 	assert.Equal(t, original.Headers, fresh.Headers)
 	assert.Equal(t, original.Body, fresh.Body)
-	assert.Equal(t, int64(len(original.Body)), fresh.BodyBytes)
 	assert.Equal(t, original.ContentType, fresh.ContentType)
 	assert.Equal(t, original.EntrypointID, fresh.EntrypointID)
 	assert.Equal(t, original.WebhookID, fresh.WebhookID)

internal/handlers/export_test.go

@@ -1,9 +1,11 @@
 package handlers
 
 import (
+	"context"
 	"html/template"
 	"log/slog"
 	"net/http"
+	"net/http/httptest"
 
 	"sneak.berlin/go/webhooker/internal/database"
 )
@@ -63,12 +65,20 @@ func (s *Handlers) LoadEventLogViewsForTest(
 	page int,
 ) []EventLogView {
 	views, _, _ := s.loadEventsWithDeliveries(
-		w, webhook, nil, page,
+		w, newRequestForTest(), webhook, nil, page,
 	)
 
 	return views
 }
 
+// newRequestForTest is the request the helpers here pass on for
+// callers that have none: it is used only to render the error page.
+func newRequestForTest() *http.Request {
+	return httptest.NewRequestWithContext(
+		context.Background(), http.MethodGet, "/", nil,
+	)
+}
+
 // AddTemplateForTest registers a template under a page name so that
 // the handlers_test package can drive the render path with a
 // template of its own.
@@ -122,5 +132,5 @@ func (s *Handlers) BuildDatabaseTargetConfigForTest(
 	w http.ResponseWriter,
 	expiry string,
 ) (string, error) {
-	return s.buildDatabaseTargetConfig(w, expiry)
+	return s.buildDatabaseTargetConfig(w, newRequestForTest(), expiry)
 }

internal/handlers/handlers.go

@@ -28,7 +28,7 @@ const (
 	// maxBodyShift is the bit shift for 1 MB body limit.
 	maxBodyShift = 20
 	// recentEventLimit is the number of recent events to show.
-	recentEventLimit = 50
+	recentEventLimit = 20
 	// paginationPerPage is the number of items per page.
 	paginationPerPage = 25
 
@@ -135,6 +135,7 @@ func New(
 		"source_edit.html":   parsePageTemplate("source_edit.html"),
 		"source_logs.html":   parsePageTemplate("source_logs.html"),
 		"target_edit.html":   parsePageTemplate("target_edit.html"),
+		"error.html":         parsePageTemplate("error.html"),
 	}
 
 	lc.Append(fx.Hook{
@@ -146,6 +147,15 @@ func New(
 	return s, nil
 }
 
+// HandleErrorPage returns a handler that answers every request with
+// the error page for status. The router uses it for unknown paths and
+// the CSRF middleware for a refused form.
+func (s *Handlers) HandleErrorPage(status int) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		s.renderError(w, r, status)
+	}
+}
+
 func (s *Handlers) respondJSON(
 	w http.ResponseWriter,
 	_ *http.Request,
@@ -163,15 +173,72 @@ func (s *Handlers) respondJSON(
 	}
 }
 
-// serverError logs an error and sends a 500 response.
+// serverError logs an error and answers with the 500 error page.
 func (s *Handlers) serverError(
-	w http.ResponseWriter, msg string, err error,
+	w http.ResponseWriter, r *http.Request, msg string, err error,
 ) {
 	s.log.Error(msg, "error", err)
-	http.Error(
-		w, "Internal server error",
-		http.StatusInternalServerError,
-	)
+	s.renderError(w, r, http.StatusInternalServerError)
+}
+
+// renderError answers with status and the error page: the normal
+// layout, one fixed line explaining the status, and a link back to the
+// webhook list, or to sign-in when nobody is signed in.
+//
+// It renders the page itself rather than through renderTemplate,
+// whose own failure comes here. If the error page cannot render
+// either, the answer is the same status in plain text: never a second
+// attempt, and never a different status.
+func (s *Handlers) renderError(
+	w http.ResponseWriter,
+	r *http.Request,
+	status int,
+) {
+	data := s.pageData(r, map[string]any{
+		"Status":     status,
+		"StatusText": http.StatusText(status),
+		"Message":    errorPageText(status),
+	})
+
+	var buf bytes.Buffer
+
+	err := s.templates["error.html"].Execute(&buf, data)
+	if err != nil {
+		s.log.Error("failed to render error page", "error", err)
+		http.Error(w, http.StatusText(status), status)
+
+		return
+	}
+
+	w.Header().Set("Content-Type", "text/html; charset=utf-8")
+	w.WriteHeader(status)
+
+	_, err = buf.WriteTo(w)
+	if err != nil {
+		s.log.Error("failed to write error page", "error", err)
+	}
+}
+
+// errorPageText is the line the error page shows for status. It is
+// fixed per status, so the page tells the reader no more than the
+// plain-text answers it replaced did.
+func errorPageText(status int) string {
+	switch status {
+	case http.StatusBadRequest:
+		return "The request could not be read."
+	case http.StatusForbidden:
+		return "The request was refused. If it came from a form " +
+			"left open for a long time, reload the page and try " +
+			"again."
+	case http.StatusNotFound:
+		return "There is nothing here. It may have been deleted, " +
+			"or the address may be wrong."
+	case http.StatusServiceUnavailable:
+		return "The server is busy. Please try again in a moment."
+	default: // http.StatusInternalServerError
+		return "Something went wrong on the server. Please try " +
+			"again."
+	}
 }
 
 // UserInfo represents user information for templates
@@ -224,14 +291,17 @@ func (s *Handlers) renderTemplate(
 			"template not found",
 			"template", pageTemplate,
 		)
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
+		s.renderError(w, r, http.StatusInternalServerError)
 
 		return
 	}
 
+	s.executeTemplate(w, r, tmpl, s.pageData(r, data))
+}
+
+// pageData adds the fields the shared layout renders to a page's own
+// data.
+func (s *Handlers) pageData(r *http.Request, data any) any {
 	userInfo := s.getUserInfo(r)
 	csrfToken := middleware.CSRFToken(r)
 
@@ -245,19 +315,16 @@ func (s *Handlers) renderTemplate(
 		m["User"] = userInfo
 		m["CSRFToken"] = csrfToken
 		m["Version"] = version
-		s.executeTemplate(w, tmpl, m)
 
-		return
+		return m
 	}
 
-	wrapper := templateDataWrapper{
+	return templateDataWrapper{
 		User:      userInfo,
 		CSRFToken: csrfToken,
 		Version:   version,
 		Data:      data,
 	}
-
-	s.executeTemplate(w, tmpl, wrapper)
 }
 
 // executeTemplate renders the template into a buffer and writes to
@@ -270,6 +337,7 @@ func (s *Handlers) renderTemplate(
 // this reason.
 func (s *Handlers) executeTemplate(
 	w http.ResponseWriter,
+	r *http.Request,
 	tmpl *template.Template,
 	data any,
 ) {
@@ -280,10 +348,7 @@ func (s *Handlers) executeTemplate(
 		s.log.Error(
 			"failed to execute template", "error", err,
 		)
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
+		s.renderError(w, r, http.StatusInternalServerError)
 
 		return
 	}

internal/handlers/handlers_test.go

@@ -307,10 +307,14 @@ func TestRenderTemplateMidRenderErrorSendsNoPartialBody(t *testing.T) {
 		t, http.StatusInternalServerError, w.Code,
 		"a failed render must report a 500",
 	)
-	assert.Equal(
-		t, "Internal server error\n", w.Body.String(),
+	assert.NotContains(
+		t, w.Body.String(), partialPageMarker,
 		"the response must carry no part of the aborted page",
 	)
+	assert.Contains(
+		t, w.Body.String(), "500 Internal Server Error",
+		"a failed render must answer with the error page",
+	)
 }
 
 func TestBuildDatabaseTargetConfig_Valid(t *testing.T) {

internal/handlers/profile.go

@@ -1,7 +1,6 @@
 package handlers
 
 import (
-	"context"
 	"net/http"
 
 	"github.com/go-chi/chi"
@@ -37,14 +36,14 @@ func (h *Handlers) HandlePasswordChange() http.HandlerFunc {
 		err := r.ParseForm()
 		if err != nil {
 			h.log.Error("failed to parse form", "error", err)
-			http.Error(w, "Bad request", http.StatusBadRequest)
+			h.renderError(w, r, http.StatusBadRequest)
 
 			return
 		}
 
 		successMessage, errorMessage, handled := h.applyPasswordChange(
-			r.Context(),
 			w,
+			r,
 			sessionUsername,
 			// PostFormValue, not FormValue: the credential must
 			// come from the body, never from the query string.
@@ -66,12 +65,12 @@ func (h *Handlers) HandlePasswordChange() http.HandlerFunc {
 // applyPasswordChange verifies the current password and, on success,
 // persists a fresh hash for the user, reusing the same helpers that
 // bootstrap the admin user. It returns the success and error messages
-// to display on the profile page. On an internal failure it writes a
-// 500 response itself and returns handled=false, signalling the caller
+// to display on the profile page. On an internal failure it writes the
+// error page itself and returns handled=false, signalling the caller
 // to stop without re-rendering the page.
 func (h *Handlers) applyPasswordChange(
-	ctx context.Context,
 	w http.ResponseWriter,
+	r *http.Request,
 	username, currentPassword, newPassword, confirmPassword string,
 ) (string, string, bool) {
 	// This endpoint verifies one password and hashes another, at
@@ -79,15 +78,10 @@ func (h *Handlers) applyPasswordChange(
 	// endpoint uses. The bound is per hash, not per endpoint: leaving
 	// this path outside it would leave a hole in it. The slot is held
 	// across both hashes.
-	release, ok := h.mw.BeginPasswordVerification(ctx)
+	release, ok := h.mw.BeginPasswordVerification(r.Context())
 	if !ok {
 		h.log.Warn("password verification capacity exhausted")
-		http.Error(
-			w,
-			"The server is busy verifying credentials. "+
-				"Please try again.",
-			http.StatusServiceUnavailable,
-		)
+		h.renderError(w, r, http.StatusServiceUnavailable)
 
 		return "", "", false
 	}
@@ -103,7 +97,7 @@ func (h *Handlers) applyPasswordChange(
 	).First(&user).Error
 	if err != nil {
 		h.serverError(
-			w, "failed to load user for password change", err,
+			w, r, "failed to load user for password change", err,
 		)
 
 		return "", "", false
@@ -113,7 +107,7 @@ func (h *Handlers) applyPasswordChange(
 		currentPassword, user.Password,
 	)
 	if err != nil {
-		h.serverError(w, "failed to verify password", err)
+		h.serverError(w, r, "failed to verify password", err)
 
 		return "", "", false
 	}
@@ -132,7 +126,7 @@ func (h *Handlers) applyPasswordChange(
 
 	hashedPassword, err := database.HashPassword(newPassword)
 	if err != nil {
-		h.serverError(w, "failed to hash new password", err)
+		h.serverError(w, r, "failed to hash new password", err)
 
 		return "", "", false
 	}
@@ -141,7 +135,7 @@ func (h *Handlers) applyPasswordChange(
 		"password", hashedPassword,
 	).Error
 	if err != nil {
-		h.serverError(w, "failed to update password", err)
+		h.serverError(w, r, "failed to update password", err)
 
 		return "", "", false
 	}
@@ -162,7 +156,7 @@ func (h *Handlers) profileOwnerOrDeny(
 ) (string, string, bool) {
 	requestedUsername := chi.URLParam(r, "username")
 	if requestedUsername == "" {
-		http.NotFound(w, r)
+		h.renderError(w, r, http.StatusNotFound)
 
 		return "", "", false
 	}
@@ -172,7 +166,7 @@ func (h *Handlers) profileOwnerOrDeny(
 	// unexpected retrieval error.
 	sess, err := h.session.Get(r)
 	if err != nil {
-		h.serverError(w, "failed to get session", err)
+		h.serverError(w, r, "failed to get session", err)
 
 		return "", "", false
 	}
@@ -180,10 +174,7 @@ func (h *Handlers) profileOwnerOrDeny(
 	sessionUsername, ok := h.session.GetUsername(sess)
 	if !ok {
 		h.log.Error("authenticated session missing username")
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
+		h.renderError(w, r, http.StatusInternalServerError)
 
 		return "", "", false
 	}
@@ -191,17 +182,14 @@ func (h *Handlers) profileOwnerOrDeny(
 	sessionUserID, ok := h.session.GetUserID(sess)
 	if !ok {
 		h.log.Error("authenticated session missing user ID")
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
+		h.renderError(w, r, http.StatusInternalServerError)
 
 		return "", "", false
 	}
 
 	// Only allow users to act on their own profile.
 	if requestedUsername != sessionUsername {
-		http.Error(w, "Forbidden", http.StatusForbidden)
+		h.renderError(w, r, http.StatusForbidden)
 
 		return "", "", false
 	}

internal/handlers/profile_test.go

@@ -88,8 +88,6 @@ func TestHandleProfile_OwnProfile_OK(t *testing.T) {
 	h.HandleProfile().ServeHTTP(w, req)
 
 	assert.Equal(t, http.StatusOK, w.Code)
-	assert.Contains(t, w.Body.String(), "Account Information")
-	assert.NotContains(t, w.Body.String(), "Account Type")
 }
 
 func TestHandleProfile_OtherProfile_Forbidden(t *testing.T) {
@@ -128,7 +126,9 @@ func TestUserRoute_Unauthenticated_RedirectedByMiddleware(t *testing.T) {
 
 	var sess *session.Session
 
-	app := newTestApp(t, &log, &cfg, &sess)
+	var h *handlers.Handlers
+
+	app := newTestApp(t, &log, &cfg, &sess, &h)
 	app.RequireStart()
 
 	t.Cleanup(app.RequireStop)
@@ -139,7 +139,7 @@ func TestUserRoute_Unauthenticated_RedirectedByMiddleware(t *testing.T) {
 
 	router := chi.NewRouter()
 	router.Route("/user/{username}", func(r chi.Router) {
-		r.Use(mw.CSRF())
+		r.Use(mw.CSRF(h.HandleErrorPage(http.StatusForbidden)))
 		r.Use(mw.RequireAuth())
 		r.Get("/", func(w http.ResponseWriter, _ *http.Request) {
 			handlerReached = true

internal/handlers/recent_events.go

@@ -1,293 +0,0 @@
-package handlers
-
-import (
-	"net/http"
-	"slices"
-	"strconv"
-	"time"
-
-	"github.com/dustin/go-humanize"
-	"gorm.io/gorm"
-	"sneak.berlin/go/webhooker/internal/database"
-)
-
-// recentEventColumns is the recent events list's projection. It
-// leaves out the body, for the reason maxRenderedBodyBytes gives,
-// and reads its size from body_bytes, recorded when the event was
-// stored.
-const recentEventColumns = "id, created_at, method, content_type, " +
-	"resubmitted_from_id, body_bytes"
-
-// recentAttemptColumns is the part of a recorded attempt the list
-// uses. The event log's deliveryResultColumns also reads response
-// bodies, which the list does not show.
-const recentAttemptColumns = "delivery_id, status_code, created_at"
-
-// RecentEventView is one row of the recent events list on a
-// webhook's page.
-type RecentEventView struct {
-	Method      string
-	ContentType string
-
-	// ResubmittedFromID names the event this one was copied from,
-	// empty for an event that arrived on the receiver.
-	ResubmittedFromID string
-
-	// Received is how long ago the event arrived, and ReceivedUTC
-	// the full timestamp the page shows on hover.
-	Received    string
-	ReceivedUTC string
-
-	// Size is the size of the stored body.
-	Size string
-
-	// ProcessingTime is how long the event's slowest delivery
-	// took; see processingTime.
-	ProcessingTime string
-
-	// Status is what the webhook's HTTP target answered, and
-	// StatusClass its colour; see targetStatus. Both are empty
-	// unless the webhook has exactly one HTTP target.
-	Status      string
-	StatusClass string
-}
-
-// recentEventRow is one row of recentEventColumns.
-type recentEventRow struct {
-	ID                string
-	CreatedAt         time.Time
-	Method            string
-	ContentType       string
-	ResubmittedFromID *string
-	BodyBytes         uint64
-}
-
-// recentAttemptRow is one row of recentAttemptColumns. CreatedAt is
-// when the attempt's result was recorded, which is when the attempt
-// finished.
-type recentAttemptRow struct {
-	DeliveryID string
-	StatusCode int
-	CreatedAt  time.Time
-}
-
-// singleHTTPTargetID returns the ID of the webhook's HTTP target
-// when it has exactly one, and "" when it has none or several.
-func singleHTTPTargetID(targets []database.Target) string {
-	id := ""
-	count := 0
-
-	for i := range targets {
-		if targets[i].Type == database.TargetTypeHTTP {
-			id = targets[i].ID
-			count++
-		}
-	}
-
-	if count != 1 {
-		return ""
-	}
-
-	return id
-}
-
-// loadRecentEvents loads the webhook's recentEventLimit newest
-// events for its page, newest first. statusTargetID is the
-// webhook's only HTTP target, or "" when the list shows no status.
-func loadRecentEvents(
-	webhookDB *gorm.DB, webhookID, statusTargetID string,
-) ([]RecentEventView, error) {
-	var rows []recentEventRow
-
-	err := webhookDB.Model(&database.Event{}).
-		Select(recentEventColumns).
-		Where("webhook_id = ?", webhookID).
-		Order("created_at DESC").
-		Limit(recentEventLimit).
-		Find(&rows).Error
-	if err != nil {
-		return nil, err
-	}
-
-	eventIDs := make([]string, len(rows))
-	for i := range rows {
-		eventIDs[i] = rows[i].ID
-	}
-
-	// Oldest first, so an event's last delivery to a target is its
-	// newest: a replay adds a delivery rather than changing the
-	// earlier one.
-	var deliveries []database.Delivery
-
-	err = webhookDB.
-		Select("id, event_id, target_id, status, created_at").
-		Where("event_id IN ?", eventIDs).
-		Order("created_at ASC").
-		Find(&deliveries).Error
-	if err != nil {
-		return nil, err
-	}
-
-	byEvent := make(map[string][]database.Delivery, len(rows))
-	deliveryIDs := make([]string, len(deliveries))
-
-	for i := range deliveries {
-		eventID := deliveries[i].EventID
-		byEvent[eventID] = append(byEvent[eventID], deliveries[i])
-		deliveryIDs[i] = deliveries[i].ID
-	}
-
-	attempts, err := loadRecentAttempts(webhookDB, deliveryIDs)
-	if err != nil {
-		return nil, err
-	}
-
-	views := make([]RecentEventView, len(rows))
-	for i := range rows {
-		views[i] = rows[i].view(
-			byEvent[rows[i].ID], attempts, statusTargetID,
-		)
-	}
-
-	return views, nil
-}
-
-// loadRecentAttempts loads the recorded attempts of the listed
-// events' deliveries, keyed by delivery ID, each delivery's in
-// attempt order. The IDs go in chunks for the reason
-// deliveryIDChunkSize gives.
-func loadRecentAttempts(
-	webhookDB *gorm.DB, deliveryIDs []string,
-) (map[string][]recentAttemptRow, error) {
-	byDelivery := make(map[string][]recentAttemptRow)
-
-	for chunk := range slices.Chunk(deliveryIDs, deliveryIDChunkSize) {
-		var rows []recentAttemptRow
-
-		err := webhookDB.Model(&database.DeliveryResult{}).
-			Select(recentAttemptColumns).
-			Where("delivery_id IN ?", chunk).
-			Order("attempt_num ASC").
-			Find(&rows).Error
-		if err != nil {
-			return nil, err
-		}
-
-		for i := range rows {
-			id := rows[i].DeliveryID
-			byDelivery[id] = append(byDelivery[id], rows[i])
-		}
-	}
-
-	return byDelivery, nil
-}
-
-// view projects a loaded row for rendering. deliveries is the
-// event's deliveries, oldest first, and attempts their recorded
-// attempts keyed by delivery ID.
-func (r *recentEventRow) view(
-	deliveries []database.Delivery,
-	attempts map[string][]recentAttemptRow,
-	statusTargetID string,
-) RecentEventView {
-	v := RecentEventView{
-		Method:         r.Method,
-		ContentType:    r.ContentType,
-		Received:       humanize.Time(r.CreatedAt),
-		ReceivedUTC:    r.CreatedAt.UTC().Format(time.DateTime) + " UTC",
-		Size:           humanize.Bytes(r.BodyBytes),
-		ProcessingTime: processingTime(deliveries, attempts),
-	}
-
-	if r.ResubmittedFromID != nil {
-		v.ResubmittedFromID = *r.ResubmittedFromID
-	}
-
-	if statusTargetID != "" {
-		v.Status, v.StatusClass = targetStatus(
-			deliveries, attempts, statusTargetID,
-		)
-	}
-
-	return v
-}
-
-// processingTime is how long the event's slowest delivery took,
-// from being queued to its last recorded attempt, time spent
-// waiting between retries included. A delivery is queued when its
-// event is received, or when an operator replays it, so a replay
-// is timed from the replay rather than from the event's arrival.
-// It is "in progress" while any delivery is pending or retrying,
-// and empty for an event with no deliveries.
-func processingTime(
-	deliveries []database.Delivery,
-	attempts map[string][]recentAttemptRow,
-) string {
-	if len(deliveries) == 0 {
-		return ""
-	}
-
-	var slowest time.Duration
-
-	for i := range deliveries {
-		if !deliveries[i].Status.Terminal() {
-			return "in progress"
-		}
-
-		tries := attempts[deliveries[i].ID]
-		if len(tries) == 0 {
-			continue
-		}
-
-		last := tries[len(tries)-1].CreatedAt
-		slowest = max(slowest, last.Sub(deliveries[i].CreatedAt))
-	}
-
-	return slowest.Round(time.Millisecond).String()
-}
-
-// targetStatus is what the target answered for the event, and the
-// colour to show it in: the HTTP status code of the last attempt of
-// the event's newest delivery to the target. Without a code it is
-// "no response" when that attempt failed before a response
-// arrived, the delivery's status ("pending") before any attempt,
-// and "not sent" when the event has no delivery to the target.
-func targetStatus(
-	deliveries []database.Delivery,
-	attempts map[string][]recentAttemptRow,
-	targetID string,
-) (string, string) {
-	newest := -1
-
-	for i := range deliveries {
-		if deliveries[i].TargetID == targetID {
-			newest = i
-		}
-	}
-
-	if newest < 0 {
-		return "not sent", "text-gray-400"
-	}
-
-	tries := attempts[deliveries[newest].ID]
-	if len(tries) == 0 {
-		return string(deliveries[newest].Status), "text-gray-400"
-	}
-
-	code := tries[len(tries)-1].StatusCode
-
-	switch {
-	case code == 0:
-		return "no response", "text-red-600"
-	case code >= http.StatusInternalServerError:
-		return strconv.Itoa(code), "text-red-600"
-	case code >= http.StatusBadRequest:
-		return strconv.Itoa(code), "text-yellow-600"
-	case code >= http.StatusMultipleChoices:
-		return strconv.Itoa(code), "text-gray-500"
-	case code >= http.StatusOK:
-		return strconv.Itoa(code), "text-green-600"
-	default:
-		return strconv.Itoa(code), "text-gray-500"
-	}
-}

internal/handlers/recent_events_test.go

@@ -1,362 +0,0 @@
-package handlers_test
-
-import (
-	"context"
-	"fmt"
-	"net/http"
-	"net/http/httptest"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/go-chi/chi"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gorm.io/gorm"
-	"gorm.io/gorm/clause"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/handlers"
-	"sneak.berlin/go/webhooker/internal/session"
-)
-
-// statusTitle marks the status column's cell in a recent events
-// row; it is absent from the page when the column is not shown.
-const statusTitle = `title="HTTP status from the HTTP target"`
-
-// recentEventsFixture is one started app and a webhook whose
-// recent events list a test fills.
-type recentEventsFixture struct {
-	h         *handlers.Handlers
-	sess      *session.Session
-	db        *database.Database
-	webhook   *database.Webhook
-	webhookDB *gorm.DB
-}
-
-func newRecentEventsFixture(t *testing.T) *recentEventsFixture {
-	t.Helper()
-
-	f := &recentEventsFixture{}
-
-	var dbMgr *database.WebhookDBManager
-
-	app := newTestApp(t, &f.h, &f.sess, &f.db, &dbMgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	f.webhook = seedWebhook(t, f.db)
-
-	webhookDB, err := dbMgr.GetDB(f.webhook.ID)
-	require.NoError(t, err)
-
-	f.webhookDB = webhookDB
-
-	return f
-}
-
-func (f *recentEventsFixture) render(t *testing.T) string {
-	t.Helper()
-
-	return renderSourceDetailPage(t, f.h, f.sess, f.webhook.ID)
-}
-
-// event records an event received at receivedAt, with its body's
-// size as the receiver records it.
-func (f *recentEventsFixture) event(
-	t *testing.T, contentType, body string, receivedAt time.Time,
-) *database.Event {
-	t.Helper()
-
-	event := &database.Event{
-		WebhookID:   f.webhook.ID,
-		Method:      http.MethodPost,
-		Body:        body,
-		BodyBytes:   int64(len(body)),
-		ContentType: contentType,
-	}
-	event.CreatedAt = receivedAt
-
-	require.NoError(t, f.webhookDB.Omit(
-		clause.Associations,
-	).Create(event).Error)
-
-	return event
-}
-
-// delivery records a delivery of the event to the target, queued
-// when the event was received.
-func (f *recentEventsFixture) delivery(
-	t *testing.T,
-	event *database.Event,
-	targetID string,
-	status database.DeliveryStatus,
-) *database.Delivery {
-	t.Helper()
-
-	return f.deliveryQueuedAt(
-		t, event, targetID, status, event.CreatedAt,
-	)
-}
-
-// deliveryQueuedAt records a delivery of the event to the target,
-// queued at queuedAt, as a replay is.
-func (f *recentEventsFixture) deliveryQueuedAt(
-	t *testing.T,
-	event *database.Event,
-	targetID string,
-	status database.DeliveryStatus,
-	queuedAt time.Time,
-) *database.Delivery {
-	t.Helper()
-
-	dlv := &database.Delivery{
-		EventID:  event.ID,
-		TargetID: targetID,
-		Status:   status,
-	}
-	dlv.CreatedAt = queuedAt
-
-	require.NoError(t, f.webhookDB.Omit(
-		clause.Associations,
-	).Create(dlv).Error)
-
-	return dlv
-}
-
-// attempt records one attempt of the delivery that finished took
-// after the delivery was queued, with HTTP status code (0 for no
-// response).
-func (f *recentEventsFixture) attempt(
-	t *testing.T, dlv *database.Delivery, code int, took time.Duration,
-) {
-	t.Helper()
-
-	result := &database.DeliveryResult{
-		DeliveryID: dlv.ID,
-		AttemptNum: 1,
-		StatusCode: code,
-	}
-	result.CreatedAt = dlv.CreatedAt.Add(took)
-
-	require.NoError(t, f.webhookDB.Omit(
-		clause.Associations,
-	).Create(result).Error)
-}
-
-// statusCell is the status column's cell as the page renders it.
-func statusCell(class, text string) string {
-	return `<span class="font-medium ` + class + `" ` + statusTitle +
-		`>` + text + `</span>`
-}
-
-// TestHandleSourceDetail_ShowsFiftyNewestEvents proves the list is
-// headed "50 Most Recent Events" and holds the 50 newest events,
-// newest first, and not one more.
-func TestHandleSourceDetail_ShowsFiftyNewestEvents(t *testing.T) {
-	t.Parallel()
-
-	f := newRecentEventsFixture(t)
-	base := time.Now().Add(-time.Hour)
-
-	for i := range 51 {
-		f.event(
-			t, fmt.Sprintf("application/x-recent-%02d", i), "{}",
-			base.Add(time.Duration(i)*time.Second),
-		)
-	}
-
-	body := f.render(t)
-
-	assert.Contains(t, body, ">50 Most Recent Events</h2>")
-	assert.Equal(t, 50, strings.Count(body, `title="Body size"`))
-	assert.NotContains(t, body, "application/x-recent-00")
-	assert.Contains(t, body, "application/x-recent-01")
-	assert.Less(
-		t,
-		strings.Index(body, "application/x-recent-50"),
-		strings.Index(body, "application/x-recent-49"),
-	)
-}
-
-// TestHandleSourceDetail_RecentEventColumns proves a row shows its
-// time relative with the UTC timestamp on hover, its body size,
-// and its processing time once every delivery has finished.
-func TestHandleSourceDetail_RecentEventColumns(t *testing.T) {
-	t.Parallel()
-
-	f := newRecentEventsFixture(t)
-	logTarget := seedTarget(t, f.db, f.webhook.ID, database.TargetTypeLog)
-
-	receivedAt := time.Now().Add(-210 * time.Second).
-		UTC().Truncate(time.Second)
-
-	done := f.event(
-		t, contentTypeJSON, strings.Repeat("x", 2048), receivedAt,
-	)
-	f.attempt(
-		t,
-		f.delivery(t, done, logTarget.ID, database.DeliveryStatusDelivered),
-		0, 1500*time.Millisecond,
-	)
-
-	waiting := f.event(t, "text/plain", "{}", receivedAt)
-	f.delivery(t, waiting, logTarget.ID, database.DeliveryStatusPending)
-
-	body := f.render(t)
-
-	assert.Contains(
-		t, body,
-		`<span title="`+receivedAt.Format(time.DateTime)+
-			` UTC">3 minutes ago</span>`,
-	)
-	assert.Contains(t, body, `<span title="Body size">2.0 kB</span>`)
-	assert.Contains(t, body, ">1.5s</span>")
-	assert.Contains(t, body, ">in progress</span>")
-}
-
-// TestHandleSourceDetail_StatusWithSingleHTTPTarget proves that a
-// webhook with exactly one HTTP target shows, colour-coded, what
-// that target answered for each event. The log target beside it
-// does not count against "exactly one".
-func TestHandleSourceDetail_StatusWithSingleHTTPTarget(t *testing.T) {
-	t.Parallel()
-
-	f := newRecentEventsFixture(t)
-	target := seedTarget(t, f.db, f.webhook.ID, database.TargetTypeHTTP)
-	seedTarget(t, f.db, f.webhook.ID, database.TargetTypeLog)
-
-	now := time.Now()
-
-	for _, code := range []int{204, 302, 404, 503, 0} {
-		dlv := f.delivery(
-			t, f.event(t, contentTypeJSON, "{}", now), target.ID,
-			database.DeliveryStatusDelivered,
-		)
-		f.attempt(t, dlv, code, time.Second)
-	}
-
-	f.delivery(
-		t, f.event(t, contentTypeJSON, "{}", now), target.ID,
-		database.DeliveryStatusPending,
-	)
-	f.event(t, contentTypeJSON, "{}", now)
-
-	// A replay is a newer delivery, and its answer is the one shown.
-	replayed := f.event(t, contentTypeJSON, "{}", now)
-	f.attempt(t, f.delivery(
-		t, replayed, target.ID, database.DeliveryStatusFailed,
-	), 502, time.Second)
-	f.attempt(t, f.deliveryQueuedAt(
-		t, replayed, target.ID, database.DeliveryStatusDelivered,
-		now.Add(time.Minute),
-	), 200, time.Second)
-
-	body := f.render(t)
-
-	assert.Contains(t, body, statusCell("text-green-600", "204"))
-	assert.Contains(t, body, statusCell("text-gray-500", "302"))
-	assert.Contains(t, body, statusCell("text-yellow-600", "404"))
-	assert.Contains(t, body, statusCell("text-red-600", "503"))
-	assert.Contains(t, body, statusCell("text-red-600", "no response"))
-	assert.Contains(t, body, statusCell("text-gray-400", "pending"))
-	assert.Contains(t, body, statusCell("text-gray-400", "not sent"))
-	assert.Contains(t, body, statusCell("text-green-600", "200"))
-	assert.NotContains(t, body, ">502<")
-}
-
-// TestHandleSourceDetail_NoStatusWithoutSingleHTTPTarget proves the
-// status column is absent when the webhook has no HTTP target or
-// more than one.
-func TestHandleSourceDetail_NoStatusWithoutSingleHTTPTarget(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	cases := map[string][]database.TargetType{
-		"none":    {database.TargetTypeLog},
-		"several": {database.TargetTypeHTTP, database.TargetTypeHTTP},
-	}
-
-	for name, types := range cases {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			f := newRecentEventsFixture(t)
-			event := f.event(t, contentTypeJSON, "{}", time.Now())
-
-			for _, tt := range types {
-				target := seedTarget(t, f.db, f.webhook.ID, tt)
-				f.attempt(t, f.delivery(
-					t, event, target.ID,
-					database.DeliveryStatusDelivered,
-				), 200, time.Second)
-			}
-
-			body := f.render(t)
-
-			assert.Contains(t, body, `title="Body size"`)
-			assert.NotContains(t, body, statusTitle)
-		})
-	}
-}
-
-// TestHandleWebhook_RecordsBodySize proves the receiver records the
-// body's size in bytes, not characters, with the event it stores.
-func TestHandleWebhook_RecordsBodySize(t *testing.T) {
-	t.Parallel()
-
-	f := newRecentEventsFixture(t)
-	seedEntrypoint(t, f.db, f.webhook.ID)
-
-	// Two bytes per character.
-	body := strings.Repeat("é", 1024)
-
-	req := httptest.NewRequestWithContext(
-		context.Background(), http.MethodPost, "/webhook/x",
-		strings.NewReader(body),
-	)
-
-	rctx := chi.NewRouteContext()
-	rctx.URLParams.Add("uuid", "ep-"+f.webhook.ID)
-
-	req = req.WithContext(context.WithValue(
-		req.Context(), chi.RouteCtxKey, rctx,
-	))
-
-	w := httptest.NewRecorder()
-	f.h.HandleWebhook().ServeHTTP(w, req)
-
-	require.Equal(t, http.StatusOK, w.Code)
-
-	var stored database.Event
-
-	require.NoError(t, f.webhookDB.First(&stored).Error)
-	assert.Equal(t, int64(2048), stored.BodyBytes)
-}
-
-// TestHandleSourceDetail_FailedLoadIsAnError proves that when the
-// list cannot be loaded the page answers with an error, rather than
-// an empty list claiming the webhook has no events.
-func TestHandleSourceDetail_FailedLoadIsAnError(t *testing.T) {
-	t.Parallel()
-
-	f := newRecentEventsFixture(t)
-	target := seedTarget(t, f.db, f.webhook.ID, database.TargetTypeHTTP)
-
-	f.attempt(t, f.delivery(
-		t, f.event(t, contentTypeJSON, "{}", time.Now()), target.ID,
-		database.DeliveryStatusDelivered,
-	), 200, time.Second)
-
-	// The attempts are the list's last query, so its events and
-	// deliveries have already loaded when it fails.
-	require.NoError(t, f.webhookDB.Exec(
-		"DROP TABLE delivery_results",
-	).Error)
-
-	w := serveSourceDetailPage(t, f.h, f.sess, f.webhook.ID)
-
-	assert.Equal(t, http.StatusInternalServerError, w.Code)
-	assert.NotContains(t, w.Body.String(), "No events received yet.")
-}

internal/handlers/source_detail_test.go

@@ -62,23 +62,6 @@ func renderSourceDetailPage(
 ) string {
 	t.Helper()
 
-	w := serveSourceDetailPage(t, h, sess, webhookID)
-
-	require.Equal(t, http.StatusOK, w.Code)
-
-	return w.Body.String()
-}
-
-// serveSourceDetailPage runs the real source detail handler for a
-// webhook and returns its response, whatever its status.
-func serveSourceDetailPage(
-	t *testing.T,
-	h *handlers.Handlers,
-	sess *session.Session,
-	webhookID string,
-) *httptest.ResponseRecorder {
-	t.Helper()
-
 	req := httptest.NewRequestWithContext(
 		context.Background(),
 		http.MethodGet,
@@ -104,7 +87,9 @@ func serveSourceDetailPage(
 	w := httptest.NewRecorder()
 	h.HandleSourceDetail().ServeHTTP(w, req)
 
-	return w
+	require.Equal(t, http.StatusOK, w.Code)
+
+	return w.Body.String()
 }
 
 // TestHandleSourceDetail_MasksSlackWebhookURL is the

internal/handlers/source_management.go

@@ -149,13 +149,7 @@ func (h *Handlers) HandleSourceList() http.HandlerFunc {
 			"user_id = ?", userID,
 		).Order("created_at DESC").Find(&webhooks).Error
 		if err != nil {
-			h.log.Error(
-				"failed to list webhooks", "error", err,
-			)
-			http.Error(
-				w, "Internal server error",
-				http.StatusInternalServerError,
-			)
+			h.serverError(w, r, "failed to list webhooks", err)
 
 			return
 		}
@@ -249,9 +243,7 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err := r.ParseForm()
 		if err != nil {
-			http.Error(
-				w, "Bad request", http.StatusBadRequest,
-			)
+			h.renderError(w, r, http.StatusBadRequest)
 
 			return
 		}
@@ -311,7 +303,7 @@ func (h *Handlers) createWebhookWithEntrypoint(
 
 	err := h.commitWebhook(webhook)
 	if err != nil {
-		h.serverError(w, "failed to create webhook", err)
+		h.serverError(w, r, "failed to create webhook", err)
 
 		return
 	}
@@ -388,7 +380,7 @@ func (h *Handlers) HandleSourceDetail() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			http.NotFound(w, r)
+			h.renderError(w, r, http.StatusNotFound)
 
 			return
 		}
@@ -415,23 +407,16 @@ func (h *Handlers) renderSourceDetail(
 		"webhook_id = ?", webhook.ID,
 	).Find(&targets)
 
-	var events []RecentEventView
+	var events []database.Event
 
 	if h.dbMgr.DBExists(webhook.ID) {
-		webhookDB, err := h.dbMgr.GetDB(webhook.ID)
-		if err != nil {
-			h.serverError(w, "failed to get webhook database", err)
-
-			return
-		}
-
-		events, err = loadRecentEvents(
-			webhookDB, webhook.ID, singleHTTPTargetID(targets),
-		)
-		if err != nil {
-			h.serverError(w, "failed to load recent events", err)
-
-			return
+		webhookDB, dbErr := h.dbMgr.GetDB(webhook.ID)
+		if dbErr == nil {
+			webhookDB.Where(
+				"webhook_id = ?", webhook.ID,
+			).Order("created_at DESC").Limit(
+				recentEventLimit,
+			).Find(&events)
 		}
 	}
 
@@ -482,7 +467,7 @@ func (h *Handlers) HandleSourceEdit() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			http.NotFound(w, r)
+			h.renderError(w, r, http.StatusNotFound)
 
 			return
 		}
@@ -517,7 +502,7 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			http.NotFound(w, r)
+			h.renderError(w, r, http.StatusNotFound)
 
 			return
 		}
@@ -526,9 +511,7 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err = r.ParseForm()
 		if err != nil {
-			http.Error(
-				w, "Bad request", http.StatusBadRequest,
-			)
+			h.renderError(w, r, http.StatusBadRequest)
 
 			return
 		}
@@ -582,7 +565,7 @@ func (h *Handlers) applyWebhookEdit(
 
 	err := h.db.DB().Save(webhook).Error
 	if err != nil {
-		h.serverError(w, "failed to update webhook", err)
+		h.serverError(w, r, "failed to update webhook", err)
 
 		return
 	}
@@ -612,7 +595,7 @@ func (h *Handlers) HandleSourceDelete() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			http.NotFound(w, r)
+			h.renderError(w, r, http.StatusNotFound)
 
 			return
 		}
@@ -639,7 +622,7 @@ func (h *Handlers) deleteWebhookResources(
 	// be removed by hand; deleted history cannot be recovered.
 	err := h.commitWebhookDeletion(&webhook)
 	if err != nil {
-		h.serverError(w, "failed to delete webhook", err)
+		h.serverError(w, r, "failed to delete webhook", err)
 
 		return
 	}
@@ -665,7 +648,7 @@ func (h *Handlers) deleteWebhookResources(
 		// redirecting as though everything succeeded: the file
 		// needs removing by hand, and the logged error names it.
 		h.serverError(
-			w, "failed to delete webhook event database", err,
+			w, r, "failed to delete webhook event database", err,
 		)
 
 		return
@@ -809,7 +792,7 @@ func (h *Handlers) ownedWebhook(
 		"id = ? AND user_id = ?", sourceID, userID,
 	).First(&webhook).Error
 	if err != nil {
-		http.NotFound(w, r)
+		h.renderError(w, r, http.StatusNotFound)
 
 		return database.Webhook{}, false
 	}
@@ -831,7 +814,7 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 			// Without the map every delivery renders through a
 			// zero redactor, so failing the page is the only
 			// safe answer.
-			h.serverError(w, "failed to load targets", err)
+			h.serverError(w, r, "failed to load targets", err)
 
 			return
 		}
@@ -839,7 +822,7 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 		page := h.parsePage(r)
 
 		evts, total, ok := h.loadEventsWithDeliveries(
-			w, webhook, targets, page,
+			w, r, webhook, targets, page,
 		)
 		if !ok {
 			return
@@ -949,6 +932,7 @@ func (h *Handlers) parsePage(r *http.Request) int {
 // caller must then render nothing further.
 func (h *Handlers) loadEventsWithDeliveries(
 	w http.ResponseWriter,
+	r *http.Request,
 	webhook database.Webhook,
 	targetMap map[string]eventLogTarget,
 	page int,
@@ -962,7 +946,7 @@ func (h *Handlers) loadEventsWithDeliveries(
 	webhookDB, err := h.dbMgr.GetDB(webhook.ID)
 	if err != nil {
 		h.serverError(
-			w, "failed to get webhook database", err,
+			w, r, "failed to get webhook database", err,
 		)
 
 		return nil, 0, false
@@ -999,7 +983,7 @@ func (h *Handlers) loadEventsWithDeliveries(
 	)
 	if err != nil {
 		h.serverError(
-			w, "failed to load delivery attempts", err,
+			w, r, "failed to load delivery attempts", err,
 		)
 
 		return nil, 0, false
@@ -1008,7 +992,7 @@ func (h *Handlers) loadEventsWithDeliveries(
 	resubmits, err := resubmitCounts(webhookDB, eventIDs)
 	if err != nil {
 		h.serverError(
-			w, "failed to count event resubmissions", err,
+			w, r, "failed to count event resubmissions", err,
 		)
 
 		return nil, 0, false
@@ -1231,7 +1215,7 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			http.NotFound(w, r)
+			h.renderError(w, r, http.StatusNotFound)
 
 			return
 		}
@@ -1240,9 +1224,7 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err = r.ParseForm()
 		if err != nil {
-			http.Error(
-				w, "Bad request", http.StatusBadRequest,
-			)
+			h.renderError(w, r, http.StatusBadRequest)
 
 			return
 		}
@@ -1258,7 +1240,7 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 
 		err = h.db.DB().Create(entrypoint).Error
 		if err != nil {
-			h.serverError(w, "failed to create entrypoint", err)
+			h.serverError(w, r, "failed to create entrypoint", err)
 
 			return
 		}
@@ -1289,7 +1271,7 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			http.NotFound(w, r)
+			h.renderError(w, r, http.StatusNotFound)
 
 			return
 		}
@@ -1298,9 +1280,7 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err = r.ParseForm()
 		if err != nil {
-			http.Error(
-				w, "Bad request", http.StatusBadRequest,
-			)
+			h.renderError(w, r, http.StatusBadRequest)
 
 			return
 		}
@@ -1371,7 +1351,7 @@ func (h *Handlers) processTargetCreate(
 
 	err = h.db.DB().Create(target).Error
 	if err != nil {
-		h.serverError(w, "failed to create target", err)
+		h.serverError(w, r, "failed to create target", err)
 
 		return
 	}
@@ -1465,7 +1445,7 @@ func (h *Handlers) buildTargetConfig(
 	case database.TargetTypeSlack:
 		return h.buildSlackTargetConfig(w, r, in.URL)
 	case database.TargetTypeDatabase:
-		return h.buildDatabaseTargetConfig(w, in.Expiry)
+		return h.buildDatabaseTargetConfig(w, r, in.Expiry)
 	case database.TargetTypeLog:
 		return "", nil
 	default:
@@ -1515,7 +1495,7 @@ func (h *Handlers) buildHTTPTargetConfig(
 		return "", err
 	}
 
-	return marshalTargetConfig(w, delivery.HTTPTargetConfig{
+	return h.marshalTargetConfig(w, r, delivery.HTTPTargetConfig{
 		URL:     in.URL,
 		Headers: headers,
 		Timeout: timeout,
@@ -1537,7 +1517,7 @@ func (h *Handlers) buildSlackTargetConfig(
 		return "", err
 	}
 
-	return marshalTargetConfig(w, delivery.SlackTargetConfig{
+	return h.marshalTargetConfig(w, r, delivery.SlackTargetConfig{
 		WebhookURL: targetURL,
 	})
 }
@@ -1591,16 +1571,14 @@ func (h *Handlers) validateTargetURL(
 
 // marshalTargetConfig serialises a target configuration for storage,
 // writing a 500 itself if it cannot.
-func marshalTargetConfig(
+func (h *Handlers) marshalTargetConfig(
 	w http.ResponseWriter,
+	r *http.Request,
 	cfg any,
 ) (string, error) {
 	configBytes, err := json.Marshal(cfg)
 	if err != nil {
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
+		h.serverError(w, r, "failed to encode target config", err)
 
 		return "", err
 	}
@@ -1616,6 +1594,7 @@ func marshalTargetConfig(
 // expiry yields an empty config (the keep-forever default).
 func (h *Handlers) buildDatabaseTargetConfig(
 	w http.ResponseWriter,
+	r *http.Request,
 	expiry string,
 ) (string, error) {
 	expiry = strings.TrimSpace(expiry)
@@ -1634,8 +1613,8 @@ func (h *Handlers) buildDatabaseTargetConfig(
 		return "", err
 	}
 
-	return marshalTargetConfig(
-		w, map[string]any{"expiry": expiry},
+	return h.marshalTargetConfig(
+		w, r, map[string]any{"expiry": expiry},
 	)
 }
 
@@ -1689,7 +1668,7 @@ func (h *Handlers) deleteChildResource(
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			http.NotFound(w, r)
+			h.renderError(w, r, http.StatusNotFound)
 
 			return
 		}
@@ -1699,11 +1678,7 @@ func (h *Handlers) deleteChildResource(
 			childID, webhook.ID,
 		).Delete(model)
 		if result.Error != nil {
-			h.log.Error(errMsg, "error", result.Error)
-			http.Error(
-				w, "Internal server error",
-				http.StatusInternalServerError,
-			)
+			h.serverError(w, r, errMsg, result.Error)
 
 			return
 		}
@@ -1793,18 +1768,14 @@ func (h *Handlers) toggleChildResource(
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			http.NotFound(w, r)
+			h.renderError(w, r, http.StatusNotFound)
 
 			return
 		}
 
 		err = toggleFn(webhook.ID, childID)
 		if err != nil {
-			h.log.Error(errMsg, "error", err)
-			http.Error(
-				w, "Internal server error",
-				http.StatusInternalServerError,
-			)
+			h.serverError(w, r, errMsg, err)
 
 			return
 		}

internal/handlers/target_edit.go

@@ -88,9 +88,7 @@ func (h *Handlers) HandleTargetEditSubmit() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err := r.ParseForm()
 		if err != nil {
-			http.Error(
-				w, "Bad request", http.StatusBadRequest,
-			)
+			h.renderError(w, r, http.StatusBadRequest)
 
 			return
 		}
@@ -157,7 +155,7 @@ func (h *Handlers) applyTargetEdit(
 
 	err = h.db.DB().Save(target).Error
 	if err != nil {
-		h.serverError(w, "failed to update target", err)
+		h.serverError(w, r, "failed to update target", err)
 
 		return
 	}
@@ -220,7 +218,7 @@ func (h *Handlers) ownedTarget(
 		chi.URLParam(r, "targetID"), webhook.ID,
 	).First(&target).Error
 	if err != nil {
-		http.NotFound(w, r)
+		h.renderError(w, r, http.StatusNotFound)
 
 		return database.Webhook{}, nil, false
 	}

internal/handlers/webhook.go

@@ -88,14 +88,14 @@ func (h *Handlers) processWebhookRequest(
 
 	headersJSON, err := json.Marshal(r.Header)
 	if err != nil {
-		h.serverError(w, "failed to serialize headers", err)
+		h.receiverError(w, "failed to serialize headers", err)
 
 		return
 	}
 
 	targets, err := h.loadActiveTargets(entrypoint.WebhookID)
 	if err != nil {
-		h.serverError(w, "failed to query targets", err)
+		h.receiverError(w, "failed to query targets", err)
 
 		return
 	}
@@ -196,7 +196,7 @@ func (h *Handlers) createAndDeliverEvent(
 		targets,
 	)
 	if err != nil {
-		h.serverError(w, "failed to store webhook event", err)
+		h.receiverError(w, "failed to store webhook event", err)
 
 		return
 	}
@@ -204,6 +204,19 @@ func (h *Handlers) createAndDeliverEvent(
 	h.finishWebhookResponse(w, event, entrypoint, tasks)
 }
 
+// receiverError logs an error and answers the sender with a plain-text
+// 500. The receiver's answers are for programs, so it never sends the
+// error page the web UI uses.
+func (h *Handlers) receiverError(
+	w http.ResponseWriter, msg string, err error,
+) {
+	h.log.Error(msg, "error", err)
+	http.Error(
+		w, "Internal server error",
+		http.StatusInternalServerError,
+	)
+}
+
 // eventSource carries the fields a new event is built from. The
 // receiver fills it from the live request; the resubmit handler fills
 // it from a stored event. Both then go through createAndFanOut, so an
@@ -230,7 +243,6 @@ func (s eventSource) event() *database.Event {
 		Method:            s.Method,
 		Headers:           s.HeadersJSON,
 		Body:              string(s.Body),
-		BodyBytes:         int64(len(s.Body)),
 		ContentType:       s.ContentType,
 		ResubmittedFromID: s.ResubmittedFromID,
 	}

internal/middleware/csrf.go

@@ -19,7 +19,7 @@ func CSRFToken(r *http.Request) string {
 // key to sign a CSRF cookie and validates a masked token submitted via
 // the "csrf_token" form field (or the "X-CSRF-Token" header) on
 // POST/PUT/PATCH/DELETE requests. Requests with an invalid or missing
-// token receive a 403 Forbidden response.
+// token are logged and answered by forbidden, which must write the 403.
 //
 // The middleware detects the client-facing transport protocol
 // per-request via reqtls.IsTLS, the single TLS predicate the session
@@ -36,7 +36,9 @@ func CSRFToken(r *http.Request) string {
 // Two gorilla/csrf instances are maintained — one with Secure cookies
 // (for TLS) and one without (for plaintext HTTP) — because the
 // csrf.Secure option is set at creation time, not per-request.
-func (m *Middleware) CSRF() func(http.Handler) http.Handler {
+func (m *Middleware) CSRF(
+	forbidden http.Handler,
+) func(http.Handler) http.Handler {
 	csrfErrorHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// CSRF is registered ahead of RequireAuth on every route
 		// group that uses it, so this WARN is reachable by an
@@ -57,7 +59,7 @@ func (m *Middleware) CSRF() func(http.Handler) http.Handler {
 			"remote_addr", r.RemoteAddr,
 			"reason", csrf.FailureReason(r),
 		)
-		http.Error(w, "Forbidden - invalid CSRF token", http.StatusForbidden)
+		forbidden.ServeHTTP(w, r)
 	})
 
 	key := m.session.GetKey()

internal/middleware/csrf_test.go

@@ -18,6 +18,12 @@ import (
 // csrfCookieName is the gorilla/csrf cookie name.
 const csrfCookieName = "_gorilla_csrf"
 
+// forbidden stands in for the error page the server hands CSRF to
+// answer a refused request with.
+func forbidden(w http.ResponseWriter, _ *http.Request) {
+	w.WriteHeader(http.StatusForbidden)
+}
+
 // csrfGetToken performs a GET request through the CSRF middleware
 // and returns the token and cookies.
 func csrfGetToken(
@@ -98,7 +104,7 @@ func TestCSRF_GETSetsToken(t *testing.T) {
 
 	var gotToken string
 
-	handler := m.CSRF()(http.HandlerFunc(
+	handler := m.CSRF(http.HandlerFunc(forbidden))(http.HandlerFunc(
 		func(_ http.ResponseWriter, r *http.Request) {
 			gotToken = middleware.CSRFToken(r)
 		},
@@ -120,7 +126,7 @@ func TestCSRF_POSTWithValidToken(t *testing.T) {
 	t.Parallel()
 
 	m, _ := testMiddleware(t, config.EnvironmentDev)
-	csrfMW := m.CSRF()
+	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
 
 	getReq := httptest.NewRequestWithContext(
 		context.Background(),
@@ -152,7 +158,7 @@ func csrfPOSTWithoutTokenTest(
 	t.Helper()
 
 	m, _ := testMiddleware(t, env)
-	csrfMW := m.CSRF()
+	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
 
 	// GET to establish the CSRF cookie
 	getHandler := csrfMW(http.HandlerFunc(
@@ -209,7 +215,7 @@ func TestCSRF_POSTWithInvalidToken(t *testing.T) {
 	t.Parallel()
 
 	m, _ := testMiddleware(t, config.EnvironmentDev)
-	csrfMW := m.CSRF()
+	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
 
 	// GET to establish the CSRF cookie
 	getHandler := csrfMW(http.HandlerFunc(
@@ -265,7 +271,7 @@ func TestCSRF_GETDoesNotValidate(t *testing.T) {
 
 	var called bool
 
-	handler := m.CSRF()(http.HandlerFunc(
+	handler := m.CSRF(http.HandlerFunc(forbidden))(http.HandlerFunc(
 		func(_ http.ResponseWriter, _ *http.Request) {
 			called = true
 		},
@@ -328,7 +334,7 @@ func csrfTookStrictPath(
 	t.Helper()
 
 	m, _ := testMiddleware(t, env)
-	csrfMW := m.CSRF()
+	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
 
 	newReq := func(method string) *http.Request {
 		r := httptest.NewRequestWithContext(
@@ -477,7 +483,7 @@ func TestCSRF_ProdMode_PlaintextHTTP_POSTWithValidToken(
 	t.Parallel()
 
 	m, _ := testMiddleware(t, config.EnvironmentProd)
-	csrfMW := m.CSRF()
+	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
 
 	getReq := httptest.NewRequestWithContext(
 		context.Background(),
@@ -517,7 +523,7 @@ func TestCSRF_ProdMode_BehindProxy_POSTWithValidToken(
 	t.Parallel()
 
 	m, _ := testMiddleware(t, config.EnvironmentProd)
-	csrfMW := m.CSRF()
+	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
 
 	getReq := httptest.NewRequestWithContext(
 		context.Background(),
@@ -562,7 +568,7 @@ func TestCSRF_ProdMode_DirectTLS_POSTWithValidToken(
 	t.Parallel()
 
 	m, _ := testMiddleware(t, config.EnvironmentProd)
-	csrfMW := m.CSRF()
+	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
 
 	getReq := httptest.NewRequestWithContext(
 		context.Background(),

internal/middleware/logbound_test.go

@@ -260,7 +260,9 @@ func logSites() map[string]logSite {
 			) http.Handler {
 				t.Helper()
 
-				return m.CSRF()(unreachable(t))
+				return m.CSRF(http.HandlerFunc(forbidden))(
+					unreachable(t),
+				)
 			},
 			send:       postNoToken,
 			wantStatus: http.StatusForbidden,

internal/server/error_page_test.go

@@ -0,0 +1,155 @@
+package server_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/config"
+)
+
+// The link back the error page offers: to the webhook list for a
+// signed-in user, to sign-in for anyone else.
+const (
+	backToWebhooks = `<a href="/sources" class="btn-secondary">` +
+		`Back to webhooks</a>`
+	backToSignIn = `<a href="/pages/login" class="btn-primary">` +
+		`Sign in</a>`
+)
+
+// assertErrorPage checks that w is the error page for status, in the
+// normal layout, offering link.
+func assertErrorPage(
+	t *testing.T,
+	w *httptest.ResponseRecorder,
+	status int,
+	link string,
+) {
+	t.Helper()
+
+	body := w.Body.String()
+
+	assert.Equal(t, status, w.Code)
+	assert.Equal(
+		t, "text/html; charset=utf-8", w.Header().Get("Content-Type"),
+	)
+	assert.Contains(t, body, `<nav class="app-bar"`)
+	assert.Contains(
+		t, body, strconv.Itoa(status)+" "+http.StatusText(status),
+	)
+	assert.Contains(t, body, link)
+}
+
+func TestErrorPage_DeletedWebhook(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	userID, _ := env.seedUser(t, "owner", "somepassword")
+	cookies := env.authCookies(t, userID, "owner")
+
+	wh := env.seedWebhook(t, userID)
+	require.NoError(t, env.db.DB().Delete(wh).Error)
+
+	w := env.get("/source/"+wh.ID, cookies)
+
+	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
+}
+
+func TestErrorPage_DeletedTarget(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	userID, _ := env.seedUser(t, "owner", "somepassword")
+	cookies := env.authCookies(t, userID, "owner")
+
+	wh := env.seedWebhook(t, userID)
+	tgt := env.seedTarget(t, wh.ID)
+	require.NoError(t, env.db.DB().Delete(tgt).Error)
+
+	w := env.get(
+		"/source/"+wh.ID+"/targets/"+tgt.ID+"/edit", cookies,
+	)
+
+	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
+}
+
+func TestErrorPage_UnknownPath(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	userID, _ := env.seedUser(t, "owner", "somepassword")
+	cookies := env.authCookies(t, userID, "owner")
+
+	assertErrorPage(
+		t, env.get("/no-such-page", nil),
+		http.StatusNotFound, backToSignIn,
+	)
+
+	// Outside every route group there is no form token, so the
+	// page leaves out the logout form rather than offer one that
+	// would be refused.
+	w := env.get("/no-such-page", cookies)
+	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
+	assert.NotContains(t, w.Body.String(), `action="/pages/logout"`)
+
+	// Inside a route group the page has a token, and logout works.
+	wh := env.seedWebhook(t, userID)
+	w = env.get("/source/"+wh.ID+"/no-such-page", cookies)
+	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
+	assert.Contains(t, w.Body.String(), `action="/pages/logout"`)
+}
+
+func TestErrorPage_BadCSRFToken(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	form := url.Values{}
+	form.Set("username", "someone")
+	form.Set("password", "irrelevant")
+	form.Set("csrf_token", "not-a-token")
+
+	assertErrorPage(
+		t, env.post("/pages/login", form, nil),
+		http.StatusForbidden, backToSignIn,
+	)
+
+	userID, _ := env.seedUser(t, "owner", "somepassword")
+	cookies := env.authCookies(t, userID, "owner")
+	wh := env.seedWebhook(t, userID)
+
+	edit := url.Values{}
+	edit.Set("name", "renamed")
+
+	assertErrorPage(
+		t, env.post("/source/"+wh.ID+"/edit", edit, cookies),
+		http.StatusForbidden, backToWebhooks,
+	)
+}
+
+// TestErrorPage_ReceiverStaysPlain pins that the error page is for
+// the web UI only: a sender posting to an entrypoint that does not
+// exist still gets the plain-text answer.
+func TestErrorPage_ReceiverStaysPlain(t *testing.T) {
+	t.Parallel()
+
+	// newTestEnv leaves the receiver rate limit at zero, which
+	// refuses every request before it reaches the receiver.
+	env := newTestEnvWithConfig(t, &config.Config{
+		DataDir:           t.TempDir(),
+		Environment:       config.EnvironmentDev,
+		ReceiverRateLimit: 10,
+	})
+
+	w := env.post("/webhook/no-such-entrypoint", url.Values{}, nil)
+
+	assert.Equal(t, http.StatusNotFound, w.Code)
+	assert.Equal(t, "404 page not found\n", w.Body.String())
+}

internal/server/routes.go

@@ -46,6 +46,14 @@ const requestTimeout = 60 * time.Second
 // server's router.
 func (s *Server) SetupRoutes() {
 	s.router = chi.NewRouter()
+
+	// An unknown path gets the error page. Registered before the
+	// global middleware, because chi wraps a not-found handler in the
+	// middleware already on its router, which would then run twice.
+	// The route groups below wrap it in their own middleware the same
+	// way; running theirs twice is harmless.
+	s.router.NotFound(s.h.HandleErrorPage(http.StatusNotFound))
+
 	s.setupGlobalMiddleware()
 	s.setupRoutes()
 }
@@ -150,7 +158,7 @@ func (s *Server) setupPageRoutes() {
 		// MaxBodySize precedes CSRF and RequireAuth deliberately;
 		// see maxFormBodySize for why, and for what it costs.
 		r.Use(s.mw.MaxBodySize(maxFormBodySize))
-		r.Use(s.mw.CSRF())
+		r.Use(s.mw.CSRF(s.h.HandleErrorPage(http.StatusForbidden)))
 		r.Use(s.mw.NoCache())
 
 		// The login POST carries no pre-emptive rate limiter. Behind
@@ -171,7 +179,7 @@ func (s *Server) setupUserRoutes() {
 		// MaxBodySize precedes CSRF and RequireAuth deliberately;
 		// see maxFormBodySize for why, and for what it costs.
 		r.Use(s.mw.MaxBodySize(maxFormBodySize))
-		r.Use(s.mw.CSRF())
+		r.Use(s.mw.CSRF(s.h.HandleErrorPage(http.StatusForbidden)))
 		r.Use(s.mw.NoCache())
 		r.Use(s.mw.RequireAuth())
 		r.Get("/", s.h.HandleProfile())
@@ -186,7 +194,7 @@ func (s *Server) setupSourceRoutes() {
 		// MaxBodySize precedes CSRF and RequireAuth deliberately;
 		// see maxFormBodySize for why, and for what it costs.
 		r.Use(s.mw.MaxBodySize(maxFormBodySize))
-		r.Use(s.mw.CSRF())
+		r.Use(s.mw.CSRF(s.h.HandleErrorPage(http.StatusForbidden)))
 		r.Use(s.mw.NoCache())
 		r.Use(s.mw.RequireAuth())
 		r.Get("/", s.h.HandleSourceList())
@@ -198,7 +206,7 @@ func (s *Server) setupSourceRoutes() {
 		// MaxBodySize precedes CSRF and RequireAuth deliberately;
 		// see maxFormBodySize for why, and for what it costs.
 		r.Use(s.mw.MaxBodySize(maxFormBodySize))
-		r.Use(s.mw.CSRF())
+		r.Use(s.mw.CSRF(s.h.HandleErrorPage(http.StatusForbidden)))
 		r.Use(s.mw.NoCache())
 		r.Use(s.mw.RequireAuth())
 		r.Get("/", s.h.HandleSourceDetail())

internal/versionscript/version_script_test.go

@@ -115,8 +115,8 @@ func TestVersion_EnclosingRepositoryIsNotUsed(t *testing.T) {
 	require.Equal(t, unknown, runScript(t, inner, nil))
 }
 
-// An explicit VERSION, such as the Dockerfile's build arg, wins over
-// anything derivable.
+// The Docker build has no git metadata, so the version arrives as an
+// environment override. It wins over anything derivable.
 func TestVersion_EnvironmentOverrideWins(t *testing.T) {
 	t.Parallel()
 
@@ -128,8 +128,8 @@ func TestVersion_EnvironmentOverrideWins(t *testing.T) {
 }
 
 // An empty VERSION is treated as unset rather than stamping an empty
-// string: a caller exporting VERSION= must not produce a binary
-// reporting "".
+// string: the Dockerfile's build arg has a non-empty default, but a
+// caller exporting VERSION= must not produce a binary reporting "".
 func TestVersion_EmptyOverrideFallsBackToGit(t *testing.T) {
 	t.Parallel()
 
@@ -168,8 +168,8 @@ func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
 }
 
 // A caller can define VERSION as the empty string -- `make build
-// VERSION=`, or the Dockerfile's `make build VERSION="$VERSION"` when no
-// VERSION build arg was given. script/version's own guard does not cover
+// VERSION=`, or a `--build-arg VERSION=` reaching the Dockerfile's `make
+// build VERSION="$VERSION"`. script/version's own guard does not cover
 // that: the value never passes through the script. Stamping "" would
 // leave the binary reporting no version and the footer on "dev", which
 // is the defect this package exists for.
@@ -231,7 +231,7 @@ func TestDockerfile_BuildsThroughTheMakeTarget(t *testing.T) {
 
 	require.NotContains(t, dockerfile, "go build",
 		"a raw go build bypasses the Makefile's -X flag")
-	require.Contains(t, dockerfile, "ARG VERSION")
+	require.Contains(t, dockerfile, "ARG VERSION=")
 	require.Contains(t, dockerfile,
 		`make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"'`)
 }

script/docker

@@ -2,9 +2,9 @@
 # script/docker: build the Docker image tagged with the project name.
 # The tag comes from script/projectname.
 #
-# The version script/version resolves here goes in as the VERSION build
-# arg, which takes precedence over what the build would derive from the
-# .git in its context.
+# .dockerignore excludes .git/, so the builder stage cannot derive the
+# version itself. It is resolved here, where the checkout is, and passed
+# in as a build arg; without it the image would stamp itself "unknown".
 set -eu
 
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"

script/version

@@ -7,16 +7,18 @@
 #
 # Order of precedence:
 #
-#   1. $VERSION, if set and non-empty: an explicit value, such as the
-#      Dockerfile's VERSION build arg.
+#   1. $VERSION, if set and non-empty. This is how the value reaches a
+#      build that cannot derive it: .dockerignore excludes .git/, so the
+#      builder stage has no git metadata and the Dockerfile takes the
+#      value as a build arg instead.
 #   2. `git describe --tags --always --dirty` against this checkout. At
 #      a clean tagged commit that is exactly the tag; otherwise it
 #      carries the short SHA, the commit distance when a tag is
 #      reachable, and a -dirty suffix for uncommitted changes.
 #   3. "unknown", for a tree with no git metadata and no $VERSION -- a
-#      source tarball, or a `docker build` with no .git in its context
-#      and no VERSION build arg. That case must not fail the build and
-#      must not name a tag the tree may not be at, so it names nothing.
+#      source tarball, or `docker build .` with no --build-arg. That
+#      case must not fail the build and must not name a tag the tree may
+#      not be at, so it names nothing.
 #
 # The git step insists the enclosing repository is this checkout, not
 # merely some repository above it: an unpacked tarball sitting inside an

templates/error.html

@@ -0,0 +1,15 @@
+{{template "base" .}}
+
+{{define "title"}}{{.StatusText}} - Webhooker{{end}}
+
+{{define "content"}}
+<div class="max-w-4xl mx-auto px-6 py-12">
+    <h1 class="text-2xl font-medium text-gray-900 mb-4">{{.Status}} {{.StatusText}}</h1>
+    <p class="text-gray-600 mb-6">{{.Message}}</p>
+    {{if .User}}
+    <a href="/sources" class="btn-secondary">Back to webhooks</a>
+    {{else}}
+    <a href="/pages/login" class="btn-primary">Sign in</a>
+    {{end}}
+</div>
+{{end}}

templates/navbar.html

@@ -24,10 +24,14 @@
                 </svg>
                 {{.User.Username}}
             </a>
+            {{/* The page for an unknown path is served outside the routes
+                 that issue a form token, and a logout without one is refused. */}}
+            {{if .CSRFToken}}
             <form method="POST" action="/pages/logout" class="inline">
                 <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
                 <button type="submit" class="btn-text">Logout</button>
             </form>
+            {{end}}
             {{else}}
             <a href="/pages/login" class="btn-primary">Login</a>
             {{end}}
@@ -40,10 +44,12 @@
             {{if .User}}
             <a href="/sources" class="btn-text w-full text-left">Webhooks</a>
             <a href="/user/{{.User.Username}}" class="btn-text w-full text-left">Profile</a>
+            {{if .CSRFToken}}
             <form method="POST" action="/pages/logout">
                 <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
                 <button type="submit" class="btn-text w-full text-left">Logout</button>
             </form>
+            {{end}}
             {{else}}
             <a href="/pages/login" class="btn-primary w-full">Login</a>
             {{end}}

templates/profile.html

@@ -41,6 +41,10 @@
                     <dt class="w-32 text-sm font-medium text-gray-500">Username</dt>
                     <dd class="text-sm text-gray-900">{{.User.Username}}</dd>
                 </div>
+                <div class="flex">
+                    <dt class="w-32 text-sm font-medium text-gray-500">Account Type</dt>
+                    <dd class="text-sm text-gray-900">Standard User</dd>
+                </div>
             </dl>
         </div>
     </div>

templates/source_detail.html

@@ -181,30 +181,18 @@
     <!-- Recent Events -->
     <div class="card mt-6">
         <div class="p-4 border-b border-gray-200 flex justify-between items-center">
-            <h2 class="text-lg font-medium text-gray-900">50 Most Recent Events</h2>
+            <h2 class="text-lg font-medium text-gray-900">Recent Events</h2>
             <a href="/source/{{.Webhook.ID}}/logs" class="btn-text text-sm">View All</a>
         </div>
         <div class="divide-y divide-gray-100">
             {{range .Events}}
             <div class="p-4">
-                <div class="flex flex-wrap items-center justify-between gap-3">
-                    <div class="flex flex-wrap items-center gap-3">
+                <div class="flex items-center justify-between">
+                    <div class="flex items-center gap-3">
                         <span class="badge-info">{{.Method}}</span>
-                        <span class="text-sm text-gray-500 break-all">{{.ContentType}}</span>
-                        {{if .ResubmittedFromID}}
-                        <span class="text-xs text-gray-500" title="This event is a copy of {{.ResubmittedFromID}}">resubmitted copy</span>
-                        {{end}}
-                    </div>
-                    <div class="flex flex-wrap items-center gap-3 text-xs text-gray-400">
-                        <span title="Body size">{{.Size}}</span>
-                        {{if .ProcessingTime}}
-                        <span title="Processing time: how long the slowest delivery took, from being queued to its last attempt">{{.ProcessingTime}}</span>
-                        {{end}}
-                        {{if .Status}}
-                        <span class="font-medium {{.StatusClass}}" title="HTTP status from the HTTP target">{{.Status}}</span>
-                        {{end}}
-                        <span title="{{.ReceivedUTC}}">{{.Received}}</span>
+                        <span class="text-sm text-gray-500">{{.ContentType}}</span>
                     </div>
+                    <span class="text-xs text-gray-400">{{.CreatedAt.Format "2006-01-02 15:04:05 UTC"}}</span>
                 </div>
             </div>
             {{else}}