Bound the access log line against client-chosen text (closes #146)

The access log wrote one INFO line per request carrying
r.URL.String(). Registered with Use, it runs ahead of the route
limiter, so a client flooding the unauthenticated receiver with
invented paths wrote attacker-chosen text of attacker-chosen length
into the operator's log, one line per request.

3xx and 4xx responses now log the chi route pattern in place of the
concrete URL, and the fixed literal "(unmatched)" when routing matched
nothing at all. One line per request is retained, so real traffic
stays observable and rate accounting still works, but the line's
content is now bounded by the service's own route table. The pattern
is only populated after routing, so it is read in the deferred part of
the handler rather than before next.ServeHTTP.

The route pattern alone does not close the hole, because it leaves two
other ways for a request to choose the size of the line it writes.

The query string is one: /.well-known/healthcheck and /s/* answer 200
to anyone with no rate limiter in front of them, and /pages/login
behind only the login limiter, so appending 8 KB after the '?' bought
the same amplification as an invented 404 path. The branches that keep
the concrete URL now log the path only, with the query replaced by the
fixed marker "?(redacted)". Nothing debuggable is lost: `page`, on the
authenticated pagination links, is the only query parameter this
service reads.

The headers are the other: useragent and referer are logged on every
line, including the correctly redacted ones, so an 8 KB User-Agent
plus an 8 KB Referer produced a 24 KB line whose url field read
"(unmatched)". Each field a client supplies is now truncated rather
than dropped -- a truncated User-Agent is still worth reading -- to
512 bytes for url, useragent and referer, 128 for request_id (chi
passes an inbound X-Request-Id header straight through), and 32 for
method, which Go accepts as any token up to the header size limit.
Truncation also drops invalid UTF-8, which an encoder would otherwise
expand six-fold past the budget.

Each budget is spent in encoded bytes rather than in the bytes the
client sent, because the line an operator stores is the encoded one.
slog's JSON handler escapes a quotation mark, a backslash and a tab to
two bytes each and a non-printable rune to six; its text handler
spells any non-printable rune the same six-byte way; and Go's header
parser accepts all of them in a header value. Counted raw, a 512-byte
budget therefore bought a 1,024-byte field. Plain ASCII still encodes
one byte for one, so a real browser's User-Agent fits whole, while a
value built out of escapes keeps a proportionally shorter prefix.

A complete line is now at most 2,560 bytes: 3*(512+11) for url,
useragent and referer, 128+11 for request_id, 32+11 for method and a
336-byte fixed portion come to 2,087, stated with headroom. The tests
assert it against 8 KB in the path, in the query and in each of the
three headers, including values built from the characters the handler
escapes, and against a 5xx whose concrete url is at its own budget on
the same line. The README states it so an operator can size log
storage against it.

README.md

@@ -11,14 +11,9 @@ with retry support, logging, and observability. Category: infrastructure
 
 ### Prerequisites
 
-- Go 1.26.1+ (the version in `go.mod`)
+- Go 1.26+
-- golangci-lint v2.12.2 (the version pinned in `script/bootstrap` and
+- golangci-lint v2.11+
-  in the `Dockerfile`'s lint stage; `make bootstrap` installs it)
+- Docker (for containerized deployment)
-- Docker (for containerized deployment, and for the lint and test
-  stages of the CI gate)
-- `curl`, used by `script/fetch-assets` to download the third-party
-  browser assets, which are not committed (`make bootstrap` installs
-  it if missing)
 
 ### Quick Start
 
@@ -27,18 +22,14 @@ with retry support, logging, and observability. Category: infrastructure
 git clone https://git.eeqj.de/sneak/webhooker.git
 cd webhooker
 
-# Install Go dependencies, the pinned linter, and the third-party
+# Install Go dependencies
-# browser assets. `make deps` alone is not enough: it only runs
+make deps
-# go mod download/tidy, and the checks below need the fetched assets.
-make bootstrap
 
-# Run all checks (test, lint, format check)
+# Run all checks (format, lint, test, build)
 make check
 
-# Run in development mode. DATA_DIR defaults to /var/lib/webhooker in
+# Run in development mode (uses SQLite in current directory)
-# every environment, so set it (in .env or the shell) to a writable
+make dev
-# directory when running from a clone.
-DATA_DIR=./data make dev
 
 # Build Docker image
 make docker
@@ -51,18 +42,13 @@ make bootstrap  # Install all dependencies (idempotent)
 make setup      # Bootstrap + install git pre-commit hook
 make assets     # Fetch + verify third-party browser assets
 make fmt        # Format code (gofmt + goimports)
-make fmt-check  # Fail if gofmt would change anything (writes nothing)
 make lint       # Run golangci-lint
 make test       # Run tests with race detection
 make check      # test + lint + fmt-check (CI gate)
 make build      # Build binary to bin/webhooker
-make run        # build, then run ./bin/webhooker
 make dev        # go run ./cmd/webhooker
-make deps       # go mod download + go mod tidy
 make docker     # Build Docker image
 make hooks      # Install git pre-commit hook that runs script/precommit
-make css        # Regenerate static/css/tailwind.css (needs tailwindcss)
-make clean      # Remove bin/
 ```
 
 ### Configuration
@@ -104,7 +90,7 @@ TTY detection, and security headers are always applied.
 | `PORT`                  | HTTP listen port                    | `8080`   |
 | `DATA_DIR`              | Directory for all SQLite databases  | `/var/lib/webhooker` |
 | `DEBUG`                 | Enable debug logging                | `false`  |
-| `MAINTENANCE_MODE`      | Report `maintenanceMode: true` in the healthcheck JSON. It does not change how any request is served — no maintenance page exists | `false`  |
+| `MAINTENANCE_MODE`      | Serve the maintenance page          | `false`  |
 | `METRICS_USERNAME`      | Basic auth username for `/metrics`  | `""`     |
 | `METRICS_PASSWORD`      | Basic auth password for `/metrics`  | `""`     |
 | `SENTRY_DSN`            | Sentry error reporting DSN          | `""`     |
@@ -144,13 +130,8 @@ sustained trickle re-locks them immediately.
 
 The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's
 address, which restores per-client buckets. webhooker logs a warning
-at startup whenever `TRUSTED_PROXIES` is empty, in every environment —
+at startup when `WEBHOOKER_ENVIRONMENT=prod` and `TRUSTED_PROXIES` is
-not only when `WEBHOOKER_ENVIRONMENT=prod`, because that variable
+empty. See [Rate Limiting](#rate-limiting) for what each limit shares.
-defaults to `dev` and an operator who never set it is precisely the
-one at risk. The warning is informational when nothing proxies to the
-process: with no proxy in front, the peer address is the client's own
-and the buckets are already per-client. See
-[Rate Limiting](#rate-limiting) for what each limit shares.
 
 `X-Real-IP` and `True-Client-IP` are **never** read, from any peer.
 Reverse proxies append to `X-Forwarded-For` but forward other client
@@ -182,8 +163,6 @@ Two operator requirements follow:
   makes all three limits, including the unauthenticated webhook
   receiver, silently bypassable by every client in the block.
 
-#### Sessions
-
 Sessions are bounded by two independent clocks, and end at whichever
 one runs out first:
 
@@ -253,20 +232,17 @@ docker run -d \
 The container runs as a non-root user (`webhooker`, UID 1000), exposes
 port 8080, and includes a health check against
 `/.well-known/healthcheck`. The `/var/lib/webhooker` volume holds all
-SQLite databases: the main application database (`webhooker.db`), the
+SQLite databases: the main application database (`webhooker.db`) and
-per-webhook event databases (`events-{uuid}.db`), and any archive
+the per-webhook event databases (`events-{uuid}.db`). Mount this as a
-databases written by `database` targets (`archive-{uuid}.db`). Mount
+persistent volume to preserve data across container restarts.
-this as a persistent volume to preserve data across container
-restarts.
 
 ## Entrypoints
 
 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 sixteen targets are thin
+development workflow, and the Makefile targets are thin shims that call
-shims that call them; `build`, `run`, `dev`, `deps`, `clean` and `css`
+them. We provide:
-are inline commands with no script behind them. We provide:
 
 - `script/bootstrap` — install all dependencies (idempotent)
 - `script/setup` — make a fresh clone ready for development
@@ -378,11 +354,7 @@ It uses:
 - **[gorilla/csrf](https://github.com/gorilla/csrf)** for CSRF
   protection (cookie-based double-submit tokens)
 - **[go-chi/httprate](https://github.com/go-chi/httprate)** for
-  sliding-window rate limiting of the login, password-change and
+  per-IP login rate limiting (sliding window counter)
-  webhook receiver endpoints. The bucket is per client IP only when
-  `TRUSTED_PROXIES` names the reverse proxy; unset, every client
-  behind that proxy shares one bucket per limit (see
-  [Rate Limiting](#rate-limiting))
 - **[Prometheus](https://prometheus.io)** for metrics, served at
   `/metrics` behind basic auth
 - **[Sentry](https://sentry.io)** for optional error reporting
@@ -400,7 +372,7 @@ The codebase uses consistent naming throughout (rename completed in
 
 ### Data Model
 
-webhooker's data model has nine entities organized into two tiers: the
+webhooker's data model has eight entities organized into two tiers: the
 **application tier** (user and webhook configuration) and the **event
 tier** (event ingestion, delivery, and logging).
 
@@ -492,25 +464,9 @@ days (`database.RetentionForeverDays`). The retention reaper recognises
 that sentinel and skips the webhook entirely, and the web UI displays
 such a webhook's retention as "forever" rather than as a day count.
 
-Submitted `retention_days` values therefore fall into three bands, not
+A *finite* retention is capped at `database.MaxFiniteRetentionDays`
-two:
+(106751 days, about 292 years), and a larger one is rejected with a
-
+400. The cap is not arbitrary: the reaper computes its cutoff as a
-- `1` up to `database.MaxFiniteRetentionDays` (106751 days, about 292
-  years) is accepted as a finite retention.
-- Above that ceiling but below the retain-forever sentinel of 365000
-  (`database.RetentionForeverDays`) is rejected with a 400. This is the
-  band the cap exists for.
-- `0`, and `365000` or above, are accepted and mean retain forever,
-  collapsing to the sentinel — `0` in `Webhook.BeforeSave`, the large
-  values in `parseRetentionDays`. The large values are not out of
-  range: the edit form pre-fills the sentinel for a retain-forever
-  webhook, so submitting that form back unchanged has to keep meaning
-  "forever".
-
-A negative value is in none of the three: `parseRetentionDays` rejects
-it with a 400 before `BeforeSave` ever sees it.
-
-The cap is not arbitrary: the reaper computes its cutoff as a
 `time.Duration`, an int64 nanosecond count, and a longer period
 overflows it. An overflowed cutoff lands in the future, where it
 matches every row, so the sweep would delete every event the webhook
@@ -528,7 +484,7 @@ the full request and creates an Event.
 | -------------- | ------- | ----------- |
 | `id`           | UUID    | Primary key |
 | `webhook_id`   | UUID    | Foreign key → Webhook |
-| `path`         | string  | Unique bare UUID, generated at creation. The `/webhook/` prefix is route only and is not stored: the receiver matches this column against the raw `{uuid}` path segment |
+| `path`         | string  | Unique URL path (UUID-based, e.g. `/webhook/{uuid}`) |
 | `description`  | string  | Optional description |
 | `active`       | boolean | Whether this entrypoint accepts events (default: true) |
 
@@ -552,8 +508,8 @@ events should be forwarded.
 | `type`           | TargetType | One of: `http`, `slack`, `database`, `log` |
 | `active`         | boolean    | Whether deliveries are enabled (default: true) |
 | `config`         | JSON text  | Type-specific configuration |
-| `max_retries`    | integer    | Maximum retry attempts for `http` and `slack` targets (0 = fire-and-forget, >0 = retries with backoff and a circuit breaker). Ignored by `database` and `log` targets |
+| `max_retries`    | integer    | Maximum retry attempts for HTTP targets (0 = fire-and-forget, >0 = retries with backoff) |
-| `max_queue_size` | integer    | Stored and shown on the target's detail view, but not enforced anywhere yet: nothing in the delivery engine consults it. Queue depth is set by the two fixed 10,000-entry channels |
+| `max_queue_size` | integer    | Maximum queued deliveries (for HTTP targets with retries) |
 
 **Relations:** Belongs to Webhook. Has many Deliveries.
 
@@ -566,11 +522,6 @@ events should be forwarded.
   greater than 0, failed deliveries are retried with exponential backoff
   up to `max_retries` attempts, protected by a per-target circuit
   breaker.
-- **`slack`** — Post the event as a formatted message to a
-  Slack-compatible incoming webhook URL (`webhookUrl` in `config`). It
-  is built on the same HTTP core as `http` and honours `max_retries`
-  identically, circuit breaker included. See the Slack target section
-  under "Per-Webhook Event Databases" for the message format.
 - **`database`** — Archive the full event as a row into a separate
   per-webhook archive database (`archive-{webhookID}.db`) for long-term
   retention, with an optional creation-validated expiry (default: keep
@@ -607,7 +558,7 @@ data for auditing and for the planned replay capability.
 | `id`             | UUID   | Primary key |
 | `webhook_id`     | UUID   | Foreign key → Webhook |
 | `entrypoint_id`  | UUID   | Foreign key → Entrypoint |
-| `method`       | string | HTTP method of the captured request. Always `POST`: the receiver answers every other method with 405 before an Event is created |
+| `method`       | string | HTTP method (POST, PUT, etc.) |
 | `headers`      | JSON   | Complete request headers |
 | `body`         | text   | Raw request body |
 | `content_type` | string | Content-Type header value |
@@ -662,9 +613,7 @@ retries) is individually logged for full observability.
 
 #### Common Fields
 
-Every entity except `Setting` includes these fields from `BaseModel`.
+All entities include these fields from `BaseModel`:
-`Setting` is a bare key-value row with no `id`, no timestamps and no
-soft delete:
 
 | Field        | Type      | Description |
 | ------------ | --------- | ----------- |
@@ -710,7 +659,7 @@ handles connection pooling, lazy opening, migrations, and cleanup.
 This separation provides:
 
 - **Isolation** — a high-volume webhook won't cause lock contention or
-  journal growth affecting the main application or other webhooks.
+  WAL bloat affecting the main application or other webhooks.
 - **Independent lifecycle** — event databases can be independently
   backed up, archived, rotated, or size-limited without impacting the
   application.
@@ -720,12 +669,9 @@ This separation provides:
 - **Per-webhook retention** — the `retention_days` field on each webhook
   controls automatic cleanup of old events in that webhook's database
   only, or disables cleanup entirely when set to `0` (retain forever).
-- **Performance** — each webhook's database has its own page cache and
+- **Performance** — each webhook's database has its own WAL, its own
-  its own lock, so concurrent event ingestion across webhooks won't
+  page cache, and its own lock, so concurrent event ingestion across
-  contend. No write-ahead log is involved: both DSNs are
+  webhooks won't contend.
-  `file:{path}?cache=shared&mode=rwc` and no `journal_mode` pragma is
-  ever issued, so every database runs on SQLite's default rollback
-  journal.
 
 The **database target type** builds on this architecture to provide
 long-term archiving, separate from the per-webhook event database (which
@@ -781,9 +727,8 @@ and other compatible services). Each message includes event metadata
 pretty-printed in a code block. JSON payloads are automatically
 formatted with indentation for readability; non-JSON payloads are shown
 as raw text. Large payloads are truncated to keep messages reasonable.
-Config stores `webhookUrl` — the Slack/Mattermost incoming webhook
+Config stores `webhook_url` — the Slack/Mattermost incoming webhook
-endpoint. That is the JSON key; the error text for a missing one reads
+endpoint.
-`webhook_url is required`, which is the message, not the key.
 
 The database uses the
 [modernc.org/sqlite](https://pkg.go.dev/modernc.org/sqlite) driver at
@@ -805,9 +750,8 @@ External Service
                      1. Look up Entrypoint by UUID
                      2. Capture full request as Event
                      3. Create Delivery records for each active Target
-                     4. Build self-contained delivery.Task structs
+                     4. Build self-contained DeliveryTask structs
-                        (target config + event data inline for
+                        (target config + event data inline for ≤16KB)
-                         bodies < 16 KiB)
                      5. Notify Engine via channel (no DB read needed)
                                                 │
                                                 ▼
@@ -842,7 +786,7 @@ at any time, preventing goroutine explosions regardless of queue depth.
   a delivery channel (new tasks from the webhook handler) and a retry
   channel (tasks from backoff timers). Both are buffered to 10,000.
 - **Fan-out via channel, not goroutines:** When an event arrives with
-  multiple targets, each `delivery.Task` is sent to the delivery channel.
+  multiple targets, each `DeliveryTask` is sent to the delivery channel.
   Workers pick them up and process them — no goroutine-per-target.
 - **Worker goroutines:** A fixed number of worker goroutines select from
   both channels. Each worker processes one task at a time, then picks up
@@ -866,12 +810,7 @@ This means:
 - **Independent results** — each worker records its own delivery result
   in the per-webhook database without coordination.
 - **Graceful shutdown** — cancel the context, workers finish their
-  current task and exit. The stop hook waits for the pool via
+  current task and exit. `WaitGroup.Wait()` ensures clean shutdown.
-  `lifecycle.WaitForShutdown`, which bounds that wait by fx's stop
-  timeout rather than blocking forever on a wedged worker. On timeout
-  it logs at `ERROR` and returns an error, and the goroutines that
-  did not finish are still running — an unclean shutdown is reported
-  rather than hidden.
 
 **Recovery paths:**
 
@@ -899,13 +838,12 @@ remains stored in the per-webhook event database, there is no way to
 redeliver it: manual redelivery is planned, not implemented (see
 [TODO.md](TODO.md)).
 
-### Circuit Breaker (HTTP and Slack Targets with Retries)
+### Circuit Breaker (HTTP Targets with Retries)
 
-`http` and `slack` targets with `max_retries` > 0 are protected by a
+HTTP targets with `max_retries` > 0 are protected by a **per-target circuit breaker** that
-**per-target circuit breaker** that prevents hammering a down target
+prevents hammering a down target with repeated failed delivery attempts.
-with repeated failed delivery attempts. The circuit breaker is
+The circuit breaker is in-memory only and resets on restart (which is
-in-memory only and resets on restart (which is fine — startup recovery
+fine — startup recovery rescans the database anyway).
-rescans the database anyway).
 
 **States:**
 
@@ -941,12 +879,10 @@ rescans the database anyway).
 - **Failure threshold:** 5 consecutive failures before opening
 - **Cooldown:** 30 seconds in open state before probing
 
-**Scope:** Circuit breakers apply to **`http` and `slack` targets with
+**Scope:** Circuit breakers only apply to **HTTP targets with
-`max_retries` > 0**. The Slack target is built on the same HTTP core
+`max_retries` > 0**. Fire-and-forget HTTP targets (`max_retries` == 0),
-and hands its own `max_retries` to the same retry path, so it gets a
+Slack targets, database targets (local operations), and log
-breaker with the same 5-failure / 30-second defaults. Fire-and-forget
+targets (stdout) do not use circuit breakers.
-targets of either type (`max_retries` == 0), database targets (local
-operations), and log targets (stdout) do not use circuit breakers.
 
 When a circuit is open and a new delivery arrives, the engine marks the
 delivery as `retrying` and schedules a retry timer for after the
@@ -962,11 +898,9 @@ unpredictable rates, and blanket limits shared with other routes would
 cause legitimate deliveries to be dropped.
 
 The receiver instead has its own dedicated abuse limit, scoped to the
-`/webhook/{uuid}` route only and keyed per client IP per request path
+`/webhook/{uuid}` route only and keyed per client IP per entrypoint: one
-(`httprate.KeyByEndpoint`): one misbehaving sender is throttled without
+misbehaving sender is throttled without affecting other senders of the
-affecting other senders of the same entrypoint or the same sender's
+same entrypoint or the same sender's other entrypoints. The limit is
-other entrypoints. Keying on the path rather than on the entrypoint
-matters — see the aggregate limit below. The limit is
 `RECEIVER_RATE_LIMIT` requests per minute (default 120, generous for
 legitimate webhook senders). Requests over the limit receive HTTP 429
 with a `Retry-After` header. A set-but-invalid `RECEIVER_RATE_LIMIT`
@@ -1026,16 +960,14 @@ reading; an absent one is not. A cut value ends in `[truncated]`, which
 is charged on top of the budget rather than inside it.
 
 Each budget is spent in _encoded_ bytes, not in the bytes the client
-sent. Every rune is charged what the wider of the two log handlers
+sent. The log handler escapes a quotation mark, a backslash and a tab
-emits for it: two bytes for a quotation mark, a backslash or a tab; six
+to two bytes each and a non-printable rune to six, and Go's header
-for a non-printable rune below U+10000; ten for one at or above it,
+parser accepts all of them in a header value, so a budget counted raw
-which the text handler spells `\UXXXXXXXX`. Go's header parser accepts
+would buy a field twice its nominal size — and the line, not the
-all of them in a header value, so a budget counted raw would buy a
+header, is what an operator has to store. Plain ASCII encodes one byte
-field several times its nominal size — and the line, not the header, is
+for one, so a real browser's `User-Agent` still fits whole; a value
-what an operator has to store. Plain ASCII encodes one byte for one, so
+built out of escapes keeps a proportionally shorter prefix, which is
-a real browser's `User-Agent` still fits whole; a value built out of
+the right trade.
-escapes keeps a proportionally shorter prefix, which is the right
-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`,
@@ -1046,13 +978,10 @@ 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`,
-including cases built from the characters the handlers escape, and
+including cases built from the characters the handler escapes, and
 against the widest 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
+budget. Measured over a real connection, that line is 1,972 bytes.
-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 line is 1,972 bytes.
 
 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:
@@ -1063,14 +992,7 @@ Every limiter here — receiver, login, and password change — identifies
 the client the same way, through one shared key function: the
 connection's own address, unless the peer is listed in
 `TRUSTED_PROXIES`, in which case the forwarded client address is used
-instead. That address becomes a bucket by family: IPv4 keys on the full
+instead. See [Trusted proxies](#trusted-proxies). Deployed without that
-address, IPv6 on its `/64` prefix. A routed `/64` is the normal
-residential and mobile IPv6 allocation, so keying IPv6 per address would
-let one subscriber rotate source addresses and mint a fresh bucket per
-request, evading these limits at the network layer without spoofing
-anything; the cost is that distinct clients inside one `/64` share a
-bucket. IPv4-mapped addresses (`::ffff:1.2.3.4`) key as the IPv4 address
-they carry. See [Trusted proxies](#trusted-proxies). Deployed without that
 variable set, a client behind a reverse proxy shares one bucket with
 every other client behind the same proxy. Set `TRUSTED_PROXIES` to the
 proxy's address to get per-client limits back. What the shared bucket
@@ -1093,8 +1015,8 @@ opposite directions:
   login bucket full, and the operator's own login returns HTTP 429 for
   as long as that trickle continues. A restart clears the in-memory
   buckets and a resumed trickle re-locks them. Production deployments
-  must set `TRUSTED_PROXIES`; webhooker warns at startup whenever it is
+  must set `TRUSTED_PROXIES`; webhooker warns at startup when it is
-  empty, in any environment.
+  empty in `prod`.
 
 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
@@ -1106,17 +1028,17 @@ abuse limit later; they are tracked as future work.
 
 | Method | Path                        | Description |
 | ------ | --------------------------- | ----------- |
-| `GET`  | `/`                         | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) |
+| `GET`  | `/`                         | Root redirect (authenticated → `/sources`, unauthenticated → `/pages/login`) |
-| `GET`  | `/.well-known/healthcheck`  | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) |
+| `GET`  | `/.well-known/healthcheck`  | Health check (JSON: status, uptime, version) |
-| any    | `/s/*`                      | Static file serving (embedded CSS, JS). Mounted for every method, not just `GET`/`HEAD`: chi's `Mount` registers all methods and `http.FileServer` special-cases only `HEAD` (by omitting the body), so a `POST` or `DELETE` to an asset is answered `200` with the file. Pinned by `TestStaticServesEveryMethod` |
+| `GET`  | `/s/*`                      | Static file serving (embedded CSS, JS) |
-| `POST` | `/webhook/{uuid}`           | Webhook receiver endpoint. `POST` only — every other method is answered `405 Method Not Allowed` with `Allow: POST`. Rate limited (see [Rate Limiting](#rate-limiting)) |
+| `ANY`  | `/webhook/{uuid}`           | Webhook receiver endpoint (accepts all methods) |
 
 #### Authentication Endpoints
 
 | Method | Path            | Description |
 | ------ | --------------- | ----------- |
-| `GET`  | `/pages/login`  | Login page (not rate limited; the limiter applies to POST only) |
+| `GET`  | `/pages/login`  | Login page |
-| `POST` | `/pages/login`  | Login form submission (5 per minute per bucket, then 429) |
+| `POST` | `/pages/login`  | Login form submission |
 | `POST` | `/pages/logout` | Logout (destroys session) |
 
 #### Authenticated Endpoints
@@ -1124,7 +1046,6 @@ abuse limit later; they are tracked as future work.
 | Method | Path                     | Description |
 | ------ | ------------------------ | ----------- |
 | `GET`  | `/user/{username}`       | User profile page |
-| `POST` | `/user/{username}/password` | Change the user's password (5 per minute per bucket, then 429) |
 | `GET`  | `/sources`               | List user's webhooks |
 | `GET`  | `/sources/new`           | Create webhook form |
 | `POST` | `/sources/new`           | Create webhook submission |
@@ -1134,17 +1055,13 @@ abuse limit later; they are tracked as future work.
 | `POST` | `/source/{id}/delete`    | Delete webhook |
 | `GET`  | `/source/{id}/logs`      | Webhook event logs |
 | `POST` | `/source/{id}/entrypoints` | Add entrypoint to webhook |
-| `POST` | `/source/{id}/entrypoints/{entrypointID}/delete` | Delete an entrypoint |
-| `POST` | `/source/{id}/entrypoints/{entrypointID}/toggle` | Enable or disable an entrypoint |
 | `POST` | `/source/{id}/targets`   | Add target to webhook |
-| `POST` | `/source/{id}/targets/{targetID}/delete` | Delete a target |
-| `POST` | `/source/{id}/targets/{targetID}/toggle` | Enable or disable a target |
 
 #### Infrastructure Endpoints
 
 | Method | Path       | Description |
 | ------ | ---------- | ----------- |
-| `GET`  | `/metrics` | Prometheus metrics, behind basic auth. The route is registered only when `METRICS_USERNAME` is set; otherwise it does not exist and returns 404 |
+| `GET`  | `/metrics` | Prometheus metrics (requires basic auth) |
 
 #### API (Planned)
 
@@ -1158,10 +1075,8 @@ abuse limit later; they are tracked as future work.
 | `GET`    | `/api/v1/webhooks/{id}/events` | List events for webhook |
 | `POST`   | `/api/v1/events/{id}/redeliver`| Redeliver an event |
 
-None of these exist yet. `/api/v1` is mounted with no routes, so every
+API authentication will use API keys passed via `Authorization: Bearer
-path under it returns 404 today. API authentication will use API keys
+<key>` header.
-passed via `Authorization: Bearer <key>` header; no Bearer middleware
-is implemented either.
 
 ### Package Layout
 
@@ -1189,28 +1104,16 @@ webhooker/
 │   │   ├── model_delivery_result.go  # DeliveryResult entity (per-webhook DB)
 │   │   ├── model_apikey.go    # APIKey entity
 │   │   ├── password.go        # Argon2id hashing and verification
-│   │   ├── retention.go       # Retention reaper (per-webhook event expiry)
-│   │   ├── testing.go         # NewTestDatabase: wrapper for tests, no fx lifecycle
 │   │   └── webhook_db_manager.go  # Per-webhook DB lifecycle manager
 │   ├── globals/
 │   │   └── globals.go         # Build-time variables (appname, version, arch)
 │   ├── delivery/
 │   │   ├── engine.go          # Event-driven delivery engine (channel + timer based)
-│   │   ├── circuit_breaker.go # Per-target circuit breaker for http/slack targets with retries
+│   │   ├── circuit_breaker.go # Per-target circuit breaker for HTTP targets with retries
-│   │   ├── target.go          # Target interface, Task, Scheduler
-│   │   ├── target_http.go     # HTTP target (retries, circuit breaker)
-│   │   ├── target_slack.go    # Slack/Mattermost incoming-webhook target
-│   │   ├── target_database.go # Database archive target
-│   │   ├── target_database_archive.go  # Archive file lifecycle and pruning
-│   │   ├── target_log.go      # Log target (stdout)
-│   │   ├── target_config_view.go  # Masked target config for templates
-│   │   ├── archive_sweeper.go # Periodic pruning of idle archives
-│   │   ├── url_mask.go        # Strips credentials from *url.Error
 │   │   └── ssrf.go            # SSRF prevention (IP validation, safe HTTP transport)
 │   ├── handlers/
 │   │   ├── handlers.go        # Base handler struct, JSON helpers, template rendering
 │   │   ├── auth.go            # Login, logout handlers
-│   │   ├── event_log_view.go  # Event log projection, byte-capped in SQL
 │   │   ├── healthcheck.go     # Health check handler
 │   │   ├── index.go           # Index page handler
 │   │   ├── profile.go         # User profile handler
@@ -1218,34 +1121,25 @@ webhooker/
 │   │   └── webhook.go         # Webhook receiver handler
 │   ├── healthcheck/
 │   │   └── healthcheck.go     # Health check service (uptime, version)
-│   ├── lifecycle/
-│   │   └── lifecycle.go       # Shared stop-hook waiter, bounded by the stop context
 │   ├── logger/
 │   │   └── logger.go          # slog setup with TTY detection
 │   ├── middleware/
 │   │   ├── middleware.go      # Logging, CORS, Auth, Metrics, MetricsAuth, SecurityHeaders, MaxBodySize
 │   │   ├── csrf.go            # CSRF protection middleware (gorilla/csrf)
-│   │   ├── ratelimit.go       # Per-IP rate limiting middleware (go-chi/httprate)
+│   │   └── ratelimit.go       # Per-IP rate limiting middleware (go-chi/httprate)
-│   │   └── testing.go         # NewForTest: Middleware without the fx lifecycle
 │   ├── server/
 │   │   ├── server.go          # Server struct, fx lifecycle, signal handling
 │   │   ├── http.go            # HTTP server setup with timeouts
 │   │   └── routes.go          # All route definitions
 │   └── session/
-│       ├── session.go         # Cookie-based session management
+│       └── session.go         # Cookie-based session management
-│       └── testing.go         # NewForTest: Session without the fx lifecycle
 ├── static/
 │   ├── static.go              # //go:embed directive
-│   ├── css/input.css          # Tailwind input, source for tailwind.css (make css)
+│   ├── css/style.css          # Custom stylesheet (system font stack, card effects, layout)
-│   ├── css/tailwind.css       # Generated stylesheet the pages load
+│   └── js/app.js              # Client-side JavaScript (minimal bootstrap)
-│   ├── css/style.css          # Older hand-written stylesheet, no longer loaded
+├── templates/                 # Go HTML templates (base, index, login, etc.)
-│   ├── js/app.js              # Progressive-enhancement copy-to-clipboard
+├── Dockerfile                 # Multi-stage: lint, build+test, then Alpine runtime
-│   ├── js/alpine.min.js       # Alpine.js, fetched by script/fetch-assets, not committed
+├── Makefile                   # fmt, lint, test, check, build, docker targets
-│   └── vendor.sha256          # Pinned hashes the fetched assets are verified against
-├── templates/                 # Go HTML templates (base, login, sources, etc.)
-├── script/                    # Scripts to Rule Them All entrypoints
-├── Dockerfile                 # Three stages: lint, test+build, Alpine runtime
-├── Makefile                   # 10 of 16 targets shim script/; 6 are inline
 ├── go.mod / go.sum
 └── .golangci.yml              # Linter configuration
 ```
@@ -1261,27 +1155,21 @@ Components are wired via Uber fx in this order:
    user seed
 5. `database.NewWebhookDBManager` — Per-webhook event database
    lifecycle manager
-6. `database.NewRetentionReaper` — Per-webhook event retention sweep
+6. `healthcheck.New` — Health check service
-7. `healthcheck.New` — Health check service
+7. `session.New` — Cookie-based session manager (key from database)
-8. `session.New` — Cookie-based session manager (key from database)
+8. `handlers.New` — HTTP handlers
-9. `handlers.New` — HTTP handlers
+9. `middleware.New` — HTTP middleware
-10. `middleware.New` — HTTP middleware
+10. `delivery.New` — Event-driven delivery engine
-11. `delivery.New` — Event-driven delivery engine
+11. `delivery.Engine` → `handlers.DeliveryNotifier` — interface bridge
-12. `delivery.NewArchiveSweeper` — Periodic pruning of idle archives
+12. `server.New` — HTTP server and router
-13. `delivery.Engine` → `delivery.Notifier` — interface bridge
-14. `delivery.Engine` → `delivery.WebhookEvictor` — interface bridge so
-    deleting a webhook releases its archive writer
-15. `server.New` — HTTP server and router
 
-The server starts via `fx.Invoke(func(*server.Server, *delivery.Engine,
+The server starts via `fx.Invoke(func(*server.Server, *delivery.Engine)
-*database.RetentionReaper, *delivery.ArchiveSweeper) {})`, which
+{})` which triggers the fx lifecycle hooks in dependency order. The
-triggers the fx lifecycle hooks in dependency order. The
+`DeliveryNotifier` interface allows the webhook handler to send
-`delivery.Notifier` interface allows the webhook handler to send
+self-contained `DeliveryTask` slices to the engine without a direct
-self-contained `delivery.Task` slices to the engine without a direct
 package dependency. Each task carries all target config and event data
-inline (for bodies under 16 KiB, `delivery.MaxInlineBodySize`), so the
+inline (for bodies ≤16KB), so the engine can deliver without reading
-engine can deliver without reading from any database — it only writes
+from any database — it only writes to record results.
-to record results.
 
 ### Middleware Stack
 
@@ -1307,32 +1195,16 @@ CSRF middleware in every one of those route groups, because
 gorilla/csrf parses the form; if the cap were installed after it, form
 parsing would run under net/http's 10 MB default and the 1 MB limit
 would never apply. A request that declares a `Content-Length` over the
-limit is answered with `413 Request Entity Too Large` without its body
+limit is answered with `413 Request Entity Too Large` before any other
-being read and without reaching CSRF, the route group's remaining
+middleware or handler runs; a chunked request, or one that lies about
-middleware, or the handler. It is not rejected before *any* other
+its length, is hard-capped by `http.MaxBytesReader` and fails
-middleware, though: the global entries listed above all run first, so
+downstream at form-parse time.
-such a request is still logged and given the security headers — and
-counted in the metrics, on a deployment where `METRICS_USERNAME` is
-set and the Metrics middleware is therefore registered at all. The
-rejection itself is logged at `WARN` with the method, path and
-declared length. A chunked request, or
-one that lies about its length, is hard-capped by
-`http.MaxBytesReader` and fails downstream at form-parse time.
-
-Those same four route groups then apply **CSRF** and **NoCache**
-(`Cache-Control: no-store`, `Pragma: no-cache`), and every group except
-`/pages` applies **RequireAuth**. The rate limiters are per-route
-rather than global: **LoginRateLimit** on `/pages/login`,
-**PasswordChangeRateLimit** on `/user/{username}/password`, and
-**ReceiverRateLimit** on `/webhook/{uuid}`.
 
 ### Authentication
 
 - **Web UI:** Cookie-based sessions using gorilla/sessions with
   encrypted cookies. Sessions are configured with HttpOnly, SameSite
-  Lax, and Secure (in production). Absolute session lifetime is 7 days,
+  Lax, and Secure (in production). Session lifetime is 7 days.
-  with a sliding idle timeout on top of it (see
-  [Sessions](#sessions)).
 - **API (planned):** API key authentication via `Authorization: Bearer`
   header. API keys are stored per-user with usage tracking
   (`last_used_at`).
@@ -1364,125 +1236,33 @@ rather than global: **LoginRateLimit** on `/pages/login`,
   IPs before connecting, preventing DNS rebinding attacks)
 - **Login rate limiting** via [go-chi/httprate](https://github.com/go-chi/httprate):
   sliding-window rate limiter on the login endpoint, 5 POST attempts
-  per minute per bucket, to slow brute-force attacks. GET requests to
+  per minute per bucket, to slow brute-force attacks. The bucket is per
-  the login page are not limited. The password-change endpoint carries
+  client IP only when `TRUSTED_PROXIES` names the reverse proxy;
-  the same 5-per-minute limit. The bucket is per client IP only when
+  unset, every client shares one bucket and the login becomes remotely
-  `TRUSTED_PROXIES` names the reverse proxy; unset, every client
+  deniable (see [Rate Limiting](#rate-limiting))
-  shares one bucket and the login becomes remotely deniable (see
-  [Rate Limiting](#rate-limiting)). webhooker warns at startup
-  whenever `TRUSTED_PROXIES` is empty
 - Prometheus metrics behind basic auth
 - Static assets embedded in binary (no filesystem access needed at
   runtime)
 - Container runs as non-root user (UID 1000)
-- GORM soft deletes on every entity that carries `BaseModel`, which is
+- GORM soft deletes on all entities (data preserved for audit)
-  all of them but `Setting` (data preserved for audit)
-
-### Shutdown
-
-On SIGINT or SIGTERM, fx runs the registered stop hooks in reverse
-dependency order under a **5 second budget** (`fx.StopTimeout` in
-`cmd/webhooker/main.go`). That budget covers the whole sequence, not
-each hook. The order, read off the fx stop-hook log:
-
-1. `ArchiveSweeper`
-2. `RetentionReaper`
-3. `server` — the HTTP drain, bounded separately by
-   `server.ShutdownTimeout` (**3 seconds**), then a Sentry flush if
-   `SENTRY_DSN` is set
-4. `delivery.Engine`
-5. `healthcheck`
-6. `WebhookDBManager`
-7. the database close
-
-The two components that can realistically hold the budget run
-first: a retention sweep or an archive prune caught mid-tick each
-waits on its `WaitGroup` bounded by the stop context, so a wedge
-there consumes the 5 seconds before the HTTP server hook is ever
-entered. The hooks after the server are microsecond-scale in normal
-operation.
-
-The HTTP drain budget is deliberately **shorter** than the sequence
-budget. Were the two equal, a drain that used its whole budget would
-exhaust the sequence budget at the instant it finished, and every
-later hook — the delivery engine, the healthcheck, the webhook DB
-manager and the database close — would be skipped in exactly the
-case where the drain mattered. 3 seconds leaves 2 seconds
-(`server.TailHookReserve`) for the tail, which is far more than the
-microseconds it needs.
-
-That reserve belongs to the tail hooks, not to the server hook, and
-the Sentry flush is what could take it: it runs after the drain
-**inside the same hook**, and `sentry.Flush` takes a bare duration
-and honours no context, so an unreachable Sentry endpoint would add
-its own timeout on top of a full-length drain and consume the whole
-sequence budget by itself. It is therefore clamped to whatever is
-left on the stop context minus the reserve, and skipped when that
-leaves too little to be worth attempting — so a full-length drain
-means Sentry events are dropped rather than the database close being
-skipped.
-
-This does not make the database close unconditional: a wedged
-`ArchiveSweeper` or `RetentionReaper` still runs first and can
-consume the whole budget on its own.
-
-The value is chosen to sit inside the container stop grace period.
-Docker's default `docker stop` grace is 10 seconds and the Dockerfile
-sets no `STOPSIGNAL` or grace override, so the process must be gone
-before that. fx's own default is 15 seconds, which is past the grace:
-the container would be SIGKILLed (exit 137) before the bound could
-fire, and nothing that depends on it — including the
-`shutdown timed out, goroutines still running` error log that tells
-an operator a component is wedged — would ever be reached.
-
-Two operational consequences follow from bounding the sequence:
-
-- **A wedged component aborts the rest of the shutdown.** fx checks
-  the stop context before each remaining hook and returns outright
-  once it has expired, skipping the hooks it has not reached. If the
-  first-stopped component consumes the whole budget, the later hooks
-  never run — **the database close among them**. SQLite is crash-safe,
-  so this is not corruption, but it is not a clean close either.
-- **Lowering the grace below 5 seconds reintroduces the silent
-  truncation.** `docker stop --time`, Compose's `stop_grace_period`,
-  or Kubernetes' `terminationGracePeriodSeconds` set under 5 seconds
-  put SIGKILL back in front of the bound, and the process dies with
-  no shutdown diagnostics at all. Keep the deployment's grace above
-  the stop timeout.
 
 ### Docker
 
-The Dockerfile uses a three-stage build. Each stage is pinned by
+The Dockerfile uses a multi-stage build:
-digest, and the two check stages are separate images so the linter's
-version is fixed independently of the compiler's:
 
-1. **Lint stage** (`golangci/golangci-lint:v2.12.2`, Debian-based) —
+1. **Builder stage** (Debian-based `golang:1.24`) — installs
-   installs `make`, downloads dependencies, copies the source, and runs
+   golangci-lint, downloads dependencies, copies source, runs `make
-   `make fmt-check` then `make lint`.
+   check` (format verification, linting, tests, compilation).
-2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint
+2. **Runtime stage** (`alpine:3.21`) — copies the binary, creates the
-   stage passing (it copies a file from it), runs `script/fetch-assets`
+   `/var/lib/webhooker` directory for all SQLite databases, runs as
-   to download and verify the third-party browser assets, then runs
+   non-root user, exposes port 8080, includes a health check.
-   `make test` and `make build`, and finally rebuilds the binary with
-   `CGO_ENABLED=1` and static linking so it runs on musl.
-3. **Runtime stage** (`alpine:3.21`) — copies the static binary,
-   creates the `/var/lib/webhooker` directory for all SQLite databases,
-   runs as the non-root `webhooker` user (UID 1000), exposes port 8080,
-   and includes a health check against `/.well-known/healthcheck`.
 
-Both check stages use Debian rather than Alpine because
+The builder uses Debian rather than Alpine because GORM's SQLite
-`gorm.io/driver/sqlite` pulls in `mattn/go-sqlite3`, which needs CGO
+dialect pulls in CGO-dependent headers at compile time. The runtime
-and does not compile against musl. Only the final binary is statically
+binary is statically linked and runs on Alpine.
-linked, which is what lets it run on the Alpine runtime image.
 
-`script/cibuild` — `docker build .` — is the CI gate: the four check
+`docker build .` is the CI gate — if it passes, the code is formatted,
-targets run inside the image, so a build that succeeds is a repo that
+linted, tested, and compiled.
-is formatted, linted, tested and compiled. Only `script/cibuild` and
-`script/docker` involve Docker. `script/lint`, and therefore
-`make lint` and `make check`, run whatever `golangci-lint` is on the
-host, which can be a different version from the pinned one — so the
-container is the authoritative lint result
-([issue #109](https://git.eeqj.de/sneak/webhooker/issues/109) tracks
-routing local linting through it as well).
 
 #### CI gate honesty
 
@@ -1498,17 +1278,16 @@ the hash of the last commit that touched the build context, so:
   `make fmt-check`, `make lint`, `make test`, and `make build`. A run
   that reports success ran them.
 - A docs-only commit leaves the fingerprint unchanged — `.dockerignore`
-  excludes `*.md`, `LICENSE` and `.editorconfig` from the context
+  excludes `*.md` and `LICENSE` from the context anyway — so the image
-  anyway — so the image replays from cache and costs seconds.
+  replays from cache and costs seconds.
 
 The module download layer sits above `COPY . .` and stays cached either
 way.
 
-A separate workflow step, run before the fingerprint is written, covers
+The workflow's first step covers a second way the gate lied: Gitea
-a second way the gate lied: Gitea cancels an in-flight run when a newer
+cancels an in-flight run when a newer commit lands on the same branch
-commit lands on the same branch and records that cancellation as a
+and records that cancellation as a `failure` status, marking a commit
-`failure` status, marking a commit red that was never tested.
+red that was never tested. Cancellation is unconditional server-side for
-Cancellation is unconditional server-side for
 push events, so the superseding run rewrites the exact
 `Has been cancelled` status to `skipped`. Genuine failures are never
 touched.

TODO.md

@@ -25,17 +25,13 @@ password change flow (#65), policy compliance (#6), pinned lint tooling
 (#55), and fail-loud configuration parsing (#80).
 
 `next` holds the completed 1.0.0 milestone: every issue in it is closed,
-and it is verified green by cache-defeated container runs
+and it is verified green both by CI and by cache-defeated container
-(`docker build --no-cache-filter=lint --no-cache-filter=builder`). The
+runs. The two were only made to mean the same thing this cycle — before
-CI status is not independently claimed here: a superseded run is
+#119, a warm layer cache let the gate report success without executing
-recorded as `skipped` and still rolls up green, so a commit status on
+anything, and replayed the previous build's console log so the lie
-`next` does not by itself evidence an executed check (#152). Before
+looked like a real run. Note: TODO.md was deliberately deleted from this
-#119, a warm layer cache also let the gate report success without
+repo in f9a9569 (2026-03-01, #6); its content was folded into the README
-executing anything, and replayed the previous build's console log so
+TODO section, which this draft reconstructs as of 2026-07-06.
-the lie looked like a real run. Note: `TODO.md` was deliberately
-deleted from this repo in f9a9569 (2026-03-01, #6); its content was
-folded into the README TODO section, which this draft reconstructs as
-of 2026-07-06.
 
 # Next Step
 
@@ -194,9 +190,7 @@ rate-limit keys should bucket by `/64`).
     - OpenAPI specification
 - Analytics dashboard: success rates, response times, volume
 - A remember-me option at login
-- Password reset flow for a forgotten password. The authenticated
+- Password change and reset flow
-  password *change* flow already landed on `main` (#65); reset does not
-  exist
 - Later, nice to have
     - email delivery target type
     - SNS and S3 delivery targets

cmd/webhooker/main.go

@@ -2,8 +2,6 @@
 package main
 
 import (
-	"time"
-
 	"go.uber.org/fx"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/database"
@@ -17,33 +15,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/session"
 )
 
-// stopTimeout bounds the whole fx stop sequence, not each hook.
-//
-// fx defaults to 15s, which is longer than Docker's 10s default
-// stop grace: the container would be SIGKILLed before the bound
-// could fire, so nothing bounded by it would ever be observed.
-// 5s leaves headroom inside that grace for signal delivery and
-// process exit; the observed wedge case already exits at ~5.3s,
-// so a larger bound would trade a rare skipped database close for
-// a more common hard kill.
-//
-// The server's stop hook must fit inside it with room to spare: a
-// hook that used the whole budget would exhaust it at that instant,
-// and fx would skip every hook after the server — the delivery
-// engine, the healthcheck, the webhook DB manager and the database
-// close. That hook is the 3s HTTP drain plus the Sentry flush that
-// follows it in the same hook, so the flush is clamped to the stop
-// context's remaining time less server.TailHookReserve rather than
-// running for its own fixed 2s; the reserve is what the tail hooks
-// live on, and they are microsecond-scale in normal operation.
-// TestStopTimeout_LeavesHeadroomForTailHooks pins the arithmetic
-// across every drain length.
-//
-// This does not make the database close unconditional: the
-// ArchiveSweeper and RetentionReaper hooks run before the server
-// and can still consume the whole budget on their own.
-const stopTimeout = 5 * time.Second
-
 // Build-time variables set via -ldflags.
 //
 //nolint:gochecknoglobals // Build-time variables injected by the linker.
@@ -56,14 +27,7 @@ func main() {
 	globals.Appname = appname
 	globals.Version = version
 
-	newApp().Run()
+	fx.New(
-}
-
-// newApp builds the application graph. It is separate from main so
-// a test can assert the options it carries.
-func newApp() *fx.App {
-	return fx.New(
-		fx.StopTimeout(stopTimeout),
 		fx.Provide(
 			globals.New,
 			logger.New,
@@ -96,5 +60,5 @@ func newApp() *fx.App {
 			) {
 			},
 		),
-	)
+	).Run()
 }

cmd/webhooker/main_test.go

@@ -1,75 +0,0 @@
-package main
-
-import (
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/server"
-)
-
-// dockerStopGrace is Docker's default `docker stop` grace period.
-// The Dockerfile sets no STOPSIGNAL or grace override, so this is
-// the deadline the container is actually held to, and the fx stop
-// timeout has to fit inside it with room for signal delivery and
-// process exit.
-const dockerStopGrace = 10 * time.Second
-
-// TestNewApp_StopTimeout pins the fx stop timeout. Without the
-// explicit fx.StopTimeout option the app reads fx's 15s
-// DefaultTimeout, which exceeds dockerStopGrace: the container is
-// SIGKILLed before the bound fires and every shutdown hook bounded
-// by it — including the operator-facing timeout log — becomes
-// unreachable in the image this repo produces.
-//
-// fx.New applies options before it executes invokes, so the timeout
-// is set whether or not the graph itself can be constructed here.
-func TestNewApp_StopTimeout(t *testing.T) {
-	t.Setenv("DATA_DIR", t.TempDir())
-
-	got := newApp().StopTimeout()
-
-	require.Equal(t, stopTimeout, got)
-	require.Less(t, got, dockerStopGrace)
-}
-
-// tailHeadroom is the slack the fx stop budget must keep beyond the
-// server stop hook. The hooks that run after the server — the
-// delivery engine, the healthcheck, the webhook DB manager and the
-// database close — are microsecond-scale in normal operation, so
-// this is generous for them.
-const tailHeadroom = 2 * time.Second
-
-// TestStopTimeout_LeavesHeadroomForTailHooks pins the relationship
-// between the server's stop hook and the fx stop budget. fx bounds
-// the whole stop sequence, and returns without running its
-// remaining hooks once the stop context has expired. If the hook
-// could use the entire budget, every later hook — the database close
-// included — would be skipped in exactly the case where the drain
-// mattered.
-//
-// The hook is not just the HTTP drain: a Sentry flush follows it in
-// the same hook, and sentry.Flush honours no context, so both halves
-// have to be counted. The sweep walks every drain length the hook
-// can produce, since a shorter drain leaves the flush more room and
-// the worst case is not necessarily at either extreme.
-//
-// Shrinking either budget, or unbounding the flush again, must fail
-// here rather than silently recreating a hook that swallows the
-// whole sequence.
-func TestStopTimeout_LeavesHeadroomForTailHooks(t *testing.T) {
-	t.Parallel()
-
-	require.Less(t, server.ShutdownTimeout, stopTimeout)
-
-	const step = 10 * time.Millisecond
-
-	for drain := time.Duration(0); drain <= server.ShutdownTimeout; drain += step {
-		hook := drain + server.SentryFlushBudget(stopTimeout-drain)
-
-		require.LessOrEqual(
-			t, hook+tailHeadroom, stopTimeout,
-			"a %s drain leaves the tail hooks short", drain,
-		)
-	}
-}

internal/config/config.go

@@ -422,43 +422,33 @@ func loadFromEnv() (*Config, error) {
 	}, nil
 }
 
-// warnSharedRateLimitBucket logs a startup warning whenever
+// warnSharedRateLimitBucket logs a startup warning when a production
-// TRUSTED_PROXIES is empty, in any environment.
+// deployment leaves TRUSTED_PROXIES empty.
 //
 // With no trusted proxies every rate limiter keys on the connecting
-// peer's address. Whether that is harmless or dangerous depends on
+// peer's address. A production deployment is required to run behind a
-// what is in front of the process, which this code cannot observe:
+// TLS-terminating reverse proxy, and the peer is then that proxy for
-// with nothing in front, the peer is the client and the limits are
+// every request, so all clients share one bucket per limiter. The
-// per-client as intended; behind a reverse proxy the peer is the proxy
-// for every request, so all clients share one bucket per limiter. The
 // login limiter's bucket is the dangerous one: any remote client can
-// keep it full, which denies the only administrative login to everyone
+// keep it full, which denies the only administrative login to
-// until the process restarts.
+// everyone until the process restarts.
-//
-// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT. That
-// variable defaults to dev, so gating on it would silence the warning
-// for exactly the operator who forgot to configure the deployment —
-// the case it exists to catch.
 //
 // The default of trusting nobody is deliberate — trusting forwarded
 // headers from arbitrary peers lets any client choose its own bucket —
 // so this warns rather than failing startup or changing the key.
 func (c *Config) warnSharedRateLimitBucket(log *slog.Logger) {
-	if len(c.TrustedProxies) > 0 {
+	if !c.IsProd() || len(c.TrustedProxies) > 0 {
 		return
 	}
 
 	log.Warn(
-		"TRUSTED_PROXIES is empty: every rate limit keys on the "+
+		"TRUSTED_PROXIES is empty: rate limits key on the "+
-			"connecting peer's address. With nothing proxying to "+
+			"connecting peer, so behind the reverse proxy a "+
-			"this process that is the client itself and the limits "+
+			"production deployment runs behind, every client "+
-			"are per-client as intended. Behind a reverse proxy the "+
+			"shares one bucket per limit. Any remote client can "+
-			"peer is the proxy on every request, so all clients "+
+			"then keep the login limit full and deny the admin "+
-			"share one bucket per limit and any remote client can "+
+			"login, the only administrative path, until restart. "+
-			"keep the login limit full, denying the admin login — "+
+			"Set TRUSTED_PROXIES to your reverse proxy's address.",
-			"the only administrative path — until restart. If "+
-			"anything proxies to this process, set TRUSTED_PROXIES "+
-			"to its address.",
 		"environment", c.Environment,
 		"trustedProxies", len(c.TrustedProxies),
 	)
@@ -501,10 +491,6 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
 		"maintenanceMode", s.MaintenanceMode,
 		"dataDir", s.DataDir,
 		"retentionSweepInterval", s.RetentionSweepInterval.String(),
-		// Logged because a perfectly valid non-positive value here
-		// disables idle expiry entirely, and that is worth showing
-		// back to the operator.
-		"sessionIdleTimeout", s.SessionIdleTimeout.String(),
 		"receiverRateLimit", s.ReceiverRateLimit,
 		"trustedProxies", len(s.TrustedProxies),
 		"hasSentryDSN", s.SentryDSN != "",

internal/config/config_test.go

@@ -628,12 +628,10 @@ func testTrustedProxiesSuccess(
 }
 
 // TestSharedRateLimitBucketWarning covers the startup warning that
-// tells an operator a deployment behind a reverse proxy shares one
+// tells an operator their production deployment shares one rate-limit
-// rate-limit bucket between every client, which makes the admin login
+// bucket between every client, which makes the admin login remotely
-// remotely deniable. It must fire whenever TRUSTED_PROXIES is empty,
+// deniable. It must fire when TRUSTED_PROXIES is empty in production
-// in any environment: WEBHOOKER_ENVIRONMENT defaults to dev, so gating
+// and stay quiet otherwise.
-// on it would silence the warning for exactly the operator who never
-// configured the deployment. It stays quiet once proxies are named.
 func TestSharedRateLimitBucketWarning(t *testing.T) {
 	tests := []struct {
 		name           string
@@ -653,19 +651,12 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
 			expectWarning:  false,
 		},
 		{
-			// The default environment. An internet-exposed
+			// Development is not required to run behind a
-			// deployment whose operator never set
+			// reverse proxy, so the shared bucket the warning
-			// WEBHOOKER_ENVIRONMENT lands here and has exactly
+			// describes is not the expected shape there.
-			// the exposure the warning announces.
+			name:          "dev without trusted proxies is quiet",
-			name:          "dev without trusted proxies warns",
 			environment:   config.EnvironmentDev,
-			expectWarning: true,
+			expectWarning: false,
-		},
-		{
-			name:           "dev with trusted proxies is quiet",
-			environment:    config.EnvironmentDev,
-			trustedProxies: cidrPrivateV4,
-			expectWarning:  false,
 		},
 	}
 
@@ -706,14 +697,8 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
 
 			assert.Contains(t, logged, `"level":"WARN"`)
 			assert.Contains(t, logged, "TRUSTED_PROXIES")
-			assert.Contains(t, logged, "share one bucket")
+			assert.Contains(t, logged, "shares one bucket")
-			assert.Contains(t, logged, "denying the admin login")
+			assert.Contains(t, logged, "deny the admin login")
-			// The text must stay accurate for a developer with
-			// nothing in front of the process, where an empty
-			// list costs nothing.
-			assert.Contains(
-				t, logged, "nothing proxying to this process",
-			)
 		})
 	}
 }

internal/lifecycle/export_test.go

@@ -1,21 +0,0 @@
-package lifecycle
-
-import (
-	"context"
-	"log/slog"
-)
-
-// WaitDone exposes waitDone to the external test package. Only the
-// unexported waiter can be handed a channel that is already closed
-// before the call, which is the state the preamble exists for;
-// through WaitForShutdown the waiter goroutine may or may not have
-// closed the channel yet, so the case is not reachable
-// deterministically from outside.
-func WaitDone(
-	ctx context.Context,
-	log *slog.Logger,
-	component string,
-	done <-chan struct{},
-) error {
-	return waitDone(ctx, log, component, done)
-}

internal/lifecycle/lifecycle.go

@@ -38,29 +38,6 @@ func WaitForShutdown(
 		wg.Wait()
 	}()
 
-	return waitDone(ctx, log, component, done)
-}
-
-// waitDone waits for done to close, bounded by ctx.
-//
-// The non-blocking preamble is load-bearing. When the component has
-// already drained and ctx has already expired, both cases of the
-// bounded select are ready and Go picks between them uniformly at
-// random, so a clean shutdown would be reported as a timeout about
-// half the time. Draining wins: the goroutines are gone, and there
-// is nothing left for the operator to act on.
-func waitDone(
-	ctx context.Context,
-	log *slog.Logger,
-	component string,
-	done <-chan struct{},
-) error {
-	select {
-	case <-done:
-		return nil
-	default:
-	}
-
 	select {
 	case <-done:
 		return nil

internal/lifecycle/lifecycle_test.go

@@ -37,57 +37,6 @@ func TestWaitForShutdown_DrainedGroup(t *testing.T) {
 	)
 }
 
-// racePasses is how many times the both-cases-ready race is run.
-// Without the preamble each pass is an independent coin flip, so
-// the probability of the whole loop passing by luck is 2^-N: at
-// this N the test is deterministic in practice, and it involves no
-// wall-clock waiting at all.
-const racePasses = 1000
-
-// TestWaitDone_DrainedBeforeExpiredContext covers the case where a
-// component drained cleanly but the stop context had already
-// expired. Both select cases are ready, and Go chooses among ready
-// cases uniformly at random, so the drained case must be settled by
-// the preamble before the bounded select ever runs.
-func TestWaitDone_DrainedBeforeExpiredContext(t *testing.T) {
-	t.Parallel()
-
-	done := make(chan struct{})
-	close(done)
-
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel()
-
-	for pass := range racePasses {
-		require.NoErrorf(
-			t,
-			lifecycle.WaitDone(
-				ctx, discardLogger(), "test component", done,
-			),
-			"pass %d reported a timeout for a drained component",
-			pass,
-		)
-	}
-}
-
-// TestWaitDone_ExpiredContext pins the other side of the preamble:
-// an expired context with a component that has not drained is still
-// a timeout.
-func TestWaitDone_ExpiredContext(t *testing.T) {
-	t.Parallel()
-
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel()
-
-	err := lifecycle.WaitDone(
-		ctx, discardLogger(), "test component",
-		make(chan struct{}),
-	)
-
-	require.ErrorIs(t, err, context.Canceled)
-	require.ErrorContains(t, err, "test component")
-}
-
 func TestWaitForShutdown_ContextExpires(t *testing.T) {
 	t.Parallel()
 

internal/middleware/accesslog_test.go

@@ -55,7 +55,6 @@ const tailMarker = "QQTRUNCATEDTAILQQ"
 const (
 	maxFieldBytes         = 512
 	maxRequestIDBytes     = 128
-	maxMethodBytes        = 32
 	truncationSuffix      = "[truncated]"
 	unmatchedRouteLiteral = "(unmatched)"
 )
@@ -77,27 +76,6 @@ func capturingMiddleware(t *testing.T) (*middleware.Middleware, *bytes.Buffer) {
 	return middleware.NewForTest(log, cfg, nil), buf
 }
 
-// capturingTextMiddleware is capturingMiddleware for the other handler
-// 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.
-func capturingTextMiddleware(
-	t *testing.T,
-) (*middleware.Middleware, *bytes.Buffer) {
-	t.Helper()
-
-	buf := new(bytes.Buffer)
-	log := slog.New(slog.NewTextHandler(
-		buf,
-		&slog.HandlerOptions{Level: slog.LevelInfo},
-	))
-
-	cfg := &config.Config{Environment: config.EnvironmentDev}
-
-	return middleware.NewForTest(log, cfg, nil), buf
-}
-
 // accessLogRouter mirrors the production route shapes that an
 // unauthenticated client can reach: the public receiver, the
 // authenticated profile route (which redirects to login rather than
@@ -380,23 +358,14 @@ func lineSizeCases() map[string]sizeCase {
 	longPath := "/boom/" + strings.Repeat("x", oversizedSegmentBytes)
 	wantLongURL := longPath[:maxFieldBytes] + truncationSuffix
 
-	// escapeChars are the runes Go's header parser accepts in a header
+	// escapeChars are the bytes Go's header parser accepts in a header
-	// value and the log handler then escapes, coming out wider than
+	// value and the log handler then escapes, one byte in and two or
-	// they went in. A budget counted in raw bytes lets any of them buy
+	// more out. A budget counted in raw bytes lets any of them buy a
-	// a field several times its nominal size, so every one of them
+	// field twice its nominal size, so every one of them gets a case.
-	// gets a case.
-	//
-	// The astral one is the case the JSON handler alone does not
-	// reach: U+1000C is unassigned, so it is non-printable, and
-	// strconv.Quote spells a non-printable rune at or above U+10000
-	// as a ten-byte \UXXXXXXXX. The JSON handler passes it through as
-	// its four UTF-8 bytes, so only the text-handler shape of this
-	// test holds the ten-byte charge honest.
 	escapeChars := map[string]string{
 		"quote":     `"`,
 		"backslash": `\`,
 		"tab":       "\t",
-		"astral":    "\U0001000C",
 	}
 
 	for kind, char := range escapeChars {
@@ -475,91 +444,6 @@ func TestAccessLog_LineSizeDoesNotTrackInputSize(t *testing.T) {
 	}
 }
 
-// TestAccessLog_LineSizeDoesNotTrackInputSizeOnTheTextHandler runs the
-// same cases through slog's text handler, which internal/logger
-// selects on a tty.
-//
-// MaxAccessLogLineBytes is quoted to operators unqualified, so it has
-// to hold for whichever handler is installed — and the two do not
-// escape alike. The astral case is the one that separates them: the
-// JSON handler emits U+1000C as its four UTF-8 bytes, while
-// strconv.Quote spells it \U0001000C at ten. Charging six for it, as
-// this code did, put a real 2,676-byte line on the wire here while
-// every JSON case stayed comfortably inside the bound.
-//
-// Only the size bound is asserted; the url field's contents are the
-// JSON shape's business above.
-func TestAccessLog_LineSizeDoesNotTrackInputSizeOnTheTextHandler(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	for name, tc := range lineSizeCases() {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			m, buf := capturingTextMiddleware(t)
-			router := accessLogRouter(m)
-
-			assert.Equal(
-				t,
-				tc.wantStatus,
-				getWithHeaders(t, router, tc.target, tc.headers),
-			)
-
-			line := strings.TrimSpace(buf.String())
-
-			require.NotEmpty(t, line)
-			assert.NotContains(
-				t, line, "\n", "expected exactly one log line",
-			)
-			require.LessOrEqual(
-				t, len(line), tc.bound,
-				"access log line exceeded its bound",
-			)
-			assert.Contains(t, line, "url=")
-			assert.NotContains(
-				t, line, attackerMarker,
-				"access log carried attacker-chosen text",
-			)
-			assert.NotContains(
-				t, line, tailMarker,
-				"access log carried an untruncated client field",
-			)
-		})
-	}
-}
-
-// TestAccessLog_OversizedMethodIsTruncated covers the last term in the
-// MaxAccessLogLineBytes arithmetic that the size cases above cannot
-// reach: Go accepts any RFC 7230 token as a method, and getWithHeaders
-// only ever sends GET.
-func TestAccessLog_OversizedMethodIsTruncated(t *testing.T) {
-	t.Parallel()
-
-	m, buf := capturingMiddleware(t)
-	router := accessLogRouter(m)
-
-	method := strings.Repeat("M", oversizedSegmentBytes) + attackerMarker
-	req := httptest.NewRequestWithContext(
-		context.Background(), method, "/"+attackerMarker, nil,
-	)
-	rec := httptest.NewRecorder()
-	router.ServeHTTP(rec, req)
-
-	entries := accessLogEntriesWithin(t, buf, maxLineBytes)
-	require.Len(t, entries, 1)
-	assert.Equal(
-		t,
-		strings.Repeat("M", maxMethodBytes)+truncationSuffix,
-		entries[0]["method"],
-	)
-	assert.NotContains(
-		t, buf.String(), attackerMarker,
-		"access log carried attacker-chosen text",
-	)
-}
-
 // TestAccessLog_OversizedHeadersKeepATruncatedPrefix checks the other
 // half of the header cap: the fields are cut, not dropped, so a
 // truncated User-Agent is still worth reading.

internal/middleware/middleware.go

@@ -90,13 +90,9 @@ const (
 	// than sitting on the arithmetic.
 	//
 	// The tty text handler in internal/logger is covered by the same
-	// figure. encodedLogFieldBytes charges every rune at least what
+	// figure: encodedLogFieldBytes charges the worse of the two
-	// the wider of the two handlers emits for it — including the ten
+	// handlers' escapes, and the text handler's fixed portion is the
-	// bytes strconv.Quote spends on a non-printable rune at or above
+	// smaller of the two.
-	// 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.
 	MaxAccessLogLineBytes = 2560
 )
 
@@ -173,41 +169,24 @@ func (lrw *loggingResponseWriter) WriteHeader(code int) {
 //
 // slog's JSON handler escapes quote, backslash, newline, carriage
 // return and tab to two bytes each, and every other C0 control plus
-// LINE SEPARATOR and PARAGRAPH SEPARATOR to a six-byte \u escape; it
+// LINE SEPARATOR and PARAGRAPH SEPARATOR to a six-byte \u escape. Its
-// passes every other rune through as its own UTF-8. Its text handler
+// text handler quotes with strconv.Quote, which spells any
-// quotes with strconv.Quote, which spells a non-printable rune below
+// non-printable rune the same six-byte way. Both pass printable runes
-// U+10000 as \uXXXX but one at or above U+10000 as \UXXXXXXXX — ten
+// through as their own UTF-8, so unicode.IsPrint separates the two
-// bytes, not six. The text handler is therefore the worse of the two
+// cases for either handler. Go's header parser accepts quote,
-// for every non-printable rune, and by four bytes apiece for the
+// backslash, tab and non-printable multi-byte runes in a header value,
-// 955,086 unassigned, private-use and format code points on planes 1
+// so every one of these is reachable from a request.
-// to 16.
-//
-// Charging ten there is what makes MaxAccessLogLineBytes hold for the
-// tty handler as well: U+1000C encodes as F0 90 80 8C, every byte
-// >= 0x80, which httpguts.ValidHeaderFieldValue accepts and
-// net/textproto does not strip, so a header can be filled with them.
-//
-// Both handlers pass printable runes through as their own UTF-8, so
-// unicode.IsPrint separates the escaped cases from the plain ones for
-// either handler.
 func encodedLogFieldBytes(r rune) int {
 	const (
 		// A backslash and the character itself.
 		shortEscapeBytes = 2
 		// \uXXXX, which is also the width of \u00XX.
 		escapedRuneBytes = 6
-		// \UXXXXXXXX, strconv.Quote's spelling of a non-printable
-		// rune outside the basic multilingual plane.
-		escapedAstralRuneBytes = 10
-		// The first code point strconv.Quote spells with \U.
-		firstAstralRune = 0x10000
 	)
 
 	switch {
 	case r == '"' || r == '\\' || r == '\n' || r == '\r' || r == '\t':
 		return shortEscapeBytes
-	case !unicode.IsPrint(r) && r >= firstAstralRune:
-		return escapedAstralRuneBytes
 	case !unicode.IsPrint(r):
 		return escapedRuneBytes
 	default:

internal/middleware/ratelimit.go

@@ -48,12 +48,6 @@ const (
 	// bound every request pays a walk proportional to whatever the
 	// client sent.
 	maxForwardedHops = 64
-
-	// ipv6BucketBits is the prefix length IPv6 clients are bucketed
-	// on. A routed /64 is the normal residential and mobile
-	// allocation, so it is the unit an attacker gets addresses in
-	// and therefore the unit worth limiting.
-	ipv6BucketBits = 64
 )
 
 // normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
@@ -62,40 +56,6 @@ func normalizeAddr(addr netip.Addr) netip.Addr {
 	return addr.Unmap().WithZone("")
 }
 
-// bucketKey is the rate-limit bucket identity of a client address.
-// IPv4 keys on the full address; IPv6 keys on its /64 prefix,
-// because keying IPv6 per /128 lets one ordinary subscriber rotate
-// source addresses inside its own routed /64 and mint a fresh bucket
-// per request — evading every limiter here at the network layer,
-// with no spoofing and nothing to detect.
-//
-// An IPv4-mapped address (::ffff:1.2.3.4) is keyed as the IPv4
-// address it carries, never masked to a /64: mapped form all shares
-// the ::ffff:0:0/96 prefix, so masking would collapse every IPv4
-// client reaching a proxy that emits it into one bucket. Callers
-// pass addresses through normalizeAddr, which already unmaps; the
-// unmap here keeps the property true of the key function itself.
-//
-// The two families cannot collide: an IPv4 key is a bare dotted
-// quad, and an IPv6 key always carries a "/64" suffix.
-func bucketKey(addr netip.Addr) string {
-	addr = addr.Unmap()
-
-	if addr.Is4() {
-		return addr.String()
-	}
-
-	// Prefix errors only on a negative bit count, on over 32 bits
-	// for an IPv4 address, or on over 128 for IPv6. The count here
-	// is the constant 64 and the IPv4 case returned above, so the
-	// error is unreachable. (The zero Addr does not error either: it
-	// yields the zero Prefix. Neither call site can produce one,
-	// since both parse the address first.)
-	prefix, _ := addr.Prefix(ipv6BucketBits)
-
-	return prefix.String()
-}
-
 // isTrustedProxy reports whether addr belongs to a network the
 // operator listed in TRUSTED_PROXIES. The list is empty by default,
 // so by default nothing is trusted.
@@ -183,9 +143,6 @@ func (m *Middleware) forwardedClientAddr(
 // 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.
 func (m *Middleware) rateLimitKey(r *http.Request) (string, error) {
 	return m.clientKey(r), nil
 }
@@ -195,25 +152,23 @@ func (m *Middleware) clientKey(r *http.Request) string {
 	peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
 	if err != nil {
 		// Not an address we can reason about; key on the raw
-		// value, the most specific identity left. Distinct
+		// value, the most specific identity left. On a
-		// RemoteAddr values stay in distinct buckets, so this
+		// Unix-socket listener every peer carries the same
-		// path cannot silently collapse unrelated clients
+		// RemoteAddr and so shares one bucket, which is the
-		// together. On a Unix-socket listener every peer
+		// fail-closed direction.
-		// carries the same RemoteAddr and so shares one bucket,
-		// which is the fail-closed direction.
 		return r.RemoteAddr
 	}
 
 	peer = normalizeAddr(peer)
 	if !m.isTrustedProxy(peer) {
-		return bucketKey(peer)
+		return peer.String()
 	}
 
 	if addr, ok := m.forwardedClientAddr(r); ok {
-		return bucketKey(addr)
+		return addr.String()
 	}
 
-	return bucketKey(peer)
+	return peer.String()
 }
 
 // tooManyRequests returns the 429 handler used by the login,

internal/middleware/ratelimit_test.go

@@ -15,7 +15,6 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/middleware"
 )
@@ -371,30 +370,6 @@ const (
 	headerXFF  = "X-Forwarded-For"
 	headerReal = "X-Real-IP"
 	headerTrue = "True-Client-IP"
-
-	// clientIPv4 is the sample IPv4 client address these tests key
-	// on, both directly and in IPv4-mapped form. clientIPv4Alt is
-	// its neighbour, used to show the two do not share a bucket.
-	clientIPv4    = "198.51.100.7"
-	clientIPv4Alt = "198.51.100.8"
-
-	// clientIPv6 and clientIPv6Same are two addresses inside one
-	// routed /64, so both must key on clientBucketV6.
-	// clientIPv6Other is a different allocation and must key on
-	// clientOtherBucketV6.
-	clientIPv6          = "2001:db8:1:2:3:4:5:6"
-	clientIPv6Same      = "2001:db8:1:2:aaaa:bbbb:cccc:dddd"
-	clientIPv6Other     = "2001:db8:1:3::1"
-	clientBucketV6      = "2001:db8:1:2::/64"
-	clientOtherBucketV6 = "2001:db8:1:3::/64"
-
-	// trustedProxyCIDR is the proxy network the forwarded-path
-	// tests configure, and trustedPeer an address inside it. A
-	// production deployment is required to run behind a reverse
-	// proxy with TRUSTED_PROXIES set, so this is the shape the
-	// bucketing has to hold in.
-	trustedProxyCIDR = "10.0.0.0/8"
-	trustedPeer      = "10.0.0.1:44444"
 )
 
 // assertSharedBucket drives the login limiter from peer with the
@@ -483,8 +458,8 @@ func TestRateLimitKey_SingleValuedHeadersIgnoredFromTrustedPeer(
 			t.Parallel()
 
 			assertSharedBucket(
-				t, trustedProxies(trustedProxyCIDR),
+				t, trustedProxies("10.0.0.0/8"),
-				trustedPeer,
+				"10.0.0.1:44444",
 				func(i int) map[string]string {
 					return map[string]string{
 						header: fmt.Sprintf(
@@ -520,8 +495,8 @@ func TestRateLimitKey_MalformedRightmostHopFallsBackToPeer(
 			t.Parallel()
 
 			assertSharedBucket(
-				t, trustedProxies(trustedProxyCIDR),
+				t, trustedProxies("10.0.0.0/8"),
-				trustedPeer,
+				"10.0.0.1:44444",
 				func(i int) map[string]string {
 					return map[string]string{
 						headerXFF: fmt.Sprintf(
@@ -547,13 +522,13 @@ func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
 	t.Parallel()
 
 	m := rateLimitMiddleware(t, &config.Config{
-		TrustedProxies: trustedProxies(trustedProxyCIDR),
+		TrustedProxies: trustedProxies("10.0.0.0/8"),
 	})
 	handler := m.LoginRateLimit()(okHandler())
 
-	const peer = trustedPeer
+	const peer = "10.0.0.1:44444"
 
-	first := map[string]string{headerXFF: clientIPv4}
+	first := map[string]string{headerXFF: "198.51.100.7"}
 
 	for range middleware.LoginRateLimitConst {
 		postWithHeaders(handler, peer, loginPath, first)
@@ -567,7 +542,7 @@ func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
 
 	w = postWithHeaders(
 		handler, peer, loginPath,
-		map[string]string{headerXFF: clientIPv4Alt},
+		map[string]string{headerXFF: "198.51.100.8"},
 	)
 	assert.Equal(
 		t, http.StatusOK, w.Code,
@@ -584,7 +559,7 @@ func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
 	t.Parallel()
 
 	assertSharedBucket(
-		t, trustedProxies(trustedProxyCIDR), trustedPeer,
+		t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
 		func(i int) map[string]string {
 			return map[string]string{
 				headerXFF: fmt.Sprintf(
@@ -619,7 +594,7 @@ func TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer(
 	start := time.Now()
 
 	assertSharedBucket(
-		t, trustedProxies(trustedProxyCIDR), trustedPeer,
+		t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
 		func(i int) map[string]string {
 			return map[string]string{
 				headerXFF: fmt.Sprintf("9.9.9.%d%s", i+1, padding),
@@ -658,13 +633,13 @@ func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
 	)
 
 	m := rateLimitMiddleware(t, &config.Config{
-		TrustedProxies: trustedProxies(trustedProxyCIDR),
+		TrustedProxies: trustedProxies("10.0.0.0/8"),
 	})
 
 	req := httptest.NewRequestWithContext(
 		context.Background(), http.MethodPost, loginPath, nil,
 	)
-	req.RemoteAddr = trustedPeer
+	req.RemoteAddr = "10.0.0.1:44444"
 	req.Header.Set(
 		headerXFF, "9.9.9.9"+strings.Repeat(", 10.0.0.2", hops),
 	)
@@ -860,369 +835,3 @@ func TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer(
 			"not mint a fresh receiver bucket",
 	)
 }
-
-// clientKeyFor returns the bucket key m computes for a request whose
-// direct peer is remoteAddr and which carries no forwarded headers.
-func clientKeyFor(
-	t *testing.T, m *middleware.Middleware, remoteAddr string,
-) string {
-	t.Helper()
-
-	req := httptest.NewRequestWithContext(
-		context.Background(), http.MethodPost, loginPath, nil,
-	)
-	req.RemoteAddr = remoteAddr
-
-	return middleware.ClientKeyForTest(m, req)
-}
-
-// TestRateLimitKey_IPv6BucketsByPrefix pins the key function's
-// address-family behaviour. IPv6 clients must bucket by /64 — a
-// routed /64 is the normal residential and mobile allocation, so
-// per-/128 keying lets one subscriber rotate source addresses and
-// mint a fresh bucket per request — while IPv4 keeps keying on the
-// full address and IPv4-mapped form is keyed as the IPv4 address it
-// carries.
-func TestRateLimitKey_IPv6BucketsByPrefix(t *testing.T) {
-	t.Parallel()
-
-	m := rateLimitMiddleware(t, &config.Config{})
-
-	for _, tc := range []struct {
-		name  string
-		peer  string
-		want  string
-		about string
-	}{{
-		name:  "ipv6",
-		peer:  "[" + clientIPv6 + "]:44444",
-		want:  clientBucketV6,
-		about: "an IPv6 peer must key on its /64",
-	}, {
-		name:  "ipv6-other-in-same-64",
-		peer:  "[" + clientIPv6Same + "]:1",
-		want:  clientBucketV6,
-		about: "another address in the same /64 must key the same",
-	}, {
-		name:  "ipv6-different-64",
-		peer:  "[" + clientIPv6Other + "]:44444",
-		want:  clientOtherBucketV6,
-		about: "a different /64 must key differently",
-	}, {
-		name:  "ipv4",
-		peer:  clientIPv4 + ":44444",
-		want:  clientIPv4,
-		about: "IPv4 must keep keying on the full address",
-	}, {
-		name:  "ipv4-neighbour",
-		peer:  clientIPv4Alt + ":44444",
-		want:  clientIPv4Alt,
-		about: "adjacent IPv4 addresses must not share a bucket",
-	}, {
-		name: "ipv4-mapped",
-		peer: "[::ffff:" + clientIPv4 + "]:44444",
-		want: clientIPv4,
-		about: "IPv4-mapped form must key as the IPv4 address, " +
-			"not be masked to a /64: mapped addresses all share " +
-			"::ffff:0:0/96, so masking would collapse every IPv4 " +
-			"client behind a mapping proxy into one bucket",
-	}} {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			assert.Equal(
-				t, tc.want, clientKeyFor(t, m, tc.peer), tc.about,
-			)
-		})
-	}
-}
-
-// TestRateLimitKey_FamiliesDoNotCollide pins the structure the
-// no-collision property rests on, rather than one sample pair: every
-// IPv4 key is a bare address and every IPv6 key is a /64 in CIDR
-// form, so the two name spaces are disjoint by shape. Dropping the
-// masking strips the suffix that guarantees it, which is why this
-// asserts the form of each key and not just that two of them differ.
-func TestRateLimitKey_FamiliesDoNotCollide(t *testing.T) {
-	t.Parallel()
-
-	// Restated here rather than imported from the package under
-	// test, so that changing the production bucket width fails this
-	// test instead of silently moving with it.
-	const wantBits = 64
-
-	m := rateLimitMiddleware(t, &config.Config{})
-
-	v4Keys := map[string]bool{}
-
-	for _, peer := range []string{
-		clientIPv4 + ":44444",
-		clientIPv4Alt + ":44444",
-		"[::ffff:" + clientIPv4 + "]:44444",
-	} {
-		key := clientKeyFor(t, m, peer)
-
-		addr, err := netip.ParseAddr(key)
-		require.NoError(
-			t, err, "%s: an IPv4 key must be a bare address", peer,
-		)
-		assert.True(
-			t, addr.Is4(),
-			"%s: an IPv4 key must be a dotted quad, got %q", peer, key,
-		)
-
-		v4Keys[key] = true
-	}
-
-	for _, peer := range []string{
-		"[" + clientIPv6 + "]:44444",
-		"[" + clientIPv6Same + "]:44444",
-		"[" + clientIPv6Other + "]:44444",
-		"[2001:db8::" + clientIPv4 + "]:44444",
-	} {
-		key := clientKeyFor(t, m, peer)
-
-		prefix, err := netip.ParsePrefix(key)
-		require.NoError(
-			t, err, "%s: an IPv6 key must be a CIDR prefix", peer,
-		)
-		assert.Equal(
-			t, wantBits, prefix.Bits(),
-			"%s: an IPv6 key must name a /64", peer,
-		)
-		assert.False(
-			t, v4Keys[key],
-			"%s: an IPv6 key must never equal an IPv4 key", peer,
-		)
-	}
-}
-
-// TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets covers the
-// fallback path. A RemoteAddr that is not an address must not panic,
-// and must not drop unrelated clients into one shared bucket by
-// accident: the raw value is the most specific identity left, so
-// distinct values stay in distinct buckets.
-func TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	m := rateLimitMiddleware(t, &config.Config{})
-
-	first := clientKeyFor(t, m, "not-an-address")
-	second := clientKeyFor(t, m, "also-not-an-address:1234")
-
-	assert.NotEmpty(t, first)
-	assert.NotEqual(
-		t, first, second,
-		"unparseable peers must not collapse into one bucket",
-	)
-}
-
-// TestLoginRateLimit_IPv6SharesBucketWithinSlash64 is the behavioural
-// half, and the regression test for the bypass itself: a client that
-// rotates source addresses inside its own routed /64 must stay in one
-// bucket. Reverting the masking makes this test fail, because each
-// rotated address would mint a fresh bucket and nothing would be
-// rejected.
-func TestLoginRateLimit_IPv6SharesBucketWithinSlash64(t *testing.T) {
-	t.Parallel()
-
-	m := rateLimitMiddleware(t, &config.Config{})
-	handler := m.LoginRateLimit()(okHandler())
-
-	for i := range middleware.LoginRateLimitConst {
-		w := postWithHeaders(
-			handler,
-			fmt.Sprintf("[2001:db8:1:2::%d]:44444", i+1),
-			loginPath, nil,
-		)
-		assert.Equal(
-			t, http.StatusOK, w.Code, "request %d should pass", i,
-		)
-	}
-
-	w := postWithHeaders(
-		handler, "[2001:db8:1:2::ffff]:44444", loginPath, nil,
-	)
-	assert.Equal(
-		t, http.StatusTooManyRequests, w.Code,
-		"rotating source addresses inside one routed /64 must not "+
-			"mint fresh buckets",
-	)
-}
-
-// TestLoginRateLimit_IPv6IndependentAcrossSlash64 is the other side
-// of the trade: bucketing by /64 must not merge separate allocations,
-// so a client in a different /64 keeps its own limit.
-func TestLoginRateLimit_IPv6IndependentAcrossSlash64(t *testing.T) {
-	t.Parallel()
-
-	m := rateLimitMiddleware(t, &config.Config{})
-	handler := m.LoginRateLimit()(okHandler())
-
-	for range middleware.LoginRateLimitConst + 1 {
-		postWithHeaders(
-			handler, "[2001:db8:1:2::1]:44444", loginPath, nil,
-		)
-	}
-
-	w := postWithHeaders(
-		handler, "[2001:db8:1:3::1]:44444", loginPath, nil,
-	)
-	assert.Equal(
-		t, http.StatusOK, w.Code,
-		"a different /64 must have its own bucket",
-	)
-}
-
-// TestLoginRateLimit_IPv4IndependentPerAddress guards against the
-// masking leaking into IPv4: two addresses one apart must still hold
-// separate buckets.
-func TestLoginRateLimit_IPv4IndependentPerAddress(t *testing.T) {
-	t.Parallel()
-
-	m := rateLimitMiddleware(t, &config.Config{})
-	handler := m.LoginRateLimit()(okHandler())
-
-	for range middleware.LoginRateLimitConst + 1 {
-		postWithHeaders(
-			handler, clientIPv4+":44444", loginPath, nil,
-		)
-	}
-
-	w := postWithHeaders(
-		handler, clientIPv4Alt+":44444", loginPath, nil,
-	)
-	assert.Equal(
-		t, http.StatusOK, w.Code,
-		"a second IPv4 address must have its own bucket",
-	)
-}
-
-// forwardedKeyFor returns the bucket key m computes for a request
-// that arrives from trustedPeer — a configured trusted proxy — and
-// names forwarded as its client in X-Forwarded-For. That is the
-// production path: a deployment is required to run behind a reverse
-// proxy with TRUSTED_PROXIES set, so the forwarded address, not the
-// peer, is what the limiters bucket on there.
-func forwardedKeyFor(
-	t *testing.T, m *middleware.Middleware, forwarded string,
-) string {
-	t.Helper()
-
-	req := httptest.NewRequestWithContext(
-		context.Background(), http.MethodPost, loginPath, nil,
-	)
-	req.RemoteAddr = trustedPeer
-	req.Header.Set(headerXFF, forwarded)
-
-	return middleware.ClientKeyForTest(m, req)
-}
-
-// TestRateLimitKey_ForwardedIPv6BucketsByPrefix pins the /64
-// bucketing on the trusted-proxy branch. The direct-peer tests above
-// cannot reach it, so without this the masking could be reverted for
-// forwarded clients alone — the only shape a production deployment
-// runs in — and the rest of the suite would stay green.
-func TestRateLimitKey_ForwardedIPv6BucketsByPrefix(t *testing.T) {
-	t.Parallel()
-
-	m := rateLimitMiddleware(t, &config.Config{
-		TrustedProxies: trustedProxies(trustedProxyCIDR),
-	})
-
-	for _, tc := range []struct {
-		name      string
-		forwarded string
-		want      string
-		about     string
-	}{{
-		name:      "ipv6",
-		forwarded: clientIPv6,
-		want:      clientBucketV6,
-		about:     "a forwarded IPv6 client must key on its /64",
-	}, {
-		name:      "ipv6-other-in-same-64",
-		forwarded: clientIPv6Same,
-		want:      clientBucketV6,
-		about: "another forwarded address in the same /64 must " +
-			"key the same",
-	}, {
-		name:      "ipv6-different-64",
-		forwarded: clientIPv6Other,
-		want:      clientOtherBucketV6,
-		about:     "a forwarded address in another /64 must differ",
-	}, {
-		name:      "ipv4",
-		forwarded: clientIPv4,
-		want:      clientIPv4,
-		about:     "a forwarded IPv4 client must key on the address",
-	}, {
-		name:      "ipv4-mapped",
-		forwarded: "::ffff:" + clientIPv4,
-		want:      clientIPv4,
-		about: "a proxy that forwards IPv4-mapped form must key as " +
-			"the IPv4 address it carries, not be masked to a /64: " +
-			"mapped addresses all share ::ffff:0:0/96",
-	}} {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			assert.Equal(
-				t, tc.want,
-				forwardedKeyFor(t, m, tc.forwarded), tc.about,
-			)
-		})
-	}
-}
-
-// TestLoginRateLimit_ForwardedIPv6SharesBucketWithinSlash64 is the
-// behavioural half on the production path: behind a trusted proxy, a
-// client rotating source addresses inside its own routed /64 must
-// stay in one bucket.
-func TestLoginRateLimit_ForwardedIPv6SharesBucketWithinSlash64(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	assertSharedBucket(
-		t, trustedProxies(trustedProxyCIDR), trustedPeer,
-		func(i int) map[string]string {
-			return map[string]string{
-				headerXFF: fmt.Sprintf("2001:db8:1:2::%d", i+1),
-			}
-		},
-		"rotating forwarded source addresses inside one routed /64 "+
-			"must not mint fresh buckets",
-	)
-}
-
-// TestLoginRateLimit_ForwardedIPv6IndependentAcrossSlash64 is the
-// other side of that trade on the same path: bucketing by /64 must
-// not merge two allocations reaching the proxy.
-func TestLoginRateLimit_ForwardedIPv6IndependentAcrossSlash64(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	m := rateLimitMiddleware(t, &config.Config{
-		TrustedProxies: trustedProxies(trustedProxyCIDR),
-	})
-	handler := m.LoginRateLimit()(okHandler())
-
-	spent := map[string]string{headerXFF: clientIPv6}
-	for range middleware.LoginRateLimitConst + 1 {
-		postWithHeaders(handler, trustedPeer, loginPath, spent)
-	}
-
-	w := postWithHeaders(
-		handler, trustedPeer, loginPath,
-		map[string]string{headerXFF: clientIPv6Other},
-	)
-	assert.Equal(
-		t, http.StatusOK, w.Code,
-		"a forwarded client in a different /64 must have its own "+
-			"bucket",
-	)
-}

internal/server/routes_test.go

@@ -24,7 +24,6 @@ import (
 	"sneak.berlin/go/webhooker/internal/middleware"
 	"sneak.berlin/go/webhooker/internal/server"
 	"sneak.berlin/go/webhooker/internal/session"
-	"sneak.berlin/go/webhooker/static"
 )
 
 // csrfCookieName is the cookie gorilla/csrf issues when it runs. Its
@@ -247,56 +246,6 @@ func (e *testEnv) storedHash(t *testing.T, username string) string {
 	return user.Password
 }
 
-// --- /s static group ---
-
-// TestStaticServesEveryMethod pins what the static mount actually
-// answers. chi's Mount registers the handler for all methods and
-// http.FileServer only special-cases HEAD (by suppressing the body),
-// so a POST or a DELETE to an asset is served the file rather than
-// refused. The README documents this; the test is what keeps the two
-// from drifting.
-func TestStaticServesEveryMethod(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	body, err := static.Static.ReadFile("js/app.js")
-	require.NoError(t, err)
-	require.NotEmpty(t, body)
-
-	for _, method := range []string{
-		http.MethodGet,
-		http.MethodHead,
-		http.MethodPost,
-		http.MethodPut,
-		http.MethodDelete,
-	} {
-		t.Run(method, func(t *testing.T) {
-			t.Parallel()
-
-			req := httptest.NewRequestWithContext(
-				context.Background(), method,
-				"/s/js/app.js", nil,
-			)
-			w := httptest.NewRecorder()
-			env.router.ServeHTTP(w, req)
-
-			assert.Equal(t, http.StatusOK, w.Code,
-				"static mount answers every method")
-
-			if method == http.MethodHead {
-				assert.Empty(t, w.Body.Bytes(),
-					"HEAD must not carry a body")
-
-				return
-			}
-
-			assert.Equal(t, body, w.Body.Bytes(),
-				"the asset itself is returned")
-		})
-	}
-}
-
 // --- /pages group ---
 
 // TestPagesLogin_OversizeBody_RejectedBeforeCSRF proves the cap runs

internal/server/server.go

@@ -24,48 +24,15 @@ import (
 )
 
 const (
-	// ShutdownTimeout is the maximum time to wait for the HTTP
+	// shutdownTimeout is the maximum time to wait for the HTTP
 	// server to finish in-flight requests during shutdown.
-	//
+	shutdownTimeout = 5 * time.Second
-	// It must stay strictly below the fx stop timeout in
-	// cmd/webhooker, which bounds the whole stop sequence: a drain
-	// that used the entire sequence budget would leave nothing for
-	// the hooks that run after the server, including the database
-	// close. It is exported so that relationship can be tested.
-	ShutdownTimeout = 3 * time.Second
 
-	// TailHookReserve is the share of the fx stop budget this hook
+	// sentryFlushTimeout is the maximum time to wait for Sentry
-	// refuses to spend, leaving it for the hooks that run after the
+	// to flush pending events during shutdown.
-	// server: the delivery engine, the healthcheck, the webhook DB
-	// manager and the database close.
-	TailHookReserve = 2 * time.Second
-
-	// sentryFlushTimeout is the longest wait for Sentry to flush
-	// pending events during shutdown, before the remaining stop
-	// budget is taken into account.
 	sentryFlushTimeout = 2 * time.Second
-
-	// minSentryFlush is the shortest flush worth attempting. Below
-	// it the remaining budget goes to the tail hooks instead.
-	minSentryFlush = 250 * time.Millisecond
 )
 
-// SentryFlushBudget reports how long the Sentry flush may run when
-// remaining is the time left on the fx stop context after the HTTP
-// drain. sentry.Flush takes a bare duration and honours no context,
-// so this clamp is the only thing keeping a stalled flush from
-// spending the tail hooks' share of the budget on top of a
-// full-length drain. TailHookReserve is held back, and anything
-// under minSentryFlush is skipped rather than attempted uselessly.
-func SentryFlushBudget(remaining time.Duration) time.Duration {
-	budget := min(remaining-TailHookReserve, sentryFlushTimeout)
-	if budget < minSentryFlush {
-		return 0
-	}
-
-	return budget
-}
-
 //nolint:revive // ServerParams is a standard fx naming convention.
 type ServerParams struct {
 	fx.In
@@ -197,7 +164,7 @@ func (s *Server) cleanShutdown(ctx context.Context) {
 	s.exitCode = 0
 
 	ctxShutdown, shutdownCancel := context.WithTimeout(
-		ctx, ShutdownTimeout,
+		ctx, shutdownTimeout,
 	)
 	defer shutdownCancel()
 
@@ -211,31 +178,10 @@ func (s *Server) cleanShutdown(ctx context.Context) {
 	s.cleanupForExit()
 
 	if s.sentryEnabled {
-		s.flushSentry(ctx)
+		sentry.Flush(sentryFlushTimeout)
 	}
 }
 
-// flushSentry drains Sentry's queue inside what is left of the fx
-// stop budget. A context carrying no deadline — a caller outside the
-// fx lifecycle — gets the full timeout.
-func (s *Server) flushSentry(ctx context.Context) {
-	flush := sentryFlushTimeout
-
-	if deadline, ok := ctx.Deadline(); ok {
-		flush = SentryFlushBudget(time.Until(deadline))
-	}
-
-	if flush <= 0 {
-		s.log.Warn(
-			"skipping sentry flush, stop budget exhausted",
-		)
-
-		return
-	}
-
-	sentry.Flush(flush)
-}
-
 func (s *Server) configure() {
 	// identify ourselves in the logs
 	s.params.Logger.Identify()

internal/server/shutdown_test.go

@@ -1,59 +0,0 @@
-package server_test
-
-import (
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/server"
-)
-
-// TestSentryFlushBudget covers the clamp that keeps the Sentry flush
-// from spending the tail hooks' share of the fx stop budget.
-// sentry.Flush ignores the stop context, so without the clamp a
-// stalled flush adds its whole timeout on top of the HTTP drain.
-func TestSentryFlushBudget(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name      string
-		remaining time.Duration
-		want      time.Duration
-	}{
-		{
-			name:      "full drain leaves only the reserve",
-			remaining: server.TailHookReserve,
-			want:      0,
-		},
-		{
-			name:      "expired budget",
-			remaining: -time.Second,
-			want:      0,
-		},
-		{
-			name:      "sliver above the reserve is not worth it",
-			remaining: server.TailHookReserve + 10*time.Millisecond,
-			want:      0,
-		},
-		{
-			name:      "partial flush when some room is left",
-			remaining: server.TailHookReserve + time.Second,
-			want:      time.Second,
-		},
-		{
-			name:      "capped at the nominal timeout",
-			remaining: time.Hour,
-			want:      2 * time.Second,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			require.Equal(
-				t, tt.want, server.SentryFlushBudget(tt.remaining),
-			)
-		})
-	}
-}