Derive the image's version from the .git in the build context (closes #366)

A plain docker build, as upaas runs it, passed no VERSION build arg and
had no .git, so every such image stamped "unknown". .dockerignore now
sends .git and every tracked file (an excluded one would read as
deleted and mark the version -dirty), and the VERSION build arg loses
its "unknown" default, so script/version derives the version inside the
build. A VERSION build arg still takes precedence.

The builder stage installs git and fails when its context carries .git
and the version still comes out "unknown".

Model: opus-5-5

.dockerignore

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

.gitea/workflows/check.yml

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

Dockerfile

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

Makefile

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

README.md

@@ -142,7 +142,7 @@ TTY detection, and security headers are always applied.
 | `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
 | `SESSION_IDLE_TIMEOUT`  | Idle session timeout (Go duration)  | `24h`    |
 | `RECEIVER_RATE_LIMIT`   | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
-| `TRUSTED_PROXIES`       | CIDRs whose forwarded headers are trusted. A set value replaces the default. If any client can reach webhooker, or the proxy in front of it, from an RFC 1918 source address, set it to the proxy's address alone. See [Trusted proxies](#trusted-proxies) | `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16` (RFC 1918) |
+| `TRUSTED_PROXIES`       | CIDRs whose forwarded headers are trusted (unset: all clients behind a proxy share one rate-limit bucket; a correct login password is never throttled either way) | `""` (none) |
 | `ALLOWED_EGRESS_CIDRS`  | CIDRs that delivery targets may reach despite the SSRF blocklist. Read [Allowing egress to your own network](#allowing-egress-to-your-own-network) before setting it | `""` (none) |
 
 #### Allowing egress to your own network
@@ -157,21 +157,6 @@ public cloud metadata addresses: currently only `168.63.129.16`, Azure's
 WireServer, which serves an Azure VM its credentials. Because it is a
 public address, listing it in `ALLOWED_EGRESS_CIDRS` reopens it.
 
-That is all the default blocklist covers: the IPv4 private and reserved
-ranges; of IPv6, only loopback (`::1`), unique local addresses
-(`fc00::/7`) and link-local addresses (`fe80::/10`); and certain public
-addresses. 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. A provider's other public
-addresses are not refused. IBM Cloud, for example, serves its package
-mirrors, time servers and object storage on `161.26.0.0/16`, and the
-private endpoints of its own cloud services on `166.8.0.0/14`. Neither
-range hands out credentials that way: the token service among those
-endpoints issues a token only in exchange for something the caller
-presents, such as an API key. Reaching these services can be a
-legitimate delivery, and every cloud has some, so a partial list would
-promise coverage it does not give.
-
 That default is also inconvenient for the thing webhooker is mostly
 for: taking a public webhook and forwarding it to something on your own
 network. A container on the same Docker network, a box on `10.x`, a
@@ -389,37 +374,41 @@ unlocked.
 `TRUSTED_PROXIES` is a comma-separated list of CIDR blocks (a bare
 address such as `192.168.1.7` is accepted and treated as a single
 host), for example `192.168.1.7, 2001:db8::5`. It decides whose
-`X-Forwarded-For` header the rate limiters believe, so it should cover
+`X-Forwarded-For` header the rate limiters believe, so it should name
-the addresses of your reverse proxies.
+the addresses of your reverse proxies and nothing else.
 
 `X-Forwarded-For` is honoured **only** when the connecting peer is
 inside one of these blocks; for every other peer the client identity is
-the connection's own address and the header is ignored. Unset (or
+the connection's own address and the header is ignored. The default is
-empty), the list is the RFC 1918 private ranges: `10.0.0.0/8`,
+the empty list, which trusts nobody — anything else would let any
-`172.16.0.0/12` and `192.168.0.0/16`. A set value replaces the default
+client pick its own rate limit bucket, minting a fresh one per request
-entirely. A set but unparseable value aborts startup.
+or draining someone else's. Set it to the address of your reverse
+proxy, and to nothing wider. A set but unparseable value aborts
+startup.
 
-If any client can reach webhooker, or the proxy in front of it, from an
+That default is safe against forged headers, but leaving it unset in
-RFC 1918 source address (directly, or through anything that can
+production has a cost you must know about. Production runs behind a
-rewrite source addresses, such as NAT or a published container port),
+TLS-terminating reverse proxy, so with `TRUSTED_PROXIES` unset every
-set `TRUSTED_PROXIES` to the proxy's address alone, or every rate
+request keys on the proxy's own address and all clients share a single
-limit, the webhook receiver's included, can be bypassed by those
+bucket per limit. The receiver limits become service-wide ceilings,
-clients. The address to set is the `remoteIP` field of the
+and the login endpoint's failure counting collapses onto one key, so a
-`http request` log line for a request that came through the proxy.
+stranger's wrong passwords throttle every other client's wrong
-
+passwords.
-Behind a proxy the list does not cover, every request keys on the
-proxy's own address and all clients share a single bucket per limit.
-The receiver limits become service-wide ceilings, and the login
-endpoint's failure counting collapses onto one key, so a stranger's
-wrong passwords throttle every other client's wrong passwords. Set
-`TRUSTED_PROXIES` to that proxy's address to restore per-client
-buckets.
 
 What it cannot do is lock the operator out. The login endpoint
 verifies credentials **before** it consults any limit and charges only
 failures, so a correct password is never throttled no matter how full
 the bucket is. See [Rate Limiting](#rate-limiting).
 
+The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's
+address, which restores per-client buckets. webhooker logs a warning
+at startup whenever `TRUSTED_PROXIES` is empty, in every environment,
+because behind a proxy every client shares one bucket in `dev` and
+`prod` alike. The warning is informational when nothing proxies to the
+process: with no proxy in front, the peer address is the client's own
+and the buckets are already per-client. See
+[Rate Limiting](#rate-limiting) for what each limit shares.
+
 `X-Real-IP` and `True-Client-IP` are **never** read, from any peer.
 Reverse proxies append to `X-Forwarded-For` but forward other client
 headers verbatim, so a single-valued header is client-controlled even
@@ -435,10 +424,20 @@ instead, since past such an entry the chain is not the shape assumed
 here. The peer address is likewise used when the header is absent or
 every hop in it is a trusted proxy.
 
-Your proxy must therefore **append** the peer address to
+Two operator requirements follow:
-`X-Forwarded-For` (nginx `$proxy_add_x_forwarded_for`, HAProxy
+
-`option forwardfor`, Caddy and AWS ALB by default), and must append a
+- Your proxy must **append** the peer address to `X-Forwarded-For`
-bare address with no port.
+  (nginx `$proxy_add_x_forwarded_for`, HAProxy `option forwardfor`,
+  Caddy and AWS ALB by default), and must append a bare address with
+  no port.
+- List proxy hosts **only**. Any address inside `TRUSTED_PROXIES`
+  chooses its own rate-limit key: its `X-Forwarded-For` is walked, so
+  it can name a different address on every request to get a fresh
+  bucket each time, or name another client's address to drain that
+  client's bucket. Never list a block that also covers clients — a
+  broad `10.0.0.0/8` on a network where clients live in the same range
+  makes all three limits, including the unauthenticated webhook
+  receiver, silently bypassable by every client in the block.
 
 #### Sessions
 
@@ -737,15 +736,10 @@ repository's `Dockerfile` and runs it. The app needs:
 - **Volume:** one host directory mounted at `/var/lib/webhooker`.
 - **Environment variables:**
     - `WEBHOOKER_ENVIRONMENT=prod`
-    - `TRUSTED_PROXIES`: unset, it is the RFC 1918 ranges. Set it to
+    - `TRUSTED_PROXIES`: your reverse proxy's address on that Docker
-      your reverse proxy's address alone if that address is outside
+      network. The `remoteIP` field of the `http request` log line for a
-      those ranges, or if any client can reach webhooker, or the proxy,
+      request that came through the proxy shows it; the health check's
-      from an RFC 1918 source address (directly, or through anything
+      own lines show `::1`. See [Trusted proxies](#trusted-proxies).
-      that can rewrite source addresses, such as NAT or a published
-      container port). The `remoteIP` field of the `http request` log
-      line for a request that came through the proxy shows that
-      address; the health check's own lines show `::1`. See
-      [Trusted proxies](#trusted-proxies).
     - Leave `BIND_ADDRESS` and `DATA_DIR` unset: the image sets
       `BIND_ADDRESS` to `0.0.0.0`, and `DATA_DIR` defaults to
       `/var/lib/webhooker`.
@@ -808,16 +802,12 @@ reports.
    behind a proxy means the `X-Forwarded-Proto` header. The block below
    sets it; without it every request is read as plaintext and cookies
    ship without `Secure`. See [Configuration](#configuration).
-3. **Make sure `TRUSTED_PROXIES` covers the proxy's address.** For a
+3. **Set `TRUSTED_PROXIES` to the proxy's address.** Unset, every rate
-   proxy it does not cover, every rate limiter keys on the proxy, so
+   limiter keys on the connecting peer, which behind a proxy is the
-   all clients share one bucket per limit. Unset, the list is the RFC
+   proxy on every request: all clients collapse into one global bucket
-   1918 ranges, which do not cover a proxy that reaches the binary
+   per limit and the receiver's per-IP limits become service-wide
-   itself over loopback (the binary bound to `127.0.0.1`). With the
+   ceilings. See [Trusted proxies](#trusted-proxies). List the proxy
-   image, the address to check is the `remoteIP` field of the
+   and nothing else.
-   `http request` log line for a request that came through the proxy.
-   If any client can reach webhooker, or the proxy, from an RFC 1918
-   source address, set the list to the proxy's address alone. See
-   [Trusted proxies](#trusted-proxies).
 4. **Send `Host` as `$http_host`, not `$host`.** `$host` strips the
    port. webhooker's Origin/Referer check compares against the host it
    was given, so on any port other than 443 `$host` makes every form
@@ -1133,13 +1123,21 @@ build itself.
 | Uncommitted changes | the above with a `-dirty` suffix |
 | No git metadata | `unknown` |
 
-`unknown` is what a source tarball or a `docker build .` with no
+The image derives it the same way, from the `.git` that the build
-`--build-arg VERSION=...` reports. `.dockerignore` excludes `.git/`, so
+context carries, so any `docker build .` of a clone stamps the commit it
-the build context carries no git metadata and the image cannot derive
+was built from; a shallow clone of one branch has no tags and stamps the
-the version itself: `script/docker` (and so `make docker`) resolves it
+short SHA. `.dockerignore` must therefore leave out neither `.git` nor
-on the host and passes it in as the `VERSION` build arg. A build that
+any tracked file, which git in the build would see as deleted, marking
-reports `unknown` is a build nobody told what it was; it is not a
+the version `-dirty`. A `VERSION` build arg (`--build-arg VERSION=...`)
-failure, but it cannot be traced back to a commit.
+takes precedence; `script/docker` (and so `make docker`) passes the one
+`script/version` resolves on the host. The image build fails if its
+context carries `.git` and the version still comes out `unknown`, which
+means git in the build could not read the checkout.
+
+`unknown` is what a source tarball, or a `docker build` with no `.git`
+in its context and no `VERSION` build arg, reports. A build that reports
+`unknown` is a build nobody told what it was; it is not a failure, but
+it cannot be traced back to a commit.
 
 `make version` prints what the current checkout would stamp, and
 `make build VERSION=v1.2.3` overrides it. An empty override — from
@@ -1370,11 +1368,10 @@ It uses:
 - **[go-chi/httprate](https://github.com/go-chi/httprate)** for
   sliding-window rate limiting of the password-change and webhook
   receiver endpoints. The bucket is per client IP only when
-  `TRUSTED_PROXIES` covers the reverse proxy (by default it covers the
+  `TRUSTED_PROXIES` names the reverse proxy; unset, every client
-  RFC 1918 private ranges); otherwise every client behind that proxy
+  behind that proxy shares one bucket per limit. The login endpoint
-  shares one bucket per limit. The login endpoint counts failed
+  counts failed attempts itself instead, so that a correct password is
-  attempts itself instead, so that a correct password is never
+  never throttled (see [Rate Limiting](#rate-limiting))
-  throttled (see [Rate Limiting](#rate-limiting))
 - **[Prometheus](https://prometheus.io)** for metrics, served at
   `/metrics` behind basic auth
 - **[Sentry](https://sentry.io)** for optional error reporting
@@ -2545,44 +2542,47 @@ the tree is checked out: four checkouts have reported 3,959, 3,961,
 client-supplied field was cut, and that the shipped chain's stack
 arrived uncut — never the numbers.
 
-Every limiter here — receiver, login, password change, delivery replay
+Every limiter here — receiver, login, and password change — identifies
-and event resubmit — identifies the client the same way, through one
+the client the same way, through one shared key function: the
-shared key function: the connection's own address, unless the peer is
+connection's own address, unless the peer is listed in
-inside `TRUSTED_PROXIES`, in which case the forwarded client address is
+`TRUSTED_PROXIES`, in which case the forwarded client address is used
-used instead. That address becomes a bucket by family: IPv4 keys on
+instead. That address becomes a bucket by family: IPv4 keys on the full
-the full address, IPv6 on its `/64` prefix. A routed `/64` is the normal
+address, IPv6 on its `/64` prefix. A routed `/64` is the normal
 residential and mobile IPv6 allocation, so keying IPv6 per address would
 let one subscriber rotate source addresses and mint a fresh bucket per
 request, evading these limits at the network layer without spoofing
 anything; the cost is that distinct clients inside one `/64` share a
 bucket. IPv4-mapped addresses (`::ffff:1.2.3.4`) key as the IPv4 address
-they carry. See [Trusted proxies](#trusted-proxies). When that variable
+they carry. See [Trusted proxies](#trusted-proxies). Deployed without that
-does not cover the reverse proxy, a client behind it shares one bucket
+variable set, a client behind a reverse proxy shares one bucket with
-with every other client behind the same proxy. Set `TRUSTED_PROXIES` to
+every other client behind the same proxy. Set `TRUSTED_PROXIES` to the
-the proxy's address to get per-client limits back. What the shared bucket
+proxy's address to get per-client limits back. What the shared bucket
 costs is not the same for every limiter, and the two cases pull in
 opposite directions:
 
 - For the **receiver** limits it costs throughput, which is the safe
   direction to be wrong in: sharing can only make a limit bind sooner,
   never let a sender past it. It matters more for the aggregate limit
-  than for the per-entrypoint one: with every request keyed on the
+  than for the per-entrypoint one: with `TRUSTED_PROXIES` unset behind
-  proxy, the aggregate limit becomes a service-wide ceiling of 1200
+  the reverse proxy a production deployment is required to run behind,
-  requests per minute across all senders and all entrypoints, where the
+  every request keys on the proxy, so the aggregate limit becomes a
-  per-entrypoint limit's capacity still grows with the number of
+  service-wide ceiling of 1200 requests per minute across all senders
-  entrypoints.
+  and all entrypoints, where the per-entrypoint limit's capacity still
+  grows with the number of entrypoints. Any deployment with more than a
+  handful of busy entrypoints must set `TRUSTED_PROXIES`.
 - For the **login and password-change** limits it costs precision, not
   availability. Login failures from every client land in one counter,
   so a stranger's wrong passwords make the operator's own wrong
   passwords answer `429` sooner; the operator's _correct_ password is
-  never affected, because it is never counted.
+  never affected, because it is never counted. Production deployments
+  should still set `TRUSTED_PROXIES`; webhooker warns at startup
+  whenever it is empty, in any environment.
 
 #### The login endpoint
 
 The login `POST` is the one endpoint with no pre-emptive limiter in
 front of it, and that is deliberate. A limiter that spends budget on
-arrival is a lockout wherever clients share one bucket, as they do
+arrival is a lockout in this deployment shape: sharing one bucket, a
-behind a reverse proxy that `TRUSTED_PROXIES` does not cover: a
 stranger sending five POSTs a minute — about 0.08 requests per second,
 from anywhere — keeps it permanently full, and the operator has no
 second administrative path. So the handler inverts the order:
@@ -2679,10 +2679,8 @@ re-fills both verification slots on its first two requests. The
 remedies are to block the source at the reverse proxy, or to
 rate-limit `POST /pages/login` there — the one place a limit can be
 applied without reintroducing the lockout, because the proxy sees the
-real client address. `TRUSTED_PROXIES` does not stop the saturation.
+real client address. Setting `TRUSTED_PROXIES` does not stop the
-The flood's source is in the proxy's access log: webhooker's own logs
+saturation, but it makes the source visible in the failure logs.
-record the proxy's address, not the client's (see
-[Deployment behind a reverse proxy](#deployment-behind-a-reverse-proxy)).
 
 Finer-grained per-webhook rate limits (configured in the web UI and
 enforced in the webhook handler) can layer on top of this env-level
@@ -2932,12 +2930,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/*`, `/sources`,
-`/source/*`) 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/*`, `/sources`,
 `/source/*`) apply a **MaxBodySize** middleware that limits
 POST/PUT/PATCH request bodies to 1 MB. It is registered ahead of the
@@ -3046,9 +3038,10 @@ check, see [The login endpoint](#the-login-endpoint).
   It runs behind session auth, so only a client already holding a
   valid session reaches it, and an operator throttled out of changing
   a password can still log in. The bucket is per client IP only when
-  `TRUSTED_PROXIES` covers the reverse proxy; otherwise every client
+  `TRUSTED_PROXIES` names the reverse proxy; unset, every client
   shares one bucket, which costs precision rather than availability
-  (see [Rate Limiting](#rate-limiting))
+  (see [Rate Limiting](#rate-limiting)). webhooker warns at startup
+  whenever `TRUSTED_PROXIES` is empty
 - Prometheus metrics behind basic auth
 - Static assets embedded in binary (no filesystem access needed at
   runtime)
@@ -3180,8 +3173,9 @@ 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 arrives as the `VERSION` build arg,
+   stamps the version. The version is the `VERSION` build arg if one is
-   since the context has no `.git` (see
+   given, otherwise derived from the `.git` in the context, and the
+   stage fails if a context with `.git` would stamp `unknown` (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`
@@ -3213,16 +3207,17 @@ A layer cache lets `docker build .` exit 0 in seconds with the lint and
 test stages replayed rather than executed, which would make a green
 check meaningless. The `check` workflow therefore writes
 `.ci-fingerprint` into the build context before building. Its value is
-the hash of the last commit that touched the build context, so:
+the hash of the last commit that touched anything other than `*.md`,
+`LICENSE` and `.editorconfig`, so:
 
 - Any commit that changes code (including a squash merge whose tree
   matches an already-built branch) gets a new fingerprint, invalidates
   the `COPY . .` layer of both check stages, and really runs
   `make fmt-check`, `golangci-lint`, `make test`, and `make build`. A
   run that reports success ran them.
-- A docs-only commit leaves the fingerprint unchanged — `.dockerignore`
+- A docs-only commit leaves the fingerprint unchanged, but it still
-  excludes `*.md`, `LICENSE` and `.editorconfig` from the context
+  rebuilds in full: the context also carries `.git`, which changes with
-  anyway — so the image replays from cache and costs seconds.
+  every commit (see [Version stamping](#version-stamping)).
 
 The module download layer sits above `COPY . .` and stays cached either
 way.

TODO.md

@@ -387,7 +387,7 @@ point of the branch.
 - 2026-03-05 security headers middleware, session regeneration on
   login, request body size limits (#41)
 - 2026-03-04 tests for delivery, middleware, and session packages
-  (#32); removed the build-architecture global (#31)
+  (#32); removed globals.Buildarch (#31)
 - 2026-03-04 1.0 MVP merge: Webhook/Entrypoint/Target rename, core
   delivery engine with bounded worker pool and circuit breaker,
   parallel fan-out, per-webhook event databases, management UI (#16)

internal/config/config.go

@@ -75,11 +75,6 @@ const (
 	// internet-exposed endpoint.
 	defaultReceiverRateLimit = 120
 
-	// defaultTrustedProxies is TRUSTED_PROXIES when it is unset: the
-	// RFC 1918 private ranges, which a reverse proxy reaching the
-	// process over a Docker network or a private LAN connects from.
-	defaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
-
 	// maxPort is the highest valid TCP port number. The lower
 	// bound (at least 1) is enforced by envPositiveInt.
 	maxPort = 65535
@@ -177,14 +172,13 @@ type Config struct {
 
 	// TrustedProxies is the set of networks whose members are
 	// allowed to speak for the client with X-Forwarded-For, the
-	// only forwarded header read. Unless TRUSTED_PROXIES is set it
+	// only forwarded header read. It is empty unless
-	// is the RFC 1918 private ranges (defaultTrustedProxies); a set
+	// TRUSTED_PROXIES is set, and empty means no peer is
-	// value replaces them. If any client can reach the process, or
+	// trusted: forwarded headers are then ignored entirely and
-	// the proxy in front of it, from an RFC 1918 source address
+	// clients are identified by the connection's own address.
-	// (directly, or through anything that can rewrite source
+	// Members can choose their own rate-limit key, so this must
-	// addresses, such as NAT or a published container port), it
+	// name proxy hosts only, never a block that also covers
-	// must be set to the proxy's address alone, or every rate limit
+	// clients.
-	// can be bypassed by those clients.
 	TrustedProxies []netip.Prefix
 
 	// AllowedEgressCIDRs is the set of networks a delivery target
@@ -466,15 +460,14 @@ func parseCIDR(entry string) (netip.Prefix, error) {
 
 // envPrefixList returns the value of the named environment variable
 // parsed as a comma-separated list of CIDR blocks (bare addresses
-// allowed). An unset, empty, or blank value is read as defaultValue
+// allowed). An unset, empty, or blank value yields an empty list. A
-// instead. A set value containing an unparseable entry is a hard
+// set value containing an unparseable entry is a hard error naming
-// error naming the key and the bad entry, so startup fails loudly
+// the key and the bad entry, so startup fails loudly rather than
-// rather than silently running with a list the operator did not
+// silently running with a list the operator did not intend.
-// intend.
+func envPrefixList(key string) ([]netip.Prefix, error) {
-func envPrefixList(key, defaultValue string) ([]netip.Prefix, error) {
 	v := strings.TrimSpace(os.Getenv(key))
 	if v == "" {
-		v = defaultValue
+		return nil, nil
 	}
 
 	var prefixes []netip.Prefix
@@ -688,12 +681,12 @@ func loadFromEnv() (*Config, error) {
 		return nil, err
 	}
 
-	trustedProxies, err := envPrefixList("TRUSTED_PROXIES", defaultTrustedProxies)
+	trustedProxies, err := envPrefixList("TRUSTED_PROXIES")
 	if err != nil {
 		return nil, err
 	}
 
-	allowedEgressCIDRs, err := envPrefixList("ALLOWED_EGRESS_CIDRS", "")
+	allowedEgressCIDRs, err := envPrefixList("ALLOWED_EGRESS_CIDRS")
 	if err != nil {
 		return nil, err
 	}
@@ -767,6 +760,50 @@ func (c *Config) warnEgressAllowlist(log *slog.Logger) {
 	)
 }
 
+// warnSharedRateLimitBucket logs a startup warning whenever
+// TRUSTED_PROXIES is empty, in any environment.
+//
+// With no trusted proxies every rate limiter keys on the connecting
+// peer's address. Whether that is harmless or dangerous depends on
+// what is in front of the process, which this code cannot observe:
+// with nothing in front, the peer is the client and the limits are
+// per-client as intended; behind a reverse proxy the peer is the proxy
+// for every request, so all clients share one bucket per limiter.
+//
+// The login endpoint no longer spends budget on arrival — it verifies
+// credentials first and charges only failures — so a shared bucket
+// cannot deny the operator a correct password. What it does collapse
+// is the failure counting: one client's wrong passwords throttle
+// everyone else's wrong passwords, and the receiver's limits become
+// service-wide ceilings.
+//
+// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT:
+// behind a proxy every client shares one bucket in dev and prod alike.
+//
+// The default of trusting nobody is deliberate — trusting forwarded
+// headers from arbitrary peers lets any client choose its own bucket —
+// so this warns rather than failing startup or changing the key.
+func (c *Config) warnSharedRateLimitBucket(log *slog.Logger) {
+	if len(c.TrustedProxies) > 0 {
+		return
+	}
+
+	log.Warn(
+		"TRUSTED_PROXIES is empty: every rate limit keys on the "+
+			"connecting peer's address. With nothing proxying to "+
+			"this process that is the client itself and the limits "+
+			"are per-client as intended. Behind a reverse proxy the "+
+			"peer is the proxy on every request, so all clients "+
+			"share one bucket per limit: the receiver limits become "+
+			"service-wide ceilings, and one client's failed logins "+
+			"throttle every other client's failed logins — a "+
+			"correct password still gets in. If anything proxies to "+
+			"this process, set TRUSTED_PROXIES to its address.",
+		"environment", c.Environment,
+		"trustedProxies", len(c.TrustedProxies),
+	)
+}
+
 // New creates a Config by reading environment variables.
 //
 //nolint:revive // lc parameter is required by fx even if unused.
@@ -812,6 +849,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
 		"hasMetricsAuth", s.MetricsAuthEnabled(),
 	)
 
+	s.warnSharedRateLimitBucket(log)
 	s.warnEgressAllowlist(log)
 
 	return s, nil

internal/config/config_test.go

@@ -551,11 +551,6 @@ func testReceiverRateLimitSuccess(
 }
 
 func TestTrustedProxies(t *testing.T) {
-	// Unset, the RFC 1918 private ranges are trusted, so a reverse
-	// proxy on a Docker network or a private LAN is covered without
-	// configuration.
-	defaultProxies := []string{cidrPrivateV4, "172.16.0.0/12", "192.168.0.0/16"}
-
 	tests := []struct {
 		name        string
 		set         bool
@@ -564,21 +559,18 @@ func TestTrustedProxies(t *testing.T) {
 		expected    []string
 	}{
 		{
+			// The default must be "trust nobody": an empty list
+			// means forwarded headers are ignored, never that
+			// every peer may speak for the client.
 			name:     caseUnsetUsesDefault,
 			set:      false,
-			expected: defaultProxies,
+			expected: []string{},
 		},
 		{
-			name:     "blank value uses default",
+			name:     "blank value trusts nothing",
 			set:      true,
 			value:    "  ",
-			expected: defaultProxies,
+			expected: []string{},
-		},
-		{
-			name:     "set value replaces the default entirely",
-			set:      true,
-			value:    "203.0.113.7",
-			expected: []string{"203.0.113.7/32"},
 		},
 		{
 			name:  caseValidValueParsed,
@@ -853,6 +845,101 @@ func TestEgressAllowlistWarning(t *testing.T) {
 	}
 }
 
+// TestSharedRateLimitBucketWarning covers the startup warning that
+// tells an operator a deployment behind a reverse proxy shares one
+// rate-limit bucket between every client, which turns the receiver
+// limits into service-wide ceilings and collapses login failure
+// counting. It must fire whenever TRUSTED_PROXIES is empty, in any
+// environment, because behind a proxy every client shares one bucket
+// in dev and prod alike. It stays quiet once proxies are named.
+func TestSharedRateLimitBucketWarning(t *testing.T) {
+	tests := []struct {
+		name           string
+		environment    string
+		trustedProxies string
+		expectWarning  bool
+	}{
+		{
+			name:          "prod without trusted proxies warns",
+			environment:   config.EnvironmentProd,
+			expectWarning: true,
+		},
+		{
+			name:           "prod with trusted proxies is quiet",
+			environment:    config.EnvironmentProd,
+			trustedProxies: cidrPrivateV4,
+			expectWarning:  false,
+		},
+		{
+			name:          "dev without trusted proxies warns",
+			environment:   config.EnvironmentDev,
+			expectWarning: true,
+		},
+		{
+			name:           "dev with trusted proxies is quiet",
+			environment:    config.EnvironmentDev,
+			trustedProxies: cidrPrivateV4,
+			expectWarning:  false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Cannot use t.Parallel() here because t.Setenv
+			// is incompatible with parallel subtests.
+			t.Setenv("WEBHOOKER_ENVIRONMENT", tt.environment)
+
+			if tt.trustedProxies == "" {
+				require.NoError(
+					t, os.Unsetenv("TRUSTED_PROXIES"),
+				)
+			} else {
+				t.Setenv("TRUSTED_PROXIES", tt.trustedProxies)
+			}
+
+			var buf bytes.Buffer
+
+			log := slog.New(slog.NewJSONHandler(
+				&buf, &slog.HandlerOptions{
+					Level: slog.LevelDebug,
+				},
+			))
+
+			require.NoError(
+				t,
+				config.WarnSharedRateLimitBucketForTest(log),
+			)
+
+			if !tt.expectWarning {
+				assert.Empty(t, buf.String())
+
+				return
+			}
+
+			logged := buf.String()
+
+			assert.Contains(t, logged, `"level":"WARN"`)
+			assert.Contains(t, logged, "TRUSTED_PROXIES")
+			assert.Contains(t, logged, "share one bucket")
+			assert.Contains(
+				t, logged, "throttle every other client's failed logins",
+			)
+			// The warning must not claim a lockout the login
+			// endpoint no longer permits: credentials are verified
+			// before any budget is spent.
+			assert.Contains(
+				t, logged, "a correct password still gets in",
+			)
+			// The text must stay accurate for a developer with
+			// nothing in front of the process, where an empty
+			// list costs nothing.
+			assert.Contains(
+				t, logged, "nothing proxying to this process",
+			)
+		})
+	}
+}
+
 // metricsEnv describes what one subtest below puts in the
 // environment for a single METRICS_ variable. A variable that is
 // set to the empty string and one that is not set at all are

internal/config/export_test.go

@@ -6,6 +6,21 @@ import "log/slog"
 // the external config_test package so each helper can be covered by
 // its own table-driven test without weakening the package API.
 
+// WarnSharedRateLimitBucketForTest loads a Config from the current
+// environment and emits its startup warnings to log. The real logger
+// writes to stdout, so this lets the warning's firing condition be
+// asserted against a handler the test controls.
+func WarnSharedRateLimitBucketForTest(log *slog.Logger) error {
+	c, err := loadFromEnv()
+	if err != nil {
+		return err
+	}
+
+	c.warnSharedRateLimitBucket(log)
+
+	return nil
+}
+
 // WarnEgressAllowlistForTest loads a Config from the current
 // environment and emits its egress-allowlist startup warning to
 // log, so a test can assert both that the warning fires only when

internal/delivery/ssrf.go

@@ -43,13 +43,6 @@ var (
 // 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. 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 blockedNetworks []*net.IPNet
 

internal/handlers/auth.go

@@ -36,7 +36,7 @@ func (h *Handlers) HandleLoginSubmit() http.HandlerFunc {
 		err := r.ParseForm()
 		if err != nil {
 			h.log.Error("failed to parse form", "error", err)
-			h.renderError(w, r, http.StatusBadRequest)
+			http.Error(w, "Bad request", http.StatusBadRequest)
 
 			return
 		}
@@ -103,10 +103,9 @@ func (h *Handlers) renderLoginError(
 // The credential check runs BEFORE any rate-limit budget is
 // consulted, and only a failed check spends budget. That is what
 // keeps the single administrative path reachable: behind the reverse
-// proxy this deployment requires, when TRUSTED_PROXIES does not cover
+// proxy this deployment requires, with TRUSTED_PROXIES unset, every
-// it, every client shares one bucket, so a limiter spent on arrival
+// client shares one bucket, so a limiter spent on arrival lets any
-// lets any stranger deny the operator's own correct password
+// stranger deny the operator's own correct password indefinitely.
-// indefinitely.
 //
 // Verifying first means every login POST costs an Argon2id hash, so
 // the work is taken under a bounded number of verification slots.
@@ -166,7 +165,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
 	}
@@ -238,14 +241,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
 	}
@@ -254,7 +267,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
 	}

internal/handlers/auth_test.go

@@ -25,7 +25,7 @@ const (
 
 	// sharedProxyPeer is the whole point of this file. Production is
 	// required to run behind a TLS-terminating reverse proxy, and
-	// when TRUSTED_PROXIES does not cover it every client — attacker
+	// TRUSTED_PROXIES defaults to empty, so every client — attacker
 	// and operator alike — reaches the process from the proxy's
 	// address and shares one rate-limit bucket. Both parties in
 	// these tests therefore use the same RemoteAddr.
@@ -115,11 +115,11 @@ func floodFailures(
 // done-criterion of https://git.eeqj.de/sneak/webhooker/issues/150.
 //
 // The attacker and the operator share one rate-limit bucket, because
-// behind the mandated reverse proxy, when TRUSTED_PROXIES does not
+// behind the mandated reverse proxy with TRUSTED_PROXIES unset every
-// cover it, every client keys on the proxy's address. The attacker
+// client keys on the proxy's address. The attacker floods the
-// floods the operator's own username — a single-admin product has a
+// operator's own username — a single-admin product has a predictable
-// predictable one — far past the failure limit. The operator must
+// one — far past the failure limit. The operator must still be able
-// still be able to log in with the correct password.
+// to log in with the correct password.
 //
 // This fails if credentials stop being verified ahead of the limiter.
 func TestLogin_StrangersFloodCannotLockOutTheOperator(t *testing.T) {

internal/handlers/delivery_replay.go

@@ -105,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
 		}
@@ -122,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
 	}
@@ -171,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
 	}
@@ -193,7 +195,7 @@ 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
@@ -210,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
 	}
@@ -220,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

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

@@ -99,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
 		}
@@ -120,20 +120,20 @@ func (h *Handlers) resubmitEvent(
 	// alphabet rather than from the request.
 	eventID, err := uuid.Parse(chi.URLParam(r, "eventID"))
 	if err != nil {
-		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
 	}
@@ -147,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
 	}
@@ -155,7 +155,7 @@ func (h *Handlers) resubmitEvent(
 	// A miss is a 404 whether the event was reaped, belongs to
 	// another webhook, or never existed.
 	if !found {
-		h.renderError(w, r, http.StatusNotFound)
+		http.NotFound(w, r)
 
 		return
 	}
@@ -207,7 +207,7 @@ func (h *Handlers) queueResubmit(
 	// inactive one is skipped rather than refused.
 	targets, err := h.loadActiveTargets(webhook.ID)
 	if err != nil {
-		h.serverError(w, r, "failed to query targets", err)
+		h.serverError(w, "failed to query targets", err)
 
 		return
 	}
@@ -225,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
 	}

internal/handlers/export_test.go

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

@@ -135,7 +135,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{
@@ -147,16 +146,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,
@@ -174,76 +163,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)
+	http.Error(
-}
+		w, "Internal server error",
-
+		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")
-
-	data := s.pageData(r, map[string]any{
-		"Status":     status,
-		"StatusText": http.StatusText(status),
-		"Message":    errorPageText(status),
-	})
-
-	var buf bytes.Buffer
-
-	err := s.templates["error.html"].Execute(&buf, data)
-	if err != nil {
-		s.log.Error("failed to render error page", "error", err)
-		http.Error(w, http.StatusText(status), status)
-
-		return
-	}
-
-	w.Header().Set("Content-Type", "text/html; charset=utf-8")
-	w.WriteHeader(status)
-
-	_, err = buf.WriteTo(w)
-	if err != nil {
-		s.log.Error("failed to write error page", "error", err)
-	}
-}
-
-// errorPageText is the line the error page shows for status. It is
-// fixed per status, so the page tells the reader no more than the
-// plain-text answers it replaced did.
-func errorPageText(status int) string {
-	switch status {
-	case http.StatusBadRequest:
-		return "The request could not be read."
-	case http.StatusForbidden:
-		return "The request was refused. If it came from a form " +
-			"left open for a long time, reload the page and try " +
-			"again."
-	case http.StatusNotFound:
-		return "There is nothing here. It may have been deleted, " +
-			"or the address may be wrong."
-	case http.StatusServiceUnavailable:
-		return "The server is busy. Please try again in a moment."
-	default: // http.StatusInternalServerError
-		return "Something went wrong on the server. Please try " +
-			"again."
-	}
 }
 
 // UserInfo represents user information for templates
@@ -296,17 +224,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))
-}
-
-// pageData adds the fields the shared layout renders to a page's own
-// data.
-func (s *Handlers) pageData(r *http.Request, data any) any {
 	userInfo := s.getUserInfo(r)
 	csrfToken := middleware.CSRFToken(r)
 
@@ -320,16 +245,19 @@ func (s *Handlers) pageData(r *http.Request, data any) any {
 		m["User"] = userInfo
 		m["CSRFToken"] = csrfToken
 		m["Version"] = version
+		s.executeTemplate(w, tmpl, m)
 
-		return m
+		return
 	}
 
-	return templateDataWrapper{
+	wrapper := templateDataWrapper{
 		User:      userInfo,
 		CSRFToken: csrfToken,
 		Version:   version,
 		Data:      data,
 	}
+
+	s.executeTemplate(w, tmpl, wrapper)
 }
 
 // executeTemplate renders the template into a buffer and writes to
@@ -342,7 +270,6 @@ func (s *Handlers) pageData(r *http.Request, data any) any {
 // this reason.
 func (s *Handlers) executeTemplate(
 	w http.ResponseWriter,
-	r *http.Request,
 	tmpl *template.Template,
 	data any,
 ) {
@@ -353,7 +280,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

@@ -307,14 +307,10 @@ func TestRenderTemplateMidRenderErrorSendsNoPartialBody(t *testing.T) {
 		t, http.StatusInternalServerError, w.Code,
 		"a failed render must report a 500",
 	)
-	assert.NotContains(
+	assert.Equal(
-		t, w.Body.String(), partialPageMarker,
+		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/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
+// to display on the profile page. On an internal failure it writes a
-// error page itself and returns handled=false, signalling the caller
+// 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)
-
-	app := newTestApp(t, &log, &cfg, &sess, &h)
 	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_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
 	}
@@ -380,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
 		}
@@ -412,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
 		}
@@ -421,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
 		}
@@ -474,7 +482,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
 		}
@@ -509,7 +517,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
 		}
@@ -518,7 +526,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
 		}
@@ -572,7 +582,7 @@ 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
 	}
@@ -602,7 +612,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
 		}
@@ -629,7 +639,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
 	}
@@ -655,7 +665,7 @@ func (h *Handlers) deleteWebhookResources(
 		// redirecting as though everything succeeded: the file
 		// needs removing by hand, and the logged error names it.
 		h.serverError(
-			w, r, "failed to delete webhook event database", err,
+			w, "failed to delete webhook event database", err,
 		)
 
 		return
@@ -799,7 +809,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
 	}
@@ -821,7 +831,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
 		}
@@ -829,7 +839,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
@@ -939,7 +949,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,
@@ -953,7 +962,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
@@ -990,7 +999,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
@@ -999,7 +1008,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
@@ -1222,7 +1231,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
 		}
@@ -1231,7 +1240,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
 		}
@@ -1247,7 +1258,7 @@ 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
 		}
@@ -1278,7 +1289,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
 		}
@@ -1287,7 +1298,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
 		}
@@ -1358,7 +1371,7 @@ 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
 	}
@@ -1452,7 +1465,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:
@@ -1502,7 +1515,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,
@@ -1524,7 +1537,7 @@ func (h *Handlers) buildSlackTargetConfig(
 		return "", err
 	}
 
-	return h.marshalTargetConfig(w, r, delivery.SlackTargetConfig{
+	return marshalTargetConfig(w, delivery.SlackTargetConfig{
 		WebhookURL: targetURL,
 	})
 }
@@ -1578,14 +1591,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
 	}
@@ -1601,7 +1616,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)
@@ -1620,8 +1634,8 @@ func (h *Handlers) buildDatabaseTargetConfig(
 		return "", err
 	}
 
-	return h.marshalTargetConfig(
+	return marshalTargetConfig(
-		w, r, map[string]any{"expiry": expiry},
+		w, map[string]any{"expiry": expiry},
 	)
 }
 
@@ -1675,7 +1689,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
 		}
@@ -1685,7 +1699,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
 		}
@@ -1775,14 +1793,18 @@ 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
 		}
 
 		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
 		}

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,7 +157,7 @@ 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
 	}
@@ -218,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/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/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(
+func (m *Middleware) CSRF() func(http.Handler) http.Handler {
-	forbidden http.Handler,
-) func(http.Handler) http.Handler {
 	csrfErrorHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// CSRF is registered ahead of RequireAuth on every route
 		// group that uses it, so this WARN is reachable by an
@@ -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/logbound_test.go

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

internal/middleware/loginguard.go

@@ -108,10 +108,10 @@ type failureWindow struct {
 //
 // A limiter that spends budget on arrival cannot protect a
 // single-admin product: behind the reverse proxy the deployment
-// requires, when TRUSTED_PROXIES does not cover it, every client
+// requires, with TRUSTED_PROXIES unset, every client keys on the
-// keys on the proxy, so a stranger trickling five POSTs a minute
+// proxy, so a stranger trickling five POSTs a minute keeps the one
-// keeps the one bucket full and the operator's own correct password
+// bucket full and the operator's own correct password is answered 429
-// is answered 429 forever. There is no second administrative path.
+// forever. There is no second administrative path.
 //
 // So budget is spent only by a FAILED verification. A correct
 // password is never throttled, whatever the counters say, which is

internal/middleware/ratelimit.go

@@ -123,8 +123,9 @@ func bucketKey(addr netip.Addr) string {
 	return prefix.String()
 }
 
-// isTrustedProxy reports whether addr belongs to a network in
+// isTrustedProxy reports whether addr belongs to a network the
-// TRUSTED_PROXIES, which by default is the RFC 1918 private ranges.
+// operator listed in TRUSTED_PROXIES. The list is empty by default,
+// so by default nothing is trusted.
 func (m *Middleware) isTrustedProxy(addr netip.Addr) bool {
 	for _, prefix := range m.params.Config.TrustedProxies {
 		if prefix.Contains(addr) {

internal/middleware/ratelimit_test.go

@@ -384,8 +384,8 @@ const (
 	// trustedProxyCIDR is the proxy network the forwarded-path
 	// tests configure, and trustedPeer an address inside it. A
 	// production deployment is required to run behind a reverse
-	// proxy that TRUSTED_PROXIES covers, either by the default or by
+	// proxy with TRUSTED_PROXIES set, so this is the shape the
-	// a set value, so this is the shape the bucketing has to hold in.
+	// bucketing has to hold in.
 	trustedProxyCIDR = "10.0.0.0/8"
 	trustedPeer      = "10.0.0.1:44444"
 )
@@ -426,8 +426,8 @@ func assertSharedBucket(
 }
 
 // TestRateLimitKey_SpoofedForwardedFromUntrustedPeer is the test
-// this gating exists for: from a peer that is not a trusted
+// this gating exists for: with no trusted proxies configured (the
-// proxy, a client that rotates a forwarded header on every
+// default), a client that rotates a forwarded header on every
 // request must stay in one bucket. If forwarded headers were
 // trusted unconditionally, each spoofed value would mint a fresh
 // bucket and the limit would stop no one.
@@ -1097,9 +1097,8 @@ func TestPostRateLimit_IPv4IndependentPerAddress(t *testing.T) {
 // that arrives from trustedPeer — a configured trusted proxy — and
 // names forwarded as its client in X-Forwarded-For. That is the
 // production path: a deployment is required to run behind a reverse
-// proxy that TRUSTED_PROXIES covers, either by the default or by a
+// proxy with TRUSTED_PROXIES set, so the forwarded address, not the
-// set value, so the forwarded address, not the peer, is what the
+// peer, is what the limiters bucket on there.
-// limiters bucket on there.
 func forwardedKeyFor(
 	t *testing.T, m *middleware.Middleware, forwarded string,
 ) string {
@@ -1179,9 +1178,9 @@ func TestRateLimitKey_ForwardedIPv6BucketsByPrefix(t *testing.T) {
 //
 // Every existing test of this fallback uses an IPv4 proxy, where
 // bucketKey is the identity function, so replacing the call with
-// peer.String() leaves the whole suite green. Only addresses inside
+// peer.String() leaves the whole suite green. Only operator-listed
-// TRUSTED_PROXIES reach this line and the fallback is fail-closed, so
+// addresses reach this line and the fallback is fail-closed, so this
-// this pins behaviour rather than fixing a defect.
+// pins behaviour rather than fixing a defect.
 func TestRateLimitKey_TrustedPeerUnusableForwardedMasksPeer(
 	t *testing.T,
 ) {

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
+// connection.
-// when it is nil or when page panics before writing anything.
 //
 // 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
+// the client a credential; every other header is left to http.Error.
-// answers, every other header the handler set goes out with it, apart
-// from any page sets itself; otherwise they are left to http.Error.
 // See https://git.eeqj.de/sneak/webhooker/issues/193.
-func (s *Middleware) Recoverer(
+func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
-	page http.Handler,
-) 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/server/error_page_test.go

@@ -1,220 +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="/sources" class="btn-secondary">` +
-		`Back to webhooks</a>`
-	backToSignIn = `<a href="/pages/login" class="btn-primary">` +
-		`Sign in</a>`
-)
-
-// assertErrorPage checks that w is the error page for status, in the
-// normal layout, offering link.
-func assertErrorPage(
-	t *testing.T,
-	w *httptest.ResponseRecorder,
-	status int,
-	link string,
-) {
-	t.Helper()
-
-	body := w.Body.String()
-
-	assert.Equal(t, status, w.Code)
-	assert.Equal(
-		t, "text/html; charset=utf-8", w.Header().Get("Content-Type"),
-	)
-	assert.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("/source/"+wh.ID, cookies)
-
-	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
-}
-
-func TestErrorPage_DeletedTarget(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	userID, _ := env.seedUser(t, "owner", "somepassword")
-	cookies := env.authCookies(t, userID, "owner")
-
-	wh := env.seedWebhook(t, userID)
-	tgt := env.seedTarget(t, wh.ID)
-	require.NoError(t, env.db.DB().Delete(tgt).Error)
-
-	w := env.get(
-		"/source/"+wh.ID+"/targets/"+tgt.ID+"/edit", cookies,
-	)
-
-	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
-}
-
-func TestErrorPage_UnknownPath(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	userID, _ := env.seedUser(t, "owner", "somepassword")
-	cookies := env.authCookies(t, userID, "owner")
-
-	assertErrorPage(
-		t, env.get("/no-such-page", nil),
-		http.StatusNotFound, backToSignIn,
-	)
-
-	// Outside every route group there is no form token, so the
-	// page leaves out the logout form rather than offer one that
-	// would be refused.
-	w := env.get("/no-such-page", cookies)
-	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
-	assert.NotContains(t, w.Body.String(), `action="/pages/logout"`)
-
-	// Inside a route group the page has a token, and logout works.
-	wh := env.seedWebhook(t, userID)
-	w = env.get("/source/"+wh.ID+"/no-such-page", cookies)
-	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
-	assert.Contains(t, w.Body.String(), `action="/pages/logout"`)
-}
-
-func TestErrorPage_BadCSRFToken(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	form := url.Values{}
-	form.Set("username", "someone")
-	form.Set("password", "irrelevant")
-	form.Set("csrf_token", "not-a-token")
-
-	assertErrorPage(
-		t, env.post("/pages/login", form, nil),
-		http.StatusForbidden, backToSignIn,
-	)
-
-	userID, _ := env.seedUser(t, "owner", "somepassword")
-	cookies := env.authCookies(t, userID, "owner")
-	wh := env.seedWebhook(t, userID)
-
-	edit := url.Values{}
-	edit.Set("name", "renamed")
-
-	assertErrorPage(
-		t, env.post("/source/"+wh.ID+"/edit", edit, cookies),
-		http.StatusForbidden, backToWebhooks,
-	)
-}
-
-// TestErrorPage_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("/webhook/no-such-entrypoint", url.Values{}, nil)
-
-	assert.Equal(t, http.StatusNotFound, w.Code)
-	assert.Equal(t, "404 page not found\n", w.Body.String())
-}

internal/server/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

@@ -15,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},
+// Every route group below installs MaxBodySize(maxFormBodySize) as
-// /sources and /source/{sourceID}) install MaxBodySize(maxFormBodySize)
+// its FIRST middleware, ahead of both CSRF and RequireAuth. Both
-// right after their recoverer and error reporting, ahead of both CSRF
+// orderings are deliberate.
-// 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
@@ -47,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()
 }
@@ -78,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)
+	s.router.Use(s.mw.Recoverer())
-}
-
-// 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))
 
+	// 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)
 	}
 }
 
@@ -166,22 +147,18 @@ 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
-		// the reverse proxy production requires, when TRUSTED_PROXIES
+		// the reverse proxy production requires, with TRUSTED_PROXIES
-		// does not cover it, every client shares one bucket, so a
+		// unset, every client shares one bucket, so a limiter spent
-		// limiter spent on arrival lets any stranger deny the operator
+		// on arrival lets any stranger deny the operator the only
-		// the only administrative path. The handler verifies
+		// administrative path. The handler verifies credentials first
-		// credentials first and charges only failures; see
+		// and charges only failures; see Handlers.authenticateUser.
-		// Handlers.authenticateUser.
 		r.Get("/login", s.h.HandleLoginPage())
 		r.Post("/login", s.h.HandleLoginSubmit())
 
@@ -191,13 +168,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())
@@ -209,13 +183,10 @@ func (s *Server) setupUserRoutes() {
 
 func (s *Server) setupSourceRoutes() {
 	s.router.Route("/sources", 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())
@@ -224,13 +195,10 @@ func (s *Server) setupSourceRoutes() {
 	})
 
 	s.router.Route("/source/{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/versionscript/version_script_test.go

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

script/docker

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

script/version

@@ -7,18 +7,16 @@
 #
 # Order of precedence:
 #
-#   1. $VERSION, if set and non-empty. This is how the value reaches a
+#   1. $VERSION, if set and non-empty: an explicit value, such as the
-#      build that cannot derive it: .dockerignore excludes .git/, so the
+#      Dockerfile's VERSION build arg.
-#      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 `docker build .` with no --build-arg. That
+#      source tarball, or a `docker build` with no .git in its context
-#      case must not fail the build and must not name a tag the tree may
+#      and no VERSION build arg. That case must not fail the build and
-#      not be at, so it names nothing.
+#      must not name a tag the tree may not be at, so it names nothing.
 #
 # The git step insists the enclosing repository is this checkout, not
 # merely some repository above it: an unpacked tarball sitting inside an

templates/error.html

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

templates/navbar.html

@@ -24,14 +24,10 @@
                 </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}}
             {{else}}
             <a href="/pages/login" class="btn-primary">Login</a>
             {{end}}
@@ -44,12 +40,10 @@
             {{if .User}}
             <a href="/sources" class="btn-text w-full text-left">Webhooks</a>
             <a href="/user/{{.User.Username}}" class="btn-text w-full text-left">Profile</a>
-            {{if .CSRFToken}}
             <form method="POST" action="/pages/logout">
                 <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
                 <button type="submit" class="btn-text w-full text-left">Logout</button>
             </form>
-            {{end}}
             {{else}}
             <a href="/pages/login" class="btn-primary w-full">Login</a>
             {{end}}