1 Commits

Author SHA1 Message Date
clawbot
ea6639ff31 Add per-delivery replay to the event log (closes #203)
All checks were successful
check / check (push) Successful in 3m3s
A delivery that exhausted max_retries was failed forever. The event
body is durably stored, so the only way to get it delivered was to
download it and re-POST by hand.

The event log now offers a Replay action on any finished delivery.
Replay creates a NEW pending delivery for the same event and target
and hands it to the delivery engine through the same Notifier the
receiver uses, so it is retried, SSRF-guarded and circuit-broken
exactly as a first attempt. The original delivery's status,
timestamps and recorded attempts are never touched, and what is
re-sent is the stored event body, not the response the original
attempt received.

The target is read as it stands now, including soft-deleted rows so
that a deleted target refuses the replay with a message on the page
instead of erroring or delivering from stale configuration. A
deactivated target and a target id that names nothing refuse the same
way, as does a replay of a delivery the engine has not finished.

Two bounds on replay storms: the route carries a per-client POST rate
limit of 30 per minute, and the handler refuses a replay while an
earlier one for the same event and target is still pending or
retrying.

One new metric, webhooker_delivery_replays_total, on the existing
target_type label. A replay is a real delivery and moves the attempt,
outcome and duration series like any other; this counter is what
separates it from ordinary traffic without adding a dimension to
every existing series.

The delivery row is written with associations omitted and with
neither Event nor Target populated, so no target row reaches the
per-webhook event database.
2026-08-20 06:03:35 +00:00
34 changed files with 234 additions and 3667 deletions

172
README.md
View File

@@ -114,116 +114,6 @@ TTY detection, and security headers are always applied.
| `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` |
| `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
@@ -382,9 +272,8 @@ additionally be a number in the range 165535,
`RECEIVER_RATE_LIMIT` must be at least 1,
`RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker
period, so `0s` or a negative value would crash the reaper after
startup), and every entry in `TRUSTED_PROXIES` and
`ALLOWED_EGRESS_CIDRS` must be a CIDR block or a bare IP address.
`SESSION_IDLE_TIMEOUT` is the exception: a
startup), and every entry in `TRUSTED_PROXIES` 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.
Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
@@ -1160,53 +1049,6 @@ events should be forwarded.
The `config` field stores type-specific configuration as JSON (e.g.,
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
A programmatic access credential for API authentication.
@@ -2490,15 +2332,7 @@ check, see [The login endpoint](#the-login-endpoint).
ranges (RFC 1918, loopback, link-local, cloud metadata) are blocked
both at target creation time (URL validation) and at delivery time
(custom HTTP transport with SSRF-safe dialer that validates resolved
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
IPs before connecting, preventing DNS rebinding attacks)
- **Login limiting is inverted, deliberately.** The login `POST` has
no pre-emptive rate limiter in front of it. Credentials are
verified first and only a _failed_ attempt spends budget, so a

113
TODO.md
View File

@@ -18,52 +18,68 @@ Issue branches do NOT touch this file — the manager maintains it on
# Status
1.0.0 is complete: 55 closed, 0 open. `next` (6874059) is 62 commits
ahead of `main` and a strict fast-forward. No git tags exist yet.
pre-1.0. No git tags exist. `main` (4f5ecb1) is a working webhook proxy
with auth, CSRF/SSRF protections, login rate limiting, Slack target,
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).
The bar was not "the milestone is empty" but "sneak can deploy this and
use it in low-volume production". Every gap the deployability audit
named against that bar is now closed:
`next` is green — verified both by CI and by cache-defeated container
runs (`docker build --no-cache-filter=lint --no-cache-filter=builder`) —
but the **1.0.0 milestone is no longer complete**. It was reopened on
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).
- `DATA_DIR` locking, so two instances cannot both deliver
(https://git.eeqj.de/sneak/webhooker/issues/201)
- shutdown on listener failure, rather than a live non-serving process
(https://git.eeqj.de/sneak/webhooker/issues/200)
- inbound signature verification
(https://git.eeqj.de/sneak/webhooker/issues/67)
- per-attempt delivery detail in the event log
(https://git.eeqj.de/sneak/webhooker/issues/202)
- replay of a terminally failed delivery
(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)
The bar for 1.0 is not "the milestone is empty" but "sneak can deploy
this and use it in low-volume production". The audit found the gap
between those two: two instances on one `DATA_DIR` both deliver
(reproduced), a failed listen leaves a live non-serving process that
restart policies never fire on, there is no inbound authentication of
any kind, delivery failures render as a bare word with no status code or
error, a terminally failed delivery can never be replayed, the SSRF
blocklist has no escape hatch so the proxy cannot forward to your own
network at all, and target credentials leak into the per-webhook event
databases.
One caveat on reading a green check: a docs-only commit deliberately
replays from the layer cache
(https://git.eeqj.de/sneak/webhooker/issues/119), so a green status on
such a commit evidences a replay rather than an executed run. A code
commit invalidates the `COPY` layer and genuinely executes.
One caveat on reading a green check, narrower than it used to be. A
docs-only commit deliberately replays from the layer cache (#119), so a
green status on such a commit evidences a replay rather than an executed
run; a code commit invalidates the `COPY` layer and genuinely executes.
Superseded runs are no longer the hazard they were: before #152 they
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
Merge the milestone PR (https://git.eeqj.de/sneak/webhooker/pulls/111)
and tag `v1.0.0`. It is `merge-ready` and assigned to sneak; nothing
else gates it.
Clear the reopened 1.0.0 milestone. The milestone PR
(https://git.eeqj.de/sneak/webhooker/pulls/111) is held: it carries a
`WIP: ` prefix, no labels and is assigned to `clawbot`, and it stays
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.
Post-1.0 follow-ups are open, none blocking the tag:
https://git.eeqj.de/sneak/webhooker/issues/245,
https://git.eeqj.de/sneak/webhooker/issues/246,
https://git.eeqj.de/sneak/webhooker/issues/247 and
https://git.eeqj.de/sneak/webhooker/issues/248. Also still open and
unmilestoned: https://git.eeqj.de/sneak/webhooker/issues/193 (a design
question, not a defect), https://git.eeqj.de/sneak/webhooker/issues/198
(`make test` is past the org 20s target) and
https://git.eeqj.de/sneak/webhooker/issues/212 (encrypting target config
at rest).
Three items belong to the owner, none of them blocking. #150 was decided
by the manager rather than left to stall the queue and is flagged on the
issue for reversal if that call was wrong. #112 (whether `Completed
Steps` should exist at all, given it once conflicted on every unit) is
unanswered; the provisional ruling in force is that issue branches do
not touch this file. #198 records that `make test` is past the org 20s
target — 46s of test execution inside a 62.8s CI layer — and turns on
which quantity the 60s hard cap governs; it is scoped as the improvement
bug the 20-60s band requires, and should be milestoned instead if the
cap is read as covering the whole invocation.
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
@@ -292,16 +308,14 @@ at rest).
# Future Steps
- Delivery status and retry management UI. Replay of a terminally
failed delivery and per-attempt detail already landed
(https://git.eeqj.de/sneak/webhooker/issues/203,
https://git.eeqj.de/sneak/webhooker/issues/202)
- Manual event redelivery from the web UI — the "Replay" capability the
README describes as planned. No redelivery code exists anywhere in the
tree; events are stored in full, which is all it would be built on
- Delivery status and retry management UI
- Per-webhook rate limiting in the receiver handler (per-webhook config
plus handler enforcement; global limits must not apply to receiver
endpoints)
- Stripe HMAC signature verification. The GitHub and GitLab schemes
landed with inbound verification
(https://git.eeqj.de/sneak/webhooker/issues/67)
- Webhook signature verification for GitHub and Stripe HMAC formats
- API key authentication for programmatic access (APIKey model exists;
Bearer token middleware does not)
- REST API v1
@@ -311,10 +325,9 @@ at rest).
- OpenAPI specification
- Analytics dashboard: success rates, response times, volume
- A remember-me option at login
- Password reset flow for a forgotten password over the web. The
authenticated password *change* flow already landed, and a lost
password is recoverable from the console with `webhooker resetpw`
(https://git.eeqj.de/sneak/webhooker/issues/208)
- Password reset flow for a forgotten password. The authenticated
password *change* flow already landed on `main` (#65); reset does not
exist
- Later, nice to have
- email delivery target type
- SNS and S3 delivery targets

View File

@@ -157,10 +157,6 @@ func newApp() *fx.App {
session.New,
handlers.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.NewArchiveSweeper,
// Wire *delivery.Engine as delivery.Notifier so the

View File

@@ -128,22 +128,6 @@ type Config struct {
// clients.
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
log *slog.Logger
}
@@ -488,11 +472,6 @@ func loadFromEnv() (*Config, error) {
return nil, err
}
allowedEgressCIDRs, err := envPrefixList("ALLOWED_EGRESS_CIDRS")
if err != nil {
return nil, err
}
metricsUsername, metricsPassword, err := resolveMetricsAuth()
if err != nil {
return nil, err
@@ -511,49 +490,9 @@ func loadFromEnv() (*Config, error) {
SessionIdleTimeout: sessionIdleTimeout,
ReceiverRateLimit: receiverRateLimit,
TrustedProxies: trustedProxies,
AllowedEgressCIDRs: allowedEgressCIDRs,
}, 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
// TRUSTED_PROXIES is empty, in any environment.
//
@@ -635,13 +574,11 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
"sessionIdleTimeout", s.SessionIdleTimeout.String(),
"receiverRateLimit", s.ReceiverRateLimit,
"trustedProxies", len(s.TrustedProxies),
"allowedEgressCIDRs", len(s.AllowedEgressCIDRs),
"hasSentryDSN", s.SentryDSN != "",
"hasMetricsAuth", s.MetricsAuthEnabled(),
)
s.warnSharedRateLimitBucket(log)
s.warnEgressAllowlist(log)
return s, nil
}

View File

@@ -663,187 +663,6 @@ func testTrustedProxiesSuccess(
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
// tells an operator a deployment behind a reverse proxy shares one
// rate-limit bucket between every client, which turns the receiver

View File

@@ -21,21 +21,6 @@ func WarnSharedRateLimitBucketForTest(log *slog.Logger) error {
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.
func EnvBoolForTest(key string, defaultValue bool) (bool, error) {
return envBool(key, defaultValue)

View File

@@ -18,27 +18,26 @@ func newSSRFTestEngine() *delivery.Engine {
log := slog.New(slog.DiscardHandler)
client := &http.Client{
Timeout: 30 * time.Second,
Transport: delivery.NewTestGuard().
NewSSRFSafeTransport(),
Timeout: 30 * time.Second,
Transport: delivery.NewSSRFSafeTransport(),
}
return delivery.NewTestEngine(log, client, 1)
}
// TestClientForRequest_TimeoutKeepsSSRFGuard asserts that a
// client returned by clientForRequest for a config with a
// TestClientForConfig_TimeoutKeepsSSRFGuard asserts that a
// client returned by clientForConfig for a config with a
// per-target timeout still refuses connections to
// private/reserved addresses (the timeout must not drop the
// SSRF-safe transport).
func TestClientForRequest_TimeoutKeepsSSRFGuard(t *testing.T) {
func TestClientForConfig_TimeoutKeepsSSRFGuard(t *testing.T) {
t.Parallel()
engine := newSSRFTestEngine()
blocked := []string{
loopbackHookURL,
metadataURL,
"http://127.0.0.1/hook",
"http://169.254.169.254/latest/meta-data/",
"http://[fe80::1]/hook",
}
@@ -51,7 +50,7 @@ func TestClientForRequest_TimeoutKeepsSSRFGuard(t *testing.T) {
Timeout: 5,
}
client := engine.ExportClientForRequest(cfg, nil)
client := engine.ExportClientForConfig(cfg)
require.NotSame(t, engine.ExportClient(), client,
"a per-target timeout must yield a "+
@@ -92,11 +91,10 @@ func TestClientForRequest_TimeoutKeepsSSRFGuard(t *testing.T) {
}
}
// TestClientForRequest_NoTimeoutUnchanged asserts that a
// request with neither a per-target timeout nor an origin-scoped
// header gets the shared SSRF-safe client unchanged: there is then
// nothing for a redirect policy to strip.
func TestClientForRequest_NoTimeoutUnchanged(t *testing.T) {
// TestClientForConfig_NoTimeoutUnchanged asserts that with
// no per-target timeout the shared SSRF-safe client is
// returned unchanged.
func TestClientForConfig_NoTimeoutUnchanged(t *testing.T) {
t.Parallel()
engine := newSSRFTestEngine()
@@ -105,46 +103,10 @@ func TestClientForRequest_NoTimeoutUnchanged(t *testing.T) {
URL: "https://example.com/hook",
}
client := engine.ExportClientForRequest(cfg, nil)
client := engine.ExportClientForConfig(cfg)
assert.Same(t, engine.ExportClient(), client,
"without a per-target timeout the shared client "+
"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,7 +121,6 @@ type EngineParams struct {
DB *database.Database
DBManager *database.WebhookDBManager
Logger *logger.Logger
SSRFGuard *Guard
}
// Engine processes queued deliveries in the background
@@ -177,7 +176,7 @@ func New(
e.initTargets(&http.Client{
Timeout: httpClientTimeout,
Transport: params.SSRFGuard.NewSSRFSafeTransport(),
Transport: NewSSRFSafeTransport(),
})
e.registerHooks(lc)

View File

@@ -1201,83 +1201,6 @@ func TestDeliverHTTP_TargetTimeout(t *testing.T) {
iAssertResultFailed(t, db, del.ID)
}
// TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog pins the size
// this engine stores for an oversized response, because the
// event log's redaction is written against it: the row holds
// exactly maxBodyLog bytes and records nothing about how much
// more the remote sent, so a credential echoed across that
// boundary reaches the database already severed and no reader
// of the row can tell the cut happened.
func TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog(
t *testing.T,
) {
t.Parallel()
// Padded so the cut falls five bytes before the end of the
// echoed webhook URL.
const (
severedTail = 5
overshoot = 100000
)
sent := strings.Repeat(
"A",
delivery.ExportMaxBodyLog-len(slackWebhookURL)+
severedTail,
) + slackWebhookURL + strings.Repeat("Z", overshoot)
s := newISetup(t)
ts := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusBadGateway)
_, _ = io.WriteString(w, sent)
},
))
defer ts.Close()
cfgJSON := iHTTPConfig(ts.URL)
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, `{"cut":"test"}`,
)
targetID := uuid.New().String()
del := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusPending,
)
bodyStr := event.Body
task := iTask(
del, event, s.WebhookID, targetID,
"cut-target", cfgJSON, 0, 1, &bodyStr,
)
s.Engine.ExportProcessNewTask(context.TODO(), &task)
results := iResults(t, s.WebhookDB, del.ID)
require.Len(t, results, 1)
stored := results[0].ResponseBody
assert.Len(
t, stored, delivery.ExportMaxBodyLog,
"an oversized response is stored at exactly the cap",
)
assert.Equal(
t, sent[:delivery.ExportMaxBodyLog], stored,
)
assert.NotContains(
t, stored, slackWebhookURL,
"the echoed URL is severed by the cut",
)
assert.Contains(
t, stored, "T00000000",
"the severed prefix still carries the credential",
)
}
// iSeedEventAndDelivery creates event + delivery
// for standalone tests.
func iSeedEventAndDelivery(

View File

@@ -5,8 +5,6 @@ import (
"log/slog"
"net"
"net/http"
"net/netip"
"net/url"
"time"
"go.uber.org/fx"
@@ -27,12 +25,6 @@ const (
ExportRetryChannelSize = retryChannelSize
ExportDefaultFailureThreshold = defaultFailureThreshold
ExportDefaultCooldown = defaultCooldown
// ExportMaxBodyLog is the cap the engine applies to a
// recorded response body. The event log's handling of a cut
// response is written against this number, so a test has to
// be able to name it.
ExportMaxBodyLog = maxBodyLog
)
// ExportIsBlockedIP exposes isBlockedIP for testing.
@@ -40,26 +32,6 @@ func ExportIsBlockedIP(ip net.IP) bool {
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.
func ExportBlockedNetworks() []*net.IPNet {
return blockedNetworks
@@ -71,14 +43,13 @@ func ExportIsForwardableHeader(name string) bool {
}
// ExportApplyRequestHeaders exposes applyRequestHeaders, so a test
// can inspect the header set an outbound delivery actually carries
// and the origin-scoped names it reports for the redirect policy.
// can inspect the header set an outbound delivery actually carries.
func ExportApplyRequestHeaders(
req *http.Request,
event *database.Event,
cfg *HTTPTargetConfig,
) []string {
return applyRequestHeaders(req, event, cfg)
) {
applyRequestHeaders(req, event, cfg)
}
// ExportTruncate exposes truncate for testing.
@@ -188,27 +159,12 @@ func (e *Engine) ExportDoHTTPRequest(
return e.httpTarget.doHTTPRequest(ctx, cfg, event)
}
// ExportClientForRequest exposes the http target's
// clientForRequest.
func (e *Engine) ExportClientForRequest(
// ExportClientForConfig exposes the http target's
// clientForConfig.
func (e *Engine) ExportClientForConfig(
cfg *HTTPTargetConfig,
originScoped []string,
) *http.Client {
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)
return e.httpTarget.clientForConfig(cfg)
}
// ExportClient returns the http target's shared HTTP client.

View File

@@ -1,107 +0,0 @@
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

@@ -1,383 +0,0 @@
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,11 +6,8 @@ import (
"fmt"
"net"
"net/http"
"net/netip"
"net/url"
"time"
"sneak.berlin/go/webhooker/internal/config"
)
const (
@@ -28,83 +25,20 @@ var (
errBlockedIP = errors.New(
"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(
"only http and https are allowed",
)
)
// blockedNetworks contains all private/reserved IP ranges
// that should be blocked to prevent SSRF attacks. An operator
// can permit specific blocks out of this set with
// ALLOWED_EGRESS_CIDRS; see Guard.
// that should be blocked to prevent SSRF attacks.
//
//nolint:gochecknoglobals // package-level network list is appropriate here
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
func init() {
blockedNetworks = mustParseCIDRs([]string{
cidrs := []string{
"127.0.0.0/8",
"10.0.0.0/8",
"172.16.0.0/12",
@@ -122,72 +56,7 @@ func init() {
"::1/128",
"fc00::/7",
"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 {
_, network, err := net.ParseCIDR(cidr)
@@ -198,15 +67,16 @@ func mustParseCIDRs(cidrs []string) []*net.IPNet {
))
}
networks = append(networks, network)
blockedNetworks = append(
blockedNetworks, network,
)
}
return networks
}
// matchesAny reports whether ip falls inside any of networks.
func matchesAny(networks []*net.IPNet, ip net.IP) bool {
for _, network := range networks {
// isBlockedIP checks whether an IP address falls within
// any blocked private/reserved network range.
func isBlockedIP(ip net.IP) bool {
for _, network := range blockedNetworks {
if network.Contains(ip) {
return true
}
@@ -215,40 +85,9 @@ func matchesAny(networks []*net.IPNet, ip net.IP) bool {
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
// URL is safe from SSRF attacks.
func (g *Guard) ValidateTargetURL(
func ValidateTargetURL(
ctx context.Context, targetURL string,
) error {
parsed, err := url.Parse(targetURL)
@@ -272,79 +111,36 @@ func (g *Guard) ValidateTargetURL(
}
if ip := net.ParseIP(host); ip != nil {
return g.checkIP(ip)
return checkBlockedIP(ip)
}
return g.validateHostname(ctx, host)
return validateHostname(ctx, host)
}
// NewSSRFSafeTransport creates an http.Transport with a
// 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) {
func validateScheme(scheme string) error {
if scheme != "http" && scheme != "https" {
return fmt.Errorf(
"target IP %s: %w", ip, errBlockedMetadata,
)
}
if g.allows(ip) {
return nil
}
if isBlockedIP(ip) {
return fmt.Errorf(
"target IP %s: %w", ip, errBlockedIP,
"unsupported URL scheme %q: %w",
scheme, errInvalidScheme,
)
}
return nil
}
func (g *Guard) validateHostname(
func checkBlockedIP(ip net.IP) error {
if isBlockedIP(ip) {
return fmt.Errorf(
"target IP %s is in a blocked "+
"private/reserved range: %w",
ip, errBlockedIP,
)
}
return nil
}
func validateHostname(
ctx context.Context, host string,
) error {
dnsCtx, cancel := context.WithTimeout(
@@ -369,11 +165,11 @@ func (g *Guard) validateHostname(
}
for _, ipAddr := range ips {
err = g.checkIP(ipAddr.IP)
if err != nil {
if isBlockedIP(ipAddr.IP) {
return fmt.Errorf(
"hostname %q resolves to a blocked address: %w",
host, err,
"hostname %q resolves to blocked "+
"IP %s: %w",
host, ipAddr.IP, errBlockedIP,
)
}
}
@@ -381,7 +177,16 @@ func (g *Guard) validateHostname(
return nil
}
func (g *Guard) ssrfDialContext(
// NewSSRFSafeTransport creates an http.Transport with a
// custom DialContext that blocks connections to
// private/reserved IP addresses.
func NewSSRFSafeTransport() *http.Transport {
return &http.Transport{
DialContext: ssrfDialContext,
}
}
func ssrfDialContext(
ctx context.Context,
network, addr string,
) (net.Conn, error) {
@@ -404,11 +209,11 @@ func (g *Guard) ssrfDialContext(
}
for _, ipAddr := range ips {
err = g.checkIP(ipAddr.IP)
if err != nil {
if isBlockedIP(ipAddr.IP) {
return nil, fmt.Errorf(
"ssrf: connection to %s blocked: %w",
host, err,
"ssrf: connection to %s (%s) "+
"blocked: %w",
host, ipAddr.IP, errBlockedIP,
)
}
}
@@ -420,14 +225,3 @@ func (g *Guard) ssrfDialContext(
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

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

View File

@@ -65,11 +65,6 @@ func isReservedTargetHeader(name string) bool {
// the configured headers, so a configured one would always
// be overwritten.
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:
return false
}
@@ -123,11 +118,9 @@ func parseHeaderLine(line string) (string, string, error) {
rawName = strings.TrimSpace(rawName)
if !validHeaderName(rawName) {
// Quotes nothing. The text before the first colon is only
// 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
return "", "", fmt.Errorf(
"%w: %q", errHeaderNameInvalid, rawName,
)
}
name := http.CanonicalHeaderKey(rawName)

View File

@@ -82,16 +82,6 @@ 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
// rendered into a 400 body, so no message may quote one.
func TestParseTargetHeaders_ErrorsNeverQuoteAValue(t *testing.T) {
@@ -99,26 +89,17 @@ func TestParseTargetHeaders_ErrorsNeverQuoteAValue(t *testing.T) {
const secret = "QQNEVERINAMESSAGEQQ"
inputs := []string{
// The value, after the colon, in a duplicate name.
_, err := delivery.ParseTargetHeaders(
"X-A: " + secret + "\nx-a: " + secret,
// The value after the colon of an unusable name.
"X Bad Name: " + secret,
// The line splits on the value's own colon, so the
// secret lands in the text an unusable-name error is
// 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",
}
)
require.Error(t, err)
assert.NotContains(t, err.Error(), secret)
for _, input := range inputs {
_, err := delivery.ParseTargetHeaders(input)
require.Error(t, err, input)
assert.NotContains(t, err.Error(), secret, input)
}
_, err = delivery.ParseTargetHeaders(
"X Bad Name: " + secret,
)
require.Error(t, err)
assert.NotContains(t, err.Error(), secret)
}
// Loading the edit form twice without saving must not reshuffle

View File

@@ -8,7 +8,6 @@ import (
"fmt"
"io"
"net/http"
"sort"
"sync"
"time"
@@ -405,9 +404,9 @@ func (t *httpTarget) doHTTPRequest(
)
}
originScoped := applyRequestHeaders(req, event, cfg)
applyRequestHeaders(req, event, cfg)
client := t.clientForRequest(cfg, originScoped)
client := t.clientForConfig(cfg)
resp, doErr := executeHTTPRequest(client, req)
@@ -433,41 +432,23 @@ func (t *httpTarget) doHTTPRequest(
return resp.StatusCode, string(body), dur, nil
}
// 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(
func (t *httpTarget) clientForConfig(
cfg *HTTPTargetConfig,
originScoped []string,
) *http.Client {
if cfg.Timeout <= 0 && len(originScoped) == 0 {
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
// Reuse the shared client's SSRF-safe transport so
// 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,
}
}
if len(originScoped) > 0 {
client.CheckRedirect = offOriginHeaderPolicy(originScoped)
}
return client
return t.client
}
func parseHTTPConfig(
@@ -509,88 +490,40 @@ 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(
req *http.Request,
event *database.Event,
cfg *HTTPTargetConfig,
) []string {
) {
if event.ContentType != "" {
req.Header.Set(
"Content-Type", event.ContentType,
)
}
originScoped := forwardEventHeaders(req, event)
var originalHeaders map[string][]string
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 {
req.Header.Set(k, v)
originScoped[http.CanonicalHeaderKey(k)] = struct{}{}
}
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

View File

@@ -1,272 +0,0 @@
package delivery
import (
"net/url"
"slices"
"strings"
"sneak.berlin/go/webhooker/internal/database"
)
// RedactionMarker stands in for a target credential found in
// text the target's remote peer chose.
const RedactionMarker = "(redacted)"
// Redactor removes one target's own credential material from
// text that target's remote peer chose: a delivery response
// body, or a delivery error stored before the delivery path
// learned to mask the URLs it embeds.
//
// It removes byte-identical echoes of strings taken from the
// target's stored configuration, and nothing else. Anything
// the remote re-encodes survives: JSON "\/" escaping (what
// PHP's json_encode emits by default), percent-encoding, HTML
// entities, and an echo of only part of a path. It cannot
// remove a secret the remote invented.
//
// The zero Redactor removes nothing, which is what a caller
// holding no target for a delivery gets.
type Redactor struct {
secrets []string
}
// NewRedactor builds the redactor for one target.
func NewRedactor(t *database.Target) Redactor {
// Drop empty strings here rather than at the site that
// produced one. strings.ReplaceAll with an empty old string
// inserts the marker at every byte boundary, so a single
// empty secret destroys every body and error the target
// renders; filtering at the collection point means no field
// added to targetSecrets later can reintroduce that.
// url.Parse("https://@example.com/in") is the known
// producer: a non-nil User whose String is "".
secrets := slices.DeleteFunc(
targetSecrets(t),
func(s string) bool { return s == "" },
)
// Longest first, so replacing a secret that is contained
// in a longer one cannot leave a fragment of the longer
// one behind. Configured headers arrive in map order, so
// the sort is also what makes the result deterministic.
slices.SortFunc(secrets, func(a, b string) int {
if d := len(b) - len(a); d != 0 {
return d
}
return strings.Compare(a, b)
})
return Redactor{secrets: secrets}
}
// Redact replaces every occurrence of the target's credential
// material in s.
func (r Redactor) Redact(s string) string {
if s == "" {
return s
}
for _, secret := range r.secrets {
s = strings.ReplaceAll(s, secret, RedactionMarker)
}
return s
}
// RedactCut redacts s, which its caller has already cut to a
// byte budget, and additionally drops any tail of s that is a
// proper prefix of a secret.
//
// The cut lands wherever the remote's padding puts it, so the
// remote chooses where inside the credential it falls. The
// severed prefix left behind equals no secret, so plain
// Redact would render it verbatim.
func (r Redactor) RedactCut(s string) string {
s = r.Redact(s)
if n := r.secretPrefixSuffix(s); n > 0 {
return s[:len(s)-n] + RedactionMarker
}
return s
}
// secretPrefixSuffix returns the length of the longest suffix
// of s that is a proper prefix of one of the secrets, or 0
// when there is none.
func (r Redactor) secretPrefixSuffix(s string) int {
longest := 0
for _, secret := range r.secrets {
// Proper prefixes only: a whole secret at the tail was
// already replaced by Redact.
n := min(len(secret)-1, len(s))
for ; n > longest; n-- {
if strings.HasSuffix(s, secret[:n]) {
longest = n
break
}
}
}
return longest
}
// targetSecrets returns the credential-bearing strings a
// target's configuration carries.
//
// The destination URL contributes. Its path, query and
// userinfo are the credential for both target types that have
// one — an incoming-webhook URL is a bearer token, which is
// why MaskURL elides exactly those parts — and they are the
// material this service actually sends, so a remote that
// echoes the request back echoes them.
//
// Configured request headers contribute their values, but
// only for the credential-shaped names isCredentialHeaderName
// picks out. That is the same class-based rule applied to
// URLs: an echoed Accept or User-Agent still renders, an
// echoed Authorization does not.
func targetSecrets(t *database.Target) []string {
if t == nil {
return nil
}
switch t.Type {
case database.TargetTypeSlack:
cfg, err := parseSlackConfig(t.Config)
if err != nil {
return nil
}
return urlSecrets(cfg.WebhookURL)
case database.TargetTypeHTTP:
cfg, err := parseHTTPConfig(t.Config)
if err != nil {
return nil
}
return append(
urlSecrets(cfg.URL),
headerSecrets(cfg.Headers)...,
)
case database.TargetTypeDatabase, database.TargetTypeLog:
// Neither has a destination URL, so neither has
// anything to redact.
return nil
default:
return nil
}
}
// urlSecrets returns the substrings of a destination URL that
// must not survive into a rendered page: the whole URL, the
// parts of it MaskURL elides, and any userinfo.
//
// No length floor is applied to the path, and none to the
// userinfo. A short path or a four-byte username is treated as
// a credential exactly like a long one, because the field takes
// an arbitrary URL and no part of it can be assumed non-secret —
// the same rule MaskURL applies. headerSecrets does carry a
// floor, and the difference is deliberate: a header is picked
// out by a name-shaped guess and its value may be ordinary
// text, whereas a URL's path and userinfo are credential
// material by position.
func urlSecrets(raw string) []string {
raw = strings.TrimSpace(raw)
if raw == "" {
return nil
}
secrets := []string{raw}
parsed, err := url.Parse(raw)
if err != nil {
return secrets
}
if parsed.Path != "" && parsed.Path != "/" {
requestURI := parsed.RequestURI()
secrets = append(secrets, requestURI)
if escaped := parsed.EscapedPath(); escaped != requestURI {
secrets = append(secrets, escaped)
}
}
if parsed.User != nil {
secrets = append(secrets, parsed.User.String())
if pw, ok := parsed.User.Password(); ok && pw != "" {
secrets = append(secrets, pw)
}
}
return secrets
}
// minHeaderSecretBytes is the shortest header value treated as
// a credential. Unlike a URL path, a header value can be a
// couple of bytes long, and redacting those would scatter the
// marker through ordinary response text for no gain.
const minHeaderSecretBytes = 4
// headerSecrets returns the values of the configured headers
// whose names are credential-shaped.
func headerSecrets(headers map[string]string) []string {
var secrets []string
for name, value := range headers {
value = strings.TrimSpace(value)
if len(value) < minHeaderSecretBytes {
continue
}
if isCredentialHeaderName(name) {
secrets = append(secrets, value)
}
}
return secrets
}
// isCredentialHeaderName classifies a header by its name. The
// value is never inspected, so the rule is the same
// class-based one MaskURL applies to a destination URL.
//
// The fragments are short on purpose, and match anywhere in
// the name, so abbreviations an operator might use are covered
// too: X-Sig, X-Pass, X-HMAC. That over-matches — a header
// named X-Design contains "sig" — and over-matching is the
// safe direction here: the cost is a marker where an echoed
// header value would have rendered.
func isCredentialHeaderName(name string) bool {
name = strings.ToLower(strings.TrimSpace(name))
// Names that carry a credential by definition.
switch name {
case "authorization", "proxy-authorization", "cookie":
return true
}
// What operators call their own credential headers:
// X-Api-Key, X-Hub-Signature, X-Auth-Token.
for _, fragment := range []string{
"auth",
"credential",
"hmac",
"key",
"pass",
"secret",
"sig",
"token",
} {
if strings.Contains(name, fragment) {
return true
}
}
return false
}

View File

@@ -1,284 +0,0 @@
package delivery_test
import (
"net/url"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// The secret path segments of a Slack incoming webhook URL.
const (
redactSecretPath = "/services/T11111111/B11111111/" +
"YYYYYYYYYYYYYYYYYYYYYYYY"
redactWebhookURL = "https://hooks.slack.com" +
redactSecretPath
)
func TestRedactor_RemovesSlackWebhookURL(t *testing.T) {
t.Parallel()
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
})
got := r.Redact("no_service for " + redactWebhookURL)
assert.NotContains(t, got, redactSecretPath)
assert.NotContains(t, got, "T11111111")
// One marker, not a marker with the host left in front of
// it: the whole URL is replaced before the path it
// contains, which is what sorting the secrets longest
// first buys.
assert.Equal(
t,
"no_service for "+delivery.RedactionMarker,
got,
)
}
// TestRedactor_RemovesSecretSeveredByACut covers the input the
// redactor exists for: text cut to a byte budget with the
// credential straddling the cut. The remote chooses the
// padding, so it chooses where the cut lands inside the
// credential, and the severed prefix that remains equals no
// secret.
func TestRedactor_RemovesSecretSeveredByACut(t *testing.T) {
t.Parallel()
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
})
// Every cut position inside the credential, not just a
// convenient one.
for n := 1; n < len(redactWebhookURL); n++ {
severed := redactWebhookURL[:n]
cut := "padding " + severed
got := r.RedactCut(cut)
assert.Equal(
t,
"padding "+delivery.RedactionMarker,
got,
"cut after %d bytes of the credential", n,
)
}
}
// TestRedactor_RedactsCredentialShapedHeaderValues pins the
// class-based header rule: a header whose name says credential
// has its value redacted, and a routine header does not, so
// ordinary response content survives.
func TestRedactor_RedactsCredentialShapedHeaderValues(
t *testing.T,
) {
t.Parallel()
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"https://example.com/in",` +
`"headers":{` +
`"Authorization":"Bearer AAAAAAAAAAAA",` +
`"Cookie":"session=BBBBBBBBBBBB",` +
`"X-Api-Key":"CCCCCCCCCCCC",` +
`"X-Hub-Signature":"sha256=DDDDDDDDDDDD",` +
`"X-Sig":"EEEEEEEEEEEE",` +
`"X-Pass":"FFFFFFFFFFFF",` +
`"X-HMAC":"GGGGGGGGGGGG",` +
`"X-Credential":"HHHHHHHHHHHH",` +
`"Accept":"application/json",` +
`"User-Agent":"webhooker/1.0"}}`,
})
for _, secret := range []string{
"Bearer AAAAAAAAAAAA",
"session=BBBBBBBBBBBB",
"CCCCCCCCCCCC",
"sha256=DDDDDDDDDDDD",
// Abbreviated names an operator might use.
"EEEEEEEEEEEE",
"FFFFFFFFFFFF",
"GGGGGGGGGGGG",
"HHHHHHHHHHHH",
} {
got := r.Redact("echo: " + secret)
assert.Equal(
t,
"echo: "+delivery.RedactionMarker,
got,
secret,
)
}
const routine = "Accept: application/json, " +
"User-Agent: webhooker/1.0"
assert.Equal(t, routine, r.Redact(routine))
}
// TestRedactor_IgnoresVeryShortHeaderValues pins the floor
// under a header value. Redacting a two-byte value would put
// the marker through every response that happens to contain
// those bytes.
func TestRedactor_IgnoresVeryShortHeaderValues(t *testing.T) {
t.Parallel()
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"https://example.com/in",` +
`"headers":{"X-Api-Key":"ab"}}`,
})
const response = "rabbit"
assert.Equal(t, response, r.Redact(response))
}
// TestRedactor_RemovesBarePath covers a remote that echoes
// only the request path rather than the whole URL. The path
// segments are the credential on their own.
func TestRedactor_RemovesBarePath(t *testing.T) {
t.Parallel()
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
})
got := r.Redact("POST " + redactSecretPath + " 404")
assert.NotContains(t, got, redactSecretPath)
assert.Equal(
t,
"POST "+delivery.RedactionMarker+" 404",
got,
)
}
// TestRedactor_RemovesHTTPURLQueryAndUserinfo covers the HTTP
// target, whose destination is an arbitrary URL: the query
// string and the userinfo carry credentials as readily as the
// path does.
func TestRedactor_RemovesHTTPURLQueryAndUserinfo(t *testing.T) {
t.Parallel()
// Assembled rather than written out, so the literal is
// not itself a credential-shaped string.
dest := url.URL{
Scheme: "https",
User: url.UserPassword("user", "hunter2"),
Host: "example.com",
Path: "/in",
RawQuery: "token=s3cr3t",
}
raw := dest.String()
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"` + raw + `"}`,
})
for _, echoed := range []string{
raw,
"/in?token=s3cr3t",
"hunter2",
} {
got := r.Redact("rejected: " + echoed)
assert.NotContains(t, got, "s3cr3t", echoed)
assert.NotContains(t, got, "hunter2", echoed)
assert.Contains(
t, got, delivery.RedactionMarker, echoed,
)
}
}
// TestRedactor_LeavesUnrelatedTextAlone pins that the
// redactor matches literally: it does not guess at what a
// secret looks like, so ordinary response content survives.
func TestRedactor_LeavesUnrelatedTextAlone(t *testing.T) {
t.Parallel()
const response = "ok=false error=channel_not_found"
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeSlack,
Config: `{"webhookUrl":"` + redactWebhookURL + `"}`,
})
assert.Equal(t, response, r.Redact(response))
}
// TestRedactor_EmptyUserinfoDoesNotShredTheBody covers a
// destination URL written with a bare "@" and no userinfo:
// url.Parse returns a non-nil User whose String is empty. An
// empty secret in the list would make strings.ReplaceAll
// insert the marker at every byte boundary, destroying every
// body and error string the target renders.
func TestRedactor_EmptyUserinfoDoesNotShredTheBody(t *testing.T) {
t.Parallel()
const dest = "https://@example.com/in"
// The premise: this URL really does parse to a non-nil
// User contributing an empty string.
parsed, err := url.Parse(dest)
require.NoError(t, err)
require.NotNil(t, parsed.User)
require.Empty(t, parsed.User.String())
r := delivery.NewRedactor(&database.Target{
Type: database.TargetTypeHTTP,
Config: `{"url":"` + dest + `"}`,
})
const body = "ok=false error=channel_not_found"
assert.Equal(t, body, r.Redact(body))
assert.Equal(t, body, r.RedactCut(body))
// The real credential material still goes, so filtering the
// empty string out did not disarm the redactor.
assert.Equal(
t,
"POST "+delivery.RedactionMarker,
r.Redact("POST "+dest),
)
}
// TestRedactor_ZeroValueAndConfiglessTargets pins that a
// caller with no target, an unparseable config, or a target
// type with no destination URL gets a redactor that changes
// nothing rather than one that panics.
func TestRedactor_ZeroValueAndConfiglessTargets(t *testing.T) {
t.Parallel()
const text = "some response body"
var zero delivery.Redactor
assert.Equal(t, text, zero.Redact(text))
assert.Equal(t, text, delivery.NewRedactor(nil).Redact(text))
for _, tgt := range []database.Target{
{Type: database.TargetTypeLog},
{Type: database.TargetTypeDatabase},
{Type: database.TargetTypeSlack, Config: "not json"},
{Type: database.TargetTypeHTTP, Config: ""},
} {
assert.Equal(
t, text,
delivery.NewRedactor(&tgt).Redact(text),
tgt.Type,
)
}
}

View File

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

View File

@@ -42,7 +42,7 @@ func seedFailedDelivery(
Method: http.MethodPost,
Headers: `{"X-Test":["yes"]}`,
Body: `{"replay":"me"}`,
ContentType: contentTypeJSON,
ContentType: "application/json",
}
require.NoError(t, webhookDB.Omit(

View File

@@ -1,169 +0,0 @@
package handlers
import (
"sneak.berlin/go/webhooker/internal/delivery"
)
// maxRenderedResponseBytes caps how many bytes of one stored
// delivery response body reach the event log page.
//
// The bound is enforced here and in SQL, because this page's
// memory profile must not depend on a constant in another
// package staying where it is, and because rows predating the
// delivery engine's own cap or restored from an archive are
// not covered by it at all.
//
// It happens to equal that engine cap, so a row written by the
// current engine reaches this one exactly and is never cut
// twice. Nothing here may assume the two differ: see view.
const maxRenderedResponseBytes = 4096
// deliveryResultColumns is the delivery attempt projection.
// The casts to blob are load-bearing for the same reason they
// are in eventLogColumns: they make substr and length count
// bytes rather than characters, and they make SQLite do the
// cut, so an oversized stored response never becomes a Go
// string at all.
const deliveryResultColumns = "delivery_id, attempt_num, success, " +
"status_code, error, duration, " +
"substr(cast(response_body as blob), 1, ?) AS response_body, " +
"length(cast(response_body as blob)) AS response_bytes"
// DeliveryResultView is the display-safe projection of one
// delivery attempt for the event log page. It carries a
// capped response body plus the true stored size, so the page
// can mark a response as truncated without holding the whole
// thing.
//
// Both Error and ResponseBody have been through the target's
// Redactor. The engine already masks the URL out of the
// errors it stores, so for errors this is a second line
// covering rows written before it did; for response bodies it
// is the only line, and its reach is what
// delivery.Redactor documents.
type DeliveryResultView struct {
AttemptNum int
Success bool
// StatusCode is 0 when the attempt never got a response,
// which is why the page asks HasStatusCode rather than
// printing the number.
StatusCode int
// Error is the stored failure message, redacted.
Error string
// DurationMS is how long the attempt took.
DurationMS int64
// ResponseBody holds at most maxRenderedResponseBytes
// bytes of the stored response, redacted. It is remote
// content and must only ever be rendered escaped.
ResponseBody string
// ResponseBytes is the size of the stored response body,
// before the cut and before redaction. It is what the
// remote sent only when ResponseSizeKnown is set.
ResponseBytes int64
// ResponseShownBytes is how much of that the page is
// showing. It is the size of the cut, taken before
// redaction, so the truncation marker reports what SQLite
// returned rather than how much the marker substitution
// then changed the length.
ResponseShownBytes int
// ResponseTruncated reports that the body shown may be
// incomplete, so the page owes the reader a marker. Every
// body that reaches the cap counts, because one the
// delivery engine cut at its own equal cap is
// indistinguishable from a complete one.
ResponseTruncated bool
// ResponseSizeKnown reports that ResponseBytes is the whole
// response the remote sent, so the marker may quote it. It
// is false for a body that only reaches the cap, where how
// much came after it was never recorded.
ResponseSizeKnown bool
}
// HasStatusCode reports whether the attempt got as far as an
// HTTP response. A transport failure stores no status code,
// and rendering that as "0" would read as a real status.
func (v DeliveryResultView) HasStatusCode() bool {
return v.StatusCode != 0
}
// deliveryResultRow is one row of the delivery attempt
// projection. Its response body arrives already cut to the
// cap by SQLite, with the true size beside it.
type deliveryResultRow struct {
DeliveryID string
AttemptNum int
Success bool
StatusCode int
Error string
Duration int64
ResponseBody []byte
ResponseBytes int64
}
// view projects a loaded row for rendering, stripping the
// target's own credential out of the two fields a remote peer
// gets to influence.
func (r *deliveryResultRow) view(
redactor delivery.Redactor,
) DeliveryResultView {
body := r.ResponseBody
// Two different cuts can have shortened this body, and the
// row records only one of them. SQLite cuts here, whenever
// the stored value is larger than the cap. The delivery
// engine cut earlier, whenever the remote sent more than
// its own maxBodyLog — which is this same number, so such a
// row stores the cut length as its whole length and nothing
// in it separates a response that ended at the cap from one
// severed there.
//
// So a body that reaches the cap is treated as cut either
// way. Gating on ResponseBytes alone would assume the two
// caps differ, and they do not: under the current engine
// that gate never opens.
cut := r.ResponseBytes > int64(len(body)) ||
len(body) >= maxRenderedResponseBytes
// The row holds more than the page shows only in the first
// of those cases. In the second the stored row is all there
// is, and its size is a floor rather than the true one.
sizeKnown := r.ResponseBytes > int64(len(body))
// Only a cut response can have been left mid-sequence,
// exactly as with an event body.
if cut {
body = trimPartialRune(body)
}
// A cut body goes through RedactCut: the remote controls
// the padding ahead of a credential it echoes, so it
// controls where the cut falls inside that credential, and
// the severed prefix left behind matches no secret whole.
rendered := string(body)
if cut {
rendered = redactor.RedactCut(rendered)
} else {
rendered = redactor.Redact(rendered)
}
return DeliveryResultView{
AttemptNum: r.AttemptNum,
Success: r.Success,
StatusCode: r.StatusCode,
Error: redactor.Redact(r.Error),
DurationMS: r.Duration,
ResponseBody: rendered,
ResponseBytes: r.ResponseBytes,
ResponseShownBytes: len(body),
ResponseTruncated: cut,
ResponseSizeKnown: sizeKnown,
}
}

View File

@@ -1,515 +0,0 @@
package handlers_test
import (
"net/http"
"net/http/httptest"
"strconv"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session"
)
// responseCap is the number of response bytes the event log
// page is allowed to render for one delivery attempt.
const responseCap = handlers.MaxRenderedResponseBytesForTest
// failedAttempt describes the failed delivery every test in
// this file seeds. The values are distinctive so that finding
// them in the rendered page cannot be a coincidence.
const (
attemptStatusCode = 502
attemptDurationMS = 1234
attemptNumber = 3
attemptError = "upstream returned 502 Bad Gateway"
)
// seedFailedDeliveryWithResponse records an event, a failed
// delivery against targetID, and one delivery result carrying
// the given response body. It returns the delivery.
//
// 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,
dbMgr *database.WebhookDBManager,
webhookID, targetID, responseBody string,
) *database.Delivery {
t.Helper()
webhookDB, err := dbMgr.GetDB(webhookID)
require.NoError(t, err)
event := &database.Event{
WebhookID: webhookID,
Method: http.MethodPost,
Body: `{"test":true}`,
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: attemptNumber,
Success: false,
StatusCode: attemptStatusCode,
ResponseBody: responseBody,
Error: attemptError,
Duration: attemptDurationMS,
}
require.NoError(t, webhookDB.Omit(
clause.Associations,
).Create(result).Error)
return dlv
}
// seedFailureAndRender seeds a failed delivery against a
// target of the given type and config, and returns the
// rendered event log page.
func seedFailureAndRender(
t *testing.T,
targetType database.TargetType,
config, responseBody string,
) string {
t.Helper()
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, targetType, config,
)
seedFailedDeliveryWithResponse(
t, dbMgr, wh.ID, tgt.ID, responseBody,
)
return renderSourceLogsPage(t, h, sess, wh.ID)
}
// TestHandleSourceLogs_RendersFailedAttempt is the regression
// test for the reported gap: a failed delivery used to render
// as the status word alone, so diagnosing it meant opening the
// per-webhook SQLite file by hand.
func TestHandleSourceLogs_RendersFailedAttempt(t *testing.T) {
t.Parallel()
body := seedFailureAndRender(
t,
database.TargetTypeHTTP,
`{"url":"https://example.com/hook/abc"}`,
"upstream exploded",
)
assert.Contains(
t, body, strconv.Itoa(attemptStatusCode),
"the attempt's status code must reach the page",
)
assert.Contains(
t, body, attemptError,
"the attempt's error must reach the page",
)
assert.Contains(
t, body, strconv.Itoa(attemptDurationMS),
"the attempt's duration must reach the page",
)
assert.Contains(
t, body, "Attempt "+strconv.Itoa(attemptNumber),
"the attempt number must reach the page",
)
assert.Contains(
t, body, "upstream exploded",
"the attempt's response body must reach the page",
)
}
// TestHandleSourceLogs_EscapesResponseBody proves the
// response body is treated as the untrusted remote content it
// is. The remote chooses these bytes and the page is rendered
// inside the operator's authenticated origin, where the
// application's own CSP allows inline script from 'self'.
func TestHandleSourceLogs_EscapesResponseBody(t *testing.T) {
t.Parallel()
const payload = `<script>alert("xss")</script>`
body := seedFailureAndRender(
t,
database.TargetTypeHTTP,
`{"url":"https://example.com/hook/abc"}`,
payload,
)
assert.NotContains(t, body, payload)
assert.NotContains(t, body, "<script>alert")
assert.Contains(t, body, "alert")
}
// TestHandleSourceLogs_RedactsCredentialEchoedInResponse
// covers the case that makes rendering a response body a
// disclosure question at all: the remote echoes back the
// credential the request carried, and the page would then put
// it on the operator's screen.
func TestHandleSourceLogs_RedactsCredentialEchoedInResponse(
t *testing.T,
) {
t.Parallel()
body := seedFailureAndRender(
t,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
"no_service: "+slackWebhookURL,
)
assert.NotContains(t, body, slackSecretPath)
assert.NotContains(t, body, "T00000000")
assert.NotContains(t, body, "B00000000")
assert.Contains(t, body, delivery.RedactionMarker)
// The rest of the response is still shown, or the
// redaction would have cost the operator the diagnosis.
assert.Contains(t, body, "no_service")
}
// TestHandleSourceLogs_RedactsCredentialEchoedInError covers
// the same disclosure through the error field. The delivery
// engine masks the URL out of the errors it stores, so this
// holds the read path to the rows written before it did.
func TestHandleSourceLogs_RedactsCredentialEchoedInError(
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.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
)
dlv := seedFailedDeliveryWithResponse(t, dbMgr, wh.ID, tgt.ID, "")
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
// An unmasked transport error, exactly as Go's HTTP
// client renders one.
require.NoError(t, webhookDB.Model(
&database.DeliveryResult{},
).Where(
"delivery_id = ?", dlv.ID,
).Update(
"error",
`Post "`+slackWebhookURL+`": dial tcp: i/o timeout`,
).Error)
body := renderSourceLogsPage(t, h, sess, wh.ID)
assert.NotContains(t, body, slackSecretPath)
assert.Contains(t, body, delivery.RedactionMarker)
assert.Contains(t, body, "i/o timeout")
}
// severedPadding is the filler that puts the end of an echoed
// webhook URL five bytes past a cut at the response cap, so
// the cut leaves the workspace ID, the bot ID and all but the
// last few token characters behind.
func severedPadding() string {
const severedTail = 5
return strings.Repeat(
"A", responseCap-len(slackWebhookURL)+severedTail,
)
}
// TestHandleSourceLogs_RedactsCredentialSeveredByTheEngineCut
// is the regression test for a redactor gated on the SQL cut
// alone. The delivery engine stops reading a response at its
// own cap, which is the same number of bytes this page
// renders, so a row the engine cut is byte-for-byte
// indistinguishable from a complete response and that gate
// never opened on anything the engine writes.
//
// The seeded body is what the engine stores for any remote
// that sends at least that much: exactly responseCap bytes,
// ending in a severed webhook URL.
// TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog in
// internal/delivery pins that this is the size it produces.
func TestHandleSourceLogs_RedactsCredentialSeveredByTheEngineCut(
t *testing.T,
) {
t.Parallel()
sent := severedPadding() + slackWebhookURL +
strings.Repeat("Z", 128)
stored := sent[:responseCap]
require.Len(
t, stored, responseCap,
"the engine stores exactly the cap, never more",
)
require.Contains(
t, stored, "T00000000",
"the severed credential must be in what is seeded",
)
body := seedFailureAndRender(
t,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
stored,
)
assert.NotContains(t, body, "T00000000")
assert.NotContains(t, body, "B00000000")
assert.Contains(t, body, delivery.RedactionMarker)
assert.Contains(
t, body, "reached the recording limit",
"a body the engine cut must not be shown as complete",
)
}
// TestHandleSourceLogs_RedactsCredentialSeveredBySQLCut covers
// the same severing for a row larger than the cap, which is
// SQLite's cut rather than the engine's. The current engine
// writes no such row; rows predating its cap or restored from
// an archive are not bounded by it, which is why the page cuts
// again in SQL and has to redact that cut too.
func TestHandleSourceLogs_RedactsCredentialSeveredBySQLCut(
t *testing.T,
) {
t.Parallel()
stored := severedPadding() + slackWebhookURL +
strings.Repeat("Z", 128)
require.Greater(
t, len(stored), responseCap,
"the stored body must exceed the cap or nothing is cut",
)
body := seedFailureAndRender(
t,
database.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
stored,
)
assert.NotContains(t, body, "T00000000")
assert.NotContains(t, body, "B00000000")
assert.NotContains(
t, body, slackWebhookURL[:len(slackWebhookURL)-10],
)
assert.Contains(t, body, delivery.RedactionMarker)
}
// TestHandleSourceLogs_RedactsForSoftDeletedTarget covers a
// target an operator has deleted. The row is only soft deleted
// and its deliveries survive in the per-webhook database, so
// its redactor has to survive with it or every response body
// it ever recorded renders unredacted.
func TestHandleSourceLogs_RedactsForSoftDeletedTarget(
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.TargetTypeSlack,
`{"webhookUrl":"`+slackWebhookURL+`"}`,
)
seedFailedDeliveryWithResponse(
t, dbMgr, wh.ID, tgt.ID,
"no_service: "+slackWebhookURL,
)
require.NoError(t, db.DB().Delete(tgt).Error)
body := renderSourceLogsPage(t, h, sess, wh.ID)
assert.NotContains(t, body, slackSecretPath)
assert.NotContains(t, body, "T00000000")
assert.Contains(t, body, delivery.RedactionMarker)
assert.Contains(t, body, "no_service")
}
// TestHandleSourceLogs_BoundsRenderedAttempts pins the ceiling
// on how many of one delivery's attempts reach the page, and
// that what it drops is counted rather than hidden.
func TestHandleSourceLogs_BoundsRenderedAttempts(t *testing.T) {
t.Parallel()
const extraAttempts = 7
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.TargetTypeLog, "",
)
dlv := seedFailedDeliveryWithResponse(t, dbMgr, wh.ID, tgt.ID, "")
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
total := handlers.MaxRenderedAttemptsForTest + extraAttempts
// seedFailedDeliveryWithResponse already recorded one attempt.
for i := range total - 1 {
require.NoError(t, webhookDB.Omit(
clause.Associations,
).Create(&database.DeliveryResult{
DeliveryID: dlv.ID,
AttemptNum: attemptNumber + 1 + i,
Error: attemptError,
}).Error)
}
views := h.LoadEventLogViewsForTest(
httptest.NewRecorder(), *wh, 1,
)
require.Len(t, views, 1)
require.Len(t, views[0].Deliveries, 1)
dv := views[0].Deliveries[0]
assert.Equal(t, total, dv.AttemptCount)
assert.Len(
t, dv.Results, handlers.MaxRenderedAttemptsForTest,
)
assert.Equal(t, extraAttempts, dv.AttemptsOmitted)
page := renderSourceLogsPage(t, h, sess, wh.ID)
assert.Contains(t, page, "attempts omitted")
assert.Contains(
t, page, strconv.Itoa(total)+" attempts",
"the header must count every recorded attempt",
)
}
// TestHandleSourceLogs_BoundsOversizeResponse proves the
// rendered page is bounded by the response cap rather than by
// the stored response size. The cut happens in SQLite, so the
// oversized value never becomes a Go string; this asserts the
// observable consequence, that neither the page nor the
// projection carries the tail.
func TestHandleSourceLogs_BoundsOversizeResponse(t *testing.T) {
t.Parallel()
const tail = "QQRESPONSETAILQQ"
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.TargetTypeLog, "",
)
stored := strings.Repeat("A", responseCap*4) + tail
seedFailedDeliveryWithResponse(t, dbMgr, wh.ID, tgt.ID, stored)
views := h.LoadEventLogViewsForTest(
httptest.NewRecorder(), *wh, 1,
)
require.Len(t, views, 1)
require.Len(t, views[0].Deliveries, 1)
require.Len(t, views[0].Deliveries[0].Results, 1)
attempt := views[0].Deliveries[0].Results[0]
assert.LessOrEqual(
t, len(attempt.ResponseBody), responseCap,
)
assert.Equal(
t, int64(len(stored)), attempt.ResponseBytes,
)
assert.True(t, attempt.ResponseTruncated)
page := renderSourceLogsPage(t, h, sess, wh.ID)
assert.NotContains(t, page, tail)
assert.Contains(
t, page, "Response truncated for display",
)
}

View File

@@ -19,14 +19,6 @@ func (s *Handlers) SetLogForTest(log *slog.Logger) {
// to the handlers_test package.
const MaxRenderedBodyBytesForTest = maxRenderedBodyBytes
// MaxRenderedResponseBytesForTest exposes the event log's
// delivery response cap to the handlers_test package.
const MaxRenderedResponseBytesForTest = maxRenderedResponseBytes
// MaxRenderedAttemptsForTest exposes the event log's
// per-delivery attempt ceiling to the handlers_test package.
const MaxRenderedAttemptsForTest = maxRenderedAttempts
// DummyVerificationsForTest reports how many equivalent-cost
// verifications were charged for usernames that do not exist. It
// lets a test prove the anti-enumeration path ran without timing
@@ -51,9 +43,7 @@ func (s *Handlers) LoadEventLogViewsForTest(
webhook database.Webhook,
page int,
) []EventLogView {
views, _, _ := s.loadEventsWithDeliveries(
w, webhook, nil, page,
)
views, _ := s.loadEventsWithDeliveries(w, webhook, nil, page)
return views
}

View File

@@ -60,7 +60,6 @@ type HandlersParams struct {
Middleware *middleware.Middleware
Notifier delivery.Notifier
Evictor delivery.WebhookEvictor
SSRFGuard *delivery.Guard
}
// Handlers provides HTTP handler methods for all application
@@ -78,11 +77,6 @@ type Handlers struct {
mtr *metrics.Set
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
// charged for usernames that do not exist. It exists so a test
// can prove that path runs without measuring wall-clock time.
@@ -123,7 +117,6 @@ func New(
s.notifier = params.Notifier
s.evictor = params.Evictor
s.mtr = metrics.Default()
s.ssrf = params.SSRFGuard
// Parse all page templates once at startup
s.templates = map[string]*template.Template{

View File

@@ -110,7 +110,6 @@ func newTestApp(
return r
},
middleware.New,
delivery.NewGuard,
handlers.New,
),
fx.Populate(targets...),

View File

@@ -15,11 +15,6 @@ import (
"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
// the webhook's own database, so the log page has a delivery
// to render against the target.
@@ -37,7 +32,7 @@ func seedDeliveredEvent(
WebhookID: webhookID,
Method: http.MethodPost,
Body: `{"test":true}`,
ContentType: contentTypeJSON,
ContentType: "application/json",
}
require.NoError(t, webhookDB.Omit(

View File

@@ -4,13 +4,11 @@ import (
"encoding/json"
"errors"
"net/http"
"slices"
"strconv"
"strings"
"github.com/go-chi/chi"
"github.com/google/uuid"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/signature"
@@ -103,32 +101,6 @@ type DeliveryView struct {
ID string
Status database.DeliveryStatus
Target delivery.TargetView
// Results is this delivery's attempts in attempt order,
// bounded by maxRenderedAttempts. Without them a failure
// renders as the status word alone and says nothing about
// why.
Results []DeliveryResultView
// AttemptCount is how many attempts were recorded, which
// is more than len(Results) once the middle was dropped.
AttemptCount int
// AttemptsOmitted is how many attempts were dropped from
// the middle of Results. The page must show it, or the
// bound would hide history rather than fold it.
AttemptsOmitted int
}
// eventLogTarget is what the event log needs to know about
// one target: the display-safe view its template renders, and
// the redactor that keeps that target's own credential out of
// the text its remote peer chose. The two are kept together
// so a caller cannot pick up one without the other, and apart
// from TargetView so the secrets never reach a template.
type eventLogTarget struct {
View delivery.TargetView
Redactor delivery.Redactor
}
// HandleSourceList shows a list of user's webhooks.
@@ -797,24 +769,12 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
return
}
targets, err := h.loadTargetMap(webhook.ID)
if err != nil {
// Without the map every delivery renders through a
// zero redactor, so failing the page is the only
// safe answer.
h.serverError(w, "failed to load targets", err)
return
}
targets := h.loadTargetMap(webhook.ID)
page := h.parsePage(r)
evts, total, ok := h.loadEventsWithDeliveries(
evts, total := h.loadEventsWithDeliveries(
w, webhook, targets, page,
)
if !ok {
return
}
totalPages := int(total) / paginationPerPage
if int(total)%paginationPerPage != 0 {
@@ -847,54 +807,29 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
}
// loadTargetMap loads targets into a map of display-safe
// views keyed by target ID, each paired with its redactor.
// The projection happens here so that no caller can hand a
// raw target, configuration blob and all, to a template: the
// raw rows do not leave this function.
//
// The load is Unscoped because deleting a target only soft
// deletes the row while its deliveries survive in the
// per-webhook database: a scoped load leaves those deliveries
// with a zero redactor, which renders their response bodies
// unredacted. Only the redactor half of the map is built from
// deleted rows. The view half, which is what the page lists,
// stays scoped.
// views keyed by target ID. The projection happens here so
// that no caller can hand a raw target, configuration blob
// and all, to a template.
func (h *Handlers) loadTargetMap(
webhookID string,
) (map[string]eventLogTarget, error) {
) map[string]delivery.TargetView {
var targets []database.Target
err := h.db.DB().Unscoped().Where(
h.db.DB().Where(
"webhook_id = ?", webhookID,
).Find(&targets).Error
if err != nil {
return nil, err
}
).Find(&targets)
views := delivery.NewTargetViews(targets)
targetMap := make(
map[string]eventLogTarget, len(targets),
map[string]delivery.TargetView, len(views),
)
live := make([]database.Target, 0, len(targets))
for i := range targets {
targetMap[targets[i].ID] = eventLogTarget{
Redactor: delivery.NewRedactor(&targets[i]),
}
if !targets[i].DeletedAt.Valid {
live = append(live, targets[i])
}
for _, v := range views {
targetMap[v.ID] = v
}
// The views come from NewTargetViews rather than being
// rebuilt here, so the masking rules stay in one place.
for _, v := range delivery.NewTargetViews(live) {
entry := targetMap[v.ID]
entry.View = v
targetMap[v.ID] = entry
}
return targetMap, nil
return targetMap
}
// parsePage extracts a page number from the query string.
@@ -915,22 +850,18 @@ func (h *Handlers) parsePage(r *http.Request) int {
// deliveries from the per-webhook database. Events come back
// as capped projections rather than database.Event rows: see
// eventLogColumns for why the cut happens in SQL.
//
// The bool reports whether the load succeeded. It is false
// once this has answered the request with an error, and the
// caller must then render nothing further.
func (h *Handlers) loadEventsWithDeliveries(
w http.ResponseWriter,
webhook database.Webhook,
targetMap map[string]eventLogTarget,
targetMap map[string]delivery.TargetView,
page int,
) ([]EventLogView, int64, bool) {
) ([]EventLogView, int64) {
var totalEvents int64
var result []EventLogView
if !h.dbMgr.DBExists(webhook.ID) {
return result, totalEvents, true
return result, totalEvents
}
webhookDB, err := h.dbMgr.GetDB(webhook.ID)
@@ -939,7 +870,7 @@ func (h *Handlers) loadEventsWithDeliveries(
w, "failed to get webhook database", err,
)
return nil, 0, false
return nil, 0
}
webhookDB.Model(&database.Event{}).Where(
@@ -959,170 +890,43 @@ func (h *Handlers) loadEventsWithDeliveries(
).Find(&rows)
result = make([]EventLogView, len(rows))
eventDeliveries := make([][]database.Delivery, len(rows))
var deliveryIDs []string
for i := range rows {
result[i] = rows[i].view()
var deliveries []database.Delivery
webhookDB.Where(
"event_id = ?", rows[i].ID,
).Find(&eventDeliveries[i])
).Find(&deliveries)
for j := range eventDeliveries[i] {
deliveryIDs = append(
deliveryIDs, eventDeliveries[i][j].ID,
)
}
}
attempts, err := h.loadDeliveryResults(
webhookDB, deliveryIDs,
)
if err != nil {
h.serverError(
w, "failed to load delivery attempts", err,
)
return nil, 0, false
}
for i := range rows {
result[i].Deliveries = newDeliveryViews(
eventDeliveries[i], targetMap, attempts,
deliveries, targetMap,
)
}
return result, totalEvents, true
}
// deliveryIDChunkSize bounds how many delivery IDs go into one
// IN clause. SQLite refuses a statement carrying more than
// SQLITE_MAX_VARIABLE_NUMBER (32766) bound parameters, and a
// page holds one delivery per target per event, so a webhook
// with enough targets would turn the whole query into an error
// and the page into zero attempts.
const deliveryIDChunkSize = 500
// loadDeliveryResults loads the recorded attempts for the
// page's deliveries, keyed by delivery ID.
//
// Each response body is cut by SQLite rather than in Go, for
// the reason deliveryResultColumns gives. How many attempts a
// delivery has is the target's MaxRetries, which the
// authenticated operator sets; how many of them reach the page
// is bounded again by maxRenderedAttempts.
func (h *Handlers) loadDeliveryResults(
webhookDB *gorm.DB,
deliveryIDs []string,
) (map[string][]deliveryResultRow, error) {
byDelivery := make(map[string][]deliveryResultRow)
for chunk := range slices.Chunk(
deliveryIDs, deliveryIDChunkSize,
) {
var rows []deliveryResultRow
err := webhookDB.Model(
&database.DeliveryResult{},
).Select(
deliveryResultColumns, maxRenderedResponseBytes,
).Where(
"delivery_id IN ?", chunk,
).Order("attempt_num ASC").Find(&rows).Error
if err != nil {
// Returning what was loaded so far renders the
// deliveries in the failed chunk as never having run,
// which is indistinguishable from ones that really
// never ran. The page fails instead.
return nil, err
}
for i := range rows {
byDelivery[rows[i].DeliveryID] = append(
byDelivery[rows[i].DeliveryID], rows[i],
)
}
}
return byDelivery, nil
return result, totalEvents
}
// newDeliveryViews projects deliveries for rendering,
// resolving each one's target to its display-safe view and
// each one's attempts through that target's redactor.
// resolving each one's target to its display-safe view.
func newDeliveryViews(
deliveries []database.Delivery,
targetMap map[string]eventLogTarget,
attempts map[string][]deliveryResultRow,
targetMap map[string]delivery.TargetView,
) []DeliveryView {
views := make([]DeliveryView, len(deliveries))
for i := range deliveries {
target := targetMap[deliveries[i].TargetID]
rows := attempts[deliveries[i].ID]
results, omitted := renderedAttempts(
rows, target.Redactor,
)
views[i] = DeliveryView{
ID: deliveries[i].ID,
Status: deliveries[i].Status,
Target: target.View,
Results: results,
AttemptCount: len(rows),
AttemptsOmitted: omitted,
ID: deliveries[i].ID,
Status: deliveries[i].Status,
Target: targetMap[deliveries[i].TargetID],
}
}
return views
}
// maxRenderedAttempts bounds how many of one delivery's
// attempts the page renders. Past it the middle is dropped and
// counted, keeping the first attempts and the last ones: how
// the delivery started failing and how it ended are what a
// reader needs, and the count says plainly that the rest was
// dropped rather than never recorded.
const (
renderedAttemptsHead = 10
renderedAttemptsTail = 10
maxRenderedAttempts = renderedAttemptsHead +
renderedAttemptsTail
)
// renderedAttempts projects a delivery's attempts through the
// target's redactor, at most maxRenderedAttempts of them, and
// reports how many it dropped.
func renderedAttempts(
rows []deliveryResultRow,
redactor delivery.Redactor,
) ([]DeliveryResultView, int) {
omitted := 0
if len(rows) > maxRenderedAttempts {
omitted = len(rows) - maxRenderedAttempts
kept := make(
[]deliveryResultRow, 0, maxRenderedAttempts,
)
kept = append(kept, rows[:renderedAttemptsHead]...)
kept = append(
kept, rows[len(rows)-renderedAttemptsTail:]...,
)
rows = kept
}
views := make([]DeliveryResultView, len(rows))
for i := range rows {
views[i] = rows[i].view(redactor)
}
return views, omitted
}
// HandleEntrypointCreate handles adding a new entrypoint.
func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
@@ -1607,7 +1411,7 @@ func (h *Handlers) validateTargetURL(
return errMissingURL
}
err := h.ssrf.ValidateTargetURL(
err := delivery.ValidateTargetURL(
r.Context(), targetURL,
)
if err != nil {

View File

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

View File

@@ -113,7 +113,6 @@ func newTestEnvWithConfig(
func() delivery.Notifier { return &noopNotifier{} },
func() delivery.WebhookEvictor { return &noopEvictor{} },
middleware.New,
delivery.NewGuard,
handlers.New,
),
fx.Populate(&log, &mw, &hnd, &sess, &db, &dbMgr),

File diff suppressed because one or more lines are too long

View File

@@ -46,64 +46,19 @@
{{end}}
{{if .Deliveries}}
<div class="mt-4 border-t border-gray-200 pt-3">
<h3 class="text-xs font-medium uppercase tracking-wide text-gray-500">Deliveries</h3>
<div class="mt-2 divide-y divide-gray-200">
{{range .Deliveries}}
<div class="py-2" x-data="{ attempts: false }">
<div class="flex items-center justify-between cursor-pointer" @click="attempts = !attempts">
<div class="flex items-center gap-3">
<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>
</div>
<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>
<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"/>
</svg>
</div>
</div>
<div x-show="attempts" x-cloak class="mt-2 space-y-2">
{{if .AttemptsOmitted}}
<p class="text-xs text-gray-500">{{.AttemptsOmitted}} attempt{{if ne .AttemptsOmitted 1}}s{{end}} omitted between the first and last shown.</p>
{{end}}
{{range .Results}}
<div class="rounded-md bg-white border border-gray-200 p-2">
<div class="flex flex-wrap items-center gap-3 text-xs">
<span class="text-gray-500">Attempt {{.AttemptNum}}</span>
<span class="{{if .Success}}text-green-600{{else}}text-red-600{{end}}">{{if .Success}}success{{else}}failure{{end}}</span>
<span class="text-gray-500">Status: {{if .HasStatusCode}}{{.StatusCode}}{{else}}&mdash; (no response){{end}}</span>
<span class="text-gray-500">Duration: {{.DurationMS}} ms</span>
</div>
{{if .Error}}
<p class="mt-2 text-xs text-red-700 break-all">Error: {{.Error}}</p>
{{end}}
{{if .ResponseBody}}
<pre class="mt-2 text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all">{{.ResponseBody}}</pre>
{{end}}
{{if .ResponseTruncated}}
{{if .ResponseSizeKnown}}
<p class="mt-1 text-xs text-gray-500">Response truncated for display: showing {{.ResponseShownBytes}} of {{.ResponseBytes}} bytes.</p>
{{else}}
<p class="mt-1 text-xs text-gray-500">Showing {{.ResponseShownBytes}} of the {{.ResponseBytes}} recorded bytes. The response reached the recording limit, so the remote may have sent more that was never stored.</p>
{{end}}
{{end}}
</div>
{{else}}
<p class="text-xs text-gray-500">No attempts recorded yet.</p>
{{end}}
</div>
</div>
<div class="mt-3 pt-4 border-t border-gray-200">
{{range .Deliveries}}
<div class="flex items-center justify-between py-2 text-xs text-gray-700">
<span><span class="font-medium">{{.Target.Name}}</span>: {{.Status}}</span>
{{if .Status.Terminal}}
<form method="POST" action="/source/{{$.Webhook.ID}}/deliveries/{{.ID}}/replay" class="inline">
<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}}
</div>
{{end}}
</div>
{{end}}
</div>

View File

@@ -39,7 +39,7 @@
<div class="form-group">
<label for="headers" class="label">Headers</label>
<textarea id="headers" name="headers" rows="4" class="input" placeholder="Authorization: Bearer ...">{{.Target.Config.Headers}}</textarea>
<p class="text-xs text-gray-500 mt-1">One <code>Name: value</code> per line, sent with every delivery. Leave blank for none. <code>Host</code>, <code>Content-Length</code>, <code>Transfer-Encoding</code>, <code>Connection</code>, <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>
<p class="text-xs text-gray-500 mt-1">One <code>Name: value</code> per line, sent with every delivery. Leave blank for none. <code>Host</code>, <code>Content-Length</code>, <code>Transfer-Encoding</code>, <code>Connection</code> and <code>User-Agent</code> are set by the delivery engine and are rejected here rather than silently ignored.</p>
</div>
<div class="form-group">