Add a Download button that exports a database target's archive as gzipped JSON (closes #374)

Each database target on the webhook page links to
/hook/ID/targets/TARGETID/download, which streams the target's archive
as archive-WEBHOOK-TARGET-YYYYMMDDTHHMMSSZ.json.gz: the webhook, the
target, exported_at, and archived_events, one object per row keyed by
column, a body that is not valid UTF-8 in base64 with body_encoding.
The export reads the rows through one cursor on its own connection
inside a read-only transaction: one snapshot, and no write lock. A
download runs for as long as the client keeps reading, and one that
fails after it has started aborts the connection. The handler holds the
rename lock only while it reads the names and opens the file. A missing
archive exports empty and is not created.

Model: opus-5-5

.gitea/workflows/check.yml

@@ -33,5 +33,5 @@ jobs:
         # report success from cache.
         run: git rev-parse HEAD > .ci-fingerprint
 
-      - name: Build Docker image (runs make check)
+      - name: Build Docker image (runs make fmt-check, golangci-lint, make test, make build)
         run: script/cibuild

Dockerfile

@@ -26,7 +26,7 @@ COPY . .
 # Dockerfile.lint, including --network=none (see its header for why).
 RUN make fmt-check
 RUN --network=none golangci-lint config verify --config .golangci.yml
-RUN --network=none golangci-lint run --config .golangci.yml ./...
+RUN --network=none golangci-lint run --config .golangci.yml --build-tags browser ./...
 
 # Build stage
 # golang:1.26.1-bookworm (Debian-based), 2026-03-17

Dockerfile.browser

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

Dockerfile.lint

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

Makefile

@@ -1,4 +1,4 @@
-.PHONY: bootstrap setup assets test lint fmt fmt-check check build run dev deps docker clean hooks css version
+.PHONY: bootstrap setup assets test test-browser lint fmt fmt-check check build run dev deps docker clean hooks css version
 
 # Default target
 .DEFAULT_GOAL := check
@@ -33,6 +33,9 @@ assets:
 test:
 	@script/test
 
+test-browser:
+	@script/test-browser
+
 lint:
 	@script/lint
 

README.md

@@ -19,8 +19,8 @@ before deploying one.
 ### Prerequisites
 
 - Go 1.26.1+ (the version in `go.mod`)
-- Docker (for linting, for the test stage of the CI gate, and for
-  containerized deployment)
+- Docker (for `make lint` and so for `make check`, for the browser test in
+  `make test-browser`, for the CI gate, and for containerized deployment)
 
 golangci-lint is not a prerequisite and must not be installed on the
 host: `script/bootstrap` does not install it, and `make lint` runs the
@@ -58,6 +58,7 @@ make fmt        # Format code (gofmt + goimports)
 make fmt-check  # Fail if gofmt would change anything (writes nothing)
 make lint       # Run golangci-lint in Docker (Dockerfile.lint)
 make test       # Run tests with race detection
+make test-browser # Run the browser test in Docker (Dockerfile.browser)
 make check      # test + lint + fmt-check (CI gate)
 make build      # Build binary to bin/webhooker (version-stamped)
 make version    # Print the version this checkout would stamp
@@ -456,6 +457,19 @@ Your proxy must therefore **append** the peer address to
 `option forwardfor`, Caddy and AWS ALB by default), and must append a
 bare address with no port.
 
+Every log line that names a client carries two addresses: `remoteIP`,
+the connecting peer, which behind a proxy is the proxy; and `clientIP`,
+the client the rate limiters identify by the rules above, which is the
+field to read when tracing who sent what. Those lines are the
+`http request` access log line, the rate-limit rejection lines
+(`login failure limit exceeded` among them), the
+`csrf: token validation failed` warning and the receiver's
+`webhook request received` line. `clientIP` is only as trustworthy as
+`TRUSTED_PROXIES`: for a request from a peer inside the list, it is
+read out of the `X-Forwarded-For` that peer sent, so a peer that does
+not belong in the list can make it name any address it likes. For a
+request from any other peer, both fields name the peer.
+
 #### Sessions
 
 Sessions are bounded by two independent clocks, and end at whichever
@@ -840,9 +854,9 @@ reports.
    was given, so on any port other than 443 `$host` makes every form
    POST — including login — fail with `403 origin invalid`, with
    nothing in the error naming the cause.
-5. **Keep the proxy's access log.** webhooker's own access log records
-   the peer address, which behind a proxy is always the proxy. The
-   proxy's log is the only record of which client sent what. nginx's
+5. **Keep the proxy's access log.** webhooker's own access log names
+   the client in its `clientIP` field only while `TRUSTED_PROXIES`
+   covers the proxy; the proxy's log names it regardless. nginx's
    default `combined` format already logs `$remote_addr`; do not
    replace it with one that drops the client address, and retain those
    logs as long as you would want to answer a question about traffic.
@@ -871,9 +885,8 @@ server {
     # webhooker's message.
     client_max_body_size 1m;
 
-    # $remote_addr is the client. webhooker's own log records this
-    # proxy and nothing else, so this file is the only place the
-    # client's address is written down.
+    # $remote_addr is the client. webhooker's own log names it, as
+    # clientIP, only while TRUSTED_PROXIES covers this proxy.
     access_log /var/log/nginx/webhooker.access.log combined;
 
     location / {
@@ -1257,7 +1270,7 @@ What that means for an operator:
 This repository adheres to the
 [Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all)
 standard: normalized scripts in `script/` are the entrypoints for the
-development workflow. Ten of the Makefile's seventeen targets are thin
+development workflow. Eleven of the Makefile's eighteen targets are thin
 shims that call them; `build`, `run`, `dev`, `deps`, `clean`, `css` and
 `version` are inline commands with no script behind them, though `build`,
 `run` and `dev` first run `script/assets`, and `build` and `version` both
@@ -1278,6 +1291,8 @@ We provide:
 - `script/assets` — extract Alpine.js from its tarball in `3p/` (see
   [Third-party browser assets](#third-party-browser-assets))
 - `script/test` — run the test suite
+- `script/test-browser` — run the browser test in Docker (see
+  [Third-party browser assets](#third-party-browser-assets))
 - `script/lint` — run golangci-lint in Docker (see Linting below)
 - `script/fmt` — format all code (writes)
 - `script/fmt-check` — check formatting (read-only)
@@ -1298,11 +1313,32 @@ We provide:
 
 ## Third-party browser assets
 
-The web UI serves one third-party script, Alpine.js. Its npm package tarball
-is committed as `3p/alpinejs-3.14.9.tgz`, byte for byte as the npm registry
-publishes it. It is a dependency, not this repo's build output, so
-`REPO_POLICIES.md`'s rule against committed build artifacts does not apply.
-The directory is `3p/` rather than `vendor/` because Go treats a root
+The web UI serves one third-party script, Alpine.js, in its CSP build: the npm
+package `@alpinejs/csp`. The pages' Content-Security-Policy forbids eval, which
+the standard `alpinejs` build needs to run the expressions written in the
+markup. The CSP build runs no expressions, so every Alpine directive in
+`templates/` only names a property or method of a component registered in
+`static/js/app.js`: `x-data="collapsible"` and `@click="toggle"`, never
+`x-data="{ open: false }"` or `@click="open = !open"`.
+
+A browser test in `internal/server` loads the webhook page and the event log
+under the real policy and checks that: both add forms stay hidden until Add is
+clicked; choosing Slack in the add target form leaves the HTTP fields out of
+what it submits, also after leaving the page and going back to it, when the
+browser restores the choice; an event expands and collapses, and so do a
+delivery's attempts inside it; and at phone width the menu button opens and
+closes the mobile menu. It also fails if the browser reports a console warning
+or error, an uncaught exception, or anything the policy refused. `make check`
+and the image build lint it but do not run it, and `make test` leaves it out
+(its file is built only with the `browser` build tag). Run it with
+`make test-browser` after changing `templates/` or `static/js/`: that builds
+`Dockerfile.browser`, which runs the test in a digest-pinned headless browser
+image, so the host needs no browser.
+
+The package's tarball is committed as `3p/alpinejs-csp-3.14.9.tgz`, byte for
+byte as the npm registry publishes it. It is a dependency, not this repo's build
+output, so `REPO_POLICIES.md`'s rule against committed build artifacts does not
+apply. The directory is `3p/` rather than `vendor/` because Go treats a root
 `vendor/` directory as its module vendor directory.
 
 `script/assets` (`make assets`) extracts the browser build,
@@ -1313,10 +1349,11 @@ nothing downloads Alpine.js. The extracted file is not committed, and
 `.dockerignore` keeps any host copy out of the build context.
 
 To move to a new version: download
-`https://registry.npmjs.org/alpinejs/-/alpinejs-<version>.tgz`, check it
-against the `dist.integrity` hash listed at
-`https://registry.npmjs.org/alpinejs/<version>`, replace the tarball in `3p/`
-with it, update its file name in `script/assets`, and run `make check`.
+`https://registry.npmjs.org/@alpinejs/csp/-/csp-<version>.tgz`, check it against
+the `dist.integrity` hash listed at
+`https://registry.npmjs.org/@alpinejs/csp/<version>`, replace the tarball in
+`3p/` with it as `alpinejs-csp-<version>.tgz`, update its file name in
+`script/assets`, and run `make check` and `make test-browser`.
 
 ## Rationale
 
@@ -2013,8 +2050,7 @@ checkpointed past what it reads, so a long download lets the `-wal`
 grow. It finds the file by the stored names under the lock that webhook
 edits, target edits and target creation hold, and lets go once the file
 is open: a rename during the download moves the file without affecting
-it. Like every request it is cut off after 60 seconds, which leaves the
-file incomplete and failing to decompress.
+it.
 
 Deleting a webhook releases its archives: the delivery engine's cached
 archive writers are dropped and their file handles closed, so nothing
@@ -2472,20 +2508,21 @@ trade.
 Net: **one `INFO` line per request, of at most 2,560 bytes.** That
 ceiling is arithmetic, not an observation: 3 × (512 + 11) for `url`,
 `useragent` and `referer`, plus 128 + 11 for `request_id`, plus 32 + 11
-for `method`, plus a 336-byte fixed portion (the field names, the
-punctuation, both timestamps at their longest, an IPv6 `remoteIP` with
-a zone, the status and the latency) — 2,087 bytes, stated at 2,560 so
-the figure has headroom. `internal/middleware/accesslog_test.go`
-asserts it against 8 KB of client-chosen text in the path, in the
-query, and in each of `User-Agent`, `Referer` and `X-Request-Id`,
+for `method`, plus a 405-byte fixed portion (the field names, the
+punctuation, both timestamps at their longest, `remoteIP` and
+`clientIP` each charged as an IPv6 address with a zone, the status and
+the latency) — 2,156 bytes, stated at 2,560 so the figure has headroom.
+`internal/middleware/accesslog_test.go` asserts it against 8 KB of
+client-chosen text in the path, in the query, and in each of
+`User-Agent`, `Referer`, `X-Request-Id` and `X-Forwarded-For`,
 including cases built from the characters the handlers escape, and
-against the widest access log line the service can be made to write: a
-5xx that keeps its concrete path while all three header fields are also
-at their budget. Every case runs through both handlers
-`internal/logger` can select — the JSON one and the text one it installs
-on a tty — since the two do not escape alike and the ceiling is quoted
-unqualified. Measured over a real connection, the widest access log line
-is 1,972 bytes.
+against a 5xx that keeps its concrete path while all three header fields
+are also at their budget and an `X-Forwarded-For` sent from a trusted
+proxy ends in an IPv6 client address at its longest followed by an 8 KB
+zone, where `clientIP` must name the address without the zone. Every
+case runs through both handlers `internal/logger` can select — the JSON
+one and the text one it installs on a tty — since the two do not escape
+alike and the ceiling is quoted unqualified.
 
 Multiply that ceiling by the request rate to size log storage. Note
 that the rate is not bounded by the limits above on every route:
@@ -2823,9 +2860,9 @@ 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.
-The flood's source is in the proxy's access log: webhooker's own logs
-record the proxy's address, not the client's (see
-[Deployment behind a reverse proxy](#deployment-behind-a-reverse-proxy)).
+The flood's source is in the `clientIP` field of webhooker's access
+log while `TRUSTED_PROXIES` covers the proxy, and in the proxy's own
+access log either way (see [Trusted proxies](#trusted-proxies)).
 
 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
@@ -2913,7 +2950,7 @@ imports. The entry point is `cmd/webhooker/main.go`.
 ```
 webhooker/
 ├── 3p/
-│   └── alpinejs-3.14.9.tgz    # Alpine.js npm package, extracted by make assets
+│   └── alpinejs-csp-3.14.9.tgz  # Alpine.js CSP build npm package, extracted by make assets
 ├── cmd/webhooker/
 │   └── main.go               # Entry point: subcommand dispatch; no args locks DATA_DIR and wires fx
 ├── internal/
@@ -3009,13 +3046,14 @@ webhooker/
 │   ├── css/input.css          # Tailwind input, source for tailwind.css (make css)
 │   ├── css/tailwind.css       # Generated stylesheet the pages load
 │   ├── css/style.css          # Older hand-written stylesheet, no longer loaded
-│   ├── js/app.js              # Progressive-enhancement copy-to-clipboard
-│   └── js/alpine.min.js       # Alpine.js, extracted from 3p/ by make assets, not committed
+│   ├── js/app.js              # Copy-to-clipboard, and the Alpine.js components
+│   └── js/alpine.min.js       # Alpine.js CSP build, extracted from 3p/ by make assets, not committed
 ├── templates/                 # Go HTML templates (base, login, sources, etc.)
 ├── script/                    # Scripts to Rule Them All entrypoints
 ├── Dockerfile                 # Three stages: lint, test+build, Alpine runtime
 ├── Dockerfile.lint            # Lint-only image built by script/lint
-├── Makefile                   # 10 of 17 targets shim script/; 7 are inline
+├── Dockerfile.browser         # Browser test image built by script/test-browser
+├── Makefile                   # 11 of 18 targets shim script/; 7 are inline
 ├── go.mod / go.sum
 └── .golangci.yml              # Linter configuration
 ```
@@ -3064,7 +3102,7 @@ Applied to all routes in this order:
    (HSTS, X-Content-Type-Options, X-Frame-Options, CSP, Referrer-Policy,
    Permissions-Policy)
 3. **Logging** — Structured request logging (method, URL, status,
-   latency, remote IP, user agent, request ID)
+   latency, remote IP, client IP, user agent, request ID)
 4. **Metrics** — Prometheus HTTP metrics (if `METRICS_USERNAME` and
    `METRICS_PASSWORD` are both set)
 5. **CORS** — Cross-origin resource sharing headers
@@ -3361,8 +3399,9 @@ linked, which is what lets it run on the Alpine runtime image.
 inside the image, so a build that succeeds is a repo that is formatted,
 linted, tested and compiled. `script/lint` also uses Docker
 (`Dockerfile.lint`, see Linting above), so `make lint` and `make check`
-run the same pinned linter version the gate does; only `script/test`
-and `script/fmt-check` run on the host.
+run the same pinned linter version the gate does; of the steps
+`make check` runs, only `script/test` and `script/fmt-check` run on the
+host.
 
 #### CI gate honesty
 

go.mod

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

go.sum

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

internal/database/model_apikey.go

@@ -15,6 +15,7 @@ type APIKey struct {
 	Description string     `json:"description"`
 	LastUsedAt  *time.Time `json:"lastUsedAt,omitempty"`
 
-	// Relations
-	User User `json:"user,omitzero"`
+	// Relations. No model marshals the record it belongs to:
+	// User.APIKeys leads back here, and the JSON could loop.
+	User User `json:"-"`
 }

internal/database/model_delivery.go

@@ -56,8 +56,10 @@ type Delivery struct {
 	// the index.
 	FinishedAt *time.Time `gorm:"index:idx_deliveries_status,priority:3" json:"finishedAt,omitempty"`
 
-	// Relations
-	Event           Event            `json:"event,omitzero"`
-	Target          Target           `json:"target,omitzero"`
+	// Relations. No model marshals the record it belongs to:
+	// Event.Deliveries and Target.Deliveries lead back here, and the
+	// JSON could loop.
+	Event           Event            `json:"-"`
+	Target          Target           `json:"-"`
 	DeliveryResults []DeliveryResult `json:"deliveryResults,omitempty"`
 }

internal/database/model_delivery_result.go

@@ -23,6 +23,7 @@ type DeliveryResult struct {
 	Error        string `json:"error,omitempty"`
 	Duration     int64  `json:"durationMs"` // Duration in milliseconds
 
-	// Relations
-	Delivery Delivery `json:"delivery,omitzero"`
+	// Relations. No model marshals the record it belongs to:
+	// Delivery.DeliveryResults leads back here, and the JSON could loop.
+	Delivery Delivery `json:"-"`
 }

internal/database/model_entrypoint.go

@@ -15,6 +15,7 @@ type Entrypoint struct {
 	Description string `json:"description"`
 	Active      bool   `gorm:"default:true" json:"active"`
 
-	// Relations
-	Webhook Webhook `json:"webhook,omitzero"`
+	// Relations. No model marshals the record it belongs to:
+	// Webhook.Entrypoints leads back here, and the JSON could loop.
+	Webhook Webhook `json:"-"`
 }

internal/database/model_event.go

@@ -44,8 +44,9 @@ type Event struct {
 	// kept as the record of where the copy came from either way.
 	ResubmittedFromID *string `gorm:"type:uuid;index" json:"resubmittedFromId,omitempty"`
 
-	// Relations
-	Webhook    Webhook    `json:"webhook,omitzero"`
-	Entrypoint Entrypoint `json:"entrypoint,omitzero"`
+	// Relations. No model marshals the record it belongs to, so
+	// Webhook and Entrypoint are left out of the JSON.
+	Webhook    Webhook    `json:"-"`
+	Entrypoint Entrypoint `json:"-"`
 	Deliveries []Delivery `json:"deliveries,omitempty"`
 }

internal/database/model_parent_test.go

@@ -0,0 +1,126 @@
+package database_test
+
+import (
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// TestPreloadedModelsMarshalWithoutTheirParent pins that a child's
+// reference to the record it belongs to is left out of the JSON, so a
+// webhook and its targets cannot marshal each other in a loop, and that
+// GORM still preloads that reference, since it ignores json tags.
+func TestPreloadedModelsMarshalWithoutTheirParent(t *testing.T) {
+	t.Parallel()
+
+	db := startedTestDB(t)
+
+	stored := database.Webhook{
+		UserID:      uuid.New().String(),
+		Name:        testWebhookName,
+		Entrypoints: []database.Entrypoint{{Path: uuid.New().String()}},
+		Targets: []database.Target{{
+			Name: "log",
+			Type: database.TargetTypeLog,
+		}},
+	}
+	require.NoError(t, db.Create(&stored).Error)
+
+	entrypointID := stored.Entrypoints[0].ID
+	targetID := stored.Targets[0].ID
+
+	var webhook database.Webhook
+
+	require.NoError(t, db.
+		Preload("Entrypoints.Webhook").
+		Preload("Targets.Webhook").
+		First(&webhook, "id = ?", stored.ID).Error)
+
+	require.Len(t, webhook.Entrypoints, 1)
+	require.Len(t, webhook.Targets, 1)
+	assert.Equal(t, stored.ID, webhook.Entrypoints[0].Webhook.ID)
+	assert.Equal(t, stored.ID, webhook.Targets[0].Webhook.ID)
+
+	encoded := marshalModel(t, webhook)
+
+	assert.Contains(t, encoded, entrypointID)
+	assert.Contains(t, encoded, targetID)
+
+	// Each child holds the parent's id as its webhookId, so the parent
+	// is looked for by its own id field.
+	parentIDField := `"id":"` + stored.ID + `"`
+
+	assert.NotContains(t, marshalModel(t, webhook.Entrypoints[0]), parentIDField)
+	assert.NotContains(t, marshalModel(t, webhook.Targets[0]), parentIDField)
+
+	var target database.Target
+
+	require.NoError(t, db.
+		Preload("Webhook").
+		First(&target, "id = ?", targetID).Error)
+
+	assert.Equal(t, stored.ID, target.Webhook.ID)
+
+	encoded = marshalModel(t, target)
+
+	assert.Contains(t, encoded, stored.ID)
+	assert.NotContains(t, encoded, parentIDField)
+}
+
+// TestModelsMarshalWithoutTheirParent covers the other references to a
+// parent: each model is built with its parent set, and the parent's id
+// must not appear in the JSON.
+func TestModelsMarshalWithoutTheirParent(t *testing.T) {
+	t.Parallel()
+
+	parent := database.BaseModel{ID: uuid.New().String()}
+
+	cases := []struct {
+		name  string
+		model any
+	}{
+		{
+			name:  "Webhook.User",
+			model: database.Webhook{User: database.User{BaseModel: parent}},
+		},
+		{
+			name:  "APIKey.User",
+			model: database.APIKey{User: database.User{BaseModel: parent}},
+		},
+		{
+			name:  "Delivery.Event",
+			model: database.Delivery{Event: database.Event{BaseModel: parent}},
+		},
+		{
+			name:  "Delivery.Target",
+			model: database.Delivery{Target: database.Target{BaseModel: parent}},
+		},
+		{
+			name: "DeliveryResult.Delivery",
+			model: database.DeliveryResult{
+				Delivery: database.Delivery{BaseModel: parent},
+			},
+		},
+		{
+			name:  "Event.Webhook",
+			model: database.Event{Webhook: database.Webhook{BaseModel: parent}},
+		},
+		{
+			name: "Event.Entrypoint",
+			model: database.Event{
+				Entrypoint: database.Entrypoint{BaseModel: parent},
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			assert.NotContains(t, marshalModel(t, tc.model), parent.ID)
+		})
+	}
+}

internal/database/model_target.go

@@ -34,7 +34,8 @@ type Target struct {
 	MaxRetries   int `json:"maxRetries,omitempty"`
 	MaxQueueSize int `json:"maxQueueSize,omitempty"`
 
-	// Relations
-	Webhook    Webhook    `json:"webhook,omitzero"`
+	// Relations. No model marshals the record it belongs to:
+	// Webhook.Targets leads back here, and the JSON could loop.
+	Webhook    Webhook    `json:"-"`
 	Deliveries []Delivery `json:"deliveries,omitempty"`
 }

internal/database/model_webhook.go

@@ -66,8 +66,9 @@ type Webhook struct {
 	// must equal DefaultRetentionDays.
 	RetentionDays int `gorm:"default:30" json:"retentionDays"`
 
-	// Relations
-	User        User         `json:"user,omitzero"`
+	// Relations. No model marshals the record it belongs to:
+	// User.Webhooks leads back here, and the JSON could loop.
+	User        User         `json:"-"`
 	Entrypoints []Entrypoint `json:"entrypoints,omitempty"`
 	Targets     []Target     `json:"targets,omitempty"`
 }

internal/database/retention.go

@@ -184,16 +184,6 @@ func (r *RetentionReaper) sweep(ctx context.Context) {
 
 		wh := webhooks[i]
 
-		// Skip retain-forever webhooks before building any query.
-		// RetainsForever covers both the RetentionForeverDays
-		// sentinel and the non-positive values that predate it: the
-		// sentinel is a positive number, so without this the reaper
-		// would compute a cutoff a thousand years in the past and
-		// issue a DELETE matching nothing on every single sweep.
-		if wh.RetainsForever() {
-			continue
-		}
-
 		// Nothing to reap if the per-webhook database has never
 		// been created.
 		if !r.dbManager.DBExists(wh.ID) {
@@ -212,6 +202,13 @@ func (r *RetentionReaper) reapWebhook(
 	webhookID string,
 	retentionDays int,
 ) {
+	// A retain-forever webhook has no cutoff, so its database is not
+	// even opened.
+	cutoff, ok := retentionCutoff(time.Now(), retentionDays)
+	if !ok {
+		return
+	}
+
 	db, err := r.dbManager.GetDB(webhookID)
 	if err != nil {
 		r.log.Error(
@@ -223,11 +220,6 @@ func (r *RetentionReaper) reapWebhook(
 		return
 	}
 
-	cutoff, ok := retentionCutoff(time.Now(), retentionDays)
-	if !ok {
-		return
-	}
-
 	deleted, err := reapExpired(ctx, db, cutoff)
 	if err != nil {
 		r.log.Error(

internal/database/retention_test.go

@@ -362,7 +362,7 @@ func TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents(
 		t,
 		overflowingRetentionDays,
 		database.RetentionForeverDays,
-		"the test value must not be rescued by the forever skip",
+		"the test value must not be treated as retain-forever",
 	)
 
 	webhookID := createWebhook(

internal/database/totals_test.go

@@ -102,6 +102,13 @@ func TestWebhookDBManager_TotalsSurviveReopen(t *testing.T) {
 // seedExpiredEvents stores count events created at the given time,
 // each with a delivered delivery to one target and a failed delivery
 // to the other, and one attempt for each delivery.
+//
+// It and seedBareEvents insert 50 rows per statement, not more. The
+// SQLite driver looks up each parameter's value by scanning the
+// statement's arguments from the first until it reaches that
+// parameter's, so the time to bind a statement grows with the square of
+// its parameter count: at 500 rows, several thousand parameters, the
+// seeding took most of these tests' time under -race.
 func seedExpiredEvents(
 	t *testing.T,
 	db *gorm.DB,
@@ -138,8 +145,8 @@ func seedExpiredEvents(
 		)
 	}
 
-	require.NoError(t, db.CreateInBatches(events, 500).Error)
-	require.NoError(t, db.CreateInBatches(deliveries, 500).Error)
+	require.NoError(t, db.CreateInBatches(events, 50).Error)
+	require.NoError(t, db.CreateInBatches(deliveries, 50).Error)
 
 	results := make([]database.DeliveryResult, len(deliveries))
 	for i := range deliveries {
@@ -148,7 +155,7 @@ func seedExpiredEvents(
 		}
 	}
 
-	require.NoError(t, db.CreateInBatches(results, 500).Error)
+	require.NoError(t, db.CreateInBatches(results, 50).Error)
 }
 
 // seedBareEvents stores count events created at the given time, with
@@ -172,7 +179,7 @@ func seedBareEvents(
 		events[i].CreatedAt = createdAt
 	}
 
-	require.NoError(t, db.CreateInBatches(events, 500).Error)
+	require.NoError(t, db.CreateInBatches(events, 50).Error)
 }
 
 // TestRetentionReaper_PrunesMoreThanOneBatch verifies that a prune

internal/delivery/export_test.go

@@ -512,6 +512,12 @@ func (e *ExportArchiveWriter) Reopen(
 	return e.w.reopen(expiry)
 }
 
+// SetNow replaces the clock the writer measures its reopen
+// debounce on.
+func (e *ExportArchiveWriter) SetNow(now func() time.Time) {
+	e.w.now = now
+}
+
 // Reopens reports how many times the file has been opened.
 func (e *ExportArchiveWriter) Reopens() int {
 	return e.w.reopens

internal/delivery/target_database_archive.go

@@ -194,6 +194,10 @@ type archiveWriter struct {
 	lastReopen time.Time
 	reopens    int
 
+	// now is the clock the reopen debounce is measured on. It is
+	// time.Now outside tests.
+	now func() time.Time
+
 	// evicted marks a writer that has been removed from the
 	// registry. Its handle is closed and it must never open the
 	// file again: nothing holds it any more, so a reopen would
@@ -228,6 +232,7 @@ func newArchiveWriter(
 		path:     path,
 		log:      log,
 		debounce: archiveReopenDebounce,
+		now:      time.Now,
 	}
 }
 
@@ -263,7 +268,7 @@ func (w *archiveWriter) write(
 		)
 	}
 
-	if time.Since(w.lastReopen) >= w.debounce {
+	if w.now().Sub(w.lastReopen) >= w.debounce {
 		return w.reopen(expiry)
 	}
 
@@ -323,7 +328,7 @@ func (w *archiveWriter) openMode(
 	}
 
 	w.db = gdb
-	w.lastReopen = time.Now()
+	w.lastReopen = w.now()
 	w.reopens++
 
 	if expiry > 0 {

internal/delivery/target_database_export.go

@@ -6,14 +6,17 @@ import (
 	"database/sql"
 	"encoding/base64"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
+	"log/slog"
 	"path/filepath"
 	"time"
 	"unicode/utf8"
 
+	"gorm.io/driver/sqlite"
+	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/gormlog"
 )
 
 // archiveTableQuery counts the archive's table: 0 when the archive
@@ -21,15 +24,6 @@ import (
 const archiveTableQuery = "SELECT count(*) FROM sqlite_master " +
 	"WHERE type = 'table' AND name = 'archived_events'"
 
-// archiveNextRowQuery reads every column of the first archived row
-// after a given id. An export reads the archive a row at a time this
-// way rather than through one cursor, because the Scan check in
-// internal/gormlog accepts only a Scan straight on QueryRowContext's
-// result.
-const archiveNextRowQuery = "SELECT id, event_id, webhook_id, " +
-	"entrypoint_id, method, headers, body, content_type, archived_at " +
-	"FROM archived_events WHERE id > ? ORDER BY id LIMIT 1"
-
 // ArchivePath returns where a database target's archive file is: in
 // the data directory, beside the webhook's event database, under the
 // name ArchiveFileName gives it.
@@ -67,7 +61,7 @@ func ArchiveExportFileName(
 // past an open snapshot, so the -wal grows until the export is closed.
 type ArchiveExport struct {
 	db *sql.DB
-	tx *sql.Tx
+	tx *gorm.DB
 
 	// empty is true when there is nothing to read: no file, or a file
 	// without the archive's table yet.
@@ -87,8 +81,11 @@ type exportedName struct {
 // Once it has returned, the file is open, so a rename or a move of it
 // does not affect the export, which reads the same file under its new
 // name.
+//
+// The transaction lasts as long as ctx does, so ctx must last for the
+// whole export.
 func OpenArchiveExport(
-	ctx context.Context, path string,
+	ctx context.Context, path string, log *slog.Logger,
 ) (*ArchiveExport, error) {
 	if !fileExists(path) {
 		return &ArchiveExport{empty: true}, nil
@@ -99,20 +96,33 @@ func OpenArchiveExport(
 		return nil, fmt.Errorf("opening archive %s: %w", path, err)
 	}
 
-	// ReadOnly makes the driver begin a deferred transaction in place
-	// of the BEGIN IMMEDIATE the connection string asks for, so the
-	// export never takes the archive's write lock.
-	tx, err := db.BeginTx(ctx, &sql.TxOptions{ReadOnly: true})
+	gdb, err := gorm.Open(
+		sqlite.Dialector{Conn: db}, &gorm.Config{
+			// Never leave this at GORM's default. See
+			// internal/gormlog.
+			Logger: gormlog.New(log),
+		},
+	)
 	if err != nil {
 		_ = db.Close()
 
-		return nil, fmt.Errorf("reading archive %s: %w", path, err)
+		return nil, fmt.Errorf("opening archive %s: %w", path, err)
+	}
+
+	// ReadOnly makes the driver begin a deferred transaction in place
+	// of the BEGIN IMMEDIATE the connection string asks for, so the
+	// export never takes the archive's write lock.
+	tx := gdb.WithContext(ctx).Begin(&sql.TxOptions{ReadOnly: true})
+	if tx.Error != nil {
+		_ = db.Close()
+
+		return nil, fmt.Errorf("reading archive %s: %w", path, tx.Error)
 	}
 
 	// The transaction's first read is what takes the snapshot.
 	var tables int
 
-	err = tx.QueryRowContext(ctx, archiveTableQuery).Scan(&tables)
+	err = tx.Raw(archiveTableQuery).Row().Scan(&tables)
 	if err != nil {
 		_ = tx.Rollback()
 		_ = db.Close()
@@ -204,25 +214,18 @@ func (x *ArchiveExport) writeRows(ctx context.Context, w io.Writer) error {
 		return nil
 	}
 
-	next, err := x.tx.PrepareContext(ctx, archiveNextRowQuery)
+	rows, err := x.tx.WithContext(ctx).
+		Model(&archivedEvent{}).Order("id").Rows()
 	if err != nil {
 		return err
 	}
 
-	defer func() { _ = next.Close() }()
+	defer func() { _ = rows.Close() }()
 
+	for sep := "\n"; rows.Next(); sep = ",\n" {
 		var ev archivedEvent
 
-	for sep := "\n"; ; sep = ",\n" {
-		err = next.QueryRowContext(ctx, ev.ID).Scan(
-			&ev.ID, &ev.EventID, &ev.WebhookID, &ev.EntrypointID,
-			&ev.Method, &ev.Headers, &ev.Body, &ev.ContentType,
-			&ev.ArchivedAt,
-		)
-		if errors.Is(err, sql.ErrNoRows) {
-			return nil
-		}
-
+		err = x.tx.ScanRows(rows, &ev)
 		if err != nil {
 			return err
 		}
@@ -237,6 +240,8 @@ func (x *ArchiveExport) writeRows(ctx context.Context, w io.Writer) error {
 			return err
 		}
 	}
+
+	return rows.Err()
 }
 
 // writeRow writes an archived row to w as a JSON object keyed by

internal/delivery/target_database_export_test.go

@@ -3,6 +3,7 @@ package delivery_test
 import (
 	"bytes"
 	"compress/gzip"
+	"crypto/rand"
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
@@ -11,7 +12,6 @@ import (
 	"path/filepath"
 	"runtime"
 	"runtime/debug"
-	"strings"
 	"testing"
 	"time"
 
@@ -64,7 +64,9 @@ func writeExportTo(
 func exportArchive(t *testing.T, path string) map[string]any {
 	t.Helper()
 
-	export, err := delivery.OpenArchiveExport(t.Context(), path)
+	export, err := delivery.OpenArchiveExport(
+		t.Context(), path, archiveTestLogger(),
+	)
 	require.NoError(t, err)
 
 	defer func() { require.NoError(t, export.Close()) }()
@@ -252,7 +254,9 @@ func TestArchiveExport_ReadsOneSnapshot(t *testing.T) {
 
 	require.NoError(t, w.Write(delivery.ExportArchivedEvent{EventID: openedEventID}, 0))
 
-	export, err := delivery.OpenArchiveExport(t.Context(), path)
+	export, err := delivery.OpenArchiveExport(
+		t.Context(), path, archiveTestLogger(),
+	)
 	require.NoError(t, err)
 
 	defer func() { require.NoError(t, export.Close()) }()
@@ -281,7 +285,9 @@ func TestArchiveExport_SurvivesRename(t *testing.T) {
 
 	require.NoError(t, w.Write(delivery.ExportArchivedEvent{EventID: openedEventID}, 0))
 
-	export, err := delivery.OpenArchiveExport(t.Context(), path)
+	export, err := delivery.OpenArchiveExport(
+		t.Context(), path, archiveTestLogger(),
+	)
 	require.NoError(t, err)
 
 	defer func() { require.NoError(t, export.Close()) }()
@@ -311,10 +317,11 @@ func (p *heapPeak) Write(b []byte) (int, error) {
 }
 
 // TestArchiveExport_Streams proves an export holds neither the archive
-// nor its JSON in memory whole: exporting a 16 MiB archive grows the
-// heap by less than half of that. An export that read every row before
-// writing, or built the JSON before writing it, would hold all 16 MiB
-// at a write.
+// nor its output in memory whole: exporting a 16 MiB archive grows the
+// heap by less than half of that. The bodies are random bytes in
+// base64, which gzip shrinks by only a quarter, so an export that read
+// every row before writing, or built the JSON or the gzipped file
+// before writing it, would hold at least 12 MiB at a write.
 //
 //nolint:paralleltest // It measures the heap, which tests share.
 func TestArchiveExport_Streams(t *testing.T) {
@@ -326,13 +333,21 @@ func TestArchiveExport_Streams(t *testing.T) {
 
 	path := filepath.Join(t.TempDir(), "archive.db")
 	w := delivery.NewExportArchiveWriter(path, archiveTestLogger(), 0)
-	body := strings.Repeat("x", bodySize)
+
+	// Base64 makes four characters of every three bytes.
+	random := make([]byte, bodySize/4*3)
 
 	for range rows {
-		require.NoError(t, w.Write(delivery.ExportArchivedEvent{Body: body}, 0))
+		_, _ = rand.Read(random)
+
+		require.NoError(t, w.Write(delivery.ExportArchivedEvent{
+			Body: base64.StdEncoding.EncodeToString(random),
+		}, 0))
 	}
 
-	export, err := delivery.OpenArchiveExport(t.Context(), path)
+	export, err := delivery.OpenArchiveExport(
+		t.Context(), path, archiveTestLogger(),
+	)
 	require.NoError(t, err)
 
 	defer func() { require.NoError(t, export.Close()) }()

internal/delivery/target_database_test.go

@@ -184,13 +184,20 @@ func TestArchiveWriter_RecreatesAfterRemoval(
 func TestArchiveWriter_ReopenDebounce(t *testing.T) {
 	t.Parallel()
 
-	// A generous debounce keeps the two rapid writes inside
-	// the window even on a heavily loaded test machine.
+	const debounce = 2 * time.Second
+
 	path := filepath.Join(t.TempDir(), "archive-wh.db")
 	w := delivery.NewExportArchiveWriter(
-		path, archiveTestLogger(), 2*time.Second,
+		path, archiveTestLogger(), debounce,
 	)
 
+	// The writer measures its reopen debounce on this clock, which
+	// only the test moves, so how long the host takes between
+	// writes cannot change the result.
+	now := time.Now()
+
+	w.SetNow(func() time.Time { return now })
+
 	require.NoError(t, w.Write(
 		delivery.ExportArchivedEvent{EventID: "a"}, 0,
 	))
@@ -202,7 +209,7 @@ func TestArchiveWriter_ReopenDebounce(t *testing.T) {
 	// initial open — no extra close/reopen.
 	assert.Equal(t, 1, w.Reopens())
 
-	time.Sleep(2100 * time.Millisecond)
+	now = now.Add(debounce)
 
 	require.NoError(t, w.Write(
 		delivery.ExportArchivedEvent{EventID: "c"}, 0,

internal/handlers/auth.go

@@ -139,8 +139,7 @@ func (h *Handlers) renderLoginError(
 		),
 	}
 
-	w.WriteHeader(status)
-	h.renderTemplate(w, r, "login.html", data)
+	h.renderTemplateStatus(w, r, "login.html", data, status)
 }
 
 // authenticateUser looks up and verifies a user's credentials.

internal/handlers/auth_test.go

@@ -3,6 +3,7 @@ package handlers_test
 import (
 	"context"
 	"fmt"
+	"html/template"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -404,6 +405,60 @@ func TestLogin_MissingCredentialsRejectedBeforeAnyHash(t *testing.T) {
 	)
 }
 
+// TestLogin_FormErrorAnswersItsStatusWithThePage proves that the login
+// form shown again with an error still answers 400 with the whole page.
+func TestLogin_FormErrorAnswersItsStatusWithThePage(t *testing.T) {
+	t.Parallel()
+
+	var h *handlers.Handlers
+
+	app := newTestApp(t, &h)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	w := submitLogin(h, sharedProxyPeer, "", "")
+
+	assert.Equal(t, http.StatusBadRequest, w.Code)
+	assert.Contains(
+		t, w.Body.String(), "Username and password are required",
+	)
+	assert.Contains(
+		t, w.Body.String(), "</html>",
+		"the page must render to completion",
+	)
+}
+
+// TestLogin_FormErrorRenderFailureAnswers500 proves that a login form
+// error page whose template fails answers 500 with the error page and
+// none of the form page, rather than the 400 it meant to send.
+func TestLogin_FormErrorRenderFailureAnswers500(t *testing.T) {
+	t.Parallel()
+
+	var h *handlers.Handlers
+
+	app := newTestApp(t, &h)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	// The page prints its error message and then fails.
+	h.AddTemplateForTest("login.html", template.Must(
+		template.New("login").Funcs(template.FuncMap{
+			"fail": func() (string, error) { return "", errMidRender },
+		}).Parse(`{{.Error}}{{fail}}`),
+	))
+
+	w := submitLogin(h, sharedProxyPeer, "", "")
+
+	assert.Equal(t, http.StatusInternalServerError, w.Code)
+	assert.NotContains(
+		t, w.Body.String(), "Username and password are required",
+		"the response must carry no part of the aborted page",
+	)
+	assert.Contains(t, w.Body.String(), "500 Internal Server Error")
+}
+
 // TestLogin_SuccessCreatesSession is the control for the tests above:
 // the success path they assert on really does authenticate.
 func TestLogin_SuccessCreatesSession(t *testing.T) {

internal/handlers/handlers.go

@@ -310,12 +310,26 @@ func (s *Handlers) getUserInfo(
 }
 
 // renderTemplate renders a pre-parsed template with common
-// data
+// data and answers 200.
 func (s *Handlers) renderTemplate(
 	w http.ResponseWriter,
 	r *http.Request,
 	pageTemplate string,
 	data any,
+) {
+	s.renderTemplateStatus(w, r, pageTemplate, data, http.StatusOK)
+}
+
+// renderTemplateStatus is renderTemplate answering with status, for a
+// form shown again with an error. Call it instead of WriteHeader
+// followed by renderTemplate: the status is written only once the page
+// has rendered, so a failed render can still answer 500.
+func (s *Handlers) renderTemplateStatus(
+	w http.ResponseWriter,
+	r *http.Request,
+	pageTemplate string,
+	data any,
+	status int,
 ) {
 	tmpl, ok := s.templates[pageTemplate]
 	if !ok {
@@ -328,7 +342,9 @@ func (s *Handlers) renderTemplate(
 		return
 	}
 
-	s.executeTemplate(w, r, tmpl, s.pageData(r, data, noticeFor(r)))
+	s.executeTemplate(
+		w, r, tmpl, s.pageData(r, data, noticeFor(r)), status,
+	)
 }
 
 // pageData adds the fields the shared layout renders to a page's own
@@ -364,19 +380,20 @@ func (s *Handlers) pageData(
 	}
 }
 
-// executeTemplate renders the template into a buffer and writes to
-// the response only once rendering has fully succeeded. Executing
-// straight into the ResponseWriter commits a partial body and a 200
-// status before a mid-render error can be reported, leaving no way
-// to serve a 500. Buffering makes a page's rendered size resident
-// memory per concurrent viewer, so every page owes it a bound: the
-// event log caps each stored body at maxRenderedBodyBytes for exactly
-// this reason.
+// executeTemplate renders the template into a buffer and writes status
+// and the page to the response only once rendering has fully
+// succeeded. Executing straight into the ResponseWriter commits a
+// partial body and the status before a mid-render error can be
+// reported, leaving no way to serve a 500. Buffering makes a page's
+// rendered size resident memory per concurrent viewer, so every page
+// owes it a bound: the event log caps each stored body at
+// maxRenderedBodyBytes for exactly this reason.
 func (s *Handlers) executeTemplate(
 	w http.ResponseWriter,
 	r *http.Request,
 	tmpl *template.Template,
 	data any,
+	status int,
 ) {
 	var buf bytes.Buffer
 
@@ -391,6 +408,7 @@ func (s *Handlers) executeTemplate(
 	}
 
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
+	w.WriteHeader(status)
 
 	_, err = buf.WriteTo(w)
 	if err != nil {

internal/handlers/source_management.go

@@ -41,71 +41,51 @@ type WebhookListItem struct {
 // errMissingURL signals that a required URL was not provided.
 var errMissingURL = errors.New("missing URL")
 
-// errInvalidRetention signals a retention_days form value that is not
-// a non-negative whole number.
-var errInvalidRetention = errors.New("invalid retention days")
-
-// errRetentionTooLarge signals a retention_days form value that is a
-// whole number but larger than the reaper's cutoff arithmetic can
-// represent. It is distinguished from errInvalidRetention so the form
-// can tell the user the actual ceiling instead of implying their input
-// was not a number.
-var errRetentionTooLarge = errors.New("retention days out of range")
-
-// retentionErrorMessage returns the message the create and edit forms
-// show the user for a rejected retention_days value. Any error other
-// than errRetentionTooLarge falls back to the generic wording, so an
-// unrecognised parse failure still produces a sensible 400 rather than
-// an empty alert.
-func retentionErrorMessage(err error) string {
-	if errors.Is(err, errRetentionTooLarge) {
-		return "Retention must be at most " +
-			strconv.Itoa(database.MaxFiniteRetentionDays) +
-			" days, or 0 to retain events forever."
-	}
-
-	return "Retention must be a whole number of days, or 0 to " +
-		"retain events forever."
-}
-
-// parseRetentionDays interprets a retention_days form value.
+// parseRetentionDays interprets a retention_days form value. It
+// returns the number of days, or, for a value it refuses, the message
+// the create and edit forms show; the message is empty when the value
+// is accepted.
 //
 // An empty value yields fallback, which lets the create path apply the
 // default and the edit path leave the stored value unchanged. A value
 // of 0 is returned as 0 and is rewritten to the retain-forever
 // sentinel by database.Webhook's BeforeSave hook. Anything unparseable
-// or negative is an error rather than a silently substituted default.
+// or negative is refused rather than silently given a default.
 //
 // The upper bound is not cosmetic. The reaper computes its cutoff as a
 // time.Duration, an int64 nanosecond count, so a day count above
 // database.MaxFiniteRetentionDays overflows, puts the cutoff in the
 // future, and deletes every event the webhook has. A finite value
-// above that ceiling is therefore a 400.
+// above that ceiling is therefore refused, and the message names the
+// ceiling rather than implying the input was not a number.
 //
 // A value at or above the retain-forever sentinel is not out of range:
 // it is what the edit form pre-fills for a retain-forever webhook, so
 // submitting the form back unchanged has to keep meaning "forever"
 // rather than being rejected.
-func parseRetentionDays(raw string, fallback int) (int, error) {
+func parseRetentionDays(raw string, fallback int) (int, string) {
 	raw = strings.TrimSpace(raw)
 	if raw == "" {
-		return fallback, nil
+		return fallback, ""
 	}
 
 	v, err := strconv.Atoi(raw)
 	if err != nil || v < 0 {
-		return 0, errInvalidRetention
+		return 0, "Retention must be a whole number of days, or 0 to " +
+			"retain events forever."
 	}
 
 	if v >= database.RetentionForeverDays {
-		return database.RetentionForeverDays, nil
+		return database.RetentionForeverDays, ""
 	}
 
 	if v > database.MaxFiniteRetentionDays {
-		return 0, errRetentionTooLarge
+		return 0, "Retention must be at most " +
+			strconv.Itoa(database.MaxFiniteRetentionDays) +
+			" days, or 0 to retain events forever."
 	}
 
-	return v, nil
+	return v, ""
 }
 
 // DeliveryView is the display-safe projection of a delivery
@@ -350,28 +330,25 @@ func (h *Handlers) HandleSourceCreateSubmit() http.HandlerFunc {
 		retentionStr := r.PostFormValue("retention_days")
 
 		if name == "" {
-			w.WriteHeader(http.StatusBadRequest)
-			h.renderTemplate(
+			h.renderTemplateStatus(
 				w, r, "sources_new.html",
 				newSourceFormData(
 					"Name is required", name, description,
 				),
+				http.StatusBadRequest,
 			)
 
 			return
 		}
 
-		retentionDays, retErr := parseRetentionDays(
+		retentionDays, errMsg := parseRetentionDays(
 			retentionStr, database.DefaultRetentionDays,
 		)
-		if retErr != nil {
-			w.WriteHeader(http.StatusBadRequest)
-			h.renderTemplate(
+		if errMsg != "" {
+			h.renderTemplateStatus(
 				w, r, "sources_new.html",
-				newSourceFormData(
-					retentionErrorMessage(retErr),
-					name, description,
-				),
+				newSourceFormData(errMsg, name, description),
+				http.StatusBadRequest,
 			)
 
 			return
@@ -644,8 +621,7 @@ func (h *Handlers) applyWebhookEdit(
 			tmplKeyError:   "Name is required",
 		}
 
-		w.WriteHeader(http.StatusBadRequest)
-		h.renderTemplate(w, r, "source_edit.html", data)
+		h.renderTemplateStatus(w, r, "source_edit.html", data, http.StatusBadRequest)
 
 		return
 	}
@@ -656,17 +632,16 @@ func (h *Handlers) applyWebhookEdit(
 
 	// An empty field falls back to the stored value, so submitting the
 	// form without touching retention leaves the policy alone.
-	retentionDays, retErr := parseRetentionDays(
+	retentionDays, errMsg := parseRetentionDays(
 		r.PostFormValue("retention_days"), webhook.RetentionDays,
 	)
-	if retErr != nil {
+	if errMsg != "" {
 		data := map[string]any{
 			tmplKeyWebhook: webhook,
-			tmplKeyError:   retentionErrorMessage(retErr),
+			tmplKeyError:   errMsg,
 		}
 
-		w.WriteHeader(http.StatusBadRequest)
-		h.renderTemplate(w, r, "source_edit.html", data)
+		h.renderTemplateStatus(w, r, "source_edit.html", data, http.StatusBadRequest)
 
 		return
 	}
@@ -703,8 +678,7 @@ func (h *Handlers) applyWebhookEdit(
 					"it, then save again.",
 			}
 
-			w.WriteHeader(http.StatusConflict)
-			h.renderTemplate(w, r, "source_edit.html", data)
+			h.renderTemplateStatus(w, r, "source_edit.html", data, http.StatusConflict)
 
 			return
 		}

internal/handlers/source_management_test.go

@@ -191,6 +191,7 @@ func storedRetentionDays(
 type sourceTestEnv struct {
 	handlers *handlers.Handlers
 	db       *database.Database
+	dbMgr    *database.WebhookDBManager
 	archives *recordingArchives
 	cookies  []*http.Cookie
 }
@@ -204,9 +205,11 @@ func setupSourceTest(t *testing.T) *sourceTestEnv {
 
 	var db *database.Database
 
+	var dbMgr *database.WebhookDBManager
+
 	var archives *recordingArchives
 
-	app := newTestApp(t, &h, &sess, &db, &archives)
+	app := newTestApp(t, &h, &sess, &db, &dbMgr, &archives)
 	app.RequireStart()
 
 	t.Cleanup(app.RequireStop)
@@ -214,6 +217,7 @@ func setupSourceTest(t *testing.T) *sourceTestEnv {
 	return &sourceTestEnv{
 		handlers: h,
 		db:       db,
+		dbMgr:    dbMgr,
 		archives: archives,
 		cookies: authenticatedCookies(
 			t, sess, sourceTestUserID, "sourceuser",
@@ -368,16 +372,24 @@ func TestHandleSourceCreateSubmit_OverflowingRetentionIsRejected(
 // boundary between "too large to represent" and "retain forever": the
 // sentinel is above MaxFiniteRetentionDays, but it is the value the
 // edit form pre-fills, so it must be accepted rather than rejected as
-// out of range.
+// out of range. A value above the sentinel is stored as the sentinel.
 func TestHandleSourceCreateSubmit_SentinelIsAcceptedAsForever(
 	t *testing.T,
 ) {
 	t.Parallel()
 
-	env := setupSourceTest(t)
-	sentinel := strconv.Itoa(database.RetentionForeverDays)
+	for _, days := range []int{
+		database.RetentionForeverDays,
+		database.RetentionForeverDays + 1,
+	} {
+		raw := strconv.Itoa(days)
 
-	w := submitCreate(t, env.handlers, env.cookies, "forever", &sentinel)
+		t.Run(raw, func(t *testing.T) {
+			t.Parallel()
+
+			env := setupSourceTest(t)
+
+			w := submitCreate(t, env.handlers, env.cookies, "forever", &raw)
 			require.Equal(t, http.StatusSeeOther, w.Code)
 
 			wh := onlyWebhook(t, env.db)
@@ -386,13 +398,16 @@ func TestHandleSourceCreateSubmit_SentinelIsAcceptedAsForever(
 				database.RetentionForeverDays,
 				storedRetentionDays(t, env.db, wh.ID),
 			)
+		})
+	}
 }
 
 // TestHandleSourceCreateSubmit_RejectedFormKeepsUserInput checks that a
 // validation failure hands the user's typing back, matching what the
 // edit form already does. Losing a long description to a mistyped
 // retention value is the kind of thing that makes people give up on a
-// form.
+// form. Both values carry HTML-special characters, which must come
+// back escaped rather than as markup.
 func TestHandleSourceCreateSubmit_RejectedFormKeepsUserInput(
 	t *testing.T,
 ) {
@@ -401,8 +416,8 @@ func TestHandleSourceCreateSubmit_RejectedFormKeepsUserInput(
 	env := setupSourceTest(t)
 
 	const (
-		name        = "kept-name"
-		description = "a description worth not losing"
+		name        = `kept"><b>name`
+		description = `a </textarea> worth not losing`
 	)
 
 	form := url.Values{}
@@ -419,8 +434,10 @@ func TestHandleSourceCreateSubmit_RejectedFormKeepsUserInput(
 
 	body := w.Body.String()
 
-	assert.Contains(t, body, `value="`+name+`"`)
-	assert.Contains(t, body, description)
+	assert.Contains(t, body, `value="kept&#34;&gt;&lt;b&gt;name"`)
+	assert.Contains(t, body, `a &lt;/textarea&gt; worth not losing`)
+	assert.NotContains(t, body, name)
+	assert.NotContains(t, body, description)
 }
 
 // submitEdit posts the webhook edit form for the given webhook.

internal/handlers/target_download.go

@@ -1,6 +1,8 @@
 package handlers
 
 import (
+	"context"
+	"errors"
 	"net/http"
 	"time"
 
@@ -8,13 +10,24 @@ import (
 	"sneak.berlin/go/webhooker/internal/delivery"
 )
 
+// downloadWriteTimeout is how long one write of a download may wait
+// for a client that has stopped reading.
+const downloadWriteTimeout = 60 * time.Second
+
 // HandleTargetDownload serves a database target's archive as one
 // gzipped JSON file, named for the webhook, the target and the time;
 // see delivery.ArchiveExport.WriteGzipJSON for what it holds. Other
 // target types have no archive and are a 404.
+//
+// A download runs for as long as the client keeps reading: it reads
+// under a context the request limit does not cancel, and gives each
+// write its own deadline in place of the server's write timeout. It
+// stops when a write fails.
 func (h *Handlers) HandleTargetDownload() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		webhook, target, export, ok := h.openTargetArchive(w, r)
+		ctx := context.WithoutCancel(r.Context())
+
+		webhook, target, export, ok := h.openTargetArchive(ctx, w, r)
 		if !ok {
 			return
 		}
@@ -31,27 +44,53 @@ func (h *Handlers) HandleTargetDownload() http.HandlerFunc {
 			)+`"`,
 		)
 
-		err := export.WriteGzipJSON(r.Context(), w, &webhook, target, now)
+		err := export.WriteGzipJSON(
+			ctx,
+			downloadWriter{w: w, rc: http.NewResponseController(w)},
+			&webhook, target, now,
+		)
 		if err != nil {
-			// The 200 has gone out. The client is left with a file
-			// that does not decompress; the log is the record.
 			h.log.Error(
 				"failed to export archive",
 				"target_id", target.ID,
 				"error", err,
 			)
+
+			// The 200 has gone out. Aborting the connection is what
+			// tells the client the file is incomplete.
+			panic(http.ErrAbortHandler)
 		}
 	}
 }
 
+// downloadWriter writes a download to the client, giving each write
+// downloadWriteTimeout to finish.
+type downloadWriter struct {
+	w  http.ResponseWriter
+	rc *http.ResponseController
+}
+
+func (d downloadWriter) Write(b []byte) (int, error) {
+	// A writer that has no write deadline, such as a test's recorder,
+	// answers http.ErrNotSupported and needs none extended.
+	err := d.rc.SetWriteDeadline(time.Now().Add(downloadWriteTimeout))
+	if err != nil && !errors.Is(err, http.ErrNotSupported) {
+		return 0, err
+	}
+
+	return d.w.Write(b)
+}
+
 // openTargetArchive opens the archive of the request's database target
-// for export. It reports false once it has written the response.
+// for export, with its reads under ctx. It reports false once it has
+// written the response.
 //
 // It holds renameMu, which every archive rename runs under, while it
 // reads the stored names and opens the file, so the file it opens is
 // the one those names give. It lets go before the export is streamed:
 // once the file is open, a rename does not affect the export.
 func (h *Handlers) openTargetArchive(
+	ctx context.Context,
 	w http.ResponseWriter,
 	r *http.Request,
 ) (database.Webhook, *database.Target, *delivery.ArchiveExport, bool) {
@@ -70,7 +109,7 @@ func (h *Handlers) openTargetArchive(
 	}
 
 	export, err := delivery.OpenArchiveExport(
-		r.Context(), delivery.ArchivePath(h.dbMgr, &webhook, target),
+		ctx, delivery.ArchivePath(h.dbMgr, &webhook, target), h.log,
 	)
 	if err != nil {
 		h.serverError(w, r, "failed to open archive for export", err)

internal/handlers/target_download_test.go

@@ -2,7 +2,10 @@ package handlers_test
 
 import (
 	"compress/gzip"
+	"crypto/rand"
 	"encoding/json"
+	"errors"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -10,11 +13,16 @@ import (
 	"testing"
 	"time"
 
+	chimw "github.com/go-chi/chi/middleware"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/delivery"
 )
 
+// errClientGone is the write failure of a client that has gone away.
+var errClientGone = errors.New("client gone")
+
 // downloadPath is the archive download route of a target.
 func downloadPath(webhookID, targetID string) string {
 	return "/hook/" + webhookID + "/targets/" + targetID + "/download"
@@ -183,3 +191,138 @@ func TestHandleTargetDownload_StreamsWithoutTheLock(t *testing.T) {
 	<-downloaded
 	assert.Equal(t, http.StatusOK, sw.Code)
 }
+
+// seedArchive writes rows to the archive file at path, each with a
+// body of bodySize random bytes, which do not compress. Its table has
+// only the columns the test fills; an export writes the others empty.
+func seedArchive(t *testing.T, path string, rows, bodySize int) {
+	t.Helper()
+
+	db, err := database.OpenSQLite(path, database.SQLiteModeCreate)
+	require.NoError(t, err)
+
+	defer func() { require.NoError(t, db.Close()) }()
+
+	_, err = db.ExecContext(t.Context(),
+		"CREATE TABLE archived_events (id INTEGER PRIMARY KEY, body TEXT)",
+	)
+	require.NoError(t, err)
+
+	body := make([]byte, bodySize)
+
+	for range rows {
+		_, _ = rand.Read(body)
+
+		_, err = db.ExecContext(t.Context(),
+			"INSERT INTO archived_events (body) VALUES (?)", string(body),
+		)
+		require.NoError(t, err)
+	}
+}
+
+// TestHandleTargetDownload_OutlastsTheRequestLimit proves a download
+// runs for as long as the client keeps reading. Behind a request limit
+// and a server write timeout of a tenth of a second, the client stops
+// reading once the response has started, waits three times as long,
+// and still gets the whole file. The archive is larger than a
+// connection holds, so the download is still being written while the
+// client waits.
+func TestHandleTargetDownload_OutlastsTheRequestLimit(t *testing.T) {
+	t.Parallel()
+
+	const (
+		limit    = 100 * time.Millisecond
+		rows     = 12
+		bodySize = 1 << 20
+	)
+
+	env := setupSourceTest(t)
+	wh := seedWebhookWithRetention(t, env.db, 7)
+	archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
+	seedArchive(
+		t, delivery.ArchivePath(env.dbMgr, &wh, archive), rows, bodySize,
+	)
+
+	srv := httptest.NewUnstartedServer(
+		chimw.Timeout(limit)(targetRouter(env)),
+	)
+	srv.Config.WriteTimeout = limit
+	srv.Start()
+	t.Cleanup(srv.Close)
+
+	req, err := http.NewRequestWithContext(
+		t.Context(), http.MethodGet,
+		srv.URL+downloadPath(wh.ID, archive.ID), nil,
+	)
+	require.NoError(t, err)
+
+	for _, c := range env.cookies {
+		req.AddCookie(c)
+	}
+
+	resp, err := srv.Client().Do(req)
+	require.NoError(t, err)
+
+	defer func() { _ = resp.Body.Close() }()
+
+	require.Equal(t, http.StatusOK, resp.StatusCode)
+
+	time.Sleep(3 * limit)
+
+	zr, err := gzip.NewReader(resp.Body)
+	require.NoError(t, err)
+
+	var (
+		got    map[string]json.RawMessage
+		events []json.RawMessage
+	)
+
+	require.NoError(t, json.NewDecoder(zr).Decode(&got))
+	require.NoError(t, json.Unmarshal(got["archived_events"], &events))
+	assert.Len(t, events, rows)
+
+	// Reading to the end makes the gzip reader check that the file was
+	// finished.
+	_, err = io.ReadAll(zr)
+	require.NoError(t, err)
+}
+
+// brokenWriter is a response writer whose writes fail once the
+// response has started, as they do when the client goes away.
+type brokenWriter struct {
+	*httptest.ResponseRecorder
+}
+
+func (b brokenWriter) Write(p []byte) (int, error) {
+	if b.Body.Len() > 0 {
+		return 0, errClientGone
+	}
+
+	return b.ResponseRecorder.Write(p)
+}
+
+// TestHandleTargetDownload_AbortsWhenItFails proves a download that
+// fails after its response has started aborts the connection, so the
+// client sees a failed download rather than a file that looks
+// complete and does not decompress.
+func TestHandleTargetDownload_AbortsWhenItFails(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+	wh := seedWebhookWithRetention(t, env.db, 7)
+	archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
+
+	req := httptest.NewRequestWithContext(
+		t.Context(), http.MethodGet, downloadPath(wh.ID, archive.ID), nil,
+	)
+	for _, c := range env.cookies {
+		req.AddCookie(c)
+	}
+
+	w := brokenWriter{ResponseRecorder: httptest.NewRecorder()}
+
+	assert.PanicsWithValue(t, http.ErrAbortHandler, func() {
+		targetRouter(env).ServeHTTP(w, req)
+	})
+	assert.Equal(t, http.StatusOK, w.Code)
+}

internal/handlers/webhook.go

@@ -11,6 +11,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/delivery"
 	"sneak.berlin/go/webhooker/internal/logfield"
+	"sneak.berlin/go/webhooker/internal/middleware"
 )
 
 const (
@@ -57,7 +58,8 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
 		h.log.Info("webhook request received",
 			"entrypoint_uuid", entrypointUUID,
 			"method", r.Method,
-			"remote_addr", r.RemoteAddr,
+			"remoteIP", middleware.RemoteIP(r),
+			"clientIP", middleware.ClientIP(r),
 		)
 
 		if !entrypoint.Active {

internal/handlers/webhook_test.go

@@ -0,0 +1,124 @@
+package handlers_test
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"strings"
+	"testing"
+
+	"github.com/go-chi/chi"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/handlers"
+	"sneak.berlin/go/webhooker/internal/middleware"
+)
+
+// TestHandleWebhook_LogsClientNextToThePeer checks that the
+// receiver's "webhook request received" line carries both addresses:
+// remoteIP, the connecting peer, and clientIP, the client the access
+// log attributes the request to.
+func TestHandleWebhook_LogsClientNextToThePeer(t *testing.T) {
+	t.Parallel()
+
+	// untrustedPeer is outside the trusted 10.0.0.0/8, so its
+	// X-Forwarded-For is ignored and it is the client.
+	const untrustedPeer = "192.0.2.10"
+
+	cases := map[string]struct {
+		peer       string
+		wantRemote string
+		wantClient string
+	}{
+		"trusted proxy with a forwarded chain": {
+			peer:       "10.0.0.1:44444",
+			wantRemote: "10.0.0.1",
+			wantClient: "198.51.100.7",
+		},
+		"untrusted peer": {
+			peer:       untrustedPeer + ":5555",
+			wantRemote: untrustedPeer,
+			wantClient: untrustedPeer,
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			var (
+				h  *handlers.Handlers
+				mw *middleware.Middleware
+				db *database.Database
+			)
+
+			app := newTestAppWithConfig(t, &config.Config{
+				DataDir: t.TempDir(),
+				TrustedProxies: []netip.Prefix{
+					netip.MustParsePrefix("10.0.0.0/8"),
+				},
+			}, &h, &mw, &db)
+			app.RequireStart()
+
+			t.Cleanup(app.RequireStop)
+
+			buf := new(bytes.Buffer)
+			h.SetLogForTest(slog.New(slog.NewJSONHandler(buf, nil)))
+
+			webhook := seedWebhook(t, db)
+			seedEntrypoint(t, db, webhook.ID)
+
+			// Logging is what works the client address out, so the
+			// request goes through it as it does in production.
+			router := chi.NewRouter()
+			router.Use(mw.Logging())
+			router.Post("/h/{uuid}", h.HandleWebhook())
+
+			req := httptest.NewRequestWithContext(
+				context.Background(), http.MethodPost,
+				"/h/ep-"+webhook.ID, strings.NewReader("{}"),
+			)
+			req.RemoteAddr = tc.peer
+			req.Header.Set("X-Forwarded-For", "198.51.100.7, 10.0.0.2")
+
+			w := httptest.NewRecorder()
+			router.ServeHTTP(w, req)
+
+			require.Equal(t, http.StatusOK, w.Code)
+
+			line := receivedLine(t, buf)
+			assert.Equal(t, tc.wantRemote, line["remoteIP"])
+			assert.Equal(t, tc.wantClient, line["clientIP"])
+		})
+	}
+}
+
+// receivedLine returns the one "webhook request received" line in the
+// captured JSON log.
+func receivedLine(t *testing.T, buf *bytes.Buffer) map[string]any {
+	t.Helper()
+
+	var found []map[string]any
+
+	for line := range strings.SplitSeq(
+		strings.TrimSpace(buf.String()), "\n",
+	) {
+		var entry map[string]any
+
+		require.NoError(t, json.Unmarshal([]byte(line), &entry))
+
+		if entry["msg"] == "webhook request received" {
+			found = append(found, entry)
+		}
+	}
+
+	require.Len(t, found, 1)
+
+	return found[0]
+}

internal/middleware/accesslog_test.go

@@ -63,6 +63,12 @@ const (
 // capturingMiddleware returns a Middleware whose logger writes JSON
 // lines into the returned buffer, so the access log can be asserted
 // on directly.
+//
+// It trusts 192.0.2.1, the peer address httptest.NewRequestWithContext
+// gives a request, as a proxy, the way a deployment trusts its reverse
+// proxy: a request built that way and carrying X-Forwarded-For is
+// logged with the client that header names as clientIP, and one
+// without it with the peer.
 func capturingMiddleware(t *testing.T) (*middleware.Middleware, *bytes.Buffer) {
 	t.Helper()
 
@@ -72,7 +78,10 @@ func capturingMiddleware(t *testing.T) (*middleware.Middleware, *bytes.Buffer) {
 		&slog.HandlerOptions{Level: slog.LevelInfo},
 	))
 
-	cfg := &config.Config{Environment: config.EnvironmentDev}
+	cfg := &config.Config{
+		Environment:    config.EnvironmentDev,
+		TrustedProxies: trustedProxies("192.0.2.1/32"),
+	}
 
 	return middleware.NewForTest(log, cfg, nil), buf
 }
@@ -81,7 +90,7 @@ func capturingMiddleware(t *testing.T) (*middleware.Middleware, *bytes.Buffer) {
 // internal/logger can select: slog's text handler, which
 // internal/logger/logger.go installs when stderr is a tty. It escapes
 // differently from the JSON one, so the line bound has to be asserted
-// against both.
+// against both. It trusts the same peer.
 func capturingTextMiddleware(
 	t *testing.T,
 ) (*middleware.Middleware, *bytes.Buffer) {
@@ -93,7 +102,10 @@ func capturingTextMiddleware(
 		&slog.HandlerOptions{Level: slog.LevelInfo},
 	))
 
-	cfg := &config.Config{Environment: config.EnvironmentDev}
+	cfg := &config.Config{
+		Environment:    config.EnvironmentDev,
+		TrustedProxies: trustedProxies("192.0.2.1/32"),
+	}
 
 	return middleware.NewForTest(log, cfg, nil), buf
 }
@@ -338,6 +350,7 @@ type sizeCase struct {
 	headers      map[string]string
 	wantStatus   int
 	wantURL      string
+	wantClientIP string
 	bound        int
 }
 
@@ -375,8 +388,7 @@ func lineSizeCases() map[string]sizeCase {
 	}
 
 	// The url field on a 5xx keeps the concrete path, so it reaches its
-	// own budget on the same line as the three header fields. That is
-	// the widest access log line the service can be made to write.
+	// own budget on the same line as the three header fields.
 	longPath := "/boom/" + strings.Repeat("x", oversizedSegmentBytes)
 	wantLongURL := longPath[:maxFieldBytes] + truncationSuffix
 
@@ -420,6 +432,29 @@ func lineSizeCases() map[string]sizeCase {
 			}
 	}
 
+	// From a trusted proxy, clientIP is read out of X-Forwarded-For,
+	// which the client writes. What bounds the field is that only one
+	// address from the header is written, and it is written parsed, with
+	// no zone. An IPv6 address with all eight groups at four digits is
+	// the longest such address; here it carries an 8 KB zone, which must
+	// not reach the line. It goes on the 5xx line with all three header
+	// fields at their budget.
+	const longestIPv6 = "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"
+
+	forwarded := oversizedHeaders(oversizedValue("h"))
+	forwarded[headerXFF] = oversizedValue("h") + ", " +
+		longestIPv6 + "%" + oversizedValue("h")
+
+	cases["oversized X-Forwarded-For from a trusted proxy "+
+		"with a 5xx concrete url"] = sizeCase{
+		target:       longPath,
+		headers:      forwarded,
+		wantStatus:   http.StatusInternalServerError,
+		wantURL:      wantLongURL,
+		wantClientIP: longestIPv6,
+		bound:        maxCappedLineBytes,
+	}
+
 	return cases
 }
 
@@ -460,6 +495,14 @@ func TestAccessLog_LineSizeDoesNotTrackInputSize(t *testing.T) {
 			require.Len(t, entries, 1)
 			assert.Equal(t, tc.wantURL, entries[0]["url"])
 
+			// Set only by the X-Forwarded-For case, where it proves
+			// the header was read rather than ignored.
+			if tc.wantClientIP != "" {
+				assert.Equal(
+					t, tc.wantClientIP, entries[0]["clientIP"],
+				)
+			}
+
 			// The markers sit at the far end of the client-chosen
 			// text, so their absence is what proves the redaction and
 			// the truncation actually ran.
@@ -648,7 +691,8 @@ func TestAccessLog_RetainsEveryOtherField(t *testing.T) {
 
 	for _, key := range []string{
 		"request_start", "method", "url", "useragent", "request_id",
-		"referer", "proto", "remoteIP", "status", "latency_ms",
+		"referer", "proto", "remoteIP", "clientIP", "status",
+		"latency_ms",
 	} {
 		assert.Contains(t, entries[0], key)
 	}

internal/middleware/clientip_test.go

@@ -0,0 +1,200 @@
+package middleware_test
+
+import (
+	"bytes"
+	"context"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/middleware"
+)
+
+const (
+	// forwardedChain is the X-Forwarded-For a request arrives with:
+	// the client, then a second proxy inside trustedProxyCIDR that the
+	// request passed through before reaching trustedPeer.
+	forwardedChain = clientIPv4 + ", 10.0.0.2"
+
+	// untrustedPeer is a peer outside trustedProxyCIDR, so its
+	// X-Forwarded-For is ignored and the peer is the client.
+	untrustedPeer = "192.0.2.10:5555"
+
+	// oneRequestPerMinute is the receiver limit these tests install:
+	// the second request on a path is rejected, and the aggregate
+	// limit is ReceiverAggregateMultiplierConst.
+	oneRequestPerMinute = 1
+)
+
+// clientLogSite is one log line that names the client. build wraps the
+// middleware that writes it around a handler, and requests is how many
+// identical requests it takes before the line is written.
+type clientLogSite struct {
+	build    func(m *middleware.Middleware) http.Handler
+	requests int
+}
+
+// clientLogSites maps the message of each line that names the client
+// to the way to make it be written.
+func clientLogSites() map[string]clientLogSite {
+	served := func(*middleware.Middleware) http.Handler {
+		return okHandler()
+	}
+
+	receiver := func(m *middleware.Middleware) http.Handler {
+		return m.ReceiverRateLimit()(okHandler())
+	}
+
+	login := func(m *middleware.Middleware) http.Handler {
+		return http.HandlerFunc(
+			func(w http.ResponseWriter, r *http.Request) {
+				m.RecordLoginFailure(r, "someone")
+				w.WriteHeader(http.StatusUnauthorized)
+			},
+		)
+	}
+
+	csrf := func(m *middleware.Middleware) http.Handler {
+		return m.CSRF(http.HandlerFunc(forbidden))(okHandler())
+	}
+
+	passwordChange := func(m *middleware.Middleware) http.Handler {
+		return m.PasswordChangeRateLimit()(okHandler())
+	}
+
+	replay := func(m *middleware.Middleware) http.Handler {
+		return m.ReplayRateLimit()(okHandler())
+	}
+
+	resubmit := func(m *middleware.Middleware) http.Handler {
+		return m.ResubmitRateLimit()(okHandler())
+	}
+
+	return map[string]clientLogSite{
+		"http request": {
+			build:    served,
+			requests: 1,
+		},
+		"webhook receiver rate limit exceeded": {
+			build:    receiver,
+			requests: oneRequestPerMinute + 1,
+		},
+		// The aggregate limit sits in front of the per-entrypoint
+		// one, so the requests that one rejects count towards it.
+		"webhook receiver aggregate rate limit exceeded": {
+			build: receiver,
+			requests: middleware.ReceiverAggregateMultiplierConst*
+				oneRequestPerMinute + 1,
+		},
+		"login failure limit exceeded": {
+			build:    login,
+			requests: middleware.LoginRateLimitConst + 1,
+		},
+		"csrf: token validation failed": {
+			build:    csrf,
+			requests: 1,
+		},
+		"password change rate limit exceeded": {
+			build:    passwordChange,
+			requests: middleware.PasswordChangeRateLimitConst + 1,
+		},
+		"delivery replay rate limit exceeded": {
+			build:    replay,
+			requests: middleware.ReplayRateLimitConst + 1,
+		},
+		"event resubmit rate limit exceeded": {
+			build:    resubmit,
+			requests: middleware.ResubmitRateLimitConst + 1,
+		},
+	}
+}
+
+// clientLogLines sends the site's requests from peer, each carrying
+// forwardedChain, through Logging and then the site, as production
+// does, and returns the logged lines whose message is msg.
+func clientLogLines(
+	t *testing.T, site clientLogSite, msg, peer string,
+) []map[string]any {
+	t.Helper()
+
+	buf := new(bytes.Buffer)
+	log := slog.New(slog.NewJSONHandler(
+		buf,
+		&slog.HandlerOptions{Level: slog.LevelDebug},
+	))
+
+	cfg := &config.Config{
+		Environment:       config.EnvironmentDev,
+		ReceiverRateLimit: oneRequestPerMinute,
+		TrustedProxies:    trustedProxies(trustedProxyCIDR),
+	}
+
+	m := middleware.NewForTest(
+		log, cfg, newTestSessionManager(cfg, log, nil),
+	)
+	handler := m.Logging()(site.build(m))
+
+	for range site.requests {
+		req := httptest.NewRequestWithContext(
+			context.Background(), http.MethodPost, "/h/x", nil,
+		)
+		req.RemoteAddr = peer
+		req.Header.Set(headerXFF, forwardedChain)
+
+		handler.ServeHTTP(httptest.NewRecorder(), req)
+	}
+
+	var lines []map[string]any
+
+	for _, entry := range accessLogEntries(t, buf) {
+		if entry["msg"] == msg {
+			lines = append(lines, entry)
+		}
+	}
+
+	return lines
+}
+
+// TestClientIP_LoggedNextToThePeer checks that every line that names
+// the client carries both addresses: remoteIP, the connecting peer,
+// and clientIP, the client the rate limiters key on.
+func TestClientIP_LoggedNextToThePeer(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]struct {
+		peer       string
+		wantRemote string
+		wantClient string
+	}{
+		"trusted proxy with a forwarded chain": {
+			peer:       trustedPeer,
+			wantRemote: "10.0.0.1",
+			wantClient: clientIPv4,
+		},
+		"untrusted peer": {
+			peer:       untrustedPeer,
+			wantRemote: "192.0.2.10",
+			wantClient: "192.0.2.10",
+		},
+	}
+
+	for msg, site := range clientLogSites() {
+		for name, tc := range cases {
+			t.Run(msg+"/"+name, func(t *testing.T) {
+				t.Parallel()
+
+				lines := clientLogLines(t, site, msg, tc.peer)
+				require.NotEmpty(t, lines, "%q was never logged", msg)
+
+				for _, line := range lines {
+					assert.Equal(t, tc.wantRemote, line["remoteIP"])
+					assert.Equal(t, tc.wantClient, line["clientIP"])
+				}
+			})
+		}
+	}
+}

internal/middleware/csrf.go

@@ -45,10 +45,10 @@ func (m *Middleware) CSRF(
 		// unauthenticated client: a POST with no token to
 		// /hook/<any length of any text>/edit lands here. The
 		// method and path are capped against the same budgets as
-		// the access log. remote_addr is set by net/http from the
-		// accepted connection rather than by the client, and
+		// the access log. remoteIP and clientIP are the same
+		// addresses the access log carries, and
 		// csrf.FailureReason returns one of gorilla/csrf's own
-		// fixed error values, so neither is client-sized.
+		// fixed error values, so none of them is client-sized.
 		m.log.Warn("csrf: token validation failed",
 			"method", logfield.Truncate(
 				r.Method, maxLogMethodBytes,
@@ -56,7 +56,8 @@ func (m *Middleware) CSRF(
 			"path", logfield.Truncate(
 				r.URL.Path, logfield.MaxBytes,
 			),
-			"remote_addr", r.RemoteAddr,
+			"remoteIP", RemoteIP(r),
+			"clientIP", ClientIP(r),
 			"reason", csrf.FailureReason(r),
 		)
 		forbidden.ServeHTTP(w, r)

internal/middleware/export_test.go

@@ -132,6 +132,12 @@ func (g *LoginGuard) TrackedKeysForTest() (int, int) {
 // passwordChangeRateLimit constant.
 const PasswordChangeRateLimitConst = passwordChangeRateLimit
 
+// ReplayRateLimitConst exposes the replayRateLimit constant.
+const ReplayRateLimitConst = replayRateLimit
+
+// ResubmitRateLimitConst exposes the resubmitRateLimit constant.
+const ResubmitRateLimitConst = resubmitRateLimit
+
 // ReceiverAggregateMultiplierConst exposes the
 // receiverAggregateMultiplier constant.
 const ReceiverAggregateMultiplierConst = receiverAggregateMultiplier

internal/middleware/loginguard.go

@@ -385,6 +385,8 @@ func (m *Middleware) RecordLoginFailure(
 			"path", logfield.Truncate(
 				r.URL.Path, logfield.MaxBytes,
 			),
+			"remoteIP", RemoteIP(r),
+			"clientIP", ClientIP(r),
 		)
 	}
 

internal/middleware/middleware.go

@@ -3,6 +3,7 @@
 package middleware
 
 import (
+	"context"
 	"log/slog"
 	"net"
 	"net/http"
@@ -69,18 +70,19 @@ const (
 	//	url, useragent, referer   3*(512+11) = 1569
 	//	request_id                    128+11 =  139
 	//	method                         32+11 =   43
-	//	fixed portion                        =  336
+	//	fixed portion                        =  405
 	//	                                        ----
-	//	                                        2087
+	//	                                        2156
 	//
 	// The 512 is logfield.MaxBytes; the 11 is the truncation marker,
 	// charged on top of each budget rather than inside it.
 	//
 	// The fixed portion is the JSON punctuation, the field names, the
-	// level and the message, both timestamps at their longest, an IPv6
-	// remoteIP with a zone, a three-digit status and a full-width int64
-	// latency. Stated at 2560 so the figure carries headroom rather
-	// than sitting on the arithmetic.
+	// level and the message, both timestamps at their longest, remoteIP
+	// and clientIP each charged as an IPv6 address with a zone, a
+	// three-digit status and a full-width int64 latency. Stated at 2560
+	// so the figure carries headroom rather than sitting on the
+	// arithmetic.
 	//
 	// The tty text handler in internal/logger is covered by the same
 	// figure. logfield.EncodedBytes charges every rune at least what
@@ -88,8 +90,8 @@ const (
 	// bytes strconv.Quote spends on a non-printable rune at or above
 	// U+10000, which is four more than the JSON handler ever spends —
 	// so each budget bounds the encoded field under either handler.
-	// The text handler's fixed portion is 286, the smaller of the two,
-	// which puts its worst case at 2037.
+	// The text handler's fixed portion is 351, the smaller of the two,
+	// which puts its worst case at 2102.
 	//
 	// It is also the ceiling on every OTHER line this service writes
 	// THROUGH SLOG that carries text an UNAUTHENTICATED client
@@ -215,6 +217,28 @@ func ipFromHostPort(hp string) string {
 	return h
 }
 
+// RemoteIP returns the address of the connecting peer, without its
+// port. Behind a reverse proxy it is the proxy. Every log line that
+// names the client logs it as remoteIP, next to clientIP.
+func RemoteIP(r *http.Request) string {
+	return ipFromHostPort(r.RemoteAddr)
+}
+
+// clientIPKey is the request context key under which Logging stores
+// the value ClientIP returns.
+type clientIPKey struct{}
+
+// ClientIP returns the address the request is attributed to, which
+// Logging works out once per request with clientAddr in ratelimit.go
+// and logs as clientIP. The other lines that name the client read it
+// from here, so all of them agree. It is empty for a request Logging
+// has not seen.
+func ClientIP(r *http.Request) string {
+	ip, _ := r.Context().Value(clientIPKey{}).(string)
+
+	return ip
+}
+
 type loggingResponseWriter struct {
 	http.ResponseWriter
 
@@ -316,6 +340,13 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
 			lrw := newLoggingResponseWriter(w)
 			ctx := r.Context()
 
+			// When RemoteAddr is not an address, the peer's own
+			// text is all the request can be attributed to.
+			clientIP := RemoteIP(r)
+			if addr, ok := s.clientAddr(r); ok {
+				clientIP = addr.String()
+			}
+
 			defer func() {
 				latency := time.Since(start)
 				requestID := ""
@@ -350,13 +381,16 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
 						r.Referer(), logfield.MaxBytes,
 					),
 					"proto", r.Proto,
-					"remoteIP", ipFromHostPort(r.RemoteAddr),
+					"remoteIP", RemoteIP(r),
+					"clientIP", clientIP,
 					"status", lrw.statusCode,
 					"latency_ms", latency.Milliseconds(),
 				)
 			}()
 
-			next.ServeHTTP(lrw, r)
+			next.ServeHTTP(lrw, r.WithContext(
+				context.WithValue(ctx, clientIPKey{}, clientIP),
+			))
 		})
 	}
 }

internal/middleware/ratelimit.go

@@ -202,24 +202,17 @@ func (m *Middleware) forwardedClientAddr(
 }
 
 // rateLimitKey is the client identity every rate limiter in this
-// package buckets on. Forwarded headers are honoured only when the
-// direct peer (RemoteAddr) is inside the configured trusted-proxy
-// set; otherwise the peer address itself is the key. Without that
-// gate any client could mint a fresh bucket per request, or starve
-// another client's bucket, by picking an X-Forwarded-For value —
-// which makes every limit here decorative against a deliberate
-// attacker.
-//
-// The address that identifies the client is then reduced to a bucket
-// by bucketKey: full address for IPv4, /64 prefix for IPv6.
+// package buckets on: the address clientAddr attributes the request
+// to, reduced to a bucket by bucketKey — full address for IPv4, /64
+// prefix for IPv6.
 func (m *Middleware) rateLimitKey(r *http.Request) (string, error) {
 	return m.clientKey(r), nil
 }
 
 // clientKey computes the bucket key described on rateLimitKey.
 func (m *Middleware) clientKey(r *http.Request) string {
-	peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
-	if err != nil {
+	addr, ok := m.clientAddr(r)
+	if !ok {
 		// Not an address we can reason about; key on the raw
 		// value, the most specific identity left. Distinct
 		// RemoteAddr values stay in distinct buckets, so this
@@ -230,16 +223,36 @@ func (m *Middleware) clientKey(r *http.Request) string {
 		return r.RemoteAddr
 	}
 
-	peer = normalizeAddr(peer)
-	if !m.isTrustedProxy(peer) {
-		return bucketKey(peer)
-	}
-
-	if addr, ok := m.forwardedClientAddr(r); ok {
 	return bucketKey(addr)
 }
 
-	return bucketKey(peer)
+// clientAddr is the address a request is attributed to. The rate
+// limiters key on it and the logs name it as clientIP.
+//
+// Forwarded headers are honoured only when the direct peer
+// (RemoteAddr) is inside the configured trusted-proxy set; otherwise
+// the peer address itself is the client. Without that gate any client
+// could mint a fresh bucket per request, or starve another client's
+// bucket, by picking an X-Forwarded-For value — which makes every
+// limit here decorative against a deliberate attacker.
+//
+// ok is false when RemoteAddr is not an address at all.
+func (m *Middleware) clientAddr(r *http.Request) (netip.Addr, bool) {
+	peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
+	if err != nil {
+		return netip.Addr{}, false
+	}
+
+	peer = normalizeAddr(peer)
+	if !m.isTrustedProxy(peer) {
+		return peer, true
+	}
+
+	if addr, ok := m.forwardedClientAddr(r); ok {
+		return addr, true
+	}
+
+	return peer, true
 }
 
 // tooManyRequests returns the 429 handler used by the
@@ -262,6 +275,8 @@ func (m *Middleware) tooManyRequests(
 			"path", logfield.Truncate(
 				r.URL.Path, logfield.MaxBytes,
 			),
+			"remoteIP", RemoteIP(r),
+			"clientIP", ClientIP(r),
 		)
 		http.Error(w, responseMessage, http.StatusTooManyRequests)
 	}
@@ -286,8 +301,12 @@ func (m *Middleware) tooManyRequests(
 func (m *Middleware) floodTooManyRequests(
 	logMessage, responseMessage string,
 ) http.HandlerFunc {
-	return func(w http.ResponseWriter, _ *http.Request) {
-		m.log.Debug(logMessage)
+	return func(w http.ResponseWriter, r *http.Request) {
+		m.log.Debug(
+			logMessage,
+			"remoteIP", RemoteIP(r),
+			"clientIP", ClientIP(r),
+		)
 		http.Error(w, responseMessage, http.StatusTooManyRequests)
 	}
 }

internal/server/alpine_browser_test.go

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

script/assets

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

script/test

@@ -27,7 +27,11 @@
 # Those figures predate tests hashing the admin password at 1 MB instead of
 # 64 MB (https://git.eeqj.de/sneak/webhooker/pulls/404). After that change, in
 # a cache-defeated build at host load 44-109 (2026-10-02), internal/handlers
-# took 8.5s and the slowest package was internal/database at 15.8s.
+# took 8.5s and the slowest package was internal/database at 15.8s. Once its
+# retention tests seeded 50 rows per insert instead of 500
+# (https://git.eeqj.de/sneak/webhooker/issues/198), internal/database took
+# 7.3s and the slowest package was internal/handlers at 8.1s to 10.0s, at host
+# load 25-48 (2026-10-02).
 #
 # -p 4 -parallel 8 keep the run under 2 GB of memory: at most four test
 # binaries build or run at once, each with at most eight parallel tests. Under

script/test-browser

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

static/js/app.js

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

templates/navbar.html

@@ -1,5 +1,5 @@
 {{define "navbar"}}
-<nav class="app-bar" x-data="{ open: false }">
+<nav class="app-bar" x-data="collapsible">
     <div class="max-w-6xl mx-auto flex justify-between items-center">
         <div class="flex items-center gap-3">
             <a href="/" class="text-xl font-medium text-gray-900 hover:text-primary-600 transition-colors">Webhooker</a>
@@ -7,9 +7,9 @@
 
         <!-- Mobile menu button -->
         {{if .User}}
-        <button @click="open = !open" class="md:hidden p-2 rounded-md text-gray-500 hover:bg-gray-100">
+        <button @click="toggle" class="md:hidden p-2 rounded-md text-gray-500 hover:bg-gray-100">
             <svg class="w-6 h-6" fill="none" stroke="currentColor" viewBox="0 0 24 24">
-                <path x-show="!open" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 6h16M4 12h16M4 18h16"/>
+                <path x-show="closed" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 6h16M4 12h16M4 18h16"/>
                 <path x-show="open" x-cloak stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M6 18L18 6M6 6l12 12"/>
             </svg>
         </button>

templates/source_detail.html

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

templates/source_logs.html

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