5 Commits

Author SHA1 Message Date
a83e8fe654 Record the completed 1.0.0 milestone in TODO.md (#249)
All checks were successful
check / check (push) Successful in 7s
2026-08-20 11:16:51 +02:00
687405993e Harden operator-set target headers (closes #233) (#242)
All checks were successful
check / check (push) Successful in 2m58s
2026-08-20 10:54:43 +02:00
03cd1859d7 Add an egress CIDR allowlist to the SSRF guard (closes #204) (#217)
All checks were successful
check / check (push) Successful in 3m1s
2026-08-20 10:34:42 +02:00
f0512f1c3c Render delivery attempt detail in the event log (closes #202) (#219)
All checks were successful
check / check (push) Successful in 2m55s
delivery_results stored status_code, response_body, error, duration and
attempt_num, and no template rendered any of it, so a failure read as
"target: failed" and diagnosing it meant opening the per-webhook SQLite
file by hand.

An expanded delivery now lists its attempts with attempt number, status
code, duration, error and response body. The body is bounded in the query
rather than read whole and truncated in Go (#135), and a body the engine
itself cut is no longer presented as complete.

The response body and error are untrusted remote content, so target
credentials are removed before rendering. Two cases needed care: a secret
severed by the 4096-byte cut matches nothing as a whole string, and the
engine's io.LimitReader cuts at the same constant the renderer uses, so
the guard keys on the body reaching the cap rather than on the stored size
exceeding it. Empty secrets are filtered where the secret list is built,
because an empty string passed to strings.ReplaceAll inserts the marker at
every byte boundary.

loadTargetMap builds the redactor half unscoped, so a soft-deleted
target's historical deliveries still render redacted.

Also regenerates static/css/tailwind.css, which had drifted from the
templates: hover:text-red-700, text-red-500, underline and w-28 were in
use but absent from the served stylesheet (#236).
2026-08-20 08:36:25 +02:00
3b0ed826bc Add per-delivery replay to the event log (closes #203) (#240)
All checks were successful
check / check (push) Successful in 3m21s
There was no redelivery path anywhere: once a delivery exhausted
max_retries it was failed permanently, even though the event body is
durably stored. Storing an event and being unable to re-send it defeats
the reason it is stored, and the ordinary case is a destination that was
down longer than the backoff ladder.

Adds POST /source/{sourceID}/deliveries/{deliveryID}/replay, inside the
authenticated group so it inherits MaxBodySize, CSRF, NoCache and
RequireAuth. Replay creates a NEW pending delivery against the target's
CURRENT config and hands it to the engine through the same notifier the
receiver uses, so it runs the normal path with the retry ladder, the
SSRF-guarded transport and the circuit breaker. The original delivery's
rows are never touched, and the stored event body is re-sent, never the
recorded response.

Replay is refused, with a distinct message, for a non-terminal delivery, a
deleted target, a deactivated target, and when an earlier replay of the
same event and target is still in flight. Bounded by a per-client rate
limit and by that in-flight check.

The new delivery row is written with Omit(clause.Associations) and with
neither Event nor Target populated, so it cannot upsert a targets row into
the per-webhook event database (#206).

Counted by webhooker_delivery_replays_total on the existing target_type
label. A replay also moves the ordinary attempt, outcome and duration
series, because it is a real delivery.
2026-08-20 08:11:35 +02:00
33 changed files with 3294 additions and 213 deletions

192
README.md
View File

@@ -114,6 +114,116 @@ TTY detection, and security headers are always applied.
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` | | `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` | | `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
| `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted (unset: all clients behind a proxy share one rate-limit bucket; a correct login password is never throttled either way) | `""` (none) | | `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted (unset: all clients behind a proxy share one rate-limit bucket; a correct login password is never throttled either way) | `""` (none) |
| `ALLOWED_EGRESS_CIDRS` | CIDRs that delivery targets may reach despite the SSRF blocklist. Read [Allowing egress to your own network](#allowing-egress-to-your-own-network) before setting it | `""` (none) |
#### Allowing egress to your own network
By default every delivery target must resolve to a public address. The
private and reserved ranges — RFC 1918, loopback, CGNAT, link-local and
the rest — are refused, which stops a target from being used to make
webhooker probe the network it sits in.
That default is also inconvenient for the thing webhooker is mostly
for: taking a public webhook and forwarding it to something on your own
network. A container on the same Docker network, a box on `10.x`, a
service on `127.0.0.1` — all refused, until you name them.
`ALLOWED_EGRESS_CIDRS` is a comma-separated list of CIDR blocks (a bare
address such as `10.0.0.7` is accepted and treated as a single host),
for example `10.0.0.0/8, 172.17.0.0/16`. Addresses inside those blocks
become valid delivery destinations. Everything outside them keeps the
default answer, so this only ever adds destinations — it never removes
any, and it cannot narrow what was already reachable.
**The risk, plainly.** Each block you list is a network that anyone who
can create a delivery target can now make this process issue requests
into, and read the response body back out of via the delivery log. That
is server-side request forgery, deliberately enabled and scoped by you.
A webhooker admin account is therefore as trusted as the narrowest
thing on those networks: an unauthenticated admin panel, a database
listening without a password, or an internal API that trusts its
network position is reachable through it. List the smallest blocks that
cover the destinations you actually deliver to — prefer
`10.1.2.3/32` over `10.0.0.0/8` — and never list a block wider than the
network you are willing to expose.
Listing `0.0.0.0/0` or `::/0` opens **every** other private and
reserved range at once — loopback, RFC 1918, CGNAT, ULA, the lot. It is
a functional off switch for everything except the addresses listed as
unconditionally blocked below, and it makes any delivery target a probe
into your entire network and this host's own loopback services. Do not
list it.
Two things this setting cannot do:
- **It cannot turn the guard off.** There is no boolean, and no value
that disables SSRF protection wholesale. The guard is always on and
the list is always an allowlist; an empty list (the default) means
every private and reserved range stays refused. Note that
`0.0.0.0/0` gets you most of the way there anyway, per above.
- **It cannot open link-local, or a cloud metadata endpoint that
discloses credentials or user data.** An address is on the list below
when both of these hold: the provider fixes it, so it cannot collide
with anything you run; and reaching it hands out credentials, user
data or bootstrap material. Those stay blocked no matter what you
list, including when you list them outright or list a supernet such
as `0.0.0.0/0`, `::/0`, `fd00::/8` or `100.64.0.0/10`. Treat this as
best effort rather than a guarantee — it is a hand-maintained list
and the caveat below the table applies:
| Blocked unconditionally | What it is |
| ----------------------- | ---------- |
| `169.254.0.0/16` | IPv4 link-local, carrying `169.254.169.254` (AWS, Azure, DigitalOcean, Hetzner, OpenStack and others — not Alibaba, which uses `100.100.100.200` below) |
| `fe80::/10` | IPv6 link-local |
| `fd00:ec2::254/128` | AWS IPv6 IMDS |
| `fd00:ec2::23/128` | AWS EKS Pod Identity Agent |
| `fd20:ce::254/128` | GCP metadata for IPv6-only instances |
| `fd00:c1::a9fe:a9fe/128` | Oracle OCI IMDS over IPv6 |
| `fd00:42::42/128` | Scaleway metadata over IPv6 |
| `fd00:a9fe:a9fe::1/128` | Linode/Akamai metadata over IPv6 |
| `100.100.100.200/32` | Alibaba Cloud metadata, inside CGNAT |
| `192.0.0.192/32` | Oracle Cloud Classic metadata |
| `::a9fe:a9fe/128` | `169.254.169.254` as an IPv4-compatible IPv6 address |
| `64:ff9b::a9fe:a9fe/128` | `169.254.169.254` behind the NAT64 well-known prefix |
The IPv4-mapped form `::ffff:169.254.169.254` is covered by the
`169.254.0.0/16` entry. Reaching any of these is credential or
user-data theft rather than delivery to an internal service. Every
entry outside the two link-local blocks is a single address, so
blocking it costs you nothing else on the network around it.
The six ULA entries, all inside `fd00::/8`, are why this matters in
practice: `fd00::/8` is an ordinary block to allowlist for your own
IPv6 network, and without those host routes that one line would hand
out cloud credentials on five providers at once. There is only one
`/8` involved — `fd20:ce::254` masks into `fd00::/8` as well — and
the six endpoints are five providers because AWS appears twice, IMDS
and EKS Pod Identity. Several of them are described as "link-local" —
or even "localhost" — in their own vendor's documentation, but they
are ULAs and `fe80::/10` does not cover them.
Every entry above is reserved space. All but the last two are already
refused with no allowlist set, and listing them here is only what
stops an allowlist from reopening them; the last two are the alternate
encodings, which the default blocklist does not match. A publicly
routable metadata address is not listed here, because nothing on this
list can be reopened and blocking one that way would leave you no
escape hatch at all.
This list is not exhaustive of every cloud's metadata address — if
yours is not here, do not allowlist the block that contains it.
The list is applied at one place in the code, which both target
creation and delivery consult, so a URL that the target form accepts is
one that delivery will actually attempt — the two cannot disagree.
Delivery re-resolves and re-checks the destination at dial time, so a
hostname that resolves to an allowed address during validation and a
different one later (DNS rebinding) is still refused unless the new
address is also allowed.
A set but unparseable value aborts startup. When the list is non-empty
webhooker logs it at startup, blocks and all, so the hole is visible in
the log of any deployment that has one.
#### Metrics credentials #### Metrics credentials
@@ -272,8 +382,9 @@ additionally be a number in the range 165535,
`RECEIVER_RATE_LIMIT` must be at least 1, `RECEIVER_RATE_LIMIT` must be at least 1,
`RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker `RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker
period, so `0s` or a negative value would crash the reaper after period, so `0s` or a negative value would crash the reaper after
startup), and every entry in `TRUSTED_PROXIES` must be a CIDR block or startup), and every entry in `TRUSTED_PROXIES` and
a bare IP address. `SESSION_IDLE_TIMEOUT` is the exception: a `ALLOWED_EGRESS_CIDRS` must be a CIDR block or a bare IP address.
`SESSION_IDLE_TIMEOUT` is the exception: a
non-positive value there means idle expiry is disabled, not invalid. non-positive value there means idle expiry is disabled, not invalid.
Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
@@ -1049,6 +1160,53 @@ events should be forwarded.
The `config` field stores type-specific configuration as JSON (e.g., The `config` field stores type-specific configuration as JSON (e.g.,
destination URL, custom headers, timeout settings). destination URL, custom headers, timeout settings).
**`http` target configuration:**
| Key | Type | Description |
| --------- | ------------- | ----------- |
| `url` | string | Destination the event is POSTed to |
| `headers` | object | Extra request headers, applied last so they win over the event's own forwarded headers |
| `timeout` | integer (sec) | Per-target request timeout; unset (or 0) uses the shared 30-second client timeout |
`timeout` is capped at **300 seconds**, and the form rejects anything
above it rather than substituting the cap. A delivery attempt holds one
of the bounded pool's workers for its whole duration, so an unbounded
timeout would let a single unresponsive destination stall the queue.
`headers` rejects the names the delivery path or `net/http` writes
regardless of what is configured: `Host`, `Content-Length`,
`Transfer-Encoding`, `Connection`, `Trailer` and `User-Agent`. These are
refused at the form rather than accepted and ignored, because a stored
header that provably never reaches the wire tells the operator their
configuration took effect when it did not. `Content-Type` is _not_
reserved: a configured one deliberately overrides the event's.
**Redirects.** A redirect from an `http` target's destination is
followed, up to ten hops, and the delivery's recorded status and body
come from the final hop. One rule governs every header the delivery
carries for someone else — the configured `headers` and the inbound
event headers forwarded from the sender alike: **a hop that leaves the
origin the target names carries none of them.** Leaving the origin
means a different host, a different port, or a step down from `https`
to `http`. Both classes routinely carry a secret — a configured
`X-Api-Key` or `PRIVATE-TOKEN`, an inbound `X-Hub-Signature` — and an
open redirect at the destination would otherwise hand it to a host the
operator never chose. `net/http` already does this for `Authorization`
and `Cookie`. The delivery path's own headers (`Content-Type`,
`User-Agent`) are not origin-scoped and always travel, so a body
preserved across a `307` is still typed. A `301`, `302` or `303` is a
different matter, and this is `net/http`'s behaviour rather than
webhooker's: the POST becomes a GET and the event body and its
`Content-Type` are dropped, so the destination the chain ends at
receives no event at all — and the delivery is still recorded
`Delivered` on that hop's `2xx`. Redirects within the target's own
origin keep everything, so a destination that redirects its own paths
is unaffected; the drop is per hop rather than permanent, so a chain
that returns to the configured origin carries the headers again,
exactly as `net/http` treats `Authorization`. Each hop is dialled
through the same SSRF guard as the first, so a redirect aimed at a
private or reserved address is refused at connect time.
#### APIKey #### APIKey
A programmatic access credential for API authentication. A programmatic access credential for API authentication.
@@ -1108,6 +1266,23 @@ DeliveryResults.
succeeded). succeeded).
- **`failed`** — All retry attempts exhausted without success. - **`failed`** — All retry attempts exhausted without success.
**Replay.** A `delivered` or `failed` delivery is finished as far as
the engine is concerned, but the event is still stored, so the event
log offers a per-delivery **Replay** action for it. Replay creates a
NEW `pending` delivery for the same event and target and hands it to
the engine on the ordinary path — same retries, same SSRF guard, same
circuit breaker as a first attempt. It never touches the delivery it
repeats: that row's status, timestamps and recorded attempts stand as
the record of what happened.
What is re-sent is the stored event body, against the target's
configuration **as it stands now** — the point of a replay is to
deliver where the destination has since been fixed. A target that has
been deleted or deactivated therefore refuses the replay with a
message on the event log rather than delivering from stale
configuration, and a replay is refused while an earlier one for the
same event and target is still pending or retrying.
#### DeliveryResult #### DeliveryResult
The result of a single delivery attempt. Every attempt (including The result of a single delivery attempt. Every attempt (including
@@ -1439,6 +1614,7 @@ arriving and being stored, they are just not getting anywhere.
| `webhooker_deliveries_succeeded_total` | counter | Deliveries that reached `delivered` | | `webhooker_deliveries_succeeded_total` | counter | Deliveries that reached `delivered` |
| `webhooker_deliveries_failed_total` | counter | Deliveries that failed terminally and will not be retried | | `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_retries_total` | counter | Deliveries put back into `retrying` |
| `webhooker_delivery_replays_total` | counter | Deliveries an operator replayed from the event log. A replay runs the ordinary engine path, so it also moves the attempt, outcome and duration series; this is the only one that separates it from ordinary traffic |
| `webhooker_delivery_duration_seconds` | histogram | Wall time of a single dispatched delivery attempt, the same duration the attempt's `DeliveryResult` records | | `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_pending` | gauge | Deliveries currently in `pending` |
| `webhooker_deliveries_retrying` | gauge | Deliveries currently in `retrying` | | `webhooker_deliveries_retrying` | gauge | Deliveries currently in `retrying` |
@@ -2036,6 +2212,7 @@ abuse limit later; they are tracked as future work.
| `POST` | `/source/{id}/edit` | Edit webhook submission | | `POST` | `/source/{id}/edit` | Edit webhook submission |
| `POST` | `/source/{id}/delete` | Delete webhook | | `POST` | `/source/{id}/delete` | Delete webhook |
| `GET` | `/source/{id}/logs` | Webhook event logs | | `GET` | `/source/{id}/logs` | Webhook event logs |
| `POST` | `/source/{id}/deliveries/{deliveryID}/replay` | Replay a finished delivery: creates a new delivery for the same event against the target's current configuration (30 per minute per bucket, then `429`) |
| `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 |
@@ -2125,6 +2302,7 @@ webhooker/
│ ├── 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
│ │ ├── delivery_replay.go # Per-delivery replay: new delivery, current target config
│ │ ├── entrypoint_view.go # Masked entrypoint view for templates │ │ ├── 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
@@ -2312,7 +2490,15 @@ check, see [The login endpoint](#the-login-endpoint).
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
(custom HTTP transport with SSRF-safe dialer that validates resolved (custom HTTP transport with SSRF-safe dialer that validates resolved
IPs before connecting, preventing DNS rebinding attacks) IPs before connecting, preventing DNS rebinding attacks). Both paths
route through a single decision function, so they cannot disagree
about a destination. An operator can permit specific blocks with
[`ALLOWED_EGRESS_CIDRS`](#allowing-egress-to-your-own-network); the
guard cannot be switched off, and link-local plus a
[pinned set](#allowing-egress-to-your-own-network) of known cloud
metadata endpoints — several of which are ULAs outside link-local —
stay blocked whatever is listed, though listing `0.0.0.0/0` or
`::/0` does open every other private range
- **Login limiting is inverted, deliberately.** The login `POST` has - **Login limiting is inverted, deliberately.** The login `POST` has
no pre-emptive rate limiter in front of it. Credentials are no pre-emptive rate limiter in front of it. Credentials are
verified first and only a _failed_ attempt spends budget, so a verified first and only a _failed_ attempt spends budget, so a

113
TODO.md
View File

@@ -18,68 +18,52 @@ Issue branches do NOT touch this file — the manager maintains it on
# Status # Status
pre-1.0. No git tags exist. `main` (4f5ecb1) is a working webhook proxy 1.0.0 is complete: 55 closed, 0 open. `next` (6874059) is 62 commits
with auth, CSRF/SSRF protections, login rate limiting, Slack target, ahead of `main` and a strict fast-forward. No git tags exist yet.
event retention (#63), the database archiving target (#43), the admin
password change flow (#65), policy compliance (#6), pinned lint tooling
(#55), and fail-loud configuration parsing (#80).
`next` is green — verified both by CI and by cache-defeated container The bar was not "the milestone is empty" but "sneak can deploy this and
runs (`docker build --no-cache-filter=lint --no-cache-filter=builder`) — use it in low-volume production". Every gap the deployability audit
but the **1.0.0 milestone is no longer complete**. It was reopened on named against that bar is now closed:
2026-08-20 by a code-level deployability audit that ran the service end
to end (verdict:
https://git.eeqj.de/sneak/webhooker/issues/33#issuecomment-66686).
The bar for 1.0 is not "the milestone is empty" but "sneak can deploy - `DATA_DIR` locking, so two instances cannot both deliver
this and use it in low-volume production". The audit found the gap (https://git.eeqj.de/sneak/webhooker/issues/201)
between those two: two instances on one `DATA_DIR` both deliver - shutdown on listener failure, rather than a live non-serving process
(reproduced), a failed listen leaves a live non-serving process that (https://git.eeqj.de/sneak/webhooker/issues/200)
restart policies never fire on, there is no inbound authentication of - inbound signature verification
any kind, delivery failures render as a bare word with no status code or (https://git.eeqj.de/sneak/webhooker/issues/67)
error, a terminally failed delivery can never be replayed, the SSRF - per-attempt delivery detail in the event log
blocklist has no escape hatch so the proxy cannot forward to your own (https://git.eeqj.de/sneak/webhooker/issues/202)
network at all, and target credentials leak into the per-webhook event - replay of a terminally failed delivery
databases. (https://git.eeqj.de/sneak/webhooker/issues/203)
- `ALLOWED_EGRESS_CIDRS`, an allowlist escape hatch for the SSRF guard
(https://git.eeqj.de/sneak/webhooker/issues/204)
- the three credential exposures
(https://git.eeqj.de/sneak/webhooker/issues/205,
https://git.eeqj.de/sneak/webhooker/issues/206,
https://git.eeqj.de/sneak/webhooker/issues/207)
One caveat on reading a green check, narrower than it used to be. A One caveat on reading a green check: a docs-only commit deliberately
docs-only commit deliberately replays from the layer cache (#119), so a replays from the layer cache
green status on such a commit evidences a replay rather than an executed (https://git.eeqj.de/sneak/webhooker/issues/119), so a green status on
run; a code commit invalidates the `COPY` layer and genuinely executes. such a commit evidences a replay rather than an executed run. A code
Superseded runs are no longer the hazard they were: before #152 they commit invalidates the `COPY` layer and genuinely executes.
were recorded as `skipped` and rolled up green, and before #119 a warm
layer cache let the gate report success without executing anything,
replaying the previous build's console log so the lie looked like a real
run. Both are fixed. Note: `TODO.md` was deliberately
deleted from this repo in f9a9569 (2026-03-01, #6); its content was
folded into the README TODO section, which this draft reconstructs as
of 2026-07-06.
# Next Step # Next Step
Clear the reopened 1.0.0 milestone. The milestone PR Merge the milestone PR (https://git.eeqj.de/sneak/webhooker/pulls/111)
(https://git.eeqj.de/sneak/webhooker/pulls/111) is held: it carries a and tag `v1.0.0`. It is `merge-ready` and assigned to sneak; nothing
`WIP: ` prefix, no labels and is assigned to `clawbot`, and it stays else gates it.
that way until the milestone is empty. Correctness first — the
duplicate-delivery lock and the listen-failure shutdown — then the
operability gaps that make the service usable in production, then the
three credential exposures.
Three items belong to the owner, none of them blocking. #150 was decided Post-1.0 follow-ups are open, none blocking the tag:
by the manager rather than left to stall the queue and is flagged on the https://git.eeqj.de/sneak/webhooker/issues/245,
issue for reversal if that call was wrong. #112 (whether `Completed https://git.eeqj.de/sneak/webhooker/issues/246,
Steps` should exist at all, given it once conflicted on every unit) is https://git.eeqj.de/sneak/webhooker/issues/247 and
unanswered; the provisional ruling in force is that issue branches do https://git.eeqj.de/sneak/webhooker/issues/248. Also still open and
not touch this file. #198 records that `make test` is past the org 20s unmilestoned: https://git.eeqj.de/sneak/webhooker/issues/193 (a design
target — 46s of test execution inside a 62.8s CI layer — and turns on question, not a defect), https://git.eeqj.de/sneak/webhooker/issues/198
which quantity the 60s hard cap governs; it is scoped as the improvement (`make test` is past the org 20s target) and
bug the 20-60s band requires, and should be milestoned instead if the https://git.eeqj.de/sneak/webhooker/issues/212 (encrypting target config
cap is read as covering the whole invocation. at rest).
After the tag, the largest open cluster is the unmilestoned follow-up
backlog these units generated: #183, #184, #185, #190, #191, #193, #198,
#211 and #212 (encrypting target config at rest, split out of the
credential-leak fix because it needs a key-rotation and re-wrap story).
# Completed Steps # Completed Steps
@@ -308,14 +292,16 @@ credential-leak fix because it needs a key-rotation and re-wrap story).
# Future Steps # Future Steps
- Manual event redelivery from the web UI — the "Replay" capability the - Delivery status and retry management UI. Replay of a terminally
README describes as planned. No redelivery code exists anywhere in the failed delivery and per-attempt detail already landed
tree; events are stored in full, which is all it would be built on (https://git.eeqj.de/sneak/webhooker/issues/203,
- Delivery status and retry management UI https://git.eeqj.de/sneak/webhooker/issues/202)
- Per-webhook rate limiting in the receiver handler (per-webhook config - Per-webhook rate limiting in the receiver handler (per-webhook config
plus handler enforcement; global limits must not apply to receiver plus handler enforcement; global limits must not apply to receiver
endpoints) endpoints)
- Webhook signature verification for GitHub and Stripe HMAC formats - Stripe HMAC signature verification. The GitHub and GitLab schemes
landed with inbound verification
(https://git.eeqj.de/sneak/webhooker/issues/67)
- API key authentication for programmatic access (APIKey model exists; - API key authentication for programmatic access (APIKey model exists;
Bearer token middleware does not) Bearer token middleware does not)
- REST API v1 - REST API v1
@@ -325,9 +311,10 @@ credential-leak fix because it needs a key-rotation and re-wrap story).
- OpenAPI specification - OpenAPI specification
- Analytics dashboard: success rates, response times, volume - Analytics dashboard: success rates, response times, volume
- A remember-me option at login - A remember-me option at login
- Password reset flow for a forgotten password. The authenticated - Password reset flow for a forgotten password over the web. The
password *change* flow already landed on `main` (#65); reset does not authenticated password *change* flow already landed, and a lost
exist password is recoverable from the console with `webhooker resetpw`
(https://git.eeqj.de/sneak/webhooker/issues/208)
- Later, nice to have - Later, nice to have
- email delivery target type - email delivery target type
- SNS and S3 delivery targets - SNS and S3 delivery targets

View File

@@ -157,6 +157,10 @@ func newApp() *fx.App {
session.New, session.New,
handlers.New, handlers.New,
middleware.New, middleware.New,
// The one SSRF guard both target-creation validation
// and the delivery dialer consult, so they cannot
// disagree about a destination.
delivery.NewGuard,
delivery.New, delivery.New,
delivery.NewArchiveSweeper, delivery.NewArchiveSweeper,
// Wire *delivery.Engine as delivery.Notifier so the // Wire *delivery.Engine as delivery.Notifier so the

View File

@@ -128,6 +128,22 @@ type Config struct {
// clients. // clients.
TrustedProxies []netip.Prefix TrustedProxies []netip.Prefix
// AllowedEgressCIDRs is the set of networks a delivery target
// may reach even though the SSRF guard's default blocklist
// covers them. It is empty unless ALLOWED_EGRESS_CIDRS is set,
// and empty means every private/reserved range stays refused.
//
// This only ever adds destinations to what the guard would
// otherwise refuse. The guard itself is always on: there is no
// setting that disables SSRF protection, and delivery's
// alwaysBlockedNetworks stays blocked no matter what is listed
// here. That set is link-local plus the cloud metadata
// endpoints outside it that disclose credentials or user data
// at a provider-fixed address; it is not exhaustive of every
// cloud's metadata address. See alwaysBlockedNetworks for the
// authoritative list and the criterion it is built from.
AllowedEgressCIDRs []netip.Prefix
params *ConfigParams params *ConfigParams
log *slog.Logger log *slog.Logger
} }
@@ -472,6 +488,11 @@ func loadFromEnv() (*Config, error) {
return nil, err return nil, err
} }
allowedEgressCIDRs, err := envPrefixList("ALLOWED_EGRESS_CIDRS")
if err != nil {
return nil, err
}
metricsUsername, metricsPassword, err := resolveMetricsAuth() metricsUsername, metricsPassword, err := resolveMetricsAuth()
if err != nil { if err != nil {
return nil, err return nil, err
@@ -490,9 +511,49 @@ func loadFromEnv() (*Config, error) {
SessionIdleTimeout: sessionIdleTimeout, SessionIdleTimeout: sessionIdleTimeout,
ReceiverRateLimit: receiverRateLimit, ReceiverRateLimit: receiverRateLimit,
TrustedProxies: trustedProxies, TrustedProxies: trustedProxies,
AllowedEgressCIDRs: allowedEgressCIDRs,
}, nil }, nil
} }
// PrefixStrings renders a prefix list as its CIDR strings, for
// logging a list an operator has to be able to read back.
func PrefixStrings(prefixes []netip.Prefix) []string {
out := make([]string, 0, len(prefixes))
for _, prefix := range prefixes {
out = append(out, prefix.String())
}
return out
}
// warnEgressAllowlist logs the effective ALLOWED_EGRESS_CIDRS
// whenever it is non-empty.
//
// It prints the blocks themselves rather than a count, because
// this is the one setting that lets a delivery target reach the
// host's own network: an operator reading the startup log has to
// be able to see exactly which hole is open. Silence means the
// list is empty and the SSRF guard is refusing every
// private/reserved range, which is the default.
func (c *Config) warnEgressAllowlist(log *slog.Logger) {
if len(c.AllowedEgressCIDRs) == 0 {
return
}
log.Warn(
"ALLOWED_EGRESS_CIDRS lets delivery targets reach these "+
"otherwise-blocked private/reserved networks. Anyone "+
"who can create a delivery target can now make this "+
"process issue requests into them, and read back the "+
"response. Link-local and the known cloud instance "+
"metadata endpoints outside it stay blocked "+
"regardless of what is listed here.",
"allowedEgressCIDRs",
strings.Join(PrefixStrings(c.AllowedEgressCIDRs), ","),
)
}
// warnSharedRateLimitBucket logs a startup warning whenever // warnSharedRateLimitBucket logs a startup warning whenever
// TRUSTED_PROXIES is empty, in any environment. // TRUSTED_PROXIES is empty, in any environment.
// //
@@ -574,11 +635,13 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
"sessionIdleTimeout", s.SessionIdleTimeout.String(), "sessionIdleTimeout", s.SessionIdleTimeout.String(),
"receiverRateLimit", s.ReceiverRateLimit, "receiverRateLimit", s.ReceiverRateLimit,
"trustedProxies", len(s.TrustedProxies), "trustedProxies", len(s.TrustedProxies),
"allowedEgressCIDRs", len(s.AllowedEgressCIDRs),
"hasSentryDSN", s.SentryDSN != "", "hasSentryDSN", s.SentryDSN != "",
"hasMetricsAuth", s.MetricsAuthEnabled(), "hasMetricsAuth", s.MetricsAuthEnabled(),
) )
s.warnSharedRateLimitBucket(log) s.warnSharedRateLimitBucket(log)
s.warnEgressAllowlist(log)
return s, nil return s, nil
} }

View File

@@ -663,6 +663,187 @@ func testTrustedProxiesSuccess(
assert.Equal(t, expected, got) assert.Equal(t, expected, got)
} }
// TestAllowedEgressCIDRs covers ALLOWED_EGRESS_CIDRS, the escape
// hatch that lets a self-hosted deployment forward to its own
// network. Unset it must stay empty, so the SSRF guard keeps
// refusing every private/reserved range; a set-but-unparseable
// value must abort startup naming the variable rather than
// silently running with a list the operator did not write.
func TestAllowedEgressCIDRs(t *testing.T) {
tests := []struct {
name string
set bool
value string
expected []string
expectError bool
}{
{
name: caseUnsetUsesDefault,
set: false,
expected: []string{},
},
{
name: "empty value yields empty list",
set: true,
value: "",
expected: []string{},
},
{
name: caseValidValueParsed,
set: true,
value: cidrPrivateV4,
expected: []string{cidrPrivateV4},
},
{
name: "multiple blocks with whitespace",
set: true,
value: " 10.0.0.0/8 , 127.0.0.0/8 ",
expected: []string{cidrPrivateV4, "127.0.0.0/8"},
},
{
name: "bare address becomes a single host",
set: true,
value: "172.17.0.5",
expected: []string{"172.17.0.5/32"},
},
{
name: caseUnparseableFails,
set: true,
value: cidrPrivateV4 + ",not-an-address",
expectError: true,
},
{
name: "out-of-range prefix length fails startup",
set: true,
value: "10.0.0.0/33",
expectError: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
if tt.set {
t.Setenv("ALLOWED_EGRESS_CIDRS", tt.value)
} else {
require.NoError(
t, os.Unsetenv("ALLOWED_EGRESS_CIDRS"),
)
}
if tt.expectError {
expectStartupErrorFor(
t, "ALLOWED_EGRESS_CIDRS", config.ErrInvalidCIDR,
)
} else {
testAllowedEgressCIDRsSuccess(t, tt.expected)
}
})
}
}
func testAllowedEgressCIDRsSuccess(
t *testing.T,
expected []string,
) {
t.Helper()
var cfg *config.Config
app := fxtest.New(
t,
fx.Provide(
globals.New,
logger.New,
config.New,
),
fx.Populate(&cfg),
)
require.NoError(t, app.Err())
app.RequireStart()
defer app.RequireStop()
assert.Equal(
t, expected, config.PrefixStrings(cfg.AllowedEgressCIDRs),
)
}
// TestEgressAllowlistWarning covers the startup log that shows an
// operator the hole ALLOWED_EGRESS_CIDRS opened. It must stay
// silent on the default (empty) list and, when set, print the
// blocks themselves rather than a count.
func TestEgressAllowlistWarning(t *testing.T) {
tests := []struct {
name string
allowed string
expectWarning bool
}{
{
name: "empty allowlist is quiet",
expectWarning: false,
},
{
name: "non-empty allowlist warns",
allowed: "10.0.0.0/8,127.0.0.0/8",
expectWarning: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
t.Setenv("WEBHOOKER_ENVIRONMENT", config.EnvironmentDev)
if tt.allowed == "" {
require.NoError(
t, os.Unsetenv("ALLOWED_EGRESS_CIDRS"),
)
} else {
t.Setenv("ALLOWED_EGRESS_CIDRS", tt.allowed)
}
var buf bytes.Buffer
log := slog.New(slog.NewJSONHandler(
&buf, &slog.HandlerOptions{
Level: slog.LevelDebug,
},
))
require.NoError(
t, config.WarnEgressAllowlistForTest(log),
)
if !tt.expectWarning {
assert.Empty(t, buf.String())
return
}
logged := buf.String()
assert.Contains(t, logged, `"level":"WARN"`)
assert.Contains(t, logged, "ALLOWED_EGRESS_CIDRS")
// The blocks themselves, not a count: the operator has
// to be able to read back which networks are open.
assert.Contains(t, logged, "10.0.0.0/8")
assert.Contains(t, logged, "127.0.0.0/8")
// What stays shut. Asserted on the clause naming the
// wider set rather than on "Link-local" alone, so the
// string cannot narrow back to link-local only while
// the always-blocked set covers ULA, CGNAT and two
// public metadata addresses as well.
assert.Contains(t, logged, "metadata endpoints outside it")
})
}
}
// TestSharedRateLimitBucketWarning covers the startup warning that // TestSharedRateLimitBucketWarning covers the startup warning that
// tells an operator a deployment behind a reverse proxy shares one // tells an operator a deployment behind a reverse proxy shares one
// rate-limit bucket between every client, which turns the receiver // rate-limit bucket between every client, which turns the receiver

View File

@@ -21,6 +21,21 @@ func WarnSharedRateLimitBucketForTest(log *slog.Logger) error {
return nil return nil
} }
// WarnEgressAllowlistForTest loads a Config from the current
// environment and emits its egress-allowlist startup warning to
// log, so a test can assert both that the warning fires only when
// the list is non-empty and that it names the blocks it opened.
func WarnEgressAllowlistForTest(log *slog.Logger) error {
c, err := loadFromEnv()
if err != nil {
return err
}
c.warnEgressAllowlist(log)
return nil
}
// EnvBoolForTest exposes envBool. // EnvBoolForTest exposes envBool.
func EnvBoolForTest(key string, defaultValue bool) (bool, error) { func EnvBoolForTest(key string, defaultValue bool) (bool, error) {
return envBool(key, defaultValue) return envBool(key, defaultValue)

View File

@@ -11,6 +11,23 @@ const (
DeliveryStatusRetrying DeliveryStatus = "retrying" DeliveryStatusRetrying DeliveryStatus = "retrying"
) )
// Terminal reports whether a delivery in this status has finished, so
// the delivery engine will make no further attempt of its own.
//
// It is what decides which deliveries the event log offers to replay:
// a pending or retrying delivery is still the engine's, and replaying
// one would race it.
func (s DeliveryStatus) Terminal() bool {
switch s {
case DeliveryStatusDelivered, DeliveryStatusFailed:
return true
case DeliveryStatusPending, DeliveryStatusRetrying:
return false
default:
return false
}
}
// Delivery represents a delivery attempt for an event to a target // Delivery represents a delivery attempt for an event to a target
type Delivery struct { type Delivery struct {
BaseModel BaseModel

View File

@@ -18,26 +18,27 @@ func newSSRFTestEngine() *delivery.Engine {
log := slog.New(slog.DiscardHandler) log := slog.New(slog.DiscardHandler)
client := &http.Client{ client := &http.Client{
Timeout: 30 * time.Second, Timeout: 30 * time.Second,
Transport: delivery.NewSSRFSafeTransport(), Transport: delivery.NewTestGuard().
NewSSRFSafeTransport(),
} }
return delivery.NewTestEngine(log, client, 1) return delivery.NewTestEngine(log, client, 1)
} }
// TestClientForConfig_TimeoutKeepsSSRFGuard asserts that a // TestClientForRequest_TimeoutKeepsSSRFGuard asserts that a
// client returned by clientForConfig for a config with a // client returned by clientForRequest for a config with a
// per-target timeout still refuses connections to // per-target timeout still refuses connections to
// private/reserved addresses (the timeout must not drop the // private/reserved addresses (the timeout must not drop the
// SSRF-safe transport). // SSRF-safe transport).
func TestClientForConfig_TimeoutKeepsSSRFGuard(t *testing.T) { func TestClientForRequest_TimeoutKeepsSSRFGuard(t *testing.T) {
t.Parallel() t.Parallel()
engine := newSSRFTestEngine() engine := newSSRFTestEngine()
blocked := []string{ blocked := []string{
"http://127.0.0.1/hook", loopbackHookURL,
"http://169.254.169.254/latest/meta-data/", metadataURL,
"http://[fe80::1]/hook", "http://[fe80::1]/hook",
} }
@@ -50,7 +51,7 @@ func TestClientForConfig_TimeoutKeepsSSRFGuard(t *testing.T) {
Timeout: 5, Timeout: 5,
} }
client := engine.ExportClientForConfig(cfg) client := engine.ExportClientForRequest(cfg, nil)
require.NotSame(t, engine.ExportClient(), client, require.NotSame(t, engine.ExportClient(), client,
"a per-target timeout must yield a "+ "a per-target timeout must yield a "+
@@ -91,10 +92,11 @@ func TestClientForConfig_TimeoutKeepsSSRFGuard(t *testing.T) {
} }
} }
// TestClientForConfig_NoTimeoutUnchanged asserts that with // TestClientForRequest_NoTimeoutUnchanged asserts that a
// no per-target timeout the shared SSRF-safe client is // request with neither a per-target timeout nor an origin-scoped
// returned unchanged. // header gets the shared SSRF-safe client unchanged: there is then
func TestClientForConfig_NoTimeoutUnchanged(t *testing.T) { // nothing for a redirect policy to strip.
func TestClientForRequest_NoTimeoutUnchanged(t *testing.T) {
t.Parallel() t.Parallel()
engine := newSSRFTestEngine() engine := newSSRFTestEngine()
@@ -103,10 +105,46 @@ func TestClientForConfig_NoTimeoutUnchanged(t *testing.T) {
URL: "https://example.com/hook", URL: "https://example.com/hook",
} }
client := engine.ExportClientForConfig(cfg) client := engine.ExportClientForRequest(cfg, nil)
assert.Same(t, engine.ExportClient(), client, assert.Same(t, engine.ExportClient(), client,
"without a per-target timeout the shared client "+ "without a per-target timeout the shared client "+
"must be returned unchanged", "must be returned unchanged",
) )
} }
// TestClientForRequest_HeadersKeepSSRFGuard asserts that the
// redirect policy an origin-scoped header installs is added to a
// client that still carries the SSRF-safe transport. The guard is
// a dial hook, so keeping it is what makes each redirect hop pass
// the private-IP check too.
func TestClientForRequest_HeadersKeepSSRFGuard(t *testing.T) {
t.Parallel()
engine := newSSRFTestEngine()
cfg := &delivery.HTTPTargetConfig{
URL: "https://example.com/with-headers",
Headers: map[string]string{
"X-Api-Key": "configured",
},
}
client := engine.ExportClientForRequest(
cfg, []string{"X-Api-Key"},
)
require.NotNil(t, client.CheckRedirect,
"an origin-scoped header must install a redirect policy",
)
assert.Same(t,
engine.ExportClient().Transport, client.Transport,
"the SSRF-safe transport must be reused, not dropped",
)
assert.Equal(t,
engine.ExportClient().Timeout, client.Timeout,
"the shared client's timeout must be inherited",
)
}

View File

@@ -121,6 +121,7 @@ type EngineParams struct {
DB *database.Database DB *database.Database
DBManager *database.WebhookDBManager DBManager *database.WebhookDBManager
Logger *logger.Logger Logger *logger.Logger
SSRFGuard *Guard
} }
// Engine processes queued deliveries in the background // Engine processes queued deliveries in the background
@@ -176,7 +177,7 @@ func New(
e.initTargets(&http.Client{ e.initTargets(&http.Client{
Timeout: httpClientTimeout, Timeout: httpClientTimeout,
Transport: NewSSRFSafeTransport(), Transport: params.SSRFGuard.NewSSRFSafeTransport(),
}) })
e.registerHooks(lc) e.registerHooks(lc)

View File

@@ -5,6 +5,8 @@ import (
"log/slog" "log/slog"
"net" "net"
"net/http" "net/http"
"net/netip"
"net/url"
"time" "time"
"go.uber.org/fx" "go.uber.org/fx"
@@ -38,6 +40,26 @@ func ExportIsBlockedIP(ip net.IP) bool {
return isBlockedIP(ip) return isBlockedIP(ip)
} }
// NewTestGuard builds an SSRF Guard from an explicit egress
// allowlist, without going through config. Passing no prefixes
// yields the default guard, which blocks every private/reserved
// range.
func NewTestGuard(allowed ...netip.Prefix) *Guard {
return &Guard{allowed: allowed}
}
// ExportCheckIP exposes the guard's single decision point, so a
// test can assert the policy both the validator and the dialer
// inherit without needing a live destination.
func (g *Guard) ExportCheckIP(ip net.IP) error {
return g.checkIP(ip)
}
// ExportAlwaysBlockedNetworks exposes alwaysBlockedNetworks.
func ExportAlwaysBlockedNetworks() []*net.IPNet {
return alwaysBlockedNetworks
}
// ExportBlockedNetworks exposes blockedNetworks. // ExportBlockedNetworks exposes blockedNetworks.
func ExportBlockedNetworks() []*net.IPNet { func ExportBlockedNetworks() []*net.IPNet {
return blockedNetworks return blockedNetworks
@@ -49,13 +71,14 @@ func ExportIsForwardableHeader(name string) bool {
} }
// ExportApplyRequestHeaders exposes applyRequestHeaders, so a test // ExportApplyRequestHeaders exposes applyRequestHeaders, so a test
// can inspect the header set an outbound delivery actually carries. // can inspect the header set an outbound delivery actually carries
// and the origin-scoped names it reports for the redirect policy.
func ExportApplyRequestHeaders( func ExportApplyRequestHeaders(
req *http.Request, req *http.Request,
event *database.Event, event *database.Event,
cfg *HTTPTargetConfig, cfg *HTTPTargetConfig,
) { ) []string {
applyRequestHeaders(req, event, cfg) return applyRequestHeaders(req, event, cfg)
} }
// ExportTruncate exposes truncate for testing. // ExportTruncate exposes truncate for testing.
@@ -165,12 +188,27 @@ func (e *Engine) ExportDoHTTPRequest(
return e.httpTarget.doHTTPRequest(ctx, cfg, event) return e.httpTarget.doHTTPRequest(ctx, cfg, event)
} }
// ExportClientForConfig exposes the http target's // ExportClientForRequest exposes the http target's
// clientForConfig. // clientForRequest.
func (e *Engine) ExportClientForConfig( func (e *Engine) ExportClientForRequest(
cfg *HTTPTargetConfig, cfg *HTTPTargetConfig,
originScoped []string,
) *http.Client { ) *http.Client {
return e.httpTarget.clientForConfig(cfg) return e.httpTarget.clientForRequest(cfg, originScoped)
}
// ErrExportTooManyRedirects exposes the sentinel the redirect
// policy returns once a chain exceeds the hop cap. It carries the
// Err prefix rather than this file's usual Export one because it
// is a sentinel error.
var ErrExportTooManyRedirects = errTooManyRedirects
// ExportMaxDeliveryRedirects exposes the redirect hop cap.
const ExportMaxDeliveryRedirects = maxDeliveryRedirects
// ExportSameDeliveryOrigin exposes sameDeliveryOrigin.
func ExportSameDeliveryOrigin(origin, dest *url.URL) bool {
return sameDeliveryOrigin(origin, dest)
} }
// ExportClient returns the http target's shared HTTP client. // ExportClient returns the http target's shared HTTP client.

View File

@@ -0,0 +1,107 @@
package delivery
import (
"errors"
"fmt"
"net"
"net/http"
"net/url"
"strings"
)
// maxDeliveryRedirects caps a redirect chain. Installing a
// CheckRedirect replaces net/http's default policy including its
// own limit, so the limit is restated rather than dropped.
const maxDeliveryRedirects = 10
// schemeHTTPS names the scheme the origin comparison treats
// specially: a step down from it is never the same origin.
const schemeHTTPS = "https"
var errTooManyRedirects = errors.New("too many redirects")
// offOriginHeaderPolicy returns a CheckRedirect that drops every
// origin-scoped header once a redirect leaves the origin the
// operator configured. names is the set applyRequestHeaders
// reports: the operator's configured headers and the inbound event
// headers this delivery forwarded, under one rule rather than two.
//
// net/http withholds Authorization and Cookie across a host change
// and forwards everything else. A target header is routinely a
// credential under another name — X-Api-Key, PRIVATE-TOKEN,
// X-Auth-Token — and a forwarded inbound header is routinely a
// sender's signature — X-Hub-Signature — so an open redirect at an
// otherwise trusted destination would hand either to a host the
// operator never named. Redirects are still followed: refusing them
// would break every destination that legitimately redirects and
// would record the 3xx as the delivery's result.
//
// The strip is per hop, not permanent: net/http re-copies the
// initial request's headers for every hop, so a chain that returns
// to the configured origin carries them again, exactly as net/http
// treats Authorization.
//
// Each hop is dialled through the same SSRF-safe transport, whose
// guard runs per connection, so a redirect aimed at a private or
// reserved address is still refused at connect time.
func offOriginHeaderPolicy(
names []string,
) func(*http.Request, []*http.Request) error {
return func(req *http.Request, via []*http.Request) error {
if len(via) >= maxDeliveryRedirects {
return fmt.Errorf(
"%w: stopped after %d",
errTooManyRedirects, maxDeliveryRedirects,
)
}
if sameDeliveryOrigin(via[0].URL, req.URL) {
return nil
}
for _, name := range names {
req.Header.Del(name)
}
return nil
}
}
// sameDeliveryOrigin reports whether dest is close enough to the
// configured target URL to keep carrying its origin-scoped headers.
//
// This is stricter than the rule net/http applies to Authorization:
// the port is part of the comparison (a different port is a
// different service), and a subdomain of the configured host is not
// the same origin. An https origin stepping down to http is never
// the same origin whatever the hosts are, because that puts the
// header on the wire in clear.
func sameDeliveryOrigin(origin, dest *url.URL) bool {
if origin.Scheme == schemeHTTPS && dest.Scheme != schemeHTTPS {
return false
}
return originHostPort(origin) == originHostPort(dest)
}
// originHostPort renders a URL's host for comparison, lowercased
// and with the scheme's default port normalised away so that
// "https://h" and "https://h:443" are one origin.
//
// The port is joined with net.JoinHostPort rather than a bare
// colon: Hostname() unwraps an IPv6 literal's brackets, so
// "[2001:db8::1]:8080" and "[2001:db8::1:8080]" — a different
// address on a different port — would otherwise render the same
// string and pass as one origin.
func originHostPort(u *url.URL) string {
host := strings.ToLower(u.Hostname())
port := u.Port()
if port == "" ||
(u.Scheme == "http" && port == "80") ||
(u.Scheme == schemeHTTPS && port == "443") {
return host
}
return net.JoinHostPort(host, port)
}

View File

@@ -0,0 +1,383 @@
package delivery_test
import (
"context"
"encoding/json"
"log/slog"
"net/http"
"net/http/httptest"
"net/url"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// The headers these tests drive stand in for the two classes the
// off-origin rule covers: an operator-configured credential and an
// inbound header the delivery path forwards. net/http withholds
// Authorization and Cookie across a host change, and nothing else.
const (
probeHeaderName = "X-Api-Key"
probeHeaderValue = "QQNEVERONTHEWIREQQ"
inboundHeaderName = "X-Hub-Signature"
inboundHeaderValue = "sha1=QQINBOUNDQQ"
)
// redirectProbe records what the last hop of a redirect chain
// actually received.
type redirectProbe struct {
mu sync.Mutex
seen http.Header
hits int
}
func (p *redirectProbe) serve(
w http.ResponseWriter, r *http.Request,
) {
p.mu.Lock()
p.seen = r.Header.Clone()
p.hits++
p.mu.Unlock()
w.WriteHeader(http.StatusOK)
}
func (p *redirectProbe) result() (http.Header, int) {
p.mu.Lock()
defer p.mu.Unlock()
return p.seen, p.hits
}
// deliverWithProbeHeaders runs one real delivery of a new task
// through the engine to targetURL, carrying both probe headers —
// probeHeaderName configured on the target, inboundHeaderName
// forwarded from the event — and returns the delivery status the
// engine recorded.
func deliverWithProbeHeaders(
t *testing.T, targetURL string,
) database.DeliveryStatus {
t.Helper()
s := newISetup(t)
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, `{"hello":"world"}`,
)
targetID := uuid.New().String()
inbound, err := json.Marshal(map[string][]string{
inboundHeaderName: {inboundHeaderValue},
})
require.NoError(t, err)
event.Headers = string(inbound)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusPending,
)
cfg, err := json.Marshal(delivery.HTTPTargetConfig{
URL: targetURL,
Headers: map[string]string{
probeHeaderName: probeHeaderValue,
},
})
require.NoError(t, err)
body := event.Body
task := iTask(
d, event, s.WebhookID, targetID,
"redirect-target", string(cfg), 0, 1, &body,
)
s.Engine.ExportProcessNewTask(context.TODO(), &task)
var updated database.Delivery
require.NoError(t, s.WebhookDB.First(
&updated, "id = ?", d.ID,
).Error)
return updated.Status
}
// A 302 to an origin the operator never configured must not carry
// the credential they configured for the one they did, nor the
// inbound header this delivery forwarded — one rule for both
// classes. The chain is still followed, so the delivery is recorded
// from the final hop.
func TestDelivery_CrossOriginRedirectDropsOriginScopedHeaders(
t *testing.T,
) {
t.Parallel()
var probe redirectProbe
final := httptest.NewServer(
http.HandlerFunc(probe.serve),
)
defer final.Close()
// httptest listens on loopback, so reach the second server
// under loopback's other name: the hop then differs in
// hostname as well as port and is cross-host by any reading.
finalURL, err := url.Parse(final.URL)
require.NoError(t, err)
finalURL.Host = "localhost:" + finalURL.Port()
finalURL.Path = "/moved"
origin := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
http.Redirect(
w, r, finalURL.String(),
http.StatusFound,
)
},
))
defer origin.Close()
status := deliverWithProbeHeaders(t, origin.URL)
seen, hits := probe.result()
assert.Equal(t, 1, hits,
"the redirect must still be followed",
)
assert.Empty(t, seen.Get(probeHeaderName),
"a configured credential header must not reach an "+
"origin the operator did not configure",
)
assert.Empty(t, seen.Get(inboundHeaderName),
"a forwarded inbound header must not reach an origin "+
"the operator did not configure",
)
assert.Equal(t,
database.DeliveryStatusDelivered, status,
"the final hop's 200 is the delivery's result",
)
}
// Stripping must not fire within the configured origin, or every
// destination that redirects its own path would lose its
// credential and start answering 401 — and would lose the inbound
// signature the receiver verifies.
func TestDelivery_SameOriginRedirectKeepsOriginScopedHeaders(
t *testing.T,
) {
t.Parallel()
var probe redirectProbe
srv := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/moved" {
probe.serve(w, r)
return
}
http.Redirect(
w, r, "/moved", http.StatusFound,
)
},
))
defer srv.Close()
status := deliverWithProbeHeaders(t, srv.URL+"/hook")
seen, hits := probe.result()
assert.Equal(t, 1, hits)
assert.Equal(t, probeHeaderValue, seen.Get(probeHeaderName),
"a redirect within the configured origin must keep "+
"the configured header",
)
assert.Equal(t,
inboundHeaderValue, seen.Get(inboundHeaderName),
"a redirect within the configured origin must keep "+
"the forwarded inbound header",
)
assert.Equal(t,
database.DeliveryStatusDelivered, status,
)
}
// The origin comparison is deliberately stricter than the one
// net/http applies to Authorization: the port counts and a
// subdomain does not inherit. Only the default-port spellings of
// one origin are the same origin.
func TestSameDeliveryOrigin(t *testing.T) {
t.Parallel()
// The configured target URL every case redirects away from.
// Destination paths differ only so that no literal repeats.
const configured = "https://h/a"
cases := map[string]struct {
origin string
dest string
want bool
}{
"other path": {configured, "https://h/b", true},
"default port spelled": {configured, "https://h:443/c", true},
"host in another case": {configured, "https://H/d", true},
"http default port": {"http://h:80/a", "http://h/e", true},
"upgrade to https": {"http://h/a", "https://h/f", true},
"downgrade to http": {configured, "http://h/g", false},
"another host": {configured, "https://i/h", false},
"a subdomain": {configured, "https://x.h/i", false},
"the parent domain": {"https://x.h/a", "https://h/j", false},
"another port": {configured, "https://h:8443/k", false},
// Hostname() unwraps an IPv6 literal's brackets, so a
// bracketed host whose last group is the origin's port
// renders identically to the origin unless the port is
// re-joined with brackets. Each dest below differs from
// its origin in address AND in port.
"ipv6 port as final group": {
"https://[2001:db8::1]:8080/a",
"https://[2001:db8::1:8080]/l",
false,
},
"ipv6 loopback port as final group": {
"https://[::1]:8080/a",
"https://[::1:8080]/m",
false,
},
"ipv6 same origin": {
"https://[2001:db8::1]:8080/a",
"https://[2001:DB8::1]:8080/n",
true,
},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
t.Parallel()
origin, err := url.Parse(tc.origin)
require.NoError(t, err)
dest, err := url.Parse(tc.dest)
require.NoError(t, err)
assert.Equal(t, tc.want,
delivery.ExportSameDeliveryOrigin(
origin, dest,
),
)
})
}
}
// Installing a CheckRedirect discards net/http's own redirect
// limit, so the cap this policy restates is the only thing between
// a self-redirecting destination and an unbounded chain. A
// destination that always redirects must be cut off after exactly
// maxDeliveryRedirects requests, with the sentinel surfacing to the
// caller rather than a generic net/http error.
func TestRedirectPolicy_StopsAtHopCap(t *testing.T) {
t.Parallel()
var hits atomic.Int64
srv := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
hits.Add(1)
http.Redirect(
w, r, "/loop", http.StatusFound,
)
},
))
defer srv.Close()
engine := delivery.NewTestEngine(
slog.New(slog.DiscardHandler),
&http.Client{Timeout: 10 * time.Second},
1,
)
client := engine.ExportClientForRequest(
&delivery.HTTPTargetConfig{URL: srv.URL},
[]string{probeHeaderName},
)
require.NotNil(t, client.CheckRedirect)
req, err := http.NewRequestWithContext(
context.Background(), http.MethodGet, srv.URL, http.NoBody,
)
require.NoError(t, err)
resp, doErr := client.Do(req)
if resp != nil {
_ = resp.Body.Close()
}
require.Error(t, doErr,
"an endless redirect chain must not be followed forever",
)
require.ErrorIs(t, doErr, delivery.ErrExportTooManyRedirects)
assert.Equal(t,
int64(delivery.ExportMaxDeliveryRedirects), hits.Load(),
"the chain must stop after exactly %d hops",
delivery.ExportMaxDeliveryRedirects,
)
}
// The set the redirect policy strips is whatever the delivery path
// actually put on the wire, so a header added to the forward set is
// covered without a second edit. A header the event never carried
// is not in the set, and the delivery path's own two are deliberately
// excluded: Content-Type describes the body, which a 307 carries
// across hosts, and the inbound User-Agent every real sender supplies
// is overwritten before the request goes out.
func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
t.Parallel()
inbound, err := json.Marshal(map[string][]string{
inboundHeaderName: {inboundHeaderValue},
"Content-Type": {testContentType},
"User-Agent": {"curl/8.7.1"},
"Host": {"inbound.example.com"},
})
require.NoError(t, err)
req, err := http.NewRequestWithContext(
context.Background(),
http.MethodPost,
"https://target.example.com/hook",
http.NoBody,
)
require.NoError(t, err)
names := delivery.ExportApplyRequestHeaders(
req,
&database.Event{
Headers: string(inbound),
ContentType: testContentType,
},
&delivery.HTTPTargetConfig{
Headers: map[string]string{
probeHeaderName: probeHeaderValue,
},
},
)
assert.Equal(t,
[]string{probeHeaderName, inboundHeaderName}, names,
"both header classes are reported, and only those: "+
"Host is never forwarded, Content-Type and "+
"User-Agent are the delivery path's own",
)
}

View File

@@ -6,8 +6,11 @@ import (
"fmt" "fmt"
"net" "net"
"net/http" "net/http"
"net/netip"
"net/url" "net/url"
"time" "time"
"sneak.berlin/go/webhooker/internal/config"
) )
const ( const (
@@ -25,20 +28,83 @@ var (
errBlockedIP = errors.New( errBlockedIP = errors.New(
"blocked private/reserved IP range", "blocked private/reserved IP range",
) )
errBlockedMetadata = errors.New(
"blocked link-local or cloud instance metadata " +
"address: ALLOWED_EGRESS_CIDRS cannot open it",
)
errInvalidScheme = errors.New( errInvalidScheme = errors.New(
"only http and https are allowed", "only http and https are allowed",
) )
) )
// blockedNetworks contains all private/reserved IP ranges // blockedNetworks contains all private/reserved IP ranges
// that should be blocked to prevent SSRF attacks. // that should be blocked to prevent SSRF attacks. An operator
// can permit specific blocks out of this set with
// ALLOWED_EGRESS_CIDRS; see Guard.
// //
//nolint:gochecknoglobals // package-level network list is appropriate here //nolint:gochecknoglobals // package-level network list is appropriate here
var blockedNetworks []*net.IPNet var blockedNetworks []*net.IPNet
// alwaysBlockedNetworks are the ranges no configuration can
// open: the link-local blocks and the cloud instance metadata
// endpoints that live outside them. Reaching one is credential
// or user-data theft rather than delivery to an internal
// service, so a supplied CIDR that covers such an address still
// leaves it blocked.
//
// Inclusion criterion — an address belongs here only if BOTH
// hold, and every entry below satisfies both:
//
// 1. It is a fixed address assigned by the provider, or a
// range reserved by IANA — never one the operator chose.
// That is what makes a host route free: it cannot collide
// with anything the operator runs.
// 2. Reaching it discloses credentials, or user data or
// bootstrap material — something granting onward access, or
// not cheaply rotated.
//
// Both halves are load-bearing, so use them to refuse a
// candidate and say why. An endpoint disclosing only the
// operator's own inventory (instance id, region, disks, NICs)
// fails (2): letting a delivery target reach the operator's own
// infrastructure is the feature ALLOWED_EGRESS_CIDRS exists to
// provide. But (2) is not "IAM credentials only" either —
// fd00:42::42 serves /user_data and /conf rather than tokens,
// and user data routinely carries bootstrap secrets. An address
// stays out if it fails (1) however well it clears (2): a host
// route inside a block operators really assign from, such as
// 10.0.0.0/8, can collide with a real internal service and
// forfeits the justification in (1).
//
// A publicly routable unicast address does not belong here even
// when it clears both halves. Nothing in this list can be
// reopened, so putting a public address here leaves the operator
// no escape hatch at all — the condition ALLOWED_EGRESS_CIDRS
// exists to remove. Default-block it in blockedNetworks instead,
// which an allowlist can override.
//
// This is a criterion, not an enumeration of every metadata
// address in existence.
//
// Every entry is either already in blockedNetworks — this list is
// what makes it unconditional — or an alternate encoding of
// 169.254.169.254 that Contains does not match against
// 169.254.0.0/16. Every entry outside the link-local blocks is a
// /32 or /128 host route, so blocking it costs an operator
// nothing else on the surrounding network.
//
// Derive membership from the address, never from the vendor's
// prose. Several providers call these endpoints "link-local" or
// even "localhost" in their own documentation while the address
// is a ULA outside fe80::/10, so a set derived from the docs
// comes out wrong.
//
//nolint:gochecknoglobals // package-level network list is appropriate here
var alwaysBlockedNetworks []*net.IPNet
//nolint:gochecknoinits // init is the idiomatic way to parse CIDRs once at startup //nolint:gochecknoinits // init is the idiomatic way to parse CIDRs once at startup
func init() { func init() {
cidrs := []string{ blockedNetworks = mustParseCIDRs([]string{
"127.0.0.0/8", "127.0.0.0/8",
"10.0.0.0/8", "10.0.0.0/8",
"172.16.0.0/12", "172.16.0.0/12",
@@ -56,7 +122,72 @@ func init() {
"::1/128", "::1/128",
"fc00::/7", "fc00::/7",
"fe80::/10", "fe80::/10",
} })
// Every entry is named. The set must not grow or shrink
// without a matching change to
// TestAlwaysBlockedNetworks_PinnedSet.
//
// The IPv4-mapped form ::ffff:169.254.169.254 needs no
// entry: net.IPNet.Contains normalises it via To4() before
// comparing, so 169.254.0.0/16 already matches it. To4()
// does not normalise the IPv4-compatible or NAT64 forms,
// which is why those are listed separately.
alwaysBlockedNetworks = mustParseCIDRs([]string{
// IPv4 link-local, carrying the 169.254.169.254
// metadata service used by AWS, Azure, DigitalOcean,
// Hetzner, OpenStack and others. Not Alibaba, which uses
// 100.100.100.200 below exclusively.
"169.254.0.0/16",
// IPv6 link-local, its IPv6 counterpart.
"fe80::/10",
// IPv6 metadata endpoints in ULA space. Each is a host
// route, and fd00::/8 is an ordinary block for an
// operator to allowlist, so without these entries that
// one allowlist line hands out cloud credentials on
// every provider below.
//
// AWS IPv6 IMDS.
"fd00:ec2::254/128",
// AWS EKS Pod Identity Agent, which issues pod identity
// credentials. A second AWS endpoint, distinct from
// IMDS above. AWS's own docs call it "localhost".
"fd00:ec2::23/128",
// GCP metadata server for IPv6-only instances.
"fd20:ce::254/128",
// Oracle OCI IMDS, serving /opc/v2 instance principals.
"fd00:c1::a9fe:a9fe/128",
// Scaleway metadata, serving /user_data and /conf.
"fd00:42::42/128",
// Linode/Akamai metadata. Akamai's docs call it
// "link-local"; it is not.
"fd00:a9fe:a9fe::1/128",
// IPv4 metadata endpoints outside link-local.
//
// Alibaba Cloud metadata. It sits in CGNAT
// 100.64.0.0/10, which Tailscale also uses, so an
// operator allowlisting a Tailscale peer's range would
// otherwise reopen it.
"100.100.100.200/32",
// Oracle Cloud Classic metadata. Inside the blocked
// 192.0.0.0/24, so this entry is what stops an
// allowlist from opening it.
"192.0.0.192/32",
// 169.254.169.254 as an IPv4-compatible IPv6 address.
"::a9fe:a9fe/128",
// 169.254.169.254 behind the NAT64 well-known prefix.
"64:ff9b::a9fe:a9fe/128",
})
}
// mustParseCIDRs parses a list of CIDR literals, panicking on a
// bad one. The inputs are compile-time constants, so a failure
// is a programming error rather than a runtime condition.
func mustParseCIDRs(cidrs []string) []*net.IPNet {
networks := make([]*net.IPNet, 0, len(cidrs))
for _, cidr := range cidrs { for _, cidr := range cidrs {
_, network, err := net.ParseCIDR(cidr) _, network, err := net.ParseCIDR(cidr)
@@ -67,16 +198,15 @@ func init() {
)) ))
} }
blockedNetworks = append( networks = append(networks, network)
blockedNetworks, network,
)
} }
return networks
} }
// isBlockedIP checks whether an IP address falls within // matchesAny reports whether ip falls inside any of networks.
// any blocked private/reserved network range. func matchesAny(networks []*net.IPNet, ip net.IP) bool {
func isBlockedIP(ip net.IP) bool { for _, network := range networks {
for _, network := range blockedNetworks {
if network.Contains(ip) { if network.Contains(ip) {
return true return true
} }
@@ -85,9 +215,40 @@ func isBlockedIP(ip net.IP) bool {
return false return false
} }
// isBlockedIP checks whether an IP address falls within
// any blocked private/reserved network range, before any
// operator allowlist is considered.
func isBlockedIP(ip net.IP) bool {
return matchesAny(blockedNetworks, ip)
}
// Guard makes every SSRF decision in the process.
//
// It holds the operator's ALLOWED_EGRESS_CIDRS allowlist and
// applies it in exactly one place, checkIP, which both the
// target-creation validator (ValidateTargetURL) and the delivery
// dialer call. Routing both through the same function is the
// point: when the two paths decided separately they drifted and
// disagreed, which is what made a target creatable but
// undeliverable.
//
// The guard is always on. The allowlist only ever adds specific
// networks to what the default blocklist refuses, and no
// configuration turns the guard off wholesale.
type Guard struct {
// allowed is the operator's ALLOWED_EGRESS_CIDRS. Empty
// (the default) means the default blocklist stands as-is.
allowed []netip.Prefix
}
// NewGuard builds the process-wide SSRF guard from configuration.
func NewGuard(cfg *config.Config) *Guard {
return &Guard{allowed: cfg.AllowedEgressCIDRs}
}
// ValidateTargetURL checks that an HTTP delivery target // ValidateTargetURL checks that an HTTP delivery target
// URL is safe from SSRF attacks. // URL is safe from SSRF attacks.
func ValidateTargetURL( func (g *Guard) ValidateTargetURL(
ctx context.Context, targetURL string, ctx context.Context, targetURL string,
) error { ) error {
parsed, err := url.Parse(targetURL) parsed, err := url.Parse(targetURL)
@@ -111,36 +272,79 @@ func ValidateTargetURL(
} }
if ip := net.ParseIP(host); ip != nil { if ip := net.ParseIP(host); ip != nil {
return checkBlockedIP(ip) return g.checkIP(ip)
} }
return validateHostname(ctx, host) return g.validateHostname(ctx, host)
} }
func validateScheme(scheme string) error { // NewSSRFSafeTransport creates an http.Transport with a
if scheme != "http" && scheme != "https" { // custom DialContext that refuses connections to any address
// this guard blocks. It resolves and checks at dial time, so a
// name that passed validation but now answers with a blocked
// address (DNS rebinding) is still refused.
func (g *Guard) NewSSRFSafeTransport() *http.Transport {
return &http.Transport{
DialContext: g.ssrfDialContext,
}
}
// allows reports whether ip falls inside the operator's
// configured egress allowlist.
func (g *Guard) allows(ip net.IP) bool {
if len(g.allowed) == 0 {
return false
}
addr, ok := netip.AddrFromSlice(ip)
if !ok {
return false
}
// Config unmaps every parsed prefix, so an IPv4-mapped
// address has to be unmapped too or it would never match.
addr = addr.Unmap()
for _, prefix := range g.allowed {
if prefix.Contains(addr) {
return true
}
}
return false
}
// checkIP is the single point at which SSRF policy is decided.
//
// The order is the policy:
//
// 1. alwaysBlockedNetworks is refused before the allowlist is
// consulted, so no configured CIDR reaches link-local or a
// cloud instance metadata endpoint.
// 2. The allowlist is consulted next, so a listed private
// network becomes reachable.
// 3. Everything else keeps the default blocklist's answer.
func (g *Guard) checkIP(ip net.IP) error {
if matchesAny(alwaysBlockedNetworks, ip) {
return fmt.Errorf( return fmt.Errorf(
"unsupported URL scheme %q: %w", "target IP %s: %w", ip, errBlockedMetadata,
scheme, errInvalidScheme,
) )
} }
return nil if g.allows(ip) {
} return nil
}
func checkBlockedIP(ip net.IP) error {
if isBlockedIP(ip) { if isBlockedIP(ip) {
return fmt.Errorf( return fmt.Errorf(
"target IP %s is in a blocked "+ "target IP %s: %w", ip, errBlockedIP,
"private/reserved range: %w",
ip, errBlockedIP,
) )
} }
return nil return nil
} }
func validateHostname( func (g *Guard) validateHostname(
ctx context.Context, host string, ctx context.Context, host string,
) error { ) error {
dnsCtx, cancel := context.WithTimeout( dnsCtx, cancel := context.WithTimeout(
@@ -165,11 +369,11 @@ func validateHostname(
} }
for _, ipAddr := range ips { for _, ipAddr := range ips {
if isBlockedIP(ipAddr.IP) { err = g.checkIP(ipAddr.IP)
if err != nil {
return fmt.Errorf( return fmt.Errorf(
"hostname %q resolves to blocked "+ "hostname %q resolves to a blocked address: %w",
"IP %s: %w", host, err,
host, ipAddr.IP, errBlockedIP,
) )
} }
} }
@@ -177,16 +381,7 @@ func validateHostname(
return nil return nil
} }
// NewSSRFSafeTransport creates an http.Transport with a func (g *Guard) ssrfDialContext(
// custom DialContext that blocks connections to
// private/reserved IP addresses.
func NewSSRFSafeTransport() *http.Transport {
return &http.Transport{
DialContext: ssrfDialContext,
}
}
func ssrfDialContext(
ctx context.Context, ctx context.Context,
network, addr string, network, addr string,
) (net.Conn, error) { ) (net.Conn, error) {
@@ -209,11 +404,11 @@ func ssrfDialContext(
} }
for _, ipAddr := range ips { for _, ipAddr := range ips {
if isBlockedIP(ipAddr.IP) { err = g.checkIP(ipAddr.IP)
if err != nil {
return nil, fmt.Errorf( return nil, fmt.Errorf(
"ssrf: connection to %s (%s) "+ "ssrf: connection to %s blocked: %w",
"blocked: %w", host, err,
host, ipAddr.IP, errBlockedIP,
) )
} }
} }
@@ -225,3 +420,14 @@ func ssrfDialContext(
net.JoinHostPort(ips[0].IP.String(), port), net.JoinHostPort(ips[0].IP.String(), port),
) )
} }
func validateScheme(scheme string) error {
if scheme != "http" && scheme != "https" {
return fmt.Errorf(
"unsupported URL scheme %q: %w",
scheme, errInvalidScheme,
)
}
return nil
}

View File

@@ -0,0 +1,562 @@
package delivery_test
import (
"context"
"net"
"net/http"
"net/http/httptest"
"net/netip"
"net/url"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/delivery"
)
// Addresses the SSRF tests in this package share.
const (
// metadataIP is the cloud instance metadata address, and
// metadataURL an endpoint on it. The guard must never reach
// either, whatever an operator lists.
metadataIP = "169.254.169.254"
metadataURL = "http://" + metadataIP + "/latest/meta-data/"
// loopbackHookURL is a target on this host: blocked by
// default, reachable only once an operator allowlists
// loopback.
loopbackHookURL = "http://127.0.0.1/hook"
// publicIP is an ordinary public address, which the guard
// permits with or without an allowlist.
publicIP = "93.184.216.34"
// allowAllIPv4 and allowAllIPv6 are the widest allowlist
// entries expressible: the whole internet, in each family.
// Nothing unconditionally blocked may be reachable under
// them.
allowAllIPv4 = "0.0.0.0/0"
allowAllIPv6 = "::/0"
// allowAllULA is the ordinary ULA block an operator lists to
// reach their own IPv6 network. Several providers park a
// metadata endpoint inside it.
allowAllULA = "fd00::/8"
// metadataRefusalClause is the part of the refusal that only
// alwaysBlockedNetworks produces. Asserting it, rather than
// the bare word "blocked", is what proves the unconditional
// set did the refusing and not the default blocklist.
metadataRefusalClause = "ALLOWED_EGRESS_CIDRS cannot open it"
)
// TestGuardAllowlist_PermittedCIDRDelivers proves the escape
// hatch actually works end to end: with 127.0.0.0/8 allowed, the
// guard's own transport connects to a loopback server and gets a
// response back. The default guard, given the identical URL,
// refuses it — so the delivery succeeds because of the allowlist
// and nothing else.
func TestGuardAllowlist_PermittedCIDRDelivers(t *testing.T) {
t.Parallel()
srv := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNoContent)
},
))
t.Cleanup(srv.Close)
// httptest listens on loopback, which the default blocklist
// covers: exactly the "forward to a service on this host"
// case the allowlist exists for.
requireLoopback(t, srv.URL)
guard := delivery.NewTestGuard(
netip.MustParsePrefix("127.0.0.0/8"),
)
require.NoError(t,
guard.ValidateTargetURL(context.Background(), srv.URL),
"an allowlisted loopback target must pass validation",
)
client := &http.Client{
Timeout: 5 * time.Second,
Transport: guard.NewSSRFSafeTransport(),
}
req, err := http.NewRequestWithContext(
context.Background(), http.MethodPost, srv.URL, nil,
)
require.NoError(t, err)
resp, err := client.Do(req)
require.NoError(t,
err, "an allowlisted loopback target must be deliverable",
)
defer func() { _ = resp.Body.Close() }()
assert.Equal(t, http.StatusNoContent, resp.StatusCode)
// The same URL through the default guard must still fail, or
// this test would pass without the allowlist doing anything.
assert.Error(t,
delivery.NewTestGuard().ValidateTargetURL(
context.Background(), srv.URL,
),
"without the allowlist the same target must be refused",
)
}
// TestGuardAllowlist_UnlistedPrivateStillRefused proves the
// allowlist grants only what it names. A guard that opens one
// private block must keep refusing every other one, at both the
// validation and the delivery entry point.
func TestGuardAllowlist_UnlistedPrivateStillRefused(t *testing.T) {
t.Parallel()
// Only 10.1.0.0/16 is open — a narrow block inside a much
// wider private range, so the test can tell "permits the
// listed block" from "permits anything private".
guard := delivery.NewTestGuard(
netip.MustParsePrefix("10.1.0.0/16"),
)
refused := []string{
"http://192.168.1.10/hook",
"http://172.16.0.1/hook",
loopbackHookURL,
"http://[fc00::1]/hook",
"http://100.64.0.1/hook",
// Private, adjacent to the allowed block, outside it.
"http://10.2.0.1/hook",
}
for _, target := range refused {
t.Run(target, func(t *testing.T) {
t.Parallel()
err := guard.ValidateTargetURL(
context.Background(), target,
)
require.Error(t,
err, "%s is not allowlisted and must be refused",
target,
)
assert.Contains(t, err.Error(), "blocked")
assertDialRefused(t, guard, target)
})
}
// The block that is listed must in fact be permitted, so the
// refusals above are selective rather than a guard that
// ignores its allowlist entirely.
assert.NoError(t,
guard.ValidateTargetURL(
context.Background(), "http://10.1.2.3/hook",
),
"the allowlisted block must be permitted",
)
}
// TestGuardAllowlist_MetadataAlwaysRefused is the load-bearing
// case: cloud instance metadata endpoints are credential theft
// rather than delivery to an internal service, so no allowlist
// reaches one. Every guard below names a CIDR that covers its
// target — including 0.0.0.0/0, ::/0, and the ordinary ULA and
// CGNAT blocks an operator would really list — and the address
// must stay refused anyway, on both the validation and the
// delivery path.
func TestGuardAllowlist_MetadataAlwaysRefused(t *testing.T) {
t.Parallel()
for _, tt := range metadataAlwaysRefusedCases() {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
guard := delivery.NewTestGuard(
netip.MustParsePrefix(tt.allow),
)
err := guard.ValidateTargetURL(
context.Background(), tt.target,
)
require.Error(t,
err,
"%s must stay blocked even though %s covers it",
tt.target, tt.allow,
)
assert.Contains(t,
err.Error(),
metadataRefusalClause,
"the refusal must say why it cannot be opened",
)
// The metadata clause, not just "blocked": that is
// what distinguishes the unconditional set from the
// ordinary blocklist.
assertDialRefusedWith(
t, guard, tt.target, metadataRefusalClause,
)
})
}
}
// metadataAlwaysRefusedCase is one (allowlist, target) pair that
// must be refused: allow covers target, and target must stay
// blocked regardless.
type metadataAlwaysRefusedCase struct {
name string
allow string
target string
}
// metadataAlwaysRefusedCases enumerates every unconditionally
// blocked address together with an allowlist entry that would
// otherwise reach it. Split by family of address only to stay
// under the function-length limit.
func metadataAlwaysRefusedCases() []metadataAlwaysRefusedCase {
cases := linkLocalRefusedCases()
cases = append(cases, ulaMetadataRefusedCases()...)
cases = append(cases, ipv4MetadataRefusedCases()...)
return append(cases, encodedMetadataRefusedCases()...)
}
// linkLocalRefusedCases covers the link-local blocks, including
// an operator naming the metadata address outright.
func linkLocalRefusedCases() []metadataAlwaysRefusedCase {
return []metadataAlwaysRefusedCase{
{
name: "exact metadata host",
allow: "169.254.169.254/32",
target: metadataURL,
},
{
name: "whole link-local block",
allow: "169.254.0.0/16",
target: metadataURL,
},
{
name: "supernet covering link-local",
allow: "169.0.0.0/8",
target: metadataURL,
},
{
name: "the entire IPv4 internet",
allow: allowAllIPv4,
target: metadataURL,
},
{
name: "other link-local address",
allow: allowAllIPv4,
target: "http://169.254.1.1/",
},
{
name: "IPv6 link-local",
allow: allowAllIPv6,
target: "http://[fe80::1]/",
},
}
}
// ulaMetadataRefusedCases covers the metadata endpoints parked
// in ULA space. Every one is opened by the single ordinary
// allowlist entry fd00::/8, which is the whole reason they need
// their own /128 host routes: fe80::/10 does not cover a ULA,
// whatever the vendor's documentation calls the address.
func ulaMetadataRefusedCases() []metadataAlwaysRefusedCase {
return []metadataAlwaysRefusedCase{
{
name: "AWS IPv6 IMDS under an allowlisted ULA block",
allow: allowAllULA,
target: "http://[fd00:ec2::254]/latest/meta-data/",
},
{
// A second AWS credential endpoint, distinct from
// IMDS. AWS's own docs call this one "localhost".
name: "AWS EKS Pod Identity under an allowlisted ULA block",
allow: allowAllULA,
target: "http://[fd00:ec2::23]/v1/credentials",
},
{
name: "GCP IPv6 metadata under an allowlisted ULA block",
allow: allowAllULA,
target: "http://[fd20:ce::254]/computeMetadata/v1/",
},
{
name: "Oracle OCI IPv6 IMDS under an allowlisted ULA block",
allow: allowAllULA,
target: "http://[fd00:c1::a9fe:a9fe]/opc/v2/instance/",
},
{
name: "Scaleway IPv6 metadata under an allowlisted ULA block",
allow: allowAllULA,
target: "http://[fd00:42::42]/conf",
},
{
// Akamai's docs call this "link-local"; it is a ULA,
// so fe80::/10 does not cover it.
name: "Linode IPv6 metadata under an allowlisted ULA block",
allow: allowAllULA,
target: "http://[fd00:a9fe:a9fe::1]/v1/instance",
},
}
}
// ipv4MetadataRefusedCases covers the IPv4 metadata endpoints
// that sit outside link-local: one in CGNAT and one in the
// blocked 192.0.0.0/24, each reachable only through an allowlist
// that this set overrides.
func ipv4MetadataRefusedCases() []metadataAlwaysRefusedCase {
return []metadataAlwaysRefusedCase{
{
// Tailscale uses 100.64.0.0/10, so an operator
// forwarding to a Tailscale peer lists exactly this.
name: "Alibaba metadata under allowlisted CGNAT",
allow: "100.64.0.0/10",
target: "http://100.100.100.200/latest/meta-data/",
},
{
// Inside the already-blocked 192.0.0.0/24, so only
// an allowlist can reach it — and must not.
name: "Oracle Cloud Classic metadata under 0.0.0.0/0",
allow: allowAllIPv4,
target: "http://192.0.0.192/latest/meta-data/",
},
}
}
// encodedMetadataRefusedCases covers the alternate IPv6
// encodings of 169.254.169.254.
func encodedMetadataRefusedCases() []metadataAlwaysRefusedCase {
return []metadataAlwaysRefusedCase{
{
// To4() does not normalise the IPv4-compatible form,
// so this needs its own always-blocked entry.
name: "IPv4-compatible IPv6 form of the metadata IP",
allow: allowAllIPv6,
target: "http://[::a9fe:a9fe]/latest/meta-data/",
},
{
// Nor the NAT64 well-known prefix form.
name: "NAT64 form of the metadata IP",
allow: allowAllIPv6,
target: "http://[64:ff9b::a9fe:a9fe]/latest/meta-data/",
},
{
// Already refused before this change: IPNet.Contains
// calls To4() first, so the mapped form matches
// 169.254.0.0/16. Pinned so it cannot regress.
//
// Allowed under 0.0.0.0/0 rather than ::/0: allows()
// unmaps before matching, so ::/0 would not cover the
// unmapped v4 address and the case would not prove
// the allowlist was overridden.
name: "IPv4-mapped IPv6 form of the metadata IP",
allow: allowAllIPv4,
target: "http://[::ffff:169.254.169.254]/latest/meta-data/",
},
}
}
// TestGuardAllowlist_PublicUnaffected asserts the allowlist does
// not narrow anything: public addresses were reachable before it
// existed and stay reachable, whether or not a list is set.
func TestGuardAllowlist_PublicUnaffected(t *testing.T) {
t.Parallel()
guards := map[string]*delivery.Guard{
"default": delivery.NewTestGuard(),
"with allowlist": delivery.NewTestGuard(
netip.MustParsePrefix("10.0.0.0/8"),
),
}
for name, guard := range guards {
t.Run(name, func(t *testing.T) {
t.Parallel()
assert.NoError(t,
guard.ValidateTargetURL(
context.Background(),
"http://"+publicIP+"/webhook",
),
)
})
}
}
// TestGuardCheckIP_BothPathsShareOneDecision asserts that the
// validator and the dialer are not two policies that happen to
// agree: both are defined in terms of checkIP, so the exported
// decision function is the whole answer for a given address.
func TestGuardCheckIP_BothPathsShareOneDecision(t *testing.T) {
t.Parallel()
guard := delivery.NewTestGuard(
netip.MustParsePrefix("10.0.0.0/8"),
)
tests := []struct {
ip string
allowed bool
}{
{"10.1.2.3", true},
{publicIP, true},
{"192.168.1.1", false},
{"127.0.0.1", false},
{metadataIP, false},
}
for _, tt := range tests {
t.Run(tt.ip, func(t *testing.T) {
t.Parallel()
ip := net.ParseIP(tt.ip)
require.NotNil(t, ip)
decision := guard.ExportCheckIP(ip)
validation := guard.ValidateTargetURL(
context.Background(), "http://"+hostFor(tt.ip)+"/x",
)
if tt.allowed {
require.NoError(t, decision)
require.NoError(t, validation)
return
}
require.Error(t, decision)
require.Error(t, validation,
"validation must refuse what checkIP refuses",
)
})
}
}
// TestAlwaysBlockedNetworks_PinnedSet pins the unconditional set
// exactly, so it cannot quietly grow or shrink.
//
// It stays deliberately small. Everything else in the default
// blocklist is an operator's own network and must remain
// openable, or the escape hatch would not work — which is why
// the metadata endpoints outside the link-local range are host
// routes rather than the blocks that contain them.
func TestAlwaysBlockedNetworks_PinnedSet(t *testing.T) {
t.Parallel()
nets := delivery.ExportAlwaysBlockedNetworks()
got := make([]string, 0, len(nets))
for _, n := range nets {
got = append(got, n.String())
}
want := []string{
// IPv4 link-local: the 169.254.169.254 metadata
// service on AWS, Azure and others.
"169.254.0.0/16",
// IPv6 link-local.
"fe80::/10",
// AWS IPv6 IMDS, inside the ULA space an operator may
// legitimately allowlist.
"fd00:ec2::254/128",
// AWS EKS Pod Identity Agent, likewise ULA.
"fd00:ec2::23/128",
// GCP metadata for IPv6-only instances, likewise ULA.
"fd20:ce::254/128",
// Oracle OCI IMDS over IPv6, likewise ULA.
"fd00:c1::a9fe:a9fe/128",
// Scaleway metadata over IPv6, likewise ULA.
"fd00:42::42/128",
// Linode/Akamai metadata over IPv6, likewise ULA.
"fd00:a9fe:a9fe::1/128",
// Alibaba Cloud metadata, inside CGNAT.
"100.100.100.200/32",
// Oracle Cloud Classic metadata, inside the blocked
// 192.0.0.0/24.
"192.0.0.192/32",
// 169.254.169.254 as an IPv4-compatible IPv6 address.
"::a9fe:a9fe/128",
// 169.254.169.254 behind the NAT64 well-known prefix.
"64:ff9b::a9fe:a9fe/128",
}
assert.Equal(t, want, got)
}
// requireLoopback fails the test unless rawURL's host is a
// loopback address, so the allowlist test cannot silently stop
// exercising a blocked range.
func requireLoopback(t *testing.T, rawURL string) {
t.Helper()
parsed, err := url.Parse(rawURL)
require.NoError(t, err)
ip := net.ParseIP(parsed.Hostname())
require.NotNil(t, ip, "test server host must be an IP literal")
require.True(t, ip.IsLoopback(),
"test server must listen on loopback, got %s", ip,
)
}
// assertDialRefused asserts the guard's transport refuses to
// connect to target, which is the delivery-time half of the
// policy. It never reaches the network: the guard checks the
// resolved address before dialling.
func assertDialRefused(
t *testing.T, guard *delivery.Guard, target string,
) {
t.Helper()
assertDialRefusedWith(t, guard, target, "blocked")
}
// assertDialRefusedWith is assertDialRefused with the refusal
// text pinned. Callers testing the unconditional set pass
// metadataRefusalClause so the subtest cannot pass on an
// ordinary blocklist refusal instead.
func assertDialRefusedWith(
t *testing.T, guard *delivery.Guard, target, clause string,
) {
t.Helper()
client := &http.Client{
Timeout: 5 * time.Second,
Transport: guard.NewSSRFSafeTransport(),
}
req, err := http.NewRequestWithContext(
context.Background(), http.MethodPost, target, nil,
)
require.NoError(t, err)
resp, err := client.Do(req)
if resp != nil {
_ = resp.Body.Close()
}
require.Error(t, err,
"delivery to %s must be refused by the dialer", target,
)
assert.Contains(t, err.Error(), clause,
"the refusal must come from the SSRF guard",
)
}
// hostFor renders an IP as it appears in a URL host, bracketing
// IPv6 literals.
func hostFor(ip string) string {
if net.ParseIP(ip).To4() == nil {
return "[" + ip + "]"
}
return ip
}

View File

@@ -31,10 +31,10 @@ func TestIsBlockedIP_PrivateRanges(t *testing.T) {
{"192.168.0.1", "192.168.0.1", true}, {"192.168.0.1", "192.168.0.1", true},
{"192.168.255.255", "192.168.255.255", true}, {"192.168.255.255", "192.168.255.255", true},
{"169.254.0.1", "169.254.0.1", true}, {"169.254.0.1", "169.254.0.1", true},
{"169.254.169.254", "169.254.169.254", true}, {metadataIP, metadataIP, true},
{"8.8.8.8", "8.8.8.8", false}, {"8.8.8.8", "8.8.8.8", false},
{"1.1.1.1", "1.1.1.1", false}, {"1.1.1.1", "1.1.1.1", false},
{"93.184.216.34", "93.184.216.34", false}, {publicIP, publicIP, false},
{"::1", "::1", true}, {"::1", "::1", true},
{"fd00::1", "fd00::1", true}, {"fd00::1", "fd00::1", true},
{"fc00::1", "fc00::1", true}, {"fc00::1", "fc00::1", true},
@@ -72,12 +72,12 @@ func TestValidateTargetURL_Blocked(t *testing.T) {
t.Parallel() t.Parallel()
blockedURLs := []string{ blockedURLs := []string{
"http://127.0.0.1/hook", loopbackHookURL,
"http://127.0.0.1:8080/hook", "http://127.0.0.1:8080/hook",
"https://10.0.0.1/hook", "https://10.0.0.1/hook",
"http://192.168.1.1/webhook", "http://192.168.1.1/webhook",
"http://172.16.0.1/api", "http://172.16.0.1/api",
"http://169.254.169.254/latest/meta-data/", metadataURL,
"http://[::1]/hook", "http://[::1]/hook",
"http://[fc00::1]/hook", "http://[fc00::1]/hook",
"http://[fe80::1]/hook", "http://[fe80::1]/hook",
@@ -88,7 +88,7 @@ func TestValidateTargetURL_Blocked(t *testing.T) {
t.Run(u, func(t *testing.T) { t.Run(u, func(t *testing.T) {
t.Parallel() t.Parallel()
err := delivery.ValidateTargetURL( err := delivery.NewTestGuard().ValidateTargetURL(
context.Background(), u, context.Background(), u,
) )
@@ -112,7 +112,7 @@ func TestValidateTargetURL_Allowed(t *testing.T) {
t.Run(u, func(t *testing.T) { t.Run(u, func(t *testing.T) {
t.Parallel() t.Parallel()
err := delivery.ValidateTargetURL( err := delivery.NewTestGuard().ValidateTargetURL(
context.Background(), u, context.Background(), u,
) )
@@ -126,7 +126,7 @@ func TestValidateTargetURL_Allowed(t *testing.T) {
func TestValidateTargetURL_InvalidScheme(t *testing.T) { func TestValidateTargetURL_InvalidScheme(t *testing.T) {
t.Parallel() t.Parallel()
err := delivery.ValidateTargetURL( err := delivery.NewTestGuard().ValidateTargetURL(
context.Background(), "ftp://example.com/hook", context.Background(), "ftp://example.com/hook",
) )
@@ -140,7 +140,7 @@ func TestValidateTargetURL_InvalidScheme(t *testing.T) {
func TestValidateTargetURL_EmptyHost(t *testing.T) { func TestValidateTargetURL_EmptyHost(t *testing.T) {
t.Parallel() t.Parallel()
err := delivery.ValidateTargetURL( err := delivery.NewTestGuard().ValidateTargetURL(
context.Background(), "http:///path", context.Background(), "http:///path",
) )
@@ -150,7 +150,7 @@ func TestValidateTargetURL_EmptyHost(t *testing.T) {
func TestValidateTargetURL_InvalidURL(t *testing.T) { func TestValidateTargetURL_InvalidURL(t *testing.T) {
t.Parallel() t.Parallel()
err := delivery.ValidateTargetURL( err := delivery.NewTestGuard().ValidateTargetURL(
context.Background(), "://invalid", context.Background(), "://invalid",
) )

View File

@@ -65,6 +65,11 @@ func isReservedTargetHeader(name string) bool {
// the configured headers, so a configured one would always // the configured headers, so a configured one would always
// be overwritten. // be overwritten.
return true return true
case "Trailer":
// net/http strips Trailer from the request it writes
// (reqWriteExcludeHeader), so a configured one is accepted
// and stored and then provably never reaches the wire.
return true
default: default:
return false return false
} }
@@ -118,9 +123,11 @@ func parseHeaderLine(line string) (string, string, error) {
rawName = strings.TrimSpace(rawName) rawName = strings.TrimSpace(rawName)
if !validHeaderName(rawName) { if !validHeaderName(rawName) {
return "", "", fmt.Errorf( // Quotes nothing. The text before the first colon is only
"%w: %q", errHeaderNameInvalid, rawName, // a name if it parses as one; when it does not, it is as
) // likely to be a pasted value whose own colon split the
// line, and half of a token would be echoed into the 400.
return "", "", errHeaderNameInvalid
} }
name := http.CanonicalHeaderKey(rawName) name := http.CanonicalHeaderKey(rawName)

View File

@@ -82,6 +82,16 @@ func TestParseTargetHeaders_Rejects(t *testing.T) {
} }
} }
// net/http strips Trailer from the request it writes, so accepting
// one would store a header that never reaches the target.
func TestParseTargetHeaders_RejectsTrailer(t *testing.T) {
t.Parallel()
_, err := delivery.ParseTargetHeaders("Trailer: X-Checksum")
require.Error(t, err)
assert.Contains(t, err.Error(), "Trailer")
}
// A header value is routinely a bearer token and these errors are // A header value is routinely a bearer token and these errors are
// rendered into a 400 body, so no message may quote one. // rendered into a 400 body, so no message may quote one.
func TestParseTargetHeaders_ErrorsNeverQuoteAValue(t *testing.T) { func TestParseTargetHeaders_ErrorsNeverQuoteAValue(t *testing.T) {
@@ -89,17 +99,26 @@ func TestParseTargetHeaders_ErrorsNeverQuoteAValue(t *testing.T) {
const secret = "QQNEVERINAMESSAGEQQ" const secret = "QQNEVERINAMESSAGEQQ"
_, err := delivery.ParseTargetHeaders( inputs := []string{
// The value, after the colon, in a duplicate name.
"X-A: " + secret + "\nx-a: " + secret, "X-A: " + secret + "\nx-a: " + secret,
) // The value after the colon of an unusable name.
require.Error(t, err)
assert.NotContains(t, err.Error(), secret)
_, err = delivery.ParseTargetHeaders(
"X Bad Name: " + secret, "X Bad Name: " + secret,
) // The line splits on the value's own colon, so the
require.Error(t, err) // secret lands in the text an unusable-name error is
assert.NotContains(t, err.Error(), secret) // tempted to quote as the name.
"X-Api-Key " + secret + ":x",
// The same, with nothing before the secret at all.
secret + " and more:x",
// A control character in the value.
"X-A: " + secret + "\x01",
}
for _, input := range inputs {
_, err := delivery.ParseTargetHeaders(input)
require.Error(t, err, input)
assert.NotContains(t, err.Error(), secret, input)
}
} }
// Loading the edit form twice without saving must not reshuffle // Loading the edit form twice without saving must not reshuffle

View File

@@ -8,6 +8,7 @@ import (
"fmt" "fmt"
"io" "io"
"net/http" "net/http"
"sort"
"sync" "sync"
"time" "time"
@@ -404,9 +405,9 @@ func (t *httpTarget) doHTTPRequest(
) )
} }
applyRequestHeaders(req, event, cfg) originScoped := applyRequestHeaders(req, event, cfg)
client := t.clientForConfig(cfg) client := t.clientForRequest(cfg, originScoped)
resp, doErr := executeHTTPRequest(client, req) resp, doErr := executeHTTPRequest(client, req)
@@ -432,23 +433,41 @@ func (t *httpTarget) doHTTPRequest(
return resp.StatusCode, string(body), dur, nil return resp.StatusCode, string(body), dur, nil
} }
func (t *httpTarget) clientForConfig( // clientForRequest returns the client for one delivery attempt.
// originScoped is the header set applyRequestHeaders built for that
// attempt; a request with neither a per-target timeout nor an
// origin-scoped header gets the shared client, because there is
// then nothing for the redirect policy to strip and net/http's
// default policy already withholds Authorization and Cookie across
// hosts.
func (t *httpTarget) clientForRequest(
cfg *HTTPTargetConfig, cfg *HTTPTargetConfig,
originScoped []string,
) *http.Client { ) *http.Client {
if cfg.Timeout > 0 { if cfg.Timeout <= 0 && len(originScoped) == 0 {
// Reuse the shared client's SSRF-safe transport so return t.client
// a per-target timeout does not drop the
// request-time private-IP guard. Only the timeout
// is overridden.
return &http.Client{
Timeout: time.Duration(
cfg.Timeout,
) * time.Second,
Transport: t.client.Transport,
}
} }
return t.client // Reuse the shared client's SSRF-safe transport so neither a
// per-target timeout nor the redirect policy drops the
// request-time private-IP guard — which, being a dial hook,
// also covers every redirect hop.
client := &http.Client{
Timeout: t.client.Timeout,
Transport: t.client.Transport,
}
if cfg.Timeout > 0 {
client.Timeout = time.Duration(
cfg.Timeout,
) * time.Second
}
if len(originScoped) > 0 {
client.CheckRedirect = offOriginHeaderPolicy(originScoped)
}
return client
} }
func parseHTTPConfig( func parseHTTPConfig(
@@ -490,40 +509,88 @@ func isForwardableHeader(name string) bool {
} }
} }
// applyRequestHeaders builds one outbound delivery's header set and
// returns the canonical names of every header in it that is scoped
// to the configured origin: the inbound event headers this delivery
// forwarded, plus the operator's configured headers. The redirect
// policy strips exactly that set on a hop that leaves the origin,
// so the forward set is decided here and only here — a header added
// to it is covered off-origin without a second edit elsewhere.
func applyRequestHeaders( func applyRequestHeaders(
req *http.Request, req *http.Request,
event *database.Event, event *database.Event,
cfg *HTTPTargetConfig, cfg *HTTPTargetConfig,
) { ) []string {
if event.ContentType != "" { if event.ContentType != "" {
req.Header.Set( req.Header.Set(
"Content-Type", event.ContentType, "Content-Type", event.ContentType,
) )
} }
var originalHeaders map[string][]string originScoped := forwardEventHeaders(req, event)
if event.Headers != "" {
jsonErr := json.Unmarshal(
[]byte(event.Headers),
&originalHeaders,
)
if jsonErr == nil {
for k, vals := range originalHeaders {
if isForwardableHeader(k) {
for _, v := range vals {
req.Header.Add(k, v)
}
}
}
}
}
for k, v := range cfg.Headers { for k, v := range cfg.Headers {
req.Header.Set(k, v) req.Header.Set(k, v)
originScoped[http.CanonicalHeaderKey(k)] = struct{}{}
} }
req.Header.Set("User-Agent", "webhooker/1.0") req.Header.Set("User-Agent", "webhooker/1.0")
// Content-Type describes the body being sent rather than the
// sender, and the delivery path sets it from the event itself.
// A 307/308 preserves the body across hosts, so stripping it
// would send that body untyped.
delete(originScoped, "Content-Type")
// User-Agent is overwritten just above, so an inbound one never
// reaches the wire and the value that does identifies this
// delivery path rather than the sender. Reporting it would strip
// it off-origin and leave net/http's own default in its place.
delete(originScoped, "User-Agent")
names := make([]string, 0, len(originScoped))
for name := range originScoped {
names = append(names, name)
}
sort.Strings(names)
return names
}
// forwardEventHeaders copies the inbound event's forwardable
// headers onto the outbound request and returns the canonical names
// it forwarded. Headers the event never carried are absent from the
// result, so the redirect policy strips what was actually sent.
func forwardEventHeaders(
req *http.Request,
event *database.Event,
) map[string]struct{} {
forwarded := make(map[string]struct{})
if event.Headers == "" {
return forwarded
}
var inbound map[string][]string
if json.Unmarshal([]byte(event.Headers), &inbound) != nil {
return forwarded
}
for k, vals := range inbound {
if !isForwardableHeader(k) || len(vals) == 0 {
continue
}
for _, v := range vals {
req.Header.Add(k, v)
}
forwarded[http.CanonicalHeaderKey(k)] = struct{}{}
}
return forwarded
} }
// executeHTTPRequest sends an HTTP request using the provided // executeHTTPRequest sends an HTTP request using the provided

View File

@@ -185,7 +185,7 @@ func TestDoHTTPRequest_TransportErrorMasksURL(t *testing.T) {
func TestValidateTargetURL_UnparsableURLIsMasked(t *testing.T) { func TestValidateTargetURL_UnparsableURLIsMasked(t *testing.T) {
t.Parallel() t.Parallel()
err := delivery.ValidateTargetURL( err := delivery.NewTestGuard().ValidateTargetURL(
context.TODO(), context.TODO(),
"https://hooks.slack.com"+maskSecretPath+"\n", "https://hooks.slack.com"+maskSecretPath+"\n",
) )

View File

@@ -0,0 +1,378 @@
package handlers
import (
"net/http"
"strconv"
"github.com/go-chi/chi"
"gorm.io/gorm"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// replayOutcomeParam is the query parameter the replay POST redirects
// with and the event log page reads its banner from.
const replayOutcomeParam = "replay"
// replayOutcomeCode is the outcome of a replay POST. The redirect
// carries one of these fixed codes rather than a message, so nothing a
// client submits can reach the rendered page through it.
type replayOutcomeCode string
const (
// replayQueued reports that a new delivery was created and handed
// to the delivery engine.
replayQueued replayOutcomeCode = "queued"
// replayTargetDeleted reports a target that once existed and has
// since been deleted. Deletes are soft and deliveries carry no
// foreign key to the target row, so the history survives its
// target and this is the ordinary case for an old event.
replayTargetDeleted replayOutcomeCode = "target-deleted"
// replayTargetMissing reports a target id that names no row at
// all, deleted or otherwise.
replayTargetMissing replayOutcomeCode = "target-missing"
// replayTargetInactive reports a target the operator has
// deactivated. A deactivated target receives no new deliveries, so
// a replay to it would be a delivery they switched off.
replayTargetInactive replayOutcomeCode = "target-inactive"
// replayNotTerminal reports a delivery the engine has not finished
// with.
replayNotTerminal replayOutcomeCode = "not-terminal"
// replayInFlight reports that an earlier replay of this event to
// this target is still running.
replayInFlight replayOutcomeCode = "in-flight"
)
// replayOutcome returns the banner the event log page shows for an
// outcome code, and whether the replay was queued. An unrecognised
// code yields no banner.
func replayOutcome(code string) (string, bool) {
switch replayOutcomeCode(code) {
case replayQueued:
return "Replay queued: a new delivery was created against " +
"the target's current configuration.", true
case replayTargetDeleted:
return "Not replayed: the target this delivery was for has " +
"been deleted. Recreate the target, then replay.", false
case replayTargetMissing:
return "Not replayed: the target this delivery was for no " +
"longer exists.", false
case replayTargetInactive:
return "Not replayed: the target this delivery was for is " +
"deactivated. Activate it, then replay.", false
case replayNotTerminal:
return "Not replayed: this delivery has not finished yet.",
false
case replayInFlight:
return "Not replayed: a delivery of this event to this " +
"target is already in flight.", false
default:
return "", false
}
}
// HandleDeliveryReplay re-sends a finished delivery's event to its
// target.
//
// A replay never touches the delivery it repeats. It creates a NEW
// pending delivery row for the same event and target and hands it to
// the delivery engine through the same Notifier the receiver uses, so
// the original's status, attempts and timestamps stand as the record
// of what actually happened, and the replay is retried, SSRF-guarded
// and circuit-broken exactly as a first attempt is.
//
// What is re-sent is the stored EVENT body, never the response the
// original delivery received.
//
// The target's configuration is read now rather than as it stood when
// the original ran: a replay exists to deliver where the operator
// currently wants the event to go. That is also why a deleted target
// is refused rather than delivered to from stale configuration.
func (h *Handlers) HandleDeliveryReplay() 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
}
h.replayDelivery(w, r, webhook)
}
}
// replayDelivery performs the replay for a webhook the caller has
// already established the session's user owns.
func (h *Handlers) replayDelivery(
w http.ResponseWriter,
r *http.Request,
webhook database.Webhook,
) {
if !h.dbMgr.DBExists(webhook.ID) {
http.NotFound(w, r)
return
}
webhookDB, err := h.dbMgr.GetDB(webhook.ID)
if err != nil {
h.serverError(w, "failed to get webhook database", err)
return
}
original, ok := h.loadReplaySource(w, r, webhookDB)
if !ok {
return
}
if !original.Status.Terminal() {
h.finishReplay(w, r, webhook, replayNotTerminal)
return
}
target, code := h.replayTarget(webhook.ID, original.TargetID)
if target == nil {
h.finishReplay(w, r, webhook, code)
return
}
h.queueReplay(w, r, webhookDB, webhook, original, target)
}
// loadReplaySource loads the delivery to be replayed, selecting only
// the columns the replay needs so no association is populated. A
// delivery id that names no row in this webhook's database is a 404.
func (h *Handlers) loadReplaySource(
w http.ResponseWriter,
r *http.Request,
webhookDB *gorm.DB,
) (*database.Delivery, bool) {
var original database.Delivery
err := webhookDB.
Select("id", "event_id", "target_id", "status").
First(
&original, "id = ?", chi.URLParam(r, "deliveryID"),
).Error
if err != nil {
http.NotFound(w, r)
return nil, false
}
return &original, true
}
// queueReplay writes the new delivery and hands it to the engine.
func (h *Handlers) queueReplay(
w http.ResponseWriter,
r *http.Request,
webhookDB *gorm.DB,
webhook database.Webhook,
original *database.Delivery,
target *database.Target,
) {
inFlight, err := countInFlightDeliveries(
webhookDB, original.EventID, target.ID,
)
if err != nil {
h.serverError(
w, "failed to count in-flight deliveries", err,
)
return
}
if inFlight > 0 {
h.finishReplay(w, r, webhook, replayInFlight)
return
}
var event database.Event
err = webhookDB.
First(&event, "id = ?", original.EventID).Error
if err != nil {
h.serverError(w, "failed to load event for replay", err)
return
}
task, err := createReplayDelivery(
webhookDB, webhook.ID, &event, target,
)
if err != nil {
h.serverError(
w, "failed to create replay delivery", err,
)
return
}
h.mtr.DeliveryReplayed(target.Type)
h.notifier.Notify([]delivery.Task{task})
h.log.Info(
"delivery replay queued",
"webhook_id", webhook.ID,
"event_id", event.ID,
"target_id", target.ID,
"replayed_delivery_id", original.ID,
"delivery_id", task.DeliveryID,
)
h.finishReplay(w, r, webhook, replayQueued)
}
// replayTarget loads the delivery's target as it stands now.
//
// The load is Unscoped so that a soft-deleted row is still found:
// deletes are soft and a delivery carries no foreign key to its
// target, so a target's history outlives it, and without the deleted
// row there is no way to tell "you deleted this target" from "this id
// never named anything". A nil target means the replay is refused,
// with the returned code saying why.
func (h *Handlers) replayTarget(
webhookID, targetID string,
) (*database.Target, replayOutcomeCode) {
var target database.Target
err := h.db.DB().Unscoped().Where(
"id = ? AND webhook_id = ?", targetID, webhookID,
).First(&target).Error
if err != nil {
return nil, replayTargetMissing
}
if target.DeletedAt.Valid {
return nil, replayTargetDeleted
}
if !target.Active {
return nil, replayTargetInactive
}
return &target, replayQueued
}
// countInFlightDeliveries reports how many deliveries of this event to
// this target the engine has not finished.
//
// It is the replay-storm guard: a replay is refused while an earlier
// one is still pending or retrying, so a held-down button or a scripted
// loop cannot stack copies of work already queued. It is a check and
// not a lock, so two simultaneous POSTs can still both pass it; the
// per-client rate limit on the route is what bounds that.
func countInFlightDeliveries(
webhookDB *gorm.DB, eventID, targetID string,
) (int64, error) {
var count int64
err := webhookDB.Model(&database.Delivery{}).Where(
"event_id = ? AND target_id = ? AND status IN ?",
eventID, targetID,
[]database.DeliveryStatus{
database.DeliveryStatusPending,
database.DeliveryStatusRetrying,
},
).Count(&count).Error
return count, err
}
// createReplayDelivery writes the new pending delivery row and returns
// the task that carries it to the delivery engine.
//
// The row is written with associations omitted, and neither Event nor
// Target is populated on it: GORM's SaveBeforeAssociations would
// otherwise 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.
func createReplayDelivery(
webhookDB *gorm.DB,
webhookID string,
event *database.Event,
target *database.Target,
) (delivery.Task, error) {
dlv := &database.Delivery{
EventID: event.ID,
TargetID: target.ID,
Status: database.DeliveryStatusPending,
}
err := webhookDB.Omit(clause.Associations).Create(dlv).Error
if err != nil {
return delivery.Task{}, err
}
return delivery.Task{
DeliveryID: dlv.ID,
EventID: event.ID,
WebhookID: webhookID,
EntrypointID: event.EntrypointID,
TargetID: target.ID,
TargetName: target.Name,
TargetType: target.Type,
TargetConfig: target.Config,
MaxRetries: target.MaxRetries,
Method: event.Method,
Headers: event.Headers,
ContentType: event.ContentType,
Body: replayBody(event.Body),
AttemptNum: 1,
}, nil
}
// replayBody returns the stored event body for a replay task to carry
// inline, or nil when it is large enough that the engine should fetch
// it from the per-webhook database instead.
func replayBody(body string) *string {
if len(body) >= delivery.MaxInlineBodySize {
return nil
}
return &body
}
// finishReplay redirects back to the event log the replay was
// triggered from, carrying the outcome code the page turns into a
// banner and the page number the form submitted.
func (h *Handlers) finishReplay(
w http.ResponseWriter,
r *http.Request,
webhook database.Webhook,
code replayOutcomeCode,
) {
dest := "/source/" + webhook.ID + "/logs?" +
replayOutcomeParam + "=" + string(code)
// The page is read from the form rather than the query string:
// this is a POST, and its query string is what logs and Referer
// headers record.
if page := parseNonNegativeInt(
r.PostFormValue("page"),
); page > 1 {
dest += "&page=" + strconv.Itoa(page)
}
http.Redirect(w, r, dest, http.StatusSeeOther)
}

View File

@@ -0,0 +1,526 @@
package handlers_test
import (
"net/http"
"net/http/httptest"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session"
)
// paramDeliveryID is the chi URL parameter name the replay handler
// reads.
const paramDeliveryID = "deliveryID"
// replayTargetURL is a public destination, so a target configured with
// it is one the SSRF guard would accept. Nothing in these tests
// dispatches to it: the notifier is recorded, not run.
const replayTargetURL = "http://93.184.216.34/hook"
// seedFailedDelivery records an event, a terminally failed delivery of
// it to the given target, and the attempt that failed.
func seedFailedDelivery(
t *testing.T,
dbMgr *database.WebhookDBManager,
webhookID, targetID string,
) (*database.Event, *database.Delivery) {
t.Helper()
webhookDB, err := dbMgr.GetDB(webhookID)
require.NoError(t, err)
event := &database.Event{
WebhookID: webhookID,
EntrypointID: "entrypoint-" + webhookID,
Method: http.MethodPost,
Headers: `{"X-Test":["yes"]}`,
Body: `{"replay":"me"}`,
ContentType: contentTypeJSON,
}
require.NoError(t, webhookDB.Omit(
clause.Associations,
).Create(event).Error)
dlv := &database.Delivery{
EventID: event.ID,
TargetID: targetID,
Status: database.DeliveryStatusFailed,
}
require.NoError(t, webhookDB.Omit(
clause.Associations,
).Create(dlv).Error)
result := &database.DeliveryResult{
DeliveryID: dlv.ID,
AttemptNum: 1,
Success: false,
StatusCode: http.StatusBadGateway,
Error: "connection refused",
}
require.NoError(t, webhookDB.Omit(
clause.Associations,
).Create(result).Error)
return event, dlv
}
// loadDelivery reads a delivery back out of a webhook's database.
func loadDelivery(
t *testing.T, webhookDB *gorm.DB, deliveryID string,
) database.Delivery {
t.Helper()
var dlv database.Delivery
require.NoError(
t,
webhookDB.First(&dlv, "id = ?", deliveryID).Error,
)
return dlv
}
// listDeliveries reads every delivery of an event.
func listDeliveries(
t *testing.T, webhookDB *gorm.DB, eventID string,
) []database.Delivery {
t.Helper()
var deliveries []database.Delivery
require.NoError(t, webhookDB.Where(
"event_id = ?", eventID,
).Find(&deliveries).Error)
return deliveries
}
// theOtherDelivery returns the one delivery in the slice that is not
// excludeID. Identity is used rather than an ordering because the rows
// are minted milliseconds apart and their ids are random.
func theOtherDelivery(
t *testing.T,
deliveries []database.Delivery,
excludeID string,
) database.Delivery {
t.Helper()
var found []database.Delivery
for _, d := range deliveries {
if d.ID != excludeID {
found = append(found, d)
}
}
require.Len(t, found, 1)
return found[0]
}
// postReplay runs the real replay handler for one delivery.
func postReplay(
t *testing.T,
h *handlers.Handlers,
sess *session.Session,
webhookID, deliveryID string,
) *httptest.ResponseRecorder {
t.Helper()
req := postRequest(
"/source/"+webhookID+"/deliveries/"+
deliveryID+"/replay",
authenticatedCookies(
t, sess, deleteTestUserID, deleteTestUsername,
),
map[string]string{
paramSourceID: webhookID,
paramDeliveryID: deliveryID,
},
)
w := httptest.NewRecorder()
h.HandleDeliveryReplay().ServeHTTP(w, req)
return w
}
// TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal is the
// core requirement: replaying a failed delivery succeeds, appends a
// new delivery, and leaves the original row and its recorded attempt
// exactly as they were.
//
// It also pins the two things a replay would be wrong to get from the
// original: the task carries the target's CURRENT configuration, which
// this test changes between the failure and the replay, and it carries
// the stored EVENT body rather than anything the failed attempt
// received back.
func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
t *testing.T,
) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
)
event, original := seedFailedDelivery(
t, dbMgr, wh.ID, tgt.ID,
)
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
before := loadDelivery(t, webhookDB, original.ID)
// The operator fixes the destination, which is the whole reason
// to replay. The replay must use this, not the config the
// original delivery ran against.
const fixedConfig = `{"url":"http://93.184.216.34/fixed"}`
require.NoError(t, db.DB().Model(&database.Target{}).
Where("id = ?", tgt.ID).
Update("config", fixedConfig).Error)
w := postReplay(t, h, sess, wh.ID, original.ID)
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t,
"/source/"+wh.ID+"/logs?replay=queued",
w.Header().Get("Location"),
)
deliveries := listDeliveries(t, webhookDB, event.ID)
require.Len(
t, deliveries, 2,
"replay must append a delivery, not reuse one",
)
replayed := theOtherDelivery(t, deliveries, original.ID)
assert.Equal(t, tgt.ID, replayed.TargetID)
assert.Equal(t, event.ID, replayed.EventID)
assert.Equal(
t, database.DeliveryStatusPending, replayed.Status,
)
assertDeliveryUntouched(t, webhookDB, before)
tasks := notif.Tasks()
require.Len(t, tasks, 1)
assertReplayTask(
t, tasks[0], wh.ID, event, tgt, replayed.ID, fixedConfig,
)
assertNoLeakedTarget(t, webhookDB)
}
// assertDeliveryUntouched proves a delivery row is exactly as it was
// read before: same terminal status, same timestamps, and the same
// recorded attempts.
func assertDeliveryUntouched(
t *testing.T,
webhookDB *gorm.DB,
before database.Delivery,
) {
t.Helper()
after := loadDelivery(t, webhookDB, before.ID)
assert.Equal(
t, before.Status, after.Status,
"replay must not resurrect the original delivery",
)
assert.Equal(t, before.UpdatedAt, after.UpdatedAt)
assert.Equal(t, before.CreatedAt, after.CreatedAt)
var attempts int64
require.NoError(t, webhookDB.
Model(&database.DeliveryResult{}).
Where("delivery_id = ?", before.ID).
Count(&attempts).Error)
assert.Equal(
t, int64(1), attempts,
"the original delivery's attempt history must stand",
)
}
// assertReplayTask proves the task handed to the delivery engine is
// the one the receiver would build for this event and this target, and
// that it carries wantConfig — the target's configuration as it stands
// now rather than as the original delivery ran against it.
func assertReplayTask(
t *testing.T,
task delivery.Task,
webhookID string,
event *database.Event,
target *database.Target,
wantDeliveryID, wantConfig string,
) {
t.Helper()
assert.Equal(t, wantDeliveryID, task.DeliveryID)
assert.Equal(t, event.ID, task.EventID)
assert.Equal(t, webhookID, task.WebhookID)
assert.Equal(t, event.EntrypointID, task.EntrypointID)
assert.Equal(t, target.ID, task.TargetID)
assert.Equal(t, target.Type, task.TargetType)
assert.JSONEq(
t, wantConfig, task.TargetConfig,
"replay must use the target's current configuration",
)
assert.Equal(t, event.Method, task.Method)
assert.Equal(t, event.Headers, task.Headers)
assert.Equal(t, event.ContentType, task.ContentType)
assert.Equal(t, 1, task.AttemptNum)
require.NotNil(t, task.Body)
assert.Equal(
t, event.Body, *task.Body,
"replay re-sends the stored event body",
)
}
// assertNoLeakedTarget proves the per-webhook database holds no target
// rows. AutoMigrate creates the table there because Delivery declares
// the relation, so it is a ROW that signals a leak: an association
// write would have upserted the whole target, plaintext config and
// all, into the event database. See
// https://git.eeqj.de/sneak/webhooker/issues/206.
func assertNoLeakedTarget(t *testing.T, webhookDB *gorm.DB) {
t.Helper()
var leaked int64
require.NoError(t, webhookDB.Unscoped().
Model(&database.Target{}).Count(&leaked).Error)
assert.Zero(
t, leaked,
"replay must not write the target into the event database",
)
}
// TestHandleDeliveryReplay_RefusesDeletedTarget proves the required
// refusal: a target deleted since the delivery ran is reported as
// deleted rather than erroring, and nothing is created or queued.
func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
)
event, original := seedFailedDelivery(
t, dbMgr, wh.ID, tgt.ID,
)
// Deletes are soft, so the delivery history outlives the target.
require.NoError(t, db.DB().Where(
"id = ?", tgt.ID,
).Delete(&database.Target{}).Error)
w := postReplay(t, h, sess, wh.ID, original.ID)
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t,
"/source/"+wh.ID+"/logs?replay=target-deleted",
w.Header().Get("Location"),
)
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
assert.Len(
t, listDeliveries(t, webhookDB, event.ID), 1,
"a refused replay must create no delivery",
)
assert.Empty(
t, notif.Tasks(),
"a refused replay must queue nothing",
)
// The refusal is specific, which is why the target is looked up
// including soft-deleted rows: an id that never named a target
// is a different outcome, and a different message, from one the
// operator deleted.
_, orphan := seedFailedDelivery(
t, dbMgr, wh.ID, "target-that-never-existed",
)
missing := postReplay(t, h, sess, wh.ID, orphan.ID)
require.Equal(t, http.StatusSeeOther, missing.Code)
assert.Equal(
t,
"/source/"+wh.ID+"/logs?replay=target-missing",
missing.Header().Get("Location"),
)
}
// TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight proves
// the replay-storm guard: a second replay of the same event to the
// same target is refused while the first is still queued, so repeated
// submissions cannot stack copies of work the engine has not done.
func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
t *testing.T,
) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
notif *recordingNotifier
)
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
)
event, original := seedFailedDelivery(
t, dbMgr, wh.ID, tgt.ID,
)
first := postReplay(t, h, sess, wh.ID, original.ID)
require.Equal(t, http.StatusSeeOther, first.Code)
require.Equal(
t,
"/source/"+wh.ID+"/logs?replay=queued",
first.Header().Get("Location"),
)
second := postReplay(t, h, sess, wh.ID, original.ID)
require.Equal(t, http.StatusSeeOther, second.Code)
assert.Equal(
t,
"/source/"+wh.ID+"/logs?replay=in-flight",
second.Header().Get("Location"),
)
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
assert.Len(
t, listDeliveries(t, webhookDB, event.ID), 2,
"the refused second replay must add nothing",
)
assert.Len(
t, notif.Tasks(), 1,
"only the first replay reaches the delivery engine",
)
// A delivery the engine has not finished is not replayable
// either, which is the same rule seen from the other side.
queued := theOtherDelivery(
t, listDeliveries(t, webhookDB, event.ID), original.ID,
)
pending := postReplay(t, h, sess, wh.ID, queued.ID)
require.Equal(t, http.StatusSeeOther, pending.Code)
assert.Equal(
t,
"/source/"+wh.ID+"/logs?replay=not-terminal",
pending.Header().Get("Location"),
)
}
// TestHandleSourceLogs_RendersReplayControlAndBanner proves the action
// reaches the page it belongs on: a finished delivery renders a POST
// form carrying a CSRF token, and the outcome code a refusal redirects
// with becomes a readable message.
func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
`{"url":"`+replayTargetURL+`"}`,
)
_, original := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID)
body := renderSourceLogsPage(t, h, sess, wh.ID)
assert.Contains(
t, body,
`action="/source/`+wh.ID+`/deliveries/`+
original.ID+`/replay"`,
)
assert.Contains(t, body, `method="POST"`)
assert.Contains(t, body, `name="csrf_token"`)
assert.Contains(t, body, ">Replay<")
refused := renderSourceLogsPageWithQuery(
t, h, sess, wh.ID, "?replay=target-deleted",
)
assert.Contains(t, refused, "alert-error")
assert.Contains(t, refused, "has been deleted")
// An outcome code nobody issued renders no banner at all.
unknown := renderSourceLogsPageWithQuery(
t, h, sess, wh.ID, "?replay=made-up",
)
assert.NotContains(t, unknown, "alert-error")
assert.NotContains(t, unknown, "alert-success")
assert.NotContains(t, unknown, "made-up")
}

View File

@@ -30,10 +30,14 @@ const (
attemptError = "upstream returned 502 Bad Gateway" attemptError = "upstream returned 502 Bad Gateway"
) )
// seedFailedDelivery records an event, a failed delivery // seedFailedDeliveryWithResponse records an event, a failed
// against targetID, and one delivery result carrying the // delivery against targetID, and one delivery result carrying
// given response body. It returns the delivery. // the given response body. It returns the delivery.
func seedFailedDelivery( //
// Distinct from seedFailedDelivery in delivery_replay_test.go,
// which seeds an attempt with no response body and returns the
// event as well; these tests need the recorded response.
func seedFailedDeliveryWithResponse(
t *testing.T, t *testing.T,
dbMgr *database.WebhookDBManager, dbMgr *database.WebhookDBManager,
webhookID, targetID, responseBody string, webhookID, targetID, responseBody string,
@@ -47,7 +51,7 @@ func seedFailedDelivery(
WebhookID: webhookID, WebhookID: webhookID,
Method: http.MethodPost, Method: http.MethodPost,
Body: `{"test":true}`, Body: `{"test":true}`,
ContentType: "application/json", ContentType: contentTypeJSON,
} }
require.NoError(t, webhookDB.Omit( require.NoError(t, webhookDB.Omit(
@@ -108,7 +112,9 @@ func seedFailureAndRender(
t, db, wh.ID, targetType, config, t, db, wh.ID, targetType, config,
) )
seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID, responseBody) seedFailedDeliveryWithResponse(
t, dbMgr, wh.ID, tgt.ID, responseBody,
)
return renderSourceLogsPage(t, h, sess, wh.ID) return renderSourceLogsPage(t, h, sess, wh.ID)
} }
@@ -226,7 +232,7 @@ func TestHandleSourceLogs_RedactsCredentialEchoedInError(
`{"webhookUrl":"`+slackWebhookURL+`"}`, `{"webhookUrl":"`+slackWebhookURL+`"}`,
) )
dlv := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID, "") dlv := seedFailedDeliveryWithResponse(t, dbMgr, wh.ID, tgt.ID, "")
webhookDB, err := dbMgr.GetDB(wh.ID) webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err) require.NoError(t, err)
@@ -371,7 +377,7 @@ func TestHandleSourceLogs_RedactsForSoftDeletedTarget(
`{"webhookUrl":"`+slackWebhookURL+`"}`, `{"webhookUrl":"`+slackWebhookURL+`"}`,
) )
seedFailedDelivery( seedFailedDeliveryWithResponse(
t, dbMgr, wh.ID, tgt.ID, t, dbMgr, wh.ID, tgt.ID,
"no_service: "+slackWebhookURL, "no_service: "+slackWebhookURL,
) )
@@ -411,14 +417,14 @@ func TestHandleSourceLogs_BoundsRenderedAttempts(t *testing.T) {
t, db, wh.ID, database.TargetTypeLog, "", t, db, wh.ID, database.TargetTypeLog, "",
) )
dlv := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID, "") dlv := seedFailedDeliveryWithResponse(t, dbMgr, wh.ID, tgt.ID, "")
webhookDB, err := dbMgr.GetDB(wh.ID) webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err) require.NoError(t, err)
total := handlers.MaxRenderedAttemptsForTest + extraAttempts total := handlers.MaxRenderedAttemptsForTest + extraAttempts
// seedFailedDelivery already recorded one attempt. // seedFailedDeliveryWithResponse already recorded one attempt.
for i := range total - 1 { for i := range total - 1 {
require.NoError(t, webhookDB.Omit( require.NoError(t, webhookDB.Omit(
clause.Associations, clause.Associations,
@@ -481,7 +487,7 @@ func TestHandleSourceLogs_BoundsOversizeResponse(t *testing.T) {
) )
stored := strings.Repeat("A", responseCap*4) + tail stored := strings.Repeat("A", responseCap*4) + tail
seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID, stored) seedFailedDeliveryWithResponse(t, dbMgr, wh.ID, tgt.ID, stored)
views := h.LoadEventLogViewsForTest( views := h.LoadEventLogViewsForTest(
httptest.NewRecorder(), *wh, 1, httptest.NewRecorder(), *wh, 1,

View File

@@ -60,6 +60,7 @@ type HandlersParams struct {
Middleware *middleware.Middleware Middleware *middleware.Middleware
Notifier delivery.Notifier Notifier delivery.Notifier
Evictor delivery.WebhookEvictor Evictor delivery.WebhookEvictor
SSRFGuard *delivery.Guard
} }
// Handlers provides HTTP handler methods for all application // Handlers provides HTTP handler methods for all application
@@ -77,6 +78,11 @@ type Handlers struct {
mtr *metrics.Set mtr *metrics.Set
templates map[string]*template.Template templates map[string]*template.Template
// ssrf validates submitted target URLs. It is the same guard
// the delivery engine dials through, so a URL accepted here
// is one delivery will actually attempt.
ssrf *delivery.Guard
// dummyVerifications counts the equivalent-cost verifications // dummyVerifications counts the equivalent-cost verifications
// charged for usernames that do not exist. It exists so a test // charged for usernames that do not exist. It exists so a test
// can prove that path runs without measuring wall-clock time. // can prove that path runs without measuring wall-clock time.
@@ -117,6 +123,7 @@ func New(
s.notifier = params.Notifier s.notifier = params.Notifier
s.evictor = params.Evictor s.evictor = params.Evictor
s.mtr = metrics.Default() s.mtr = metrics.Default()
s.ssrf = params.SSRFGuard
// 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{

View File

@@ -24,9 +24,32 @@ import (
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
) )
type noopNotifier struct{} // recordingNotifier is a delivery.Notifier that records the tasks it
// was handed, so a test can prove a handler queued the delivery it
// claims to have queued — and, on the refusal paths, that it queued
// nothing.
type recordingNotifier struct {
mu sync.Mutex
tasks []delivery.Task
}
func (n *noopNotifier) Notify([]delivery.Task) {} func (n *recordingNotifier) Notify(tasks []delivery.Task) {
n.mu.Lock()
defer n.mu.Unlock()
n.tasks = append(n.tasks, tasks...)
}
// Tasks returns a copy of the recorded tasks.
func (n *recordingNotifier) Tasks() []delivery.Task {
n.mu.Lock()
defer n.mu.Unlock()
out := make([]delivery.Task, len(n.tasks))
copy(out, n.tasks)
return out
}
// recordingEvictor is a delivery.WebhookEvictor that records // recordingEvictor is a delivery.WebhookEvictor that records
// the webhook ids it was asked to evict, so a test can prove // the webhook ids it was asked to evict, so a test can prove
@@ -74,8 +97,11 @@ func newTestApp(
database.NewWebhookDBManager, database.NewWebhookDBManager,
healthcheck.New, healthcheck.New,
session.New, session.New,
func() delivery.Notifier { func() *recordingNotifier {
return &noopNotifier{} return &recordingNotifier{}
},
func(n *recordingNotifier) delivery.Notifier {
return n
}, },
func() *recordingEvictor { func() *recordingEvictor {
return &recordingEvictor{} return &recordingEvictor{}
@@ -84,6 +110,7 @@ func newTestApp(
return r return r
}, },
middleware.New, middleware.New,
delivery.NewGuard,
handlers.New, handlers.New,
), ),
fx.Populate(targets...), fx.Populate(targets...),

View File

@@ -15,6 +15,11 @@ import (
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
) )
// contentTypeJSON is the content type the seeded events in this
// package carry. Shared across the seed helpers so the literal
// appears once.
const contentTypeJSON = "application/json"
// seedDeliveredEvent records an event and a delivery for it in // seedDeliveredEvent records an event and a delivery for it in
// the webhook's own database, so the log page has a delivery // the webhook's own database, so the log page has a delivery
// to render against the target. // to render against the target.
@@ -32,7 +37,7 @@ func seedDeliveredEvent(
WebhookID: webhookID, WebhookID: webhookID,
Method: http.MethodPost, Method: http.MethodPost,
Body: `{"test":true}`, Body: `{"test":true}`,
ContentType: "application/json", ContentType: contentTypeJSON,
} }
require.NoError(t, webhookDB.Omit( require.NoError(t, webhookDB.Omit(
@@ -60,10 +65,26 @@ func renderSourceLogsPage(
) string { ) string {
t.Helper() t.Helper()
return renderSourceLogsPageWithQuery(
t, h, sess, webhookID, "",
)
}
// renderSourceLogsPageWithQuery is renderSourceLogsPage over a
// caller-supplied query string, for the page state a redirect back to
// the log carries in one.
func renderSourceLogsPageWithQuery(
t *testing.T,
h *handlers.Handlers,
sess *session.Session,
webhookID, query string,
) string {
t.Helper()
req := httptest.NewRequestWithContext( req := httptest.NewRequestWithContext(
context.Background(), context.Background(),
http.MethodGet, http.MethodGet,
"/source/"+webhookID+"/logs", "/source/"+webhookID+"/logs"+query,
nil, nil,
) )

View File

@@ -821,16 +821,25 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
totalPages++ totalPages++
} }
// The banner a replay POST redirected back with. The
// message comes from a fixed set keyed by the outcome
// code, never from the query string itself.
replayMsg, replayOK := replayOutcome(
r.URL.Query().Get(replayOutcomeParam),
)
data := map[string]any{ data := map[string]any{
tmplKeyWebhook: &webhook, tmplKeyWebhook: &webhook,
"Events": evts, "Events": evts,
"Page": page, "ReplayMessage": replayMsg,
"TotalPages": totalPages, "ReplayQueued": replayOK,
"TotalEvents": total, "Page": page,
"HasPrev": page > 1, "TotalPages": totalPages,
"HasNext": page < totalPages, "TotalEvents": total,
"PrevPage": page - 1, "HasPrev": page > 1,
"NextPage": page + 1, "HasNext": page < totalPages,
"PrevPage": page - 1,
"NextPage": page + 1,
} }
h.renderTemplate(w, r, "source_logs.html", data) h.renderTemplate(w, r, "source_logs.html", data)
@@ -1598,7 +1607,7 @@ func (h *Handlers) validateTargetURL(
return errMissingURL return errMissingURL
} }
err := delivery.ValidateTargetURL( err := h.ssrf.ValidateTargetURL(
r.Context(), targetURL, r.Context(), targetURL,
) )
if err != nil { if err != nil {

View File

@@ -82,6 +82,7 @@ type Set struct {
deliveriesSucceeded *prometheus.CounterVec deliveriesSucceeded *prometheus.CounterVec
deliveriesFailed *prometheus.CounterVec deliveriesFailed *prometheus.CounterVec
deliveryRetries *prometheus.CounterVec deliveryRetries *prometheus.CounterVec
deliveryReplays *prometheus.CounterVec
deliveryDuration *prometheus.HistogramVec deliveryDuration *prometheus.HistogramVec
deliveriesPending *prometheus.GaugeVec deliveriesPending *prometheus.GaugeVec
deliveriesRetrying *prometheus.GaugeVec deliveriesRetrying *prometheus.GaugeVec
@@ -149,6 +150,22 @@ func (s *Set) ObserveDeliveryDuration(
Observe(d.Seconds()) Observe(d.Seconds())
} }
// DeliveryReplayed counts one delivery an operator replayed from the
// event log.
//
// A replay runs the ordinary engine path, so it already moves the
// attempt, outcome and duration series exactly as a first delivery
// does — deliberately, since a replay is a real delivery and hiding it
// from those would misreport the pipeline. This counter is the one
// place the two are distinguishable, and it carries the existing
// target-type label rather than adding a replay dimension to every
// other series.
func (s *Set) DeliveryReplayed(t database.TargetType) {
s.deliveryReplays.
WithLabelValues(normalizeTargetType(t)).
Inc()
}
// DeliveryStatusChanged counts a delivery's transition into a new // DeliveryStatusChanged counts a delivery's transition into a new
// status. The mapping from status to counter lives here, next to the // status. The mapping from status to counter lives here, next to the
// collectors, so the engine has a single call for every transition it // collectors, so the engine has a single call for every transition it
@@ -271,6 +288,16 @@ func (s *Set) registerCounters(factory promauto.Factory) {
}, },
[]string{targetTypeLabel}, []string{targetTypeLabel},
) )
s.deliveryReplays = factory.NewCounterVec(
prometheus.CounterOpts{
Namespace: namespace,
Name: "delivery_replays_total",
Help: "Deliveries an operator replayed from the " +
"event log, by target type.",
},
[]string{targetTypeLabel},
)
} }
func (s *Set) registerGauges(factory promauto.Factory) { func (s *Set) registerGauges(factory promauto.Factory) {
@@ -322,6 +349,7 @@ func (s *Set) initSeries() {
s.deliveriesSucceeded.WithLabelValues(label) s.deliveriesSucceeded.WithLabelValues(label)
s.deliveriesFailed.WithLabelValues(label) s.deliveriesFailed.WithLabelValues(label)
s.deliveryRetries.WithLabelValues(label) s.deliveryRetries.WithLabelValues(label)
s.deliveryReplays.WithLabelValues(label)
s.deliveriesPending.WithLabelValues(label) s.deliveriesPending.WithLabelValues(label)
s.deliveriesRetrying.WithLabelValues(label) s.deliveriesRetrying.WithLabelValues(label)
s.circuitBreakersOpen.WithLabelValues(label) s.circuitBreakersOpen.WithLabelValues(label)

View File

@@ -34,6 +34,16 @@ const (
// password change rate limit. // password change rate limit.
passwordChangeRateInterval = 1 * time.Minute passwordChangeRateInterval = 1 * time.Minute
// replayRateLimit is the maximum number of delivery replays one
// client may queue per interval. Each replay puts a delivery on
// the engine's queue, so without a ceiling one operator holding
// the button down — or scripting it — queues unbounded outbound
// work. It sits far above any rate a person clicks at.
replayRateLimit = 30
// replayRateInterval is the time window for the replay limit.
replayRateInterval = 1 * time.Minute
// receiverRateInterval is the time window for the webhook // receiverRateInterval is the time window for the webhook
// receiver rate limit. The configured limit is expressed in // receiver rate limit. The configured limit is expressed in
// requests per minute. // requests per minute.
@@ -290,6 +300,21 @@ func (m *Middleware) PasswordChangeRateLimit() func(http.Handler) http.Handler {
) )
} }
// ReplayRateLimit returns middleware that enforces per-IP rate
// limiting on delivery replays.
//
// Like the password-change limit it is spent on arrival, which is safe
// for the same reason: RequireAuth runs ahead of it, so only a request
// already carrying a valid session can reach the bucket.
func (m *Middleware) ReplayRateLimit() func(http.Handler) http.Handler {
return m.postRateLimit(
replayRateLimit,
replayRateInterval,
"delivery replay rate limit exceeded",
"Too many replays. Please try again later.",
)
}
// postRateLimit builds middleware that enforces a per-IP rate // postRateLimit builds middleware that enforces a per-IP rate
// limit on POST requests only; all other methods pass through // limit on POST requests only; all other methods pass through
// unaffected. Requests over the limit receive a 429 with the // unaffected. Requests over the limit receive a 429 with the

View File

@@ -164,6 +164,7 @@ func newServerApp(
func() delivery.Notifier { return &noopNotifier{} }, func() delivery.Notifier { return &noopNotifier{} },
func() delivery.WebhookEvictor { return &noopEvictor{} }, func() delivery.WebhookEvictor { return &noopEvictor{} },
middleware.New, middleware.New,
delivery.NewGuard,
handlers.New, handlers.New,
), ),
fx.Populate(&h), fx.Populate(&h),

View File

@@ -201,6 +201,18 @@ func (s *Server) setupSourceRoutes() {
"/logs/{eventID}/body", "/logs/{eventID}/body",
s.h.HandleEventBodyDownload(), s.h.HandleEventBodyDownload(),
) )
// Replay is the one page action that queues outbound work:
// it creates a delivery from a stored event and hands it to
// the delivery engine. The rate limit is what bounds a
// held-down button or a scripted loop; the handler
// separately refuses a replay while an earlier one for the
// same event and target is still in flight. POST only, so
// the action cannot be taken by a link, a prefetch or an
// image tag.
r.With(s.mw.ReplayRateLimit()).Post(
"/deliveries/{deliveryID}/replay",
s.h.HandleDeliveryReplay(),
)
r.Post( r.Post(
"/entrypoints", "/entrypoints",
s.h.HandleEntrypointCreate(), s.h.HandleEntrypointCreate(),

View File

@@ -113,6 +113,7 @@ func newTestEnvWithConfig(
func() delivery.Notifier { return &noopNotifier{} }, func() delivery.Notifier { return &noopNotifier{} },
func() delivery.WebhookEvictor { return &noopEvictor{} }, func() delivery.WebhookEvictor { return &noopEvictor{} },
middleware.New, middleware.New,
delivery.NewGuard,
handlers.New, handlers.New,
), ),
fx.Populate(&log, &mw, &hnd, &sess, &db, &dbMgr), fx.Populate(&log, &mw, &hnd, &sess, &db, &dbMgr),
@@ -310,6 +311,75 @@ func (e *testEnv) seedEvent(
return event return event
} }
// seedTarget creates an active HTTP target for a webhook.
func (e *testEnv) seedTarget(
t *testing.T,
webhookID string,
) *database.Target {
t.Helper()
tgt := &database.Target{
WebhookID: webhookID,
Name: "routed-target",
Type: database.TargetTypeHTTP,
Active: true,
Config: `{"url":"http://93.184.216.34/hook"}`,
}
require.NoError(
t,
e.db.DB().Omit(clause.Associations).Create(tgt).Error,
)
return tgt
}
// seedFailedDelivery records a terminally failed delivery of an event
// to a target in the webhook's own database.
func (e *testEnv) seedFailedDelivery(
t *testing.T,
webhookID, eventID, targetID string,
) *database.Delivery {
t.Helper()
webhookDB, err := e.dbMgr.GetDB(webhookID)
require.NoError(t, err)
dlv := &database.Delivery{
EventID: eventID,
TargetID: targetID,
Status: database.DeliveryStatusFailed,
}
require.NoError(
t,
webhookDB.Omit(clause.Associations).Create(dlv).Error,
)
return dlv
}
// countDeliveries reports how many deliveries a webhook's database
// holds.
func (e *testEnv) countDeliveries(
t *testing.T, webhookID string,
) int64 {
t.Helper()
webhookDB, err := e.dbMgr.GetDB(webhookID)
require.NoError(t, err)
var count int64
require.NoError(
t,
webhookDB.Model(&database.Delivery{}).
Count(&count).Error,
)
return count
}
// storedHash reads the current password hash for a username. // storedHash reads the current password hash for a username.
func (e *testEnv) storedHash(t *testing.T, username string) string { func (e *testEnv) storedHash(t *testing.T, username string) string {
t.Helper() t.Helper()
@@ -674,6 +744,85 @@ func TestSourceLogsBody_OtherUser404s(t *testing.T) {
assert.Equal(t, "/pages/login", anon.Header().Get("Location")) assert.Equal(t, "/pages/login", anon.Header().Get("Location"))
} }
// TestDeliveryReplay_PostOnlyAndCSRFProtected walks the replay action
// through the production router rather than a forged route context,
// which is the only way to prove what the route group actually gives
// it: a GET cannot trigger a replay, an unauthenticated request never
// reaches the handler, a POST without the token is refused by CSRF,
// and the form the template emits — token and action URL both — works
// as rendered.
func TestDeliveryReplay_PostOnlyAndCSRFProtected(t *testing.T) {
t.Parallel()
env := newTestEnv(t)
userID, _ := env.seedUser(t, "replayer", "somepassword")
cookies := env.authCookies(t, userID, "replayer")
wh := env.seedWebhook(t, userID)
tgt := env.seedTarget(t, wh.ID)
evt := env.seedEvent(t, wh.ID, `{"replay":"me"}`)
dlv := env.seedFailedDelivery(t, wh.ID, evt.ID, tgt.ID)
path := "/source/" + wh.ID + "/deliveries/" + dlv.ID +
"/replay"
assert.Equal(
t, http.StatusMethodNotAllowed,
env.get(path, cookies).Code,
"a replay must not be reachable by GET",
)
assert.Equal(
t, http.StatusForbidden,
env.post(path, url.Values{}, cookies).Code,
"a replay POST without a CSRF token must be refused",
)
anon := env.post(path, url.Values{}, nil)
assert.Equal(t, http.StatusForbidden, anon.Code)
require.Equal(
t, int64(1), env.countDeliveries(t, wh.ID),
"no refused request may have created a delivery",
)
// The token and the action URL both come out of the rendered
// page, so a typo in either the route pattern or the template
// fails here.
logsPath := "/source/" + wh.ID + "/logs"
token, cookies := env.csrfFrom(t, logsPath, cookies)
page := env.get(logsPath, cookies)
require.Equal(t, http.StatusOK, page.Code)
action := regexp.MustCompile(
`action="(/source/[^"]+/replay)"`,
).FindStringSubmatch(page.Body.String())
require.Len(
t, action, 2,
"a finished delivery should render a replay form",
)
form := url.Values{}
form.Set("csrf_token", token)
w := env.post(
html.UnescapeString(action[1]), form, cookies,
)
require.Equal(t, http.StatusSeeOther, w.Code)
assert.Equal(
t, logsPath+"?replay=queued",
w.Header().Get("Location"),
)
assert.Equal(
t, int64(2), env.countDeliveries(t, wh.ID),
"the replay appends a delivery",
)
}
// metricsConfig is a Config differing from the routing default only // metricsConfig is a Config differing from the routing default only
// in the two /metrics credentials. // in the two /metrics credentials.
func metricsConfig( func metricsConfig(

View File

@@ -12,6 +12,10 @@
</div> </div>
</div> </div>
{{if .ReplayMessage}}
<div class="{{if .ReplayQueued}}alert-success{{else}}alert-error{{end}}">{{.ReplayMessage}}</div>
{{end}}
<div class="card"> <div class="card">
<div class="divide-y divide-gray-100"> <div class="divide-y divide-gray-100">
{{range .Events}} {{range .Events}}
@@ -52,7 +56,14 @@
<span class="text-sm text-gray-700">{{.Target.Name}}</span> <span class="text-sm text-gray-700">{{.Target.Name}}</span>
<span class="text-xs {{if eq .Status "delivered"}}text-green-600{{else if eq .Status "failed"}}text-red-600{{else if eq .Status "retrying"}}text-yellow-600{{else}}text-gray-400{{end}}">{{.Status}}</span> <span class="text-xs {{if eq .Status "delivered"}}text-green-600{{else if eq .Status "failed"}}text-red-600{{else if eq .Status "retrying"}}text-yellow-600{{else}}text-gray-400{{end}}">{{.Status}}</span>
</div> </div>
<div class="flex items-center gap-2"> <div class="flex items-center gap-3">
{{if .Status.Terminal}}
<form method="POST" action="/source/{{$.Webhook.ID}}/deliveries/{{.ID}}/replay" class="inline" @click.stop>
<input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">
<input type="hidden" name="page" value="{{$.Page}}">
<button type="submit" class="text-xs text-primary-600 hover:text-primary-700" title="Send this event to the target again">Replay</button>
</form>
{{end}}
<span class="text-xs text-gray-400">{{.AttemptCount}} attempt{{if ne .AttemptCount 1}}s{{end}}</span> <span class="text-xs text-gray-400">{{.AttemptCount}} attempt{{if ne .AttemptCount 1}}s{{end}}</span>
<svg class="w-3 h-3 text-gray-400 transition-transform" :class="{ 'rotate-180': attempts }" fill="none" stroke="currentColor" viewBox="0 0 24 24"> <svg class="w-3 h-3 text-gray-400 transition-transform" :class="{ 'rotate-180': attempts }" fill="none" stroke="currentColor" viewBox="0 0 24 24">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M19 9l-7 7-7-7"/> <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M19 9l-7 7-7-7"/>

View File

@@ -39,7 +39,7 @@
<div class="form-group"> <div class="form-group">
<label for="headers" class="label">Headers</label> <label for="headers" class="label">Headers</label>
<textarea id="headers" name="headers" rows="4" class="input" placeholder="Authorization: Bearer ...">{{.Target.Config.Headers}}</textarea> <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> <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>, <code>Trailer</code> and <code>User-Agent</code> are set by the delivery engine and are rejected here rather than silently ignored. Headers set here are dropped if a redirect leaves the destination's own origin, so a credential cannot follow one to another host.</p>
</div> </div>
<div class="form-group"> <div class="form-group">