Load Alpine's CSP build so the UI's directives run (closes #371)

The pages' Content-Security-Policy forbids eval, which the standard
Alpine.js build needs, so no directive ran: add forms showed open and
events never collapsed. 3p/ now holds the @alpinejs/csp 3.14.9 tarball
instead, and every directive in templates/ names a property or method
of a component registered in static/js/app.js, as that build requires.
The policy is unchanged.

A browser test in internal/server, built only with the browser tag,
loads the webhook page and the event log under the real headers.
make test-browser runs it in Docker, in a digest-pinned headless
browser image (Dockerfile.browser).

Model: opus-5-5

.dockerignore

@@ -1,20 +1,14 @@
-# .git is sent so the build can derive the version it stamps into the binary
-# (script/version). Its config, which can hold a remote URL carrying a
-# credential and which `git describe` does not need, is left out of a
-# directory context. A context sent as a tar is not filtered by this file, so
-# it carries .git/config unless its sender leaves it out.
-.git/config
-
-# No tracked file may be listed here: git in the build would see it as
-# deleted and mark the version -dirty.
-#
 # .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

@@ -12,8 +12,9 @@ jobs:
       - name: Checkout
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
         with:
-          # The superseded-status step needs history to walk ancestors (it
-          # aborts on a shallow clone).
+          # The fingerprint step below needs history to find the last commit
+          # that touched the Docker build context, and the superseded-status
+          # step needs it to walk ancestors (it aborts on a shallow clone).
           fetch-depth: 0
 
       - name: Mark superseded run statuses
@@ -27,11 +28,16 @@ jobs:
         run: script/ci-mark-superseded
 
       - name: Fingerprint the build context
-        # Writes the hash of the commit being checked into the context, which
-        # invalidates the `COPY . .` layer of both check stages: a commit
-        # that was never linted, format-checked, tested and built cannot
-        # report success from cache.
-        run: git rev-parse HEAD > .ci-fingerprint
+        # `.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')"
+          printf '%s\n' "${fp:-$GITHUB_SHA}" > .ci-fingerprint
 
       - name: Build Docker image (runs make check)
         run: script/cibuild

Dockerfile

@@ -12,8 +12,8 @@ WORKDIR /src
 COPY go.mod go.sum ./
 RUN go mod download
 
-# Copy source code. In CI the context also carries .ci-fingerprint, which
-# holds the hash of the commit being checked (see
+# Copy source code. In CI the context also carries .ci-fingerprint, whose
+# value changes with every commit that touches the build context (see
 # .gitea/workflows/check.yml). That invalidates this layer, so the checks
 # below cannot report success by replaying a cached pass. Do not add it to
 # .dockerignore.
@@ -38,13 +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/*
-
-# A build context sent as a tar archive keeps its files' owners, and git
-# refuses to read a checkout owned by another user. Trust this one
-# whoever owns it.
-RUN git config --system --add safe.directory /build
+# 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
 
@@ -60,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 could not 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"
 

Dockerfile.browser

@@ -0,0 +1,29 @@
+# Browser test image, built by script/test-browser (make test-browser). It
+# runs the test in internal/server that loads the pages in a headless
+# browser under the real Content-Security-Policy. That test is built only
+# with the browser build tag, so make test leaves it out. Here the browser
+# comes from a digest-pinned image, and if it is missing the test fails.
+
+# golang:1.26.1-bookworm, 2026-03-17: the builder stage's image in Dockerfile.
+FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a3492282a6c820bf4755fd64a4 AS build
+
+WORKDIR /src
+
+COPY go.mod go.sum ./
+RUN go mod download
+
+COPY . .
+
+# The test binary embeds the templates and static files, so the browser
+# stage needs nothing else. -p 4 keeps the compile's memory down, as in
+# script/test.
+RUN make assets && go test -c -p 4 -tags browser -o /browser.test ./internal/server
+
+# chromedp/headless-shell:151.0.7922.109 (Debian trixie), 2026-08-11. The
+# browser is on PATH as headless-shell, where the test's browser library
+# looks for it.
+FROM chromedp/headless-shell:151.0.7922.109@sha256:2d349b544a1ea6b5b5fd7c0fe99215ff662339c57407ee2e8c0a11af93516b04 AS browser
+
+COPY --from=build /browser.test /browser.test
+
+RUN /browser.test -test.v -test.timeout 90s -test.run '^TestAlpineRunsUnderTheSecurityPolicy$'

Dockerfile.lint

@@ -34,4 +34,6 @@ COPY . .
 # `run` silently ignores config keys it does not recognize, so a typo would
 # disable a setting without a word. `config verify` is what catches that.
 RUN --network=none golangci-lint config verify --config .golangci.yml
-RUN --network=none golangci-lint run --config .golangci.yml ./...
+# --build-tags browser also lints the browser test, which is built only with
+# that tag (make test-browser).
+RUN --network=none golangci-lint run --config .golangci.yml --build-tags browser ./...

Makefile

@@ -1,15 +1,15 @@
-.PHONY: bootstrap setup assets test lint fmt fmt-check check build run dev deps docker clean hooks css version
+.PHONY: bootstrap setup assets test test-browser lint fmt fmt-check check build run dev deps docker clean hooks css version
 
 # Default target
 .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
@@ -33,6 +33,9 @@ assets:
 test:
 	@script/test
 
+test-browser:
+	@script/test-browser
+
 lint:
 	@script/lint
 

README.md

@@ -19,8 +19,8 @@ before deploying one.
 ### Prerequisites
 
 - Go 1.26.1+ (the version in `go.mod`)
-- Docker (for linting, for the test stage of the CI gate, and for
-  containerized deployment)
+- Docker (for linting, for the browser test, for the test stage of the
+  CI gate, and for containerized deployment)
 
 golangci-lint is not a prerequisite and must not be installed on the
 host: `script/bootstrap` does not install it, and `make lint` runs the
@@ -58,6 +58,7 @@ make fmt        # Format code (gofmt + goimports)
 make fmt-check  # Fail if gofmt would change anything (writes nothing)
 make lint       # Run golangci-lint in Docker (Dockerfile.lint)
 make test       # Run tests with race detection
+make test-browser # Run the browser test in Docker (Dockerfile.browser)
 make check      # test + lint + fmt-check (CI gate)
 make build      # Build binary to bin/webhooker (version-stamped)
 make version    # Print the version this checkout would stamp
@@ -1133,29 +1134,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, with no build
-arguments, 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`. It does
-leave `.git/config`, which can hold a remote URL carrying a credential
-and which `git describe` does not need, out of a directory context. A
-context sent as a tar is not filtered by `.dockerignore`, so it carries
-`.git/config` unless its sender leaves it out; for upaas, that is
-https://git.eeqj.de/sneak/upaas/issues/274. git in the build
-reads the checkout whoever owns its files, since a context sent as a tar
-archive keeps the sender's owners and git otherwise refuses a checkout
-owned by another user. 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 is missing from the build or 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
@@ -1240,7 +1225,7 @@ What that means for an operator:
 This repository adheres to the
 [Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all)
 standard: normalized scripts in `script/` are the entrypoints for the
-development workflow. Ten of the Makefile's seventeen targets are thin
+development workflow. Eleven of the Makefile's eighteen targets are thin
 shims that call them; `build`, `run`, `dev`, `deps`, `clean`, `css` and
 `version` are inline commands with no script behind them, though `build`,
 `run` and `dev` first run `script/assets`, and `build` and `version` both
@@ -1261,6 +1246,8 @@ We provide:
 - `script/assets` — extract Alpine.js from its tarball in `3p/` (see
   [Third-party browser assets](#third-party-browser-assets))
 - `script/test` — run the test suite
+- `script/test-browser` — run the browser test in Docker (see
+  [Third-party browser assets](#third-party-browser-assets))
 - `script/lint` — run golangci-lint in Docker (see Linting below)
 - `script/fmt` — format all code (writes)
 - `script/fmt-check` — check formatting (read-only)
@@ -1281,9 +1268,30 @@ We provide:
 
 ## Third-party browser assets
 
-The web UI serves one third-party script, Alpine.js. Its npm package tarball
-is committed as `3p/alpinejs-3.14.9.tgz`, byte for byte as the npm registry
-publishes it. It is a dependency, not this repo's build output, so
+The web UI serves one third-party script, Alpine.js, in its CSP build: the npm
+package `@alpinejs/csp`. The pages' Content-Security-Policy forbids eval, which
+the standard `alpinejs` build needs to run the expressions written in the
+markup. The CSP build runs no expressions, so every Alpine directive in
+`templates/` only names a property or method of a component registered in
+`static/js/app.js`: `x-data="collapsible"` and `@click="toggle"`, never
+`x-data="{ open: false }"` or `@click="open = !open"`.
+
+A browser test in `internal/server` loads the webhook page and the event log
+under the real policy and checks that: both add forms stay hidden until Add is
+clicked; choosing Slack in the add target form leaves the HTTP fields out of
+what it submits, also after leaving the page and going back to it, when the
+browser restores the choice; an event expands and collapses, and so do a
+delivery's attempts inside it; and at phone width the menu button opens and
+closes the mobile menu. It also fails if the browser reports a console warning or error,
+an uncaught exception, or anything the policy refused. It is not part of
+`make test`, `make check` or the image build (its file is built only with the
+`browser` build tag). Run it with `make test-browser` after changing
+`templates/` or `static/js/`: that builds `Dockerfile.browser`, which runs the
+test in a digest-pinned headless browser image, so the host needs no browser.
+
+The package's tarball is committed as `3p/alpinejs-csp-3.14.9.tgz`, byte for
+byte as the npm registry publishes it. It is a dependency, not this repo's
+build output, so
 `REPO_POLICIES.md`'s rule against committed build artifacts does not apply.
 The directory is `3p/` rather than `vendor/` because Go treats a root
 `vendor/` directory as its module vendor directory.
@@ -1296,10 +1304,11 @@ nothing downloads Alpine.js. The extracted file is not committed, and
 `.dockerignore` keeps any host copy out of the build context.
 
 To move to a new version: download
-`https://registry.npmjs.org/alpinejs/-/alpinejs-<version>.tgz`, check it
+`https://registry.npmjs.org/@alpinejs/csp/-/csp-<version>.tgz`, check it
 against the `dist.integrity` hash listed at
-`https://registry.npmjs.org/alpinejs/<version>`, replace the tarball in `3p/`
-with it, update its file name in `script/assets`, and run `make check`.
+`https://registry.npmjs.org/@alpinejs/csp/<version>`, replace the tarball in
+`3p/` with it as `alpinejs-csp-<version>.tgz`, update its file name in
+`script/assets`, and run `make check`.
 
 ## Rationale
 
@@ -2831,7 +2840,7 @@ imports. The entry point is `cmd/webhooker/main.go`.
 ```
 webhooker/
 ├── 3p/
-│   └── alpinejs-3.14.9.tgz    # Alpine.js npm package, extracted by make assets
+│   └── alpinejs-csp-3.14.9.tgz  # Alpine.js CSP build npm package, extracted by make assets
 ├── cmd/webhooker/
 │   └── main.go               # Entry point: subcommand dispatch; no args locks DATA_DIR and wires fx
 ├── internal/
@@ -2925,13 +2934,14 @@ webhooker/
 │   ├── css/input.css          # Tailwind input, source for tailwind.css (make css)
 │   ├── css/tailwind.css       # Generated stylesheet the pages load
 │   ├── css/style.css          # Older hand-written stylesheet, no longer loaded
-│   ├── js/app.js              # Progressive-enhancement copy-to-clipboard
-│   └── js/alpine.min.js       # Alpine.js, extracted from 3p/ by make assets, not committed
+│   ├── js/app.js              # Copy-to-clipboard, and the Alpine.js components
+│   └── js/alpine.min.js       # Alpine.js CSP build, extracted from 3p/ by make assets, not committed
 ├── templates/                 # Go HTML templates (base, login, sources, etc.)
 ├── script/                    # Scripts to Rule Them All entrypoints
 ├── Dockerfile                 # Three stages: lint, test+build, Alpine runtime
 ├── Dockerfile.lint            # Lint-only image built by script/lint
-├── Makefile                   # 10 of 17 targets shim script/; 7 are inline
+├── Dockerfile.browser         # Browser test image built by script/test-browser
+├── Makefile                   # 11 of 18 targets shim script/; 7 are inline
 ├── go.mod / go.sum
 └── .golangci.yml              # Linter configuration
 ```
@@ -2951,15 +2961,13 @@ Components are wired via Uber fx in this order:
 7. `healthcheck.New` — Health check service
 8. `session.New` — Cookie-based session manager (key from database)
 9. `handlers.New` — HTTP handlers
-10. `metrics.NewRegistry` — The registry `/metrics` serves
-11. `metrics.New` — The delivery collectors, registered on that registry
-12. `middleware.New` — HTTP middleware
-13. `delivery.New` — Event-driven delivery engine
-14. `delivery.NewArchiveSweeper` — Periodic pruning of idle archives
-15. `delivery.Engine` → `delivery.Notifier` — interface bridge
-16. `delivery.Engine` → `delivery.WebhookEvictor` — interface bridge so
+10. `middleware.New` — HTTP middleware
+11. `delivery.New` — Event-driven delivery engine
+12. `delivery.NewArchiveSweeper` — Periodic pruning of idle archives
+13. `delivery.Engine` → `delivery.Notifier` — interface bridge
+14. `delivery.Engine` → `delivery.WebhookEvictor` — interface bridge so
     deleting a webhook releases its archive writer
-17. `server.New` — HTTP server and router
+15. `server.New` — HTTP server and router
 
 The server starts via `fx.Invoke(func(*server.Server, *delivery.Engine,
 *database.RetentionReaper, *delivery.ArchiveSweeper) {})`, which
@@ -3002,12 +3010,6 @@ local record instead of nothing. What that placement gives up is
 recovery of a panic in the six entries above it, none of which does
 more than set a header or start a timer.
 
-Each admin page route group (`/pages`, `/user/*`, `/hooks`,
-`/hook/*`) starts with its own **Recoverer** and, if `SENTRY_DSN` is
-set, its own **Sentry** error reporting. That Recoverer answers a panic
-with the `500` error page in the normal layout; the global one keeps
-the plain-text `500` for every other route.
-
 Additionally, form endpoints (`/pages`, `/user/*`, `/hooks`,
 `/hook/*`) apply a **MaxBodySize** middleware that limits
 POST/PUT/PATCH request bodies to 1 MB. It is registered ahead of the
@@ -3251,9 +3253,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`
@@ -3285,13 +3286,19 @@ 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 commit being checked, so every commit, docs-only ones
-and a squash merge whose tree matches an already-built branch included,
-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.
+the hash of the last commit that touched the build context, so:
 
-The module download layer sits above `COPY . .` and stays cached.
+- 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 — `.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.
 
 A separate workflow step, run before the fingerprint is written, covers
 a second way the gate lied: Gitea cancels an in-flight run when a newer

TODO.md

@@ -40,6 +40,12 @@ duplicate. That is deliberate — the alternative is a silent lost
 delivery — and the README says so under Rationale. It is not a defect
 to re-file.
 
+One caveat on reading a green check: a docs-only commit deliberately
+replays from the layer cache
+(https://git.eeqj.de/sneak/webhooker/issues/119), so a green status on
+such a commit evidences a replay rather than an executed run. A code
+commit invalidates the `COPY` layer and genuinely executes.
+
 # Next Step
 
 Clear the rest of the open 1.0.0 milestone

cmd/webhooker/main.go

@@ -16,7 +16,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/handlers"
 	"sneak.berlin/go/webhooker/internal/healthcheck"
 	"sneak.berlin/go/webhooker/internal/logger"
-	"sneak.berlin/go/webhooker/internal/metrics"
 	"sneak.berlin/go/webhooker/internal/middleware"
 	"sneak.berlin/go/webhooker/internal/resetpw"
 	"sneak.berlin/go/webhooker/internal/server"
@@ -178,10 +177,6 @@ func newApp() *fx.App {
 			healthcheck.New,
 			session.New,
 			handlers.New,
-			// The registry /metrics serves, and the delivery
-			// collectors registered on it.
-			metrics.NewRegistry,
-			metrics.New,
 			middleware.New,
 			// The one SSRF guard both target-creation validation
 			// and the delivery dialer consult, so they cannot

go.mod

@@ -4,6 +4,8 @@ go 1.26.1
 
 require (
 	github.com/99designs/basicauth-go v0.0.0-20230316000542-bf6f9cbbf0f8
+	github.com/chromedp/cdproto v0.0.0-20260922220944-a19bff23514f
+	github.com/chromedp/chromedp v0.16.0
 	github.com/dustin/go-humanize v1.0.1
 	github.com/getsentry/sentry-go v0.25.0
 	github.com/go-chi/chi v1.5.5
@@ -29,7 +31,12 @@ require (
 require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
+	github.com/chromedp/sysutil v1.1.0 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
+	github.com/go-json-experiment/json v0.0.0-20260820222146-c27c302e5fc3 // indirect
+	github.com/gobwas/httphead v0.1.0 // indirect
+	github.com/gobwas/pool v0.2.1 // indirect
+	github.com/gobwas/ws v1.4.0 // 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
@@ -50,7 +57,7 @@ require (
 	go.uber.org/zap v1.23.0 // indirect
 	golang.org/x/mod v0.17.0 // indirect
 	golang.org/x/sync v0.14.0 // indirect
-	golang.org/x/sys v0.37.0 // indirect
+	golang.org/x/sys v0.47.0 // indirect
 	golang.org/x/text v0.25.0 // indirect
 	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	google.golang.org/protobuf v1.31.0 // indirect

go.sum

@@ -6,6 +6,12 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
 github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
 github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
+github.com/chromedp/cdproto v0.0.0-20260922220944-a19bff23514f h1:8PK9FM4bE0C8GMoWBW5lVsef3U7sPICjDg6JqngyYhk=
+github.com/chromedp/cdproto v0.0.0-20260922220944-a19bff23514f/go.mod h1:3v4FIp5njIUyPDvqXsxEOxnB34lijG0up98/5kM1KaE=
+github.com/chromedp/chromedp v0.16.0 h1:rOO4deOm4CbZgBCa8mD9g2rDyIoNs0BkgvNrlbp5ouk=
+github.com/chromedp/chromedp v0.16.0/go.mod h1:rbuGKFT1vMcFcFqKfPIO1GpX/N+2s8onm2qMxZLbU5U=
+github.com/chromedp/sysutil v1.1.0 h1:PUFNv5EcprjqXZD9nJb9b/c9ibAbxiYo4exNWZyipwM=
+github.com/chromedp/sysutil v1.1.0/go.mod h1:WiThHUdltqCNKGc4gaU50XgYjwjYIhKWoHGPTUfWTJ8=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -23,6 +29,14 @@ github.com/go-chi/httprate v0.15.0 h1:j54xcWV9KGmPf/X4H32/aTH+wBlrvxL7P+SdnRqxh5
 github.com/go-chi/httprate v0.15.0/go.mod h1:rzGHhVrsBn3IMLYDOZQsSU4fJNWcjui4fWKJcCId1R4=
 github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=
 github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og=
+github.com/go-json-experiment/json v0.0.0-20260820222146-c27c302e5fc3 h1:UADEEmDKgfXbtnGJZ97beY5XLo9ZechG1nlU4KnRrkE=
+github.com/go-json-experiment/json v0.0.0-20260820222146-c27c302e5fc3/go.mod h1:tphK2c80bpPhMOI4v6bIc2xWywPfbqi1Z06+RcrMkDg=
+github.com/gobwas/httphead v0.1.0 h1:exrUm0f4YX0L7EBwZHuCF4GDp8aJfVeBrlLQrs6NqWU=
+github.com/gobwas/httphead v0.1.0/go.mod h1:O/RXo79gxV8G+RqlR/otEwx4Q36zl9rqC5u12GKvMCM=
+github.com/gobwas/pool v0.2.1 h1:xfeeEhW7pwmX8nuLVlqbzVc7udMDrwetjEv+TZIz1og=
+github.com/gobwas/pool v0.2.1/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw=
+github.com/gobwas/ws v1.4.0 h1:CTaoG1tojrh4ucGPcoJFiAQUAsEWekEWvLy7GsVNqGs=
+github.com/gobwas/ws v1.4.0/go.mod h1:G3gNqMNtPppf5XUz7O4shetPpcZ1VJ7zt18dlUeakrc=
 github.com/gofrs/flock v0.13.0 h1:95JolYOvGMqeH31+FC7D2+uULf6mG61mEZ/A8dRYMzw=
 github.com/gofrs/flock v0.13.0/go.mod h1:jxeyy9R1auM5S6JYDBhDt+E2TCo7DkratH4Pgi8P+Z0=
 github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
@@ -55,12 +69,16 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
 github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
+github.com/ledongthuc/pdf v0.0.0-20220302134840-0c2507a12d80 h1:6Yzfa6GP0rIo/kULo2bwGEkFvCePZ3qHDDTC3/J9Swo=
+github.com/ledongthuc/pdf v0.0.0-20220302134840-0c2507a12d80/go.mod h1:imJHygn/1yfhB7XSJJKlFZKl/J+dCPAknuiaGOshXAs=
 github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
 github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
 github.com/mattn/go-sqlite3 v1.14.17 h1:mCRHCLDUBXgpKAqIKsaAaAsrAlbkeomtRFKXh2L6YIM=
 github.com/mattn/go-sqlite3 v1.14.17/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S2DGjv9HUNg=
 github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 h1:jWpvCLoY8Z/e3VKvlsiIGKtc+UG6U5vzxaoagmhXfyg=
 github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0/go.mod h1:QUyp042oQthUoa9bqDv0ER0wrtXnBruoNd7aNjkbP+k=
+github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde h1:x0TT0RDC7UhAVbbWWBzr41ElhJx5tXPWkIHA2HWPRuw=
+github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde/go.mod h1:nZgzbfBr3hhjoZnS66nKrHmduYNpc34ny7RK4z5/HM0=
 github.com/pingcap/errors v0.11.4 h1:lFuQV/oaUMGcD2tqt+01ROSmJs75VG1ToEOkZIZ4nE4=
 github.com/pingcap/errors v0.11.4/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
@@ -111,8 +129,8 @@ golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
 golang.org/x/sync v0.14.0 h1:woo0S4Yywslg6hp4eUFjTVOyKt0RookbpAHG4c1HmhQ=
 golang.org/x/sync v0.14.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.37.0 h1:fdNQudmxPjkdUTPnLn5mdQv7Zwvbvpaxqs831goi9kQ=
-golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
+golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs=
+golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
 golang.org/x/text v0.25.0 h1:qVyWApTSYLk/drJRO5mDlNYskwQznZmkpV2c8q9zls4=
 golang.org/x/text v0.25.0/go.mod h1:WEdwpYrmk1qmdHvhkSTNPm3app7v4rsT8F2UD6+VHIA=
 golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d h1:vU5i/LfpvrRCpgM/VPfJLg5KjxD3E+hfT1SH+d9zLwg=

internal/delivery/engine.go

@@ -14,7 +14,6 @@ import (
 	"go.uber.org/fx"
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/globals"
 	"sneak.berlin/go/webhooker/internal/lifecycle"
 	"sneak.berlin/go/webhooker/internal/logger"
 	"sneak.berlin/go/webhooker/internal/metrics"
@@ -147,10 +146,8 @@ type EngineParams struct {
 
 	DB        *database.Database
 	DBManager *database.WebhookDBManager
-	Globals   *globals.Globals
 	Logger    *logger.Logger
 	SSRFGuard *Guard
-	Metrics   *metrics.Set
 }
 
 // Engine processes queued deliveries in the background
@@ -170,14 +167,10 @@ type Engine struct {
 	retryCh    chan Task
 	workers    int
 
-	// version is the running build's version, the one the web UI
-	// footer shows. userAgent puts it on every outbound request.
-	version string
-
-	// mtr is the delivery metric set. Production wires the one
-	// registered on the registry /metrics serves; a test can
-	// substitute a set registered on a registry it holds, so it can
-	// gather what its own deliveries recorded.
+	// mtr is the delivery metric set. Production wires the
+	// process-wide one; a test can substitute a set registered on
+	// a private registry so its assertions are not disturbed by
+	// deliveries other tests are making at the same time.
 	mtr *metrics.Set
 
 	// targets maps each target type to its implementation.
@@ -211,8 +204,7 @@ func New(
 		deliveryCh: make(chan Task, deliveryChannelSize),
 		retryCh:    make(chan Task, retryChannelSize),
 		workers:    defaultWorkers,
-		version:    params.Globals.Version,
-		mtr:        params.Metrics,
+		mtr:        metrics.Default(),
 	}
 
 	e.initTargets(&http.Client{
@@ -308,13 +300,6 @@ func (e *Engine) ScheduleRetry(
 	})
 }
 
-// userAgent is the User-Agent header of every http and slack
-// delivery request: the program name and the running build's
-// version.
-func (e *Engine) userAgent() string {
-	return "webhooker/" + e.version
-}
-
 // registerHooks wires the engine's start and stop into the fx
 // lifecycle. The start hook's context is deliberately ignored
 // (see start for why the worker pool must not inherit it); the

internal/delivery/engine_test.go

@@ -1247,6 +1247,11 @@ func TestDoHTTPRequest_ForwardsHeaders(t *testing.T) {
 		testContentType,
 		receivedHeaders.Get("Content-Type"),
 	)
+
+	assert.Equal(t,
+		"webhooker/1.0",
+		receivedHeaders.Get("User-Agent"),
+	)
 }
 
 // The event's stored inbound headers carry the same Content-Type the
@@ -1315,7 +1320,6 @@ func TestApplyRequestHeaders_SendsOneContentType(t *testing.T) {
 					ContentType: tc.event,
 				},
 				cfg,
-				"webhooker/dev",
 			)
 
 			assert.Equal(t,

internal/delivery/export_test.go

@@ -9,7 +9,6 @@ import (
 	"net/url"
 	"time"
 
-	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx"
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/database"
@@ -41,6 +40,11 @@ const (
 	ExportPendingSweepMinAge = pendingSweepMinAge
 )
 
+// ExportIsBlockedIP exposes isBlockedIP for testing.
+func ExportIsBlockedIP(ip net.IP) bool {
+	return isBlockedIP(ip)
+}
+
 // NewTestGuard builds an SSRF Guard from an explicit egress
 // allowlist, without going through config. Passing no prefixes
 // yields the default guard, which blocks every private/reserved
@@ -66,11 +70,6 @@ func ExportBlockedNetworks() []*net.IPNet {
 	return blockedNetworks
 }
 
-// ExportBlockedPublicNetworks exposes blockedPublicNetworks.
-func ExportBlockedPublicNetworks() []*net.IPNet {
-	return blockedPublicNetworks
-}
-
 // ExportIsForwardableHeader exposes isForwardableHeader.
 func ExportIsForwardableHeader(name string) bool {
 	return isForwardableHeader(name)
@@ -83,9 +82,8 @@ func ExportApplyRequestHeaders(
 	req *http.Request,
 	event *database.Event,
 	cfg *HTTPTargetConfig,
-	userAgent string,
 ) []string {
-	return applyRequestHeaders(req, event, cfg, userAgent)
+	return applyRequestHeaders(req, event, cfg)
 }
 
 // ExportTruncate exposes truncate for testing.
@@ -401,7 +399,7 @@ func NewTestEngine(
 		deliveryCh: make(chan Task, deliveryChannelSize),
 		retryCh:    make(chan Task, retryChannelSize),
 		workers:    workers,
-		mtr:        metrics.New(prometheus.NewRegistry()),
+		mtr:        metrics.Default(),
 	}
 	e.initTargets(client)
 
@@ -416,7 +414,7 @@ func NewTestEngineSmallRetry(
 	e := &Engine{
 		log:     log,
 		retryCh: make(chan Task, 1),
-		mtr:     metrics.New(prometheus.NewRegistry()),
+		mtr:     metrics.Default(),
 	}
 	e.initTargets(nil)
 
@@ -439,7 +437,7 @@ func NewTestEngineWithDB(
 		deliveryCh: make(chan Task, deliveryChannelSize),
 		retryCh:    make(chan Task, retryChannelSize),
 		workers:    workers,
-		mtr:        metrics.New(prometheus.NewRegistry()),
+		mtr:        metrics.Default(),
 	}
 	e.initTargets(client)
 
@@ -447,7 +445,8 @@ func NewTestEngineWithDB(
 }
 
 // ExportSetMetrics substitutes the engine's metric set, so a test can
-// assert on collectors registered on a registry it holds.
+// assert on collectors registered on a private registry instead of
+// the process-wide ones every other test is also moving.
 func (e *Engine) ExportSetMetrics(mtr *metrics.Set) {
 	e.mtr = mtr
 }

internal/delivery/metrics_test.go

@@ -35,8 +35,9 @@ const (
 )
 
 // mIsolate gives the setup's engine a metric set registered on a
-// registry this test holds, so its exact assertions can gather from
-// it.
+// private registry. The process-wide collectors are moved by every
+// other delivery test running in parallel, so exact assertions are
+// only possible against a registry this test owns.
 func mIsolate(
 	t *testing.T, s iSetup,
 ) *prometheus.Registry {

internal/delivery/redirect_test.go

@@ -375,7 +375,6 @@ func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
 				"Content-Type":  testContentType,
 			},
 		},
-		"webhooker/dev",
 	)
 
 	assert.Equal(t,

internal/delivery/ssrf.go

@@ -25,16 +25,8 @@ var (
 	errNoIPs      = errors.New(
 		"hostname resolved to no IP addresses",
 	)
-	// ErrBlockedPrivateOrReservedIP reports an address in the
-	// default blocklist's private and reserved ranges,
-	// blockedNetworks.
-	ErrBlockedPrivateOrReservedIP = errors.New(
-		"blocked private or reserved address",
-	)
-	// errBlockedPublicMetadata reports a public address on the
-	// default blocklist, one in blockedPublicNetworks.
-	errBlockedPublicMetadata = errors.New(
-		"blocked cloud metadata address",
+	errBlockedIP = errors.New(
+		"blocked private, reserved or cloud metadata address",
 	)
 	errBlockedMetadata = errors.New(
 		"blocked link-local or cloud instance metadata " +
@@ -45,31 +37,21 @@ var (
 	)
 )
 
-// blockedNetworks and blockedPublicNetworks together are the
-// default blocklist: the private and reserved IP ranges, plus
-// the public cloud metadata addresses, that are blocked to
-// prevent SSRF attacks. An operator can permit specific blocks
-// out of this set with ALLOWED_EGRESS_CIDRS; see Guard.
-//
-// blockedNetworks holds the private and reserved IP ranges.
-//
-//nolint:gochecknoglobals // package-level network list is appropriate here
-var blockedNetworks []*net.IPNet
-
-// blockedPublicNetworks holds the default blocklist's public
-// addresses, kept apart from blockedNetworks so that they are
-// refused as cloud metadata addresses, never as private or
-// reserved ones.
+// blockedNetworks is the default blocklist: the private and
+// reserved IP ranges, plus the public cloud metadata addresses,
+// that are blocked to prevent SSRF attacks. An operator can
+// permit specific blocks out of this set with
+// ALLOWED_EGRESS_CIDRS; see Guard.
 //
 // A public address belongs on the default blocklist only if it
 // hands credentials, user data or bootstrap material to whatever
-// can reach it, without the caller presenting anything; it goes
-// in this list. A provider's other public addresses are not
-// refused, since reaching them can be legitimate and no list of
-// them could be complete.
+// can reach it, without the caller presenting anything. A
+// provider's other public addresses are not refused, since
+// reaching them can be legitimate and no list of them could be
+// complete.
 //
 //nolint:gochecknoglobals // package-level network list is appropriate here
-var blockedPublicNetworks []*net.IPNet
+var blockedNetworks []*net.IPNet
 
 // alwaysBlockedNetworks are the ranges no configuration can
 // open: the link-local blocks and the cloud instance metadata
@@ -106,8 +88,8 @@ var blockedPublicNetworks []*net.IPNet
 // when it clears both halves. Nothing in this list can be
 // reopened, so putting a public address here leaves the operator
 // no escape hatch at all — the condition ALLOWED_EGRESS_CIDRS
-// exists to remove. Default-block it in blockedPublicNetworks
-// instead, which an allowlist can override.
+// exists to remove. Default-block it in blockedNetworks instead,
+// which an allowlist can override.
 //
 // This is a criterion, not an enumeration of every metadata
 // address in existence.
@@ -148,9 +130,6 @@ func init() {
 		"::1/128",
 		"fc00::/7",
 		"fe80::/10",
-	})
-
-	blockedPublicNetworks = mustParseCIDRs([]string{
 		// Azure WireServer, a public address that serves VM credentials.
 		"168.63.129.16/32",
 	})
@@ -246,6 +225,13 @@ func matchesAny(networks []*net.IPNet, ip net.IP) bool {
 	return false
 }
 
+// isBlockedIP checks whether an IP address falls within
+// the default blocklist, before any operator allowlist is
+// considered.
+func isBlockedIP(ip net.IP) bool {
+	return matchesAny(blockedNetworks, ip)
+}
+
 // Guard makes every SSRF decision in the process.
 //
 // It holds the operator's ALLOWED_EGRESS_CIDRS allowlist and
@@ -346,8 +332,7 @@ func (g *Guard) allows(ip net.IP) bool {
 //     consulted, so no configured CIDR reaches link-local or a
 //     cloud metadata endpoint at a non-public address.
 //  2. The allowlist is consulted next, so a listed private
-//     network, or a listed public address on the default
-//     blocklist, becomes reachable.
+//     network becomes reachable.
 //  3. Everything else keeps the default blocklist's answer.
 func (g *Guard) checkIP(ip net.IP) error {
 	if matchesAny(alwaysBlockedNetworks, ip) {
@@ -360,15 +345,9 @@ func (g *Guard) checkIP(ip net.IP) error {
 		return nil
 	}
 
-	if matchesAny(blockedNetworks, ip) {
+	if isBlockedIP(ip) {
 		return fmt.Errorf(
-			"target IP %s: %w", ip, ErrBlockedPrivateOrReservedIP,
-		)
-	}
-
-	if matchesAny(blockedPublicNetworks, ip) {
-		return fmt.Errorf(
-			"target IP %s: %w", ip, errBlockedPublicMetadata,
+			"target IP %s: %w", ip, errBlockedIP,
 		)
 	}
 

internal/delivery/ssrf_allowlist_test.go

@@ -23,10 +23,6 @@ const (
 	metadataIP  = "169.254.169.254"
 	metadataURL = "http://" + metadataIP + "/latest/meta-data/"
 
-	// linkLocalIPv4 is the IPv4 link-local block, which holds
-	// metadataIP.
-	linkLocalIPv4 = "169.254.0.0/16"
-
 	// loopbackHookURL is a target on this host: blocked by
 	// default, reachable only once an operator allowlists
 	// loopback.
@@ -241,7 +237,7 @@ func linkLocalRefusedCases() []metadataAlwaysRefusedCase {
 		},
 		{
 			name:   "whole link-local block",
-			allow:  linkLocalIPv4,
+			allow:  "169.254.0.0/16",
 			target: metadataURL,
 		},
 		{
@@ -416,9 +412,6 @@ func TestGuardAllowlist_AzureWireServerReopenable(t *testing.T) {
 		"WireServer must be refused by the default blocklist, "+
 			"which an allowlist can override",
 	)
-	require.NotErrorIs(t, err, delivery.ErrBlockedPrivateOrReservedIP,
-		"WireServer is public, not private or reserved",
-	)
 
 	assertDialRefused(t, defaultGuard, target)
 
@@ -503,7 +496,7 @@ func TestAlwaysBlockedNetworks_PinnedSet(t *testing.T) {
 	want := []string{
 		// IPv4 link-local: the 169.254.169.254 metadata
 		// service on AWS, Azure and others.
-		linkLocalIPv4,
+		"169.254.0.0/16",
 		// IPv6 link-local.
 		"fe80::/10",
 		// AWS IPv6 IMDS, inside the ULA space an operator may
@@ -533,90 +526,6 @@ func TestAlwaysBlockedNetworks_PinnedSet(t *testing.T) {
 	assert.Equal(t, want, got)
 }
 
-// TestDefaultBlocklist_PinnedSet pins each list of the default
-// blocklist on its own, the private and reserved ranges in
-// blockedNetworks and the public addresses in
-// blockedPublicNetworks, so moving an entry from one list to the
-// other fails it. For the first address of each entry it then
-// checks that the default guard refuses it, and that listing the
-// entry in ALLOWED_EGRESS_CIDRS opens it unless the unconditional
-// set holds that address.
-func TestDefaultBlocklist_PinnedSet(t *testing.T) {
-	t.Parallel()
-
-	// public marks an entry of blockedPublicNetworks; every other
-	// entry belongs in blockedNetworks.
-	tests := []struct {
-		cidr       string
-		public     bool
-		reopenable bool
-	}{
-		{cidr: "127.0.0.0/8", reopenable: true},
-		{cidr: "10.0.0.0/8", reopenable: true},
-		{cidr: "172.16.0.0/12", reopenable: true},
-		{cidr: "192.168.0.0/16", reopenable: true},
-		{cidr: linkLocalIPv4, reopenable: false},
-		{cidr: "0.0.0.0/8", reopenable: true},
-		{cidr: "100.64.0.0/10", reopenable: true},
-		{cidr: "192.0.0.0/24", reopenable: true},
-		{cidr: "192.0.2.0/24", reopenable: true},
-		{cidr: "198.18.0.0/15", reopenable: true},
-		{cidr: "198.51.100.0/24", reopenable: true},
-		{cidr: "203.0.113.0/24", reopenable: true},
-		{cidr: "224.0.0.0/4", reopenable: true},
-		{cidr: "240.0.0.0/4", reopenable: true},
-		{cidr: "::1/128", reopenable: true},
-		{cidr: "fc00::/7", reopenable: true},
-		{cidr: "fe80::/10", reopenable: false},
-		{cidr: "168.63.129.16/32", public: true, reopenable: true},
-	}
-
-	wantPrivate := make([]string, 0, len(tests))
-	wantPublic := make([]string, 0, len(tests))
-
-	for _, tt := range tests {
-		if tt.public {
-			wantPublic = append(wantPublic, tt.cidr)
-		} else {
-			wantPrivate = append(wantPrivate, tt.cidr)
-		}
-	}
-
-	gotPrivate := make([]string, 0, len(tests))
-	for _, n := range delivery.ExportBlockedNetworks() {
-		gotPrivate = append(gotPrivate, n.String())
-	}
-
-	gotPublic := make([]string, 0, len(tests))
-	for _, n := range delivery.ExportBlockedPublicNetworks() {
-		gotPublic = append(gotPublic, n.String())
-	}
-
-	assert.ElementsMatch(t, wantPrivate, gotPrivate, "blockedNetworks")
-	assert.ElementsMatch(t, wantPublic, gotPublic, "blockedPublicNetworks")
-
-	for _, tt := range tests {
-		t.Run(tt.cidr, func(t *testing.T) {
-			t.Parallel()
-
-			prefix := netip.MustParsePrefix(tt.cidr)
-			ip := net.IP(prefix.Addr().AsSlice())
-
-			require.Error(t,
-				delivery.NewTestGuard().ExportCheckIP(ip),
-				"the default guard must refuse %s", ip,
-			)
-
-			err := delivery.NewTestGuard(prefix).ExportCheckIP(ip)
-			if tt.reopenable {
-				assert.NoError(t, err, "listing %s must open it", tt.cidr)
-			} else {
-				assert.Error(t, err, "listing %s must not open it", tt.cidr)
-			}
-		})
-	}
-}
-
 // requireLoopback fails the test unless rawURL's host is a
 // loopback address, so the allowlist test cannot silently stop
 // exercising a blocked range.

internal/delivery/ssrf_test.go

@@ -10,7 +10,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/delivery"
 )
 
-func TestGuardCheckIP_PrivateRanges(t *testing.T) {
+func TestIsBlockedIP_PrivateRanges(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
@@ -56,14 +56,12 @@ func TestGuardCheckIP_PrivateRanges(t *testing.T) {
 				"failed to parse IP %s", tt.ip,
 			)
 
-			refused := delivery.NewTestGuard().ExportCheckIP(ip) != nil
-
 			assert.Equal(t,
 				tt.blocked,
-				refused,
-				"default guard refuses %s = %v, want %v",
+				delivery.ExportIsBlockedIP(ip),
+				"isBlockedIP(%s) = %v, want %v",
 				tt.ip,
-				refused,
+				delivery.ExportIsBlockedIP(ip),
 				tt.blocked,
 			)
 		})

internal/delivery/target_http.go

@@ -442,9 +442,7 @@ func (t *httpTarget) doHTTPRequest(
 		)
 	}
 
-	originScoped := applyRequestHeaders(
-		req, event, cfg, t.eng.userAgent(),
-	)
+	originScoped := applyRequestHeaders(req, event, cfg)
 
 	client := t.clientForRequest(cfg, originScoped)
 
@@ -564,13 +562,10 @@ func isForwardableHeader(name string) bool {
 // Content-Type goes out once: a Content-Type configured on the target
 // wins, otherwise the event's ContentType, otherwise none. The inbound
 // Content-Type in the event's headers is never forwarded.
-//
-// userAgent is set last, over any configured or inbound User-Agent.
 func applyRequestHeaders(
 	req *http.Request,
 	event *database.Event,
 	cfg *HTTPTargetConfig,
-	userAgent string,
 ) []string {
 	if event.ContentType != "" {
 		req.Header.Set(
@@ -585,7 +580,7 @@ func applyRequestHeaders(
 		originScoped[http.CanonicalHeaderKey(k)] = struct{}{}
 	}
 
-	req.Header.Set("User-Agent", userAgent)
+	req.Header.Set("User-Agent", "webhooker/1.0")
 
 	// A Content-Type configured on the target describes the body
 	// being sent rather than the sender. A 307/308 preserves the

internal/delivery/target_slack.go

@@ -136,7 +136,7 @@ func (t *slackTarget) attempt(
 	}
 
 	req.Header.Set("Content-Type", "application/json")
-	req.Header.Set("User-Agent", t.eng.userAgent())
+	req.Header.Set("User-Agent", "webhooker/1.0")
 
 	resp, doErr := executeHTTPRequest(t.client, req)
 	durationMs := time.Since(start).Milliseconds()

internal/delivery/user_agent_test.go

@@ -1,91 +0,0 @@
-package delivery_test
-
-import (
-	"context"
-	"encoding/json"
-	"net/http"
-	"net/http/httptest"
-	"net/netip"
-	"testing"
-
-	"github.com/google/uuid"
-	"github.com/prometheus/client_golang/prometheus"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"go.uber.org/fx/fxtest"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/delivery"
-	"sneak.berlin/go/webhooker/internal/globals"
-	"sneak.berlin/go/webhooker/internal/logger"
-	"sneak.berlin/go/webhooker/internal/metrics"
-)
-
-// Both the http and the slack target send webhooker/ and the version
-// in Globals, the value the web UI footer shows. A User-Agent
-// configured on the target or carried in by the sender does not
-// replace it.
-func TestUserAgent_IsTheBuildVersion(t *testing.T) {
-	t.Parallel()
-
-	const want = "webhooker/1.2.3-test"
-
-	userAgents := make(chan string, 1)
-
-	ts := httptest.NewServer(http.HandlerFunc(
-		func(w http.ResponseWriter, r *http.Request) {
-			userAgents <- r.Header.Get("User-Agent")
-
-			w.WriteHeader(http.StatusOK)
-		},
-	))
-	defer ts.Close()
-
-	g := &globals.Globals{Version: "1.2.3-test"}
-	lc := fxtest.NewLifecycle(t)
-
-	log, err := logger.New(lc, logger.LoggerParams{Globals: g})
-	require.NoError(t, err)
-
-	e := delivery.New(lc, delivery.EngineParams{
-		Globals: g,
-		Logger:  log,
-		// httptest listens on loopback, which the default guard
-		// refuses.
-		SSRFGuard: delivery.NewTestGuard(
-			netip.MustParsePrefix("127.0.0.0/8"),
-		),
-		Metrics: metrics.New(prometheus.NewRegistry()),
-	})
-
-	statusCode, _, _, err := e.ExportDoHTTPRequest(
-		context.Background(),
-		&delivery.HTTPTargetConfig{
-			URL:     ts.URL,
-			Headers: map[string]string{"User-Agent": "configured/1"},
-		},
-		&database.Event{Headers: `{"User-Agent":["curl/8"]}`},
-	)
-	require.NoError(t, err)
-	require.Equal(t, http.StatusOK, statusCode)
-	require.Len(t, userAgents, 1, "the http target sent no request")
-	assert.Equal(t, want, <-userAgents, "http target")
-
-	db := testWebhookDB(t)
-	targetID := uuid.New().String()
-
-	slackCfg, err := json.Marshal(
-		delivery.SlackTargetConfig{WebhookURL: ts.URL},
-	)
-	require.NoError(t, err)
-
-	event := seedEvent(t, db, `{"action":"test"}`)
-	dlv := seedDelivery(
-		t, db, event.ID, targetID, database.DeliveryStatusPending,
-	)
-
-	e.ExportDeliverSlack(context.Background(), db, buildSlackDelivery(
-		dlv, event, targetID, "test-slack", string(slackCfg),
-	))
-	require.Len(t, userAgents, 1, "the slack target sent no request")
-	assert.Equal(t, want, <-userAgents, "slack target")
-}

internal/handlers/auth.go

@@ -74,7 +74,7 @@ func (h *Handlers) HandleLoginSubmit() http.HandlerFunc {
 		err := r.ParseForm()
 		if err != nil {
 			h.log.Error("failed to parse form", "error", err)
-			h.renderError(w, r, http.StatusBadRequest)
+			http.Error(w, "Bad request", http.StatusBadRequest)
 
 			return
 		}
@@ -212,7 +212,11 @@ func (h *Handlers) authenticateUser(
 
 	valid, err := database.VerifyPassword(password, user.Password)
 	if err != nil {
-		h.serverError(w, r, "failed to verify password", err)
+		h.log.Error("failed to verify password", "error", err)
+		http.Error(
+			w, "Internal server error",
+			http.StatusInternalServerError,
+		)
 
 		return user, err
 	}
@@ -284,14 +288,24 @@ func (h *Handlers) createAuthenticatedSession(
 ) error {
 	oldSess, err := h.session.Get(r)
 	if err != nil {
-		h.serverError(w, r, "failed to get session", err)
+		h.log.Error("failed to get session", "error", err)
+		http.Error(
+			w, "Internal server error",
+			http.StatusInternalServerError,
+		)
 
 		return err
 	}
 
 	sess, err := h.session.Regenerate(r, w, oldSess)
 	if err != nil {
-		h.serverError(w, r, "failed to regenerate session", err)
+		h.log.Error(
+			"failed to regenerate session", "error", err,
+		)
+		http.Error(
+			w, "Internal server error",
+			http.StatusInternalServerError,
+		)
 
 		return err
 	}
@@ -300,7 +314,11 @@ func (h *Handlers) createAuthenticatedSession(
 
 	err = h.session.Save(r, w, sess)
 	if err != nil {
-		h.serverError(w, r, "failed to save session", err)
+		h.log.Error("failed to save session", "error", err)
+		http.Error(
+			w, "Internal server error",
+			http.StatusInternalServerError,
+		)
 
 		return err
 	}
@@ -333,9 +351,7 @@ func (h *Handlers) HandleLogout() http.HandlerFunc {
 			)
 		}
 
-		http.Redirect(
-			w, r, withNotice("/pages/login", signedOut),
-			http.StatusSeeOther,
-		)
+		// Redirect to login page
+		http.Redirect(w, r, "/pages/login", http.StatusSeeOther)
 	}
 }

internal/handlers/delivery_replay.go

@@ -11,37 +11,72 @@ import (
 	"sneak.berlin/go/webhooker/internal/delivery"
 )
 
-// The outcomes of a replay POST, as the notice codes its redirect
-// carries. noticeFor holds the line each one shows.
+// replayOutcomeParam is the query parameter the replay POST redirects
+// with and the event log page reads its banner from.
+const replayOutcomeParam = "replay"
+
+// replayOutcomeCode is the outcome of a replay POST. The redirect
+// carries one of these fixed codes rather than a message, so nothing a
+// client submits can reach the rendered page through it.
+type replayOutcomeCode string
+
 const (
 	// replayQueued reports that a new delivery was created and handed
 	// to the delivery engine.
-	replayQueued noticeCode = "replay-queued"
+	replayQueued replayOutcomeCode = "queued"
 
 	// replayTargetDeleted reports a target that once existed and has
 	// since been deleted. Deletes are soft and deliveries carry no
 	// foreign key to the target row, so the history survives its
 	// target and this is the ordinary case for an old event.
-	replayTargetDeleted noticeCode = "replay-target-deleted"
+	replayTargetDeleted replayOutcomeCode = "target-deleted"
 
 	// replayTargetMissing reports a target id that names no row at
 	// all, deleted or otherwise.
-	replayTargetMissing noticeCode = "replay-target-missing"
+	replayTargetMissing replayOutcomeCode = "target-missing"
 
 	// replayTargetInactive reports a target the operator has
 	// deactivated. A deactivated target receives no new deliveries, so
 	// a replay to it would be a delivery they switched off.
-	replayTargetInactive noticeCode = "replay-target-inactive"
+	replayTargetInactive replayOutcomeCode = "target-inactive"
 
 	// replayNotTerminal reports a delivery the engine has not finished
 	// with.
-	replayNotTerminal noticeCode = "replay-not-terminal"
+	replayNotTerminal replayOutcomeCode = "not-terminal"
 
 	// replayInFlight reports that an earlier replay of this event to
 	// this target is still running.
-	replayInFlight noticeCode = "replay-in-flight"
+	replayInFlight replayOutcomeCode = "in-flight"
 )
 
+// replayOutcome returns the banner the event log page shows for an
+// outcome code, and whether the replay was queued. An unrecognised
+// code yields no banner.
+func replayOutcome(code string) (string, bool) {
+	switch replayOutcomeCode(code) {
+	case replayQueued:
+		return "Replay queued: a new delivery was created against " +
+			"the target's current configuration.", true
+	case replayTargetDeleted:
+		return "Not replayed: the target this delivery was for has " +
+			"been deleted. Recreate the target, then replay.", false
+	case replayTargetMissing:
+		return "Not replayed: the target this delivery was for no " +
+			"longer exists.", false
+	case replayTargetInactive:
+		return "Not replayed: the target this delivery was for is " +
+			"deactivated. Activate it, then replay.", false
+	case replayNotTerminal:
+		return "Not replayed: this delivery has not finished yet.",
+			false
+	case replayInFlight:
+		return "Not replayed: a delivery of this event to this " +
+			"target is already in flight.", false
+	default:
+		return "", false
+	}
+}
+
 // HandleDeliveryReplay re-sends a finished delivery's event to its
 // target.
 //
@@ -70,7 +105,9 @@ func (h *Handlers) HandleDeliveryReplay() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err := r.ParseForm()
 		if err != nil {
-			h.renderError(w, r, http.StatusBadRequest)
+			http.Error(
+				w, "Bad request", http.StatusBadRequest,
+			)
 
 			return
 		}
@@ -87,14 +124,14 @@ func (h *Handlers) replayDelivery(
 	webhook database.Webhook,
 ) {
 	if !h.dbMgr.DBExists(webhook.ID) {
-		h.renderError(w, r, http.StatusNotFound)
+		http.NotFound(w, r)
 
 		return
 	}
 
 	webhookDB, err := h.dbMgr.GetDB(webhook.ID)
 	if err != nil {
-		h.serverError(w, r, "failed to get webhook database", err)
+		h.serverError(w, "failed to get webhook database", err)
 
 		return
 	}
@@ -105,14 +142,14 @@ func (h *Handlers) replayDelivery(
 	}
 
 	if !original.Status.Terminal() {
-		redirectToEventLog(w, r, webhook, replayNotTerminal)
+		h.finishReplay(w, r, webhook, replayNotTerminal)
 
 		return
 	}
 
 	target, code := h.replayTarget(webhook.ID, original.TargetID)
 	if target == nil {
-		redirectToEventLog(w, r, webhook, code)
+		h.finishReplay(w, r, webhook, code)
 
 		return
 	}
@@ -136,7 +173,7 @@ func (h *Handlers) loadReplaySource(
 			&original, "id = ?", chi.URLParam(r, "deliveryID"),
 		).Error
 	if err != nil {
-		h.renderError(w, r, http.StatusNotFound)
+		http.NotFound(w, r)
 
 		return nil, false
 	}
@@ -158,14 +195,14 @@ func (h *Handlers) queueReplay(
 	)
 	if err != nil {
 		h.serverError(
-			w, r, "failed to count in-flight deliveries", err,
+			w, "failed to count in-flight deliveries", err,
 		)
 
 		return
 	}
 
 	if inFlight > 0 {
-		redirectToEventLog(w, r, webhook, replayInFlight)
+		h.finishReplay(w, r, webhook, replayInFlight)
 
 		return
 	}
@@ -175,7 +212,7 @@ func (h *Handlers) queueReplay(
 	err = webhookDB.
 		First(&event, "id = ?", original.EventID).Error
 	if err != nil {
-		h.serverError(w, r, "failed to load event for replay", err)
+		h.serverError(w, "failed to load event for replay", err)
 
 		return
 	}
@@ -185,7 +222,7 @@ func (h *Handlers) queueReplay(
 	)
 	if err != nil {
 		h.serverError(
-			w, r, "failed to create replay delivery", err,
+			w, "failed to create replay delivery", err,
 		)
 
 		return
@@ -203,7 +240,7 @@ func (h *Handlers) queueReplay(
 		"delivery_id", task.DeliveryID,
 	)
 
-	redirectToEventLog(w, r, webhook, replayQueued)
+	h.finishReplay(w, r, webhook, replayQueued)
 }
 
 // replayTarget loads the delivery's target as it stands now.
@@ -216,7 +253,7 @@ func (h *Handlers) queueReplay(
 // with the returned code saying why.
 func (h *Handlers) replayTarget(
 	webhookID, targetID string,
-) (*database.Target, noticeCode) {
+) (*database.Target, replayOutcomeCode) {
 	var target database.Target
 
 	err := h.db.DB().Unscoped().Where(
@@ -326,16 +363,17 @@ func replayBody(body string) *string {
 	return &body
 }
 
-// redirectToEventLog redirects a replay or resubmit back to the event
-// log it was triggered from, carrying the outcome as its notice and
-// the page number the form submitted.
-func redirectToEventLog(
+// finishReplay redirects back to the event log the replay was
+// triggered from, carrying the outcome code the page turns into a
+// banner and the page number the form submitted.
+func (h *Handlers) finishReplay(
 	w http.ResponseWriter,
 	r *http.Request,
 	webhook database.Webhook,
-	code noticeCode,
+	code replayOutcomeCode,
 ) {
-	dest := withNotice("/hook/"+webhook.ID+"/events", code)
+	dest := "/hook/" + webhook.ID + "/events?" +
+		replayOutcomeParam + "=" + string(code)
 
 	// The page is read from the form rather than the query string:
 	// this is a POST, and its query string is what logs and Referer

internal/handlers/delivery_replay_test.go

@@ -212,7 +212,7 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-queued",
+		"/hook/"+wh.ID+"/events?replay=queued",
 		w.Header().Get("Location"),
 	)
 
@@ -362,7 +362,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-target-deleted",
+		"/hook/"+wh.ID+"/events?replay=target-deleted",
 		w.Header().Get("Location"),
 	)
 
@@ -390,7 +390,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
 	require.Equal(t, http.StatusSeeOther, missing.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-target-missing",
+		"/hook/"+wh.ID+"/events?replay=target-missing",
 		missing.Header().Get("Location"),
 	)
 }
@@ -431,7 +431,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
 	require.Equal(t, http.StatusSeeOther, first.Code)
 	require.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-queued",
+		"/hook/"+wh.ID+"/events?replay=queued",
 		first.Header().Get("Location"),
 	)
 
@@ -439,7 +439,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
 	require.Equal(t, http.StatusSeeOther, second.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-in-flight",
+		"/hook/"+wh.ID+"/events?replay=in-flight",
 		second.Header().Get("Location"),
 	)
 
@@ -465,7 +465,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
 	require.Equal(t, http.StatusSeeOther, pending.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-not-terminal",
+		"/hook/"+wh.ID+"/events?replay=not-terminal",
 		pending.Header().Get("Location"),
 	)
 }
@@ -509,7 +509,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
 	assert.Contains(t, body, ">Replay<")
 
 	refused := renderSourceLogsPageWithQuery(
-		t, h, sess, wh.ID, "?notice=replay-target-deleted",
+		t, h, sess, wh.ID, "?replay=target-deleted",
 	)
 
 	assert.Contains(t, refused, "alert-error")
@@ -517,7 +517,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
 
 	// An outcome code nobody issued renders no banner at all.
 	unknown := renderSourceLogsPageWithQuery(
-		t, h, sess, wh.ID, "?notice=made-up",
+		t, h, sess, wh.ID, "?replay=made-up",
 	)
 
 	assert.NotContains(t, unknown, "alert-error")

internal/handlers/error_page_test.go

@@ -1,53 +0,0 @@
-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 {
-			h.renderError(w, r, http.StatusNotFound)
+			http.NotFound(w, r)
 
 			return
 		}
@@ -103,21 +103,21 @@ func (h *Handlers) serveEventBody(
 	eventID string,
 ) {
 	if !h.dbMgr.DBExists(webhook.ID) {
-		h.renderError(w, r, http.StatusNotFound)
+		http.NotFound(w, r)
 
 		return
 	}
 
 	webhookDB, err := h.dbMgr.GetDB(webhook.ID)
 	if err != nil {
-		h.serverError(w, r, "failed to get webhook database", err)
+		h.serverError(w, "failed to get webhook database", err)
 
 		return
 	}
 
 	body, found, err := eventBody(webhookDB, webhook.ID, eventID)
 	if err != nil {
-		h.serverError(w, r, "failed to read event body", err)
+		h.serverError(w, "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 {
-		h.renderError(w, r, http.StatusNotFound)
+		http.NotFound(w, r)
 
 		return
 	}

internal/handlers/event_resubmit.go

@@ -3,6 +3,7 @@ package handlers
 import (
 	"errors"
 	"net/http"
+	"strconv"
 
 	"github.com/go-chi/chi"
 	"github.com/google/uuid"
@@ -10,19 +11,43 @@ import (
 	"sneak.berlin/go/webhooker/internal/database"
 )
 
-// The outcomes of a resubmit POST, as the notice codes its redirect
-// carries. noticeFor holds the line each one shows.
+// resubmitOutcomeParam is the query parameter the resubmit POST
+// redirects with and the event log page reads its banner from.
+const resubmitOutcomeParam = "resubmit"
+
+// resubmitOutcomeCode is the outcome of a resubmit POST. The redirect
+// carries one of these fixed codes rather than a message, so nothing a
+// client submits can reach the rendered page through it.
+type resubmitOutcomeCode string
+
 const (
 	// resubmitQueued reports that a new event was stored and its
 	// deliveries handed to the delivery engine.
-	resubmitQueued noticeCode = "resubmit-queued"
+	resubmitQueued resubmitOutcomeCode = "queued"
 
 	// resubmitNoTargets reports a source with no active targets. The
 	// new event is stored either way, exactly as a received event
 	// with no targets is.
-	resubmitNoTargets noticeCode = "resubmit-no-targets"
+	resubmitNoTargets resubmitOutcomeCode = "no-targets"
 )
 
+// resubmitOutcome returns the banner the event log page shows for an
+// outcome code, and whether the resubmit was queued. An unrecognised
+// code yields no banner.
+func resubmitOutcome(code string) (string, bool) {
+	switch resubmitOutcomeCode(code) {
+	case resubmitQueued:
+		return "Resubmitted: a new event was created from the stored " +
+			"one and queued to every active target.", true
+	case resubmitNoTargets:
+		return "Resubmitted: a new event was created, but this " +
+				"source has no active targets, so nothing was queued.",
+			true
+	default:
+		return "", false
+	}
+}
+
 // resubmitSource is the stored event a resubmit copies. Its body is
 // read as bytes rather than as a string so the copy is byte-identical
 // to what was received, whatever the payload's encoding.
@@ -74,7 +99,7 @@ func (h *Handlers) HandleEventResubmit() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err := r.ParseForm()
 		if err != nil {
-			h.renderError(w, r, http.StatusBadRequest)
+			http.Error(w, "Bad request", http.StatusBadRequest)
 
 			return
 		}
@@ -95,20 +120,20 @@ func (h *Handlers) resubmitEvent(
 	// alphabet rather than from the request.
 	eventID, err := uuid.Parse(chi.URLParam(r, "eventID"))
 	if err != nil {
-		h.renderError(w, r, http.StatusNotFound)
+		http.NotFound(w, r)
 
 		return
 	}
 
 	if !h.dbMgr.DBExists(webhook.ID) {
-		h.renderError(w, r, http.StatusNotFound)
+		http.NotFound(w, r)
 
 		return
 	}
 
 	webhookDB, err := h.dbMgr.GetDB(webhook.ID)
 	if err != nil {
-		h.serverError(w, r, "failed to get webhook database", err)
+		h.serverError(w, "failed to get webhook database", err)
 
 		return
 	}
@@ -122,7 +147,7 @@ func (h *Handlers) resubmitEvent(
 		webhookDB, webhook.ID, eventID.String(),
 	)
 	if err != nil {
-		h.serverError(w, r, "failed to load event to resubmit", err)
+		h.serverError(w, "failed to load event to resubmit", err)
 
 		return
 	}
@@ -130,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 {
-		h.renderError(w, r, http.StatusNotFound)
+		http.NotFound(w, r)
 
 		return
 	}
@@ -182,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, r, "failed to query targets", err)
+		h.serverError(w, "failed to query targets", err)
 
 		return
 	}
@@ -200,7 +225,7 @@ func (h *Handlers) queueResubmit(
 		targets,
 	)
 	if err != nil {
-		h.serverError(w, r, "failed to store resubmitted event", err)
+		h.serverError(w, "failed to store resubmitted event", err)
 
 		return
 	}
@@ -220,5 +245,29 @@ func (h *Handlers) queueResubmit(
 		code = resubmitNoTargets
 	}
 
-	redirectToEventLog(w, r, webhook, code)
+	h.finishResubmit(w, r, webhook, code)
+}
+
+// finishResubmit redirects back to the event log the resubmit was
+// triggered from, carrying the outcome code the page turns into a
+// banner and the page number the form submitted.
+func (h *Handlers) finishResubmit(
+	w http.ResponseWriter,
+	r *http.Request,
+	webhook database.Webhook,
+	code resubmitOutcomeCode,
+) {
+	dest := "/hook/" + webhook.ID + "/events?" +
+		resubmitOutcomeParam + "=" + string(code)
+
+	// The page is read from the form rather than the query string:
+	// this is a POST, and its query string is what logs and Referer
+	// headers record.
+	if page := pageOrFirst(
+		r.PostFormValue("page"),
+	); page > 1 {
+		dest += "&page=" + strconv.Itoa(page)
+	}
+
+	http.Redirect(w, r, dest, http.StatusSeeOther)
 }

internal/handlers/event_resubmit_test.go

@@ -154,7 +154,7 @@ func TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=resubmit-queued",
+		"/hook/"+wh.ID+"/events?resubmit=queued",
 		w.Header().Get("Location"),
 	)
 
@@ -282,7 +282,7 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
 		require.Equal(t, http.StatusSeeOther, w.Code)
 		assert.Equal(
 			t,
-			"/hook/"+wh.ID+"/events?notice=resubmit-queued",
+			"/hook/"+wh.ID+"/events?resubmit=queued",
 			w.Header().Get("Location"),
 			"a resubmit must not be refused while an earlier "+
 				"one is in flight",
@@ -436,7 +436,7 @@ func TestHandleEventResubmit_SkipsInactiveTarget(t *testing.T) {
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=resubmit-queued",
+		"/hook/"+wh.ID+"/events?resubmit=queued",
 		w.Header().Get("Location"),
 		"an inactive target is skipped, not an error",
 	)
@@ -482,7 +482,7 @@ func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=resubmit-no-targets",
+		"/hook/"+wh.ID+"/events?resubmit=no-targets",
 		w.Header().Get("Location"),
 	)
 

internal/handlers/export_test.go

@@ -1,11 +1,9 @@
 package handlers
 
 import (
-	"context"
 	"html/template"
 	"log/slog"
 	"net/http"
-	"net/http/httptest"
 	"time"
 
 	"gorm.io/gorm"
@@ -67,7 +65,7 @@ func (s *Handlers) LoadEventLogViewsForTest(
 	page int,
 ) []EventLogView {
 	views, _, _ := s.loadEventsWithDeliveries(
-		w, newRequestForTest(), webhook, nil, page,
+		w, webhook, nil, page,
 	)
 
 	return views
@@ -96,14 +94,6 @@ func FinishedByTargetForTest(
 	return finishedByTarget(webhookDB, since)
 }
 
-// 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.
@@ -157,5 +147,5 @@ func (s *Handlers) BuildDatabaseTargetConfigForTest(
 	w http.ResponseWriter,
 	expiry string,
 ) (string, error) {
-	return s.buildDatabaseTargetConfig(w, newRequestForTest(), expiry)
+	return s.buildDatabaseTargetConfig(w, expiry)
 }

internal/handlers/handlers.go

@@ -12,7 +12,6 @@ import (
 	"net/http"
 	"sync/atomic"
 
-	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx"
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/delivery"
@@ -65,8 +64,6 @@ type HandlersParams struct {
 	Notifier     delivery.Notifier
 	Evictor      delivery.WebhookEvictor
 	SSRFGuard    *delivery.Guard
-	Metrics      *metrics.Set
-	Registry     *prometheus.Registry
 }
 
 // Handlers provides HTTP handler methods for all application
@@ -97,10 +94,10 @@ type Handlers struct {
 
 // parsePageTemplate parses a page-specific template set from the
 // embedded FS. Each page template is combined with the shared
-// base, htmlheader, navbar and notice templates, and with any further
-// files the page includes. The page file must be listed first so that
-// its root action ({{template "base" .}}) becomes the template set's
-// entry point.
+// base, htmlheader, and navbar templates, and with any further files
+// the page includes. The page file must be listed first so that its
+// root action ({{template "base" .}}) becomes the template set's entry
+// point.
 func parsePageTemplate(
 	pageFile string, included ...string,
 ) *template.Template {
@@ -109,7 +106,6 @@ func parsePageTemplate(
 		"base.html",
 		"htmlheader.html",
 		"navbar.html",
-		"notice.html",
 	}, included...)
 
 	return template.Must(
@@ -133,7 +129,7 @@ func New(
 	s.mw = params.Middleware
 	s.notifier = params.Notifier
 	s.evictor = params.Evictor
-	s.mtr = params.Metrics
+	s.mtr = metrics.Default()
 	s.ssrf = params.SSRFGuard
 
 	// Parse all page templates once at startup
@@ -146,7 +142,6 @@ 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{
@@ -158,16 +153,6 @@ 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, the
-// CSRF middleware for a refused form, and each admin page route
-// group's recoverer for a panic.
-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,
@@ -185,78 +170,15 @@ func (s *Handlers) respondJSON(
 	}
 }
 
-// serverError logs an error and answers with the 500 error page.
+// serverError logs an error and sends a 500 response.
 func (s *Handlers) serverError(
-	w http.ResponseWriter, r *http.Request, msg string, err error,
+	w http.ResponseWriter, msg string, err error,
 ) {
 	s.log.Error(msg, "error", err)
-	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,
-) {
-	// The page names the signed-in user, and some error pages are
-	// served outside the routes where NoCache runs.
-	w.Header().Set("Cache-Control", "no-store")
-
-	// No notice: one would say an action worked above a page saying
-	// the request failed.
-	data := s.pageData(r, map[string]any{
-		"Status":     status,
-		"StatusText": http.StatusText(status),
-		"Message":    errorPageText(status),
-	}, nil)
-
-	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."
-	}
+	http.Error(
+		w, "Internal server error",
+		http.StatusInternalServerError,
+	)
 }
 
 // UserInfo represents user information for templates
@@ -270,7 +192,6 @@ type templateDataWrapper struct {
 	User      *UserInfo
 	CSRFToken string
 	Version   string
-	Notice    *notice
 	Data      any
 }
 
@@ -310,20 +231,14 @@ func (s *Handlers) renderTemplate(
 			"template not found",
 			"template", pageTemplate,
 		)
-		s.renderError(w, r, http.StatusInternalServerError)
+		http.Error(
+			w, "Internal server error",
+			http.StatusInternalServerError,
+		)
 
 		return
 	}
 
-	s.executeTemplate(w, r, tmpl, s.pageData(r, data, noticeFor(r)))
-}
-
-// pageData adds the fields the shared layout renders to a page's own
-// data. The layout shows the notice, when there is one, above the
-// page.
-func (s *Handlers) pageData(
-	r *http.Request, data any, pageNotice *notice,
-) any {
 	userInfo := s.getUserInfo(r)
 	csrfToken := middleware.CSRFToken(r)
 
@@ -337,18 +252,19 @@ func (s *Handlers) pageData(
 		m["User"] = userInfo
 		m["CSRFToken"] = csrfToken
 		m["Version"] = version
-		m["Notice"] = pageNotice
+		s.executeTemplate(w, tmpl, m)
 
-		return m
+		return
 	}
 
-	return templateDataWrapper{
+	wrapper := templateDataWrapper{
 		User:      userInfo,
 		CSRFToken: csrfToken,
 		Version:   version,
-		Notice:    pageNotice,
 		Data:      data,
 	}
+
+	s.executeTemplate(w, tmpl, wrapper)
 }
 
 // executeTemplate renders the template into a buffer and writes to
@@ -361,7 +277,6 @@ func (s *Handlers) pageData(
 // this reason.
 func (s *Handlers) executeTemplate(
 	w http.ResponseWriter,
-	r *http.Request,
 	tmpl *template.Template,
 	data any,
 ) {
@@ -372,7 +287,10 @@ func (s *Handlers) executeTemplate(
 		s.log.Error(
 			"failed to execute template", "error", err,
 		)
-		s.renderError(w, r, http.StatusInternalServerError)
+		http.Error(
+			w, "Internal server error",
+			http.StatusInternalServerError,
+		)
 
 		return
 	}

internal/handlers/handlers_test.go

@@ -20,7 +20,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/handlers"
 	"sneak.berlin/go/webhooker/internal/healthcheck"
 	"sneak.berlin/go/webhooker/internal/logger"
-	"sneak.berlin/go/webhooker/internal/metrics"
 	"sneak.berlin/go/webhooker/internal/middleware"
 	"sneak.berlin/go/webhooker/internal/session"
 )
@@ -110,8 +109,6 @@ func newTestApp(
 			func(r *recordingEvictor) delivery.WebhookEvictor {
 				return r
 			},
-			metrics.NewRegistry,
-			metrics.New,
 			middleware.New,
 			delivery.NewGuard,
 			handlers.New,
@@ -310,14 +307,10 @@ func TestRenderTemplateMidRenderErrorSendsNoPartialBody(t *testing.T) {
 		t, http.StatusInternalServerError, w.Code,
 		"a failed render must report a 500",
 	)
-	assert.NotContains(
-		t, w.Body.String(), partialPageMarker,
+	assert.Equal(
+		t, "Internal server error\n", w.Body.String(),
 		"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/metrics.go

@@ -1,21 +0,0 @@
-package handlers
-
-import (
-	"net/http"
-
-	"github.com/prometheus/client_golang/prometheus/promhttp"
-)
-
-// HandleMetrics returns the Prometheus scrape handler for the
-// registry built by metrics.NewRegistry, which the HTTP, delivery, Go
-// runtime and process collectors register on. It is what
-// promhttp.Handler builds for the global default registry, including
-// the promhttp_metric_handler_* series that count scrapes, pointed at
-// that registry instead.
-func (s *Handlers) HandleMetrics() http.HandlerFunc {
-	reg := s.params.Registry
-
-	return promhttp.InstrumentMetricHandler(
-		reg, promhttp.HandlerFor(reg, promhttp.HandlerOpts{}),
-	).ServeHTTP
-}

internal/handlers/notice.go

@@ -1,109 +0,0 @@
-package handlers
-
-import "net/http"
-
-// noticeParam is the query parameter an action's redirect carries its
-// notice code in.
-const noticeParam = "notice"
-
-// noticeCode names one of the fixed lines noticeFor knows. An action
-// redirects with the code rather than the line, so nothing a client
-// puts in the URL reaches the page: a code noticeFor does not know
-// shows nothing.
-type noticeCode string
-
-// The codes of the actions on the webhook pages and of signing out.
-// Replay's codes, with the reasons a replay can be refused, and
-// resubmit's codes are defined beside those actions.
-const (
-	webhookCreated        noticeCode = "webhook-created"
-	webhookSaved          noticeCode = "webhook-saved"
-	webhookDeleted        noticeCode = "webhook-deleted"
-	entrypointAdded       noticeCode = "entrypoint-added"
-	entrypointDeleted     noticeCode = "entrypoint-deleted"
-	entrypointActivated   noticeCode = "entrypoint-activated"
-	entrypointDeactivated noticeCode = "entrypoint-deactivated"
-	targetAdded           noticeCode = "target-added"
-	targetSaved           noticeCode = "target-saved"
-	targetDeleted         noticeCode = "target-deleted"
-	targetActivated       noticeCode = "target-activated"
-	targetDeactivated     noticeCode = "target-deactivated"
-	signedOut             noticeCode = "signed-out"
-)
-
-// notice is the line templates/notice.html shows above a page to say
-// what an action did.
-type notice struct {
-	Text string
-
-	// Failed shows the line as an error: the action was refused.
-	Failed bool
-}
-
-// noticeFor returns the notice the request's URL names, or nil when it
-// names none or an unknown code.
-func noticeFor(r *http.Request) *notice {
-	n, ok := map[noticeCode]notice{
-		webhookCreated:        {Text: "Webhook created."},
-		webhookSaved:          {Text: "Webhook saved."},
-		webhookDeleted:        {Text: "Webhook deleted."},
-		entrypointAdded:       {Text: "Entrypoint added."},
-		entrypointDeleted:     {Text: "Entrypoint deleted."},
-		entrypointActivated:   {Text: "Entrypoint activated."},
-		entrypointDeactivated: {Text: "Entrypoint deactivated."},
-		targetAdded:           {Text: "Target added."},
-		targetSaved:           {Text: "Target saved."},
-		targetDeleted:         {Text: "Target deleted."},
-		targetActivated:       {Text: "Target activated."},
-		targetDeactivated:     {Text: "Target deactivated."},
-		signedOut:             {Text: "Signed out."},
-
-		replayQueued: {
-			Text: "Replay queued: a new delivery was created " +
-				"against the target's current configuration.",
-		},
-		replayTargetDeleted: {
-			Text: "Not replayed: the target this delivery was for " +
-				"has been deleted. Recreate the target, then replay.",
-			Failed: true,
-		},
-		replayTargetMissing: {
-			Text: "Not replayed: the target this delivery was for " +
-				"no longer exists.",
-			Failed: true,
-		},
-		replayTargetInactive: {
-			Text: "Not replayed: the target this delivery was for " +
-				"is deactivated. Activate it, then replay.",
-			Failed: true,
-		},
-		replayNotTerminal: {
-			Text:   "Not replayed: this delivery has not finished yet.",
-			Failed: true,
-		},
-		replayInFlight: {
-			Text: "Not replayed: a delivery of this event to this " +
-				"target is already in flight.",
-			Failed: true,
-		},
-
-		resubmitQueued: {
-			Text: "Resubmitted: a new event was created from the " +
-				"stored one and queued to every active target.",
-		},
-		resubmitNoTargets: {
-			Text: "Resubmitted: a new event was created, but this " +
-				"source has no active targets, so nothing was queued.",
-		},
-	}[noticeCode(r.URL.Query().Get(noticeParam))]
-	if !ok {
-		return nil
-	}
-
-	return &n
-}
-
-// withNotice returns path with code added as its notice.
-func withNotice(path string, code noticeCode) string {
-	return path + "?" + noticeParam + "=" + string(code)
-}

internal/handlers/profile.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"context"
 	"net/http"
 
 	"github.com/go-chi/chi"
@@ -36,14 +37,14 @@ func (h *Handlers) HandlePasswordChange() http.HandlerFunc {
 		err := r.ParseForm()
 		if err != nil {
 			h.log.Error("failed to parse form", "error", err)
-			h.renderError(w, r, http.StatusBadRequest)
+			http.Error(w, "Bad request", 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.
@@ -65,12 +66,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 the
-// error page itself and returns handled=false, signalling the caller
+// to display on the profile page. On an internal failure it writes a
+// 500 response 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
@@ -78,10 +79,15 @@ 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(r.Context())
+	release, ok := h.mw.BeginPasswordVerification(ctx)
 	if !ok {
 		h.log.Warn("password verification capacity exhausted")
-		h.renderError(w, r, http.StatusServiceUnavailable)
+		http.Error(
+			w,
+			"The server is busy verifying credentials. "+
+				"Please try again.",
+			http.StatusServiceUnavailable,
+		)
 
 		return "", "", false
 	}
@@ -97,7 +103,7 @@ func (h *Handlers) applyPasswordChange(
 	).First(&user).Error
 	if err != nil {
 		h.serverError(
-			w, r, "failed to load user for password change", err,
+			w, "failed to load user for password change", err,
 		)
 
 		return "", "", false
@@ -107,7 +113,7 @@ func (h *Handlers) applyPasswordChange(
 		currentPassword, user.Password,
 	)
 	if err != nil {
-		h.serverError(w, r, "failed to verify password", err)
+		h.serverError(w, "failed to verify password", err)
 
 		return "", "", false
 	}
@@ -126,7 +132,7 @@ func (h *Handlers) applyPasswordChange(
 
 	hashedPassword, err := database.HashPassword(newPassword)
 	if err != nil {
-		h.serverError(w, r, "failed to hash new password", err)
+		h.serverError(w, "failed to hash new password", err)
 
 		return "", "", false
 	}
@@ -135,7 +141,7 @@ func (h *Handlers) applyPasswordChange(
 		"password", hashedPassword,
 	).Error
 	if err != nil {
-		h.serverError(w, r, "failed to update password", err)
+		h.serverError(w, "failed to update password", err)
 
 		return "", "", false
 	}
@@ -156,7 +162,7 @@ func (h *Handlers) profileOwnerOrDeny(
 ) (string, string, bool) {
 	requestedUsername := chi.URLParam(r, "username")
 	if requestedUsername == "" {
-		h.renderError(w, r, http.StatusNotFound)
+		http.NotFound(w, r)
 
 		return "", "", false
 	}
@@ -166,7 +172,7 @@ func (h *Handlers) profileOwnerOrDeny(
 	// unexpected retrieval error.
 	sess, err := h.session.Get(r)
 	if err != nil {
-		h.serverError(w, r, "failed to get session", err)
+		h.serverError(w, "failed to get session", err)
 
 		return "", "", false
 	}
@@ -174,7 +180,10 @@ func (h *Handlers) profileOwnerOrDeny(
 	sessionUsername, ok := h.session.GetUsername(sess)
 	if !ok {
 		h.log.Error("authenticated session missing username")
-		h.renderError(w, r, http.StatusInternalServerError)
+		http.Error(
+			w, "Internal server error",
+			http.StatusInternalServerError,
+		)
 
 		return "", "", false
 	}
@@ -182,14 +191,17 @@ func (h *Handlers) profileOwnerOrDeny(
 	sessionUserID, ok := h.session.GetUserID(sess)
 	if !ok {
 		h.log.Error("authenticated session missing user ID")
-		h.renderError(w, r, http.StatusInternalServerError)
+		http.Error(
+			w, "Internal server error",
+			http.StatusInternalServerError,
+		)
 
 		return "", "", false
 	}
 
 	// Only allow users to act on their own profile.
 	if requestedUsername != sessionUsername {
-		h.renderError(w, r, http.StatusForbidden)
+		http.Error(w, "Forbidden", http.StatusForbidden)
 
 		return "", "", false
 	}

internal/handlers/profile_test.go

@@ -128,9 +128,7 @@ func TestUserRoute_Unauthenticated_RedirectedByMiddleware(t *testing.T) {
 
 	var sess *session.Session
 
-	var h *handlers.Handlers
-
-	app := newTestApp(t, &log, &cfg, &sess, &h)
+	app := newTestApp(t, &log, &cfg, &sess)
 	app.RequireStart()
 
 	t.Cleanup(app.RequireStop)
@@ -141,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(h.HandleErrorPage(http.StatusForbidden)))
+		r.Use(mw.CSRF())
 		r.Use(mw.RequireAuth())
 		r.Get("/", func(w http.ResponseWriter, _ *http.Request) {
 			handlerReached = true

internal/handlers/source_delete_test.go

@@ -411,9 +411,7 @@ func TestHandleSourceDelete_RemovesConfigAndEventDatabase(
 	h.HandleSourceDelete().ServeHTTP(w, req)
 
 	require.Equal(t, http.StatusSeeOther, w.Code)
-	assert.Equal(
-		t, "/hooks?notice=webhook-deleted", w.Header().Get("Location"),
-	)
+	assert.Equal(t, "/hooks", w.Header().Get("Location"))
 
 	assert.Equal(
 		t, int64(0),

internal/handlers/source_detail_test.go

@@ -241,37 +241,3 @@ func TestHandleSourceDetail_RendersNamedTargetFields(
 	assert.Contains(t, body, "(unavailable)")
 	assert.NotContains(t, body, "beak")
 }
-
-// TestHandleSourceDetail_FitsWideAndNarrowWindows pins the webhook
-// page's maximum width at 108rem (1728 px), half again the 72rem of
-// max-w-6xl that the webhook list and the event log use, so an
-// entrypoint URL fits on one line in a 1920-pixel window; and the
-// wrapping of its title row, so the buttons beside the title do not
-// push a phone-width window into scrolling sideways.
-func TestHandleSourceDetail_FitsWideAndNarrowWindows(t *testing.T) {
-	t.Parallel()
-
-	var (
-		h    *handlers.Handlers
-		sess *session.Session
-		db   *database.Database
-	)
-
-	app := newTestApp(t, &h, &sess, &db)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	wh := seedWebhook(t, db)
-
-	body := renderSourceDetailPage(t, h, sess, wh.ID)
-
-	assert.Contains(
-		t, body,
-		`<div class="mx-auto px-6 py-8" style="max-width: 108rem"`,
-	)
-	assert.Contains(
-		t, body,
-		`<div class="flex flex-wrap justify-between items-center gap-2 mt-2">`,
-	)
-}

internal/handlers/source_management.go

@@ -149,7 +149,13 @@ func (h *Handlers) HandleSourceList() http.HandlerFunc {
 			"user_id = ?", userID,
 		).Order("created_at DESC").Find(&webhooks).Error
 		if err != nil {
-			h.serverError(w, r, "failed to list webhooks", err)
+			h.log.Error(
+				"failed to list webhooks", "error", err,
+			)
+			http.Error(
+				w, "Internal server error",
+				http.StatusInternalServerError,
+			)
 
 			return
 		}
@@ -243,7 +249,9 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err := r.ParseForm()
 		if err != nil {
-			h.renderError(w, r, http.StatusBadRequest)
+			http.Error(
+				w, "Bad request", http.StatusBadRequest,
+			)
 
 			return
 		}
@@ -303,7 +311,7 @@ func (h *Handlers) createWebhookWithEntrypoint(
 
 	err := h.commitWebhook(webhook)
 	if err != nil {
-		h.serverError(w, r, "failed to create webhook", err)
+		h.serverError(w, "failed to create webhook", err)
 
 		return
 	}
@@ -322,8 +330,7 @@ func (h *Handlers) createWebhookWithEntrypoint(
 	)
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, webhookCreated),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 
@@ -381,7 +388,7 @@ func (h *Handlers) HandleSourceDetail() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			h.renderError(w, r, http.StatusNotFound)
+			http.NotFound(w, r)
 
 			return
 		}
@@ -413,7 +420,7 @@ func (h *Handlers) renderSourceDetail(
 	if h.dbMgr.DBExists(webhook.ID) {
 		webhookDB, err := h.dbMgr.GetDB(webhook.ID)
 		if err != nil {
-			h.serverError(w, r, "failed to get webhook database", err)
+			h.serverError(w, "failed to get webhook database", err)
 
 			return
 		}
@@ -422,7 +429,7 @@ func (h *Handlers) renderSourceDetail(
 			webhookDB, webhook.ID, singleHTTPTargetID(targets),
 		)
 		if err != nil {
-			h.serverError(w, r, "failed to load recent events", err)
+			h.serverError(w, "failed to load recent events", err)
 
 			return
 		}
@@ -476,7 +483,7 @@ func (h *Handlers) HandleSourceEdit() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			h.renderError(w, r, http.StatusNotFound)
+			http.NotFound(w, r)
 
 			return
 		}
@@ -511,7 +518,7 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			h.renderError(w, r, http.StatusNotFound)
+			http.NotFound(w, r)
 
 			return
 		}
@@ -520,7 +527,9 @@ func (h *Handlers) HandleSourceEditSubmit() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err = r.ParseForm()
 		if err != nil {
-			h.renderError(w, r, http.StatusBadRequest)
+			http.Error(
+				w, "Bad request", http.StatusBadRequest,
+			)
 
 			return
 		}
@@ -574,14 +583,13 @@ func (h *Handlers) applyWebhookEdit(
 
 	err := h.db.DB().Save(webhook).Error
 	if err != nil {
-		h.serverError(w, r, "failed to update webhook", err)
+		h.serverError(w, "failed to update webhook", err)
 
 		return
 	}
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, webhookSaved),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 
@@ -605,7 +613,7 @@ func (h *Handlers) HandleSourceDelete() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			h.renderError(w, r, http.StatusNotFound)
+			http.NotFound(w, r)
 
 			return
 		}
@@ -632,7 +640,7 @@ func (h *Handlers) deleteWebhookResources(
 	// be removed by hand; deleted history cannot be recovered.
 	err := h.commitWebhookDeletion(&webhook)
 	if err != nil {
-		h.serverError(w, r, "failed to delete webhook", err)
+		h.serverError(w, "failed to delete webhook", err)
 
 		return
 	}
@@ -658,15 +666,13 @@ func (h *Handlers) deleteWebhookResources(
 		// redirecting as though everything succeeded: the file
 		// needs removing by hand, and the logged error names it.
 		h.serverError(
-			w, r, "failed to delete webhook event database", err,
+			w, "failed to delete webhook event database", err,
 		)
 
 		return
 	}
 
-	http.Redirect(
-		w, r, withNotice("/hooks", webhookDeleted), http.StatusSeeOther,
-	)
+	http.Redirect(w, r, "/hooks", http.StatusSeeOther)
 }
 
 // commitWebhookDeletion soft-deletes a webhook's entrypoints,
@@ -804,7 +810,7 @@ func (h *Handlers) ownedWebhook(
 		"id = ? AND user_id = ?", sourceID, userID,
 	).First(&webhook).Error
 	if err != nil {
-		h.renderError(w, r, http.StatusNotFound)
+		http.NotFound(w, r)
 
 		return database.Webhook{}, false
 	}
@@ -826,7 +832,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, r, "failed to load targets", err)
+			h.serverError(w, "failed to load targets", err)
 
 			return
 		}
@@ -834,7 +840,7 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 		page := h.parsePage(r)
 
 		evts, total, ok := h.loadEventsWithDeliveries(
-			w, r, webhook, targets, page,
+			w, webhook, targets, page,
 		)
 		if !ok {
 			return
@@ -845,16 +851,31 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 			totalPages++
 		}
 
+		// The banner a replay or resubmit POST redirected back
+		// with. The message comes from a fixed set keyed by the
+		// outcome code, never from the query string itself.
+		replayMsg, replayOK := replayOutcome(
+			r.URL.Query().Get(replayOutcomeParam),
+		)
+
+		resubmitMsg, resubmitOK := resubmitOutcome(
+			r.URL.Query().Get(resubmitOutcomeParam),
+		)
+
 		data := map[string]any{
-			tmplKeyWebhook: &webhook,
-			"Events":       evts,
-			"Page":         page,
-			"TotalPages":   totalPages,
-			"TotalEvents":  total,
-			"HasPrev":      page > 1,
-			"HasNext":      page < totalPages,
-			"PrevPage":     page - 1,
-			"NextPage":     page + 1,
+			tmplKeyWebhook:    &webhook,
+			"Events":          evts,
+			"ReplayMessage":   replayMsg,
+			"ReplayQueued":    replayOK,
+			"ResubmitMessage": resubmitMsg,
+			"ResubmitQueued":  resubmitOK,
+			"Page":            page,
+			"TotalPages":      totalPages,
+			"TotalEvents":     total,
+			"HasPrev":         page > 1,
+			"HasNext":         page < totalPages,
+			"PrevPage":        page - 1,
+			"NextPage":        page + 1,
 		}
 
 		h.renderTemplate(w, r, "source_logs.html", data)
@@ -929,7 +950,6 @@ 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,
@@ -943,7 +963,7 @@ func (h *Handlers) loadEventsWithDeliveries(
 	webhookDB, err := h.dbMgr.GetDB(webhook.ID)
 	if err != nil {
 		h.serverError(
-			w, r, "failed to get webhook database", err,
+			w, "failed to get webhook database", err,
 		)
 
 		return nil, 0, false
@@ -980,7 +1000,7 @@ func (h *Handlers) loadEventsWithDeliveries(
 	)
 	if err != nil {
 		h.serverError(
-			w, r, "failed to load delivery attempts", err,
+			w, "failed to load delivery attempts", err,
 		)
 
 		return nil, 0, false
@@ -989,7 +1009,7 @@ func (h *Handlers) loadEventsWithDeliveries(
 	resubmits, err := resubmitCounts(webhookDB, eventIDs)
 	if err != nil {
 		h.serverError(
-			w, r, "failed to count event resubmissions", err,
+			w, "failed to count event resubmissions", err,
 		)
 
 		return nil, 0, false
@@ -1212,7 +1232,7 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			h.renderError(w, r, http.StatusNotFound)
+			http.NotFound(w, r)
 
 			return
 		}
@@ -1221,7 +1241,9 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err = r.ParseForm()
 		if err != nil {
-			h.renderError(w, r, http.StatusBadRequest)
+			http.Error(
+				w, "Bad request", http.StatusBadRequest,
+			)
 
 			return
 		}
@@ -1237,14 +1259,13 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 
 		err = h.db.DB().Create(entrypoint).Error
 		if err != nil {
-			h.serverError(w, r, "failed to create entrypoint", err)
+			h.serverError(w, "failed to create entrypoint", err)
 
 			return
 		}
 
 		http.Redirect(
-			w, r, withNotice("/hook/"+webhook.ID, entrypointAdded),
-			http.StatusSeeOther,
+			w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 		)
 	}
 }
@@ -1269,7 +1290,7 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			h.renderError(w, r, http.StatusNotFound)
+			http.NotFound(w, r)
 
 			return
 		}
@@ -1278,7 +1299,9 @@ func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
 		// middleware, which runs before CSRF parses the form.
 		err = r.ParseForm()
 		if err != nil {
-			h.renderError(w, r, http.StatusBadRequest)
+			http.Error(
+				w, "Bad request", http.StatusBadRequest,
+			)
 
 			return
 		}
@@ -1349,14 +1372,13 @@ func (h *Handlers) processTargetCreate(
 
 	err = h.db.DB().Create(target).Error
 	if err != nil {
-		h.serverError(w, r, "failed to create target", err)
+		h.serverError(w, "failed to create target", err)
 
 		return
 	}
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, targetAdded),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 
@@ -1444,7 +1466,7 @@ func (h *Handlers) buildTargetConfig(
 	case database.TargetTypeSlack:
 		return h.buildSlackTargetConfig(w, r, in.URL)
 	case database.TargetTypeDatabase:
-		return h.buildDatabaseTargetConfig(w, r, in.Expiry)
+		return h.buildDatabaseTargetConfig(w, in.Expiry)
 	case database.TargetTypeLog:
 		return "", nil
 	default:
@@ -1494,7 +1516,7 @@ func (h *Handlers) buildHTTPTargetConfig(
 		return "", err
 	}
 
-	return h.marshalTargetConfig(w, r, delivery.HTTPTargetConfig{
+	return marshalTargetConfig(w, delivery.HTTPTargetConfig{
 		URL:     in.URL,
 		Headers: headers,
 		Timeout: timeout,
@@ -1516,7 +1538,7 @@ func (h *Handlers) buildSlackTargetConfig(
 		return "", err
 	}
 
-	return h.marshalTargetConfig(w, r, delivery.SlackTargetConfig{
+	return marshalTargetConfig(w, delivery.SlackTargetConfig{
 		WebhookURL: targetURL,
 	})
 }
@@ -1556,22 +1578,11 @@ func (h *Handlers) validateTargetURL(
 			"url", delivery.MaskURL(targetURL),
 			"error", err,
 		)
-
-		msg := "Invalid target URL: " + err.Error()
-
-		// Only a private or reserved address's refusal says how
-		// to allow it. Metadata refusals never do: link-local and
-		// the other unconditional metadata addresses cannot be
-		// opened, and the default blocklist's public addresses,
-		// which listing does open, hand out credentials.
-		if errors.Is(err, delivery.ErrBlockedPrivateOrReservedIP) {
-			msg += ". Private and reserved addresses are refused " +
-				"by default; the server's ALLOWED_EGRESS_CIDRS " +
-				"setting allows named networks (see \"Allowing " +
-				"egress to your own network\" in the README)."
-		}
-
-		http.Error(w, msg, http.StatusBadRequest)
+		http.Error(
+			w,
+			"Invalid target URL: "+err.Error(),
+			http.StatusBadRequest,
+		)
 
 		return err
 	}
@@ -1581,14 +1592,16 @@ func (h *Handlers) validateTargetURL(
 
 // marshalTargetConfig serialises a target configuration for storage,
 // writing a 500 itself if it cannot.
-func (h *Handlers) marshalTargetConfig(
+func marshalTargetConfig(
 	w http.ResponseWriter,
-	r *http.Request,
 	cfg any,
 ) (string, error) {
 	configBytes, err := json.Marshal(cfg)
 	if err != nil {
-		h.serverError(w, r, "failed to encode target config", err)
+		http.Error(
+			w, "Internal server error",
+			http.StatusInternalServerError,
+		)
 
 		return "", err
 	}
@@ -1604,7 +1617,6 @@ func (h *Handlers) 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)
@@ -1623,8 +1635,8 @@ func (h *Handlers) buildDatabaseTargetConfig(
 		return "", err
 	}
 
-	return h.marshalTargetConfig(
-		w, r, map[string]any{"expiry": expiry},
+	return marshalTargetConfig(
+		w, map[string]any{"expiry": expiry},
 	)
 }
 
@@ -1634,7 +1646,6 @@ func (h *Handlers) HandleEntrypointDelete() http.HandlerFunc {
 		"entrypointID", &database.Entrypoint{},
 		"failed to delete entrypoint",
 		nil,
-		entrypointDeleted,
 	)
 }
 
@@ -1647,21 +1658,18 @@ func (h *Handlers) HandleTargetDelete() http.HandlerFunc {
 		"targetID", &database.Target{},
 		"failed to delete target",
 		h.evictArchiveWriterIfUnused,
-		targetDeleted,
 	)
 }
 
 // deleteChildResource returns a handler that deletes a child
 // resource (entrypoint or target) belonging to a webhook. The
 // optional afterDelete hook runs with the webhook's id once the
-// delete has succeeded, before the redirect, which carries done as
-// its notice.
+// delete has succeeded, before the redirect.
 func (h *Handlers) deleteChildResource(
 	idParam string,
 	model any,
 	errMsg string,
 	afterDelete func(webhookID string),
-	done noticeCode,
 ) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		userID, ok := h.getUserID(r)
@@ -1682,7 +1690,7 @@ func (h *Handlers) deleteChildResource(
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			h.renderError(w, r, http.StatusNotFound)
+			http.NotFound(w, r)
 
 			return
 		}
@@ -1692,7 +1700,11 @@ func (h *Handlers) deleteChildResource(
 			childID, webhook.ID,
 		).Delete(model)
 		if result.Error != nil {
-			h.serverError(w, r, errMsg, result.Error)
+			h.log.Error(errMsg, "error", result.Error)
+			http.Error(
+				w, "Internal server error",
+				http.StatusInternalServerError,
+			)
 
 			return
 		}
@@ -1703,7 +1715,7 @@ func (h *Handlers) deleteChildResource(
 
 		http.Redirect(
 			w, r,
-			withNotice("/hook/"+webhook.ID, done),
+			"/hook/"+webhook.ID,
 			http.StatusSeeOther,
 		)
 	}
@@ -1714,7 +1726,7 @@ func (h *Handlers) deleteChildResource(
 func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
 	return h.toggleChildResource(
 		"entrypointID",
-		func(webhookID, childID string) (bool, error) {
+		func(webhookID, childID string) error {
 			var ep database.Entrypoint
 
 			err := h.db.DB().Where(
@@ -1722,15 +1734,14 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
 				childID, webhookID,
 			).First(&ep).Error
 			if err != nil {
-				return false, err
+				return err
 			}
 
 			ep.Active = !ep.Active
 
-			return ep.Active, h.db.DB().Save(&ep).Error
+			return h.db.DB().Save(&ep).Error
 		},
 		"failed to toggle entrypoint",
-		entrypointActivated, entrypointDeactivated,
 	)
 }
 
@@ -1738,7 +1749,7 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
 func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
 	return h.toggleChildResource(
 		"targetID",
-		func(webhookID, childID string) (bool, error) {
+		func(webhookID, childID string) error {
 			var tgt database.Target
 
 			err := h.db.DB().Where(
@@ -1746,27 +1757,23 @@ func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
 				childID, webhookID,
 			).First(&tgt).Error
 			if err != nil {
-				return false, err
+				return err
 			}
 
 			tgt.Active = !tgt.Active
 
-			return tgt.Active, h.db.DB().Save(&tgt).Error
+			return h.db.DB().Save(&tgt).Error
 		},
 		"failed to toggle target",
-		targetActivated, targetDeactivated,
 	)
 }
 
 // toggleChildResource returns a handler that toggles the active
-// state of a child resource belonging to a webhook. toggleFn returns
-// the new state, and the redirect carries activated or deactivated as
-// its notice to match.
+// state of a child resource belonging to a webhook.
 func (h *Handlers) toggleChildResource(
 	idParam string,
-	toggleFn func(webhookID, childID string) (bool, error),
+	toggleFn func(webhookID, childID string) error,
 	errMsg string,
-	activated, deactivated noticeCode,
 ) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		userID, ok := h.getUserID(r)
@@ -1787,26 +1794,25 @@ func (h *Handlers) toggleChildResource(
 			"id = ? AND user_id = ?", sourceID, userID,
 		).First(&webhook).Error
 		if err != nil {
-			h.renderError(w, r, http.StatusNotFound)
+			http.NotFound(w, r)
 
 			return
 		}
 
-		active, err := toggleFn(webhook.ID, childID)
+		err = toggleFn(webhook.ID, childID)
 		if err != nil {
-			h.serverError(w, r, errMsg, err)
+			h.log.Error(errMsg, "error", err)
+			http.Error(
+				w, "Internal server error",
+				http.StatusInternalServerError,
+			)
 
 			return
 		}
 
-		done := deactivated
-		if active {
-			done = activated
-		}
-
 		http.Redirect(
 			w, r,
-			withNotice("/hook/"+webhook.ID, done),
+			"/hook/"+webhook.ID,
 			http.StatusSeeOther,
 		)
 	}

internal/handlers/target_edit.go

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

internal/handlers/target_private_refusal_test.go

@@ -1,116 +0,0 @@
-package handlers_test
-
-import (
-	"net/http"
-	"net/url"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/database"
-)
-
-// privateRefusalHint is the sentence that tells an operator a private
-// destination is refused on purpose, and how to allow one.
-const privateRefusalHint = "Private and reserved addresses are " +
-	"refused by default; the server's ALLOWED_EGRESS_CIDRS setting " +
-	"allows named networks (see \"Allowing egress to your own " +
-	"network\" in the README)."
-
-// TestTargetRefusal_PrivateDestinationSaysHowToAllowIt covers both
-// target types that take a URL, on add and on edit.
-func TestTargetRefusal_PrivateDestinationSaysHowToAllowIt(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	targetTypes := []database.TargetType{
-		database.TargetTypeHTTP,
-		database.TargetTypeSlack,
-	}
-
-	for _, targetType := range targetTypes {
-		t.Run(string(targetType), func(t *testing.T) {
-			t.Parallel()
-
-			webhook := seedWebhookWithRetention(t, env.db, 30)
-			targetsPath := "/hook/" + webhook.ID + "/targets"
-
-			form := url.Values{}
-			form.Set("name", "private")
-			form.Set("type", string(targetType))
-			form.Set("url", editBlockedURL)
-
-			added := serveTarget(
-				env, http.MethodPost, targetsPath, form,
-			)
-			assert.Equal(t, http.StatusBadRequest, added.Code)
-			assert.Contains(
-				t, added.Body.String(), privateRefusalHint,
-			)
-
-			form.Set("url", editOriginalURL)
-
-			created := serveTarget(
-				env, http.MethodPost, targetsPath, form,
-			)
-			require.Equal(
-				t, http.StatusSeeOther, created.Code,
-				created.Body.String(),
-			)
-
-			targets := targetsForWebhook(t, env.db, webhook.ID)
-			require.Len(t, targets, 1)
-
-			form.Set("url", editBlockedURL)
-
-			edited := submitTargetEdit(
-				env, webhook.ID, targets[0].ID, form,
-			)
-			assert.Equal(t, http.StatusBadRequest, edited.Code)
-			assert.Contains(
-				t, edited.Body.String(), privateRefusalHint,
-			)
-		})
-	}
-}
-
-// TestTargetRefusal_MetadataDestinationDoesNotSayHowToAllowIt: no
-// setting opens a link-local address, and Azure's WireServer hands out
-// VM credentials, so neither refusal points at the setting.
-func TestTargetRefusal_MetadataDestinationDoesNotSayHowToAllowIt(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	metadataURLs := map[string]string{
-		"link-local": "http://169.254.169.254/latest/meta-data/",
-		"wireserver": "http://168.63.129.16/?comp=versions",
-	}
-
-	for name, metadataURL := range metadataURLs {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			webhook := seedWebhookWithRetention(t, env.db, 30)
-
-			form := url.Values{}
-			form.Set("name", "metadata")
-			form.Set("type", string(database.TargetTypeHTTP))
-			form.Set("url", metadataURL)
-
-			w := serveTarget(
-				env, http.MethodPost,
-				"/hook/"+webhook.ID+"/targets", form,
-			)
-			assert.Equal(t, http.StatusBadRequest, w.Code)
-			assert.NotContains(
-				t, w.Body.String(), privateRefusalHint,
-			)
-		})
-	}
-}

internal/handlers/webhook.go

@@ -88,14 +88,14 @@ func (h *Handlers) processWebhookRequest(
 
 	headersJSON, err := json.Marshal(r.Header)
 	if err != nil {
-		h.receiverError(w, "failed to serialize headers", err)
+		h.serverError(w, "failed to serialize headers", err)
 
 		return
 	}
 
 	targets, err := h.loadActiveTargets(entrypoint.WebhookID)
 	if err != nil {
-		h.receiverError(w, "failed to query targets", err)
+		h.serverError(w, "failed to query targets", err)
 
 		return
 	}
@@ -196,7 +196,7 @@ func (h *Handlers) createAndDeliverEvent(
 		targets,
 	)
 	if err != nil {
-		h.receiverError(w, "failed to store webhook event", err)
+		h.serverError(w, "failed to store webhook event", err)
 
 		return
 	}
@@ -204,19 +204,6 @@ 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

internal/metrics/metrics.go

@@ -3,18 +3,17 @@
 // deliveries are attempted, how they end, how long they take, how
 // deep the queues are, and how many circuit breakers are open.
 //
-// It also builds the registry the authenticated /metrics route
-// serves. In production, these collectors, the inbound HTTP metrics
-// recorded in internal/middleware, and the Go runtime and process
-// collectors all register on that one registry, never on Prometheus's
-// global default.
+// The inbound HTTP metrics come from the go-http-metrics recorder in
+// internal/middleware and land on prometheus.DefaultRegisterer. These
+// collectors register there too, so both surfaces are gathered by the
+// one promhttp handler mounted on the authenticated /metrics route.
 package metrics
 
 import (
+	"sync"
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/client_golang/prometheus/collectors"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	"sneak.berlin/go/webhooker/internal/database"
 )
@@ -58,31 +57,25 @@ var knownTargetTypes = []database.TargetType{
 	database.TargetTypeSlack,
 }
 
-// NewRegistry returns the registry /metrics serves, carrying the Go
-// runtime and process collectors that Prometheus's global default
-// registry carries, so the go_* and process_* series stay in the
-// scrape.
+// defaultSet is the process-wide metric set, registered on the same
+// registry the HTTP middleware and the /metrics handler already use.
+// It is built on first use rather than in an init so that a test
+// binary that never touches metrics never registers them.
 //
-// A registry of its own, rather than the global default, is what lets
-// two dependency graphs in one process — two tests, say — each
-// register their collectors without the second registration
-// panicking.
-func NewRegistry() *prometheus.Registry {
-	reg := prometheus.NewRegistry()
-	reg.MustRegister(
-		collectors.NewGoCollector(),
-		collectors.NewProcessCollector(
-			collectors.ProcessCollectorOpts{},
-		),
-	)
+//nolint:gochecknoglobals // one process-wide registration, by design
+var defaultSet = sync.OnceValue(func() *Set {
+	return New(prometheus.DefaultRegisterer)
+})
 
-	return reg
+// Default returns the process-wide metric set.
+func Default() *Set {
+	return defaultSet()
 }
 
 // Set is one registered group of webhooker's delivery collectors.
-// Production builds one on the registry /metrics serves; tests build
-// one on a registry of their own so they can gather what their own
-// deliveries recorded.
+// Production uses the single Default set; tests build their own
+// against a private registry so assertions are not disturbed by
+// deliveries other tests are making concurrently.
 type Set struct {
 	eventsReceived      prometheus.Counter
 	deliveryAttempts    *prometheus.CounterVec
@@ -100,7 +93,7 @@ type Set struct {
 // New registers a full set of delivery collectors on reg and returns
 // it. It panics if reg already holds them, which is the intended
 // behaviour for a duplicate registration.
-func New(reg *prometheus.Registry) *Set {
+func New(reg prometheus.Registerer) *Set {
 	factory := promauto.With(reg)
 
 	s := &Set{

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 are logged and answered by forbidden, which must write the 403.
+// token receive a 403 Forbidden response.
 //
 // The middleware detects the client-facing transport protocol
 // per-request via reqtls.IsTLS, the single TLS predicate the session
@@ -36,9 +36,7 @@ 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(
-	forbidden http.Handler,
-) func(http.Handler) http.Handler {
+func (m *Middleware) CSRF() 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
@@ -59,7 +57,7 @@ func (m *Middleware) CSRF(
 			"remote_addr", r.RemoteAddr,
 			"reason", csrf.FailureReason(r),
 		)
-		forbidden.ServeHTTP(w, r)
+		http.Error(w, "Forbidden - invalid CSRF token", http.StatusForbidden)
 	})
 
 	key := m.session.GetKey()

internal/middleware/csrf_test.go

@@ -18,12 +18,6 @@ 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(
@@ -104,7 +98,7 @@ func TestCSRF_GETSetsToken(t *testing.T) {
 
 	var gotToken string
 
-	handler := m.CSRF(http.HandlerFunc(forbidden))(http.HandlerFunc(
+	handler := m.CSRF()(http.HandlerFunc(
 		func(_ http.ResponseWriter, r *http.Request) {
 			gotToken = middleware.CSRFToken(r)
 		},
@@ -126,7 +120,7 @@ func TestCSRF_POSTWithValidToken(t *testing.T) {
 	t.Parallel()
 
 	m, _ := testMiddleware(t, config.EnvironmentDev)
-	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
+	csrfMW := m.CSRF()
 
 	getReq := httptest.NewRequestWithContext(
 		context.Background(),
@@ -158,7 +152,7 @@ func csrfPOSTWithoutTokenTest(
 	t.Helper()
 
 	m, _ := testMiddleware(t, env)
-	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
+	csrfMW := m.CSRF()
 
 	// GET to establish the CSRF cookie
 	getHandler := csrfMW(http.HandlerFunc(
@@ -215,7 +209,7 @@ func TestCSRF_POSTWithInvalidToken(t *testing.T) {
 	t.Parallel()
 
 	m, _ := testMiddleware(t, config.EnvironmentDev)
-	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
+	csrfMW := m.CSRF()
 
 	// GET to establish the CSRF cookie
 	getHandler := csrfMW(http.HandlerFunc(
@@ -271,7 +265,7 @@ func TestCSRF_GETDoesNotValidate(t *testing.T) {
 
 	var called bool
 
-	handler := m.CSRF(http.HandlerFunc(forbidden))(http.HandlerFunc(
+	handler := m.CSRF()(http.HandlerFunc(
 		func(_ http.ResponseWriter, _ *http.Request) {
 			called = true
 		},
@@ -334,7 +328,7 @@ func csrfTookStrictPath(
 	t.Helper()
 
 	m, _ := testMiddleware(t, env)
-	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
+	csrfMW := m.CSRF()
 
 	newReq := func(method string) *http.Request {
 		r := httptest.NewRequestWithContext(
@@ -483,7 +477,7 @@ func TestCSRF_ProdMode_PlaintextHTTP_POSTWithValidToken(
 	t.Parallel()
 
 	m, _ := testMiddleware(t, config.EnvironmentProd)
-	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
+	csrfMW := m.CSRF()
 
 	getReq := httptest.NewRequestWithContext(
 		context.Background(),
@@ -523,7 +517,7 @@ func TestCSRF_ProdMode_BehindProxy_POSTWithValidToken(
 	t.Parallel()
 
 	m, _ := testMiddleware(t, config.EnvironmentProd)
-	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
+	csrfMW := m.CSRF()
 
 	getReq := httptest.NewRequestWithContext(
 		context.Background(),
@@ -568,7 +562,7 @@ func TestCSRF_ProdMode_DirectTLS_POSTWithValidToken(
 	t.Parallel()
 
 	m, _ := testMiddleware(t, config.EnvironmentProd)
-	csrfMW := m.CSRF(http.HandlerFunc(forbidden))
+	csrfMW := m.CSRF()
 
 	getReq := httptest.NewRequestWithContext(
 		context.Background(),

internal/middleware/export_test.go

@@ -10,7 +10,8 @@ import (
 
 // MetricsMiddlewareForTest builds the metrics recording middleware
 // against a caller-supplied recorder, so a test can gather from its
-// own Prometheus registry without building a whole Middleware.
+// own Prometheus registry rather than the process-wide default one
+// that Middleware.Metrics uses.
 func MetricsMiddlewareForTest(
 	rec httpmetrics.Recorder,
 ) func(http.Handler) http.Handler {

internal/middleware/logbound_test.go

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

internal/middleware/metrics.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/go-chi/chi"
 	httpmetrics "github.com/slok/go-http-metrics/metrics"
+	prommetrics "github.com/slok/go-http-metrics/metrics/prometheus"
 	ghmm "github.com/slok/go-http-metrics/middleware"
 	"github.com/slok/go-http-metrics/middleware/std"
 )
@@ -150,17 +151,17 @@ func (r boundedLabelRecorder) AddInflightRequests(
 
 var _ httpmetrics.Recorder = boundedLabelRecorder{}
 
-// Metrics returns middleware that records Prometheus HTTP metrics
-// with the Middleware's one recorder, which New builds on the registry
-// the /metrics route serves and NewForTest on a registry of its own.
-// Every call reuses that recorder, so any number of routers can
-// install it.
+// Metrics returns middleware that records Prometheus HTTP metrics on
+// the default registry, which is the one the /metrics route gathers.
 func (s *Middleware) Metrics() func(http.Handler) http.Handler {
-	return metricsMiddleware(s.metricsRecorder)
+	return metricsMiddleware(
+		prommetrics.NewRecorder(prommetrics.Config{}),
+	)
 }
 
 // metricsMiddleware builds the recording middleware against a given
-// recorder, so tests can gather from a registry of their own.
+// recorder, so tests can gather from a registry of their own instead
+// of the process-wide default.
 func metricsMiddleware(
 	rec httpmetrics.Recorder,
 ) func(http.Handler) http.Handler {

internal/middleware/metrics_test.go

@@ -57,8 +57,9 @@ const (
 // Server.setupWebhookRoutes inside it. That ordering is the whole
 // defect, so a test that flattens it would prove nothing.
 //
-// The recorder writes to a registry of the test's own, so each test
-// observes only its own traffic.
+// The recorder writes to a registry of the test's own rather than the
+// process-wide default one, so each test observes only its own
+// traffic.
 func metricsTestRouter(
 	t *testing.T,
 	receiverLimit int,
@@ -454,29 +455,3 @@ func TestMetrics_StatusAndSizeStillRecorded(t *testing.T) {
 		"the interceptor must still count written bytes",
 	)
 }
-
-// TestMetrics_WorksOnNewForTestMiddleware pins that a Middleware built
-// by NewForTest has a recorder of its own: its Metrics() serves a
-// request instead of panicking, and a second one does not collide
-// with the first.
-func TestMetrics_WorksOnNewForTestMiddleware(t *testing.T) {
-	t.Parallel()
-
-	log := slog.New(slog.DiscardHandler)
-	cfg := &config.Config{Environment: "prod"}
-	ok := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
-		_, _ = w.Write([]byte(okBody))
-	})
-
-	for range 2 {
-		h := middleware.NewForTest(log, cfg, nil).Metrics()(ok)
-
-		req := httptest.NewRequestWithContext(
-			t.Context(), http.MethodGet, okRoute, nil,
-		)
-		w := httptest.NewRecorder()
-		h.ServeHTTP(w, req)
-
-		assert.Equal(t, http.StatusOK, w.Code)
-	}
-}

internal/middleware/middleware.go

@@ -14,9 +14,6 @@ import (
 	"github.com/go-chi/chi"
 	"github.com/go-chi/chi/middleware"
 	"github.com/go-chi/cors"
-	"github.com/prometheus/client_golang/prometheus"
-	httpmetrics "github.com/slok/go-http-metrics/metrics"
-	prommetrics "github.com/slok/go-http-metrics/metrics/prometheus"
 	"go.uber.org/fx"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/globals"
@@ -152,11 +149,10 @@ const (
 type MiddlewareParams struct {
 	fx.In
 
-	Logger   *logger.Logger
-	Globals  *globals.Globals
-	Config   *config.Config
-	Session  *session.Session
-	Registry *prometheus.Registry
+	Logger  *logger.Logger
+	Globals *globals.Globals
+	Config  *config.Config
+	Session *session.Session
 }
 
 // Middleware provides HTTP middleware for logging, CORS, auth, and
@@ -166,14 +162,6 @@ type Middleware struct {
 	params  *MiddlewareParams
 	session *session.Session
 
-	// metricsRecorder records the inbound HTTP metrics. New builds
-	// it on the registry /metrics serves, NewForTest on a registry
-	// of its own. Either way it is built once per Middleware and
-	// Metrics reuses it, because building it registers its
-	// collectors, and a second registration on the same registry
-	// panics.
-	metricsRecorder httpmetrics.Recorder
-
 	// loginGuard counts failed credential verifications and bounds
 	// concurrent password hashing. It is built on first use so that
 	// every construction path gets one; see guard().
@@ -192,9 +180,6 @@ func New(
 	s.params = &params
 	s.log = params.Logger.Get()
 	s.session = params.Session
-	s.metricsRecorder = prommetrics.NewRecorder(
-		prommetrics.Config{Registry: params.Registry},
-	)
 
 	return s, nil
 }

internal/middleware/recoverer.go

@@ -109,8 +109,7 @@ func (w *recoverResponseWriter) Unwrap() http.ResponseWriter {
 
 // Recoverer returns middleware that turns a handler panic into one
 // structured ERROR record and a 500, rather than a dropped
-// connection. The 500 is page when page is not nil, and plain text
-// when it is nil or when page panics before writing anything.
+// connection.
 //
 // It replaces chi's middleware.Recoverer, which does neither on a
 // current Go release. chi v1.5.5's pretty-printer scans the stack for
@@ -137,13 +136,9 @@ func (w *recoverResponseWriter) Unwrap() http.ResponseWriter {
 //
 // Unlike http.Error on its own, it deletes any Set-Cookie the handler
 // set before panicking, because a request that failed must not hand
-// the client a credential. It touches no other header: when page
-// answers, every other header the handler set goes out with it, apart
-// from any page sets itself; otherwise they are left to http.Error.
+// the client a credential; every other header is left to http.Error.
 // See https://git.eeqj.de/sneak/webhooker/issues/193.
-func (s *Middleware) Recoverer(
-	page http.Handler,
-) func(http.Handler) http.Handler {
+func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(
 			w http.ResponseWriter,
@@ -176,14 +171,6 @@ func (s *Middleware) Recoverer(
 
 				rw.Header().Del("Set-Cookie")
 
-				if page != nil {
-					s.servePage(rw, r, page)
-				}
-
-				if rw.committed {
-					return
-				}
-
 				http.Error(
 					rw,
 					http.StatusText(
@@ -198,27 +185,6 @@ func (s *Middleware) Recoverer(
 	}
 }
 
-// servePage answers with page. A panic in page itself is logged and
-// recovered here, so the Recoverer can still send its plain 500.
-func (s *Middleware) servePage(
-	w http.ResponseWriter,
-	r *http.Request,
-	page http.Handler,
-) {
-	defer func() {
-		rvr := recover()
-		if rvr != nil {
-			s.log.Error("error page panic",
-				"panic", logfield.Truncate(
-					fmt.Sprint(rvr), maxPanicValueBytes,
-				),
-			)
-		}
-	}()
-
-	page.ServeHTTP(w, r)
-}
-
 // logPanic writes the record. Every field it can grow is truncated to
 // a fixed budget, so MaxPanicLogLineBytes holds.
 //

internal/middleware/recoverer_test.go

@@ -76,7 +76,7 @@ func newRecovererProbe(
 	// Logging outside so the recovered 500 is the status it records.
 	router.Use(chimw.RequestID)
 	router.Use(m.Logging())
-	router.Use(m.Recoverer(nil))
+	router.Use(m.Recoverer())
 	router.Get("/probe", handler)
 
 	serverErrors := new(bytes.Buffer)
@@ -637,7 +637,7 @@ func TestRecovererKeepsResponseControllerWorking(t *testing.T) {
 
 	m, _ := capturingMiddleware(t)
 
-	handler := m.Recoverer(nil)(http.HandlerFunc(
+	handler := m.Recoverer()(http.HandlerFunc(
 		func(w http.ResponseWriter, _ *http.Request) {
 			_, _ = w.Write([]byte("chunk"))
 
@@ -672,59 +672,3 @@ func TestRecovererKeepsResponseControllerWorking(t *testing.T) {
 	assert.Equal(t, http.StatusOK, resp.StatusCode)
 	assert.Equal(t, "chunk", string(body))
 }
-
-// TestRecovererAnswersWithThePage covers a recoverer given a page:
-// the panic is logged as before, and the 500 is that page.
-func TestRecovererAnswersWithThePage(t *testing.T) {
-	t.Parallel()
-
-	m, logs := capturingMiddleware(t)
-
-	page := http.HandlerFunc(
-		func(w http.ResponseWriter, _ *http.Request) {
-			w.WriteHeader(http.StatusInternalServerError)
-			_, _ = w.Write([]byte("the error page"))
-		},
-	)
-
-	w := httptest.NewRecorder()
-	m.Recoverer(page)(http.HandlerFunc(panicProbe)).ServeHTTP(
-		w, httptest.NewRequestWithContext(
-			t.Context(), http.MethodGet, "/", nil,
-		),
-	)
-
-	assert.Equal(t, http.StatusInternalServerError, w.Code)
-	assert.Equal(t, "the error page", w.Body.String())
-	assert.Contains(t, logs.String(), `"msg":"handler panic"`)
-	assert.Contains(t, logs.String(), panicMarker)
-}
-
-// TestRecovererFallsBackWhenThePagePanics covers a page that panics
-// before writing anything: both panics are logged, and the client
-// still gets the plain 500.
-func TestRecovererFallsBackWhenThePagePanics(t *testing.T) {
-	t.Parallel()
-
-	m, logs := capturingMiddleware(t)
-
-	const pagePanic = "QQERRORPAGEPANICQQ"
-
-	page := http.HandlerFunc(
-		func(http.ResponseWriter, *http.Request) {
-			panic(pagePanic)
-		},
-	)
-
-	w := httptest.NewRecorder()
-	m.Recoverer(page)(http.HandlerFunc(panicProbe)).ServeHTTP(
-		w, httptest.NewRequestWithContext(
-			t.Context(), http.MethodGet, "/", nil,
-		),
-	)
-
-	assert.Equal(t, http.StatusInternalServerError, w.Code)
-	assert.Equal(t, "Internal Server Error\n", w.Body.String())
-	assert.Contains(t, logs.String(), panicMarker)
-	assert.Contains(t, logs.String(), pagePanic)
-}

internal/middleware/testing.go

@@ -3,17 +3,12 @@ package middleware
 import (
 	"log/slog"
 
-	"github.com/prometheus/client_golang/prometheus"
-	prommetrics "github.com/slok/go-http-metrics/metrics/prometheus"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/session"
 )
 
 // NewForTest creates a Middleware with the minimum dependencies
 // needed for testing. This bypasses the fx lifecycle.
-//
-// Its metrics recorder writes to a fresh registry of its own, so
-// Metrics() works on it and two of them never collide.
 func NewForTest(
 	log *slog.Logger,
 	cfg *config.Config,
@@ -25,8 +20,5 @@ func NewForTest(
 			Config: cfg,
 		},
 		session: sess,
-		metricsRecorder: prommetrics.NewRecorder(
-			prommetrics.Config{Registry: prometheus.NewRegistry()},
-		),
 	}
 }

internal/resetpw/resetpw_test.go

@@ -24,7 +24,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/handlers"
 	"sneak.berlin/go/webhooker/internal/healthcheck"
 	"sneak.berlin/go/webhooker/internal/logger"
-	"sneak.berlin/go/webhooker/internal/metrics"
 	"sneak.berlin/go/webhooker/internal/middleware"
 	"sneak.berlin/go/webhooker/internal/resetpw"
 	"sneak.berlin/go/webhooker/internal/session"
@@ -164,8 +163,6 @@ func newServerApp(
 			session.New,
 			func() delivery.Notifier { return &noopNotifier{} },
 			func() delivery.WebhookEvictor { return &noopEvictor{} },
-			metrics.NewRegistry,
-			metrics.New,
 			middleware.New,
 			delivery.NewGuard,
 			handlers.New,

internal/server/alpine_browser_test.go

@@ -0,0 +1,391 @@
+//go:build browser
+
+// This test needs a headless browser, so it is built only with the
+// browser build tag: `make test` leaves it out, and `make test-browser`
+// runs it in the browser image that Dockerfile.browser pins.
+
+package server_test
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"slices"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/chromedp/cdproto/log"
+	"github.com/chromedp/cdproto/network"
+	"github.com/chromedp/cdproto/runtime"
+	"github.com/chromedp/chromedp"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gorm.io/gorm/clause"
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+const (
+	// browserTimeout bounds everything one test does in the browser.
+	browserTimeout = 60 * time.Second
+
+	// settleTimeout bounds the wait for an element to show or hide.
+	settleTimeout = 5 * time.Second
+
+	// The window size of a phone, narrow enough that the pages show
+	// the mobile menu button instead of the navigation links.
+	phoneWidth  = 390
+	phoneHeight = 844
+)
+
+// TestAlpineRunsUnderTheSecurityPolicy loads the webhook page and the
+// event log in a headless browser, served by the real router and so
+// under the real Content-Security-Policy, and checks that the pages'
+// Alpine.js directives work.
+func TestAlpineRunsUnderTheSecurityPolicy(t *testing.T) {
+	t.Parallel()
+
+	ctx, problems := startBrowser(t)
+
+	env := newTestEnv(t)
+	srv := httptest.NewServer(env.router)
+	t.Cleanup(srv.Close)
+
+	userID, _ := env.seedUser(t, "browser", "browser-password")
+	webhook := env.seedWebhook(t, userID)
+	event := env.seedEvent(t, webhook.ID, `{"hello":"browser"}`)
+	target := env.seedTarget(t, webhook.ID)
+	dlv := env.seedFailedDelivery(t, webhook.ID, event.ID, target.ID)
+
+	webhookDB, err := env.dbMgr.GetDB(webhook.ID)
+	require.NoError(t, err)
+	require.NoError(t, webhookDB.Omit(clause.Associations).Create(
+		&database.DeliveryResult{
+			DeliveryID: dlv.ID,
+			AttemptNum: 1,
+			StatusCode: http.StatusBadGateway,
+		},
+	).Error)
+
+	require.NoError(t, chromedp.Run(
+		ctx, setCookies(srv.URL, env.authCookies(t, userID, "browser")),
+	))
+
+	page := srv.URL + "/hook/" + webhook.ID
+
+	checkAddForms(ctx, t, page)
+	checkTargetType(ctx, t, page+"/events")
+	checkEventLog(ctx, t, page+"/events", event.ID, target.Name)
+	checkMobileMenu(ctx, t, page)
+
+	assert.Empty(t, problems(), "the browser reported problems")
+}
+
+// startBrowser starts a headless browser for one test. It returns the
+// context that drives it, and a function listing what the browser
+// reported going wrong on its pages: console warnings and errors,
+// which is how Alpine.js reports an expression it cannot run; uncaught
+// exceptions; and every entry in the browser's own security log, which
+// is where it reports each script, style, image or request the
+// Content-Security-Policy refused.
+//
+// The browser library finds the browser on PATH. Without one the first
+// chromedp.Run fails, and with it the test.
+func startBrowser(t *testing.T) (context.Context, func() []string) {
+	t.Helper()
+
+	allocCtx, cancelAlloc := chromedp.NewExecAllocator(
+		t.Context(),
+		append(
+			chromedp.DefaultExecAllocatorOptions[:],
+			// Dockerfile.browser runs the test as root, where the
+			// browser's sandbox cannot start.
+			chromedp.NoSandbox,
+		)...,
+	)
+	t.Cleanup(cancelAlloc)
+
+	ctx, cancel := chromedp.NewContext(allocCtx)
+	t.Cleanup(cancel)
+
+	ctx, cancelTimeout := context.WithTimeout(ctx, browserTimeout)
+	t.Cleanup(cancelTimeout)
+
+	var (
+		mu       sync.Mutex
+		problems []string
+	)
+
+	chromedp.ListenTarget(ctx, func(ev any) {
+		var problem string
+
+		switch ev := ev.(type) {
+		case *runtime.EventConsoleAPICalled:
+			if ev.Type != runtime.APITypeWarning &&
+				ev.Type != runtime.APITypeError {
+				return
+			}
+
+			args := make([]string, 0, len(ev.Args))
+			for _, arg := range ev.Args {
+				args = append(args, string(arg.Value))
+			}
+
+			problem = strings.Join(args, " ")
+		case *runtime.EventExceptionThrown:
+			problem = ev.ExceptionDetails.Error()
+		case *log.EventEntryAdded:
+			if ev.Entry.Source != log.SourceSecurity {
+				return
+			}
+
+			problem = ev.Entry.Text
+		default:
+			return
+		}
+
+		mu.Lock()
+		defer mu.Unlock()
+
+		problems = append(problems, problem)
+	})
+
+	return ctx, func() []string {
+		mu.Lock()
+		defer mu.Unlock()
+
+		return slices.Clone(problems)
+	}
+}
+
+// setCookies gives the browser the cookies for the server at base.
+func setCookies(base string, cookies []*http.Cookie) chromedp.ActionFunc {
+	return chromedp.ActionFunc(func(ctx context.Context) error {
+		for _, c := range cookies {
+			err := network.SetCookie(c.Name, c.Value).
+				WithURL(base).
+				Do(ctx)
+			if err != nil {
+				return fmt.Errorf("set cookie %s: %w", c.Name, err)
+			}
+		}
+
+		return nil
+	})
+}
+
+// loadPage opens url and waits for Alpine.js to start, which it does
+// by removing every x-cloak attribute. Until then x-cloak hides the
+// elements Alpine would hide, so a check made earlier proves nothing.
+func loadPage(url string) chromedp.Tasks {
+	return chromedp.Tasks{
+		chromedp.Navigate(url),
+		chromedp.WaitNotPresent("[x-cloak]", chromedp.ByQuery),
+	}
+}
+
+// shown waits up to settleTimeout for the elements matching a CSS
+// selector or an XPath expression to be rendered, and reports whether
+// they were. The wait is needed because Alpine.js shows an element on
+// the next animation frame, not at once.
+func shown(ctx context.Context, selector string) bool {
+	ctx, cancel := context.WithTimeout(ctx, settleTimeout)
+	defer cancel()
+
+	return chromedp.Run(
+		ctx, chromedp.WaitVisible(selector, chromedp.BySearch),
+	) == nil
+}
+
+// hidden is shown's opposite: it waits for the elements to be hidden.
+func hidden(ctx context.Context, selector string) bool {
+	ctx, cancel := context.WithTimeout(ctx, settleTimeout)
+	defer cancel()
+
+	return chromedp.Run(
+		ctx, chromedp.WaitNotVisible(selector, chromedp.BySearch),
+	) == nil
+}
+
+// click clicks the element matching an XPath expression.
+func click(ctx context.Context, t *testing.T, xpath string) {
+	t.Helper()
+
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.Click(xpath, chromedp.BySearch),
+	))
+}
+
+// checkAddForms loads a webhook page and checks that each section's add
+// form stays hidden until the Add button beside its heading is clicked.
+func checkAddForms(ctx context.Context, t *testing.T, url string) {
+	t.Helper()
+
+	require.NoError(t, chromedp.Run(ctx, loadPage(url)))
+
+	sections := []struct{ heading, form string }{
+		{"Entrypoints", `form[action$="/entrypoints"]`},
+		{"Targets", `form[action$="/targets"]`},
+	}
+
+	for _, s := range sections {
+		assert.Truef(
+			t, hidden(ctx, s.form),
+			"%s: the add form shows before Add is clicked", s.heading,
+		)
+
+		click(ctx, t, `//h2[text()="`+s.heading+
+			`"]/following-sibling::button`)
+
+		assert.Truef(
+			t, shown(ctx, s.form),
+			"%s: the add form stays hidden when Add is clicked", s.heading,
+		)
+	}
+}
+
+// checkTargetType chooses Slack in the open add target form and checks
+// what the form would then submit: one url field, the Slack one, and
+// not the HTTP url, headers or timeout, which are hidden and disabled.
+//
+// It then opens the page at elsewhere and goes back. The browser loads
+// the webhook page again and restores the form as it was left, Slack
+// chosen, without a change event; the form must again show and submit
+// Slack's fields, not the HTTP ones.
+func checkTargetType(ctx context.Context, t *testing.T, elsewhere string) {
+	t.Helper()
+
+	const (
+		chooseSlack = `(() => {
+			const type = document.querySelector('select[name="type"]');
+			type.value = "slack";
+			type.dispatchEvent(new Event("change"));
+		})()`
+		chosen    = `document.querySelector('select[name="type"]').value`
+		howLoaded = `performance.getEntriesByType("navigation")[0].type`
+		submitted = `[...new FormData(
+			document.querySelector('form[action$="/targets"]')).keys()]`
+		slackURL = `input[placeholder^="https://hooks.slack.com/"]`
+		httpURL  = `input[placeholder="https://example.com/webhook"]`
+	)
+
+	slackFields := strings.Fields("csrf_token name type max_retries url")
+
+	var fields []string
+
+	require.NoError(t, chromedp.Run(
+		ctx,
+		chromedp.Evaluate(chooseSlack, nil),
+		chromedp.Evaluate(submitted, &fields),
+	))
+
+	assert.Equal(
+		t, slackFields, fields,
+		"with Slack chosen, the HTTP fields must not be submitted",
+	)
+
+	var loaded, restored string
+
+	// Going back waits for the load event, after which the browser has
+	// restored the form.
+	require.NoError(t, chromedp.Run(
+		ctx,
+		loadPage(elsewhere),
+		chromedp.NavigateBack(),
+		chromedp.WaitNotPresent("[x-cloak]", chromedp.ByQuery),
+		chromedp.Evaluate(howLoaded, &loaded),
+		chromedp.Evaluate(chosen, &restored),
+	))
+
+	// A page the browser kept in memory and showed again as it was
+	// would prove nothing here.
+	require.Equal(
+		t, "back_forward", loaded,
+		"going back, the browser did not load the page again",
+	)
+	require.Equal(
+		t, "slack", restored,
+		"going back, the browser did not restore the chosen type",
+	)
+
+	click(ctx, t, `//h2[text()="Targets"]/following-sibling::button`)
+
+	assert.True(t, shown(ctx, slackURL),
+		"going back with Slack chosen, the Slack fields are not shown")
+	assert.True(t, hidden(ctx, httpURL),
+		"going back with Slack chosen, the HTTP fields are shown")
+
+	require.NoError(t, chromedp.Run(
+		ctx, chromedp.Evaluate(submitted, &fields),
+	))
+
+	assert.Equal(
+		t, slackFields, fields,
+		"going back with Slack chosen, the HTTP fields must not be submitted",
+	)
+}
+
+// checkEventLog loads the event log and checks that clicking an event's
+// row expands it, that in there clicking its delivery shows the
+// delivery's attempts and clicking again hides them, and that clicking
+// the event's row again collapses it.
+func checkEventLog(
+	ctx context.Context, t *testing.T, url, eventID, targetName string,
+) {
+	t.Helper()
+
+	// The event's row shows its ID, and its Resubmit form is in the part
+	// that expands. The delivery's row there shows the target's name.
+	eventRow := `//span[text()="` + eventID + `"]`
+	expanded := `form[action$="/resubmit"]`
+	deliveryRow := `//span[text()="` + targetName + `"]`
+	attempt := `//span[text()="Attempt 1"]`
+
+	require.NoError(t, chromedp.Run(ctx, loadPage(url)))
+
+	assert.True(t, hidden(ctx, expanded), "the event starts expanded")
+
+	click(ctx, t, eventRow)
+	assert.True(t, shown(ctx, expanded), "clicking the event does not expand it")
+
+	assert.True(t, hidden(ctx, attempt), "the delivery's attempts start shown")
+
+	click(ctx, t, deliveryRow)
+	assert.True(t, shown(ctx, attempt),
+		"clicking the delivery does not show its attempts")
+
+	click(ctx, t, deliveryRow)
+	assert.True(t, hidden(ctx, attempt),
+		"clicking the delivery again does not hide its attempts")
+
+	click(ctx, t, eventRow)
+	assert.True(t, hidden(ctx, expanded),
+		"clicking the event again does not collapse it")
+}
+
+// checkMobileMenu loads a page in a phone-sized window and checks that
+// the menu button opens and closes the mobile menu.
+func checkMobileMenu(ctx context.Context, t *testing.T, url string) {
+	t.Helper()
+
+	// The menu button is the only button directly in the navigation
+	// bar's top row. Profile is a link only the mobile menu has.
+	button := `//nav/div/button`
+	menu := `//nav//a[text()="Profile"]`
+
+	require.NoError(t, chromedp.Run(
+		ctx,
+		chromedp.EmulateViewport(phoneWidth, phoneHeight),
+		loadPage(url),
+	))
+
+	assert.True(t, hidden(ctx, menu), "the mobile menu starts open")
+
+	click(ctx, t, button)
+	assert.True(t, shown(ctx, menu), "the menu button does not open the menu")
+
+	click(ctx, t, button)
+	assert.True(t, hidden(ctx, menu), "the menu button does not close the menu")
+}

internal/server/error_page_test.go

@@ -1,238 +0,0 @@
-package server_test
-
-import (
-	"context"
-	"net/http"
-	"net/http/httptest"
-	"net/url"
-	"strconv"
-	"testing"
-
-	"github.com/getsentry/sentry-go"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/config"
-	"sneak.berlin/go/webhooker/internal/server"
-)
-
-// 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="/hooks" 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.Equal(t, "no-store", w.Header().Get("Cache-Control"))
-	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("/hook/"+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(
-		"/hook/"+wh.ID+"/targets/"+tgt.ID+"/edit", cookies,
-	)
-
-	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
-}
-
-// TestErrorPage_ShowsNoNotice pins that a notice code in the URL of a
-// page that fails is not shown above the error.
-func TestErrorPage_ShowsNoNotice(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	userID, _ := env.seedUser(t, "owner", "somepassword")
-	cookies := env.authCookies(t, userID, "owner")
-
-	w := env.get("/hook/no-such-webhook?notice=webhook-saved", cookies)
-
-	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
-	assert.NotContains(t, w.Body.String(), "Webhook saved.")
-}
-
-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("/hook/"+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("/hook/"+wh.ID+"/edit", edit, cookies),
-		http.StatusForbidden, backToWebhooks,
-	)
-}
-
-// TestErrorPage_PanicOnAdminPage sends a panicking handler in an
-// admin page route group through the real router, with error
-// tracking on: the client gets the 500 error page, and the tracker
-// still gets the panic, once. The same panic outside the admin page
-// route groups keeps the plain 500.
-func TestErrorPage_PanicOnAdminPage(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	transport := &captureTransport{}
-
-	opts := server.SentryClientOptionsForTest(
-		"https://public@sentry.invalid/1", "webhooker-test",
-	)
-	opts.Transport = transport
-
-	client, err := sentry.NewClient(opts)
-	require.NoError(t, err)
-
-	serve := func(router http.Handler, path string) *httptest.ResponseRecorder {
-		req := httptest.NewRequestWithContext(
-			sentry.SetHubOnContext(
-				context.Background(),
-				sentry.NewHub(client, sentry.NewScope()),
-			),
-			http.MethodGet, path, nil,
-		)
-
-		w := httptest.NewRecorder()
-		router.ServeHTTP(w, req)
-
-		return w
-	}
-
-	w := serve(
-		server.NewRouterWithPageProbeForTest(
-			env.log.Get(), env.cfg, env.mw, env.hnd,
-			true, panicProbeHandler,
-		),
-		server.PageProbePattern,
-	)
-	assertErrorPage(t, w, http.StatusInternalServerError, backToSignIn)
-
-	w = serve(
-		server.NewRouterWithProbeForTest(
-			env.log.Get(), env.cfg, env.mw, env.hnd,
-			true, panicProbeHandler,
-		),
-		server.ProbePattern,
-	)
-	assert.Equal(t, http.StatusInternalServerError, w.Code)
-	assert.Equal(t, "Internal Server Error\n", w.Body.String())
-
-	require.Len(t, transport.events, 2)
-
-	for _, event := range transport.events {
-		assert.Contains(t, marshalEvent(t, event), panicProbeMarker)
-	}
-}
-
-// 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(
-		"/h/0b8f3c1e-7d2a-4e6b-9f15-3a9c2d4e6f70", url.Values{}, nil,
-	)
-
-	assert.Equal(t, http.StatusNotFound, w.Code)
-	assert.Equal(t, "404 page not found\n", w.Body.String())
-}

internal/server/export_test.go

@@ -5,7 +5,6 @@ import (
 	"net/http"
 
 	"github.com/getsentry/sentry-go"
-	"github.com/go-chi/chi"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/handlers"
 	"sneak.berlin/go/webhooker/internal/middleware"
@@ -102,39 +101,3 @@ func NewRouterWithProbeForTest(
 
 	return s.router
 }
-
-// PageProbePattern is where NewRouterWithPageProbeForTest serves its
-// probe: inside the /pages route group, the admin page group a
-// request reaches without signing in.
-const PageProbePattern = "/pages/probe"
-
-// NewRouterWithPageProbeForTest is NewRouterWithProbeForTest with the
-// probe added to the /pages route group once SetupRoutes has built
-// it, so the probe runs behind that group's own middleware exactly as
-// the group's real routes do.
-func NewRouterWithPageProbeForTest(
-	log *slog.Logger,
-	cfg *config.Config,
-	mw *middleware.Middleware,
-	h *handlers.Handlers,
-	sentryEnabled bool,
-	probe http.HandlerFunc,
-) http.Handler {
-	s := &Server{
-		log:    log,
-		mw:     mw,
-		h:      h,
-		params: ServerParams{Config: cfg},
-	}
-	s.sentryEnabled.Store(sentryEnabled)
-	s.SetupRoutes()
-
-	for _, route := range s.router.Routes() {
-		pages, ok := route.SubRoutes.(chi.Router)
-		if ok && route.Pattern == "/pages/*" {
-			pages.Get("/probe", probe)
-		}
-	}
-
-	return s.router
-}

internal/server/routes.go

@@ -7,6 +7,7 @@ import (
 	sentryhttp "github.com/getsentry/sentry-go/http"
 	"github.com/go-chi/chi"
 	"github.com/go-chi/chi/middleware"
+	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"sneak.berlin/go/webhooker/static"
 )
 
@@ -14,10 +15,9 @@ import (
 // bytes) for form POST endpoints. 1 MB is generous for any form
 // submission while preventing abuse from oversized payloads.
 //
-// The four admin page route groups below (/pages, /user/{username},
-// /hooks and /hook/{sourceID}) install MaxBodySize(maxFormBodySize)
-// right after their recoverer and error reporting, ahead of both CSRF
-// and RequireAuth. Both orderings are deliberate.
+// Every route group below installs MaxBodySize(maxFormBodySize) as
+// its FIRST middleware, ahead of both CSRF and RequireAuth. Both
+// orderings are deliberate.
 //
 // Ahead of CSRF because gorilla/csrf parses the form. The cap has to
 // be installed before anything reads the body, or the parse runs
@@ -46,14 +46,6 @@ 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()
 }
@@ -77,33 +69,23 @@ func (s *Server) setupGlobalMiddleware() {
 	// Panic recovery, deliberately here rather than first. It has to
 	// run inside every middleware that observes the response, so the
 	// 500 it writes is the status the access log records and the
-	// metrics count, and outside the sentryhttp handler, whose
+	// metrics count, and outside the sentryhttp handler below, whose
 	// Repanic option needs something further out to catch what it
 	// re-raises. chi's own middleware.Recoverer held the first slot
 	// until it was measured: on a current Go release it crashes
 	// inside its stack pretty-printer instead of recovering, so the
 	// connection dropped and the original panic was never reported.
 	// See https://git.eeqj.de/sneak/webhooker/issues/187.
-	s.recoverPanics(s.router, nil)
-}
-
-// recoverPanics installs on r the recoverer, answering a panic with
-// page (a plain 500 when page is nil), and inside it the Sentry error
-// reporting (if SENTRY_DSN is set). Repanic is true so panics still
-// bubble up to the recoverer.
-//
-// Each admin page route group installs its own, with the error page,
-// as its first middleware. A panic there is logged, reported and
-// answered inside the group and never reaches the global recoverer,
-// which keeps the plain 500 for every other route.
-func (s *Server) recoverPanics(r chi.Router, page http.Handler) {
-	r.Use(s.mw.Recoverer(page))
+	s.router.Use(s.mw.Recoverer())
 
+	// Sentry error reporting (if SENTRY_DSN is set). Repanic is
+	// true so panics still bubble up to the Recoverer middleware
+	// registered immediately above.
 	if s.sentryEnabled.Load() {
 		sentryHandler := sentryhttp.New(sentryhttp.Options{
 			Repanic: true,
 		})
-		r.Use(sentryHandler.Handle)
+		s.router.Use(sentryHandler.Handle)
 	}
 }
 
@@ -148,7 +130,12 @@ func (s *Server) setupRoutes() {
 	if s.params.Config.MetricsAuthEnabled() {
 		s.router.Group(func(r chi.Router) {
 			r.Use(s.mw.MetricsAuth())
-			r.Get("/metrics", s.h.HandleMetrics())
+			r.Get(
+				"/metrics",
+				http.HandlerFunc(
+					promhttp.Handler().ServeHTTP,
+				),
+			)
 		})
 	}
 
@@ -160,13 +147,10 @@ func (s *Server) setupRoutes() {
 
 func (s *Server) setupPageRoutes() {
 	s.router.Route("/pages", func(r chi.Router) {
-		s.recoverPanics(
-			r, s.h.HandleErrorPage(http.StatusInternalServerError),
-		)
 		// 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(s.h.HandleErrorPage(http.StatusForbidden)))
+		r.Use(s.mw.CSRF())
 		r.Use(s.mw.NoCache())
 
 		// The login POST carries no pre-emptive rate limiter. Behind
@@ -185,13 +169,10 @@ func (s *Server) setupPageRoutes() {
 
 func (s *Server) setupUserRoutes() {
 	s.router.Route("/user/{username}", func(r chi.Router) {
-		s.recoverPanics(
-			r, s.h.HandleErrorPage(http.StatusInternalServerError),
-		)
 		// 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(s.h.HandleErrorPage(http.StatusForbidden)))
+		r.Use(s.mw.CSRF())
 		r.Use(s.mw.NoCache())
 		r.Use(s.mw.RequireAuth())
 		r.Get("/", s.h.HandleProfile())
@@ -203,13 +184,10 @@ func (s *Server) setupUserRoutes() {
 
 func (s *Server) setupSourceRoutes() {
 	s.router.Route("/hooks", func(r chi.Router) {
-		s.recoverPanics(
-			r, s.h.HandleErrorPage(http.StatusInternalServerError),
-		)
 		// 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(s.h.HandleErrorPage(http.StatusForbidden)))
+		r.Use(s.mw.CSRF())
 		r.Use(s.mw.NoCache())
 		r.Use(s.mw.RequireAuth())
 		r.Get("/", s.h.HandleSourceList())
@@ -218,13 +196,10 @@ func (s *Server) setupSourceRoutes() {
 	})
 
 	s.router.Route("/hook/{sourceID}", func(r chi.Router) {
-		s.recoverPanics(
-			r, s.h.HandleErrorPage(http.StatusInternalServerError),
-		)
 		// 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(s.h.HandleErrorPage(http.StatusForbidden)))
+		r.Use(s.mw.CSRF())
 		r.Use(s.mw.NoCache())
 		r.Use(s.mw.RequireAuth())
 		r.Get("/", s.h.HandleSourceDetail())

internal/server/routes_test.go

@@ -24,7 +24,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/handlers"
 	"sneak.berlin/go/webhooker/internal/healthcheck"
 	"sneak.berlin/go/webhooker/internal/logger"
-	"sneak.berlin/go/webhooker/internal/metrics"
 	"sneak.berlin/go/webhooker/internal/middleware"
 	"sneak.berlin/go/webhooker/internal/server"
 	"sneak.berlin/go/webhooker/internal/session"
@@ -114,8 +113,6 @@ func newTestEnvWithConfig(
 			session.New,
 			func() delivery.Notifier { return &noopNotifier{} },
 			func() delivery.WebhookEvictor { return &noopEvictor{} },
-			metrics.NewRegistry,
-			metrics.New,
 			middleware.New,
 			delivery.NewGuard,
 			handlers.New,
@@ -263,24 +260,6 @@ func (e *testEnv) urlFrom(
 	return html.UnescapeString(match[1])
 }
 
-// requireNotice requires w to redirect to dest carrying the notice
-// code, then renders that page and requires it to show text.
-func (e *testEnv) requireNotice(
-	t *testing.T,
-	w *httptest.ResponseRecorder,
-	dest, code, text string,
-	cookies []*http.Cookie,
-) {
-	t.Helper()
-
-	require.Equal(t, http.StatusSeeOther, w.Code)
-	require.Equal(t, dest+"?notice="+code, w.Header().Get("Location"))
-
-	page := e.get(w.Header().Get("Location"), cookies)
-	require.Equal(t, http.StatusOK, page.Code)
-	assert.Contains(t, page.Body.String(), text)
-}
-
 // authCookies forges an authenticated session for the given user.
 func (e *testEnv) authCookies(
 	t *testing.T,
@@ -759,31 +738,6 @@ func TestPagesLogin_ReturnsToTheRequestedPage(t *testing.T) {
 	assert.Equal(t, asked, w.Header().Get("Location"))
 }
 
-// TestPagesLogout_SaysSignedOut signs out with the navbar's form and
-// lands on the sign-in page, which says so.
-func TestPagesLogout_SaysSignedOut(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	userID, _ := env.seedUser(t, "leaver", "somepassword")
-	token, cookies := env.csrfFrom(
-		t, "/hooks", env.authCookies(t, userID, "leaver"),
-	)
-
-	form := url.Values{}
-	form.Set("csrf_token", token)
-
-	w := env.post(
-		env.urlFrom(t, "/hooks", `action="(/pages/logout)"`, cookies),
-		form, cookies,
-	)
-
-	// The sign-in page is requested without the session cookie, which
-	// the logout told the browser to delete.
-	env.requireNotice(t, w, "/pages/login", "signed-out", "Signed out.", nil)
-}
-
 // --- /user/{username} group ---
 
 // TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
@@ -901,9 +855,9 @@ func TestHooks_ListAndNewWebhookForm(t *testing.T) {
 	require.NoError(t,
 		env.db.DB().Where("name = ?", "created").First(&created).Error,
 	)
-	env.requireNotice(
-		t, w, "/hook/"+created.ID, "webhook-created", "Webhook created.",
-		cookies,
+	assert.Equal(
+		t, "/hook/"+created.ID, w.Header().Get("Location"),
+		"creating a webhook should redirect to its page",
 	)
 }
 
@@ -934,7 +888,8 @@ func TestHook_EditFormAndDelete(t *testing.T) {
 		env.urlFrom(t, editPage, `action="(/hook/[^/"]+/edit)"`, cookies),
 		form, cookies,
 	)
-	env.requireNotice(t, w, page, "webhook-saved", "Webhook saved.", cookies)
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(t, page, w.Header().Get("Location"))
 
 	var edited database.Webhook
 
@@ -948,17 +903,16 @@ func TestHook_EditFormAndDelete(t *testing.T) {
 		env.urlFrom(t, page, `action="(/hook/[^/"]+/delete)"`, cookies),
 		form, cookies,
 	)
-	env.requireNotice(
-		t, w, "/hooks", "webhook-deleted", "Webhook deleted.", cookies,
-	)
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(t, "/hooks", w.Header().Get("Location"))
 	assert.Equal(
 		t, http.StatusNotFound, env.get(page, cookies).Code,
 		"a deleted webhook's page should be gone",
 	)
 }
 
-// TestHook_EntrypointActions adds, deactivates, activates and deletes
-// an entrypoint with the forms on the webhook page, each submitted to
+// TestHook_EntrypointActions adds, deactivates and deletes an
+// entrypoint with the forms on the webhook page, each submitted to
 // the action and with the token the page rendered.
 func TestHook_EntrypointActions(t *testing.T) {
 	t.Parallel()
@@ -976,19 +930,16 @@ func TestHook_EntrypointActions(t *testing.T) {
 	form.Set("csrf_token", token)
 
 	// submit posts the webhook page's form whose action pattern
-	// captures, and requires the redirect back to that page with the
-	// notice code, and the page to show text.
-	submit := func(pattern, code, text string) {
+	// captures, and requires the redirect back to that page.
+	submit := func(pattern string) {
 		t.Helper()
 
 		w := env.post(env.urlFrom(t, page, pattern, cookies), form, cookies)
-		env.requireNotice(t, w, page, code, text, cookies)
+		require.Equal(t, http.StatusSeeOther, w.Code)
+		require.Equal(t, page, w.Header().Get("Location"))
 	}
 
-	toggle := `action="(/hook/[^/"]+/entrypoints/[^/"]+/toggle)"`
-
-	submit(`action="(/hook/[^/"]+/entrypoints)"`,
-		"entrypoint-added", "Entrypoint added.")
+	submit(`action="(/hook/[^/"]+/entrypoints)"`)
 
 	var added database.Entrypoint
 
@@ -997,7 +948,7 @@ func TestHook_EntrypointActions(t *testing.T) {
 	)
 	require.True(t, added.Active)
 
-	submit(toggle, "entrypoint-deactivated", "Entrypoint deactivated.")
+	submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/toggle)"`)
 
 	var toggled database.Entrypoint
 
@@ -1006,10 +957,7 @@ func TestHook_EntrypointActions(t *testing.T) {
 	)
 	assert.False(t, toggled.Active, "the toggle should deactivate it")
 
-	submit(toggle, "entrypoint-activated", "Entrypoint activated.")
-
-	submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/delete)"`,
-		"entrypoint-deleted", "Entrypoint deleted.")
+	submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/delete)"`)
 
 	var left int64
 
@@ -1020,8 +968,8 @@ func TestHook_EntrypointActions(t *testing.T) {
 
 // TestHook_TargetActions adds a target with the form on the webhook
 // page, follows its Edit link to the target edit form and submits
-// it, then deactivates, activates and deletes it, every URL and token
-// taken from the rendered pages.
+// it, then deactivates and deletes it, every URL and token taken from
+// the rendered pages.
 func TestHook_TargetActions(t *testing.T) {
 	t.Parallel()
 
@@ -1036,29 +984,27 @@ func TestHook_TargetActions(t *testing.T) {
 
 	// submit posts form, with the token, to the action pattern
 	// captures on the page at from, and requires the redirect back to
-	// the webhook page with the notice code, and that page to show
-	// text.
-	submit := func(from, pattern string, form url.Values, code, text string) {
+	// the webhook page.
+	submit := func(from, pattern string, form url.Values) {
 		t.Helper()
 
 		form.Set("csrf_token", token)
 
 		w := env.post(env.urlFrom(t, from, pattern, cookies), form, cookies)
-		env.requireNotice(t, w, page, code, text, cookies)
+		require.Equal(t, http.StatusSeeOther, w.Code)
+		require.Equal(t, page, w.Header().Get("Location"))
 	}
 
-	toggle := `action="(/hook/[^/"]+/targets/[^/"]+/toggle)"`
-
 	submit(page, `action="(/hook/[^/"]+/targets)"`, url.Values{
 		"name": {"added"},
 		"type": {string(database.TargetTypeLog)},
-	}, "target-added", "Target added.")
+	})
 
 	editPage := env.urlFrom(
 		t, page, `href="(/hook/[^/"]+/targets/[^/"]+/edit)"`, cookies,
 	)
 	submit(editPage, `action="(/hook/[^/"]+/targets/[^/"]+/edit)"`,
-		url.Values{"name": {"renamed"}}, "target-saved", "Target saved.")
+		url.Values{"name": {"renamed"}})
 
 	var edited database.Target
 
@@ -1068,8 +1014,8 @@ func TestHook_TargetActions(t *testing.T) {
 	assert.Equal(t, "renamed", edited.Name)
 	require.True(t, edited.Active)
 
-	submit(page, toggle, url.Values{},
-		"target-deactivated", "Target deactivated.")
+	submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/toggle)"`,
+		url.Values{})
 
 	var toggled database.Target
 
@@ -1078,11 +1024,8 @@ func TestHook_TargetActions(t *testing.T) {
 	)
 	assert.False(t, toggled.Active, "the toggle should deactivate it")
 
-	submit(page, toggle, url.Values{},
-		"target-activated", "Target activated.")
-
 	submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/delete)"`,
-		url.Values{}, "target-deleted", "Target deleted.")
+		url.Values{})
 
 	var left int64
 
@@ -1118,9 +1061,9 @@ func TestHook_ResubmitFromEventLog(t *testing.T) {
 		env.urlFrom(t, logsPath, `action="(/hook/[^"]+/resubmit)"`, cookies),
 		form, cookies,
 	)
-	env.requireNotice(
-		t, w, logsPath, "resubmit-no-targets",
-		"this source has no active targets", cookies,
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(
+		t, logsPath+"?resubmit=no-targets", w.Header().Get("Location"),
 	)
 
 	webhookDB, err := env.dbMgr.GetDB(wh.ID)
@@ -1356,8 +1299,10 @@ func TestDeliveryReplay_PostOnlyAndCSRFProtected(t *testing.T) {
 		html.UnescapeString(action[1]), form, cookies,
 	)
 
-	env.requireNotice(
-		t, w, logsPath, "replay-queued", "Replay queued:", cookies,
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(
+		t, logsPath+"?replay=queued",
+		w.Header().Get("Location"),
 	)
 	assert.Equal(
 		t, int64(2), env.countDeliveries(t, wh.ID),
@@ -1533,46 +1478,3 @@ func TestMetricsRouteUnmountedOnHalfSetConfig(t *testing.T) {
 		})
 	}
 }
-
-// TestTwoMetricsRoutersInOneProcess pins
-// https://git.eeqj.de/sneak/webhooker/issues/227: a second
-// metrics-enabled router in one process used to panic, because the
-// HTTP metrics registered on Prometheus's global default registry.
-// Two routers are built over separate dependency graphs and a third
-// over the first graph again, and each must still serve the HTTP,
-// delivery, Go runtime and process series, and the series counting
-// scrapes of /metrics itself.
-func TestTwoMetricsRoutersInOneProcess(t *testing.T) {
-	t.Parallel()
-
-	first := newTestEnvWithConfig(
-		t, metricsConfig(t, metricsUser, metricsAuthValue),
-	)
-	second := newTestEnvWithConfig(
-		t, metricsConfig(t, metricsUser, metricsAuthValue),
-	)
-	third := &testEnv{
-		router: server.NewRouterForTest(
-			first.log.Get(), first.cfg, first.mw, first.hnd,
-		),
-	}
-
-	for _, env := range []*testEnv{first, second, third} {
-		env.get("/", nil)
-
-		scrape := env.metricsRequest(metricsUser, metricsAuthValue)
-		require.Equal(t, http.StatusOK, scrape.Code)
-
-		for _, series := range []string{
-			"http_request_duration_seconds",
-			"http_response_size_bytes",
-			"http_requests_inflight",
-			"webhooker_events_received_total",
-			"go_goroutines",
-			"process_start_time_seconds",
-			"promhttp_metric_handler_requests_total",
-		} {
-			assert.Contains(t, scrape.Body.String(), series)
-		}
-	}
-}

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/assets

@@ -1,15 +1,16 @@
 #!/bin/sh
 # script/assets: extract Alpine.js from its npm package tarball, committed
-# in 3p/, to static/js/alpine.min.js, where go:embed reads it. The
-# extracted file is not committed. script/test, make build and make dev run
-# this first.
+# in 3p/, to static/js/alpine.min.js, where go:embed reads it. The package
+# is @alpinejs/csp, Alpine's build for pages whose Content-Security-Policy
+# forbids eval. The extracted file is not committed. script/test, make
+# build and make dev run this first.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    tar -xzOf 3p/alpinejs-3.14.9.tgz package/dist/cdn.min.js \
+    tar -xzOf 3p/alpinejs-csp-3.14.9.tgz package/dist/cdn.min.js \
         >static/js/alpine.min.js
 }
 

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/test

@@ -28,12 +28,10 @@
 # -race every test binary and every link costs a few hundred MB, so the
 # defaults (one per core) add up to several GB on a many-core host.
 #
-# The first run has no -v: go test then prints one result line per package,
-# with its coverage, and for a package that fails, everything its tests wrote,
-# application log lines included. Verbose output from the whole suite passes
-# the 2 MiB at which the Docker build cuts off each step's log, so on a failure
-# only the tests that failed run again, with -v. The script exits 1 after that
-# rerun whatever its result: the first run already showed the suite is broken.
+# No -v: the Docker build cuts each step's log off at 2 MiB, and verbose output
+# from the whole suite passes that before a failure is printed. Without it, go
+# test prints one result line per package and, for a package that fails,
+# everything its tests wrote, application log lines included.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -41,34 +39,7 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 main() {
     cd "$ROOT"
     "$ROOT/script/assets"
-
-    log="$(mktemp -t webhooker-test.XXXXXXXX)"
-    rcfile="$(mktemp -t webhooker-test-rc.XXXXXXXX)"
-    trap 'rm -f "$log" "$rcfile"' EXIT INT TERM
-
-    # The pipeline's status is tee's, and POSIX sh has no pipefail, so go
-    # test's status travels via a file. Output still streams live.
-    {
-        go test -race -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 \
-            && echo 0 >"$rcfile" || echo $? >"$rcfile"
-    } | tee "$log"
-    if [ "$(cat "$rcfile")" -eq 0 ]; then
-        return
-    fi
-
-    # go test reports a failed test as a line starting "--- FAIL: TestName"
-    # (a failed subtest's line is indented, and reruns with its parent), and
-    # a failed package as "FAIL<tab>package/path<tab>...". A failure that
-    # names no test, such as a build error or a timeout, is already shown in
-    # full above, so there is nothing to rerun.
-    tests="$(awk '/^--- FAIL: / { print $3 }' "$log" | paste -s -d '|' -)"
-    packages="$(awk '/^FAIL\t/ { print $2 }' "$log")"
-    if [ -n "$tests" ]; then
-        echo "--- Rerunning the failed tests with -v for details ---"
-        go test -race -v -p 4 -parallel 8 -timeout 90s \
-            -run "^($tests)\$" $packages || true
-    fi
-    exit 1
+    go test -race -p 4 -parallel 8 -timeout 90s ./...
 }
 
 main "$@"

script/test-browser

@@ -0,0 +1,23 @@
+#!/bin/sh
+# script/test-browser: run the browser test in internal/server. It runs in
+# Docker: Dockerfile.browser builds the test and runs it in a digest-pinned
+# headless browser image, so the host needs no browser.
+#
+# --no-cache-filter=browser runs the test again even when nothing changed;
+# it must name the stage in Dockerfile.browser that runs it.
+# --output=type=cacheonly leaves no image behind to clean up.
+set -eu
+
+ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
+
+main() {
+    cd "$ROOT"
+    docker build \
+        -f Dockerfile.browser \
+        --no-cache-filter=browser \
+        --progress=plain \
+        --output=type=cacheonly \
+        .
+}
+
+main "$@"

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

static/js/app.js

@@ -57,3 +57,73 @@
         init();
     }
 })();
+
+// Alpine.js components.
+//
+// The pages' Content-Security-Policy forbids eval, so the UI loads
+// Alpine's CSP build, which cannot run expressions written in the
+// markup: a directive in templates/ may only name a property or method,
+// and each x-data names a component registered here. This script runs
+// before Alpine, whose script tag is deferred, so this listener is in
+// place when Alpine starts.
+document.addEventListener("alpine:init", function () {
+    "use strict";
+
+    // Something a click shows and hides: the mobile menu, an add form,
+    // an event in the event log, a delivery's attempts.
+    window.Alpine.data("collapsible", function () {
+        return {
+            open: false,
+            toggle() {
+                this.open = !this.open;
+            },
+            get closed() {
+                return !this.open;
+            },
+            // Turns a downward caret up while open.
+            get caretClass() {
+                return { "rotate-180": this.open };
+            },
+        };
+    });
+
+    // The add target form. Only the chosen type's fields show, and the
+    // others are disabled so that the form does not submit them.
+    //
+    // The type is read from the type select when Alpine starts, when the
+    // select changes, and on pageshow. Going back to the page, the
+    // browser restores the type chosen before without a change event,
+    // in some browsers only after Alpine has started, but always before
+    // pageshow.
+    window.Alpine.data("targetForm", function () {
+        return {
+            targetType: "",
+            init() {
+                this.readType();
+            },
+            readType() {
+                this.targetType = this.$root.querySelector(
+                    'select[name="type"]'
+                ).value;
+            },
+            get isHttp() {
+                return this.targetType === "http";
+            },
+            get isSlack() {
+                return this.targetType === "slack";
+            },
+            get isDatabase() {
+                return this.targetType === "database";
+            },
+            get notHttp() {
+                return !this.isHttp;
+            },
+            get notSlack() {
+                return !this.isSlack;
+            },
+            get notDatabase() {
+                return !this.isDatabase;
+            },
+        };
+    });
+});

templates/base.html

@@ -7,7 +7,6 @@
 <body class="bg-gray-50 min-h-screen flex flex-col">
     <div class="flex-grow">
         {{template "navbar" .}}
-        {{template "notice" .}}
         {{block "content" .}}{{end}}
     </div>
     {{template "footer" .}}

templates/error.html

@@ -1,15 +0,0 @@
-{{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="/hooks" class="btn-secondary">Back to webhooks</a>
-    {{else}}
-    <a href="/pages/login" class="btn-primary">Sign in</a>
-    {{end}}
-</div>
-{{end}}

templates/navbar.html

@@ -1,5 +1,5 @@
 {{define "navbar"}}
-<nav class="app-bar" x-data="{ open: false }">
+<nav class="app-bar" x-data="collapsible">
     <div class="max-w-6xl mx-auto flex justify-between items-center">
         <div class="flex items-center gap-3">
             <a href="/" class="text-xl font-medium text-gray-900 hover:text-primary-600 transition-colors">Webhooker</a>
@@ -7,9 +7,9 @@
 
         <!-- Mobile menu button -->
         {{if .User}}
-        <button @click="open = !open" class="md:hidden p-2 rounded-md text-gray-500 hover:bg-gray-100">
+        <button @click="toggle" class="md:hidden p-2 rounded-md text-gray-500 hover:bg-gray-100">
             <svg class="w-6 h-6" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-                <path x-show="!open" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 6h16M4 12h16M4 18h16"/>
+                <path x-show="closed" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 6h16M4 12h16M4 18h16"/>
                 <path x-show="open" x-cloak stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M6 18L18 6M6 6l12 12"/>
             </svg>
         </button>
@@ -26,15 +26,11 @@
                 </svg>
                 {{.User.Username}}
             </a>
-            {{/* An error page can be served before a form token is issued,
-                 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}}
-            {{end}}
         </div>
     </div>
 
@@ -44,13 +40,11 @@
             {{if .User}}
             <a href="/hooks" 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}}
-            {{end}}
         </div>
     </div>
 </nav>

templates/notice.html

@@ -1,7 +0,0 @@
-{{define "notice"}}
-{{with .Notice}}
-<div class="max-w-6xl mx-auto px-6 pt-4">
-    <div class="{{if .Failed}}alert-error{{else}}alert-success{{end}}">{{.Text}}</div>
-</div>
-{{end}}
-{{end}}

templates/source_detail.html

@@ -3,14 +3,10 @@
 {{define "title"}}{{.Webhook.Name}} - Webhooker{{end}}
 
 {{define "content"}}
-<!-- 108rem, half again the 72rem (max-w-6xl) of the webhook list, the
-     event log, the navbar and the footer, so an entrypoint URL fits on
-     one line. An inline style, because the committed tailwind.css has
-     no class this wide. -->
-<div class="mx-auto px-6 py-8" style="max-width: 108rem" x-data="{ showAddEntrypoint: false, showAddTarget: false }">
+<div class="max-w-6xl mx-auto px-6 py-8">
     <div class="mb-6">
         <a href="/hooks" class="text-sm text-primary-600 hover:text-primary-700">&larr; Back to webhooks</a>
-        <div class="flex flex-wrap justify-between items-center gap-2 mt-2">
+        <div class="flex justify-between items-center mt-2">
             <div>
                 <h1 class="text-2xl font-medium text-gray-900">{{.Webhook.Name}}</h1>
                 {{if .Webhook.Description}}
@@ -32,10 +28,10 @@
 
     <div class="grid grid-cols-1 lg:grid-cols-2 gap-6">
         <!-- Entrypoints -->
-        <div class="card">
+        <div class="card" x-data="collapsible">
             <div class="p-4 border-b border-gray-200 flex justify-between items-center">
                 <h2 class="text-lg font-medium text-gray-900">Entrypoints</h2>
-                <button @click="showAddEntrypoint = !showAddEntrypoint" class="btn-text text-sm">
+                <button @click="toggle" class="btn-text text-sm">
                     <svg class="w-4 h-4 mr-1" fill="none" stroke="currentColor" viewBox="0 0 24 24">
                         <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 4v16m8-8H4"/>
                     </svg>
@@ -44,7 +40,7 @@
             </div>
 
             <!-- Add entrypoint form -->
-            <div x-show="showAddEntrypoint" x-cloak class="p-4 bg-gray-50 border-b border-gray-200">
+            <div x-show="open" x-cloak class="p-4 bg-gray-50 border-b border-gray-200">
                 <form method="POST" action="/hook/{{.Webhook.ID}}/entrypoints" class="flex gap-2">
                     <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
                     <input type="text" name="description" placeholder="Description (optional)" class="input text-sm flex-1">
@@ -91,10 +87,10 @@
         </div>
 
         <!-- Targets -->
-        <div class="card">
+        <div class="card" x-data="collapsible">
             <div class="p-4 border-b border-gray-200 flex justify-between items-center">
                 <h2 class="text-lg font-medium text-gray-900">Targets</h2>
-                <button @click="showAddTarget = !showAddTarget" class="btn-text text-sm">
+                <button @click="toggle" class="btn-text text-sm">
                     <svg class="w-4 h-4 mr-1" fill="none" stroke="currentColor" viewBox="0 0 24 24">
                         <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 4v16m8-8H4"/>
                     </svg>
@@ -103,42 +99,42 @@
             </div>
 
             <!-- Add target form -->
-            <div x-show="showAddTarget" x-cloak class="p-4 bg-gray-50 border-b border-gray-200">
-                <form method="POST" action="/hook/{{.Webhook.ID}}/targets" x-data="{ targetType: 'http' }" class="space-y-3">
+            <div x-show="open" x-cloak class="p-4 bg-gray-50 border-b border-gray-200">
+                <form method="POST" action="/hook/{{.Webhook.ID}}/targets" x-data="targetForm" @pageshow.window="readType" class="space-y-3">
                     <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
                     <div class="flex gap-2">
                         <input type="text" name="name" placeholder="Target name" required class="input text-sm flex-1">
-                        <select name="type" x-model="targetType" class="input text-sm w-32">
+                        <select name="type" @change="readType" class="input text-sm w-32">
                             <option value="http">HTTP</option>
                             <option value="slack">Slack</option>
                             <option value="database">Database</option>
                             <option value="log">Log</option>
                         </select>
                     </div>
-                    <div x-show="targetType === 'http'">
-                        <input type="url" name="url" placeholder="https://example.com/webhook" :disabled="targetType !== 'http'" class="input text-sm">
+                    <div x-show="isHttp">
+                        <input type="url" name="url" placeholder="https://example.com/webhook" :disabled="notHttp" class="input text-sm">
                     </div>
-                    <div x-show="targetType === 'http'">
-                        <textarea name="headers" rows="3" placeholder="Authorization: Bearer ..." :disabled="targetType !== 'http'" class="input text-sm"></textarea>
+                    <div x-show="isHttp">
+                        <textarea name="headers" rows="3" placeholder="Authorization: Bearer ..." :disabled="notHttp" class="input text-sm"></textarea>
                         <p class="text-xs text-gray-500 mt-1">Optional request headers, one <code>Name: value</code> per line, sent with every delivery.</p>
                     </div>
-                    <div x-show="targetType === 'http'" class="flex gap-2 items-center">
+                    <div x-show="isHttp" class="flex gap-2 items-center">
                         <label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
-                        <input type="number" name="timeout" min="0" max="300" :disabled="targetType !== 'http'" class="input text-sm w-24">
+                        <input type="number" name="timeout" min="0" max="300" :disabled="notHttp" class="input text-sm w-24">
                     </div>
-                    <div x-show="targetType === 'http'">
+                    <div x-show="isHttp">
                         <div class="flex gap-2 items-center">
                             <label class="text-sm text-gray-700">Max retries:</label>
                             <input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
                         </div>
                         <p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
                     </div>
-                    <div x-show="targetType === 'slack'">
-                        <input type="url" name="url" placeholder="https://hooks.slack.com/services/..." :disabled="targetType !== 'slack'" class="input text-sm">
+                    <div x-show="isSlack">
+                        <input type="url" name="url" placeholder="https://hooks.slack.com/services/..." :disabled="notSlack" class="input text-sm">
                         <p class="text-xs text-gray-500 mt-1">Slack or Mattermost incoming webhook URL. Payloads are pretty-printed in code blocks.</p>
                     </div>
-                    <div x-show="targetType === 'database'">
-                        <input type="text" name="expiry" placeholder="never" :disabled="targetType !== 'database'" class="input text-sm">
+                    <div x-show="isDatabase">
+                        <input type="text" name="expiry" placeholder="never" :disabled="notDatabase" class="input text-sm">
                         <p class="text-xs text-gray-500 mt-1">Archive expiry: "never" (default) keeps rows forever, or a duration like "720h" prunes older rows.</p>
                     </div>
                     <button type="submit" class="btn-primary text-sm">Add Target</button>

templates/source_logs.html

@@ -12,11 +12,19 @@
         </div>
     </div>
 
+    {{if .ReplayMessage}}
+    <div class="{{if .ReplayQueued}}alert-success{{else}}alert-error{{end}}">{{.ReplayMessage}}</div>
+    {{end}}
+
+    {{if .ResubmitMessage}}
+    <div class="{{if .ResubmitQueued}}alert-success{{else}}alert-error{{end}}">{{.ResubmitMessage}}</div>
+    {{end}}
+
     <div class="card">
         <div class="divide-y divide-gray-100">
             {{range .Events}}
-            <div class="p-4" x-data="{ open: false }">
-                <div class="flex items-center justify-between cursor-pointer" @click="open = !open">
+            <div class="p-4" x-data="collapsible">
+                <div class="flex items-center justify-between cursor-pointer" @click="toggle">
                     <div class="flex items-center gap-3">
                         <span class="badge-info">{{.Method}}</span>
                         <span class="text-sm font-mono text-gray-700">{{.ID}}</span>
@@ -35,7 +43,7 @@
                         </span>
                         {{end}}
                         <span class="text-xs text-gray-400">{{.CreatedAt.Format "2006-01-02 15:04:05"}}</span>
-                        <svg class="w-4 h-4 text-gray-400 transition-transform" :class="{ 'rotate-180': open }" fill="none" stroke="currentColor" viewBox="0 0 24 24">
+                        <svg class="w-4 h-4 text-gray-400 transition-transform" :class="caretClass" fill="none" stroke="currentColor" viewBox="0 0 24 24">
                             <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M19 9l-7 7-7-7"/>
                         </svg>
                     </div>
@@ -63,8 +71,8 @@
                         <h3 class="text-xs font-medium uppercase tracking-wide text-gray-500">Deliveries</h3>
                         <div class="mt-2 divide-y divide-gray-200">
                             {{range .Deliveries}}
-                            <div class="py-2" x-data="{ attempts: false }">
-                                <div class="flex items-center justify-between cursor-pointer" @click="attempts = !attempts">
+                            <div class="py-2" x-data="collapsible">
+                                <div class="flex items-center justify-between cursor-pointer" @click="toggle">
                                     <div class="flex items-center gap-3">
                                         <span class="text-sm text-gray-700">{{.Target.DisplayName}}</span>
                                         <span class="text-xs {{if eq .Status "delivered"}}text-green-600{{else if eq .Status "failed"}}text-red-600{{else if eq .Status "retrying"}}text-yellow-600{{else}}text-gray-400{{end}}">{{.Status}}</span>
@@ -78,13 +86,13 @@
                                         </form>
                                         {{end}}
                                         <span class="text-xs text-gray-400">{{.AttemptCount}} attempt{{if ne .AttemptCount 1}}s{{end}}</span>
-                                        <svg class="w-3 h-3 text-gray-400 transition-transform" :class="{ 'rotate-180': attempts }" fill="none" stroke="currentColor" viewBox="0 0 24 24">
+                                        <svg class="w-3 h-3 text-gray-400 transition-transform" :class="caretClass" fill="none" stroke="currentColor" viewBox="0 0 24 24">
                                             <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M19 9l-7 7-7-7"/>
                                         </svg>
                                     </div>
                                 </div>
 
-                                <div x-show="attempts" x-cloak class="mt-2 space-y-2">
+                                <div x-show="open" x-cloak class="mt-2 space-y-2">
                                     {{if .AttemptsOmitted}}
                                     <p class="text-xs text-gray-500">{{.AttemptsOmitted}} attempt{{if ne .AttemptsOmitted 1}}s{{end}} omitted between the first and last shown.</p>
                                     {{end}}