Compare commits

5 Commits

Author SHA1 Message Date
88e283f728 Add optional inbound webhook signature verification (closes #67)
Some checks failed
check / check (push) Failing after 2m31s
A receiver URL was a bare v4 UUID and nothing else: anyone who learned
it could store events and, because inbound headers are forwarded to
targets almost verbatim, choose what the downstream service received.

Entrypoints gain an optional scheme/secret pair. GitHub's
X-Hub-Signature-256 (HMAC-SHA256 hex over the raw body) and GitLab's
X-Gitlab-Token (plain shared token) are supported; both compare with
hmac.Equal. With nothing configured an entrypoint behaves exactly as
before, which is also where every pre-existing row lands after
AutoMigrate adds the columns.

Verification runs after the capped body read and before the first
write, so a rejected request leaves no event row, no delivery row and
no delivery task. A configuration the receiver cannot apply — unknown
scheme, or one half of the pair missing — is refused with a 500 rather
than falling back to unverified.

The secret is credential-bearing and is stored in the clear because
HMAC needs the key itself. It is excluded from JSON, kept out of
templates by a new handlers.EntrypointView projection, and absent from
every log line including the rejection path. The UI sets and rotates
it through one form that never renders the stored value.

Under the GitLab scheme the signature header is the secret rather than
a digest over the request, so an accepted request's headers are cloned
and the configured scheme's credential header dropped before they are
serialized onto the event. Stored headers are persisted verbatim in
the per-webhook database and replayed onto every outbound delivery, so
keeping the token would put it in every backup and hand every target
operator the means to forge signed requests to the entrypoint it
authenticates. Stripping sits once above the first write rather than
at each egress, and is driven by the scheme's own description with
stripping as the default: a scheme added later is covered unless it
declares its header a digest, as GitHub's HMAC over the body does.

An entrypoint holding one half of the pair now renders as
misconfigured rather than as unverified, and the scheme selector
follows the stored scheme so such a row no longer marks two options
selected.
2026-08-20 05:26:07 +00:00
aba02bc509 Add a target edit form with headers and timeout fields (closes #127) (#229)
Some checks failed
check / check (push) Failing after 2m59s
2026-08-20 07:24:12 +02:00
c6a9884f86 Take an exclusive lock on DATA_DIR at startup (closes #201) (#220)
Some checks failed
check / check (push) Superseded by a newer commit; never tested
2026-08-20 07:23:00 +02:00
5af161ef60 Log SQL with placeholders, never bound values (closes #207) (#222)
Some checks failed
check / check (push) Superseded by a newer commit; never tested
2026-08-20 07:20:59 +02:00
4cc83b2326 Expose delivery metrics on /metrics (closes #209) (#224)
Some checks failed
check / check (push) Superseded by a newer commit; never tested
2026-08-20 07:19:04 +02:00
48 changed files with 7022 additions and 126 deletions

304
README.md
View File

@@ -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 was asked for. The `hasMetricsAuth` field in the startup log and the
existence of the route are the same value, so they cannot disagree. 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
`TRUSTED_PROXIES` is a comma-separated list of CIDR blocks (a bare `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 with a randomly generated password and logs it to stdout. This password
is only displayed once. 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 ### Running with Docker
```bash ```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 `{webhook_uuid}` is the webhook's UUID primary key in its canonical
36-character hyphenated form, so a real filename looks like 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 Configuration is **not** in `DATA_DIR` — it comes from the environment
and from a `.env` file read out of the process working directory. Back and from a `.env` file read out of the process working directory. Back
@@ -435,7 +516,110 @@ backups at rest and restrict who can read them.
`events-*.db` today hands them live delivery destinations. `events-*.db` today hands them live delivery destinations.
- `webhooker.db` stores target config **unencrypted**, tracked at - `webhooker.db` stores target config **unencrypted**, tracked at
[issue #212](https://git.eeqj.de/sneak/webhooker/issues/212), next to [issue #212](https://git.eeqj.de/sneak/webhooker/issues/212), next to
the session encryption key and the Argon2id password hashes. the session encryption key and the Argon2id password hashes. It also
holds each entrypoint's inbound signature secret in the clear, for
the reason given under
[Inbound Signature Verification](#inbound-signature-verification):
HMAC verification needs the key itself, so it cannot be hashed.
## Inbound Signature Verification
A receiver URL is a bare v4 UUID in a path. That is unguessable, but it
is not a credential: anyone who learns it — from a browser history, a
proxy log, a screenshot, a copy-pasted support ticket — can post events
that webhooker stores and forwards, and the inbound headers are passed
on to your targets almost verbatim, so they also choose what the
downstream service sees. Verification is how an entrypoint stops
accepting anything that reaches its URL.
It is optional and configured per entrypoint. An entrypoint with no
scheme selected is not verified, which is what every entrypoint was
before this existed and what every entrypoint remains after an
upgrade — enabling verification is always a deliberate act, and no
existing deployment is locked out of its own receivers by installing a
new version.
When a scheme **is** selected, a request whose signature is missing,
malformed or wrong is answered `401` and **nothing is stored**: no
event row, no delivery row, no delivery attempt. Rejection happens
after the body is read (the signature covers it) and before the first
write.
### Supported schemes
| Scheme | Header | Check |
| -------- | --------------------- | ----- |
| `github` | `X-Hub-Signature-256` | HMAC-SHA256 of the raw request body under the shared secret, hex-encoded, prefixed `sha256=` |
| `gitlab` | `X-Gitlab-Token` | The header is the shared secret itself, compared as-is |
Both comparisons run in constant time (`hmac.Equal`). The HMAC is
computed over the request body exactly as received, before any parsing,
and under the same 1 MB body cap every other request obeys — an
unsigned sender cannot make webhooker buffer more than a signed one.
GitHub's older SHA-1 `X-Hub-Signature` is **not** accepted. Neither is
a GitHub digest sent without its `sha256=` prefix.
### Configuring a sender
The secret is a value you choose and enter in two places: at the sender
and in webhooker. webhooker never generates or displays one, so there
is no stored credential the UI can be made to reveal.
1. Generate a secret, e.g. `openssl rand -hex 32`.
2. In webhooker, open the webhook's page, find the entrypoint, and
click **Configure** (or **Rotate**, if it already has one). Select
the scheme and paste the secret. Surrounding whitespace is stripped,
so a value pasted with a trailing space still works; a secret whose
own first or last character is a space cannot be stored.
3. At the sender:
- **GitHub** — repository (or organization) → Settings → Webhooks →
the hook → **Secret**. GitHub then signs every delivery with
`X-Hub-Signature-256`.
- **GitLab** — project → Settings → Webhooks → the hook → **Secret
token**. GitLab sends it verbatim as `X-Gitlab-Token`.
**Rotation** is the same form: submit the new secret. Deliveries signed
with the old secret are rejected from that moment, so change it at the
sender in the same sitting. Selecting **None** removes verification and
deletes the stored secret with it.
The page shows which scheme an entrypoint uses and which header it
reads, never the secret. The value is stored in the clear — HMAC
verification needs the key itself, and a hash of it cannot recompute a
sender's digest — so it is handled like the other credentials
webhooker holds: excluded from JSON, kept out of templates by a
projection (`handlers.EntrypointView`), and absent from every log line,
including the ones written when verification fails.
### The credential is not stored or forwarded
Under the `gitlab` scheme the signature header **is** the secret. An
accepted request's headers are persisted on the event and forwarded to
every delivery target, so `X-Gitlab-Token` is removed from that copy
before the event is written — otherwise every target operator, every
backup and everyone with read access to `events-*.db` would hold the
value needed to forge signed requests to the entrypoint it protects.
The sender's other headers are untouched, and the request the receiver
itself verifies against is not modified.
The stripping is driven by the scheme's own description rather than by
a header name, and a scheme is stripped unless it declares that its
header carries a digest. `github` declares it: `X-Hub-Signature-256` is
an HMAC over the body, from which the key cannot be recovered, so it is
stored and forwarded intact. A scheme added later is stripped by
default.
### When configuration is broken
An entrypoint whose stored scheme this build does not recognise, or
which has one half of the scheme/secret pair and not the other, is
answered `500` and stores nothing. It is not treated as unverified. The
UI cannot create such a row — it rejects an unknown scheme with a `400`
— so this covers a hand-edited database or a downgrade to a build that
predates a scheme. Failing closed is the point: an entrypoint the
operator believes is protected must never quietly go back to accepting
anything.
## Entrypoints ## Entrypoints
@@ -712,9 +896,15 @@ the full request and creates an Event.
| `path` | string | Unique bare UUID, generated at creation. The `/webhook/` prefix is route only and is not stored: the receiver matches this column against the raw `{uuid}` path segment | | `path` | string | Unique bare UUID, generated at creation. The `/webhook/` prefix is route only and is not stored: the receiver matches this column against the raw `{uuid}` path segment |
| `description` | string | Optional description | | `description` | string | Optional description |
| `active` | boolean | Whether this entrypoint accepts events (default: true) | | `active` | boolean | Whether this entrypoint accepts events (default: true) |
| `signature_scheme` | string | How inbound requests are authenticated: `github`, `gitlab`, or empty for no verification (default: empty). See [Inbound Signature Verification](#inbound-signature-verification) |
| `signature_secret` | string | The secret shared with the sender, stored in the clear because HMAC verification needs the key itself. Never marshalled to JSON, never rendered, never logged. Empty when no scheme is set |
**Relations:** Belongs to Webhook. **Relations:** Belongs to Webhook.
Both signature columns arrive through `AutoMigrate` with an empty
default, so every entrypoint written before they existed migrates to
"not configured" and keeps accepting the traffic it already accepted.
A webhook can have multiple entrypoints. This allows separate URLs for A webhook can have multiple entrypoints. This allows separate URLs for
different event sources that all feed into the same processing pipeline different event sources that all feed into the same processing pipeline
(e.g., one entrypoint for GitHub, another for Stripe, both routing to (e.g., one entrypoint for GitHub, another for Stripe, both routing to
@@ -984,12 +1174,16 @@ External Service
└─────────────┘ └──────────────┘ └──────┬───────┘ └─────────────┘ └──────────────┘ └──────┬───────┘
1. Look up Entrypoint by UUID 1. Look up Entrypoint by UUID
2. Capture full request as Event 2. Read the body under the 1 MB cap
3. Create Delivery records for each active Target 3. Verify the signature, if the entrypoint has
4. Build self-contained delivery.Task structs one configured — 401 and no writes if it
fails (see Inbound Signature Verification)
4. Capture full request as Event
5. Create Delivery records for each active Target
6. Build self-contained delivery.Task structs
(target config + event data inline for (target config + event data inline for
bodies < 16 KiB) bodies < 16 KiB)
5. Notify Engine via channel (no DB read needed) 7. Notify Engine via channel (no DB read needed)
┌──────────────┐ ┌──────────────┐
@@ -1134,6 +1328,52 @@ delivery as `retrying` and schedules a retry timer for after the
remaining cooldown period. This ensures no deliveries are lost — they're remaining cooldown period. This ensures no deliveries are lost — they're
just delayed until the target is healthy again. 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 ### Rate Limiting
Global blanket rate limiting middleware (e.g., a per-IP throttle shared Global blanket rate limiting middleware (e.g., a per-IP throttle shared
@@ -1418,6 +1658,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 log's, and `internal/gormlog/gormlog_test.go` asserts each line against
`MaxAccessLogLineBytes` directly rather than leaving it as arithmetic. `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 What that ceiling does **not** cover, stated here so the figure is not
read as more than it is: read as more than it is:
@@ -1686,6 +1944,7 @@ abuse limit later; they are tracked as future work.
| `POST` | `/source/{id}/entrypoints` | Add entrypoint to webhook | | `POST` | `/source/{id}/entrypoints` | Add entrypoint to webhook |
| `POST` | `/source/{id}/entrypoints/{entrypointID}/delete` | Delete an entrypoint | | `POST` | `/source/{id}/entrypoints/{entrypointID}/delete` | Delete an entrypoint |
| `POST` | `/source/{id}/entrypoints/{entrypointID}/toggle` | Enable or disable an entrypoint | | `POST` | `/source/{id}/entrypoints/{entrypointID}/toggle` | Enable or disable an entrypoint |
| `POST` | `/source/{id}/entrypoints/{entrypointID}/secret` | Set, rotate or remove the entrypoint's inbound signature scheme and secret (see [Inbound Signature Verification](#inbound-signature-verification)) |
| `POST` | `/source/{id}/targets` | Add target to webhook | | `POST` | `/source/{id}/targets` | Add target to webhook |
| `POST` | `/source/{id}/targets/{targetID}/delete` | Delete a target | | `POST` | `/source/{id}/targets/{targetID}/delete` | Delete a target |
| `POST` | `/source/{id}/targets/{targetID}/toggle` | Enable or disable a target | | `POST` | `/source/{id}/targets/{targetID}/toggle` | Enable or disable a target |
@@ -1721,7 +1980,7 @@ imports. The entry point is `cmd/webhooker/main.go`.
``` ```
webhooker/ webhooker/
├── cmd/webhooker/ ├── cmd/webhooker/
│ └── main.go # Entry point: sets globals, wires fx │ └── main.go # Entry point: sets globals, locks DATA_DIR, wires fx
├── internal/ ├── internal/
│ ├── config/ │ ├── config/
│ │ └── config.go # Configuration loading from environment variables │ │ └── config.go # Configuration loading from environment variables
@@ -1732,7 +1991,7 @@ webhooker/
│ │ ├── model_setting.go # Setting entity (key-value app config) │ │ ├── model_setting.go # Setting entity (key-value app config)
│ │ ├── model_user.go # User entity │ │ ├── model_user.go # User entity
│ │ ├── model_webhook.go # Webhook entity │ │ ├── model_webhook.go # Webhook entity
│ │ ├── model_entrypoint.go # Entrypoint entity │ │ ├── model_entrypoint.go # Entrypoint entity and SignatureScheme enum
│ │ ├── model_target.go # Target entity and TargetType enum │ │ ├── model_target.go # Target entity and TargetType enum
│ │ ├── model_event.go # Event entity (per-webhook DB) │ │ ├── model_event.go # Event entity (per-webhook DB)
│ │ ├── model_delivery.go # Delivery entity (per-webhook DB) │ │ ├── model_delivery.go # Delivery entity (per-webhook DB)
@@ -1742,6 +2001,8 @@ webhooker/
│ │ ├── retention.go # Retention reaper (per-webhook event expiry) │ │ ├── retention.go # Retention reaper (per-webhook event expiry)
│ │ ├── testing.go # NewTestDatabase: wrapper for tests, no fx lifecycle │ │ ├── testing.go # NewTestDatabase: wrapper for tests, no fx lifecycle
│ │ └── webhook_db_manager.go # Per-webhook DB lifecycle manager │ │ └── webhook_db_manager.go # Per-webhook DB lifecycle manager
│ ├── datadir/
│ │ └── lock.go # Exclusive advisory lock on DATA_DIR (one instance)
│ ├── globals/ │ ├── globals/
│ │ └── globals.go # Build-time variables (appname, version, arch) │ │ └── globals.go # Build-time variables (appname, version, arch)
│ ├── gormlog/ │ ├── gormlog/
@@ -1759,11 +2020,13 @@ webhooker/
│ │ ├── target_log.go # Log target (stdout) │ │ ├── target_log.go # Log target (stdout)
│ │ ├── target_config_view.go # Masked target config for templates │ │ ├── target_config_view.go # Masked target config for templates
│ │ ├── archive_sweeper.go # Periodic pruning of idle archives │ │ ├── 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 │ │ ├── url_mask.go # Strips credentials from *url.Error
│ │ └── ssrf.go # SSRF prevention (IP validation, safe HTTP transport) │ │ └── ssrf.go # SSRF prevention (IP validation, safe HTTP transport)
│ ├── handlers/ │ ├── handlers/
│ │ ├── handlers.go # Base handler struct, JSON helpers, template rendering │ │ ├── handlers.go # Base handler struct, JSON helpers, template rendering
│ │ ├── auth.go # Login, logout handlers │ │ ├── auth.go # Login, logout handlers
│ │ ├── entrypoint_view.go # Masked entrypoint view for templates
│ │ ├── event_log_view.go # Event log projection, byte-capped in SQL │ │ ├── event_log_view.go # Event log projection, byte-capped in SQL
│ │ ├── healthcheck.go # Health check handler │ │ ├── healthcheck.go # Health check handler
│ │ ├── index.go # Index page handler │ │ ├── index.go # Index page handler
@@ -1776,6 +2039,8 @@ webhooker/
│ │ └── lifecycle.go # Shared stop-hook waiter, bounded by the stop context │ │ └── lifecycle.go # Shared stop-hook waiter, bounded by the stop context
│ ├── logger/ │ ├── logger/
│ │ └── logger.go # slog setup with TTY detection │ │ └── logger.go # slog setup with TTY detection
│ ├── metrics/
│ │ └── metrics.go # Delivery Prometheus collectors, labelled by target type
│ ├── middleware/ │ ├── middleware/
│ │ ├── middleware.go # Logging, CORS, Auth, Metrics, MetricsAuth, SecurityHeaders, MaxBodySize │ │ ├── middleware.go # Logging, CORS, Auth, Metrics, MetricsAuth, SecurityHeaders, MaxBodySize
│ │ ├── csrf.go # CSRF protection middleware (gorilla/csrf) │ │ ├── csrf.go # CSRF protection middleware (gorilla/csrf)
@@ -1786,9 +2051,11 @@ webhooker/
│ │ ├── server.go # Server struct, fx lifecycle, signal handling │ │ ├── server.go # Server struct, fx lifecycle, signal handling
│ │ ├── http.go # HTTP server setup with timeouts │ │ ├── http.go # HTTP server setup with timeouts
│ │ └── routes.go # All route definitions │ │ └── routes.go # All route definitions
── session/ ── session/
├── session.go # Cookie-based session management ├── session.go # Cookie-based session management
└── testing.go # NewForTest: Session without the fx lifecycle └── testing.go # NewForTest: Session without the fx lifecycle
│ └── signature/
│ └── signature.go # Inbound signature verification (GitHub, GitLab)
├── static/ ├── static/
│ ├── static.go # //go:embed directive │ ├── static.go # //go:embed directive
│ ├── css/input.css # Tailwind input, source for tailwind.css (make css) │ ├── css/input.css # Tailwind input, source for tailwind.css (make css)
@@ -1930,6 +2197,15 @@ check, see [The login endpoint](#the-login-endpoint).
`/api` (stateless API). The middleware auto-detects TLS status `/api` (stateless API). The middleware auto-detects TLS status
per-request (via `r.TLS` and `X-Forwarded-Proto`) to set appropriate per-request (via `r.TLS` and `X-Forwarded-Proto`) to set appropriate
cookie security flags and Origin/Referer validation mode cookie security flags and Origin/Referer validation mode
- **Optional inbound signature verification** per entrypoint (GitHub
`X-Hub-Signature-256`, GitLab `X-Gitlab-Token`). Off by default and
off after an upgrade, so behaviour is unchanged until an operator
turns it on. Where it is on, an unsigned or wrongly signed request
is `401` and is not persisted, and a configuration the receiver
cannot apply fails closed rather than reverting to unverified. The
comparison is constant time and the secret never reaches a template,
a JSON response or a log line (see
[Inbound Signature Verification](#inbound-signature-verification))
- **SSRF prevention** for HTTP delivery targets: private/reserved IP - **SSRF prevention** for HTTP delivery targets: private/reserved IP
ranges (RFC 1918, loopback, link-local, cloud metadata) are blocked ranges (RFC 1918, loopback, link-local, cloud metadata) are blocked
both at target creation time (URL validation) and at delivery time both at target creation time (URL validation) and at delivery time

View File

@@ -2,11 +2,15 @@
package main package main
import ( import (
"fmt"
"io"
"os"
"time" "time"
"go.uber.org/fx" "go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/config" "sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/datadir"
"sneak.berlin/go/webhooker/internal/delivery" "sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/globals" "sneak.berlin/go/webhooker/internal/globals"
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
@@ -56,7 +60,32 @@ func main() {
globals.Appname = appname globals.Appname = appname
globals.Version = version 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() newApp().Run()
return 0
} }
// newApp builds the application graph. It is separate from main so // newApp builds the application graph. It is separate from main so

View File

@@ -1,10 +1,13 @@
package main package main
import ( import (
"bytes"
"testing" "testing"
"time" "time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/datadir"
"sneak.berlin/go/webhooker/internal/server" "sneak.berlin/go/webhooker/internal/server"
) )
@@ -33,6 +36,38 @@ func TestNewApp_StopTimeout(t *testing.T) {
require.Less(t, got, dockerStopGrace) 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 // tailHeadroom is the slack the fx stop budget must keep beyond the
// server stop hook. The hooks that run after the server — the // server stop hook. The hooks that run after the server — the
// delivery engine, the healthcheck, the webhook DB manager and the // delivery engine, the healthcheck, the webhook DB manager and the

7
go.mod
View File

@@ -8,13 +8,15 @@ require (
github.com/go-chi/chi v1.5.5 github.com/go-chi/chi v1.5.5
github.com/go-chi/cors v1.2.1 github.com/go-chi/cors v1.2.1
github.com/go-chi/httprate v0.15.0 github.com/go-chi/httprate v0.15.0
github.com/gofrs/flock v0.13.0
github.com/google/uuid v1.6.0 github.com/google/uuid v1.6.0
github.com/gorilla/csrf v1.7.3 github.com/gorilla/csrf v1.7.3
github.com/gorilla/sessions v1.4.0 github.com/gorilla/sessions v1.4.0
github.com/joho/godotenv v1.5.1 github.com/joho/godotenv v1.5.1
github.com/prometheus/client_golang v1.18.0 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/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 go.uber.org/fx v1.20.1
golang.org/x/crypto v0.38.0 golang.org/x/crypto v0.38.0
gopkg.in/yaml.v3 v3.0.1 gopkg.in/yaml.v3 v3.0.1
@@ -38,7 +40,6 @@ require (
github.com/mattn/go-sqlite3 v1.14.17 // indirect github.com/mattn/go-sqlite3 v1.14.17 // indirect
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // 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/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/common v0.45.0 // indirect
github.com/prometheus/procfs v0.12.0 // indirect github.com/prometheus/procfs v0.12.0 // indirect
github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect
@@ -49,7 +50,7 @@ require (
go.uber.org/zap v1.23.0 // indirect go.uber.org/zap v1.23.0 // indirect
golang.org/x/mod v0.17.0 // indirect golang.org/x/mod v0.17.0 // indirect
golang.org/x/sync v0.14.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/text v0.25.0 // indirect
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
google.golang.org/protobuf v1.31.0 // indirect google.golang.org/protobuf v1.31.0 // indirect

14
go.sum
View File

@@ -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-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 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=
github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og= 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/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.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= 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 h1:ABJUpekCZSkQT1wQrFvS4kGbhea/w6ndFJaWJeh3zL0=
github.com/slok/go-http-metrics v0.11.0/go.mod h1:ZGKeYG1ET6TEJpQx18BqAJAvxw9jBAZXCHU7bWQqqAc= 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.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.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 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ=
github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0= github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0=
github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0= 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 h1:woo0S4Yywslg6hp4eUFjTVOyKt0RookbpAHG4c1HmhQ=
golang.org/x/sync v0.14.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= 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.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 h1:qVyWApTSYLk/drJRO5mDlNYskwQznZmkpV2c8q9zls4=
golang.org/x/text v0.25.0/go.mod h1:WEdwpYrmk1qmdHvhkSTNPm3app7v4rsT8F2UD6+VHIA= 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= golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d h1:vU5i/LfpvrRCpgM/VPfJLg5KjxD3E+hfT1SH+d9zLwg=

View File

@@ -26,6 +26,10 @@ const (
// EnvironmentProd represents production environment. // EnvironmentProd represents production environment.
EnvironmentProd = "prod" 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 is the default HTTP listen port.
defaultPort = 8080 defaultPort = 8080
@@ -159,6 +163,19 @@ func envString(key string) string {
return os.Getenv(key) 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 // envBool returns the value of the named environment variable
// parsed as a boolean. Returns defaultValue if not set. If the // parsed as a boolean. Returns defaultValue if not set. If the
// variable is set but cannot be parsed, it returns a wrapped error // variable is set but cannot be parsed, it returns a wrapped error
@@ -461,7 +478,7 @@ func loadFromEnv() (*Config, error) {
} }
return &Config{ return &Config{
DataDir: envString("DATA_DIR"), DataDir: DataDir(),
Debug: debug, Debug: debug,
MaintenanceMode: maintenanceMode, MaintenanceMode: maintenanceMode,
Environment: environment, Environment: environment,
@@ -539,14 +556,6 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
s.log = log s.log = log
s.params = &params 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 { if s.Debug {
params.Logger.EnableDebugLogging() params.Logger.EnableDebugLogging()
} }

View File

@@ -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) { func TestReceiverRateLimit(t *testing.T) {
tests := []struct { tests := []struct {
name string name string

View File

@@ -0,0 +1,85 @@
package database_test
import (
"net/http"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/signature"
)
// TestEntrypointSignatureColumnsMigrateToUnconfigured pins the
// upgrade path for a deployment that already has entrypoints.
//
// The signature columns arrive through GORM's AutoMigrate, so every
// row written before they existed acquires them with no value. That
// has to land on "not configured", because the alternative is an
// upgrade that rejects the traffic the operator was already
// receiving — a self-inflicted outage on a receiver whose senders
// cannot be told to start signing.
//
// The legacy schema is reproduced by dropping the columns from a
// migrated database and writing a row through the old shape, so the
// row really predates them rather than merely being blank.
func TestEntrypointSignatureColumnsMigrateToUnconfigured(t *testing.T) {
t.Parallel()
db, lc := setupTestDB(t)
lc.RequireStart()
t.Cleanup(lc.RequireStop)
for _, column := range []string{
"signature_scheme", "signature_secret",
} {
require.NoError(
t,
db.DB().Exec(
"ALTER TABLE entrypoints DROP COLUMN "+column,
).Error,
"dropping %s to reproduce the pre-upgrade schema",
column,
)
}
const legacyID = "legacy-entrypoint"
require.NoError(
t,
db.DB().Exec(
`INSERT INTO entrypoints
(id, created_at, updated_at, webhook_id, path,
description, active)
VALUES (?, ?, ?, ?, ?, ?, ?)`,
legacyID, "2026-01-01 00:00:00", "2026-01-01 00:00:00",
"legacy-webhook", "legacy-path", "predates signatures",
true,
).Error,
)
// The upgrade.
require.NoError(t, db.Migrate())
var ep database.Entrypoint
require.NoError(
t,
db.DB().Where("id = ?", legacyID).First(&ep).Error,
"the migrated row must still load; a NULL landing in a "+
"string column would fail here",
)
assert.Equal(t, database.SignatureSchemeNone, ep.SignatureScheme)
assert.Empty(t, ep.SignatureSecret)
assert.False(t, ep.SignatureConfigured())
assert.True(t, ep.Active, "the row's other columns survive")
// The behaviour that actually matters: an unsigned request to
// this entrypoint is still accepted.
assert.NoError(
t,
signature.Verify(&ep, http.Header{}, []byte(`{"a":1}`)),
)
}

View File

@@ -1,5 +1,22 @@
package database package database
// SignatureScheme names the way an entrypoint authenticates inbound
// requests. A scheme fixes both the header the signature arrives in
// and the algorithm used to check it, so an operator cannot pair one
// sender's header with another sender's comparison.
type SignatureScheme string
// Signature scheme values. The empty scheme means the entrypoint
// performs no inbound verification: it is the default, and it is the
// state every entrypoint created before this column existed migrates
// to, so an existing deployment keeps accepting the requests it
// accepted before.
const (
SignatureSchemeNone SignatureScheme = ""
SignatureSchemeGitHub SignatureScheme = "github"
SignatureSchemeGitLab SignatureScheme = "gitlab"
)
// Entrypoint represents an inbound URL endpoint that feeds into a webhook // Entrypoint represents an inbound URL endpoint that feeds into a webhook
type Entrypoint struct { type Entrypoint struct {
BaseModel BaseModel
@@ -12,6 +29,43 @@ type Entrypoint struct {
Description string `json:"description"` Description string `json:"description"`
Active bool `gorm:"default:true" json:"active"` Active bool `gorm:"default:true" json:"active"`
// SignatureScheme selects how inbound requests to this
// entrypoint are authenticated. Empty means unauthenticated,
// which is what a UUID-only entrypoint has always been.
SignatureScheme SignatureScheme `gorm:"default:''" json:"signatureScheme"`
// SignatureSecret is the secret shared with the sender.
//
// It is stored in the clear because HMAC verification needs the
// key itself: a hash of it cannot recompute the sender's digest.
// It is therefore a live credential, and json:"-" keeps it out of
// any handler that marshals the model, the way APIKey.Key and
// Target.Config are kept out. handlers.EntrypointView is the
// matching barrier for the HTML path.
SignatureSecret string `gorm:"default:''" json:"-"`
// Relations // Relations
Webhook Webhook `json:"webhook,omitzero"` Webhook Webhook `json:"webhook,omitzero"`
} }
// SignatureConfigured reports whether this entrypoint verifies
// inbound requests. Both halves must be present: a scheme without a
// secret, or a secret without a scheme, is a broken configuration
// rather than a configured one, and signature.Verify fails those
// closed rather than treating them as "off".
func (e *Entrypoint) SignatureConfigured() bool {
return e.SignatureScheme != SignatureSchemeNone &&
e.SignatureSecret != ""
}
// SignatureHalfConfigured reports whether exactly one half of the
// scheme/secret pair is present. The receiver refuses such a row on
// every request, so the UI must not describe it as unverified. It
// reports the state without exposing the secret, which is why it
// lives here rather than in the display projection.
func (e *Entrypoint) SignatureHalfConfigured() bool {
hasScheme := e.SignatureScheme != SignatureSchemeNone
hasSecret := e.SignatureSecret != ""
return hasScheme != hasSecret
}

View File

@@ -34,6 +34,8 @@ func marshalModel(t *testing.T, v any) string {
// - APIKey.Key is a bearer token outright. // - APIKey.Key is a bearer token outright.
// - Setting.Value holds the session encryption key. // - Setting.Value holds the session encryption key.
// - User.Password holds the Argon2 hash, and was already tagged. // - User.Password holds the Argon2 hash, and was already tagged.
// - Entrypoint.SignatureSecret is the secret its senders sign with,
// stored in the clear because HMAC verification needs the key.
func TestModelsDoNotMarshalTheirSecrets(t *testing.T) { func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
t.Parallel() t.Parallel()
@@ -72,6 +74,14 @@ func TestModelsDoNotMarshalTheirSecrets(t *testing.T) {
Password: marker, Password: marker,
}, },
}, },
{
name: "entrypoint signature secret",
model: database.Entrypoint{
Description: keptField,
SignatureScheme: database.SignatureSchemeGitHub,
SignatureSecret: marker,
},
},
} }
for _, tc := range cases { for _, tc := range cases {
@@ -105,3 +115,24 @@ func TestWebhookMarshalsNoTargetConfig(t *testing.T) {
assert.NotContains(t, encoded, marker) assert.NotContains(t, encoded, marker)
assert.Contains(t, encoded, keptField) assert.Contains(t, encoded, keptField)
} }
// TestWebhookMarshalsNoEntrypointSecret covers the same nested case
// for the entrypoint's inbound signature secret, which reaches a
// marshalled webhook through the Entrypoints association.
func TestWebhookMarshalsNoEntrypointSecret(t *testing.T) {
t.Parallel()
const marker = "QQENTRYPOINTMARKERQQ"
encoded := marshalModel(t, database.Webhook{
Name: keptField,
Entrypoints: []database.Entrypoint{{
Path: "some-uuid",
SignatureScheme: database.SignatureSchemeGitLab,
SignatureSecret: marker,
}},
})
assert.NotContains(t, encoded, marker)
assert.Contains(t, encoded, keptField)
}

126
internal/datadir/lock.go Normal file
View File

@@ -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
}

View File

@@ -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)
}

View File

@@ -15,6 +15,7 @@ import (
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/lifecycle" "sneak.berlin/go/webhooker/internal/lifecycle"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics"
) )
const ( const (
@@ -139,6 +140,12 @@ type Engine struct {
retryCh chan Task retryCh chan Task
workers int 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 maps each target type to its implementation.
targets map[database.TargetType]Target targets map[database.TargetType]Target
@@ -164,6 +171,7 @@ func New(
deliveryCh: make(chan Task, deliveryChannelSize), deliveryCh: make(chan Task, deliveryChannelSize),
retryCh: make(chan Task, retryChannelSize), retryCh: make(chan Task, retryChannelSize),
workers: defaultWorkers, workers: defaultWorkers,
mtr: metrics.Default(),
} }
e.initTargets(&http.Client{ e.initTargets(&http.Client{
@@ -283,6 +291,10 @@ func (e *Engine) start() {
go e.retrySweep(ctx) go e.retrySweep(ctx)
e.wg.Add(1)
go e.queueDepthSampler(ctx)
e.log.Info( e.log.Info(
"delivery engine started", "delivery engine started",
"workers", e.workers, "workers", e.workers,
@@ -837,8 +849,15 @@ func (e *Engine) failUnretryableRetry(
0, 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( e.updateDeliveryStatus(
webhookDB, d, database.DeliveryStatusFailed, webhookDB, d, target.Type,
database.DeliveryStatusFailed,
) )
} }
@@ -859,7 +878,8 @@ func (e *Engine) processDelivery(
) )
e.updateDeliveryStatus( e.updateDeliveryStatus(
webhookDB, d, database.DeliveryStatusFailed, webhookDB, d, d.Target.Type,
database.DeliveryStatusFailed,
) )
return return
@@ -868,6 +888,24 @@ func (e *Engine) processDelivery(
target.Deliver(ctx, webhookDB, d, task, e) 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 // recordResult persists a DeliveryResult row describing a
// single attempt. It is a cross-target helper the targets // single attempt. It is a cross-target helper the targets
// call. // call.
@@ -901,10 +939,22 @@ func (e *Engine) recordResult(
} }
// updateDeliveryStatus persists a new status for a delivery. // 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( func (e *Engine) updateDeliveryStatus(
webhookDB *gorm.DB, webhookDB *gorm.DB,
d *database.Delivery, d *database.Delivery,
targetType database.TargetType,
status database.DeliveryStatus, status database.DeliveryStatus,
) { ) {
err := webhookDB.Model(d). err := webhookDB.Model(d).
@@ -916,7 +966,11 @@ func (e *Engine) updateDeliveryStatus(
"status", status, "status", status,
"error", err, "error", err,
) )
return
} }
e.mtr.DeliveryStatusChanged(targetType, status)
} }
func truncate(s string, maxLen int) string { func truncate(s string, maxLen int) string {

View File

@@ -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( func TestRecoverSingleRetry_UnknownTargetType(
t *testing.T, t *testing.T,
) { ) {

View File

@@ -10,6 +10,7 @@ import (
"go.uber.org/fx" "go.uber.org/fx"
"gorm.io/gorm" "gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/metrics"
) )
// ErrExportArchiveWriterEvicted exposes the sentinel returned by // ErrExportArchiveWriterEvicted exposes the sentinel returned by
@@ -41,6 +42,16 @@ func ExportIsForwardableHeader(name string) bool {
return isForwardableHeader(name) return isForwardableHeader(name)
} }
// ExportApplyRequestHeaders exposes applyRequestHeaders, so a test
// can inspect the header set an outbound delivery actually carries.
func ExportApplyRequestHeaders(
req *http.Request,
event *database.Event,
cfg *HTTPTargetConfig,
) {
applyRequestHeaders(req, event, cfg)
}
// ExportTruncate exposes truncate for testing. // ExportTruncate exposes truncate for testing.
func ExportTruncate(s string, maxLen int) string { func ExportTruncate(s string, maxLen int) string {
return truncate(s, maxLen) return truncate(s, maxLen)
@@ -253,6 +264,7 @@ func NewTestEngine(
deliveryCh: make(chan Task, deliveryChannelSize), deliveryCh: make(chan Task, deliveryChannelSize),
retryCh: make(chan Task, retryChannelSize), retryCh: make(chan Task, retryChannelSize),
workers: workers, workers: workers,
mtr: metrics.Default(),
} }
e.initTargets(client) e.initTargets(client)
@@ -267,6 +279,7 @@ func NewTestEngineSmallRetry(
e := &Engine{ e := &Engine{
log: log, log: log,
retryCh: make(chan Task, 1), retryCh: make(chan Task, 1),
mtr: metrics.Default(),
} }
e.initTargets(nil) e.initTargets(nil)
@@ -289,12 +302,25 @@ func NewTestEngineWithDB(
deliveryCh: make(chan Task, deliveryChannelSize), deliveryCh: make(chan Task, deliveryChannelSize),
retryCh: make(chan Task, retryChannelSize), retryCh: make(chan Task, retryChannelSize),
workers: workers, workers: workers,
mtr: metrics.Default(),
} }
e.initTargets(client) e.initTargets(client)
return e 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 // NewTestCircuitBreaker creates a CircuitBreaker with
// custom settings for testing. // custom settings for testing.
func NewTestCircuitBreaker( func NewTestCircuitBreaker(

View File

@@ -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)
}

View File

@@ -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.
}
}
}

View File

@@ -27,6 +27,12 @@ type Scheduler interface {
// own circuit breaker, and reschedules via the injected // own circuit breaker, and reschedules via the injected
// Scheduler. Fire-and-forget targets simply record a single // Scheduler. Fire-and-forget targets simply record a single
// attempt. // 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 { type Target interface {
Deliver( Deliver(
ctx context.Context, ctx context.Context,
@@ -74,6 +80,12 @@ type attemptResult struct {
errMsg string 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 // initTargets builds the target registry, wiring each target
// to the engine's persistence helpers and giving the HTTP and // to the engine's persistence helpers and giving the HTTP and
// Slack targets the shared SSRF-safe client. It is called by // Slack targets the shared SSRF-safe client. It is called by

View File

@@ -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
}

View File

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

View File

@@ -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)
}

View File

@@ -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)
}
}

View File

@@ -74,6 +74,8 @@ func (c *httpCore) fireAndForget(
d *database.Delivery, d *database.Delivery,
res attemptResult, res attemptResult,
) { ) {
c.eng.observeAttempt(d.Target.Type, res.elapsed())
c.eng.recordResult( c.eng.recordResult(
webhookDB, d, 1, res.success, webhookDB, d, 1, res.success,
res.statusCode, res.respBody, res.errMsg, res.statusCode, res.respBody, res.errMsg,
@@ -82,7 +84,7 @@ func (c *httpCore) fireAndForget(
if res.success { if res.success {
c.eng.updateDeliveryStatus( c.eng.updateDeliveryStatus(
webhookDB, d, webhookDB, d, d.Target.Type,
database.DeliveryStatusDelivered, database.DeliveryStatusDelivered,
) )
@@ -90,7 +92,8 @@ func (c *httpCore) fireAndForget(
} }
c.eng.updateDeliveryStatus( c.eng.updateDeliveryStatus(
webhookDB, d, database.DeliveryStatusFailed, webhookDB, d, d.Target.Type,
database.DeliveryStatusFailed,
) )
} }
@@ -107,10 +110,17 @@ func (c *httpCore) withRetry(
return 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 attemptNum := task.AttemptNum
res := attempt() res := attempt()
c.eng.observeAttempt(d.Target.Type, res.elapsed())
c.eng.recordResult( c.eng.recordResult(
webhookDB, d, attemptNum, res.success, webhookDB, d, attemptNum, res.success,
res.statusCode, res.respBody, res.errMsg, res.statusCode, res.respBody, res.errMsg,
@@ -121,7 +131,7 @@ func (c *httpCore) withRetry(
cb.RecordSuccess() cb.RecordSuccess()
c.eng.updateDeliveryStatus( c.eng.updateDeliveryStatus(
webhookDB, d, webhookDB, d, d.Target.Type,
database.DeliveryStatusDelivered, database.DeliveryStatusDelivered,
) )
@@ -146,6 +156,8 @@ func (c *httpCore) circuitBreakerBlock(
return false return false
} }
defer c.publishCircuitState(d.Target.Type)
remaining := cb.CooldownRemaining() remaining := cb.CooldownRemaining()
c.eng.log.Info( c.eng.log.Info(
@@ -157,7 +169,7 @@ func (c *httpCore) circuitBreakerBlock(
) )
c.eng.updateDeliveryStatus( c.eng.updateDeliveryStatus(
webhookDB, d, webhookDB, d, d.Target.Type,
database.DeliveryStatusRetrying, database.DeliveryStatusRetrying,
) )
@@ -177,7 +189,7 @@ func (c *httpCore) handleRetry(
) { ) {
if attemptNum >= maxRetries { if attemptNum >= maxRetries {
c.eng.updateDeliveryStatus( c.eng.updateDeliveryStatus(
webhookDB, d, webhookDB, d, d.Target.Type,
database.DeliveryStatusFailed, database.DeliveryStatusFailed,
) )
@@ -185,7 +197,8 @@ func (c *httpCore) handleRetry(
} }
c.eng.updateDeliveryStatus( c.eng.updateDeliveryStatus(
webhookDB, d, database.DeliveryStatusRetrying, webhookDB, d, d.Target.Type,
database.DeliveryStatusRetrying,
) )
backoff := calcBackoff(attemptNum) backoff := calcBackoff(attemptNum)
@@ -215,6 +228,28 @@ func (c *httpCore) getCircuitBreaker(
return cb 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 // remainingBackoff returns how long remains of the backoff
// window for the last attempt of a recovered retrying // window for the last attempt of a recovered retrying
// delivery. It implements rescheduler. // delivery. It implements rescheduler.
@@ -302,7 +337,8 @@ func (t *httpTarget) Deliver(
) )
t.eng.updateDeliveryStatus( t.eng.updateDeliveryStatus(
webhookDB, d, database.DeliveryStatusFailed, webhookDB, d, d.Target.Type,
database.DeliveryStatusFailed,
) )
return return

View File

@@ -0,0 +1,142 @@
package delivery_test
import (
"context"
"encoding/json"
"net/http"
"testing"
"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/signature"
)
// gitlabDeliverySecret is the shared secret the entrypoint in these
// tests is configured with. No outbound request may contain it.
const gitlabDeliverySecret = "QQDELIVERYSECRETQQ"
// receivedEventHeaders builds the Event.Headers value the receiver
// stores for an inbound request, by running the request's headers
// through the same sanitizer the receive path uses. Going through
// signature.SanitizeHeaders rather than a literal is the point of
// the test: it joins the two egresses at the field they share, so a
// regression at either end shows up here.
func receivedEventHeaders(
t *testing.T,
scheme database.SignatureScheme,
inbound http.Header,
) string {
t.Helper()
ep := &database.Entrypoint{
SignatureScheme: scheme,
SignatureSecret: gitlabDeliverySecret,
}
encoded, err := json.Marshal(
signature.SanitizeHeaders(ep, inbound),
)
require.NoError(t, err)
return string(encoded)
}
// TestApplyRequestHeadersDropsInboundCredential proves a delivery to
// an HTTP target does not carry the GitLab shared secret.
//
// isForwardableHeader is a blocklist of hop-by-hop names, so it
// forwards X-Gitlab-Token like any other header; what keeps the
// secret out of the outbound request is that the receiver never
// stored it. Handing a target operator the token would hand them the
// ability to forge requests to the entrypoint it authenticates,
// which is the one control the receiver has.
func TestApplyRequestHeadersDropsInboundCredential(t *testing.T) {
t.Parallel()
inbound := http.Header{}
inbound.Set(signature.HeaderGitLab, gitlabDeliverySecret)
inbound.Set("X-Gitlab-Event", "Push Hook")
event := &database.Event{
Headers: receivedEventHeaders(
t, database.SignatureSchemeGitLab, inbound,
),
ContentType: "application/json",
}
req, err := http.NewRequestWithContext(
context.Background(),
http.MethodPost,
"https://target.example.com/hook",
http.NoBody,
)
require.NoError(t, err)
delivery.ExportApplyRequestHeaders(
req, event, &delivery.HTTPTargetConfig{},
)
assert.Empty(
t,
req.Header.Values(signature.HeaderGitLab),
"the shared secret header must not reach a target",
)
// Header.Values canonicalises, so a differently-cased spelling
// would be caught above; this catches the value arriving under
// some other name.
for name, values := range req.Header {
for _, v := range values {
assert.NotContains(
t, v, gitlabDeliverySecret,
"secret present in outbound header %s", name,
)
}
}
// The rest of the sender's headers still arrive. A fix that
// dropped everything would pass the assertions above while
// breaking delivery.
assert.Equal(
t,
"Push Hook",
req.Header.Get("X-Gitlab-Event"),
)
}
// TestApplyRequestHeadersKeepsGitHubDigest proves the stripping is
// scoped to headers that carry the secret itself. GitHub's
// X-Hub-Signature-256 is an HMAC over the body, so a target can be
// shown it without being handed the key.
func TestApplyRequestHeadersKeepsGitHubDigest(t *testing.T) {
t.Parallel()
const digest = "sha256=deadbeef"
inbound := http.Header{}
inbound.Set(signature.HeaderGitHub, digest)
event := &database.Event{
Headers: receivedEventHeaders(
t, database.SignatureSchemeGitHub, inbound,
),
}
req, err := http.NewRequestWithContext(
context.Background(),
http.MethodPost,
"https://target.example.com/hook",
http.NoBody,
)
require.NoError(t, err)
delivery.ExportApplyRequestHeaders(
req, event, &delivery.HTTPTargetConfig{},
)
assert.Equal(
t, digest, req.Header.Get(signature.HeaderGitHub),
)
}

View File

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

View File

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

View File

@@ -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",
)
}

View File

@@ -17,6 +17,10 @@
// level the operator controls, they are shaped by whichever handler // level the operator controls, they are shaped by whichever handler
// internal/logger selected, and every value a client can influence is // internal/logger selected, and every value a client can influence is
// spent through logfield.Truncate. // 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 package gormlog
import ( import (
@@ -26,6 +30,7 @@ import (
"log/slog" "log/slog"
"time" "time"
"gorm.io/gorm"
gormlogger "gorm.io/gorm/logger" gormlogger "gorm.io/gorm/logger"
"sneak.berlin/go/webhooker/internal/logfield" "sneak.berlin/go/webhooker/internal/logfield"
) )
@@ -47,8 +52,14 @@ type Logger struct {
} }
// Interface compliance is asserted here rather than discovered at the // 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. // New returns a GORM logger that writes through log.
func New(log *slog.Logger) *Logger { func New(log *slog.Logger) *Logger {
@@ -71,6 +82,46 @@ func (l *Logger) LogMode(gormlogger.LogLevel) gormlogger.Interface {
return l 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. // Info logs one of GORM's own informational messages.
func (l *Logger) Info( func (l *Logger) Info(
ctx context.Context, msg string, data ...any, 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 // 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 — // The arms are ordered exactly as GORM's own Trace orders them —
// non-record-not-found error, then slow, then the routine case — so // non-record-not-found error, then slow, then the routine case — so

View File

@@ -232,7 +232,7 @@ func TestSlowRecordNotFound_IsStillReportedSlow(t *testing.T) {
require.ErrorIs(t, err, gorm.ErrRecordNotFound) require.ErrorIs(t, err, gorm.ErrRecordNotFound)
assert.Contains( assert.Contains(
t, buf.String(), "slow sql statement", t, buf.String(), slowLine,
"a slow statement that missed was not "+ "a slow statement that missed was not "+
"reported as slow", "reported as slow",
) )
@@ -284,9 +284,11 @@ func TestRecordNotFoundFlood_DoesNotGrowWithInput(t *testing.T) {
} }
// TestStatementError_LineIsBounded covers the branch that does log. // 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) { func TestStatementError_LineIsBounded(t *testing.T) {
t.Parallel() t.Parallel()
@@ -313,7 +315,7 @@ func TestStatementError_LineIsBounded(t *testing.T) {
require.Error(t, err) require.Error(t, err)
assert.Contains( assert.Contains(
t, buf.String(), "sql statement failed", t, buf.String(), errorLine,
) )
assertBounded(t, buf.String()) 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 // and would have cost this report, which is the one thing GORM's
// logger gave an operator that nothing else in this service does. // logger gave an operator that nothing else in this service does.
// - routine. The branch an operator reaches by turning the level // - 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) { func TestSucceedingStatement_LineIsBoundedOnEitherArm(t *testing.T) {
t.Parallel() 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 { arms := []struct {
name string name string
slow time.Duration slow time.Duration
want string want string
notWant 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 { for _, a := range arms {

View File

@@ -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)
})
}
}

View File

@@ -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,
)
}
}

View File

@@ -0,0 +1,342 @@
package handlers_test
import (
"net/http"
"net/http/httptest"
"net/url"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session"
)
// submitEntrypointSecret posts the signature configuration form for
// an entrypoint and returns the recorder.
func submitEntrypointSecret(
t *testing.T,
h *handlers.Handlers,
cookies []*http.Cookie,
webhookID, entrypointID, scheme, secret string,
) *httptest.ResponseRecorder {
t.Helper()
form := url.Values{}
form.Set("signature_scheme", scheme)
form.Set("secret", secret)
req := formRequest(
"/source/"+webhookID+"/entrypoints/"+
entrypointID+"/secret",
cookies,
form,
map[string]string{
paramSourceID: webhookID,
entrypointIDParam: entrypointID,
},
)
w := httptest.NewRecorder()
h.HandleEntrypointSecret().ServeHTTP(w, req)
return w
}
// reloadEntrypoint reads an entrypoint back from the database,
// including the columns the model keeps out of JSON.
func reloadEntrypoint(
t *testing.T,
db *database.Database,
id string,
) database.Entrypoint {
t.Helper()
var ep database.Entrypoint
require.NoError(
t, db.DB().Where("id = ?", id).First(&ep).Error,
)
return ep
}
// TestEntrypointSecretSetRotateAndRemove walks the whole lifecycle
// the UI has to support: turning verification on, rotating the secret
// to a new value, and turning it back off.
func TestEntrypointSecretSetRotateAndRemove(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
cookies := authenticatedCookies(
t, sess, deleteTestUserID, deleteTestUsername,
)
wh := seedWebhook(t, db)
ep := seedSignedEntrypoint(
t, db, wh.ID, database.SignatureSchemeNone, "",
)
// Set.
w := submitEntrypointSecret(
t, h, cookies, wh.ID, ep.ID, "github", inboundSecret,
)
require.Equal(t, http.StatusSeeOther, w.Code)
stored := reloadEntrypoint(t, db, ep.ID)
assert.Equal(
t, database.SignatureSchemeGitHub, stored.SignatureScheme,
)
assert.Equal(t, inboundSecret, stored.SignatureSecret)
assert.True(t, stored.SignatureConfigured())
// Rotate: a new secret and a different scheme in one submission.
// The new value is submitted with surrounding whitespace, the way
// a secret pasted out of a password manager arrives; storing that
// verbatim would make every later request fail verification with
// nothing visible on either side to explain it.
const rotated = "QQROTATEDSECRETQQ"
w = submitEntrypointSecret(
t, h, cookies, wh.ID, ep.ID, "gitlab", " "+rotated+"\t",
)
require.Equal(t, http.StatusSeeOther, w.Code)
stored = reloadEntrypoint(t, db, ep.ID)
assert.Equal(
t, database.SignatureSchemeGitLab, stored.SignatureScheme,
)
assert.Equal(t, rotated, stored.SignatureSecret)
// Remove. The secret has to go with the scheme: a stored
// credential nothing reads is one more copy to leak.
w = submitEntrypointSecret(t, h, cookies, wh.ID, ep.ID, "", "")
require.Equal(t, http.StatusSeeOther, w.Code)
stored = reloadEntrypoint(t, db, ep.ID)
assert.Equal(
t, database.SignatureSchemeNone, stored.SignatureScheme,
)
assert.Empty(t, stored.SignatureSecret)
assert.False(t, stored.SignatureConfigured())
}
// TestEntrypointSecretRejectsBadInput proves the form cannot create a
// row the receiver would later have to refuse. Both rejections leave
// the stored configuration untouched rather than half-applied.
func TestEntrypointSecretRejectsBadInput(t *testing.T) {
t.Parallel()
cases := []struct {
name string
scheme string
secret string
}{
{
name: "unsupported scheme",
scheme: "stripe",
secret: inboundSecret,
},
{
name: "scheme with no secret",
scheme: "github",
secret: "",
},
{
// Whitespace is stripped, so a secret of spaces is an
// empty one.
name: "scheme with blank secret",
scheme: "github",
secret: " ",
},
}
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
cookies := authenticatedCookies(
t, sess, deleteTestUserID, deleteTestUsername,
)
for _, tc := range cases {
wh := seedWebhook(t, db)
ep := seedSignedEntrypoint(
t, db, wh.ID,
database.SignatureSchemeGitLab, inboundSecret,
)
w := submitEntrypointSecret(
t, h, cookies, wh.ID, ep.ID, tc.scheme, tc.secret,
)
assert.Equal(
t, http.StatusBadRequest, w.Code, "case %s", tc.name,
)
stored := reloadEntrypoint(t, db, ep.ID)
assert.Equal(
t,
database.SignatureSchemeGitLab,
stored.SignatureScheme,
"case %s", tc.name,
)
assert.Equal(
t, inboundSecret, stored.SignatureSecret,
"case %s", tc.name,
)
}
}
// TestEntrypointSecretRequiresOwnership proves the configuration
// endpoint is bound by the same ownership check as the rest of the
// webhook's pages: another user's entrypoint is a 404, and the secret
// is not touched.
func TestEntrypointSecretRequiresOwnership(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
ep := seedSignedEntrypoint(
t, db, wh.ID,
database.SignatureSchemeGitLab, inboundSecret,
)
stranger := authenticatedCookies(
t, sess, "someone-else", "someoneelse",
)
w := submitEntrypointSecret(
t, h, stranger, wh.ID, ep.ID, "github", "hijacked",
)
assert.Equal(t, http.StatusNotFound, w.Code)
assert.Equal(
t,
inboundSecret,
reloadEntrypoint(t, db, ep.ID).SignatureSecret,
)
}
// TestHandleSourceDetail_MasksEntrypointSecret is the regression test
// for the credential on the entrypoint: the page has to say that
// verification is configured and which header carries it, without the
// secret itself ever reaching the rendered HTML.
func TestHandleSourceDetail_MasksEntrypointSecret(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
)
app := newTestApp(t, &h, &sess, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
seedSignedEntrypoint(
t, db, wh.ID,
database.SignatureSchemeGitHub, inboundSecret,
)
body := renderSourceDetailPage(t, h, sess, wh.ID)
assert.NotContains(t, body, inboundSecret)
assert.Contains(t, body, "GitHub")
assert.Contains(t, body, "X-Hub-Signature-256")
}
// TestEntrypointViewsDropTheSecret pins the projection itself, so the
// barrier survives a template rewrite that stops rendering the field
// the page test above looks at.
func TestEntrypointViewsDropTheSecret(t *testing.T) {
t.Parallel()
views := handlers.NewEntrypointViews([]database.Entrypoint{
{
Path: "p1",
Active: true,
SignatureScheme: database.SignatureSchemeGitHub,
SignatureSecret: inboundSecret,
},
{
Path: "p2",
},
{
// Half a configuration. The receiver 500s every request
// to this row, so the UI must not call it unverified.
Path: "p2a",
SignatureScheme: database.SignatureSchemeGitLab,
},
{
// The other half.
Path: "p2b",
SignatureSecret: inboundSecret,
},
{
// A scheme this build does not know: described as
// unavailable, never echoed back.
Path: "p3",
SignatureScheme: database.SignatureScheme("stripe"),
SignatureSecret: inboundSecret,
},
})
require.Len(t, views, 5)
assert.True(t, views[0].Configured)
assert.Equal(t, "GitHub", views[0].SchemeLabel)
assert.Equal(t, "X-Hub-Signature-256", views[0].SchemeHeader)
assert.False(t, views[1].Configured)
assert.Equal(t, "not verified", views[1].SchemeLabel)
assert.Empty(t, views[1].SchemeHeader)
for _, v := range []handlers.EntrypointView{views[2], views[3]} {
assert.False(t, v.Configured)
assert.Equal(t, "misconfigured", v.SchemeLabel)
assert.Empty(t, v.SchemeHeader)
}
assert.True(t, views[4].Configured)
assert.Equal(t, "(unavailable)", views[4].SchemeLabel)
// The struct has no field that could carry the secret, so this
// fails to compile rather than fails at runtime if one is added
// and populated. The assertion covers the labels it derives.
for _, v := range views {
assert.NotContains(t, v.SchemeLabel, inboundSecret)
assert.NotContains(t, v.SchemeHeader, inboundSecret)
assert.NotContains(t, string(v.Scheme), inboundSecret)
}
}

View File

@@ -0,0 +1,91 @@
package handlers
import (
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/signature"
)
// signatureUnavailable is what an entrypoint's scheme renders as when
// the stored value is not one this build supports. The stored string
// is never echoed as a fallback: it is operator-supplied and the row
// is already in a state the receiver refuses, so the UI says so
// rather than inventing a description for it.
const signatureUnavailable = "(unavailable)"
// signatureNotVerified is the label for an entrypoint that performs
// no inbound verification.
const signatureNotVerified = "not verified"
// signatureMisconfigured is the label for a row holding one half of
// the scheme/secret pair. The receiver answers every request to such
// an entrypoint 500, so calling it "not verified" would describe a
// receiver that is refusing everything as one that is accepting
// everything. The form cannot create the state; a hand-edited
// database or a downgrade past a scheme can.
const signatureMisconfigured = "misconfigured"
// EntrypointView is the display-safe projection of an entrypoint for
// the UI. It deliberately has no secret field, so no template —
// present or future — can render the shared secret, in the same way
// delivery.TargetView keeps a target's stored credential away from
// one.
type EntrypointView struct {
ID string
Path string
Description string
Active bool
// Configured reports whether inbound requests to this entrypoint
// are verified.
Configured bool
// Scheme is the stored scheme, carried so the form can preselect
// it. It names an algorithm, not a secret.
Scheme database.SignatureScheme
// SchemeLabel and SchemeHeader describe the configured scheme for
// display: the sender's name, and the header its signature
// arrives in.
SchemeLabel string
SchemeHeader string
}
// NewEntrypointViews projects entrypoints for rendering, dropping the
// shared secret on the way.
func NewEntrypointViews(
entrypoints []database.Entrypoint,
) []EntrypointView {
views := make([]EntrypointView, 0, len(entrypoints))
for i := range entrypoints {
e := &entrypoints[i]
view := EntrypointView{
ID: e.ID,
Path: e.Path,
Description: e.Description,
Active: e.Active,
Configured: e.SignatureConfigured(),
Scheme: e.SignatureScheme,
SchemeLabel: signatureNotVerified,
SchemeHeader: "",
}
switch {
case view.Configured:
view.SchemeLabel = signatureUnavailable
info, ok := signature.Info(e.SignatureScheme)
if ok {
view.SchemeLabel = info.Label
view.SchemeHeader = info.Header
}
case e.SignatureHalfConfigured():
view.SchemeLabel = signatureMisconfigured
}
views = append(views, view)
}
return views
}

View File

@@ -69,18 +69,29 @@ func (s *Handlers) RenderTemplateForTest(
s.renderTemplate(w, r, pageTemplate, data) 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( func (s *Handlers) BuildSlackTargetConfigForTest(
w http.ResponseWriter, w http.ResponseWriter,
r *http.Request, r *http.Request,
targetURL string, targetURL string,
) (string, error) { ) (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 // BuildDatabaseTargetConfigForTest exposes

View File

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

View File

@@ -11,6 +11,7 @@ import (
"github.com/google/uuid" "github.com/google/uuid"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery" "sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/signature"
) )
// WebhookListItem holds data for the webhook list view. // WebhookListItem holds data for the webhook list view.
@@ -414,13 +415,16 @@ func (h *Handlers) renderSourceDetail(
// receivers; html/template cannot address a value stored in a map. // receivers; html/template cannot address a value stored in a map.
data := map[string]any{ data := map[string]any{
tmplKeyWebhook: &webhook, tmplKeyWebhook: &webhook,
"Entrypoints": entrypoints, // Entrypoints and targets are both projected to
// Targets are projected to a display-safe view: the // display-safe views: an entrypoint carries the shared
// stored config blob holds credentials and must never // secret its senders sign with and a target's stored
// config blob holds a credential, and neither must ever
// reach a template. // reach a template.
"Targets": delivery.NewTargetViews(targets), "Entrypoints": NewEntrypointViews(entrypoints),
"Events": events, "Targets": delivery.NewTargetViews(targets),
"BaseURL": scheme + "://" + host, "SignatureSchemes": signature.Schemes(),
"Events": events,
"BaseURL": scheme + "://" + host,
} }
h.renderTemplate(w, r, "source_detail.html", data) h.renderTemplate(w, r, "source_detail.html", data)
@@ -972,6 +976,145 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
} }
} }
// HandleEntrypointSecret sets, rotates or removes the shared secret
// an entrypoint verifies inbound requests with.
//
// Setting and rotating are the same operation: the form always takes
// the secret afresh and the stored value is never sent to the browser
// to be edited, so there is no path by which the page can display a
// credential it holds. Rotation is therefore "submit the new secret",
// and the operator already has that value — both supported senders
// require them to enter the same string on the sender's side, so
// there is no generated value for webhooker to reveal once.
func (h *Handlers) HandleEntrypointSecret() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
webhook, ok := h.ownedWebhook(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
}
var entrypoint database.Entrypoint
err = h.db.DB().Where(
"id = ? AND webhook_id = ?",
chi.URLParam(r, "entrypointID"), webhook.ID,
).First(&entrypoint).Error
if err != nil {
http.NotFound(w, r)
return
}
h.applyEntrypointSecret(w, r, &entrypoint)
}
}
// applyEntrypointSecret validates the submitted scheme and secret and
// stores them.
//
// A scheme this build does not support is a 400, never a stored value
// the receiver would later have to interpret: the receiver fails such
// a row closed, so letting one be created would take the entrypoint
// offline through a form that reported success.
func (h *Handlers) applyEntrypointSecret(
w http.ResponseWriter,
r *http.Request,
entrypoint *database.Entrypoint,
) {
// PostFormValue, not FormValue: a credential must come from the
// body. FormValue falls back to the query string, and the request
// line — unlike the body — is what logs, proxies, Referer headers
// and error trackers record.
scheme := database.SignatureScheme(
r.PostFormValue("signature_scheme"),
)
// Surrounding whitespace is stripped, because a secret pasted from
// a password manager routinely carries some and the resulting
// mismatch is undiagnosable from the sender's side. A secret whose
// own first or last character is a space cannot be stored; the
// README says so.
secret := strings.TrimSpace(r.PostFormValue("secret"))
if !signature.Supported(scheme) {
http.Error(
w, "Invalid signature scheme",
http.StatusBadRequest,
)
return
}
if scheme == database.SignatureSchemeNone {
// Turning verification off drops the secret with it: a stored
// credential nothing reads is one more copy to leak, and
// Verify refuses that pairing in any case.
secret = ""
} else if secret == "" {
http.Error(
w,
"A shared secret is required for this signature scheme.",
http.StatusBadRequest,
)
return
}
h.storeEntrypointSecret(w, r, entrypoint, scheme, secret)
}
// storeEntrypointSecret writes a validated scheme and secret to an
// entrypoint and returns the operator to the webhook page.
func (h *Handlers) storeEntrypointSecret(
w http.ResponseWriter,
r *http.Request,
entrypoint *database.Entrypoint,
scheme database.SignatureScheme,
secret string,
) {
// Updates with a map rather than a struct: a struct update skips
// zero values, and the empty pair is exactly what has to be
// written when verification is being turned off.
err := h.db.DB().Model(entrypoint).Updates(map[string]any{
"signature_scheme": scheme,
"signature_secret": secret,
}).Error
if err != nil {
// The error is logged by serverError; GORM's error text
// carries the statement, not the bound values, so the secret
// does not travel with it.
h.serverError(
w, "failed to update entrypoint signature", err,
)
return
}
h.log.Info(
"entrypoint signature configuration updated",
"entrypoint_id", entrypoint.ID,
"webhook_id", entrypoint.WebhookID,
"scheme", string(scheme),
)
http.Redirect(
w, r,
"/source/"+entrypoint.WebhookID,
http.StatusSeeOther,
)
}
// HandleTargetCreate handles adding a new target to a webhook. // HandleTargetCreate handles adding a new target to a webhook.
func (h *Handlers) HandleTargetCreate() http.HandlerFunc { func (h *Handlers) HandleTargetCreate() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
@@ -1029,9 +1172,7 @@ func (h *Handlers) processTargetCreate(
// Referer headers and error trackers record. // Referer headers and error trackers record.
name := r.PostFormValue("name") name := r.PostFormValue("name")
targetType := database.TargetType(r.PostFormValue("type")) targetType := database.TargetType(r.PostFormValue("type"))
targetURL := r.PostFormValue("url")
maxRetriesStr := r.PostFormValue("max_retries") maxRetriesStr := r.PostFormValue("max_retries")
expiry := r.PostFormValue("expiry")
if name == "" { if name == "" {
http.Error( http.Error(
@@ -1051,7 +1192,7 @@ func (h *Handlers) processTargetCreate(
} }
configJSON, err := h.buildTargetConfig( configJSON, err := h.buildTargetConfig(
w, r, targetType, targetURL, expiry, w, r, targetType, targetFormInputFrom(r),
) )
if err != nil { if err != nil {
return return
@@ -1108,28 +1249,60 @@ func parseNonNegativeInt(s string) int {
return 0 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( func (h *Handlers) buildTargetConfig(
w http.ResponseWriter, w http.ResponseWriter,
r *http.Request, r *http.Request,
targetType database.TargetType, targetType database.TargetType,
targetURL, expiry string, in targetFormInput,
) (string, error) { ) (string, error) {
switch targetType { switch targetType {
case database.TargetTypeHTTP: 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: 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: case database.TargetTypeDatabase:
return h.buildDatabaseTargetConfig(w, expiry) return h.buildDatabaseTargetConfig(w, in.Expiry)
case database.TargetTypeLog: case database.TargetTypeLog:
return "", nil return "", nil
default: default:
@@ -1142,14 +1315,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, w http.ResponseWriter,
r *http.Request, r *http.Request,
targetURL, configKey, missingMsg string, in targetFormInput,
) (string, error) { ) (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 == "" { if targetURL == "" {
http.Error( http.Error(
w, w,
@@ -1157,7 +1399,7 @@ func (h *Handlers) buildURLTargetConfig(
http.StatusBadRequest, http.StatusBadRequest,
) )
return "", errMissingURL return errMissingURL
} }
err := delivery.ValidateTargetURL( err := delivery.ValidateTargetURL(
@@ -1178,11 +1420,18 @@ func (h *Handlers) buildURLTargetConfig(
http.StatusBadRequest, 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) configBytes, err := json.Marshal(cfg)
if err != nil { if err != nil {
http.Error( http.Error(
@@ -1222,19 +1471,9 @@ func (h *Handlers) buildDatabaseTargetConfig(
return "", err 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. // HandleEntrypointDelete handles deleting an entrypoint.

View File

@@ -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
}

View File

@@ -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)
}

View File

@@ -13,6 +13,7 @@ import (
"sneak.berlin/go/webhooker/internal/delivery" "sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
"sneak.berlin/go/webhooker/internal/signature"
) )
// Template data keys the page templates read. The handlers package has // Template data keys the page templates read. The handlers package has
@@ -268,12 +269,16 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
body := renderPage(t, h, sess, "source_detail.html", map[string]any{ body := renderPage(t, h, sess, "source_detail.html", map[string]any{
dataKeyWebhook: webhook, dataKeyWebhook: webhook,
"Entrypoints": []database.Entrypoint{entrypoint}, // The handler passes projected views, never raw rows — an
// The handler passes delivery.NewTargetViews(targets), never // entrypoint carries its shared secret and a target its
// raw targets, so the test data has to have that same shape. // stored credential — so the test data has that same shape.
"Targets": delivery.NewTargetViews(nil), "Entrypoints": handlers.NewEntrypointViews(
"Events": []database.Event{}, []database.Entrypoint{entrypoint},
"BaseURL": "https://hooks.example.com", ),
"Targets": delivery.NewTargetViews(nil),
"SignatureSchemes": signature.Schemes(),
"Events": []database.Event{},
"BaseURL": "https://hooks.example.com",
}) })
assert.Contains( assert.Contains(

View File

@@ -2,6 +2,7 @@ package handlers
import ( import (
"encoding/json" "encoding/json"
"errors"
"io" "io"
"net/http" "net/http"
@@ -10,6 +11,7 @@ import (
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery" "sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/logfield" "sneak.berlin/go/webhooker/internal/logfield"
"sneak.berlin/go/webhooker/internal/signature"
) )
const ( const (
@@ -69,8 +71,8 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
} }
} }
// processWebhookRequest reads the body, serializes headers, // processWebhookRequest reads the body, verifies the sender,
// loads targets, and delivers the event. // serializes headers, loads targets, and delivers the event.
func (h *Handlers) processWebhookRequest( func (h *Handlers) processWebhookRequest(
w http.ResponseWriter, w http.ResponseWriter,
r *http.Request, r *http.Request,
@@ -81,7 +83,26 @@ func (h *Handlers) processWebhookRequest(
return return
} }
headersJSON, err := json.Marshal(r.Header) // Before anything is written. An unverified request must leave no
// event row, no delivery row and no delivery task behind, so this
// sits above every write rather than inside the transaction that
// performs them. It has to sit below the body read because the
// signature is computed over the body; readWebhookBody is what
// bounds that read, so an unauthenticated sender still cannot make
// the process hold more than the 1 MB cap.
if !h.verifyInboundSignature(w, entrypoint, r.Header, body) {
return
}
// These headers are about to be stored verbatim and handed to
// every delivery target, so the scheme's credential comes out
// first. Under GitLab's scheme the header is the shared secret
// itself, and leaving it in would hand the ability to forge
// signed requests to exactly the parties the signature is meant
// to exclude.
headersJSON, err := json.Marshal(
signature.SanitizeHeaders(&entrypoint, r.Header),
)
if err != nil { if err != nil {
h.serverError(w, "failed to serialize headers", err) h.serverError(w, "failed to serialize headers", err)
@@ -100,6 +121,63 @@ func (h *Handlers) processWebhookRequest(
) )
} }
// verifyInboundSignature authenticates the request against the
// entrypoint's configured secret, reporting false once it has written
// the response.
//
// An entrypoint with no secret configured is not checked and this
// returns true, which is the unchanged behaviour every existing
// entrypoint keeps.
//
// A configuration that cannot be applied — an unknown scheme, or one
// half of the pair missing — is a 500, not a 401: the request may well
// be authentic, and calling it unauthorized would tell a legitimate
// sender to go fix its own signing. Either way it is refused. Failing
// open here would mean an entrypoint the operator has protected
// quietly accepting anything.
func (h *Handlers) verifyInboundSignature(
w http.ResponseWriter,
entrypoint database.Entrypoint,
header http.Header,
body []byte,
) bool {
err := signature.Verify(&entrypoint, header, body)
if err == nil {
return true
}
if errors.Is(err, signature.ErrConfig) {
h.log.Error(
"entrypoint signature configuration cannot be applied",
"entrypoint_id", entrypoint.ID,
"webhook_id", entrypoint.WebhookID,
"error", err,
)
http.Error(
w, "Internal server error",
http.StatusInternalServerError,
)
return false
}
// Every field here is bounded and none is client-chosen: the ids
// are ours, the scheme is one of a fixed set, and the error is a
// static string carrying no part of the secret or of what the
// client presented. Reaching this line also requires a real
// entrypoint UUID, so it is not a line a stranger can drive.
h.log.Warn(
"inbound signature verification failed",
"entrypoint_id", entrypoint.ID,
"webhook_id", entrypoint.WebhookID,
"scheme", string(entrypoint.SignatureScheme),
"error", err,
)
http.Error(w, "Unauthorized", http.StatusUnauthorized)
return false
}
// loadActiveTargets returns all active targets for a webhook. // loadActiveTargets returns all active targets for a webhook.
func (h *Handlers) loadActiveTargets( func (h *Handlers) loadActiveTargets(
webhookID string, webhookID string,
@@ -217,6 +295,11 @@ func (h *Handlers) createAndDeliverEvent(
return 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) h.finishWebhookResponse(w, event, entrypoint, tasks)
} }

View File

@@ -0,0 +1,468 @@
package handlers_test
import (
"bytes"
"context"
"crypto/hmac"
"crypto/sha256"
"encoding/hex"
"log/slog"
"net/http"
"net/http/httptest"
"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/handlers"
"sneak.berlin/go/webhooker/internal/signature"
)
const (
// inboundSecret is the shared secret the signed-receiver tests
// configure on their entrypoint. It doubles as a marker: no log
// line and no rendered page may contain it.
inboundSecret = "QQINBOUNDSECRETQQ"
// inboundBody is the payload the sender signs.
inboundBody = `{"zen":"Non-blocking is better than blocking."}`
// entrypointIDParam is the chi URL parameter naming an entrypoint.
entrypointIDParam = "entrypointID"
)
// hubSignature returns the X-Hub-Signature-256 value a GitHub sender
// holding secret would send for inboundBody.
func hubSignature(secret string) string {
mac := hmac.New(sha256.New, []byte(secret))
_, _ = mac.Write([]byte(inboundBody))
return "sha256=" + hex.EncodeToString(mac.Sum(nil))
}
// seedSignedEntrypoint inserts an active entrypoint for a webhook
// with the given signature configuration and returns it.
func seedSignedEntrypoint(
t *testing.T,
db *database.Database,
webhookID string,
scheme database.SignatureScheme,
secret string,
) *database.Entrypoint {
t.Helper()
ep := &database.Entrypoint{
WebhookID: webhookID,
Path: "path-" + webhookID,
Description: "signed",
Active: true,
SignatureScheme: scheme,
SignatureSecret: secret,
}
require.NoError(
t,
db.DB().Omit(clause.Associations).Create(ep).Error,
)
return ep
}
// postToEntrypoint drives the real receiver handler at an
// entrypoint's path with one optional header set.
func postToEntrypoint(
t *testing.T,
h *handlers.Handlers,
path, body, headerName, headerValue string,
) *httptest.ResponseRecorder {
t.Helper()
req := httptest.NewRequestWithContext(
context.Background(),
http.MethodPost,
"/webhook/"+path,
strings.NewReader(body),
)
req.Header.Set("Content-Type", "application/json")
if headerName != "" {
req.Header.Set(headerName, headerValue)
}
rctx := chi.NewRouteContext()
rctx.URLParams.Add("uuid", path)
req = req.WithContext(
context.WithValue(req.Context(), chi.RouteCtxKey, rctx),
)
w := httptest.NewRecorder()
h.HandleWebhook().ServeHTTP(w, req)
return w
}
// storedEvents counts the event rows a webhook's per-webhook database
// holds. A database that was never opened holds none, which is the
// state a rejected request has to leave behind.
func storedEvents(
t *testing.T,
mgr *database.WebhookDBManager,
webhookID string,
) int64 {
t.Helper()
if !mgr.DBExists(webhookID) {
return 0
}
db, err := mgr.GetDB(webhookID)
require.NoError(t, err)
var count int64
require.NoError(
t,
db.Model(&database.Event{}).
Where("webhook_id = ?", webhookID).
Count(&count).Error,
)
return count
}
// storedEventHeaders reads back the Headers column of the single
// event row a webhook's per-webhook database holds.
//
// It reads the database rather than an in-memory struct on purpose:
// what matters is what an operator, a backup or the reaper's archive
// would find on disk, not what the handler passed around.
func storedEventHeaders(
t *testing.T,
mgr *database.WebhookDBManager,
webhookID string,
) string {
t.Helper()
require.True(t, mgr.DBExists(webhookID))
db, err := mgr.GetDB(webhookID)
require.NoError(t, err)
var events []database.Event
require.NoError(
t,
db.Where("webhook_id = ?", webhookID).
Find(&events).Error,
)
require.Len(t, events, 1)
return events[0].Headers
}
// signedReceiverCase is one inbound request against an entrypoint
// with a given stored signature configuration.
type signedReceiverCase struct {
name string
scheme database.SignatureScheme
secret string
headerName string
headerValue string
body string
wantStatus int
}
// signedReceiverCases covers each supported scheme with a valid
// signature, an invalid one and none at all, plus the two states that
// are not "a client got it wrong": an entrypoint with nothing
// configured, and one whose stored configuration cannot be applied.
func signedReceiverCases() []signedReceiverCase {
return append(
schemeReceiverCases(), unverifiedReceiverCases()...,
)
}
// schemeReceiverCases covers the two supported schemes.
func schemeReceiverCases() []signedReceiverCase {
return []signedReceiverCase{
{
name: "github valid",
scheme: database.SignatureSchemeGitHub,
secret: inboundSecret,
headerName: signature.HeaderGitHub,
headerValue: hubSignature(inboundSecret),
body: inboundBody,
wantStatus: http.StatusOK,
},
{
name: "github wrong secret",
scheme: database.SignatureSchemeGitHub,
secret: inboundSecret,
headerName: signature.HeaderGitHub,
headerValue: hubSignature("wrong"),
body: inboundBody,
wantStatus: http.StatusUnauthorized,
},
{
// A digest that was valid for a different body: the
// check is over the bytes as received.
name: "github body tampered",
scheme: database.SignatureSchemeGitHub,
secret: inboundSecret,
headerName: signature.HeaderGitHub,
headerValue: hubSignature(inboundSecret),
body: inboundBody + " ",
wantStatus: http.StatusUnauthorized,
},
{
name: "github unsigned",
scheme: database.SignatureSchemeGitHub,
secret: inboundSecret,
body: inboundBody,
wantStatus: http.StatusUnauthorized,
},
{
name: "gitlab valid",
scheme: database.SignatureSchemeGitLab,
secret: inboundSecret,
headerName: signature.HeaderGitLab,
headerValue: inboundSecret,
body: inboundBody,
wantStatus: http.StatusOK,
},
{
name: "gitlab wrong token",
scheme: database.SignatureSchemeGitLab,
secret: inboundSecret,
headerName: signature.HeaderGitLab,
headerValue: "wrong",
body: inboundBody,
wantStatus: http.StatusUnauthorized,
},
{
name: "gitlab unsigned",
scheme: database.SignatureSchemeGitLab,
secret: inboundSecret,
body: inboundBody,
wantStatus: http.StatusUnauthorized,
},
}
}
// unverifiedReceiverCases covers the two entrypoint states that are
// not about a client getting its signature wrong: nothing configured
// at all, and a configuration the receiver cannot apply.
func unverifiedReceiverCases() []signedReceiverCase {
return []signedReceiverCase{
{
// The pass-through case. An entrypoint with nothing
// configured is what every deployment already has, and
// it must keep accepting unsigned requests so that an
// upgrade does not lock an operator out of their own
// receivers.
name: "unconfigured accepts unsigned",
scheme: database.SignatureSchemeNone,
body: inboundBody,
wantStatus: http.StatusOK,
},
{
// A stray signature header changes nothing when nothing
// is configured to check it.
name: "unconfigured ignores a stray header",
scheme: database.SignatureSchemeNone,
headerName: signature.HeaderGitHub,
headerValue: "sha256=deadbeef",
body: inboundBody,
wantStatus: http.StatusOK,
},
{
// A scheme this build cannot apply, reachable only by
// editing the database: refused, not waved through as
// unverified.
name: "unknown scheme fails closed",
scheme: database.SignatureScheme("stripe"),
secret: inboundSecret,
headerName: signature.HeaderGitHub,
headerValue: hubSignature(inboundSecret),
body: inboundBody,
wantStatus: http.StatusInternalServerError,
},
}
}
// TestReceiverVerifiesConfiguredEntrypoints is the load-bearing test
// for the feature: for each supported scheme a correctly signed
// request is accepted and stored, and an incorrectly signed or
// unsigned one is answered 401 having stored nothing.
//
// The event count is the half that matters most. A rejection that
// still wrote a row would leave the receiver a place for a stranger
// who knows a URL to deposit content, which is exactly what the
// signature is there to prevent.
//
// The cases share one application and take a webhook each, rather
// than each standing up its own: every newTestApp seeds an admin user
// and so pays an Argon2id hash at 64 MB, and this package's test
// budget is not large enough to spend one per table row.
func TestReceiverVerifiesConfiguredEntrypoints(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
db *database.Database
mgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &db, &mgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
for _, tc := range signedReceiverCases() {
wh := seedWebhook(t, db)
ep := seedSignedEntrypoint(
t, db, wh.ID, tc.scheme, tc.secret,
)
w := postToEntrypoint(
t, h, ep.Path, tc.body,
tc.headerName, tc.headerValue,
)
assert.Equal(t, tc.wantStatus, w.Code, "case %s", tc.name)
want := int64(0)
if tc.wantStatus == http.StatusOK {
want = 1
}
assert.Equal(
t, want, storedEvents(t, mgr, wh.ID),
"case %s: stored event rows after a %d response",
tc.name, w.Code,
)
}
}
// TestReceiverLogsNoSecret proves the rejection path does not write
// the shared secret, or what the client presented, into the log. A
// GitLab token arrives as the credential itself, so echoing the
// header value would put a live secret in the log of every deployment
// whose sender is briefly misconfigured.
func TestReceiverLogsNoSecret(t *testing.T) {
t.Parallel()
const presented = "QQPRESENTEDVALUEQQ"
var (
h *handlers.Handlers
db *database.Database
)
app := newTestApp(t, &h, &db)
app.RequireStart()
t.Cleanup(app.RequireStop)
var buf bytes.Buffer
h.SetLogForTest(slog.New(slog.NewJSONHandler(&buf, nil)))
wh := seedWebhook(t, db)
ep := seedSignedEntrypoint(
t, db, wh.ID,
database.SignatureSchemeGitLab, inboundSecret,
)
w := postToEntrypoint(
t, h, ep.Path, inboundBody,
signature.HeaderGitLab, presented,
)
require.Equal(t, http.StatusUnauthorized, w.Code)
// The rejection is recorded at all — a silent 401 leaves an
// operator no way to see a sender failing to authenticate.
assert.Contains(t, buf.String(), "verification failed")
assert.NotContains(t, buf.String(), inboundSecret)
assert.NotContains(t, buf.String(), presented)
}
// TestReceiverDoesNotStoreInboundCredential proves an accepted
// request leaves no copy of the shared secret in the event store.
//
// GitLab's X-Gitlab-Token is the credential itself, not a digest
// over the request. Stored headers are read back by the UI, copied
// into every backup and archive, and handed verbatim to every
// delivery target, so a stored token is the entrypoint's only
// authentication control disclosed to precisely the parties it
// exists to exclude.
//
// The two cases share one application: every newTestApp seeds an
// admin user and pays an Argon2id hash at 64 MB, and this package's
// test budget does not stretch to one per case.
func TestReceiverDoesNotStoreInboundCredential(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
db *database.Database
mgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &db, &mgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
gitlab := seedWebhook(t, db)
gitlabEP := seedSignedEntrypoint(
t, db, gitlab.ID,
database.SignatureSchemeGitLab, inboundSecret,
)
w := postToEntrypoint(
t, h, gitlabEP.Path, inboundBody,
signature.HeaderGitLab, inboundSecret,
)
require.Equal(t, http.StatusOK, w.Code)
stored := storedEventHeaders(t, mgr, gitlab.ID)
assert.NotContains(
t, stored, inboundSecret,
"the shared secret must not be persisted",
)
assert.NotContains(
t, stored, signature.HeaderGitLab,
"the credential header must not be persisted at all",
)
// Everything else the sender set is still there. A fix that
// stored no headers would satisfy the assertions above while
// discarding the record the receiver exists to keep.
assert.Contains(t, stored, "Content-Type")
// A GitHub digest is an HMAC over the body, so the key cannot be
// recovered from it and it stays: the stripping is scoped to
// what actually carries the secret.
github := seedWebhook(t, db)
githubEP := seedSignedEntrypoint(
t, db, github.ID,
database.SignatureSchemeGitHub, inboundSecret,
)
w = postToEntrypoint(
t, h, githubEP.Path, inboundBody,
signature.HeaderGitHub, hubSignature(inboundSecret),
)
require.Equal(t, http.StatusOK, w.Code)
stored = storedEventHeaders(t, mgr, github.ID)
assert.Contains(t, stored, signature.HeaderGitHub)
assert.NotContains(t, stored, inboundSecret)
}

344
internal/metrics/metrics.go Normal file
View File

@@ -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
}

View File

@@ -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)
}

View File

@@ -213,7 +213,25 @@ func (s *Server) setupSourceRoutes() {
"/entrypoints/{entrypointID}/toggle", "/entrypoints/{entrypointID}/toggle",
s.h.HandleEntrypointToggle(), s.h.HandleEntrypointToggle(),
) )
r.Post(
"/entrypoints/{entrypointID}/secret",
s.h.HandleEntrypointSecret(),
)
r.Post("/targets", s.h.HandleTargetCreate()) 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( r.Post(
"/targets/{targetID}/delete", "/targets/{targetID}/delete",
s.h.HandleTargetDelete(), s.h.HandleTargetDelete(),

View File

@@ -0,0 +1,283 @@
// Package signature verifies that an inbound webhook request really
// came from the sender an entrypoint was configured for.
//
// Verification is optional and per entrypoint. An entrypoint with no
// scheme configured is not verified at all, which is what every
// entrypoint was before this package existed. An entrypoint whose
// configuration is present but incoherent is failed closed, never
// treated as unverified: the whole point of the feature is that
// turning it on cannot silently turn itself back off.
package signature
import (
"crypto/hmac"
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"net/http"
"strings"
"sneak.berlin/go/webhooker/internal/database"
)
// Header names each supported scheme reads its signature from.
const (
// HeaderGitHub is GitHub's HMAC-SHA256 signature header. GitHub
// also sends the older SHA-1 X-Hub-Signature; it is not accepted.
HeaderGitHub = "X-Hub-Signature-256"
// HeaderGitLab is GitLab's plain shared-token header.
HeaderGitLab = "X-Gitlab-Token"
)
// githubPrefix is the algorithm label GitHub puts in front of the hex
// digest. It is required, not optional: accepting a bare digest too
// would mean accepting a spelling no supported sender produces.
const githubPrefix = "sha256="
// ErrConfig marks a failure caused by the entrypoint's stored
// configuration rather than by the request. A caller must fail these
// closed — refuse the request — because the alternative is an
// entrypoint the operator believes is verified silently accepting
// anything.
var ErrConfig = errors.New("entrypoint signature configuration invalid")
// ErrUnauthorized marks a request that failed verification. A caller
// answers these 401.
var ErrUnauthorized = errors.New("inbound signature verification failed")
// Configuration failures. None of these carry any part of the secret.
var (
errSchemeUnknown = fmt.Errorf(
"%w: unsupported scheme", ErrConfig,
)
errSecretMissing = fmt.Errorf(
"%w: scheme set with no secret", ErrConfig,
)
errSchemeMissing = fmt.Errorf(
"%w: secret set with no scheme", ErrConfig,
)
)
// Request failures. These are logged, so none of them carries the
// value the client sent: under the GitLab scheme that value is a
// guess at the token, and under either scheme a misconfigured sender
// could be presenting the real one.
var (
errHeaderMissing = fmt.Errorf(
"%w: signature header absent", ErrUnauthorized,
)
errHeaderMalformed = fmt.Errorf(
"%w: signature header malformed", ErrUnauthorized,
)
errSignatureMismatch = fmt.Errorf(
"%w: signature does not match", ErrUnauthorized,
)
)
// SchemeInfo describes one supported scheme for the UI.
type SchemeInfo struct {
Scheme database.SignatureScheme
Label string
Header string
// HeaderIsDigest reports that Header carries a value derived from
// the request rather than the shared secret itself, and so may be
// kept when the request is stored and forwarded.
//
// The polarity is deliberate: false — the zero value — means the
// header is the credential and must be stripped. A scheme added
// later is therefore stripped unless whoever adds it positively
// declares the header safe to keep.
HeaderIsDigest bool
}
// Schemes returns the supported schemes in the order the UI offers
// them. It returns a fresh slice per call so no caller can edit the
// set out from under another.
func Schemes() []SchemeInfo {
return []SchemeInfo{
{
Scheme: database.SignatureSchemeGitHub,
Label: "GitHub",
Header: HeaderGitHub,
// An HMAC over the body, not the key. Keeping it lets an
// operator see what the sender sent.
HeaderIsDigest: true,
},
{
Scheme: database.SignatureSchemeGitLab,
Label: "GitLab",
Header: HeaderGitLab,
// X-Gitlab-Token is the shared secret in plaintext.
HeaderIsDigest: false,
},
}
}
// Info returns the description of a supported scheme. It reports
// false for the empty scheme and for anything unrecognised, which is
// what a row hand-edited in the database could hold.
func Info(scheme database.SignatureScheme) (SchemeInfo, bool) {
for _, s := range Schemes() {
if s.Scheme == scheme {
return s, true
}
}
return SchemeInfo{}, false
}
// Supported reports whether a scheme may be stored on an entrypoint.
// The empty scheme is supported: it means no verification.
func Supported(scheme database.SignatureScheme) bool {
if scheme == database.SignatureSchemeNone {
return true
}
_, ok := Info(scheme)
return ok
}
// SanitizeHeaders returns a copy of an accepted request's headers
// with the entrypoint's credential removed.
//
// Under a scheme whose header is the shared secret itself — GitLab's
// X-Gitlab-Token — every downstream use of the inbound headers is a
// disclosure of the credential: they are persisted verbatim in the
// per-webhook event store and forwarded to every delivery target, so
// a target operator or anyone who reads the event database could
// forge signed requests to the very entrypoint the secret protects.
// Stripping happens here, once, above the first write, rather than
// at each egress, so a new consumer of Event.Headers cannot reopen
// the leak by forgetting to filter.
//
// header is never modified; the caller's request keeps its headers
// intact for anything that still needs the original.
//
// An entrypoint with no scheme, or one whose stored scheme this
// build does not know, is returned unchanged: there is no configured
// credential to remove, and the unknown case is refused by Verify
// before a request reaches storage.
func SanitizeHeaders(
entrypoint *database.Entrypoint,
header http.Header,
) http.Header {
clone := header.Clone()
if clone == nil {
return header
}
info, ok := Info(entrypoint.SignatureScheme)
if !ok || info.HeaderIsDigest {
return clone
}
clone.Del(info.Header)
return clone
}
// Verify checks an inbound request against an entrypoint's
// configuration and returns nil when the request may be accepted.
//
// body must be the raw bytes exactly as received, before any parsing
// or normalisation: the sender computed its digest over those bytes,
// so anything that re-encodes them produces a different digest and a
// spurious rejection. The caller is also responsible for bounding
// that read; this package hashes what it is handed.
//
// Every non-nil error is either ErrConfig or ErrUnauthorized, so a
// caller can tell "the server is misconfigured" from "the client did
// not authenticate" with errors.Is.
func Verify(
entrypoint *database.Entrypoint,
header http.Header,
body []byte,
) error {
scheme := entrypoint.SignatureScheme
secret := entrypoint.SignatureSecret
if scheme == database.SignatureSchemeNone {
// A secret with no scheme names no header and no algorithm,
// so there is nothing to check it with. Accepting the request
// would make a half-applied configuration indistinguishable
// from no configuration at all.
if secret != "" {
return errSchemeMissing
}
return nil
}
if secret == "" {
return errSecretMissing
}
switch scheme {
case database.SignatureSchemeGitHub:
return verifyGitHub(secret, header.Get(HeaderGitHub), body)
case database.SignatureSchemeGitLab:
return verifyGitLab(secret, header.Get(HeaderGitLab))
case database.SignatureSchemeNone:
// Handled above; restated so the switch stays exhaustive and
// adding a scheme has to be decided here.
return nil
default:
return errSchemeUnknown
}
}
// verifyGitHub checks a GitHub-style X-Hub-Signature-256: the string
// "sha256=" followed by the hex HMAC-SHA256 of the raw body under the
// shared secret.
func verifyGitHub(secret, provided string, body []byte) error {
if provided == "" {
return errHeaderMissing
}
encoded, ok := strings.CutPrefix(provided, githubPrefix)
if !ok {
return errHeaderMalformed
}
got, err := hex.DecodeString(encoded)
if err != nil {
return errHeaderMalformed
}
mac := hmac.New(sha256.New, []byte(secret))
// hash.Hash.Write is documented never to return an error.
_, _ = mac.Write(body)
// hmac.Equal, never ==: string comparison stops at the first
// differing byte, which tells a client how much of a forged
// digest it got right and turns forgery into a per-byte search.
if !hmac.Equal(mac.Sum(nil), got) {
return errSignatureMismatch
}
return nil
}
// verifyGitLab checks a GitLab-style X-Gitlab-Token, which is the
// shared secret itself rather than a digest over the body.
//
// The comparison is constant time in the same way as the HMAC one.
// hmac.Equal returns early for unequal lengths, so the length of the
// token is not hidden; its contents are, and length alone does not
// let a client search for the value.
func verifyGitLab(secret, provided string) error {
if provided == "" {
return errHeaderMissing
}
if !hmac.Equal([]byte(provided), []byte(secret)) {
return errSignatureMismatch
}
return nil
}

View File

@@ -0,0 +1,340 @@
package signature_test
import (
"crypto/hmac"
"crypto/sha256"
"encoding/hex"
"net/http"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/signature"
)
const (
// testSharedKey is the shared secret under test. It is not named
// "secret": gosec reads a credential-shaped name bound to a
// high-entropy literal as a leaked credential, which is the right
// rule and the wrong finding here.
testSharedKey = "s3kr1t-shared-value"
testBody = `{"action":"opened","number":1}`
)
// githubSignature returns the X-Hub-Signature-256 value GitHub would
// send for testBody signed with secret.
func githubSignature(secret string) string {
mac := hmac.New(sha256.New, []byte(secret))
_, _ = mac.Write([]byte(testBody))
return "sha256=" + hex.EncodeToString(mac.Sum(nil))
}
// headerWith builds a request header carrying one value.
func headerWith(name, value string) http.Header {
h := http.Header{}
if name != "" {
h.Set(name, value)
}
return h
}
// entrypoint builds an entrypoint with a signature configuration.
func entrypoint(
scheme database.SignatureScheme, secret string,
) *database.Entrypoint {
return &database.Entrypoint{
SignatureScheme: scheme,
SignatureSecret: secret,
}
}
// TestVerifyUnconfiguredAcceptsAnything pins the pass-through case:
// an entrypoint with no scheme is the entrypoint every deployment
// already has, and it must keep accepting requests that carry no
// signature at all.
func TestVerifyUnconfiguredAcceptsAnything(t *testing.T) {
t.Parallel()
ep := entrypoint(database.SignatureSchemeNone, "")
require.NoError(
t, signature.Verify(ep, http.Header{}, []byte(testBody)),
)
require.NoError(
t,
signature.Verify(
ep,
headerWith(signature.HeaderGitHub, "sha256=deadbeef"),
[]byte(testBody),
),
)
}
// githubCase is one inbound request against a GitHub-scheme
// entrypoint.
type githubCase struct {
name string
header string
value string
body string
want error
}
// githubCases enumerates the shapes a GitHub signature can arrive in.
func githubCases() []githubCase {
valid := githubSignature(testSharedKey)
return []githubCase{
{
name: "valid",
header: signature.HeaderGitHub,
value: valid,
body: testBody,
want: nil,
},
{
name: "absent header",
header: "",
body: testBody,
want: signature.ErrUnauthorized,
},
{
name: "wrong secret",
header: signature.HeaderGitHub,
value: githubSignature("not-the-shared-value"),
body: testBody,
want: signature.ErrUnauthorized,
},
{
// The digest is valid for a different body: the check
// has to be over the bytes actually received.
name: "body altered in flight",
header: signature.HeaderGitHub,
value: valid,
body: testBody + " ",
want: signature.ErrUnauthorized,
},
{
name: "missing algorithm prefix",
header: signature.HeaderGitHub,
value: valid[len("sha256="):],
body: testBody,
want: signature.ErrUnauthorized,
},
{
name: "not hex",
header: signature.HeaderGitHub,
value: "sha256=zzzz",
body: testBody,
want: signature.ErrUnauthorized,
},
{
name: "empty digest",
header: signature.HeaderGitHub,
value: "sha256=",
body: testBody,
want: signature.ErrUnauthorized,
},
{
// GitLab's header does not authenticate a GitHub
// entrypoint, even holding the right secret.
name: "wrong header for the scheme",
header: signature.HeaderGitLab,
value: testSharedKey,
body: testBody,
want: signature.ErrUnauthorized,
},
}
}
func TestVerifyGitHub(t *testing.T) {
t.Parallel()
for _, tc := range githubCases() {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
err := signature.Verify(
entrypoint(
database.SignatureSchemeGitHub, testSharedKey,
),
headerWith(tc.header, tc.value),
[]byte(tc.body),
)
if tc.want == nil {
require.NoError(t, err)
return
}
require.ErrorIs(t, err, tc.want)
})
}
}
func TestVerifyGitLab(t *testing.T) {
t.Parallel()
cases := []struct {
name string
header string
value string
want error
}{
{
name: "valid",
header: signature.HeaderGitLab,
value: testSharedKey,
want: nil,
},
{
name: "absent header",
header: "",
want: signature.ErrUnauthorized,
},
{
name: "wrong token",
header: signature.HeaderGitLab,
value: "not-the-shared-value",
want: signature.ErrUnauthorized,
},
{
name: "token prefix only",
header: signature.HeaderGitLab,
value: testSharedKey[:5],
want: signature.ErrUnauthorized,
},
{
name: "wrong header for the scheme",
header: signature.HeaderGitHub,
value: githubSignature(testSharedKey),
want: signature.ErrUnauthorized,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
err := signature.Verify(
entrypoint(
database.SignatureSchemeGitLab, testSharedKey,
),
headerWith(tc.header, tc.value),
[]byte(testBody),
)
if tc.want == nil {
require.NoError(t, err)
return
}
require.ErrorIs(t, err, tc.want)
})
}
}
// TestVerifyBrokenConfigurationFailsClosed covers the rows a caller
// must refuse rather than wave through. Each is a state an operator
// could only reach outside the UI, and each one would otherwise be
// indistinguishable from "verification is off".
func TestVerifyBrokenConfigurationFailsClosed(t *testing.T) {
t.Parallel()
cases := []struct {
name string
scheme database.SignatureScheme
secret string
}{
{
name: "unknown scheme",
scheme: database.SignatureScheme("stripe"),
secret: testSharedKey,
},
{
name: "scheme without secret",
scheme: database.SignatureSchemeGitHub,
secret: "",
},
{
name: "secret without scheme",
scheme: database.SignatureSchemeNone,
secret: testSharedKey,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
err := signature.Verify(
entrypoint(tc.scheme, tc.secret),
headerWith(
signature.HeaderGitHub,
githubSignature(testSharedKey),
),
[]byte(testBody),
)
require.ErrorIs(t, err, signature.ErrConfig)
assert.NotErrorIs(t, err, signature.ErrUnauthorized)
})
}
}
// TestErrorsCarryNoSecret proves the strings that reach the log hold
// no part of the shared secret or of what the client presented.
func TestErrorsCarryNoSecret(t *testing.T) {
t.Parallel()
const presented = "QQPRESENTEDTOKENQQ"
for _, scheme := range []database.SignatureScheme{
database.SignatureSchemeGitHub,
database.SignatureSchemeGitLab,
} {
for _, header := range []string{
signature.HeaderGitHub, signature.HeaderGitLab,
} {
err := signature.Verify(
entrypoint(scheme, testSharedKey),
headerWith(header, presented),
[]byte(testBody),
)
require.Error(t, err)
assert.NotContains(t, err.Error(), testSharedKey)
assert.NotContains(t, err.Error(), presented)
}
}
}
func TestSchemeMetadata(t *testing.T) {
t.Parallel()
assert.True(t, signature.Supported(database.SignatureSchemeNone))
assert.True(t, signature.Supported(database.SignatureSchemeGitHub))
assert.True(t, signature.Supported(database.SignatureSchemeGitLab))
assert.False(
t, signature.Supported(database.SignatureScheme("stripe")),
)
// The empty scheme describes no sender, so it has no info even
// though it is a storable value.
_, ok := signature.Info(database.SignatureSchemeNone)
assert.False(t, ok)
info, ok := signature.Info(database.SignatureSchemeGitHub)
require.True(t, ok)
assert.Equal(t, "GitHub", info.Label)
assert.Equal(t, signature.HeaderGitHub, info.Header)
info, ok = signature.Info(database.SignatureSchemeGitLab)
require.True(t, ok)
assert.Equal(t, "GitLab", info.Label)
assert.Equal(t, signature.HeaderGitLab, info.Header)
}

View File

@@ -48,7 +48,7 @@
<div class="divide-y divide-gray-100"> <div class="divide-y divide-gray-100">
{{range .Entrypoints}} {{range .Entrypoints}}
<div class="p-4"> <div class="p-4" x-data="{ showSecret: false }">
<div class="flex items-center justify-between mb-1"> <div class="flex items-center justify-between mb-1">
<span class="text-sm font-medium text-gray-900">{{if .Description}}{{.Description}}{{else}}Entrypoint{{end}}</span> <span class="text-sm font-medium text-gray-900">{{if .Description}}{{.Description}}{{else}}Entrypoint{{end}}</span>
<div class="flex items-center gap-2"> <div class="flex items-center gap-2">
@@ -75,6 +75,38 @@
script the URL above stays selectable. --> script the URL above stays selectable. -->
<button type="button" hidden data-copy-target="entrypoint-url-{{.ID}}" class="text-xs text-gray-500 hover:text-primary-600">Copy</button> <button type="button" hidden data-copy-target="entrypoint-url-{{.ID}}" class="text-xs text-gray-500 hover:text-primary-600">Copy</button>
</div> </div>
<div class="flex items-center gap-2 mt-2">
<span class="text-xs text-gray-500">
Signature: {{.SchemeLabel}}{{if .SchemeHeader}} ({{.SchemeHeader}}){{end}}
</span>
<button type="button" @click="showSecret = !showSecret" class="text-xs text-gray-500 hover:text-primary-600">
{{if .Configured}}Rotate{{else}}Configure{{end}}
</button>
</div>
<!-- The stored secret is never sent to the browser: the
form takes a new one every time, so setting and
rotating are the same submission. -->
<div x-show="showSecret" x-cloak class="mt-2">
<form method="POST" action="/source/{{$.Webhook.ID}}/entrypoints/{{.ID}}/secret" class="flex gap-2">
<input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
<select name="signature_scheme" class="input text-sm w-28">
<!-- Selection follows the stored scheme, not
whether the pair is complete: a row with a
scheme and no secret would otherwise mark
both this option and its own selected. -->
<option value="" {{if not .Scheme}}selected{{end}}>None</option>
{{$current := .Scheme}}
{{range $.SignatureSchemes}}
<option value="{{.Scheme}}" {{if eq .Scheme $current}}selected{{end}}>{{.Label}}</option>
{{end}}
</select>
<input type="password" name="secret" autocomplete="new-password" placeholder="Shared secret" class="input text-sm flex-1">
<button type="submit" class="btn-primary text-sm">Save</button>
</form>
<p class="text-xs text-gray-500 mt-1">
Enter the same secret you configured at the sender. Selecting None removes verification.
</p>
</div>
</div> </div>
{{else}} {{else}}
<div class="p-4 text-sm text-gray-500">No entrypoints configured.</div> <div class="p-4 text-sm text-gray-500">No entrypoints configured.</div>
@@ -110,6 +142,14 @@
<div x-show="targetType === 'http'"> <div x-show="targetType === 'http'">
<input type="url" name="url" placeholder="https://example.com/webhook" :disabled="targetType !== 'http'" class="input text-sm"> <input type="url" name="url" placeholder="https://example.com/webhook" :disabled="targetType !== 'http'" class="input text-sm">
</div> </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"> <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> <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"> <input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
@@ -138,6 +178,7 @@
{{else}} {{else}}
<span class="badge-error">Inactive</span> <span class="badge-error">Inactive</span>
{{end}} {{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"> <form method="POST" action="/source/{{$.Webhook.ID}}/targets/{{.ID}}/toggle" class="inline">
<input type="hidden" name="csrf_token" value="{{$.CSRFToken}}"> <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}}"> <button type="submit" class="text-xs text-gray-500 hover:text-primary-600" title="{{if .Active}}Deactivate{{else}}Activate{{end}}">

View File

@@ -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}}