Stop target credentials leaking into event databases (closes #206)

A Delivery carries its Event and Target structs in memory for the
delivery engine, so GORM's automatic association save upserted the
whole target row -- config included, which holds destination URLs
and bearer credentials -- into the per-webhook event database with
an empty webhook_id. Event databases are the files most likely to be
backed up or handed to someone else, so they shipped the credentials
with them.

Register a create and update callback on every per-webhook
connection that omits associations, rather than fixing the one call
site: it covers writes inside a transaction and write paths added
later. Sweep any rows already written, before the migration on each
open, so it is idempotent and a no-op on a database with no targets
table.

Encryption of target config at rest in webhooker.db is deliberately
not part of this: it is tracked separately.

README.md

@@ -127,6 +127,40 @@ empty string, and quietly withholding it would deny an endpoint that
 was asked for. The `hasMetricsAuth` field in the startup log and the
 existence of the route are the same value, so they cannot disagree.
 
+#### Single-instance lock
+
+Exactly one webhooker process may use a `DATA_DIR` at a time. Two
+processes sharing one open the same databases and each run delivery
+recovery over the same rows, so every pending delivery goes out twice —
+duplicate delivery to your endpoints, from nothing worse than an
+overlapping deploy or a double start.
+
+At startup, before anything opens a database, the process takes an
+exclusive advisory lock (`flock(2)`) on `{DATA_DIR}/webhooker.lock` and
+holds it for its lifetime. A second process pointed at the same
+directory prints a message naming it and exits non-zero:
+
+```
+webhooker: data directory is already in use by another instance: /var/lib/webhooker (/var/lib/webhooker/webhooker.lock). Only one webhooker may use a data directory: two both run delivery recovery over the same rows and both deliver
+```
+
+The lock is the kernel's, not the file's: it is released when the
+process exits, including `kill -9`, so a leftover `webhooker.lock`
+never blocks a restart and must not be deleted by hand. The file is
+also left in place on a clean shutdown, deliberately — unlinking it
+would let the next process lock a fresh inode while a third still held
+the old one.
+
+To run two webhookers on one host, give each its own `DATA_DIR`.
+
+`flock(2)` is host-local and per-inode: it arbitrates between processes
+and containers sharing a volume or bind mount on one machine, but not
+between hosts on a network filesystem, and a `DATA_DIR` inside a
+container's own writable layer is not shared with anything. On a
+filesystem that refuses `flock` outright, startup fails closed — the
+process reports the error and refuses to start rather than running
+unlocked.
+
 #### Trusted proxies
 
 `TRUSTED_PROXIES` is a comma-separated list of CIDR blocks (a bare
@@ -256,6 +290,51 @@ On first startup, webhooker creates an `admin` user
 with a randomly generated password and logs it to stdout. This password
 is only displayed once.
 
+#### What `DEBUG=true` exposes
+
+`DEBUG=true` lowers the log level to `DEBUG`, which turns on every
+statement GORM runs, the two by-design lookup misses on the
+unauthenticated routes, and the rate limiter's own rejections. It is
+meant to be safe to turn on while diagnosing a live service and safe to
+paste the output of into a bug report.
+
+What it does **not** put in the log:
+
+- **Values bound to a SQL statement.** Statements are logged with their
+  placeholders, never with the values substituted into them, at every
+  level. That is what keeps the session encryption key out of the first
+  boot's `INSERT INTO settings` and the `admin` account's Argon2id
+  password hash out of its `INSERT INTO users` — the two statements
+  that made a debug log worth stealing. It applies to every table and
+  every statement rather than to a list of tables known to hold a
+  secret, so a table added later is covered without anyone remembering
+  to add it. The cost is that a failing statement can no longer be
+  replayed from the log alone: the statement, the table, the driver
+  error and the row count are all still there, but its values have to
+  come from the database.
+  `internal/gormlog/firstboot_test.go` boots the real graph with
+  `DEBUG=true` against an empty `DATA_DIR` and asserts that neither
+  secret appears in what that boot wrote to stdout.
+  The one exception is `(*gorm.DB).Scan`, which GORM logs through its
+  own trace recorder rather than through this filter. No production
+  code path calls it, and `internal/gormlog/scan_guard_test.go` fails
+  if a non-test file adds one.
+- **Session cookies, API keys or target credentials.** None of these is
+  logged at any level.
+
+What is in the log regardless of `DEBUG`, and is not a debug-logging
+decision:
+
+- **The initial `admin` password**, in the clear, once, at `INFO`, on
+  the first boot that creates the account. That line is the only place
+  it is ever shown; the database stores the hash. A first boot's output
+  is not safe to paste anywhere until that account's password has been
+  changed.
+- **An authenticated operator's own configuration**, echoed back
+  untruncated — webhook names, target hostnames. See the logging
+  section under Security for the full list and for the per-line size
+  bound that covers unauthenticated traffic.
+
 ### Running with Docker
 
 ```bash
@@ -295,10 +374,12 @@ is both the simplest and the only complete rule:
 
 `{webhook_uuid}` is the webhook's UUID primary key in its canonical
 36-character hyphenated form, so a real filename looks like
-`events-3f2a1c9e-....db`. Nothing else is written to `DATA_DIR`, and no
+`events-3f2a1c9e-....db`. The only other file is `webhooker.lock`, the
-`-wal` or `-shm` files are produced (see below); a transient
+always-empty [single-instance lock](#single-instance-lock); it holds no
-`{name}.db-journal` may exist beside a database while a write is in
+state and is not part of the backup set — a copied one is stale and
-flight and is not part of the backup set.
+blocks nothing. No `-wal` or `-shm` files are produced (see below); a
+transient `{name}.db-journal` may exist beside a database while a write
+is in flight and is not part of the backup set either.
 
 Configuration is **not** in `DATA_DIR` — it comes from the environment
 and from a `.env` file read out of the process working directory. Back
@@ -425,14 +506,28 @@ backups at rest and restrict who can read them.
 - `events-{uuid}.db` and `archive-{uuid}.db` hold the **full payload
   body and headers** of every event as received, including whatever the
   sending service put in them — tokens, signatures, personal data.
-- Until
+- Event databases written before
-  [issue #206](https://git.eeqj.de/sneak/webhooker/issues/206) is fixed,
+  [issue #206](https://git.eeqj.de/sneak/webhooker/issues/206) was fixed
-  the event databases **also contain target credentials**: a GORM
+  **also contain target credentials**: a GORM association upsert on the
-  association upsert on the delivery and retry write path copies
+  delivery and retry write path copied `targets` rows, `config`
-  `targets` rows, `config` included, into the per-webhook database. For
+  included, into the per-webhook database. For a Slack target the
-  a Slack target the `webhookUrl` *is* the bearer credential, and an
+  `webhookUrl` *is* the bearer credential, and an `http` target's URL
-  `http` target's URL can embed userinfo. Handing someone an
+  can embed userinfo. This version never writes those rows; the first
-  `events-*.db` today hands them live delivery destinations.
+  time it opens such a file it deletes them and vacuums the file, which
+  removes the credential bytes rather than only unlinking the rows.
+  Deleting alone would not: the bytes stay readable in the file's free
+  pages until it is rewritten. The sweep is recorded in the file's
+  `user_version` only once the vacuum returns, so a sweep that fails or
+  is interrupted fails the open and is retried on the next one, and a
+  file this version has opened without error holds no leaked rows and
+  no recoverable bytes from them. On upgrade this rewrites each
+  existing `events-{uuid}.db` once, on its first open. Two cases still
+  hand over live delivery destinations: a backup taken from an older
+  build, and a backup of a file this version has not yet opened
+  successfully. Copies already made stay affected — the sweep only
+  rewrites the file it opens, and freed blocks may persist in
+  filesystem snapshots and on the underlying storage. Rotate any target
+  credential that was in a backup you cannot account for.
 - `webhooker.db` stores target config **unencrypted**, tracked at
   [issue #212](https://git.eeqj.de/sneak/webhooker/issues/212), next to
   the session encryption key and the Argon2id password hashes.
@@ -1134,6 +1229,52 @@ delivery as `retrying` and schedules a retry timer for after the
 remaining cooldown period. This ensures no deliveries are lost — they're
 just delayed until the target is healthy again.
 
+### Metrics
+
+`/metrics` serves one Prometheus registry behind basic auth (see
+[Infrastructure Endpoints](#infrastructure-endpoints)). Alongside the
+inbound HTTP metrics recorded by the middleware, it exposes the
+delivery pipeline — the part of the service that can be failing while
+the receive side looks perfectly healthy, because it is: events are
+arriving and being stored, they are just not getting anywhere.
+
+| Metric | Type | Meaning |
+| ------ | ---- | ------- |
+| `webhooker_events_received_total` | counter | Events received and durably stored. Compare against the delivery counters on one dashboard |
+| `webhooker_delivery_attempts_total` | counter | Delivery attempts actually dispatched to a target. A delivery an open circuit breaker refused is not one: it is counted as a retry instead |
+| `webhooker_deliveries_succeeded_total` | counter | Deliveries that reached `delivered` |
+| `webhooker_deliveries_failed_total` | counter | Deliveries that failed terminally and will not be retried |
+| `webhooker_delivery_retries_total` | counter | Deliveries put back into `retrying` |
+| `webhooker_delivery_duration_seconds` | histogram | Wall time of a single dispatched delivery attempt, the same duration the attempt's `DeliveryResult` records |
+| `webhooker_deliveries_pending` | gauge | Deliveries currently in `pending` |
+| `webhooker_deliveries_retrying` | gauge | Deliveries currently in `retrying` |
+| `webhooker_circuit_breakers_open` | gauge | Circuit breakers currently open |
+
+Every delivery metric carries exactly one label, `target_type`, and
+cardinality is the whole reason for that restriction. A target type is
+one of four compile-time constants, so the label domain is bounded by
+construction; a value outside that set collapses to `unknown` rather
+than minting a series of its own. Target ids, event ids and entrypoint
+ids are deliberately not labels: they are UUIDs minted per operator
+action or per inbound request, a series is never reclaimed once it
+exists, and labelling by any of them would make `/metrics` a memory
+leak that grows with traffic.
+
+The two queue-depth gauges are counted out of the databases by a
+sampler that runs every 30 seconds for as long as the delivery engine
+does, rather than tracked as deltas alongside the status transitions: a
+delta would have to be seeded at startup from rows a previous process
+wrote, and would drift permanently on any transition that failed to
+persist.
+
+Those two gauges also publish an `unknown` series, from startup rather
+than on first occurrence. Deliveries queued against a target that has
+since been deleted are counted there: that backlog is the one nobody is
+watching, so it is the one that must not silently vanish from the
+gauge. The outcome counters move only after the status change has been
+written, so a transition the database rejected is never reported as an
+outcome that happened.
+
 ### Rate Limiting
 
 Global blanket rate limiting middleware (e.g., a per-IP throttle shared
@@ -1418,6 +1559,24 @@ and the driver error — against a smaller fixed portion than the access
 log's, and `internal/gormlog/gormlog_test.go` asserts each line against
 `MaxAccessLogLineBytes` directly rather than leaving it as arithmetic.
 
+The adapter also logs no bound value at all: it implements
+`gorm.ParamsFilter` and discards the parameters, so GORM renders the
+statement with its placeholders intact instead of substituting the
+values into it. That is a separate property from the size bound and it
+is what a bound is no substitute for — the session encryption key is 44
+base64 characters and an Argon2id hash under 100, so both fit inside
+every budget above and a truncated secret is still a secret. It holds
+on all three arms of `Trace`, including the routine one an operator
+reaches at `DEBUG`, which is the only level at which a successful
+`INSERT` is written at all. One GORM path does not consult the filter —
+`(*gorm.DB).Scan`, which records the statement through GORM's own trace
+recorder. No production code path calls it; its one caller is
+`internal/database/database_test.go:91`, whose `SELECT 1` binds
+nothing, and `internal/gormlog/scan_guard_test.go` fails if a non-test
+file calls it. `Pluck`, `Row` and `Raw` all run through the normal
+callback processor and are filtered.
+See `#### What DEBUG=true exposes` under Configuration.
+
 What that ceiling does **not** cover, stated here so the figure is not
 read as more than it is:
 
@@ -1721,7 +1880,7 @@ imports. The entry point is `cmd/webhooker/main.go`.
 ```
 webhooker/
 ├── cmd/webhooker/
-│   └── main.go               # Entry point: sets globals, wires fx
+│   └── main.go               # Entry point: sets globals, locks DATA_DIR, wires fx
 ├── internal/
 │   ├── config/
 │   │   └── config.go          # Configuration loading from environment variables
@@ -1742,6 +1901,8 @@ webhooker/
 │   │   ├── 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
+│   ├── datadir/
+│   │   └── lock.go            # Exclusive advisory lock on DATA_DIR (one instance)
 │   ├── globals/
 │   │   └── globals.go         # Build-time variables (appname, version, arch)
 │   ├── gormlog/
@@ -1759,6 +1920,7 @@ webhooker/
 │   │   ├── target_log.go      # Log target (stdout)
 │   │   ├── target_config_view.go  # Masked target config for templates
 │   │   ├── archive_sweeper.go # Periodic pruning of idle archives
+│   │   ├── queue_depth.go     # Periodic sampler behind the queue-depth gauges
 │   │   ├── url_mask.go        # Strips credentials from *url.Error
 │   │   └── ssrf.go            # SSRF prevention (IP validation, safe HTTP transport)
 │   ├── handlers/
@@ -1776,6 +1938,8 @@ webhooker/
 │   │   └── lifecycle.go       # Shared stop-hook waiter, bounded by the stop context
 │   ├── logger/
 │   │   └── logger.go          # slog setup with TTY detection
+│   ├── metrics/
+│   │   └── metrics.go         # Delivery Prometheus collectors, labelled by target type
 │   ├── middleware/
 │   │   ├── middleware.go      # Logging, CORS, Auth, Metrics, MetricsAuth, SecurityHeaders, MaxBodySize
 │   │   ├── csrf.go            # CSRF protection middleware (gorilla/csrf)

cmd/webhooker/main.go

@@ -2,11 +2,15 @@
 package main
 
 import (
+	"fmt"
+	"io"
+	"os"
 	"time"
 
 	"go.uber.org/fx"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/datadir"
 	"sneak.berlin/go/webhooker/internal/delivery"
 	"sneak.berlin/go/webhooker/internal/globals"
 	"sneak.berlin/go/webhooker/internal/handlers"
@@ -56,7 +60,32 @@ func main() {
 	globals.Appname = appname
 	globals.Version = version
 
+	os.Exit(run(os.Stderr))
+}
+
+// run takes the exclusive DATA_DIR lock, then runs the application
+// under it, and returns the process exit status.
+//
+// The lock is taken here rather than inside the fx graph because it has
+// to be held before anything opens a database, and because a refusal
+// has to reach the operator as a plain line on standard error rather
+// than as one entry in an fx failure dump. It is released by the defer
+// on a clean shutdown, and by the kernel closing the descriptor on any
+// other exit — including the one fx performs itself when a start or
+// stop hook fails, which skips deferred calls.
+func run(stderr io.Writer) int {
+	lock, err := datadir.Acquire(config.DataDir())
+	if err != nil {
+		_, _ = fmt.Fprintf(stderr, "%s: %v\n", appname, err)
+
+		return 1
+	}
+
+	defer func() { _ = lock.Release() }()
+
 	newApp().Run()
+
+	return 0
 }
 
 // newApp builds the application graph. It is separate from main so

cmd/webhooker/main_test.go

@@ -1,10 +1,13 @@
 package main
 
 import (
+	"bytes"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/datadir"
 	"sneak.berlin/go/webhooker/internal/server"
 )
 
@@ -33,6 +36,38 @@ func TestNewApp_StopTimeout(t *testing.T) {
 	require.Less(t, got, dockerStopGrace)
 }
 
+// TestRunRefusesLockedDataDir pins what an operator's second start
+// does. The entry point must refuse before it builds the fx graph —
+// nothing may open a database in a DATA_DIR another process holds —
+// and must exit non-zero with a message naming the directory rather
+// than starting a second delivery engine over the same rows.
+//
+// flock(2) locks descriptors independently, so holding the lock here
+// is the same denial a separate process gets; internal/datadir pins
+// that property and covers the real two-process case.
+func TestRunRefusesLockedDataDir(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("DATA_DIR", dir)
+
+	lock, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+
+	defer func() { _ = lock.Release() }()
+
+	var stderr bytes.Buffer
+
+	code := run(&stderr)
+
+	require.Equal(
+		t, 1, code, "a second instance must exit non-zero",
+	)
+	assert.Contains(
+		t, stderr.String(), dir,
+		"the refusal must name the directory",
+	)
+	assert.Contains(t, stderr.String(), "another instance")
+}
+
 // 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

go.mod

@@ -8,13 +8,15 @@ require (
 	github.com/go-chi/chi v1.5.5
 	github.com/go-chi/cors v1.2.1
 	github.com/go-chi/httprate v0.15.0
+	github.com/gofrs/flock v0.13.0
 	github.com/google/uuid v1.6.0
 	github.com/gorilla/csrf v1.7.3
 	github.com/gorilla/sessions v1.4.0
 	github.com/joho/godotenv v1.5.1
 	github.com/prometheus/client_golang v1.18.0
+	github.com/prometheus/client_model v0.5.0
 	github.com/slok/go-http-metrics v0.11.0
-	github.com/stretchr/testify v1.8.4
+	github.com/stretchr/testify v1.11.1
 	go.uber.org/fx v1.20.1
 	golang.org/x/crypto v0.38.0
 	gopkg.in/yaml.v3 v3.0.1
@@ -38,7 +40,6 @@ require (
 	github.com/mattn/go-sqlite3 v1.14.17 // indirect
 	github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
-	github.com/prometheus/client_model v0.5.0 // indirect
 	github.com/prometheus/common v0.45.0 // indirect
 	github.com/prometheus/procfs v0.12.0 // indirect
 	github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect
@@ -49,7 +50,7 @@ require (
 	go.uber.org/zap v1.23.0 // indirect
 	golang.org/x/mod v0.17.0 // indirect
 	golang.org/x/sync v0.14.0 // indirect
-	golang.org/x/sys v0.33.0 // indirect
+	golang.org/x/sys v0.37.0 // indirect
 	golang.org/x/text v0.25.0 // indirect
 	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	google.golang.org/protobuf v1.31.0 // indirect

go.sum

@@ -23,6 +23,8 @@ github.com/go-chi/httprate v0.15.0 h1:j54xcWV9KGmPf/X4H32/aTH+wBlrvxL7P+SdnRqxh5
 github.com/go-chi/httprate v0.15.0/go.mod h1:rzGHhVrsBn3IMLYDOZQsSU4fJNWcjui4fWKJcCId1R4=
 github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=
 github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og=
+github.com/gofrs/flock v0.13.0 h1:95JolYOvGMqeH31+FC7D2+uULf6mG61mEZ/A8dRYMzw=
+github.com/gofrs/flock v0.13.0/go.mod h1:jxeyy9R1auM5S6JYDBhDt+E2TCo7DkratH4Pgi8P+Z0=
 github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
@@ -81,11 +83,11 @@ github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncj
 github.com/slok/go-http-metrics v0.11.0 h1:ABJUpekCZSkQT1wQrFvS4kGbhea/w6ndFJaWJeh3zL0=
 github.com/slok/go-http-metrics v0.11.0/go.mod h1:ZGKeYG1ET6TEJpQx18BqAJAvxw9jBAZXCHU7bWQqqAc=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/objx v0.5.1 h1:4VhoImhV/Bm0ToFkXFi8hXNXwpDRZ/ynw3amt82mzq0=
+github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
-github.com/stretchr/objx v0.5.1/go.mod h1:/iHQpkQwBD6DLUmQ4pE+s1TXdob1mORJ4/UFdrifcy0=
+github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
-github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
-github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
+github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
 github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ=
 github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0=
 github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0=
@@ -109,8 +111,8 @@ golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
 golang.org/x/sync v0.14.0 h1:woo0S4Yywslg6hp4eUFjTVOyKt0RookbpAHG4c1HmhQ=
 golang.org/x/sync v0.14.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw=
+golang.org/x/sys v0.37.0 h1:fdNQudmxPjkdUTPnLn5mdQv7Zwvbvpaxqs831goi9kQ=
-golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
+golang.org/x/sys v0.37.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
 golang.org/x/text v0.25.0 h1:qVyWApTSYLk/drJRO5mDlNYskwQznZmkpV2c8q9zls4=
 golang.org/x/text v0.25.0/go.mod h1:WEdwpYrmk1qmdHvhkSTNPm3app7v4rsT8F2UD6+VHIA=
 golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d h1:vU5i/LfpvrRCpgM/VPfJLg5KjxD3E+hfT1SH+d9zLwg=

internal/config/config.go

@@ -26,6 +26,10 @@ const (
 	// EnvironmentProd represents production environment.
 	EnvironmentProd = "prod"
 
+	// DefaultDataDir is where all SQLite databases live when DATA_DIR
+	// is unset. The same default applies in every environment.
+	DefaultDataDir = "/var/lib/webhooker"
+
 	// defaultPort is the default HTTP listen port.
 	defaultPort = 8080
 
@@ -159,6 +163,19 @@ func envString(key string) string {
 	return os.Getenv(key)
 }
 
+// DataDir resolves DATA_DIR, applying DefaultDataDir when it is unset
+// or empty. It is exported so that entry points which must act on the
+// data directory before the fx graph exists — taking the exclusive
+// directory lock, above all — resolve it exactly as Config does.
+func DataDir() string {
+	dir := envString("DATA_DIR")
+	if dir == "" {
+		return DefaultDataDir
+	}
+
+	return dir
+}
+
 // envBool returns the value of the named environment variable
 // parsed as a boolean. Returns defaultValue if not set. If the
 // variable is set but cannot be parsed, it returns a wrapped error
@@ -461,7 +478,7 @@ func loadFromEnv() (*Config, error) {
 	}
 
 	return &Config{
-		DataDir:                envString("DATA_DIR"),
+		DataDir:                DataDir(),
 		Debug:                  debug,
 		MaintenanceMode:        maintenanceMode,
 		Environment:            environment,
@@ -539,14 +556,6 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
 	s.log = log
 	s.params = &params
 
-	// Set default DataDir. All SQLite databases (main application
-	// DB and per-webhook event DBs) live here. The same default is
-	// used regardless of environment; override with DATA_DIR if
-	// needed.
-	if s.DataDir == "" {
-		s.DataDir = "/var/lib/webhooker"
-	}
-
 	if s.Debug {
 		params.Logger.EnableDebugLogging()
 	}

internal/config/config_test.go

@@ -424,6 +424,36 @@ func TestDefaultDataDir(t *testing.T) {
 	}
 }
 
+// TestDataDirHelper pins the exported resolver against the value
+// Config carries. The entry point takes the DATA_DIR lock through the
+// helper before the fx graph exists, so the two disagreeing would mean
+// locking one directory and writing to another.
+func TestDataDirHelper(t *testing.T) {
+	for _, set := range []string{"", "/tmp/webhooker-datadir-helper"} {
+		name := "set"
+		if set == "" {
+			name = "unset"
+		}
+
+		t.Run(name, func(t *testing.T) {
+			// Cannot use t.Parallel() here because t.Setenv
+			// is incompatible with parallel subtests.
+			if set == "" {
+				require.NoError(t, os.Unsetenv("DATA_DIR"))
+			} else {
+				t.Setenv("DATA_DIR", set)
+			}
+
+			expected := set
+			if expected == "" {
+				expected = config.DefaultDataDir
+			}
+
+			assert.Equal(t, expected, config.DataDir())
+		})
+	}
+}
+
 func TestReceiverRateLimit(t *testing.T) {
 	tests := []struct {
 		name        string

internal/database/event_db_isolation.go

@@ -0,0 +1,159 @@
+package database
+
+import (
+	"fmt"
+	"log/slog"
+
+	"gorm.io/gorm"
+	"gorm.io/gorm/clause"
+)
+
+// omitAssociationsCallback is the name the association guard is
+// registered under on a per-webhook database's create and update
+// callback chains.
+const omitAssociationsCallback = "webhooker:omit_associations"
+
+// omitAssociations makes every create and update issued against a
+// per-webhook database skip GORM's automatic association save.
+//
+// A per-webhook database holds the event tier only, but Delivery
+// declares belongs-to Event and Target and the delivery engine fills
+// both in memory before writing. Without this guard GORM upserts
+// those parent rows here on the delivery and retry write paths,
+// copying targets.config, which holds destination URLs and bearer
+// credentials, into the file most likely to be backed up or handed
+// to someone else. Registering the guard on the connection covers
+// every write path, including writes inside a transaction and write
+// paths added later. Every event-tier row this file holds is written
+// explicitly, so nothing depends on the automatic save.
+func omitAssociations(db *gorm.DB) error {
+	omit := func(tx *gorm.DB) {
+		tx.Statement.Omits = append(
+			tx.Statement.Omits, clause.Associations,
+		)
+	}
+
+	err := db.Callback().Create().
+		Before("gorm:save_before_associations").
+		Register(omitAssociationsCallback, omit)
+	if err != nil {
+		return fmt.Errorf(
+			"registering create association guard: %w", err,
+		)
+	}
+
+	err = db.Callback().Update().
+		Before("gorm:save_before_associations").
+		Register(omitAssociationsCallback, omit)
+	if err != nil {
+		return fmt.Errorf(
+			"registering update association guard: %w", err,
+		)
+	}
+
+	return nil
+}
+
+// eventDBSweptVersion is the PRAGMA user_version purgeTargetRows
+// stamps into a per-webhook database once it has removed any leaked
+// target rows *and* the VACUUM that removes their bytes has returned.
+// Nothing else in the tree uses user_version, so 0 means "not swept
+// by this build".
+//
+// The stamp, not the DELETE, is what records that a file is done. A
+// DELETE commits on its own, so a sweep that is interrupted or whose
+// VACUUM fails leaves a file whose rows are gone but whose credential
+// bytes are still in the free pages -- indistinguishable, by row
+// count, from a file that never leaked. Both leave the stamp unset,
+// so the next open sweeps again.
+const eventDBSweptVersion = 1
+
+// purgeTargetRows deletes target rows that an earlier build's
+// association upsert wrote into a per-webhook database, and rewrites
+// the file so their bytes are gone with them. AutoMigrate creates a
+// targets table in every one of these files because Delivery declares
+// a belongs-to Target, but nothing in the event tier may put rows in
+// it. The rows it did put there are junk, not history: they carry an
+// empty webhook_id, and delivery rows resolve their target against
+// the main database, so nothing here refers to them.
+//
+// The DELETE only unlinks the rows: modernc.org/sqlite leaves
+// secure_delete at SQLite's default of off, so the credential bytes
+// stay readable in the file's free pages and a backup of a swept file
+// would still hand them over. VACUUM rewrites the file without them.
+//
+// This runs before every migration and is gated on
+// eventDBSweptVersion, so a file pays for the rewrite once, on the
+// first open that finds it unstamped, and every open after that is a
+// PRAGMA read. A file this build created is stamped before its
+// targets table exists, so it never vacuums at all. A failure here
+// fails the open with the stamp left unset, so the sweep is retried
+// rather than skipped -- a webhook whose file cannot be swept stays
+// unusable instead of quietly serving from a file that still holds
+// recoverable credentials.
+func purgeTargetRows(
+	db *gorm.DB, log *slog.Logger, webhookID string,
+) error {
+	var version int
+
+	// Row().Scan, not (*gorm.DB).Scan: see internal/gormlog.
+	err := db.Raw("PRAGMA user_version").Row().Scan(&version)
+	if err != nil {
+		return fmt.Errorf(
+			"reading sweep marker of webhook database %s: %w",
+			webhookID, err,
+		)
+	}
+
+	if version >= eventDBSweptVersion {
+		return nil
+	}
+
+	var purged int64
+
+	if db.Migrator().HasTable("targets") {
+		res := db.Exec("DELETE FROM targets")
+		if res.Error != nil {
+			return fmt.Errorf(
+				"purging target rows from webhook database %s: %w",
+				webhookID, res.Error,
+			)
+		}
+
+		purged = res.RowsAffected
+
+		// Unconditional: a zero row count here does not mean there is
+		// nothing to remove, only that no *live* row is left. See
+		// eventDBSweptVersion.
+		err = db.Exec("VACUUM").Error
+		if err != nil {
+			return fmt.Errorf(
+				"purged %d leaked target rows from webhook database "+
+					"%s but vacuuming it failed, so the deleted "+
+					"target credentials are still recoverable from "+
+					"the file; it stays marked unswept and the next "+
+					"open retries: %w",
+				purged, webhookID, err,
+			)
+		}
+	}
+
+	err = db.Exec(fmt.Sprintf(
+		"PRAGMA user_version = %d", eventDBSweptVersion,
+	)).Error
+	if err != nil {
+		return fmt.Errorf(
+			"marking webhook database %s swept: %w", webhookID, err,
+		)
+	}
+
+	if purged > 0 {
+		log.Warn(
+			"purged leaked target rows from per-webhook database",
+			"webhook_id", webhookID,
+			"rows", purged,
+		)
+	}
+
+	return nil
+}

internal/database/event_db_isolation_test.go

@@ -0,0 +1,438 @@
+package database_test
+
+import (
+	"bytes"
+	"database/sql"
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	_ "modernc.org/sqlite"
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// testDataDirPerm is the mode the test data directory is created
+// with.
+const testDataDirPerm = 0o750
+
+// eventDBDataDir returns a data directory that a WebhookDBManager
+// can be pointed at.
+func eventDBDataDir(t *testing.T) string {
+	t.Helper()
+
+	dir := filepath.Join(t.TempDir(), "events")
+	require.NoError(t, os.MkdirAll(dir, testDataDirPerm))
+
+	return dir
+}
+
+// openRawEventDB opens the per-webhook database file directly,
+// without the manager, so a test can put a file on disk in a state
+// the manager has to cope with, or inspect one afterwards.
+func openRawEventDB(
+	t *testing.T, dataDir, webhookID string,
+) *sql.DB {
+	t.Helper()
+
+	path := filepath.Join(
+		dataDir, fmt.Sprintf("events-%s.db", webhookID),
+	)
+
+	sqlDB, err := sql.Open(
+		"sqlite",
+		fmt.Sprintf("file:%s?mode=rwc", path),
+	)
+	require.NoError(t, err)
+
+	t.Cleanup(func() { _ = sqlDB.Close() })
+
+	return sqlDB
+}
+
+// eventDBFileBytes reads a per-webhook database file off disk, so a
+// test can assert on what the file itself still holds rather than on
+// what a query returns.
+func eventDBFileBytes(t *testing.T, dataDir, webhookID string) []byte {
+	t.Helper()
+
+	//nolint:gosec // reads a file the test just created under t.TempDir()
+	raw, err := os.ReadFile(filepath.Join(
+		dataDir, fmt.Sprintf("events-%s.db", webhookID),
+	))
+	require.NoError(t, err)
+
+	return raw
+}
+
+// eventDBUserVersion returns the PRAGMA user_version of a per-webhook
+// database file, which is the marker purgeTargetRows stamps once it
+// has swept and vacuumed.
+func eventDBUserVersion(t *testing.T, sqlDB *sql.DB) int {
+	t.Helper()
+
+	var version int
+
+	require.NoError(t, sqlDB.QueryRowContext(
+		t.Context(), "PRAGMA user_version",
+	).Scan(&version))
+
+	return version
+}
+
+// clearEventDBSweptMarker resets the sweep marker to 0, which is what
+// a file written by a build without the sweep looks like. Tests that
+// seed a leaked row have to create the file through the manager to
+// get the real targets table shape, and that stamps it.
+func clearEventDBSweptMarker(t *testing.T, sqlDB *sql.DB) {
+	t.Helper()
+
+	_, err := sqlDB.ExecContext(t.Context(), "PRAGMA user_version = 0")
+	require.NoError(t, err)
+}
+
+// countTargetRows returns the number of rows in the targets table of
+// a per-webhook database file, or -1 if the table does not exist.
+func countTargetRows(t *testing.T, sqlDB *sql.DB) int {
+	t.Helper()
+
+	var tables int
+
+	require.NoError(t, sqlDB.QueryRowContext(
+		t.Context(),
+		"SELECT count(*) FROM sqlite_master "+
+			"WHERE type = 'table' AND name = 'targets'",
+	).Scan(&tables))
+
+	if tables == 0 {
+		return -1
+	}
+
+	var rows int
+
+	require.NoError(t, sqlDB.QueryRowContext(
+		t.Context(), "SELECT count(*) FROM targets",
+	).Scan(&rows))
+
+	return rows
+}
+
+// TestOpenPurgesLeakedTargetRows covers the sweep for event
+// databases written by a build that let GORM upsert target rows
+// into them: opening the database clears them, and opening it again
+// is a no-op.
+func TestOpenPurgesLeakedTargetRows(t *testing.T) {
+	t.Parallel()
+
+	dataDir := eventDBDataDir(t)
+	webhookID := uuid.New().String()
+
+	// Create the file the way the application does, so the targets
+	// table has exactly the shape AutoMigrate gives it, then write
+	// a leaked row into it the way the association upsert did.
+	initial := database.NewTestWebhookDBManager(dataDir)
+
+	_, err := initial.GetDB(webhookID)
+	require.NoError(t, err)
+	require.NoError(t, initial.CloseAll())
+
+	seed := openRawEventDB(t, dataDir, webhookID)
+
+	_, err = seed.ExecContext(
+		t.Context(),
+		"INSERT INTO targets "+
+			"(id, webhook_id, name, type, config) "+
+			"VALUES (?, '', ?, ?, ?)",
+		uuid.New().String(),
+		"leaked-target",
+		"slack",
+		`{"webhookUrl":"https://hooks.example/T000/B000/secret"}`,
+	)
+	require.NoError(t, err)
+	require.Equal(t, 1, countTargetRows(t, seed))
+	clearEventDBSweptMarker(t, seed)
+	require.NoError(t, seed.Close())
+
+	mgr := database.NewTestWebhookDBManager(dataDir)
+
+	_, err = mgr.GetDB(webhookID)
+	require.NoError(t, err)
+	require.NoError(t, mgr.CloseAll())
+
+	check := openRawEventDB(t, dataDir, webhookID)
+	assert.Zero(t, countTargetRows(t, check))
+	assert.Equal(
+		t, 1, eventDBUserVersion(t, check),
+		"a completed sweep must mark the file so later opens skip it",
+	)
+	require.NoError(t, check.Close())
+
+	// Idempotent: a second open leaves it at zero and does not
+	// error.
+	again := database.NewTestWebhookDBManager(dataDir)
+
+	_, err = again.GetDB(webhookID)
+	require.NoError(t, err)
+	require.NoError(t, again.CloseAll())
+
+	recheck := openRawEventDB(t, dataDir, webhookID)
+	assert.Zero(t, countTargetRows(t, recheck))
+}
+
+// TestOpenPurgeRemovesCredentialBytes covers the sweep at the level
+// that matters for a backup handed to someone else: the leaked
+// credential must be gone from the raw bytes of the file, not merely
+// unreachable by query. A bare DELETE unlinks the row and leaves the
+// bytes readable in the free pages, so this fails without the VACUUM
+// in purgeTargetRows.
+func TestOpenPurgeRemovesCredentialBytes(t *testing.T) {
+	t.Parallel()
+
+	dataDir := eventDBDataDir(t)
+	webhookID := uuid.New().String()
+	credential := "T00000000/B00000000/" + uuid.New().String()
+
+	initial := database.NewTestWebhookDBManager(dataDir)
+
+	_, err := initial.GetDB(webhookID)
+	require.NoError(t, err)
+	require.NoError(t, initial.CloseAll())
+
+	seed := openRawEventDB(t, dataDir, webhookID)
+
+	_, err = seed.ExecContext(
+		t.Context(),
+		"INSERT INTO targets "+
+			"(id, webhook_id, name, type, config) "+
+			"VALUES (?, '', ?, ?, ?)",
+		uuid.New().String(),
+		"leaked-target",
+		"slack",
+		fmt.Sprintf(
+			`{"webhookUrl":"https://hooks.example/%s"}`, credential,
+		),
+	)
+	require.NoError(t, err)
+	clearEventDBSweptMarker(t, seed)
+	require.NoError(t, seed.Close())
+
+	// The seed has to be in the file for its absence later to mean
+	// anything.
+	require.True(
+		t,
+		bytes.Contains(
+			eventDBFileBytes(t, dataDir, webhookID),
+			[]byte(credential),
+		),
+		"seeded credential is not in the file, so this test proves nothing",
+	)
+
+	mgr := database.NewTestWebhookDBManager(dataDir)
+
+	_, err = mgr.GetDB(webhookID)
+	require.NoError(t, err)
+	require.NoError(t, mgr.CloseAll())
+
+	assert.NotContains(
+		t,
+		string(eventDBFileBytes(t, dataDir, webhookID)),
+		credential,
+		"leaked credential is still recoverable from the raw file",
+	)
+}
+
+// TestOpenRevacuumsAfterIncompleteSweep covers the case a row count
+// cannot see: the rows are already deleted but the file was never
+// vacuumed, because an earlier sweep died between the two or its
+// VACUUM failed. The credential bytes are still recoverable, and the
+// unset marker is the only thing that says so, so the next open must
+// vacuum rather than conclude from the empty table that there is
+// nothing to do.
+func TestOpenRevacuumsAfterIncompleteSweep(t *testing.T) {
+	t.Parallel()
+
+	dataDir := eventDBDataDir(t)
+	webhookID := uuid.New().String()
+	credential := "T00000000/B00000000/" + uuid.New().String()
+
+	initial := database.NewTestWebhookDBManager(dataDir)
+
+	_, err := initial.GetDB(webhookID)
+	require.NoError(t, err)
+	require.NoError(t, initial.CloseAll())
+
+	seed := openRawEventDB(t, dataDir, webhookID)
+
+	_, err = seed.ExecContext(
+		t.Context(),
+		"INSERT INTO targets "+
+			"(id, webhook_id, name, type, config) "+
+			"VALUES (?, '', ?, ?, ?)",
+		uuid.New().String(),
+		"leaked-target",
+		"slack",
+		fmt.Sprintf(
+			`{"webhookUrl":"https://hooks.example/%s"}`, credential,
+		),
+	)
+	require.NoError(t, err)
+
+	// Exactly the state an interrupted sweep leaves: rows gone,
+	// marker unset, bytes still in the free pages.
+	_, err = seed.ExecContext(t.Context(), "DELETE FROM targets")
+	require.NoError(t, err)
+	require.Zero(t, countTargetRows(t, seed))
+	clearEventDBSweptMarker(t, seed)
+	require.NoError(t, seed.Close())
+
+	require.True(
+		t,
+		bytes.Contains(
+			eventDBFileBytes(t, dataDir, webhookID),
+			[]byte(credential),
+		),
+		"the deleted row's bytes must still be in the file, or this "+
+			"test proves nothing",
+	)
+
+	mgr := database.NewTestWebhookDBManager(dataDir)
+
+	_, err = mgr.GetDB(webhookID)
+	require.NoError(t, err)
+	require.NoError(t, mgr.CloseAll())
+
+	assert.NotContains(
+		t,
+		string(eventDBFileBytes(t, dataDir, webhookID)),
+		credential,
+		"an interrupted sweep was not retried, so the credential is "+
+			"still recoverable from the raw file",
+	)
+
+	check := openRawEventDB(t, dataDir, webhookID)
+	assert.Equal(t, 1, eventDBUserVersion(t, check))
+}
+
+// TestOpenSkipsSweptDatabase covers the other half of the marker: a
+// file this build created is marked without ever being vacuumed, and
+// a marked file is not swept again.
+func TestOpenSkipsSweptDatabase(t *testing.T) {
+	t.Parallel()
+
+	dataDir := eventDBDataDir(t)
+	webhookID := uuid.New().String()
+
+	mgr := database.NewTestWebhookDBManager(dataDir)
+
+	_, err := mgr.GetDB(webhookID)
+	require.NoError(t, err)
+	require.NoError(t, mgr.CloseAll())
+
+	marked := openRawEventDB(t, dataDir, webhookID)
+	assert.Equal(t, 1, eventDBUserVersion(t, marked))
+
+	// A marked file is left alone, so a row written into it survives
+	// a reopen. Nothing writes target rows any more; this stands in
+	// for the sweep having run.
+	_, err = marked.ExecContext(
+		t.Context(),
+		"INSERT INTO targets "+
+			"(id, webhook_id, name, type, config) "+
+			"VALUES (?, '', ?, ?, ?)",
+		uuid.New().String(), "sentinel", "slack", `{}`,
+	)
+	require.NoError(t, err)
+	require.NoError(t, marked.Close())
+
+	again := database.NewTestWebhookDBManager(dataDir)
+
+	_, err = again.GetDB(webhookID)
+	require.NoError(t, err)
+	require.NoError(t, again.CloseAll())
+
+	check := openRawEventDB(t, dataDir, webhookID)
+	assert.Equal(
+		t, 1, countTargetRows(t, check),
+		"a marked file must not be swept again",
+	)
+}
+
+// TestOpenSucceedsWithoutTargetsTable covers an existing event
+// database that never grew a targets table. The sweep must not fail
+// startup on it.
+func TestOpenSucceedsWithoutTargetsTable(t *testing.T) {
+	t.Parallel()
+
+	dataDir := eventDBDataDir(t)
+	webhookID := uuid.New().String()
+
+	seed := openRawEventDB(t, dataDir, webhookID)
+
+	_, err := seed.ExecContext(
+		t.Context(),
+		"CREATE TABLE events (id text PRIMARY KEY)",
+	)
+	require.NoError(t, err)
+	require.NoError(t, seed.Close())
+
+	mgr := database.NewTestWebhookDBManager(dataDir)
+
+	db, err := mgr.GetDB(webhookID)
+	require.NoError(t, err)
+	assert.NotNil(t, db)
+	require.NoError(t, mgr.CloseAll())
+}
+
+// TestEventDBCreateOmitsAssociations covers the connection-level
+// guard directly: a Delivery carrying its Event and Target in
+// memory, written through the manager's handle, must store only the
+// delivery row.
+func TestEventDBCreateOmitsAssociations(t *testing.T) {
+	t.Parallel()
+
+	dataDir := eventDBDataDir(t)
+	webhookID := uuid.New().String()
+
+	mgr := database.NewTestWebhookDBManager(dataDir)
+
+	db, err := mgr.GetDB(webhookID)
+	require.NoError(t, err)
+
+	target := database.Target{
+		WebhookID: webhookID,
+		Name:      "leaky-target",
+		Type:      database.TargetTypeSlack,
+		Config:    `{"webhookUrl":"https://hooks.example/secret"}`,
+	}
+	target.ID = uuid.New().String()
+
+	event := database.Event{
+		WebhookID:    webhookID,
+		EntrypointID: uuid.New().String(),
+		Method:       "POST",
+		Headers:      `{}`,
+		Body:         `{}`,
+	}
+	event.ID = uuid.New().String()
+
+	d := &database.Delivery{
+		EventID:  event.ID,
+		TargetID: target.ID,
+		Status:   database.DeliveryStatusPending,
+		Event:    event,
+		Target:   target,
+	}
+	d.ID = uuid.New().String()
+
+	require.NoError(t, db.Create(d).Error)
+	require.NoError(t, db.Model(d).
+		Update("status", database.DeliveryStatusDelivered).
+		Error)
+	require.NoError(t, mgr.CloseAll())
+
+	check := openRawEventDB(t, dataDir, webhookID)
+	assert.Zero(t, countTargetRows(t, check))
+}

internal/database/webhook_db_manager.go

@@ -262,6 +262,25 @@ func (m *WebhookDBManager) openDB(
 		)
 	}
 
+	// Keep main-database rows out of this file. See
+	// event_db_isolation.go.
+	err = omitAssociations(db)
+	if err != nil {
+		_ = sqlDB.Close()
+
+		return nil, fmt.Errorf(
+			"guarding webhook database %s: %w",
+			webhookID, err,
+		)
+	}
+
+	err = purgeTargetRows(db, m.log, webhookID)
+	if err != nil {
+		_ = sqlDB.Close()
+
+		return nil, err
+	}
+
 	// Run migrations for event-tier models only
 	err = db.AutoMigrate(
 		&Event{}, &Delivery{}, &DeliveryResult{},

internal/datadir/lock.go

@@ -0,0 +1,126 @@
+// Package datadir guards exclusive access to the directory holding
+// every SQLite database webhooker writes.
+//
+// Two processes sharing a DATA_DIR each open the same per-webhook
+// event databases and each run delivery recovery over the same rows,
+// so every pending delivery goes out twice. SQLite's own locking does
+// not prevent that: both writers are serialised correctly and both
+// deliver. The only thing that prevents it is refusing to be the
+// second process.
+//
+// The lock lives here rather than in the server's fx graph so that any
+// entry point which touches DATA_DIR — the server, or a CLI
+// subcommand that must not operate on a live deployment's data — takes
+// it the same way.
+package datadir
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"github.com/gofrs/flock"
+)
+
+// LockFileName is the advisory lock file created inside DATA_DIR. Its
+// contents are never read: the lock is the flock(2) held on the open
+// descriptor, not the file's existence, so a leftover file from a
+// process that was killed with SIGKILL blocks nothing.
+const LockFileName = "webhooker.lock"
+
+// dirPerm is the mode Acquire creates DATA_DIR with. It matches what
+// internal/database uses, since whichever runs first creates it.
+const dirPerm = 0o750
+
+// ErrLocked reports that another live process holds the data
+// directory. Callers that need to know whether a deployment is running
+// — rather than merely failing to start — test for this with
+// errors.Is.
+var ErrLocked = errors.New(
+	"data directory is already in use by another instance",
+)
+
+// ErrNoDir reports that Acquire was given an empty directory.
+var ErrNoDir = errors.New("no data directory given")
+
+// Lock is a held exclusive advisory lock on a data directory. It is
+// valid only while the process that took it lives: the kernel drops it
+// when the descriptor closes, whether that is Release, a normal exit,
+// or a SIGKILL.
+type Lock struct {
+	dir  string
+	file *flock.Flock
+}
+
+// Acquire takes the exclusive advisory lock on dir, creating dir if it
+// does not exist. It never waits: if another process holds the lock it
+// returns an error wrapping ErrLocked and naming dir.
+//
+// The returned Lock must be held for as long as the caller intends to
+// use dir.
+func Acquire(dir string) (*Lock, error) {
+	if dir == "" {
+		return nil, ErrNoDir
+	}
+
+	err := os.MkdirAll(dir, dirPerm)
+	if err != nil {
+		return nil, fmt.Errorf(
+			"creating data directory %s: %w", dir, err,
+		)
+	}
+
+	path := filepath.Join(dir, LockFileName)
+	fl := flock.New(path)
+
+	held, err := fl.TryLock()
+	if err != nil {
+		return nil, fmt.Errorf(
+			"locking data directory %s: %w", dir, err,
+		)
+	}
+
+	if !held {
+		// A no-op on flock v0.13.0, which closes its own descriptor on
+		// a failed TryLock; kept so no version can leak one.
+		_ = fl.Close()
+
+		return nil, fmt.Errorf(
+			"%w: %s (%s). Only one webhooker may use a data "+
+				"directory: two both run delivery recovery over the "+
+				"same rows and both deliver",
+			ErrLocked, dir, path,
+		)
+	}
+
+	return &Lock{dir: dir, file: fl}, nil
+}
+
+// Dir returns the locked directory.
+func (l *Lock) Dir() string {
+	return l.dir
+}
+
+// Path returns the lock file backing the lock.
+func (l *Lock) Path() string {
+	return l.file.Path()
+}
+
+// Release drops the lock and closes the descriptor. It is safe to call
+// more than once.
+//
+// The lock file is deliberately left on disk. Unlinking it would let
+// the next process create and lock a fresh inode while a third still
+// holds the old one, which is the one outcome this package exists to
+// prevent.
+func (l *Lock) Release() error {
+	err := l.file.Unlock()
+	if err != nil {
+		return fmt.Errorf(
+			"releasing lock on data directory %s: %w", l.dir, err,
+		)
+	}
+
+	return nil
+}

internal/datadir/lock_test.go

@@ -0,0 +1,250 @@
+package datadir_test
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/datadir"
+)
+
+// holderEnv names the directory the re-executed test binary should
+// lock and hold. When it is unset the child test does nothing, so an
+// ordinary run is unaffected.
+const holderEnv = "WEBHOOKER_DATADIR_LOCK_HOLDER"
+
+// holderReadyPrefix labels the child's one-line report that it holds
+// the lock, so the parent can find it among the testing package's own
+// output on the same descriptor.
+const holderReadyPrefix = "DATADIR-LOCK-HELD "
+
+// holderReadyTimeout bounds the wait for the child to take the lock.
+// It only has to cover process start on a loaded shared host.
+const holderReadyTimeout = 60 * time.Second
+
+// holderHold is how long the child keeps the lock if nothing kills it.
+// A sleep rather than a bare block, so the runtime's deadlock detector
+// has a pending timer and the child cannot outlive a killed test run
+// by more than this.
+const holderHold = 10 * time.Minute
+
+// TestLockHolder is the child half of the two-process tests below. It
+// takes the lock on the directory named by holderEnv, reports the lock
+// file on standard output, and then holds it until it is killed.
+func TestLockHolder(t *testing.T) {
+	t.Parallel()
+
+	dir := os.Getenv(holderEnv)
+	if dir == "" {
+		return
+	}
+
+	lock, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+
+	// Written to the descriptor directly: the parent reads fd 1, not
+	// the testing package's buffered report.
+	_, err = fmt.Fprintf(
+		os.Stdout, "%s%s\n", holderReadyPrefix, lock.Path(),
+	)
+	require.NoError(t, err)
+
+	time.Sleep(holderHold)
+}
+
+// startHolder re-executes this test binary as a separate process that
+// takes and holds the lock on dir, and returns once that process
+// actually holds it. The child is killed when the test ends.
+func startHolder(t *testing.T, dir string) *exec.Cmd {
+	t.Helper()
+
+	//nolint:gosec // Re-executing this test binary, with a fixed arg.
+	cmd := exec.CommandContext(
+		t.Context(), os.Args[0], "-test.run", "^TestLockHolder$",
+	)
+
+	cmd.Env = append(os.Environ(), holderEnv+"="+dir)
+	cmd.Stderr = os.Stderr
+
+	stdout, err := cmd.StdoutPipe()
+	require.NoError(t, err)
+	require.NoError(t, cmd.Start())
+
+	t.Cleanup(func() {
+		_ = cmd.Process.Kill()
+		_ = cmd.Wait()
+	})
+
+	ready := make(chan string, 1)
+
+	go func() {
+		scanner := bufio.NewScanner(stdout)
+		for scanner.Scan() {
+			after, found := strings.CutPrefix(
+				scanner.Text(), holderReadyPrefix,
+			)
+			if found {
+				ready <- after
+
+				break
+			}
+		}
+
+		close(ready)
+
+		// Keep draining so the child never blocks on a full pipe.
+		_, _ = io.Copy(io.Discard, stdout)
+	}()
+
+	select {
+	case path, ok := <-ready:
+		require.True(
+			t, ok, "holder exited without taking the lock",
+		)
+		require.Equal(t, filepath.Join(dir, datadir.LockFileName), path)
+	case <-time.After(holderReadyTimeout):
+		t.Fatal("timed out waiting for the holder to take the lock")
+	}
+
+	return cmd
+}
+
+// TestSecondInstanceRefused is the regression test for the duplicate
+// delivery this package exists to prevent: a real second process
+// pointed at a data directory a live process already holds must be
+// refused, with an error that names the directory.
+func TestSecondInstanceRefused(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+
+	startHolder(t, dir)
+
+	lock, err := datadir.Acquire(dir)
+
+	require.Error(t, err, "the second instance took the lock too")
+	require.Nil(t, lock)
+	require.ErrorIs(
+		t, err, datadir.ErrLocked,
+		"the refusal must be distinguishable from any other failure",
+	)
+	assert.Contains(
+		t, err.Error(), dir,
+		"the refusal must name the directory it is about",
+	)
+}
+
+// TestRestartAfterHardKill is the other half of the regression: a
+// process killed with SIGKILL runs no cleanup and leaves its lock file
+// behind, and the next start must not be blocked by it. This is what a
+// pidfile would get wrong; the kernel drops a flock when the
+// descriptor closes, however the process died.
+func TestRestartAfterHardKill(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+
+	holder := startHolder(t, dir)
+
+	require.NoError(t, holder.Process.Kill())
+
+	// Wait for the kill to have actually happened. Re-acquiring while
+	// the corpse still holds a descriptor would be a race, and would
+	// make this test pass or fail on scheduling.
+	_ = holder.Wait()
+
+	require.FileExists(
+		t, filepath.Join(dir, datadir.LockFileName),
+		"the stale lock file is what must not block the restart",
+	)
+
+	lock, err := datadir.Acquire(dir)
+	require.NoError(
+		t, err, "a hard-killed instance must not block the next start",
+	)
+
+	require.NoError(t, lock.Release())
+}
+
+// TestSecondFdInSameProcessRefused pins the flock(2) property the
+// tests in cmd/webhooker rely on: descriptors are locked
+// independently, so a second acquisition is denied even when it comes
+// from the process that already holds the lock.
+func TestSecondFdInSameProcessRefused(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+
+	first, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+
+	defer func() { _ = first.Release() }()
+
+	_, err = datadir.Acquire(dir)
+	require.ErrorIs(t, err, datadir.ErrLocked)
+}
+
+// TestReleaseAllowsReacquire covers the clean-shutdown path: the lock
+// is released on exit, so a restart is not blocked by the previous
+// run.
+func TestReleaseAllowsReacquire(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+
+	first, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+	require.NoError(t, first.Release())
+
+	second, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+	require.NoError(t, second.Release())
+}
+
+// TestAcquireCreatesDataDir covers a first start against a DATA_DIR
+// that does not exist yet, which is the normal case for a fresh
+// deployment: the lock is taken before anything else creates it.
+func TestAcquireCreatesDataDir(t *testing.T) {
+	t.Parallel()
+
+	dir := filepath.Join(t.TempDir(), "nested", "data")
+
+	lock, err := datadir.Acquire(dir)
+	require.NoError(t, err)
+
+	defer func() { _ = lock.Release() }()
+
+	assert.Equal(t, dir, lock.Dir())
+	assert.FileExists(t, filepath.Join(dir, datadir.LockFileName))
+}
+
+// TestAcquireEmptyDir rejects an empty directory rather than locking
+// the process's working directory.
+func TestAcquireEmptyDir(t *testing.T) {
+	t.Parallel()
+
+	_, err := datadir.Acquire("")
+	require.ErrorIs(t, err, datadir.ErrNoDir)
+}
+
+// TestAcquireUnusableDir reports an unusable DATA_DIR clearly, naming
+// it, instead of failing later and deeper.
+func TestAcquireUnusableDir(t *testing.T) {
+	t.Parallel()
+
+	file := filepath.Join(t.TempDir(), "not-a-directory")
+	require.NoError(t, os.WriteFile(file, nil, 0o600))
+
+	_, err := datadir.Acquire(file)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), file)
+}

internal/delivery/engine.go

@@ -15,6 +15,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/lifecycle"
 	"sneak.berlin/go/webhooker/internal/logger"
+	"sneak.berlin/go/webhooker/internal/metrics"
 )
 
 const (
@@ -139,6 +140,12 @@ type Engine struct {
 	retryCh    chan Task
 	workers    int
 
+	// mtr is the delivery metric set. Production wires the
+	// process-wide one; a test can substitute a set registered on
+	// a private registry so its assertions are not disturbed by
+	// deliveries other tests are making at the same time.
+	mtr *metrics.Set
+
 	// targets maps each target type to its implementation.
 	targets map[database.TargetType]Target
 
@@ -164,6 +171,7 @@ func New(
 		deliveryCh: make(chan Task, deliveryChannelSize),
 		retryCh:    make(chan Task, retryChannelSize),
 		workers:    defaultWorkers,
+		mtr:        metrics.Default(),
 	}
 
 	e.initTargets(&http.Client{
@@ -283,6 +291,10 @@ func (e *Engine) start() {
 
 	go e.retrySweep(ctx)
 
+	e.wg.Add(1)
+
+	go e.queueDepthSampler(ctx)
+
 	e.log.Info(
 		"delivery engine started",
 		"workers", e.workers,
@@ -837,8 +849,15 @@ func (e *Engine) failUnretryableRetry(
 		0,
 	)
 
+	// The type is passed rather than assigned onto d: the delivery
+	// is loaded here without its target relation, and populating
+	// d.Target would make GORM's SaveBeforeAssociations upsert the
+	// whole target row — plaintext config, which for a slack target
+	// is the credential — into the per-webhook event database. See
+	// https://git.eeqj.de/sneak/webhooker/issues/206.
 	e.updateDeliveryStatus(
-		webhookDB, d, database.DeliveryStatusFailed,
+		webhookDB, d, target.Type,
+		database.DeliveryStatusFailed,
 	)
 }
 
@@ -859,7 +878,8 @@ func (e *Engine) processDelivery(
 		)
 
 		e.updateDeliveryStatus(
-			webhookDB, d, database.DeliveryStatusFailed,
+			webhookDB, d, d.Target.Type,
+			database.DeliveryStatusFailed,
 		)
 
 		return
@@ -868,6 +888,24 @@ func (e *Engine) processDelivery(
 	target.Deliver(ctx, webhookDB, d, task, e)
 }
 
+// observeAttempt counts one delivery attempt that was actually
+// dispatched to a target, and records how long it took.
+//
+// It is called from the dispatch paths rather than from around
+// Target.Deliver, because Deliver is also entered for deliveries
+// that never reach the wire: a delivery an open circuit breaker
+// refuses sends nothing, records no DeliveryResult, and is
+// rescheduled. Counting those would climb the attempts counter with
+// no traffic behind it and fill the duration histogram with
+// microsecond samples, which would make the delivery-duration
+// quantiles improve during exactly the outage they exist to reveal.
+func (e *Engine) observeAttempt(
+	t database.TargetType, dur time.Duration,
+) {
+	e.mtr.DeliveryAttempted(t)
+	e.mtr.ObserveDeliveryDuration(t, dur)
+}
+
 // recordResult persists a DeliveryResult row describing a
 // single attempt. It is a cross-target helper the targets
 // call.
@@ -901,10 +939,22 @@ func (e *Engine) recordResult(
 }
 
 // updateDeliveryStatus persists a new status for a delivery.
-// It is a cross-target helper the targets call.
+// It is a cross-target helper the targets call, and therefore the
+// single point where a delivery's outcome — delivered, terminally
+// failed, or put back into retry — is counted.
+//
+// The target type is a parameter rather than read off d.Target
+// because one caller — failUnretryableRetry — deliberately holds a
+// delivery loaded without its target relation, and must keep it that
+// way: a populated d.Target makes GORM upsert the target row, config
+// and all, into the per-webhook database.
+//
+// The counter moves only after the row is written, so a transition
+// the database rejected is not claimed as an outcome that happened.
 func (e *Engine) updateDeliveryStatus(
 	webhookDB *gorm.DB,
 	d *database.Delivery,
+	targetType database.TargetType,
 	status database.DeliveryStatus,
 ) {
 	err := webhookDB.Model(d).
@@ -916,7 +966,11 @@ func (e *Engine) updateDeliveryStatus(
 			"status", status,
 			"error", err,
 		)
+
+		return
 	}
+
+	e.mtr.DeliveryStatusChanged(targetType, status)
 }
 
 func truncate(s string, maxLen int) string {

internal/delivery/engine_integration_test.go

@@ -886,6 +886,82 @@ func TestSweepSingleRetry_TypeNoLongerRetries(
 	)
 }
 
+// TestFailUnretryableRetry_WritesNoTargetRow proves the
+// orphaned-retry terminal path leaves no target row — and so no
+// plaintext target config — in the per-webhook event database.
+//
+// That path loads the delivery without its Target relation on
+// purpose. Populating d.Target makes GORM's SaveBeforeAssociations
+// upsert the whole target row on the status UPDATE, which for a slack
+// target writes the incoming-webhook credential into events-*.db.
+// See https://git.eeqj.de/sneak/webhooker/issues/206.
+func TestFailUnretryableRetry_WritesNoTargetRow(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	s := newISetup(t)
+
+	iCreateWebhook(
+		t, s.MainDB, s.WebhookID, "no-target-row",
+	)
+
+	targetID := uuid.New().String()
+
+	// A Slack incoming-webhook URL: the target config IS the
+	// credential, which is what makes a leaked target row a
+	// disclosure rather than a curiosity.
+	hookURL := "https://hooks.slack.com/services/T00/B00/x"
+
+	iCreateTarget(t, s.MainDB, targetID,
+		s.WebhookID, "credential-bearing",
+		database.TargetTypeLog, iHTTPConfig(hookURL), 5,
+	)
+
+	event := iSeedEvent(
+		t, s.WebhookDB, s.WebhookID, `{"orphaned":"retry"}`,
+	)
+
+	d := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusRetrying,
+	)
+
+	iSeedFailedResult(t, s.WebhookDB, d.ID)
+
+	s.Engine.ExportSweepWebhookRetries(
+		context.Background(), s.WebhookID,
+	)
+
+	iAssertStatus(t, s.WebhookDB, d.ID,
+		database.DeliveryStatusFailed,
+	)
+
+	// The table exists in the per-webhook database because GORM
+	// migrates the Delivery relation's model alongside it. It must
+	// stay empty.
+	var targetRows int64
+
+	require.NoError(t, s.WebhookDB.
+		Table("targets").
+		Count(&targetRows).Error)
+
+	assert.Zero(t, targetRows,
+		"orphaned-retry terminal failure wrote a target row "+
+			"into the per-webhook event database",
+	)
+
+	var configs []string
+
+	require.NoError(t, s.WebhookDB.
+		Table("targets").
+		Pluck("config", &configs).Error)
+
+	assert.NotContains(
+		t, strings.Join(configs, " "), hookURL,
+	)
+}
+
 func TestRecoverSingleRetry_UnknownTargetType(
 	t *testing.T,
 ) {

internal/delivery/event_db_isolation_test.go

@@ -0,0 +1,157 @@
+package delivery_test
+
+import (
+	"context"
+	"database/sql"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	_ "modernc.org/sqlite"
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// assertNoTargetRows opens the per-webhook database file directly,
+// outside GORM, and fails if its targets table holds any rows.
+// Target config is the credential for slack and http targets, and
+// event databases are the files that get backed up and handed
+// around.
+func assertNoTargetRows(t *testing.T, dbPath string) {
+	t.Helper()
+
+	sqlDB, err := sql.Open(
+		"sqlite", fmt.Sprintf("file:%s?mode=ro", dbPath),
+	)
+	require.NoError(t, err)
+
+	defer func() { _ = sqlDB.Close() }()
+
+	var tables int
+
+	require.NoError(t, sqlDB.QueryRowContext(
+		t.Context(),
+		"SELECT count(*) FROM sqlite_master "+
+			"WHERE type = 'table' AND name = 'targets'",
+	).Scan(&tables))
+
+	if tables == 0 {
+		return
+	}
+
+	var rows int
+
+	require.NoError(t, sqlDB.QueryRowContext(
+		t.Context(), "SELECT count(*) FROM targets",
+	).Scan(&rows))
+
+	assert.Zero(
+		t, rows,
+		"per-webhook event database must hold no target rows",
+	)
+}
+
+// TestEventDBHoldsNoTargetRows drives a delivery and then a retry
+// through the real engine write paths and asserts neither leaves a
+// target row behind in events-*.db.
+func TestEventDBHoldsNoTargetRows(t *testing.T) {
+	t.Parallel()
+
+	s := newISetup(t)
+
+	ts := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		},
+	))
+	defer ts.Close()
+
+	cfg := iHTTPConfig(ts.URL)
+	targetID := uuid.New().String()
+	dbPath := s.DBMgr.DBPath(s.WebhookID)
+
+	event := iSeedEvent(
+		t, s.WebhookDB, s.WebhookID, `{"leak":"none"}`,
+	)
+	body := event.Body
+
+	// A new delivery.
+	d := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusPending,
+	)
+	task := iTask(
+		d, event, s.WebhookID, targetID,
+		"leaky-target", cfg, 5, 1, &body,
+	)
+
+	s.Engine.ExportProcessNewTask(context.TODO(), &task)
+
+	iAssertStatus(
+		t, s.WebhookDB, d.ID,
+		database.DeliveryStatusDelivered,
+	)
+	assertNoTargetRows(t, dbPath)
+
+	// A retry.
+	rd := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusRetrying,
+	)
+	rTask := iTask(
+		rd, event, s.WebhookID, targetID,
+		"leaky-target", cfg, 5, 2, &body,
+	)
+
+	s.Engine.ExportProcessRetryTask(context.TODO(), &rTask)
+
+	iAssertStatus(
+		t, s.WebhookDB, rd.ID,
+		database.DeliveryStatusDelivered,
+	)
+	assertNoTargetRows(t, dbPath)
+}
+
+// TestEventDBHoldsNoTargetRowsOnFailedDelivery covers the failure
+// write path, which updates the delivery to failed and records a
+// result, rather than the success path above.
+func TestEventDBHoldsNoTargetRowsOnFailedDelivery(t *testing.T) {
+	t.Parallel()
+
+	s := newISetup(t)
+
+	ts := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusInternalServerError)
+		},
+	))
+	defer ts.Close()
+
+	cfg := iHTTPConfig(ts.URL)
+	targetID := uuid.New().String()
+
+	event := iSeedEvent(
+		t, s.WebhookDB, s.WebhookID, `{"leak":"none"}`,
+	)
+	body := event.Body
+
+	d := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusPending,
+	)
+	task := iTask(
+		d, event, s.WebhookID, targetID,
+		"leaky-target", cfg, 0, 1, &body,
+	)
+
+	s.Engine.ExportProcessNewTask(context.TODO(), &task)
+
+	iAssertStatus(
+		t, s.WebhookDB, d.ID,
+		database.DeliveryStatusFailed,
+	)
+	assertNoTargetRows(t, s.DBMgr.DBPath(s.WebhookID))
+}

internal/delivery/export_test.go

@@ -10,6 +10,7 @@ import (
 	"go.uber.org/fx"
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/metrics"
 )
 
 // ErrExportArchiveWriterEvicted exposes the sentinel returned by
@@ -253,6 +254,7 @@ func NewTestEngine(
 		deliveryCh: make(chan Task, deliveryChannelSize),
 		retryCh:    make(chan Task, retryChannelSize),
 		workers:    workers,
+		mtr:        metrics.Default(),
 	}
 	e.initTargets(client)
 
@@ -267,6 +269,7 @@ func NewTestEngineSmallRetry(
 	e := &Engine{
 		log:     log,
 		retryCh: make(chan Task, 1),
+		mtr:     metrics.Default(),
 	}
 	e.initTargets(nil)
 
@@ -289,12 +292,25 @@ func NewTestEngineWithDB(
 		deliveryCh: make(chan Task, deliveryChannelSize),
 		retryCh:    make(chan Task, retryChannelSize),
 		workers:    workers,
+		mtr:        metrics.Default(),
 	}
 	e.initTargets(client)
 
 	return e
 }
 
+// ExportSetMetrics substitutes the engine's metric set, so a test can
+// assert on collectors registered on a private registry instead of
+// the process-wide ones every other test is also moving.
+func (e *Engine) ExportSetMetrics(mtr *metrics.Set) {
+	e.mtr = mtr
+}
+
+// ExportSampleQueueDepths runs one queue depth sample synchronously.
+func (e *Engine) ExportSampleQueueDepths(ctx context.Context) {
+	e.sampleQueueDepths(ctx)
+}
+
 // NewTestCircuitBreaker creates a CircuitBreaker with
 // custom settings for testing.
 func NewTestCircuitBreaker(

internal/delivery/metrics_test.go

@@ -0,0 +1,545 @@
+package delivery_test
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/prometheus/client_golang/prometheus"
+	dto "github.com/prometheus/client_model/go"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/delivery"
+	"sneak.berlin/go/webhooker/internal/metrics"
+)
+
+// Metric names as exposed on /metrics.
+const (
+	mAttempts  = "webhooker_delivery_attempts_total"
+	mSucceeded = "webhooker_deliveries_succeeded_total"
+	mFailed    = "webhooker_deliveries_failed_total"
+	mRetries   = "webhooker_delivery_retries_total"
+	mDuration  = "webhooker_delivery_duration_seconds"
+	mPending   = "webhooker_deliveries_pending"
+	mRetrying  = "webhooker_deliveries_retrying"
+	mBreakers  = "webhooker_circuit_breakers_open"
+)
+
+const (
+	mTypeHTTP    = "http"
+	mTypeLog     = "log"
+	mTypeUnknown = "unknown"
+)
+
+// mIsolate gives the setup's engine a metric set registered on a
+// private registry. The process-wide collectors are moved by every
+// other delivery test running in parallel, so exact assertions are
+// only possible against a registry this test owns.
+func mIsolate(
+	t *testing.T, s iSetup,
+) *prometheus.Registry {
+	t.Helper()
+
+	reg := prometheus.NewRegistry()
+	s.Engine.ExportSetMetrics(metrics.New(reg))
+
+	return reg
+}
+
+// mFind returns the series of the named metric carrying the given
+// target_type label.
+func mFind(
+	t *testing.T,
+	reg *prometheus.Registry,
+	name, targetType string,
+) *dto.Metric {
+	t.Helper()
+
+	families, err := reg.Gather()
+	require.NoError(t, err)
+
+	for _, fam := range families {
+		if fam.GetName() != name {
+			continue
+		}
+
+		for _, m := range fam.GetMetric() {
+			for _, label := range m.GetLabel() {
+				if label.GetName() == "target_type" &&
+					label.GetValue() == targetType {
+					return m
+				}
+			}
+		}
+	}
+
+	t.Fatalf(
+		"metric %s{target_type=%q} not found",
+		name, targetType,
+	)
+
+	return nil
+}
+
+func mCounter(
+	t *testing.T,
+	reg *prometheus.Registry,
+	name, targetType string,
+) float64 {
+	t.Helper()
+
+	return mFind(t, reg, name, targetType).
+		GetCounter().GetValue()
+}
+
+func mGauge(
+	t *testing.T,
+	reg *prometheus.Registry,
+	name, targetType string,
+) float64 {
+	t.Helper()
+
+	return mFind(t, reg, name, targetType).
+		GetGauge().GetValue()
+}
+
+// mHTTPDurations returns how many samples the delivery duration
+// histogram holds for the http target type, which is the type every
+// test here times.
+func mHTTPDurations(
+	t *testing.T, reg *prometheus.Registry,
+) uint64 {
+	t.Helper()
+
+	return mFind(t, reg, mDuration, mTypeHTTP).
+		GetHistogram().GetSampleCount()
+}
+
+// TestDeliveryMetrics_SuccessAndRetryExhaustion drives one delivery
+// that succeeds and one that fails every attempt until its retries
+// are exhausted, and asserts every delivery counter across both.
+func TestDeliveryMetrics_SuccessAndRetryExhaustion(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	s := newISetup(t)
+	reg := mIsolate(t, s)
+
+	mDeliverOK(t, s)
+
+	assert.InDelta(t, 1.0,
+		mCounter(t, reg, mAttempts, mTypeHTTP), 0)
+	assert.InDelta(t, 1.0,
+		mCounter(t, reg, mSucceeded, mTypeHTTP), 0)
+	assert.InDelta(t, 0.0,
+		mCounter(t, reg, mFailed, mTypeHTTP), 0)
+	assert.InDelta(t, 0.0,
+		mCounter(t, reg, mRetries, mTypeHTTP), 0)
+	assert.Equal(t, uint64(1),
+		mHTTPDurations(t, reg))
+
+	mExhaustRetries(t, s)
+
+	// Two further attempts: the first is retried, the second is
+	// the last one allowed and fails the delivery terminally.
+	assert.InDelta(t, 3.0,
+		mCounter(t, reg, mAttempts, mTypeHTTP), 0)
+	assert.InDelta(t, 1.0,
+		mCounter(t, reg, mSucceeded, mTypeHTTP), 0)
+	assert.InDelta(t, 1.0,
+		mCounter(t, reg, mRetries, mTypeHTTP), 0)
+	assert.InDelta(t, 1.0,
+		mCounter(t, reg, mFailed, mTypeHTTP), 0)
+	assert.Equal(t, uint64(3),
+		mHTTPDurations(t, reg))
+
+	// Two consecutive failures are below the trip threshold.
+	assert.InDelta(t, 0.0,
+		mGauge(t, reg, mBreakers, mTypeHTTP), 0)
+
+	// The label is the target type and nothing finer: two http
+	// targets shared one series, and no other type's moved.
+	assert.InDelta(t, 0.0,
+		mCounter(t, reg, mAttempts, mTypeLog), 0)
+	assert.InDelta(t, 0.0,
+		mCounter(t, reg, mFailed, mTypeLog), 0)
+}
+
+// mDeliverOK delivers one event to a target that answers 200.
+func mDeliverOK(t *testing.T, s iSetup) {
+	t.Helper()
+
+	ts := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		},
+	))
+	defer ts.Close()
+
+	event := iSeedEvent(
+		t, s.WebhookDB, s.WebhookID, `{"ok":true}`,
+	)
+	targetID := uuid.New().String()
+
+	d := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusPending,
+	)
+
+	body := event.Body
+	task := iTask(
+		d, event, s.WebhookID, targetID,
+		"metrics-ok", iHTTPConfig(ts.URL), 3, 1, &body,
+	)
+
+	s.Engine.ExportProcessNewTask(context.TODO(), &task)
+
+	iAssertStatus(t, s.WebhookDB, d.ID,
+		database.DeliveryStatusDelivered,
+	)
+}
+
+// mExhaustRetries delivers to a target that answers 500 with a
+// two-attempt budget, driving both attempts so the delivery ends
+// terminally failed.
+func mExhaustRetries(t *testing.T, s iSetup) {
+	t.Helper()
+
+	ts := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusInternalServerError)
+		},
+	))
+	defer ts.Close()
+
+	event := iSeedEvent(
+		t, s.WebhookDB, s.WebhookID, `{"ok":false}`,
+	)
+	targetID := uuid.New().String()
+
+	d := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusPending,
+	)
+
+	body := event.Body
+	cfg := iHTTPConfig(ts.URL)
+
+	first := iTask(
+		d, event, s.WebhookID, targetID,
+		"metrics-fail", cfg, 2, 1, &body,
+	)
+
+	s.Engine.ExportProcessNewTask(context.TODO(), &first)
+
+	iAssertStatus(t, s.WebhookDB, d.ID,
+		database.DeliveryStatusRetrying,
+	)
+
+	// The engine's own scheduler would re-enqueue this after the
+	// backoff; driving the second attempt directly keeps the test
+	// deterministic and off the wall clock.
+	second := iTask(
+		d, event, s.WebhookID, targetID,
+		"metrics-fail", cfg, 2, 2, &body,
+	)
+
+	s.Engine.ExportProcessRetryTask(
+		context.TODO(), &second,
+	)
+
+	iAssertStatus(t, s.WebhookDB, d.ID,
+		database.DeliveryStatusFailed,
+	)
+}
+
+// TestDeliveryMetrics_CircuitBreakerGauge proves the open-breaker
+// gauge follows a breaker that trips.
+func TestDeliveryMetrics_CircuitBreakerGauge(t *testing.T) {
+	t.Parallel()
+
+	s := newISetup(t)
+	reg := mIsolate(t, s)
+
+	ts := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusInternalServerError)
+		},
+	))
+	defer ts.Close()
+
+	event := iSeedEvent(
+		t, s.WebhookDB, s.WebhookID, `{"trip":true}`,
+	)
+	targetID := uuid.New().String()
+
+	d := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusPending,
+	)
+
+	body := event.Body
+	cfg := iHTTPConfig(ts.URL)
+
+	// A retry budget above the failure threshold, so the breaker
+	// rather than the budget is what stops the delivery.
+	maxRetries := delivery.ExportDefaultFailureThreshold + 5
+
+	first := iTask(
+		d, event, s.WebhookID, targetID,
+		"metrics-trip", cfg, maxRetries, 1, &body,
+	)
+
+	s.Engine.ExportProcessNewTask(context.TODO(), &first)
+
+	assert.InDelta(t, 0.0,
+		mGauge(t, reg, mBreakers, mTypeHTTP), 0)
+
+	for attempt := 2; attempt <= delivery.
+		ExportDefaultFailureThreshold; attempt++ {
+		task := iTask(
+			d, event, s.WebhookID, targetID,
+			"metrics-trip", cfg, maxRetries, attempt, &body,
+		)
+
+		s.Engine.ExportProcessRetryTask(
+			context.TODO(), &task,
+		)
+	}
+
+	assert.InDelta(t, 1.0,
+		mGauge(t, reg, mBreakers, mTypeHTTP), 0)
+}
+
+// TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt proves a delivery
+// an open circuit breaker refuses is neither counted as an attempt
+// nor observed in the duration histogram.
+//
+// It sends nothing and records no result row, so counting it would
+// climb the attempts counter with no traffic behind it and pull the
+// duration quantiles down with near-zero samples for as long as the
+// breaker stayed open — the metric moving the wrong way during the
+// outage it exists to reveal.
+func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	s := newISetup(t)
+	reg := mIsolate(t, s)
+
+	ts := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusInternalServerError)
+		},
+	))
+	defer ts.Close()
+
+	event := iSeedEvent(
+		t, s.WebhookDB, s.WebhookID, `{"blocked":true}`,
+	)
+	targetID := uuid.New().String()
+
+	d := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusPending,
+	)
+
+	body := event.Body
+	cfg := iHTTPConfig(ts.URL)
+	maxRetries := delivery.ExportDefaultFailureThreshold + 5
+
+	first := iTask(
+		d, event, s.WebhookID, targetID,
+		"metrics-blocked", cfg, maxRetries, 1, &body,
+	)
+
+	s.Engine.ExportProcessNewTask(context.TODO(), &first)
+
+	for attempt := 2; attempt <= delivery.
+		ExportDefaultFailureThreshold; attempt++ {
+		task := iTask(
+			d, event, s.WebhookID, targetID,
+			"metrics-blocked", cfg, maxRetries, attempt, &body,
+		)
+
+		s.Engine.ExportProcessRetryTask(context.TODO(), &task)
+	}
+
+	require.InDelta(t, 1.0,
+		mGauge(t, reg, mBreakers, mTypeHTTP), 0,
+		"breaker should be open before the blocked attempt")
+
+	threshold := float64(
+		delivery.ExportDefaultFailureThreshold,
+	)
+
+	assert.InDelta(t, threshold,
+		mCounter(t, reg, mAttempts, mTypeHTTP), 0)
+	assert.Equal(t, uint64(threshold),
+		mHTTPDurations(t, reg))
+
+	retriesBefore := mCounter(t, reg, mRetries, mTypeHTTP)
+
+	blocked := iTask(
+		d, event, s.WebhookID, targetID,
+		"metrics-blocked", cfg, maxRetries,
+		delivery.ExportDefaultFailureThreshold+1, &body,
+	)
+
+	s.Engine.ExportProcessRetryTask(context.TODO(), &blocked)
+
+	// The breaker refused it: rescheduled, so the retry counter
+	// moved, but nothing was attempted or timed.
+	assert.InDelta(t, retriesBefore+1,
+		mCounter(t, reg, mRetries, mTypeHTTP), 0)
+	assert.InDelta(t, threshold,
+		mCounter(t, reg, mAttempts, mTypeHTTP), 0)
+	assert.Equal(t, uint64(threshold),
+		mHTTPDurations(t, reg))
+}
+
+// TestDeliveryMetrics_OrphanedRetryFailureLabelled proves the
+// terminal failure of a delivery whose target no longer retries is
+// counted against the target's real type, not against unknown. The
+// type is threaded in as an argument because populating d.Target on
+// that path would write the target row into the per-webhook database
+// (https://git.eeqj.de/sneak/webhooker/issues/206).
+func TestDeliveryMetrics_OrphanedRetryFailureLabelled(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	s := newISetup(t)
+	reg := mIsolate(t, s)
+
+	iCreateWebhook(
+		t, s.MainDB, s.WebhookID, "orphaned-label",
+	)
+
+	deliveryID := iSeedRetryingWithType(
+		t, s, database.TargetTypeLog,
+	)
+
+	s.Engine.ExportSweepWebhookRetries(
+		context.Background(), s.WebhookID,
+	)
+
+	iAssertStatus(t, s.WebhookDB, deliveryID,
+		database.DeliveryStatusFailed,
+	)
+
+	assert.InDelta(t, 1.0,
+		mCounter(t, reg, mFailed, mTypeLog), 0)
+}
+
+// TestDeliveryMetrics_QueueDepthGauges proves the sampler publishes
+// the queued deliveries it finds in the per-webhook databases, and
+// that a drained queue reads zero rather than keeping its last
+// value.
+func TestDeliveryMetrics_QueueDepthGauges(t *testing.T) {
+	t.Parallel()
+
+	s := newISetup(t)
+	reg := mIsolate(t, s)
+
+	iCreateWebhook(
+		t, s.MainDB, s.WebhookID, "queue-depth",
+	)
+
+	targetID := uuid.New().String()
+
+	iCreateTarget(t, s.MainDB, targetID, s.WebhookID,
+		"queue-depth-target", database.TargetTypeHTTP,
+		iHTTPConfig("https://example.com/hook"), 3,
+	)
+
+	event := iSeedEvent(
+		t, s.WebhookDB, s.WebhookID, `{"queued":true}`,
+	)
+
+	pending := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusPending,
+	)
+
+	iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusPending,
+	)
+
+	retrying := iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusRetrying,
+	)
+
+	s.Engine.ExportSampleQueueDepths(context.Background())
+
+	assert.InDelta(t, 2.0,
+		mGauge(t, reg, mPending, mTypeHTTP), 0)
+	assert.InDelta(t, 1.0,
+		mGauge(t, reg, mRetrying, mTypeHTTP), 0)
+	assert.InDelta(t, 0.0,
+		mGauge(t, reg, mPending, mTypeLog), 0)
+
+	require.NoError(t, s.WebhookDB.
+		Model(&database.Delivery{}).
+		Where("id IN ?", []string{pending.ID, retrying.ID}).
+		Update(
+			"status", database.DeliveryStatusDelivered,
+		).Error)
+
+	s.Engine.ExportSampleQueueDepths(context.Background())
+
+	assert.InDelta(t, 1.0,
+		mGauge(t, reg, mPending, mTypeHTTP), 0)
+	assert.InDelta(t, 0.0,
+		mGauge(t, reg, mRetrying, mTypeHTTP), 0)
+}
+
+// TestDeliveryMetrics_QueueDepthDeletedTarget proves a backlog queued
+// against a target that has since been deleted stays visible, in the
+// unknown series, instead of being dropped. That backlog is the one
+// nobody is watching, so losing it would defeat the queue-depth
+// alerting this metric exists for.
+func TestDeliveryMetrics_QueueDepthDeletedTarget(t *testing.T) {
+	t.Parallel()
+
+	s := newISetup(t)
+	reg := mIsolate(t, s)
+
+	iCreateWebhook(
+		t, s.MainDB, s.WebhookID, "deleted-target",
+	)
+
+	// No target row is created: this is a delivery whose target was
+	// deleted out from under it.
+	targetID := uuid.New().String()
+
+	event := iSeedEvent(
+		t, s.WebhookDB, s.WebhookID, `{"orphan":true}`,
+	)
+
+	iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusPending,
+	)
+
+	iSeedDelivery(
+		t, s.WebhookDB, event.ID, targetID,
+		database.DeliveryStatusRetrying,
+	)
+
+	s.Engine.ExportSampleQueueDepths(context.Background())
+
+	assert.InDelta(t, 1.0,
+		mGauge(t, reg, mPending, mTypeUnknown), 0)
+	assert.InDelta(t, 1.0,
+		mGauge(t, reg, mRetrying, mTypeUnknown), 0)
+	assert.InDelta(t, 0.0,
+		mGauge(t, reg, mPending, mTypeHTTP), 0)
+}

internal/delivery/queue_depth.go

@@ -0,0 +1,187 @@
+package delivery
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// queueDepthSampleInterval is how often the pending and retrying
+// queue depths are counted and published as gauges.
+const queueDepthSampleInterval = 30 * time.Second
+
+// queueDepthSampler publishes the pending and retrying queue depths
+// on a timer for as long as the engine runs.
+//
+// The depths are counted out of the databases rather than tracked as
+// deltas alongside the status transitions. A delta counter would have
+// to be seeded correctly at startup from rows written by a previous
+// process, and would drift permanently on any transition that failed
+// to persist. Counting is the measurement that cannot go wrong, and
+// it is the same whole-database walk the retry sweep already makes.
+func (e *Engine) queueDepthSampler(ctx context.Context) {
+	defer e.wg.Done()
+
+	ticker := time.NewTicker(queueDepthSampleInterval)
+	defer ticker.Stop()
+
+	e.sampleQueueDepths(ctx)
+
+	for {
+		select {
+		case <-ctx.Done():
+			return
+		case <-ticker.C:
+			e.sampleQueueDepths(ctx)
+		}
+	}
+}
+
+// sampleQueueDepths counts every queued delivery across all
+// per-webhook databases and publishes the result.
+func (e *Engine) sampleQueueDepths(ctx context.Context) {
+	if e.database == nil || e.dbManager == nil {
+		return
+	}
+
+	types, err := e.targetTypesByID()
+	if err != nil {
+		e.log.Error(
+			"queue depth sample: failed to load target types",
+			"error", err,
+		)
+
+		return
+	}
+
+	var webhookIDs []string
+
+	err = e.database.DB().
+		Model(&database.Webhook{}).
+		Pluck("id", &webhookIDs).Error
+	if err != nil {
+		e.log.Error(
+			"queue depth sample: failed to query webhook IDs",
+			"error", err,
+		)
+
+		return
+	}
+
+	pending := make(map[database.TargetType]int)
+	retrying := make(map[database.TargetType]int)
+
+	for _, webhookID := range webhookIDs {
+		select {
+		case <-ctx.Done():
+			return
+		default:
+		}
+
+		if !e.dbManager.DBExists(webhookID) {
+			continue
+		}
+
+		e.sampleWebhookQueueDepths(
+			webhookID, types, pending, retrying,
+		)
+	}
+
+	e.mtr.SetQueueDepths(pending, retrying)
+}
+
+// targetTypesByID maps every configured target id to its type. The
+// deliveries live in the per-webhook databases but carry only a
+// target id, so the type label has to come from the main database.
+func (e *Engine) targetTypesByID() (
+	map[string]database.TargetType, error,
+) {
+	var rows []struct {
+		ID   string
+		Type database.TargetType
+	}
+
+	err := e.database.DB().
+		Model(&database.Target{}).
+		Select("id", "type").
+		Scan(&rows).Error
+	if err != nil {
+		return nil, fmt.Errorf("loading targets: %w", err)
+	}
+
+	types := make(map[string]database.TargetType, len(rows))
+
+	for _, row := range rows {
+		types[row.ID] = row.Type
+	}
+
+	return types, nil
+}
+
+// sampleWebhookQueueDepths adds one webhook's queued deliveries into
+// the running totals.
+//
+// A delivery whose target has since been deleted is not in the type
+// map and so counts under the empty target type. Set.SetQueueDepths
+// folds that into the unknown series rather than dropping it: a
+// backlog stuck behind a deleted target is a backlog that still needs
+// to be alertable.
+func (e *Engine) sampleWebhookQueueDepths(
+	webhookID string,
+	types map[string]database.TargetType,
+	pending, retrying map[database.TargetType]int,
+) {
+	webhookDB, err := e.dbManager.GetDB(webhookID)
+	if err != nil {
+		e.log.Error(
+			"queue depth sample: failed to get webhook database",
+			"webhook_id", webhookID,
+			"error", err,
+		)
+
+		return
+	}
+
+	var rows []struct {
+		TargetID string
+		Status   database.DeliveryStatus
+		Depth    int
+	}
+
+	err = webhookDB.
+		Model(&database.Delivery{}).
+		Select("target_id", "status", "count(*) as depth").
+		Where("status IN ?", []database.DeliveryStatus{
+			database.DeliveryStatusPending,
+			database.DeliveryStatusRetrying,
+		}).
+		Group("target_id, status").
+		Scan(&rows).Error
+	if err != nil {
+		e.log.Error(
+			"queue depth sample: "+
+				"failed to count queued deliveries",
+			"webhook_id", webhookID,
+			"error", err,
+		)
+
+		return
+	}
+
+	for _, row := range rows {
+		targetType := types[row.TargetID]
+
+		switch row.Status {
+		case database.DeliveryStatusPending:
+			pending[targetType] += row.Depth
+		case database.DeliveryStatusRetrying:
+			retrying[targetType] += row.Depth
+		case database.DeliveryStatusDelivered,
+			database.DeliveryStatusFailed:
+			// Excluded by the query above: a delivery that has
+			// reached a terminal state is not queued.
+		}
+	}
+}

internal/delivery/target.go

@@ -27,6 +27,12 @@ type Scheduler interface {
 // own circuit breaker, and reschedules via the injected
 // Scheduler. Fire-and-forget targets simply record a single
 // attempt.
+//
+// An implementation reports each attempt it actually dispatches to
+// Engine.observeAttempt, alongside the DeliveryResult it records for
+// it. Deliver is also entered for attempts that never happen — an
+// open circuit breaker refuses one — so the count cannot be taken
+// from around this call.
 type Target interface {
 	Deliver(
 		ctx context.Context,
@@ -74,6 +80,12 @@ type attemptResult struct {
 	errMsg     string
 }
 
+// elapsed returns how long the attempt took. The field is stored in
+// milliseconds because that is what DeliveryResult persists.
+func (r attemptResult) elapsed() time.Duration {
+	return time.Duration(r.duration) * time.Millisecond
+}
+
 // initTargets builds the target registry, wiring each target
 // to the engine's persistence helpers and giving the HTTP and
 // Slack targets the shared SSRF-safe client. It is called by

internal/delivery/target_config_edit.go

@@ -0,0 +1,119 @@
+package delivery
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// errUnknownTargetTypeForEdit is returned when a stored target has a
+// type the edit form has no field set for.
+var errUnknownTargetTypeForEdit = errors.New(
+	"unknown target type",
+)
+
+// TargetConfigForm is the UNMASKED projection of a target's stored
+// configuration, for pre-filling the target edit form.
+//
+// It is the deliberate exception to the rule TargetView enforces
+// everywhere else: TargetView exists so that no template can render
+// a target's stored blob, because a destination URL's path segments
+// and a header value are both routinely the credential. An operator
+// cannot correct a value they cannot see, so the edit form — and
+// only the edit form — is shown the full value.
+//
+// Everything that keeps that exception narrow lives at the call
+// site: the route is behind RequireAuth and the webhook's ownership
+// check, and its group sets NoCache so the rendered secret is not
+// written to a shared cache. Do not reach for this type from any
+// other page.
+type TargetConfigForm struct {
+	// URL is the destination for an HTTP target and the webhook
+	// URL for a Slack target.
+	URL string
+	// Headers is the HTTP target's configured headers in the
+	// textarea representation, one "Name: value" per line.
+	Headers string
+	// Timeout is the HTTP target's per-request timeout in seconds,
+	// empty when unset.
+	Timeout string
+	// Expiry is the database (archive) target's row expiry.
+	Expiry string
+}
+
+// NewTargetConfigForm parses a target's stored configuration into
+// the edit form's fields.
+//
+// A configuration that does not parse is an error rather than a
+// zero-valued form that silently looks like a target with no
+// settings. The caller shows the operator that the stored value
+// could not be read, so that saving the form is understood as
+// replacing it rather than preserving it.
+func NewTargetConfigForm(
+	t *database.Target,
+) (TargetConfigForm, error) {
+	switch t.Type {
+	case database.TargetTypeHTTP:
+		cfg, err := parseHTTPConfig(t.Config)
+		if err != nil {
+			return TargetConfigForm{}, err
+		}
+
+		return TargetConfigForm{
+			URL:     cfg.URL,
+			Headers: FormatTargetHeaders(cfg.Headers),
+			Timeout: FormatTargetTimeout(cfg.Timeout),
+		}, nil
+	case database.TargetTypeSlack:
+		cfg, err := parseSlackConfig(t.Config)
+		if err != nil {
+			return TargetConfigForm{}, err
+		}
+
+		return TargetConfigForm{URL: cfg.WebhookURL}, nil
+	case database.TargetTypeDatabase:
+		return databaseConfigForm(t.Config)
+	case database.TargetTypeLog:
+		// The log target takes no configuration.
+		return TargetConfigForm{}, nil
+	default:
+		return TargetConfigForm{}, fmt.Errorf(
+			"%w: %q", errUnknownTargetTypeForEdit, t.Type,
+		)
+	}
+}
+
+// databaseConfigForm parses an archive target's optional expiry.
+// An absent or empty configuration is the keep-forever default and
+// yields an empty field, so re-saving the form unchanged stores the
+// same empty configuration it started with. An expiry that is set
+// but not a valid duration is an error, not a blank field.
+func databaseConfigForm(
+	configJSON string,
+) (TargetConfigForm, error) {
+	if configJSON == "" {
+		return TargetConfigForm{}, nil
+	}
+
+	var cfg databaseTargetConfig
+
+	err := json.Unmarshal([]byte(configJSON), &cfg)
+	if err != nil {
+		return TargetConfigForm{}, fmt.Errorf(
+			"parsing config JSON: %w", err,
+		)
+	}
+
+	if cfg.Expiry == "" || cfg.Expiry == archiveExpiryNever {
+		return TargetConfigForm{}, nil
+	}
+
+	err = ValidateArchiveExpiry(cfg.Expiry)
+	if err != nil {
+		return TargetConfigForm{}, err
+	}
+
+	return TargetConfigForm{Expiry: cfg.Expiry}, nil
+}

internal/delivery/target_database.go

@@ -42,7 +42,14 @@ func (t *databaseTarget) Deliver(
 	_ *Task,
 	_ Scheduler,
 ) {
+	start := time.Now()
+
 	err := t.archive(d)
+
+	elapsed := time.Since(start)
+
+	t.eng.observeAttempt(d.Target.Type, elapsed)
+
 	if err != nil {
 		t.eng.log.Error(
 			"failed to archive event to database target",
@@ -53,22 +60,25 @@ func (t *databaseTarget) Deliver(
 
 		t.eng.recordResult(
 			webhookDB, d, 1, false, 0, "",
-			err.Error(), 0,
+			err.Error(), elapsed.Milliseconds(),
 		)
 
 		t.eng.updateDeliveryStatus(
-			webhookDB, d, database.DeliveryStatusFailed,
+			webhookDB, d, d.Target.Type,
+			database.DeliveryStatusFailed,
 		)
 
 		return
 	}
 
 	t.eng.recordResult(
-		webhookDB, d, 1, true, 0, "", "", 0,
+		webhookDB, d, 1, true, 0, "", "",
+		elapsed.Milliseconds(),
 	)
 
 	t.eng.updateDeliveryStatus(
-		webhookDB, d, database.DeliveryStatusDelivered,
+		webhookDB, d, d.Target.Type,
+		database.DeliveryStatusDelivered,
 	)
 }
 

internal/delivery/target_headers.go

@@ -0,0 +1,256 @@
+package delivery
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"slices"
+	"strconv"
+	"strings"
+)
+
+// MaxTargetTimeoutSeconds bounds a per-target request timeout.
+// A delivery attempt holds a worker for its whole duration, so an
+// unbounded timeout lets one misconfigured target stall the queue
+// indefinitely. Five minutes is far beyond any healthy webhook
+// receiver and still finite.
+const MaxTargetTimeoutSeconds = 300
+
+// Errors returned when a target's header or timeout form input
+// cannot be turned into a configuration.
+//
+// None of these ever quotes a header VALUE. A target header value
+// is routinely an authorization token, and these messages are shown
+// to the user in an error page body.
+var (
+	errHeaderLineMalformed = errors.New(
+		`each header line must be "Name: value"`,
+	)
+	errHeaderNameInvalid = errors.New(
+		"header name must be a valid HTTP token",
+	)
+	errHeaderValueInvalid = errors.New(
+		"header value must not contain control characters",
+	)
+	errHeaderDuplicate = errors.New(
+		"header given more than once",
+	)
+	errHeaderReserved = errors.New(
+		"header is set by the delivery engine and cannot be " +
+			"overridden",
+	)
+	errTimeoutInvalid = errors.New(
+		"timeout must be a whole number of seconds",
+	)
+	errTimeoutOutOfRange = errors.New(
+		"timeout is out of range",
+	)
+)
+
+// isReservedTargetHeader reports whether name (canonicalised) is a
+// header a target configuration may not set, because the delivery
+// path or net/http itself writes it regardless.
+//
+// These are rejected rather than accepted-and-ignored. Storing a
+// header that provably never reaches the wire tells the operator
+// their configuration took effect when it did not, which is the
+// same failure mode as silently substituting a default for an
+// invalid value.
+func isReservedTargetHeader(name string) bool {
+	switch name {
+	case "Host", "Content-Length", "Transfer-Encoding", "Connection":
+		return true
+	case "User-Agent":
+		// applyRequestHeaders sets the User-Agent after it applies
+		// the configured headers, so a configured one would always
+		// be overwritten.
+		return true
+	default:
+		return false
+	}
+}
+
+// ParseTargetHeaders turns the target form's headers field — one
+// "Name: value" pair per line, blank lines ignored — into the map
+// stored in HTTPTargetConfig.Headers. Names are canonicalised, so a
+// name repeated in a different case is still a duplicate rather than
+// one pair silently overwriting the other.
+//
+// An input with no pairs yields an empty map, which omitempty drops
+// from the stored config: a target configured with no headers keeps
+// the same config JSON it had before this field existed.
+func ParseTargetHeaders(raw string) (map[string]string, error) {
+	headers := make(map[string]string)
+
+	for i, line := range strings.Split(raw, "\n") {
+		lineNum := i + 1
+
+		line = strings.TrimSpace(line)
+		if line == "" {
+			continue
+		}
+
+		name, value, err := parseHeaderLine(line)
+		if err != nil {
+			return nil, fmt.Errorf("line %d: %w", lineNum, err)
+		}
+
+		if _, dup := headers[name]; dup {
+			return nil, fmt.Errorf(
+				"line %d: %w: %q", lineNum,
+				errHeaderDuplicate, name,
+			)
+		}
+
+		headers[name] = value
+	}
+
+	return headers, nil
+}
+
+// parseHeaderLine splits and validates one "Name: value" line,
+// returning the canonicalised name and the trimmed value.
+func parseHeaderLine(line string) (string, string, error) {
+	rawName, value, found := strings.Cut(line, ":")
+	if !found {
+		return "", "", errHeaderLineMalformed
+	}
+
+	rawName = strings.TrimSpace(rawName)
+	if !validHeaderName(rawName) {
+		return "", "", fmt.Errorf(
+			"%w: %q", errHeaderNameInvalid, rawName,
+		)
+	}
+
+	name := http.CanonicalHeaderKey(rawName)
+	if isReservedTargetHeader(name) {
+		return "", "", fmt.Errorf(
+			"%w: %q", errHeaderReserved, name,
+		)
+	}
+
+	value = strings.TrimSpace(value)
+	if !validHeaderValue(value) {
+		return "", "", fmt.Errorf(
+			"%w: %q", errHeaderValueInvalid, name,
+		)
+	}
+
+	return name, value, nil
+}
+
+// validHeaderName reports whether name is a non-empty RFC 9110
+// field name. Rejecting anything else here is what keeps a value
+// containing CR or LF from being smuggled in as part of a name and
+// injecting a second header into the outbound request.
+func validHeaderName(name string) bool {
+	if name == "" {
+		return false
+	}
+
+	for i := range len(name) {
+		if !isTokenByte(name[i]) {
+			return false
+		}
+	}
+
+	return true
+}
+
+// isTokenByte reports whether c is a "tchar" per RFC 9110 5.6.2.
+func isTokenByte(c byte) bool {
+	switch {
+	case c >= 'a' && c <= 'z',
+		c >= 'A' && c <= 'Z',
+		c >= '0' && c <= '9':
+		return true
+	}
+
+	return strings.IndexByte("!#$%&'*+-.^_`|~", c) >= 0
+}
+
+// validHeaderValue reports whether value is a legal field value:
+// no control characters, which is the other half of the header
+// injection guard. An empty value is legal.
+func validHeaderValue(value string) bool {
+	for i := range len(value) {
+		c := value[i]
+		if c < 0x20 || c == 0x7f {
+			return false
+		}
+	}
+
+	return true
+}
+
+// FormatTargetHeaders renders a stored header map back into the
+// form's textarea representation, one "Name: value" per line.
+//
+// Names are sorted so that loading the edit form twice without
+// saving produces identical text; Go map iteration order would
+// otherwise reshuffle the field on every render.
+func FormatTargetHeaders(headers map[string]string) string {
+	if len(headers) == 0 {
+		return ""
+	}
+
+	names := make([]string, 0, len(headers))
+	for name := range headers {
+		names = append(names, name)
+	}
+
+	slices.Sort(names)
+
+	var b strings.Builder
+
+	for _, name := range names {
+		b.WriteString(name)
+		b.WriteString(": ")
+		b.WriteString(headers[name])
+		b.WriteString("\n")
+	}
+
+	return b.String()
+}
+
+// ParseTargetTimeout interprets the target form's timeout field as
+// a whole number of seconds. An empty field means "unset" and yields
+// 0, which omitempty drops from the stored config and which the
+// delivery path reads as "use the shared client's timeout".
+//
+// Anything else that is not a whole number in range is an error, not
+// a silently substituted default: a target whose timeout was typed
+// wrong must say so at the form rather than deliver on a timeout its
+// operator did not choose.
+func ParseTargetTimeout(raw string) (int, error) {
+	raw = strings.TrimSpace(raw)
+	if raw == "" {
+		return 0, nil
+	}
+
+	v, err := strconv.Atoi(raw)
+	if err != nil || v < 0 {
+		return 0, errTimeoutInvalid
+	}
+
+	if v > MaxTargetTimeoutSeconds {
+		return 0, fmt.Errorf(
+			"%w: at most %d seconds",
+			errTimeoutOutOfRange, MaxTargetTimeoutSeconds,
+		)
+	}
+
+	return v, nil
+}
+
+// FormatTargetTimeout renders a stored timeout for the form field.
+// An unset timeout renders as an empty field rather than "0", so the
+// placeholder can describe the default the target actually uses.
+func FormatTargetTimeout(timeout int) string {
+	if timeout <= 0 {
+		return ""
+	}
+
+	return strconv.Itoa(timeout)
+}

internal/delivery/target_headers_test.go

@@ -0,0 +1,273 @@
+package delivery_test
+
+import (
+	"encoding/json"
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/delivery"
+)
+
+// Literals these tests repeat, named so that the header name and the
+// keep-forever archive config each have one definition.
+const (
+	headerAuthorization = "Authorization"
+	bearerValue         = "Bearer abc"
+	archiveConfigNever  = "{\"expiry\":\"never\"}"
+)
+
+func TestParseTargetHeaders_AcceptsPairs(t *testing.T) {
+	t.Parallel()
+
+	got, err := delivery.ParseTargetHeaders(
+		"  Authorization: Bearer abc \n\n" +
+			"x-tenant:acme\r\n" +
+			"X-Empty:\n",
+	)
+	require.NoError(t, err)
+
+	assert.Equal(
+		t,
+		map[string]string{
+			headerAuthorization: bearerValue,
+			"X-Tenant":          "acme",
+			"X-Empty":           "",
+		},
+		got,
+	)
+}
+
+// A configuration with no headers must stay indistinguishable from
+// one written before the field existed, so omitempty drops the key.
+func TestParseTargetHeaders_EmptyInputYieldsNoHeaders(t *testing.T) {
+	t.Parallel()
+
+	got, err := delivery.ParseTargetHeaders("\n  \n")
+	require.NoError(t, err)
+	assert.Empty(t, got)
+
+	encoded, err := json.Marshal(delivery.HTTPTargetConfig{
+		URL:     "https://example.com/h",
+		Headers: got,
+	})
+	require.NoError(t, err)
+	assert.JSONEq(
+		t, `{"url":"https://example.com/h"}`, string(encoded),
+	)
+}
+
+func TestParseTargetHeaders_Rejects(t *testing.T) {
+	t.Parallel()
+
+	cases := map[string]string{
+		"no colon":           "Authorization Bearer abc",
+		"empty name":         ": value",
+		"space in name":      "X Bad: value",
+		"reserved host":      "Host: evil.example",
+		"reserved ua":        "User-Agent: curl/8",
+		"reserved length":    "Content-Length: 0",
+		"duplicate any case": "X-A: 1\nx-a: 2",
+	}
+
+	for name, input := range cases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			_, err := delivery.ParseTargetHeaders(input)
+			require.Error(t, err)
+		})
+	}
+}
+
+// A header value is routinely a bearer token and these errors are
+// rendered into a 400 body, so no message may quote one.
+func TestParseTargetHeaders_ErrorsNeverQuoteAValue(t *testing.T) {
+	t.Parallel()
+
+	const secret = "QQNEVERINAMESSAGEQQ"
+
+	_, err := delivery.ParseTargetHeaders(
+		"X-A: " + secret + "\nx-a: " + secret,
+	)
+	require.Error(t, err)
+	assert.NotContains(t, err.Error(), secret)
+
+	_, err = delivery.ParseTargetHeaders(
+		"X Bad Name: " + secret,
+	)
+	require.Error(t, err)
+	assert.NotContains(t, err.Error(), secret)
+}
+
+// Loading the edit form twice without saving must not reshuffle
+// the textarea, which Go's map iteration order would otherwise do.
+func TestFormatTargetHeaders_IsSorted(t *testing.T) {
+	t.Parallel()
+
+	got := delivery.FormatTargetHeaders(map[string]string{
+		"X-Zed":             "z",
+		headerAuthorization: bearerValue,
+		"X-Alpha":           "a",
+	})
+
+	assert.Equal(
+		t,
+		"Authorization: Bearer abc\nX-Alpha: a\nX-Zed: z\n",
+		got,
+	)
+	assert.Empty(t, delivery.FormatTargetHeaders(nil))
+}
+
+func TestFormatTargetHeaders_RoundTripsThroughParse(t *testing.T) {
+	t.Parallel()
+
+	want := map[string]string{
+		headerAuthorization: bearerValue,
+		"X-Tenant":          "acme",
+	}
+
+	got, err := delivery.ParseTargetHeaders(
+		delivery.FormatTargetHeaders(want),
+	)
+	require.NoError(t, err)
+	assert.Equal(t, want, got)
+}
+
+func TestParseTargetTimeout(t *testing.T) {
+	t.Parallel()
+
+	got, err := delivery.ParseTargetTimeout("  30 ")
+	require.NoError(t, err)
+	assert.Equal(t, 30, got)
+
+	got, err = delivery.ParseTargetTimeout("")
+	require.NoError(t, err)
+	assert.Zero(t, got)
+
+	for _, bad := range []string{"soon", "-1", "1e3", "100000"} {
+		_, err = delivery.ParseTargetTimeout(bad)
+		require.Error(t, err, bad)
+	}
+}
+
+func TestFormatTargetTimeout(t *testing.T) {
+	t.Parallel()
+
+	assert.Equal(t, "30", delivery.FormatTargetTimeout(30))
+	assert.Empty(t, delivery.FormatTargetTimeout(0))
+	assert.Empty(t, delivery.FormatTargetTimeout(-1))
+}
+
+func TestNewTargetConfigForm(t *testing.T) {
+	t.Parallel()
+
+	form, err := delivery.NewTargetConfigForm(&database.Target{
+		Type: database.TargetTypeHTTP,
+		Config: `{"url":"https://example.com/h",` +
+			`"headers":{"Authorization":"Bearer abc"},` +
+			`"timeout":9}`,
+	})
+	require.NoError(t, err)
+	assert.Equal(t, "https://example.com/h", form.URL)
+	assert.Equal(t, "Authorization: Bearer abc\n", form.Headers)
+	assert.Equal(t, "9", form.Timeout)
+
+	form, err = delivery.NewTargetConfigForm(&database.Target{
+		Type:   database.TargetTypeSlack,
+		Config: `{"webhookUrl":"https://hooks.example/s"}`,
+	})
+	require.NoError(t, err)
+	assert.Equal(t, "https://hooks.example/s", form.URL)
+
+	form, err = delivery.NewTargetConfigForm(&database.Target{
+		Type:   database.TargetTypeDatabase,
+		Config: `{"expiry":"720h"}`,
+	})
+	require.NoError(t, err)
+	assert.Equal(t, "720h", form.Expiry)
+
+	form, err = delivery.NewTargetConfigForm(&database.Target{
+		Type: database.TargetTypeLog,
+	})
+	require.NoError(t, err)
+	assert.Empty(t, form.URL)
+}
+
+// A keep-forever archive target must pre-fill as an empty field, so
+// saving the form back unchanged stores the same empty config.
+func TestNewTargetConfigForm_DatabaseNeverIsBlank(t *testing.T) {
+	t.Parallel()
+
+	for _, cfg := range []string{"", `{}`, archiveConfigNever} {
+		form, err := delivery.NewTargetConfigForm(
+			&database.Target{
+				Type:   database.TargetTypeDatabase,
+				Config: cfg,
+			},
+		)
+		require.NoError(t, err, cfg)
+		assert.Empty(t, form.Expiry, cfg)
+	}
+}
+
+// An unreadable stored config is an error rather than a blank form
+// that looks like a target with no settings, so the caller can tell
+// the operator that saving replaces the stored value.
+func TestNewTargetConfigForm_UnreadableConfigErrors(t *testing.T) {
+	t.Parallel()
+
+	cases := []*database.Target{
+		{Type: database.TargetTypeHTTP, Config: "not json"},
+		{Type: database.TargetTypeHTTP, Config: `{}`},
+		{Type: database.TargetTypeSlack, Config: ""},
+		{
+			Type:   database.TargetTypeDatabase,
+			Config: `{"expiry":"soon"}`,
+		},
+		{Type: database.TargetType("nope")},
+	}
+
+	for _, target := range cases {
+		_, err := delivery.NewTargetConfigForm(target)
+		require.Error(t, err, target.Type)
+	}
+}
+
+// The ceiling exists so one misconfigured target cannot hold a
+// delivery worker indefinitely, and it is inclusive.
+func TestParseTargetTimeout_CeilingIsInclusive(t *testing.T) {
+	t.Parallel()
+
+	assert.Positive(t, delivery.MaxTargetTimeoutSeconds)
+
+	got, err := delivery.ParseTargetTimeout(
+		strconv.Itoa(delivery.MaxTargetTimeoutSeconds),
+	)
+	require.NoError(t, err)
+	assert.Equal(t, delivery.MaxTargetTimeoutSeconds, got)
+
+	_, err = delivery.ParseTargetTimeout(
+		strconv.Itoa(delivery.MaxTargetTimeoutSeconds + 1),
+	)
+	require.Error(t, err)
+}
+
+// Control characters in a value are how a second header would be
+// smuggled into the outbound request.
+func TestParseTargetHeaders_RejectsControlCharactersInValues(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	for _, bad := range []string{
+		"X-A: one\x01two",
+		"X-A: one\ttwo",
+		"X-A: one\x7ftwo",
+	} {
+		_, err := delivery.ParseTargetHeaders(bad)
+		require.Error(t, err, bad)
+	}
+}

internal/delivery/target_http.go

@@ -74,6 +74,8 @@ func (c *httpCore) fireAndForget(
 	d *database.Delivery,
 	res attemptResult,
 ) {
+	c.eng.observeAttempt(d.Target.Type, res.elapsed())
+
 	c.eng.recordResult(
 		webhookDB, d, 1, res.success,
 		res.statusCode, res.respBody, res.errMsg,
@@ -82,7 +84,7 @@ func (c *httpCore) fireAndForget(
 
 	if res.success {
 		c.eng.updateDeliveryStatus(
-			webhookDB, d,
+			webhookDB, d, d.Target.Type,
 			database.DeliveryStatusDelivered,
 		)
 
@@ -90,7 +92,8 @@ func (c *httpCore) fireAndForget(
 	}
 
 	c.eng.updateDeliveryStatus(
-		webhookDB, d, database.DeliveryStatusFailed,
+		webhookDB, d, d.Target.Type,
+		database.DeliveryStatusFailed,
 	)
 }
 
@@ -107,10 +110,17 @@ func (c *httpCore) withRetry(
 		return
 	}
 
+	// Allow may have moved the breaker to half-open, and the
+	// attempt below may open or close it, so the gauge is
+	// republished on every exit from here.
+	defer c.publishCircuitState(d.Target.Type)
+
 	attemptNum := task.AttemptNum
 
 	res := attempt()
 
+	c.eng.observeAttempt(d.Target.Type, res.elapsed())
+
 	c.eng.recordResult(
 		webhookDB, d, attemptNum, res.success,
 		res.statusCode, res.respBody, res.errMsg,
@@ -121,7 +131,7 @@ func (c *httpCore) withRetry(
 		cb.RecordSuccess()
 
 		c.eng.updateDeliveryStatus(
-			webhookDB, d,
+			webhookDB, d, d.Target.Type,
 			database.DeliveryStatusDelivered,
 		)
 
@@ -146,6 +156,8 @@ func (c *httpCore) circuitBreakerBlock(
 		return false
 	}
 
+	defer c.publishCircuitState(d.Target.Type)
+
 	remaining := cb.CooldownRemaining()
 
 	c.eng.log.Info(
@@ -157,7 +169,7 @@ func (c *httpCore) circuitBreakerBlock(
 	)
 
 	c.eng.updateDeliveryStatus(
-		webhookDB, d,
+		webhookDB, d, d.Target.Type,
 		database.DeliveryStatusRetrying,
 	)
 
@@ -177,7 +189,7 @@ func (c *httpCore) handleRetry(
 ) {
 	if attemptNum >= maxRetries {
 		c.eng.updateDeliveryStatus(
-			webhookDB, d,
+			webhookDB, d, d.Target.Type,
 			database.DeliveryStatusFailed,
 		)
 
@@ -185,7 +197,8 @@ func (c *httpCore) handleRetry(
 	}
 
 	c.eng.updateDeliveryStatus(
-		webhookDB, d, database.DeliveryStatusRetrying,
+		webhookDB, d, d.Target.Type,
+		database.DeliveryStatusRetrying,
 	)
 
 	backoff := calcBackoff(attemptNum)
@@ -215,6 +228,28 @@ func (c *httpCore) getCircuitBreaker(
 	return cb
 }
 
+// publishCircuitState recounts this core's open breakers and
+// publishes the gauge. Each core holds the breakers of exactly one
+// target type, so the recount is over that type's targets alone.
+// Counting rather than adjusting a delta keeps the gauge honest
+// however a breaker changed state.
+func (c *httpCore) publishCircuitState(
+	targetType database.TargetType,
+) {
+	open := 0
+
+	c.circuitBreakers.Range(func(_, val any) bool {
+		cb, ok := val.(*CircuitBreaker)
+		if ok && cb.State() == CircuitOpen {
+			open++
+		}
+
+		return true
+	})
+
+	c.eng.mtr.SetCircuitBreakersOpen(targetType, open)
+}
+
 // remainingBackoff returns how long remains of the backoff
 // window for the last attempt of a recovered retrying
 // delivery. It implements rescheduler.
@@ -302,7 +337,8 @@ func (t *httpTarget) Deliver(
 		)
 
 		t.eng.updateDeliveryStatus(
-			webhookDB, d, database.DeliveryStatusFailed,
+			webhookDB, d, d.Target.Type,
+			database.DeliveryStatusFailed,
 		)
 
 		return

internal/delivery/target_log.go

@@ -2,6 +2,7 @@ package delivery
 
 import (
 	"context"
+	"time"
 
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/database"
@@ -34,6 +35,8 @@ func (t *logTarget) Deliver(
 	_ *Task,
 	_ Scheduler,
 ) {
+	start := time.Now()
+
 	t.eng.log.Info(
 		"webhook event delivered to log target",
 		"delivery_id", d.ID,
@@ -48,11 +51,17 @@ func (t *logTarget) Deliver(
 		"body", d.Event.Body,
 	)
 
+	elapsed := time.Since(start)
+
+	t.eng.observeAttempt(d.Target.Type, elapsed)
+
 	t.eng.recordResult(
-		webhookDB, d, 1, true, 0, "", "", 0,
+		webhookDB, d, 1, true, 0, "", "",
+		elapsed.Milliseconds(),
 	)
 
 	t.eng.updateDeliveryStatus(
-		webhookDB, d, database.DeliveryStatusDelivered,
+		webhookDB, d, d.Target.Type,
+		database.DeliveryStatusDelivered,
 	)
 }

internal/delivery/target_slack.go

@@ -101,7 +101,8 @@ func (t *slackTarget) failConfig(
 	)
 
 	t.eng.updateDeliveryStatus(
-		webhookDB, d, database.DeliveryStatusFailed,
+		webhookDB, d, d.Target.Type,
+		database.DeliveryStatusFailed,
 	)
 }
 

internal/gormlog/firstboot_test.go

@@ -0,0 +1,229 @@
+package gormlog_test
+
+import (
+	"context"
+	"database/sql"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/fx"
+	"go.uber.org/fx/fxtest"
+	_ "modernc.org/sqlite" // Pure Go SQLite driver.
+	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/globals"
+	"sneak.berlin/go/webhooker/internal/logger"
+	"sneak.berlin/go/webhooker/internal/session"
+)
+
+// argon2Prefix opens every encoded Argon2id hash this service
+// produces. It is asserted on separately from the hash itself, so that
+// a change to the password encoding cannot quietly turn the hash
+// assertion into a comparison against a string the log never held.
+const argon2Prefix = "$argon2id$"
+
+// settingsInsert and usersInsert are the two statements a first boot
+// runs that carry a secret. The sqlite dialector quotes identifiers
+// with backticks.
+const (
+	settingsInsert = "INSERT INTO `settings`"
+	usersInsert    = "INSERT INTO `users`"
+)
+
+// captureStdoutToFile redirects os.Stdout into a file for the rest of
+// the test and returns a function that reads back everything written
+// to it.
+//
+// A file rather than a pipe: internal/logger writes synchronously to
+// whatever os.Stdout is when it builds its handler, so once fx's start
+// returns, every byte the boot produced is already in the file and no
+// draining goroutine is needed to prove it. Redirecting the variable
+// before the application is built is what puts the service logger —
+// and therefore the GORM adapter, which writes through it — into the
+// capture.
+//
+// The redirect also decides the handler: a regular file is not a
+// character device, so internal/logger installs its JSON handler, the
+// one it installs in production under a log collector.
+func captureStdoutToFile(t *testing.T) func() string {
+	t.Helper()
+
+	path := filepath.Join(t.TempDir(), "stdout.log")
+
+	//nolint:gosec // The path is this test's own t.TempDir().
+	f, err := os.Create(path)
+	require.NoError(t, err)
+
+	orig := os.Stdout
+	os.Stdout = f
+
+	t.Cleanup(func() {
+		os.Stdout = orig
+		_ = f.Close()
+	})
+
+	return func() string {
+		require.NoError(t, f.Sync())
+
+		//nolint:gosec // As above.
+		b, readErr := os.ReadFile(path)
+		require.NoError(t, readErr)
+
+		return string(b)
+	}
+}
+
+// firstBootSecrets are the two values a first boot generates and
+// stores, read back out of the database.
+type firstBootSecrets struct {
+	sessionKey   string
+	passwordHash string
+}
+
+// readFirstBootSecrets reads those two secrets straight out of the
+// SQLite file with database/sql rather than through GORM, so that
+// reading them cannot itself add a line to the log under test.
+func readFirstBootSecrets(
+	t *testing.T, dataDir string,
+) firstBootSecrets {
+	t.Helper()
+
+	db, err := sql.Open("sqlite", filepath.Join(
+		dataDir, "webhooker.db",
+	))
+	require.NoError(t, err)
+
+	defer func() { require.NoError(t, db.Close()) }()
+
+	ctx := context.Background()
+
+	var got firstBootSecrets
+
+	require.NoError(t, db.QueryRowContext(
+		ctx, `SELECT value FROM settings WHERE key = 'session_key'`,
+	).Scan(&got.sessionKey))
+
+	require.NoError(t, db.QueryRowContext(
+		ctx, `SELECT password FROM users WHERE username = 'admin'`,
+	).Scan(&got.passwordHash))
+
+	require.NotEmpty(t, got.sessionKey)
+	require.Contains(t, got.passwordHash, argon2Prefix)
+
+	return got
+}
+
+// bootAtDebug starts and stops the real application graph against
+// dataDir with DEBUG=true, and returns everything it wrote to standard
+// output.
+//
+// config.New reads DEBUG from the environment exactly as the binary
+// does, internal/logger builds the handler it builds in production,
+// database.New runs the migrations and creates the admin user, and
+// session.New takes the session key. Those four are the whole of the
+// path that writes either secret.
+func bootAtDebug(t *testing.T, dataDir string) string {
+	t.Helper()
+
+	t.Setenv("DEBUG", "true")
+	t.Setenv("DATA_DIR", dataDir)
+
+	read := captureStdoutToFile(t)
+
+	var sess *session.Session
+
+	app := fxtest.New(
+		t,
+		fx.Provide(
+			globals.New,
+			logger.New,
+			config.New,
+			database.New,
+			session.New,
+		),
+		fx.Populate(&sess),
+	)
+
+	app.RequireStart()
+	app.RequireStop()
+
+	return read()
+}
+
+// requireFirstBootWasLogged is the non-vacuity half of the test below.
+// Without it a build that logged no SQL at all, or that never reached
+// DEBUG, would satisfy every absence assertion.
+func requireFirstBootWasLogged(t *testing.T, out string) {
+	t.Helper()
+
+	require.Contains(
+		t, out, `"level":"DEBUG"`,
+		"DEBUG=true did not reach the logger",
+	)
+	require.Contains(
+		t, out, settingsInsert,
+		"the session key INSERT was not logged, so its absence "+
+			"proves nothing",
+	)
+	require.Contains(
+		t, out, usersInsert,
+		"the admin user INSERT was not logged, so its absence "+
+			"proves nothing",
+	)
+}
+
+// TestFirstBootAtDebug_LogsNeitherSecret is the definition of done.
+//
+// A first boot is the only boot that writes either secret. The
+// settings INSERT carries the base64 session encryption key, which is
+// the whole of the session security model: anyone holding it can forge
+// an authenticated session cookie. The users INSERT carries the admin
+// account's Argon2id hash. Under interpolated statement logging both
+// landed in the log an operator diagnosing a startup problem pastes
+// into an issue.
+//
+// The secrets are read back out of the database file afterwards, so
+// the assertions are made against the values this boot actually
+// generated rather than against a pattern that might not match them.
+//
+// What this test does not cover: the initial admin password itself,
+// which internal/database logs once in the clear, on purpose, because
+// that line is the only place an operator ever sees it. That is a
+// separate decision from the SQL log, and it is documented in the
+// README rather than asserted here.
+//
+// Not parallel: it redirects os.Stdout and sets environment variables,
+// both process-global.
+//
+//nolint:paralleltest // Deliberately sequential; see above.
+func TestFirstBootAtDebug_LogsNeitherSecret(t *testing.T) {
+	dataDir := t.TempDir()
+
+	// An empty DATA_DIR is what makes this a first boot: with a
+	// database already in place neither INSERT runs.
+	entries, err := os.ReadDir(dataDir)
+	require.NoError(t, err)
+	require.Empty(t, entries, "DATA_DIR was not empty")
+
+	out := bootAtDebug(t, dataDir)
+
+	requireFirstBootWasLogged(t, out)
+
+	secrets := readFirstBootSecrets(t, dataDir)
+
+	assert.NotContains(
+		t, out, secrets.sessionKey,
+		"the session encryption key reached the debug log",
+	)
+	assert.NotContains(
+		t, out, secrets.passwordHash,
+		"the admin password hash reached the debug log",
+	)
+	assert.NotContains(
+		t, out, argon2Prefix,
+		"an encoded Argon2id hash reached the debug log",
+	)
+}

internal/gormlog/gormlog.go

@@ -17,6 +17,10 @@
 // level the operator controls, they are shaped by whichever handler
 // internal/logger selected, and every value a client can influence is
 // spent through logfield.Truncate.
+//
+// It also logs no bound value at all. See ParamsFilter: the statement
+// is written with its placeholders intact, at every level, so the
+// values a statement carries never reach the log in the first place.
 package gormlog
 
 import (
@@ -26,6 +30,7 @@ import (
 	"log/slog"
 	"time"
 
+	"gorm.io/gorm"
 	gormlogger "gorm.io/gorm/logger"
 	"sneak.berlin/go/webhooker/internal/logfield"
 )
@@ -47,8 +52,14 @@ type Logger struct {
 }
 
 // Interface compliance is asserted here rather than discovered at the
-// gorm.Open call sites.
+// gorm.Open call sites. gorm.ParamsFilter is the optional half: GORM
-var _ gormlogger.Interface = (*Logger)(nil)
+// type-asserts for it and silently keeps interpolating if it is
+// missing, so losing it would cost no build error and no test that
+// does not look at the emitted SQL.
+var (
+	_ gormlogger.Interface = (*Logger)(nil)
+	_ gorm.ParamsFilter    = (*Logger)(nil)
+)
 
 // New returns a GORM logger that writes through log.
 func New(log *slog.Logger) *Logger {
@@ -71,6 +82,46 @@ func (l *Logger) LogMode(gormlogger.LogLevel) gormlogger.Interface {
 	return l
 }
 
+// ParamsFilter drops every bound value before GORM renders a statement
+// for the log, so what is logged is the statement's shape — its
+// placeholders — and never the values in it.
+//
+// GORM builds the string it hands to Trace by calling
+// Dialector.Explain(sql, vars...), which substitutes each value into
+// the statement. Discarding vars here leaves the '?' placeholders in
+// place, because ExplainSQL only substitutes while it still has a
+// value for the next one. That happens before Trace is reached, so it
+// holds on all three of its arms: the failed statement, the slow one,
+// and the routine one an operator sees at DEBUG.
+//
+// This is the whole of the fix, and it is deliberately unconditional
+// rather than a list of tables to redact. At first boot the two
+// statements that carry a secret are the INSERT into settings holding
+// the base64 session key — which is the entire session security model,
+// since anyone with it can mint a valid cookie — and the INSERT into
+// users holding the Argon2id hash. A denylist would have had to be
+// extended by hand for every table added afterwards, and the cost of
+// missing one is a credential in a log that gets pasted into issues.
+//
+// What is given up is the ability to read a value out of the log. The
+// statement, the table, the error and the row count are all still
+// there, which is what identifies a failing statement; reproducing it
+// needs the values, and those an operator now gets from the database
+// rather than from the log.
+//
+// One GORM path does not consult this: (*gorm.DB).Scan records the
+// statement through gorm's own traceRecorder, which does not implement
+// this interface. No production code path calls it; its one caller is
+// internal/database/database_test.go:91, whose SELECT 1 binds nothing.
+// scan_guard_test.go fails if a non-test file calls it.
+// (*gorm.DB).Pluck, Row and Raw all run through the normal callback
+// processor and are filtered.
+func (l *Logger) ParamsFilter(
+	_ context.Context, sql string, _ ...any,
+) (string, []any) {
+	return sql, nil
+}
+
 // Info logs one of GORM's own informational messages.
 func (l *Logger) Info(
 	ctx context.Context, msg string, data ...any,
@@ -93,9 +144,9 @@ func (l *Logger) Error(
 }
 
 // Trace reports the outcome of a single statement. GORM calls it for
-// every statement it runs, so the cheap paths stay cheap: fc()
+// every statement it runs, so the cheap paths stay cheap: fc() renders
-// renders the interpolated SQL and is called only on a branch that
+// the statement — with placeholders, per ParamsFilter — and is called
-// will actually emit.
+// only on a branch that will actually emit.
 //
 // The arms are ordered exactly as GORM's own Trace orders them —
 // non-record-not-found error, then slow, then the routine case — so

internal/gormlog/gormlog_test.go

@@ -232,7 +232,7 @@ func TestSlowRecordNotFound_IsStillReportedSlow(t *testing.T) {
 				require.ErrorIs(t, err, gorm.ErrRecordNotFound)
 
 				assert.Contains(
-					t, buf.String(), "slow sql statement",
+					t, buf.String(), slowLine,
 					"a slow statement that missed was not "+
 						"reported as slow",
 				)
@@ -284,9 +284,11 @@ func TestRecordNotFoundFlood_DoesNotGrowWithInput(t *testing.T) {
 }
 
 // TestStatementError_LineIsBounded covers the branch that does log.
-// A driver error is not ErrRecordNotFound, so the interpolated
+// A driver error is not ErrRecordNotFound, so the statement is
-// statement is written — and on an insert the interpolated value is
+// written, and the driver's own error text can quote what the client
-// still whatever the client supplied.
+// supplied. The statement's parameters are no longer part of that —
+// see TestBoundValues_NeverReachTheLog — but the budget is what holds
+// the line when the statement itself, or the error, is the long part.
 func TestStatementError_LineIsBounded(t *testing.T) {
 	t.Parallel()
 
@@ -313,7 +315,7 @@ func TestStatementError_LineIsBounded(t *testing.T) {
 				require.Error(t, err)
 
 				assert.Contains(
-					t, buf.String(), "sql statement failed",
+					t, buf.String(), errorLine,
 				)
 				assertBounded(t, buf.String())
 			})
@@ -329,22 +331,22 @@ func TestStatementError_LineIsBounded(t *testing.T) {
 //     and would have cost this report, which is the one thing GORM's
 //     logger gave an operator that nothing else in this service does.
 //   - routine. The branch an operator reaches by turning the level
-//     down to DEBUG: every statement is reported, so every
+//     down to DEBUG: every statement is reported, so every statement
-//     statement's interpolated parameters have to be bounded too.
+//     has to be bounded too.
 func TestSucceedingStatement_LineIsBoundedOnEitherArm(t *testing.T) {
 	t.Parallel()
 
-	// "sql statement" is a substring of "slow sql statement", so the
+	// routineLine is a substring of slowLine, so the routine arm
-	// routine arm carries notWant as well: Contains alone cannot tell
+	// carries notWant as well: Contains alone cannot tell the two arms
-	// the two arms apart in that direction.
+	// apart in that direction.
 	arms := []struct {
 		name    string
 		slow    time.Duration
 		want    string
 		notWant string
 	}{
-		{"slow", alwaysSlow, "slow sql statement", ""},
+		{"slow", alwaysSlow, slowLine, ""},
-		{"routine", neverSlow, "sql statement", "slow sql statement"},
+		{"routine", neverSlow, routineLine, slowLine},
 	}
 
 	for _, a := range arms {

internal/gormlog/scan_guard_test.go

@@ -0,0 +1,260 @@
+package gormlog_test
+
+import (
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// minNonTestFiles guards the walk below against passing because it
+// found nothing to look at. The tree held 60 non-test .go files when
+// this was written.
+const minNonTestFiles = 40
+
+// isRowProducer reports whether name is a method that returns a
+// database/sql row handle. GORM's Row and Rows return *sql.Row and
+// *sql.Rows, so Scan on the result of one of them is database/sql's
+// Scan and never (*gorm.DB).Scan.
+func isRowProducer(name string) bool {
+	switch name {
+	case "Row", "Rows", "QueryRow", "QueryRowContext":
+		return true
+	default:
+		return false
+	}
+}
+
+// receiverIsRowHandle reports whether x is syntactically a call to a
+// row producer, which is the only receiver form this check accepts for
+// a Scan.
+func receiverIsRowHandle(x ast.Expr) bool {
+	call, ok := x.(*ast.CallExpr)
+	if !ok {
+		return false
+	}
+
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return false
+	}
+
+	return isRowProducer(sel.Sel.Name)
+}
+
+// unguardedScans returns the position of every Scan call in file whose
+// receiver is not a row handle. It fails closed: a receiver it cannot
+// resolve syntactically — a local variable, a struct field — is
+// reported rather than assumed safe.
+func unguardedScans(
+	fset *token.FileSet, file *ast.File,
+) []token.Position {
+	var found []token.Position
+
+	ast.Inspect(file, func(n ast.Node) bool {
+		call, ok := n.(*ast.CallExpr)
+		if !ok {
+			return true
+		}
+
+		sel, ok := call.Fun.(*ast.SelectorExpr)
+		if !ok || sel.Sel.Name != "Scan" {
+			return true
+		}
+
+		if !receiverIsRowHandle(sel.X) {
+			found = append(found, fset.Position(sel.Sel.Pos()))
+		}
+
+		return true
+	})
+
+	return found
+}
+
+// moduleRoot walks up from the working directory to the directory
+// holding go.mod.
+func moduleRoot(t *testing.T) string {
+	t.Helper()
+
+	dir, err := os.Getwd()
+	require.NoError(t, err)
+
+	for {
+		_, statErr := os.Stat(filepath.Join(dir, "go.mod"))
+		if statErr == nil {
+			return dir
+		}
+
+		parent := filepath.Dir(dir)
+		require.NotEqual(t, parent, dir, "no go.mod above %s", dir)
+
+		dir = parent
+	}
+}
+
+// skipDir reports whether a directory holds no source this check
+// governs.
+func skipDir(name string) bool {
+	switch name {
+	case ".git", "bin", "node_modules", "testdata":
+		return true
+	default:
+		return false
+	}
+}
+
+// walkNonTestGo parses every non-test .go file under root and returns
+// how many it parsed along with every unguarded Scan it found.
+func walkNonTestGo(t *testing.T, root string) (int, []string) {
+	t.Helper()
+
+	var (
+		parsed int
+		hits   []string
+	)
+
+	fset := token.NewFileSet()
+
+	require.NoError(t, filepath.WalkDir(
+		root,
+		func(path string, d fs.DirEntry, err error) error {
+			if err != nil {
+				return err
+			}
+
+			if d.IsDir() {
+				if skipDir(d.Name()) {
+					return fs.SkipDir
+				}
+
+				return nil
+			}
+
+			if !isNonTestGo(d.Name()) {
+				return nil
+			}
+
+			file, err := parser.ParseFile(fset, path, nil, 0)
+			if err != nil {
+				return err
+			}
+
+			parsed++
+
+			for _, pos := range unguardedScans(fset, file) {
+				hits = append(hits, relPosition(root, pos))
+			}
+
+			return nil
+		},
+	))
+
+	return parsed, hits
+}
+
+// isNonTestGo reports whether a file name is Go source this check
+// governs.
+func isNonTestGo(name string) bool {
+	return strings.HasSuffix(name, ".go") &&
+		!strings.HasSuffix(name, "_test.go")
+}
+
+// relPosition renders pos with its path relative to root, so a failure
+// names the file the way the repository does.
+func relPosition(root string, pos token.Position) string {
+	name := pos.Filename
+
+	rel, err := filepath.Rel(root, name)
+	if err == nil {
+		name = rel
+	}
+
+	return fmt.Sprintf("%s:%d:%d", name, pos.Line, pos.Column)
+}
+
+// TestGormScanIsNeverCalledOutsideTests keeps (*gorm.DB).Scan out of
+// non-test code.
+//
+// It is the one statement path (*Logger).ParamsFilter does not reach:
+// Scan swaps GORM's own trace recorder in for the adapter, and that
+// recorder does not implement gorm.ParamsFilter, so the statement is
+// logged with its values interpolated. The package comment states the
+// limit; this fails when someone adds a call site anyway.
+//
+// The current tree has one caller, internal/database/database_test.go,
+// which this check does not govern: it is test-only and its SELECT 1
+// binds nothing.
+func TestGormScanIsNeverCalledOutsideTests(t *testing.T) {
+	t.Parallel()
+
+	parsed, offenders := walkNonTestGo(t, moduleRoot(t))
+
+	require.GreaterOrEqual(
+		t, parsed, minNonTestFiles,
+		"parsed %d non-test .go files, so this check found "+
+			"nothing to look at", parsed,
+	)
+	require.Empty(
+		t, offenders,
+		"Scan called on a receiver this check cannot show is a "+
+			"database/sql row handle. (*gorm.DB).Scan logs the "+
+			"statement with its bound values interpolated — use "+
+			"Find, Pluck, or Raw(...).Row().Scan instead. A "+
+			"database/sql Scan reached through a variable is "+
+			"reported too; write it as <producer>().Scan rather "+
+			"than widening this check.",
+	)
+}
+
+// scanGuardCase is one planted snippet and whether the check above
+// should report it.
+type scanGuardCase struct {
+	name string
+	body string
+	want int
+}
+
+func scanGuardCases() []scanGuardCase {
+	return []scanGuardCase{
+		{"gorm chain", `db.DB().Raw("SELECT 1").Scan(&v)`, 1},
+		{"gorm receiver", `gdb.Scan(&v)`, 1},
+		{"gorm via variable", "q := gdb.Raw(\"x\")\nq.Scan(&v)", 1},
+		{"gorm model chain", `gdb.Model(&x).Scan(&v)`, 1},
+		{"sql row", `gdb.Raw("SELECT 1").Row().Scan(&v)`, 0},
+		{"sql rows", `gdb.Raw("SELECT 1").Rows().Scan(&v)`, 0},
+		{"unrelated call", `gdb.Find(&v)`, 0},
+	}
+}
+
+// TestScanGuard_ReportsPlantedCalls proves the check fires. Without it
+// a detector that matched nothing would satisfy the walk above no
+// matter what the tree contained.
+func TestScanGuard_ReportsPlantedCalls(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range scanGuardCases() {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			fset := token.NewFileSet()
+			src := fmt.Sprintf(
+				"package p\n\nfunc f() {\n\t%s\n}\n", tc.body,
+			)
+
+			file, err := parser.ParseFile(
+				fset, tc.name+".go", src, 0,
+			)
+			require.NoError(t, err)
+
+			require.Len(t, unguardedScans(fset, file), tc.want)
+		})
+	}
+}

internal/gormlog/values_test.go

@@ -0,0 +1,185 @@
+package gormlog_test
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gorm.io/gorm"
+)
+
+// secretValue is bound as a parameter by every case below. Nothing
+// else in this package writes it, so finding it in captured output
+// means a bound value was rendered into the log.
+const secretValue = "QQBOUNDVALUEMARKERQQ"
+
+// The three messages Trace emits under, one per arm.
+//
+// routineLine is a substring of slowLine, so a case that wants the
+// routine arm has to rule the slow one out as well rather than rely on
+// Contains alone.
+const (
+	routineLine = "sql statement"
+	slowLine    = "slow sql statement"
+	errorLine   = "sql statement failed"
+)
+
+// boundValueCase is one arm of Trace, driven by a statement that binds
+// secretValue.
+type boundValueCase struct {
+	name  string
+	slow  time.Duration
+	want  string
+	drive func(t *testing.T, gdb *gorm.DB)
+}
+
+// insertSecret returns a driver that inserts one row whose Name is the
+// secret.
+func insertSecret(id string) func(*testing.T, *gorm.DB) {
+	return func(t *testing.T, gdb *gorm.DB) {
+		t.Helper()
+
+		require.NoError(t, gdb.Create(&thing{
+			ID: id, Name: secretValue,
+		}).Error)
+	}
+}
+
+// insertSecretTwice drives the error arm: the same primary key a
+// second time is a UNIQUE constraint failure, which is an error GORM
+// logs with the statement.
+func insertSecretTwice(t *testing.T, gdb *gorm.DB) {
+	t.Helper()
+
+	require.NoError(t, gdb.Create(&thing{
+		ID: secretValue, Name: secretValue,
+	}).Error)
+	require.Error(t, gdb.Create(&thing{
+		ID: secretValue, Name: "other",
+	}).Error)
+}
+
+// selectSecret drives a query whose WHERE clause binds the secret,
+// covering the read side as well as the write side.
+func selectSecret(t *testing.T, gdb *gorm.DB) {
+	t.Helper()
+
+	var got []thing
+
+	require.NoError(
+		t, gdb.Where("name = ?", secretValue).Find(&got).Error,
+	)
+}
+
+func boundValueCases() []boundValueCase {
+	return []boundValueCase{
+		{
+			name: "routine", slow: neverSlow,
+			want: routineLine, drive: insertSecret("routine"),
+		},
+		{
+			name: "slow", slow: alwaysSlow,
+			want: slowLine, drive: insertSecret("slow"),
+		},
+		{
+			name: "error", slow: neverSlow,
+			want: errorLine, drive: insertSecretTwice,
+		},
+		{
+			name: "select", slow: neverSlow,
+			want: routineLine, drive: selectSecret,
+		},
+	}
+}
+
+// TestBoundValues_NeverReachTheLog states the values-off property
+// directly, on each arm of Trace that emits.
+//
+// Truncation is not what is being asserted. A bounded secret is still
+// a secret: the session key is 44 base64 characters and an Argon2id
+// hash under 100, so both fit inside every budget this package
+// applies. What keeps them out is that the adapter logs the
+// statement's shape and discards its parameters — see
+// (*Logger).ParamsFilter — and that has to hold at DEBUG as much as on
+// an error, because DEBUG is the level at which a successful INSERT is
+// written at all.
+//
+// Each case also requires a placeholder in the logged statement.
+// Without that, the absence of the value would be satisfied by a
+// logger that wrote nothing useful.
+func TestBoundValues_NeverReachTheLog(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range boundValueCases() {
+		for _, h := range handlers() {
+			t.Run(tc.name+"/"+h.name, func(t *testing.T) {
+				t.Parallel()
+
+				var buf bytes.Buffer
+
+				gdb := openDB(t, &buf, h.make(&buf), tc.slow)
+
+				tc.drive(t, gdb)
+
+				assertNoBoundValue(t, buf.String(), tc.want)
+			})
+		}
+	}
+}
+
+// assertNoBoundValue holds one captured arm to the property: it wrote
+// the line it was supposed to write, that line kept its placeholders,
+// and it carried no bound value.
+func assertNoBoundValue(t *testing.T, out, want string) {
+	t.Helper()
+
+	require.Contains(
+		t, out, want,
+		"the arm under test wrote nothing, so the assertions "+
+			"below are vacuous",
+	)
+	assert.NotContains(
+		t, out, secretValue,
+		"a bound parameter was rendered into the log",
+	)
+	assert.Contains(
+		t, out, "?",
+		"the statement was logged without its placeholders",
+	)
+}
+
+// TestInsert_KeepsOnePlaceholderPerBoundValue pins the shape of the
+// INSERT specifically, since that is the statement that carries both
+// first-boot secrets. A statement that dropped one value and kept the
+// other would satisfy the assertions above.
+func TestInsert_KeepsOnePlaceholderPerBoundValue(t *testing.T) {
+	t.Parallel()
+
+	var buf bytes.Buffer
+
+	gdb := openDB(t, &buf, handlers()[0].make(&buf), neverSlow)
+
+	require.NoError(t, gdb.Create(&thing{
+		ID: "m", Name: secretValue,
+	}).Error)
+
+	out := buf.String()
+
+	require.Contains(t, out, "INSERT INTO")
+	assert.NotContains(t, out, secretValue)
+
+	for line := range strings.SplitSeq(out, "\n") {
+		if !strings.Contains(line, "INSERT INTO") {
+			continue
+		}
+
+		assert.GreaterOrEqual(
+			t, strings.Count(line, "?"), 2,
+			"insert logged fewer placeholders than it bound "+
+				"values: %s", line,
+		)
+	}
+}

internal/handlers/export_test.go

@@ -69,18 +69,29 @@ func (s *Handlers) RenderTemplateForTest(
 	s.renderTemplate(w, r, pageTemplate, data)
 }
 
-// BuildSlackTargetConfigForTest exposes buildURLTargetConfig
+// BuildSlackTargetConfigForTest exposes
-// with the Slack target parameters for use in the
+// buildSlackTargetConfig for use in the handlers_test package.
-// handlers_test package.
 func (s *Handlers) BuildSlackTargetConfigForTest(
 	w http.ResponseWriter,
 	r *http.Request,
 	targetURL string,
 ) (string, error) {
-	return s.buildURLTargetConfig(
+	return s.buildSlackTargetConfig(w, r, targetURL)
-		w, r, targetURL, "webhookUrl",
+}
-		"Webhook URL is required for Slack targets",
+
-	)
+// BuildHTTPTargetConfigForTest exposes buildHTTPTargetConfig
+// for use in the handlers_test package, taking the form fields
+// an HTTP target's configuration is built from.
+func (s *Handlers) BuildHTTPTargetConfigForTest(
+	w http.ResponseWriter,
+	r *http.Request,
+	targetURL, headers, timeout string,
+) (string, error) {
+	return s.buildHTTPTargetConfig(w, r, targetFormInput{
+		URL:     targetURL,
+		Headers: headers,
+		Timeout: timeout,
+	})
 }
 
 // BuildDatabaseTargetConfigForTest exposes

internal/handlers/handlers.go

@@ -18,6 +18,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/globals"
 	"sneak.berlin/go/webhooker/internal/healthcheck"
 	"sneak.berlin/go/webhooker/internal/logger"
+	"sneak.berlin/go/webhooker/internal/metrics"
 	"sneak.berlin/go/webhooker/internal/middleware"
 	"sneak.berlin/go/webhooker/internal/session"
 	"sneak.berlin/go/webhooker/templates"
@@ -73,6 +74,7 @@ type Handlers struct {
 	mw        *middleware.Middleware
 	notifier  delivery.Notifier
 	evictor   delivery.WebhookEvictor
+	mtr       *metrics.Set
 	templates map[string]*template.Template
 
 	// dummyVerifications counts the equivalent-cost verifications
@@ -114,6 +116,7 @@ func New(
 	s.mw = params.Middleware
 	s.notifier = params.Notifier
 	s.evictor = params.Evictor
+	s.mtr = metrics.Default()
 
 	// Parse all page templates once at startup
 	s.templates = map[string]*template.Template{
@@ -124,6 +127,7 @@ func New(
 		"source_detail.html": parsePageTemplate("source_detail.html"),
 		"source_edit.html":   parsePageTemplate("source_edit.html"),
 		"source_logs.html":   parsePageTemplate("source_logs.html"),
+		"target_edit.html":   parsePageTemplate("target_edit.html"),
 	}
 
 	lc.Append(fx.Hook{

internal/handlers/source_management.go

@@ -1029,9 +1029,7 @@ func (h *Handlers) processTargetCreate(
 	// Referer headers and error trackers record.
 	name := r.PostFormValue("name")
 	targetType := database.TargetType(r.PostFormValue("type"))
-	targetURL := r.PostFormValue("url")
 	maxRetriesStr := r.PostFormValue("max_retries")
-	expiry := r.PostFormValue("expiry")
 
 	if name == "" {
 		http.Error(
@@ -1051,7 +1049,7 @@ func (h *Handlers) processTargetCreate(
 	}
 
 	configJSON, err := h.buildTargetConfig(
-		w, r, targetType, targetURL, expiry,
+		w, r, targetType, targetFormInputFrom(r),
 	)
 	if err != nil {
 		return
@@ -1108,28 +1106,60 @@ func parseNonNegativeInt(s string) int {
 	return 0
 }
 
-// buildTargetConfig builds the JSON config string for a target.
+// targetFormInput carries the raw form values describing a target's
-// The expiry form value is read by the caller (which bounds the
+// configuration. Both the create and the edit path fill one and hand
-// request body) and applies to database targets only.
+// it to buildTargetConfig, so neither can come to validate a
+// destination differently from the other.
+type targetFormInput struct {
+	// URL is the destination for an HTTP target and the webhook URL
+	// for a Slack target.
+	URL string
+	// Headers is an HTTP target's headers, one "Name: value" per
+	// line.
+	Headers string
+	// Timeout is an HTTP target's per-request timeout in seconds.
+	Timeout string
+	// Expiry is a database (archive) target's row expiry.
+	Expiry string
+}
+
+// targetFormInputFrom reads the configuration fields from a request
+// body. The body size cap is enforced by the MaxBodySize middleware,
+// which runs before CSRF parses the form.
+//
+// Every field is read with PostFormValue, not FormValue. FormValue
+// falls back to the query string, which would let
+// `POST /source/{id}/targets?url=https://hooks.slack.com/...`
+// configure a target from a value the request line carries — and the
+// request line, unlike the body, is what logs, proxies, Referer
+// headers and error trackers record. The headers field is under the
+// same rule and for the same reason: its values are authorization
+// tokens.
+func targetFormInputFrom(r *http.Request) targetFormInput {
+	return targetFormInput{
+		URL:     r.PostFormValue("url"),
+		Headers: r.PostFormValue("headers"),
+		Timeout: r.PostFormValue("timeout"),
+		Expiry:  r.PostFormValue("expiry"),
+	}
+}
+
+// buildTargetConfig builds the JSON config string for a target from
+// the submitted form values, writing its own 4xx response on
+// rejection. Which fields of in apply depends on the target type.
 func (h *Handlers) buildTargetConfig(
 	w http.ResponseWriter,
 	r *http.Request,
 	targetType database.TargetType,
-	targetURL, expiry string,
+	in targetFormInput,
 ) (string, error) {
 	switch targetType {
 	case database.TargetTypeHTTP:
-		return h.buildURLTargetConfig(
+		return h.buildHTTPTargetConfig(w, r, in)
-			w, r, targetURL, "url",
-			"URL is required for HTTP targets",
-		)
 	case database.TargetTypeSlack:
-		return h.buildURLTargetConfig(
+		return h.buildSlackTargetConfig(w, r, in.URL)
-			w, r, targetURL, "webhookUrl",
-			"Webhook URL is required for Slack targets",
-		)
 	case database.TargetTypeDatabase:
-		return h.buildDatabaseTargetConfig(w, expiry)
+		return h.buildDatabaseTargetConfig(w, in.Expiry)
 	case database.TargetTypeLog:
 		return "", nil
 	default:
@@ -1142,14 +1172,83 @@ func (h *Handlers) buildTargetConfig(
 	}
 }
 
-// buildURLTargetConfig builds config JSON for a target whose
+// buildHTTPTargetConfig builds config JSON for an HTTP target: an
-// configuration is a single SSRF-validated URL stored under
+// SSRF-validated destination plus the optional headers and timeout
-// configKey. missingMsg is the error shown when no URL is given.
+// the delivery path honours.
-func (h *Handlers) buildURLTargetConfig(
+func (h *Handlers) buildHTTPTargetConfig(
 	w http.ResponseWriter,
 	r *http.Request,
-	targetURL, configKey, missingMsg string,
+	in targetFormInput,
 ) (string, error) {
+	err := h.validateTargetURL(
+		w, r, in.URL, "URL is required for HTTP targets",
+	)
+	if err != nil {
+		return "", err
+	}
+
+	headers, err := delivery.ParseTargetHeaders(in.Headers)
+	if err != nil {
+		http.Error(
+			w,
+			"Invalid headers: "+err.Error(),
+			http.StatusBadRequest,
+		)
+
+		return "", err
+	}
+
+	timeout, err := delivery.ParseTargetTimeout(in.Timeout)
+	if err != nil {
+		http.Error(
+			w,
+			"Invalid timeout: "+err.Error(),
+			http.StatusBadRequest,
+		)
+
+		return "", err
+	}
+
+	return marshalTargetConfig(w, delivery.HTTPTargetConfig{
+		URL:     in.URL,
+		Headers: headers,
+		Timeout: timeout,
+	})
+}
+
+// buildSlackTargetConfig builds config JSON for a Slack target,
+// whose whole configuration is one SSRF-validated webhook URL.
+func (h *Handlers) buildSlackTargetConfig(
+	w http.ResponseWriter,
+	r *http.Request,
+	targetURL string,
+) (string, error) {
+	err := h.validateTargetURL(
+		w, r, targetURL,
+		"Webhook URL is required for Slack targets",
+	)
+	if err != nil {
+		return "", err
+	}
+
+	return marshalTargetConfig(w, delivery.SlackTargetConfig{
+		WebhookURL: targetURL,
+	})
+}
+
+// validateTargetURL rejects an empty or SSRF-blocked destination,
+// writing the 400 itself. missingMsg is the error shown when no URL
+// is given.
+//
+// It is the single point at which a user-supplied destination enters
+// the SSRF guard, on create and on edit alike. An edit path that
+// reached storage without passing through here would reopen the hole
+// the guard closes.
+func (h *Handlers) validateTargetURL(
+	w http.ResponseWriter,
+	r *http.Request,
+	targetURL, missingMsg string,
+) error {
 	if targetURL == "" {
 		http.Error(
 			w,
@@ -1157,7 +1256,7 @@ func (h *Handlers) buildURLTargetConfig(
 			http.StatusBadRequest,
 		)
 
-		return "", errMissingURL
+		return errMissingURL
 	}
 
 	err := delivery.ValidateTargetURL(
@@ -1178,11 +1277,18 @@ func (h *Handlers) buildURLTargetConfig(
 			http.StatusBadRequest,
 		)
 
-		return "", err
+		return err
 	}
 
-	cfg := map[string]any{configKey: targetURL}
+	return nil
+}
 
+// marshalTargetConfig serialises a target configuration for storage,
+// writing a 500 itself if it cannot.
+func marshalTargetConfig(
+	w http.ResponseWriter,
+	cfg any,
+) (string, error) {
 	configBytes, err := json.Marshal(cfg)
 	if err != nil {
 		http.Error(
@@ -1222,19 +1328,9 @@ func (h *Handlers) buildDatabaseTargetConfig(
 		return "", err
 	}
 
-	cfg := map[string]any{"expiry": expiry}
+	return marshalTargetConfig(
-
+		w, map[string]any{"expiry": expiry},
-	configBytes, err := json.Marshal(cfg)
+	)
-	if err != nil {
-		http.Error(
-			w, "Internal server error",
-			http.StatusInternalServerError,
-		)
-
-		return "", err
-	}
-
-	return string(configBytes), nil
 }
 
 // HandleEntrypointDelete handles deleting an entrypoint.

internal/handlers/target_edit.go

@@ -0,0 +1,221 @@
+package handlers
+
+import (
+	"net/http"
+
+	"github.com/go-chi/chi"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/delivery"
+)
+
+// targetEditTemplate is the page the target edit form renders.
+const targetEditTemplate = "target_edit.html"
+
+// tmplKeyTarget is the template data key for the target being
+// edited, and tmplKeyMaxTimeout for the timeout ceiling the form
+// tells the user about.
+const (
+	tmplKeyTarget     = "Target"
+	tmplKeyMaxTimeout = "MaxTimeout"
+)
+
+// configUnreadableMessage is shown when a target's stored
+// configuration does not parse. It says plainly that saving replaces
+// the stored value rather than preserving it, because the form
+// cannot pre-fill what it could not read.
+const configUnreadableMessage = "The stored configuration for this " +
+	"target could not be read. Enter the values below; saving " +
+	"replaces the stored configuration."
+
+// targetEditView is the display model for the target edit page.
+//
+// It carries the target's row fields alongside its UNMASKED
+// configuration, and deliberately omits database.Target's raw
+// Config blob: the form renders named fields, and giving the
+// template the blob as well would put an unreviewed second path to
+// the credential on the page.
+type targetEditView struct {
+	ID         string
+	Name       string
+	Type       database.TargetType
+	Active     bool
+	MaxRetries int
+	Config     delivery.TargetConfigForm
+}
+
+// HandleTargetEdit shows the form to edit a target.
+//
+// This page is the one place the full destination URL and header
+// values are shown. It is reachable only through the
+// /source/{sourceID} route group, which supplies RequireAuth and
+// NoCache, and only for a target of a webhook the session's user
+// owns; masking (delivery.TargetView) is unchanged everywhere else.
+func (h *Handlers) HandleTargetEdit() http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		webhook, target, ok := h.ownedTarget(w, r)
+		if !ok {
+			return
+		}
+
+		cfg, err := delivery.NewTargetConfigForm(target)
+		msg := ""
+
+		if err != nil {
+			// The error carries the parse failure, never the
+			// blob, so it is safe to log against the target id.
+			h.log.Warn(
+				"stored target config could not be read for editing",
+				"target_id", target.ID,
+				"error", err,
+			)
+
+			msg = configUnreadableMessage
+		}
+
+		h.renderTargetEdit(w, r, webhook, target, cfg, msg)
+	}
+}
+
+// HandleTargetEditSubmit handles the target edit form submission.
+func (h *Handlers) HandleTargetEditSubmit() http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		webhook, target, ok := h.ownedTarget(w, r)
+		if !ok {
+			return
+		}
+
+		// The body size cap is enforced by the MaxBodySize
+		// middleware, which runs before CSRF parses the form.
+		err := r.ParseForm()
+		if err != nil {
+			http.Error(
+				w, "Bad request", http.StatusBadRequest,
+			)
+
+			return
+		}
+
+		h.applyTargetEdit(w, r, webhook, target)
+	}
+}
+
+// applyTargetEdit validates and saves target edits.
+//
+// The submitted configuration goes through buildTargetConfig, the
+// same builder the create path uses, so an edited destination is
+// SSRF-validated exactly as a new one is.
+//
+// The target's type is not editable. Each type stores a different
+// configuration shape and its delivery history is recorded against
+// the target row, so changing the type of an existing target is
+// really the creation of a different one. The stored type decides
+// which fields the form offers and which builder runs.
+func (h *Handlers) applyTargetEdit(
+	w http.ResponseWriter,
+	r *http.Request,
+	webhook database.Webhook,
+	target *database.Target,
+) {
+	name := r.PostFormValue("name")
+	if name == "" {
+		http.Error(
+			w, "Name is required", http.StatusBadRequest,
+		)
+
+		return
+	}
+
+	configJSON, err := h.buildTargetConfig(
+		w, r, target.Type, targetFormInputFrom(r),
+	)
+	if err != nil {
+		// buildTargetConfig has already written the response.
+		return
+	}
+
+	target.Name = name
+	target.Config = configJSON
+
+	// Retries are offered only by the forms for target types that
+	// retry, so an absent field means "this form does not edit
+	// retries" rather than "set them to zero". Reading it
+	// unconditionally would silently disable retries on any target
+	// saved from a form that does not render the input.
+	if r.PostForm.Has("max_retries") {
+		target.MaxRetries = parseNonNegativeInt(
+			r.PostFormValue("max_retries"),
+		)
+	}
+
+	err = h.db.DB().Save(target).Error
+	if err != nil {
+		h.serverError(w, "failed to update target", err)
+
+		return
+	}
+
+	http.Redirect(
+		w, r, "/source/"+webhook.ID, http.StatusSeeOther,
+	)
+}
+
+// renderTargetEdit renders the target edit page with an optional
+// error message.
+func (h *Handlers) renderTargetEdit(
+	w http.ResponseWriter,
+	r *http.Request,
+	webhook database.Webhook,
+	target *database.Target,
+	cfg delivery.TargetConfigForm,
+	errMsg string,
+) {
+	// The template calls Webhook methods, which take pointer
+	// receivers; html/template cannot address a value stored in a
+	// map.
+	data := map[string]any{
+		tmplKeyWebhook: &webhook,
+		tmplKeyTarget: targetEditView{
+			ID:         target.ID,
+			Name:       target.Name,
+			Type:       target.Type,
+			Active:     target.Active,
+			MaxRetries: target.MaxRetries,
+			Config:     cfg,
+		},
+		tmplKeyMaxTimeout: delivery.MaxTargetTimeoutSeconds,
+		tmplKeyError:      errMsg,
+	}
+
+	h.renderTemplate(w, r, targetEditTemplate, data)
+}
+
+// ownedTarget resolves the request's sourceID and targetID
+// parameters to a target of a webhook the session's user owns.
+//
+// Ownership is decided by the webhook, and the target is then
+// scoped to that webhook, so a target id belonging to someone
+// else's webhook is a 404 rather than an edit of their target. It
+// reports false once it has written the response.
+func (h *Handlers) ownedTarget(
+	w http.ResponseWriter,
+	r *http.Request,
+) (database.Webhook, *database.Target, bool) {
+	webhook, ok := h.ownedWebhook(w, r)
+	if !ok {
+		return database.Webhook{}, nil, false
+	}
+
+	var target database.Target
+
+	err := h.db.DB().Where(
+		"id = ? AND webhook_id = ?",
+		chi.URLParam(r, "targetID"), webhook.ID,
+	).First(&target).Error
+	if err != nil {
+		http.NotFound(w, r)
+
+		return database.Webhook{}, nil, false
+	}
+
+	return webhook, &target, true
+}

internal/handlers/target_edit_test.go

@@ -0,0 +1,637 @@
+package handlers_test
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/go-chi/chi"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gorm.io/gorm/clause"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/delivery"
+)
+
+// The destinations the target edit tests configure. Both are literal
+// public addresses rather than hostnames so the SSRF check resolves
+// nothing: with a hostname, a sandbox without DNS would reject the
+// URL for the wrong reason and a test asserting rejection would pass
+// even with the guard removed.
+const (
+	editOriginalURL = "https://93.184.216.34/hooks/original"
+	editReplacedURL = "https://93.184.216.34/hooks/replaced"
+	// editBlockedURL resolves to loopback, which the SSRF guard
+	// refuses. It is what proves the guard runs on the edit path.
+	editBlockedURL = "http://127.0.0.1/hooks/internal"
+)
+
+// editAuthHeader carries a bearer credential, the case the headers
+// field exists for.
+const (
+	editBearerSecret = "QQEDITSECRETQQ"
+	editAuthHeader   = "Authorization: Bearer " + editBearerSecret
+)
+
+// targetRouter mounts the target create and edit routes on a chi
+// router so the handlers see the URL parameters they read.
+func targetRouter(env *sourceTestEnv) *chi.Mux {
+	router := chi.NewRouter()
+	router.Post(
+		"/source/{sourceID}/targets",
+		env.handlers.HandleTargetCreate(),
+	)
+	router.Get(
+		"/source/{sourceID}/targets/{targetID}/edit",
+		env.handlers.HandleTargetEdit(),
+	)
+	router.Post(
+		"/source/{sourceID}/targets/{targetID}/edit",
+		env.handlers.HandleTargetEditSubmit(),
+	)
+
+	return router
+}
+
+// serveTarget drives one request through the target routes as the
+// authenticated test user.
+func serveTarget(
+	env *sourceTestEnv,
+	method, path string,
+	form url.Values,
+) *httptest.ResponseRecorder {
+	body := ""
+	if form != nil {
+		body = form.Encode()
+	}
+
+	req := httptest.NewRequestWithContext(
+		context.Background(), method, path,
+		strings.NewReader(body),
+	)
+	if form != nil {
+		req.Header.Set(
+			"Content-Type",
+			"application/x-www-form-urlencoded",
+		)
+	}
+
+	for _, c := range env.cookies {
+		req.AddCookie(c)
+	}
+
+	w := httptest.NewRecorder()
+	targetRouter(env).ServeHTTP(w, req)
+
+	return w
+}
+
+// seedHTTPTarget creates a webhook and an HTTP target on it through
+// the real create handler, so every case starts from a target the
+// production path produced rather than a hand-written row.
+//
+// Standing the fx app up is what a handler test mostly costs, and
+// internal/handlers is already the slowest package in the suite, so
+// the tests below share one env per test function and give each case
+// its own webhook rather than its own app.
+func seedHTTPTarget(
+	t *testing.T,
+	env *sourceTestEnv,
+	headers, timeout string,
+) (database.Webhook, database.Target) {
+	t.Helper()
+
+	webhook := seedWebhookWithRetention(t, env.db, 30)
+
+	form := url.Values{}
+	form.Set("name", "original-name")
+	form.Set("type", string(database.TargetTypeHTTP))
+	form.Set("url", editOriginalURL)
+	form.Set("headers", headers)
+	form.Set("timeout", timeout)
+	form.Set("max_retries", "3")
+
+	w := serveTarget(
+		env, http.MethodPost,
+		"/source/"+webhook.ID+"/targets", form,
+	)
+	require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
+
+	targets := targetsForWebhook(t, env.db, webhook.ID)
+	require.Len(t, targets, 1)
+
+	return webhook, targets[0]
+}
+
+// storedTarget reloads a target row.
+func storedTarget(
+	t *testing.T,
+	env *sourceTestEnv,
+	targetID string,
+) database.Target {
+	t.Helper()
+
+	var target database.Target
+
+	require.NoError(
+		t,
+		env.db.DB().Where("id = ?", targetID).
+			First(&target).Error,
+	)
+
+	return target
+}
+
+// storedHTTPConfig reloads a target and parses its stored HTTP
+// configuration.
+func storedHTTPConfig(
+	t *testing.T,
+	env *sourceTestEnv,
+	targetID string,
+) delivery.HTTPTargetConfig {
+	t.Helper()
+
+	var cfg delivery.HTTPTargetConfig
+
+	require.NoError(
+		t,
+		json.Unmarshal(
+			[]byte(storedTarget(t, env, targetID).Config), &cfg,
+		),
+	)
+
+	return cfg
+}
+
+// editForm is the fully populated edit submission for an HTTP
+// target.
+func editForm(targetURL, headers, timeout string) url.Values {
+	form := url.Values{}
+	form.Set("name", "edited-name")
+	form.Set("url", targetURL)
+	form.Set("headers", headers)
+	form.Set("timeout", timeout)
+	form.Set("max_retries", "5")
+
+	return form
+}
+
+// submitTargetEdit posts the edit form for a target.
+func submitTargetEdit(
+	env *sourceTestEnv,
+	webhookID, targetID string,
+	form url.Values,
+) *httptest.ResponseRecorder {
+	return serveTarget(
+		env, http.MethodPost,
+		"/source/"+webhookID+"/targets/"+targetID+"/edit",
+		form,
+	)
+}
+
+// TestHandleTargetCreate_Configuration covers the half of the gap
+// that is not about editing at all: HTTPTargetConfig has carried
+// Headers and Timeout, and the delivery path has honoured them, but
+// the create form wrote {"url":...} and nothing else, so a
+// destination needing an Authorization header could not be
+// configured through the UI at all.
+func TestHandleTargetCreate_Configuration(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	t.Run("stores headers and timeout", func(t *testing.T) {
+		t.Parallel()
+		assertCreateStoresHeadersAndTimeout(t, env)
+	})
+
+	t.Run("without them keeps a url-only config", func(t *testing.T) {
+		t.Parallel()
+		assertCreateKeepsURLOnlyConfig(t, env)
+	})
+}
+
+func assertCreateStoresHeadersAndTimeout(
+	t *testing.T, env *sourceTestEnv,
+) {
+	t.Helper()
+
+	_, target := seedHTTPTarget(
+		t, env, editAuthHeader+"\nX-Tenant: acme\n", "12",
+	)
+
+	cfg := storedHTTPConfig(t, env, target.ID)
+
+	assert.Equal(t, editOriginalURL, cfg.URL)
+	assert.Equal(t, 12, cfg.Timeout)
+	assert.Equal(
+		t,
+		map[string]string{
+			"Authorization": "Bearer " + editBearerSecret,
+			"X-Tenant":      "acme",
+		},
+		cfg.Headers,
+	)
+}
+
+// Without the new fields the stored shape must be the same
+// {"url":...} the create form wrote before they existed, so no
+// existing target's configuration is rewritten by this change.
+func assertCreateKeepsURLOnlyConfig(
+	t *testing.T, env *sourceTestEnv,
+) {
+	t.Helper()
+
+	_, target := seedHTTPTarget(t, env, "", "")
+
+	assert.JSONEq(
+		t, `{"url":"`+editOriginalURL+`"}`, target.Config,
+	)
+}
+
+// TestHandleTargetEditSubmit_Saves is the round trip the issue asks
+// for: create a target, edit it, and confirm the stored config
+// changed.
+func TestHandleTargetEditSubmit_Saves(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	t.Run("changes the destination URL", func(t *testing.T) {
+		t.Parallel()
+		assertEditChangesDestination(t, env)
+	})
+
+	t.Run("round trips headers and timeout", func(t *testing.T) {
+		t.Parallel()
+		assertEditRoundTripsHeadersAndTimeout(t, env)
+	})
+
+	t.Run("clearing them removes them", func(t *testing.T) {
+		t.Parallel()
+		assertEditClearingRemovesThem(t, env)
+	})
+
+	t.Run("absent max_retries is not zeroed", func(t *testing.T) {
+		t.Parallel()
+		assertEditKeepsAbsentMaxRetries(t, env)
+	})
+}
+
+func assertEditChangesDestination(
+	t *testing.T, env *sourceTestEnv,
+) {
+	t.Helper()
+
+	webhook, target := seedHTTPTarget(t, env, "", "")
+
+	w := submitTargetEdit(
+		env, webhook.ID, target.ID,
+		editForm(editReplacedURL, "", ""),
+	)
+	require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
+
+	assert.Equal(
+		t,
+		editReplacedURL,
+		storedHTTPConfig(t, env, target.ID).URL,
+	)
+
+	reloaded := storedTarget(t, env, target.ID)
+	assert.Equal(t, "edited-name", reloaded.Name)
+	assert.Equal(t, 5, reloaded.MaxRetries)
+	assert.Equal(
+		t, database.TargetTypeHTTP, reloaded.Type,
+		"the edit form must not change a target's type",
+	)
+}
+
+// The two previously unreachable fields must survive create,
+// pre-fill and save.
+func assertEditRoundTripsHeadersAndTimeout(
+	t *testing.T, env *sourceTestEnv,
+) {
+	t.Helper()
+
+	webhook, target := seedHTTPTarget(t, env, editAuthHeader, "7")
+
+	w := submitTargetEdit(
+		env, webhook.ID, target.ID,
+		editForm(
+			editOriginalURL,
+			"Authorization: Bearer rotated\nX-Trace: on",
+			"21",
+		),
+	)
+	require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
+
+	cfg := storedHTTPConfig(t, env, target.ID)
+
+	assert.Equal(t, 21, cfg.Timeout)
+	assert.Equal(
+		t,
+		map[string]string{
+			"Authorization": "Bearer rotated",
+			"X-Trace":       "on",
+		},
+		cfg.Headers,
+	)
+}
+
+// The direction a naive "only set what was submitted" implementation
+// gets wrong: an emptied field must remove the stored value, not
+// leave the previous one in place.
+func assertEditClearingRemovesThem(
+	t *testing.T, env *sourceTestEnv,
+) {
+	t.Helper()
+
+	webhook, target := seedHTTPTarget(t, env, editAuthHeader, "7")
+
+	w := submitTargetEdit(
+		env, webhook.ID, target.ID,
+		editForm(editOriginalURL, "", ""),
+	)
+	require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
+
+	cfg := storedHTTPConfig(t, env, target.ID)
+
+	assert.Empty(t, cfg.Headers)
+	assert.Zero(t, cfg.Timeout)
+}
+
+// Retries are offered only by the forms for target types that retry.
+// An absent field means the form does not edit retries, not that
+// they should be turned off.
+func assertEditKeepsAbsentMaxRetries(
+	t *testing.T, env *sourceTestEnv,
+) {
+	t.Helper()
+
+	webhook, target := seedHTTPTarget(t, env, "", "")
+	require.Equal(t, 3, target.MaxRetries)
+
+	form := editForm(editOriginalURL, "", "")
+	form.Del("max_retries")
+
+	w := submitTargetEdit(env, webhook.ID, target.ID, form)
+	require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
+
+	assert.Equal(
+		t, 3, storedTarget(t, env, target.ID).MaxRetries,
+	)
+}
+
+// TestHandleTargetEdit_PrefillsTheStoredValuesUnmasked covers the
+// deliberate exception to the masking rule. The operator cannot
+// correct a value they cannot see, so this page — and only this page
+// — renders the destination and the header values in full.
+func TestHandleTargetEdit_PrefillsTheStoredValuesUnmasked(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	webhook, target := seedHTTPTarget(t, env, editAuthHeader, "7")
+
+	w := serveTarget(
+		env, http.MethodGet,
+		"/source/"+webhook.ID+"/targets/"+target.ID+"/edit",
+		nil,
+	)
+	require.Equal(t, http.StatusOK, w.Code)
+
+	page := w.Body.String()
+
+	assert.Contains(t, page, editOriginalURL)
+	assert.Contains(t, page, "Bearer "+editBearerSecret)
+	assert.Contains(t, page, `value="7"`)
+	assert.Contains(t, page, "original-name")
+}
+
+// TestHandleTargetEditSubmit_Rejects covers every submission that
+// must not reach storage.
+//
+// The SSRF case is the most important assertion on this change: the
+// edited destination goes through the same guard the create path
+// uses. An edit that stored an unvalidated URL would reopen a closed
+// hole, since a target could then be created public and edited to
+// point at loopback.
+//
+// The header and timeout cases keep input that could not be
+// delivered as written out of storage: a stored value that provably
+// never reaches the wire reports a configuration that did not take
+// effect.
+func TestHandleTargetEditSubmit_Rejects(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	t.Run("an SSRF-blocked destination", func(t *testing.T) {
+		t.Parallel()
+		assertEditRejectsBlockedDestination(t, env)
+	})
+
+	t.Run("a query-string destination", func(t *testing.T) {
+		t.Parallel()
+		assertEditIgnoresQueryString(t, env)
+	})
+
+	headerCases := map[string]string{
+		"no colon":        "Authorization Bearer token",
+		"empty name":      ": value",
+		"invalid name":    "X Bad Name: value",
+		"reserved header": "User-Agent: curl/8",
+		"duplicate name":  "X-A: one\nx-a: two",
+	}
+
+	for name, headers := range headerCases {
+		t.Run("headers: "+name, func(t *testing.T) {
+			t.Parallel()
+			assertEditRejectsHeaders(t, env, headers)
+		})
+	}
+
+	timeoutCases := map[string]string{
+		"not a number": "soon",
+		"negative":     "-1",
+		"over ceiling": "100000",
+	}
+
+	for name, timeout := range timeoutCases {
+		t.Run("timeout: "+name, func(t *testing.T) {
+			t.Parallel()
+			assertEditRejectsTimeout(t, env, timeout)
+		})
+	}
+}
+
+func assertEditRejectsBlockedDestination(
+	t *testing.T, env *sourceTestEnv,
+) {
+	t.Helper()
+
+	webhook, target := seedHTTPTarget(t, env, "", "")
+
+	w := submitTargetEdit(
+		env, webhook.ID, target.ID,
+		editForm(editBlockedURL, "", ""),
+	)
+
+	assert.Equal(t, http.StatusBadRequest, w.Code)
+	assert.Contains(t, w.Body.String(), "Invalid target URL")
+	assert.Equal(
+		t, editOriginalURL,
+		storedHTTPConfig(t, env, target.ID).URL,
+		"a rejected edit must leave the stored config alone",
+	)
+}
+
+// The ingress rule the create path already follows applies to the
+// edit path too: reading a field with FormValue would let the request
+// line carry the credential, and the request line is what logs,
+// proxies and Referer headers record.
+func assertEditIgnoresQueryString(
+	t *testing.T, env *sourceTestEnv,
+) {
+	t.Helper()
+
+	webhook, target := seedHTTPTarget(t, env, "", "")
+
+	form := url.Values{}
+	form.Set("name", "edited-name")
+
+	w := serveTarget(
+		env, http.MethodPost,
+		"/source/"+webhook.ID+"/targets/"+target.ID+
+			"/edit?url="+url.QueryEscape(editReplacedURL)+
+			"&headers="+url.QueryEscape(editAuthHeader),
+		form,
+	)
+
+	assert.Equal(t, http.StatusBadRequest, w.Code)
+
+	cfg := storedHTTPConfig(t, env, target.ID)
+	assert.Equal(t, editOriginalURL, cfg.URL)
+	assert.Empty(t, cfg.Headers)
+}
+
+func assertEditRejectsHeaders(
+	t *testing.T, env *sourceTestEnv, headers string,
+) {
+	t.Helper()
+
+	webhook, target := seedHTTPTarget(t, env, "", "")
+
+	w := submitTargetEdit(
+		env, webhook.ID, target.ID,
+		editForm(editOriginalURL, headers, ""),
+	)
+
+	assert.Equal(t, http.StatusBadRequest, w.Code)
+	assert.Contains(t, w.Body.String(), "Invalid headers")
+	assert.Empty(
+		t, storedHTTPConfig(t, env, target.ID).Headers,
+		"a rejected header must not be stored",
+	)
+}
+
+func assertEditRejectsTimeout(
+	t *testing.T, env *sourceTestEnv, timeout string,
+) {
+	t.Helper()
+
+	webhook, target := seedHTTPTarget(t, env, "", "9")
+
+	w := submitTargetEdit(
+		env, webhook.ID, target.ID,
+		editForm(editOriginalURL, "", timeout),
+	)
+
+	assert.Equal(t, http.StatusBadRequest, w.Code)
+	assert.Contains(t, w.Body.String(), "Invalid timeout")
+	assert.Equal(
+		t, 9, storedHTTPConfig(t, env, target.ID).Timeout,
+		"a rejected timeout must leave the stored one alone",
+	)
+}
+
+// TestHandleTargetEdit_Scoping keeps the edit routes scoped the way
+// the delete and toggle routes are: ownership is decided by the
+// webhook, and the target is then scoped to it.
+func TestHandleTargetEdit_Scoping(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	t.Run("a target of another webhook", func(t *testing.T) {
+		t.Parallel()
+		assertTargetOfAnotherWebhook404s(t, env)
+	})
+
+	t.Run("a webhook of another user", func(t *testing.T) {
+		t.Parallel()
+		assertWebhookOfAnotherUser404s(t, env)
+	})
+}
+
+// A target id from elsewhere must not become editable by pairing it
+// with a webhook the user does own.
+func assertTargetOfAnotherWebhook404s(
+	t *testing.T, env *sourceTestEnv,
+) {
+	t.Helper()
+
+	mine := seedWebhookWithRetention(t, env.db, 30)
+	_, target := seedHTTPTarget(t, env, "", "")
+
+	get := serveTarget(
+		env, http.MethodGet,
+		"/source/"+mine.ID+"/targets/"+target.ID+"/edit", nil,
+	)
+	assert.Equal(t, http.StatusNotFound, get.Code)
+
+	post := submitTargetEdit(
+		env, mine.ID, target.ID,
+		editForm(editReplacedURL, "", ""),
+	)
+	assert.Equal(t, http.StatusNotFound, post.Code)
+
+	assert.Equal(
+		t, editOriginalURL,
+		storedHTTPConfig(t, env, target.ID).URL,
+	)
+}
+
+func assertWebhookOfAnotherUser404s(
+	t *testing.T, env *sourceTestEnv,
+) {
+	t.Helper()
+
+	other := &database.Webhook{
+		UserID:        "some-other-user",
+		Name:          "not mine",
+		RetentionDays: 30,
+	}
+	require.NoError(
+		t,
+		env.db.DB().Omit(clause.Associations).Create(other).Error,
+	)
+
+	target := seedConfiguredTarget(
+		t, env.db, other.ID, database.TargetTypeHTTP,
+		`{"url":"`+editOriginalURL+`"}`,
+	)
+
+	w := serveTarget(
+		env, http.MethodGet,
+		"/source/"+other.ID+"/targets/"+target.ID+"/edit", nil,
+	)
+
+	assert.Equal(t, http.StatusNotFound, w.Code)
+}

internal/handlers/webhook.go

@@ -217,6 +217,11 @@ func (h *Handlers) createAndDeliverEvent(
 		return
 	}
 
+	// Counted here, after the commit: an event is received once it
+	// is durably stored, which is what the delivery counters are
+	// compared against on a dashboard.
+	h.mtr.EventReceived()
+
 	h.finishWebhookResponse(w, event, entrypoint, tasks)
 }
 

internal/metrics/metrics.go

@@ -0,0 +1,344 @@
+// Package metrics defines the Prometheus collectors describing
+// webhooker's delivery pipeline: how many events arrive, how many
+// deliveries are attempted, how they end, how long they take, how
+// deep the queues are, and how many circuit breakers are open.
+//
+// The inbound HTTP metrics come from the go-http-metrics recorder in
+// internal/middleware and land on prometheus.DefaultRegisterer. These
+// collectors register there too, so both surfaces are gathered by the
+// one promhttp handler mounted on the authenticated /metrics route.
+package metrics
+
+import (
+	"sync"
+	"time"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
+	"sneak.berlin/go/webhooker/internal/database"
+)
+
+// namespace prefixes every collector defined here.
+const namespace = "webhooker"
+
+// targetTypeLabel is the only label any delivery metric carries, and
+// cardinality is the whole reason for that.
+//
+// A target type is one of four compile-time constants, so the label
+// domain is bounded by construction. Target ids, event ids and
+// entrypoint ids are not: they are UUIDs minted per operator action
+// or per inbound request, a series is never reclaimed once it exists,
+// and labelling by any of them makes /metrics a memory leak that
+// grows with traffic. normalizeTargetType enforces the bound at every
+// call site — a type the registry does not know collapses into
+// unknownTargetType rather than minting a series of its own.
+const targetTypeLabel = "target_type"
+
+// unknownTargetType is the bucket for a target type outside the known
+// set, so an unrecognised value cannot mint a new series.
+const unknownTargetType = "unknown"
+
+// Delivery duration buckets, exponential from 5ms so the last bucket
+// (about 98s) sits above the 30s outbound HTTP client timeout.
+const (
+	durationBucketStart  = 0.005
+	durationBucketFactor = 3
+	durationBucketCount  = 10
+)
+
+// knownTargetTypes is the fixed label domain: the target types the
+// delivery engine implements.
+//
+//nolint:gochecknoglobals // the label domain, built once per process
+var knownTargetTypes = []database.TargetType{
+	database.TargetTypeHTTP,
+	database.TargetTypeDatabase,
+	database.TargetTypeLog,
+	database.TargetTypeSlack,
+}
+
+// defaultSet is the process-wide metric set, registered on the same
+// registry the HTTP middleware and the /metrics handler already use.
+// It is built on first use rather than in an init so that a test
+// binary that never touches metrics never registers them.
+//
+//nolint:gochecknoglobals // one process-wide registration, by design
+var defaultSet = sync.OnceValue(func() *Set {
+	return New(prometheus.DefaultRegisterer)
+})
+
+// Default returns the process-wide metric set.
+func Default() *Set {
+	return defaultSet()
+}
+
+// Set is one registered group of webhooker's delivery collectors.
+// Production uses the single Default set; tests build their own
+// against a private registry so assertions are not disturbed by
+// deliveries other tests are making concurrently.
+type Set struct {
+	eventsReceived      prometheus.Counter
+	deliveryAttempts    *prometheus.CounterVec
+	deliveriesSucceeded *prometheus.CounterVec
+	deliveriesFailed    *prometheus.CounterVec
+	deliveryRetries     *prometheus.CounterVec
+	deliveryDuration    *prometheus.HistogramVec
+	deliveriesPending   *prometheus.GaugeVec
+	deliveriesRetrying  *prometheus.GaugeVec
+	circuitBreakersOpen *prometheus.GaugeVec
+}
+
+// New registers a full set of delivery collectors on reg and returns
+// it. It panics if reg already holds them, which is the intended
+// behaviour for a duplicate registration.
+func New(reg prometheus.Registerer) *Set {
+	factory := promauto.With(reg)
+
+	s := &Set{
+		eventsReceived: factory.NewCounter(
+			prometheus.CounterOpts{
+				Namespace: namespace,
+				Name:      "events_received_total",
+				Help: "Webhook events received and " +
+					"stored, so the receive and deliver " +
+					"sides can be compared.",
+			},
+		),
+		deliveryDuration: factory.NewHistogramVec(
+			prometheus.HistogramOpts{
+				Namespace: namespace,
+				Name:      "delivery_duration_seconds",
+				Help: "Wall time of a single delivery " +
+					"attempt, by target type.",
+				Buckets: prometheus.ExponentialBuckets(
+					durationBucketStart,
+					durationBucketFactor,
+					durationBucketCount,
+				),
+			},
+			[]string{targetTypeLabel},
+		),
+	}
+
+	s.registerCounters(factory)
+	s.registerGauges(factory)
+	s.initSeries()
+
+	return s
+}
+
+// EventReceived counts one inbound webhook event stored.
+func (s *Set) EventReceived() {
+	s.eventsReceived.Inc()
+}
+
+// DeliveryAttempted counts one delivery attempt dispatched to a
+// target.
+func (s *Set) DeliveryAttempted(t database.TargetType) {
+	s.deliveryAttempts.
+		WithLabelValues(normalizeTargetType(t)).
+		Inc()
+}
+
+// ObserveDeliveryDuration records how long one delivery attempt took.
+func (s *Set) ObserveDeliveryDuration(
+	t database.TargetType, d time.Duration,
+) {
+	s.deliveryDuration.
+		WithLabelValues(normalizeTargetType(t)).
+		Observe(d.Seconds())
+}
+
+// DeliveryStatusChanged counts a delivery's transition into a new
+// status. The mapping from status to counter lives here, next to the
+// collectors, so the engine has a single call for every transition it
+// persists. A move back to pending is not an outcome and counts
+// nothing.
+func (s *Set) DeliveryStatusChanged(
+	t database.TargetType, status database.DeliveryStatus,
+) {
+	label := normalizeTargetType(t)
+
+	switch status {
+	case database.DeliveryStatusDelivered:
+		s.deliveriesSucceeded.WithLabelValues(label).Inc()
+	case database.DeliveryStatusFailed:
+		s.deliveriesFailed.WithLabelValues(label).Inc()
+	case database.DeliveryStatusRetrying:
+		s.deliveryRetries.WithLabelValues(label).Inc()
+	case database.DeliveryStatusPending:
+	}
+}
+
+// SetQueueDepths publishes the pending and retrying queue depths from
+// one sample. Every label in the queue domain is written on every
+// call, so a type whose queue has drained reads zero instead of
+// holding its last value forever.
+func (s *Set) SetQueueDepths(
+	pending, retrying map[database.TargetType]int,
+) {
+	pendingByLabel := foldToLabels(pending)
+	retryingByLabel := foldToLabels(retrying)
+
+	for _, label := range queueDepthLabels() {
+		s.deliveriesPending.WithLabelValues(label).
+			Set(float64(pendingByLabel[label]))
+		s.deliveriesRetrying.WithLabelValues(label).
+			Set(float64(retryingByLabel[label]))
+	}
+}
+
+// queueDepthLabels is the label domain of the two queue-depth gauges:
+// the known target types plus unknown.
+//
+// Unknown is a real bucket here, not a safety net. A delivery queued
+// against a target that has since been deleted carries a target id no
+// longer in the targets table, so the sample resolves it to the empty
+// type; folding it into unknown is what keeps that backlog visible.
+// Dropping it would hide the one queue nobody is watching.
+func queueDepthLabels() []string {
+	labels := make([]string, 0, len(knownTargetTypes)+1)
+
+	for _, t := range knownTargetTypes {
+		labels = append(labels, string(t))
+	}
+
+	return append(labels, unknownTargetType)
+}
+
+// foldToLabels collapses a per-target-type count onto the bounded
+// label domain, summing everything outside the known set into
+// unknown.
+func foldToLabels(
+	counts map[database.TargetType]int,
+) map[string]int {
+	byLabel := make(map[string]int, len(counts))
+
+	for t, n := range counts {
+		byLabel[normalizeTargetType(t)] += n
+	}
+
+	return byLabel
+}
+
+// SetCircuitBreakersOpen publishes how many of a target type's
+// circuit breakers are currently open.
+func (s *Set) SetCircuitBreakersOpen(
+	t database.TargetType, open int,
+) {
+	s.circuitBreakersOpen.
+		WithLabelValues(normalizeTargetType(t)).
+		Set(float64(open))
+}
+
+func (s *Set) registerCounters(factory promauto.Factory) {
+	s.deliveryAttempts = factory.NewCounterVec(
+		prometheus.CounterOpts{
+			Namespace: namespace,
+			Name:      "delivery_attempts_total",
+			Help: "Delivery attempts dispatched to a " +
+				"target, by target type.",
+		},
+		[]string{targetTypeLabel},
+	)
+
+	s.deliveriesSucceeded = factory.NewCounterVec(
+		prometheus.CounterOpts{
+			Namespace: namespace,
+			Name:      "deliveries_succeeded_total",
+			Help: "Deliveries that reached the delivered " +
+				"state, by target type.",
+		},
+		[]string{targetTypeLabel},
+	)
+
+	s.deliveriesFailed = factory.NewCounterVec(
+		prometheus.CounterOpts{
+			Namespace: namespace,
+			Name:      "deliveries_failed_total",
+			Help: "Deliveries that failed terminally and " +
+				"will not be retried, by target type.",
+		},
+		[]string{targetTypeLabel},
+	)
+
+	s.deliveryRetries = factory.NewCounterVec(
+		prometheus.CounterOpts{
+			Namespace: namespace,
+			Name:      "delivery_retries_total",
+			Help: "Deliveries put back into the retrying " +
+				"state, by target type.",
+		},
+		[]string{targetTypeLabel},
+	)
+}
+
+func (s *Set) registerGauges(factory promauto.Factory) {
+	s.deliveriesPending = factory.NewGaugeVec(
+		prometheus.GaugeOpts{
+			Namespace: namespace,
+			Name:      "deliveries_pending",
+			Help: "Deliveries currently in the pending " +
+				"state, by target type.",
+		},
+		[]string{targetTypeLabel},
+	)
+
+	s.deliveriesRetrying = factory.NewGaugeVec(
+		prometheus.GaugeOpts{
+			Namespace: namespace,
+			Name:      "deliveries_retrying",
+			Help: "Deliveries currently in the retrying " +
+				"state, by target type.",
+		},
+		[]string{targetTypeLabel},
+	)
+
+	s.circuitBreakersOpen = factory.NewGaugeVec(
+		prometheus.GaugeOpts{
+			Namespace: namespace,
+			Name:      "circuit_breakers_open",
+			Help: "Delivery circuit breakers currently " +
+				"open, by target type.",
+		},
+		[]string{targetTypeLabel},
+	)
+}
+
+// initSeries materialises every known-target-type series at zero, so
+// a dashboard and an alert rule see a target type that has not
+// delivered yet rather than a missing series.
+//
+// The queue-depth gauges additionally get their unknown series, which
+// holds deliveries queued against a deleted target. That backlog can
+// predate the process — it is read out of the databases, not counted
+// from transitions — so its series has to exist from the first scrape
+// rather than appearing only once a backlog has already built up.
+func (s *Set) initSeries() {
+	for _, t := range knownTargetTypes {
+		label := string(t)
+
+		s.deliveryAttempts.WithLabelValues(label)
+		s.deliveriesSucceeded.WithLabelValues(label)
+		s.deliveriesFailed.WithLabelValues(label)
+		s.deliveryRetries.WithLabelValues(label)
+		s.deliveriesPending.WithLabelValues(label)
+		s.deliveriesRetrying.WithLabelValues(label)
+		s.circuitBreakersOpen.WithLabelValues(label)
+	}
+
+	s.deliveriesPending.WithLabelValues(unknownTargetType)
+	s.deliveriesRetrying.WithLabelValues(unknownTargetType)
+}
+
+// normalizeTargetType maps a target type onto the bounded label
+// domain, collapsing anything outside it to unknownTargetType.
+func normalizeTargetType(t database.TargetType) string {
+	for _, known := range knownTargetTypes {
+		if t == known {
+			return string(known)
+		}
+	}
+
+	return unknownTargetType
+}

internal/metrics/metrics_test.go

@@ -0,0 +1,285 @@
+package metrics_test
+
+import (
+	"testing"
+
+	"github.com/prometheus/client_golang/prometheus"
+	dto "github.com/prometheus/client_model/go"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/metrics"
+)
+
+// knownLabels is the target_type label domain built from the target
+// types the delivery engine implements.
+func knownLabels() []string {
+	return []string{"http", "database", "log", "slack"}
+}
+
+// labelValues returns the target_type label values a metric family
+// currently carries.
+func labelValues(
+	t *testing.T, reg *prometheus.Registry, name string,
+) []string {
+	t.Helper()
+
+	families, err := reg.Gather()
+	require.NoError(t, err)
+
+	var values []string
+
+	for _, fam := range families {
+		if fam.GetName() != name {
+			continue
+		}
+
+		for _, m := range fam.GetMetric() {
+			for _, label := range m.GetLabel() {
+				if label.GetName() == "target_type" {
+					values = append(
+						values, label.GetValue(),
+					)
+				}
+			}
+		}
+	}
+
+	return values
+}
+
+func gaugeValue(
+	t *testing.T,
+	reg *prometheus.Registry,
+	name, targetType string,
+) float64 {
+	t.Helper()
+
+	families, err := reg.Gather()
+	require.NoError(t, err)
+
+	for _, fam := range families {
+		if fam.GetName() != name {
+			continue
+		}
+
+		for _, m := range fam.GetMetric() {
+			if hasTargetType(m, targetType) {
+				return m.GetGauge().GetValue()
+			}
+		}
+	}
+
+	t.Fatalf(
+		"gauge %s{target_type=%q} not found",
+		name, targetType,
+	)
+
+	return 0
+}
+
+func hasTargetType(m *dto.Metric, targetType string) bool {
+	for _, label := range m.GetLabel() {
+		if label.GetName() == "target_type" &&
+			label.GetValue() == targetType {
+			return true
+		}
+	}
+
+	return false
+}
+
+// TestUnknownTargetTypeCollapses is the cardinality guard: a target
+// type outside the known set must not mint a series of its own, or
+// /metrics grows without bound.
+func TestUnknownTargetTypeCollapses(t *testing.T) {
+	t.Parallel()
+
+	reg := prometheus.NewRegistry()
+	set := metrics.New(reg)
+
+	for _, bogus := range []string{
+		"a1b2c3d4-0000-0000-0000-000000000001",
+		"a1b2c3d4-0000-0000-0000-000000000002",
+		"webhook-forwarder",
+	} {
+		set.DeliveryAttempted(database.TargetType(bogus))
+	}
+
+	values := labelValues(
+		t, reg, "webhooker_delivery_attempts_total",
+	)
+
+	assert.ElementsMatch(t,
+		append(knownLabels(), "unknown"),
+		values,
+	)
+}
+
+// TestSetQueueDepthsZeroesDrainedTypes proves a queue that has
+// drained reads zero rather than holding its last sample forever.
+func TestSetQueueDepthsZeroesDrainedTypes(t *testing.T) {
+	t.Parallel()
+
+	reg := prometheus.NewRegistry()
+	set := metrics.New(reg)
+
+	set.SetQueueDepths(
+		map[database.TargetType]int{
+			database.TargetTypeHTTP: 7,
+		},
+		map[database.TargetType]int{
+			database.TargetTypeSlack: 2,
+		},
+	)
+
+	assert.InDelta(t, 7.0, gaugeValue(
+		t, reg, "webhooker_deliveries_pending", "http",
+	), 0)
+	assert.InDelta(t, 2.0, gaugeValue(
+		t, reg, "webhooker_deliveries_retrying", "slack",
+	), 0)
+
+	set.SetQueueDepths(
+		map[database.TargetType]int{},
+		map[database.TargetType]int{},
+	)
+
+	assert.InDelta(t, 0.0, gaugeValue(
+		t, reg, "webhooker_deliveries_pending", "http",
+	), 0)
+	assert.InDelta(t, 0.0, gaugeValue(
+		t, reg, "webhooker_deliveries_retrying", "slack",
+	), 0)
+}
+
+// TestKnownSeriesExistBeforeAnyDelivery proves every known target
+// type is published at zero from registration, so an alert rule does
+// not have to cope with a missing series.
+func TestKnownSeriesExistBeforeAnyDelivery(t *testing.T) {
+	t.Parallel()
+
+	reg := prometheus.NewRegistry()
+	metrics.New(reg)
+
+	for _, name := range []string{
+		"webhooker_delivery_attempts_total",
+		"webhooker_deliveries_succeeded_total",
+		"webhooker_deliveries_failed_total",
+		"webhooker_delivery_retries_total",
+		"webhooker_circuit_breakers_open",
+	} {
+		assert.ElementsMatch(t,
+			knownLabels(),
+			labelValues(t, reg, name),
+			"metric %s", name,
+		)
+	}
+
+	// The queue gauges additionally publish unknown from
+	// registration: a backlog queued against a deleted target lands
+	// there, and it can predate the process, so the series has to
+	// exist before the first sample rather than appearing only once
+	// something is already stuck.
+	for _, name := range []string{
+		"webhooker_deliveries_pending",
+		"webhooker_deliveries_retrying",
+	} {
+		assert.ElementsMatch(t,
+			append(knownLabels(), "unknown"),
+			labelValues(t, reg, name),
+			"metric %s", name,
+		)
+	}
+}
+
+// TestSetQueueDepthsFoldsUnknownTypes proves a queued delivery whose
+// target type is not a known one — a target deleted out from under it
+// resolves to the empty type — is summed into the unknown series
+// instead of being dropped, and that the fold is a sum rather than a
+// last-writer-wins.
+func TestSetQueueDepthsFoldsUnknownTypes(t *testing.T) {
+	t.Parallel()
+
+	reg := prometheus.NewRegistry()
+	set := metrics.New(reg)
+
+	set.SetQueueDepths(
+		map[database.TargetType]int{
+			database.TargetTypeHTTP:             1,
+			database.TargetType(""):             4,
+			database.TargetType("retired-type"): 3,
+		},
+		map[database.TargetType]int{
+			database.TargetType(""): 2,
+		},
+	)
+
+	assert.InDelta(t, 7.0, gaugeValue(
+		t, reg, "webhooker_deliveries_pending", "unknown",
+	), 0)
+	assert.InDelta(t, 2.0, gaugeValue(
+		t, reg, "webhooker_deliveries_retrying", "unknown",
+	), 0)
+	assert.InDelta(t, 1.0, gaugeValue(
+		t, reg, "webhooker_deliveries_pending", "http",
+	), 0)
+
+	set.SetQueueDepths(
+		map[database.TargetType]int{},
+		map[database.TargetType]int{},
+	)
+
+	assert.InDelta(t, 0.0, gaugeValue(
+		t, reg, "webhooker_deliveries_pending", "unknown",
+	), 0)
+}
+
+// TestDeliveryStatusChangedCounts maps each persisted status onto the
+// counter it moves.
+func TestDeliveryStatusChangedCounts(t *testing.T) {
+	t.Parallel()
+
+	reg := prometheus.NewRegistry()
+	set := metrics.New(reg)
+
+	set.DeliveryStatusChanged(
+		database.TargetTypeLog,
+		database.DeliveryStatusDelivered,
+	)
+	set.DeliveryStatusChanged(
+		database.TargetTypeLog,
+		database.DeliveryStatusFailed,
+	)
+	set.DeliveryStatusChanged(
+		database.TargetTypeLog,
+		database.DeliveryStatusRetrying,
+	)
+	set.DeliveryStatusChanged(
+		database.TargetTypeLog,
+		database.DeliveryStatusPending,
+	)
+
+	families, err := reg.Gather()
+	require.NoError(t, err)
+
+	counts := map[string]float64{}
+
+	for _, fam := range families {
+		for _, m := range fam.GetMetric() {
+			if hasTargetType(m, "log") {
+				counts[fam.GetName()] =
+					m.GetCounter().GetValue()
+			}
+		}
+	}
+
+	assert.InDelta(t, 1.0,
+		counts["webhooker_deliveries_succeeded_total"], 0)
+	assert.InDelta(t, 1.0,
+		counts["webhooker_deliveries_failed_total"], 0)
+	assert.InDelta(t, 1.0,
+		counts["webhooker_delivery_retries_total"], 0)
+	assert.InDelta(t, 0.0,
+		counts["webhooker_delivery_attempts_total"], 0)
+}

internal/server/routes.go

@@ -214,6 +214,20 @@ func (s *Server) setupSourceRoutes() {
 			s.h.HandleEntrypointToggle(),
 		)
 		r.Post("/targets", s.h.HandleTargetCreate())
+		// The edit form is the one page that renders a target's
+		// destination URL and header values in full; see
+		// delivery.TargetConfigForm. It belongs to this group for
+		// its RequireAuth and NoCache, which are what keep that
+		// exception from reaching an unauthenticated request or a
+		// shared cache.
+		r.Get(
+			"/targets/{targetID}/edit",
+			s.h.HandleTargetEdit(),
+		)
+		r.Post(
+			"/targets/{targetID}/edit",
+			s.h.HandleTargetEditSubmit(),
+		)
 		r.Post(
 			"/targets/{targetID}/delete",
 			s.h.HandleTargetDelete(),

templates/source_detail.html

@@ -110,6 +110,14 @@
                     <div x-show="targetType === 'http'">
                         <input type="url" name="url" placeholder="https://example.com/webhook" :disabled="targetType !== 'http'" class="input text-sm">
                     </div>
+                    <div x-show="targetType === 'http'">
+                        <textarea name="headers" rows="3" placeholder="Authorization: Bearer ..." :disabled="targetType !== 'http'" class="input text-sm"></textarea>
+                        <p class="text-xs text-gray-500 mt-1">Optional request headers, one <code>Name: value</code> per line, sent with every delivery.</p>
+                    </div>
+                    <div x-show="targetType === 'http'" class="flex gap-2 items-center">
+                        <label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
+                        <input type="number" name="timeout" min="0" max="300" :disabled="targetType !== 'http'" class="input text-sm w-24">
+                    </div>
                     <div x-show="targetType === 'http'" class="flex gap-2 items-center">
                         <label class="text-sm text-gray-700">Max retries (0 = fire-and-forget):</label>
                         <input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
@@ -138,6 +146,7 @@
                             {{else}}
                             <span class="badge-error">Inactive</span>
                             {{end}}
+                            <a href="/source/{{$.Webhook.ID}}/targets/{{.ID}}/edit" class="text-xs text-gray-500 hover:text-primary-600" title="Edit">Edit</a>
                             <form method="POST" action="/source/{{$.Webhook.ID}}/targets/{{.ID}}/toggle" class="inline">
                                 <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
                                 <button type="submit" class="text-xs text-gray-500 hover:text-primary-600" title="{{if .Active}}Deactivate{{else}}Activate{{end}}">

templates/target_edit.html

@@ -0,0 +1,83 @@
+{{template "base" .}}
+
+{{define "title"}}Edit {{.Target.Name}} - Webhooker{{end}}
+
+{{define "content"}}
+<div class="max-w-2xl mx-auto px-6 py-8">
+    <div class="mb-6">
+        <a href="/source/{{.Webhook.ID}}" class="text-sm text-primary-600 hover:text-primary-700">&larr; Back to {{.Webhook.Name}}</a>
+        <h1 class="text-2xl font-medium text-gray-900 mt-2">Edit Target</h1>
+        <p class="text-sm text-gray-500 mt-1">Type: {{.Target.Type}}. A target's type cannot be changed; create a new target to deliver a different way.</p>
+    </div>
+
+    <div class="card p-6">
+        {{if .Error}}
+        <div class="alert-error">{{.Error}}</div>
+        {{end}}
+
+        {{if or (eq .Target.Type "http") (eq .Target.Type "slack")}}
+        <div class="mb-6 rounded-md bg-gray-50 p-4 text-sm text-gray-700">
+            This form shows the target's stored destination in full, including any credential carried in its URL or headers. It is the only page that does; everywhere else the value is masked.
+        </div>
+        {{end}}
+
+        <form method="POST" action="/source/{{.Webhook.ID}}/targets/{{.Target.ID}}/edit" class="space-y-6">
+            <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
+
+            <div class="form-group">
+                <label for="name" class="label">Name</label>
+                <input type="text" id="name" name="name" value="{{.Target.Name}}" required class="input">
+            </div>
+
+            {{if eq .Target.Type "http"}}
+            <div class="form-group">
+                <label for="url" class="label">Destination URL</label>
+                <input type="url" id="url" name="url" value="{{.Target.Config.URL}}" required class="input">
+                <p class="text-xs text-gray-500 mt-1">Revalidated on save; destinations that resolve to private or link-local addresses are rejected.</p>
+            </div>
+
+            <div class="form-group">
+                <label for="headers" class="label">Headers</label>
+                <textarea id="headers" name="headers" rows="4" class="input" placeholder="Authorization: Bearer ...">{{.Target.Config.Headers}}</textarea>
+                <p class="text-xs text-gray-500 mt-1">One <code>Name: value</code> per line, sent with every delivery. Leave blank for none. <code>Host</code>, <code>Content-Length</code>, <code>Transfer-Encoding</code>, <code>Connection</code> and <code>User-Agent</code> are set by the delivery engine and are rejected here rather than silently ignored.</p>
+            </div>
+
+            <div class="form-group">
+                <label for="timeout" class="label">Timeout (seconds)</label>
+                <input type="number" id="timeout" name="timeout" value="{{.Target.Config.Timeout}}" min="0" max="{{.MaxTimeout}}" class="input">
+                <p class="text-xs text-gray-500 mt-1">Per-request timeout, at most {{.MaxTimeout}} seconds. Leave blank to use the default.</p>
+            </div>
+            {{end}}
+
+            {{if eq .Target.Type "slack"}}
+            <div class="form-group">
+                <label for="url" class="label">Webhook URL</label>
+                <input type="url" id="url" name="url" value="{{.Target.Config.URL}}" required class="input">
+                <p class="text-xs text-gray-500 mt-1">Slack or Mattermost incoming webhook URL. Revalidated on save.</p>
+            </div>
+            {{end}}
+
+            {{if eq .Target.Type "database"}}
+            <div class="form-group">
+                <label for="expiry" class="label">Archive Expiry</label>
+                <input type="text" id="expiry" name="expiry" value="{{.Target.Config.Expiry}}" placeholder="never" class="input">
+                <p class="text-xs text-gray-500 mt-1">"never" (the default when blank) keeps archived rows forever, or a Go duration like "720h" prunes older rows.</p>
+            </div>
+            {{end}}
+
+            {{if or (eq .Target.Type "http") (eq .Target.Type "slack")}}
+            <div class="form-group">
+                <label for="max_retries" class="label">Max retries</label>
+                <input type="number" id="max_retries" name="max_retries" value="{{.Target.MaxRetries}}" min="0" max="20" class="input">
+                <p class="text-xs text-gray-500 mt-1">0 is fire-and-forget: one attempt, no circuit breaker.</p>
+            </div>
+            {{end}}
+
+            <div class="flex gap-3">
+                <button type="submit" class="btn-primary">Save Changes</button>
+                <a href="/source/{{.Webhook.ID}}" class="btn-secondary">Cancel</a>
+            </div>
+        </form>
+    </div>
+</div>
+{{end}}