Add an egress CIDR allowlist to the SSRF guard (closes #204 )

The SSRF blocklist had no escape hatch, so the thing webhooker is mostly for — taking a public webhook and forwarding it to something on your own network — could not be configured at all. Every private address, Docker sibling and loopback service was permanently unreachable as a delivery destination. ALLOWED_EGRESS_CIDRS (default empty) names blocks that delivery targets may reach despite the default blocklist. It is an allowlist and only ever adds destinations: there is no boolean, and no value disables SSRF protection wholesale. Empty, it adds nothing and the guard permits and refuses the same addresses it did before, save the two spellings named below. A small set of addresses is refused before the allowlist is consulted, so no supplied CIDR opens one — not the exact address, not a supernet, not 0.0.0.0/0 or ::/0. alwaysBlockedNetworks in internal/delivery/ssrf.go is the authoritative list and states the membership criterion in full; it is deliberately not copied here, because a copy drifts out of date. In short: the provider fixes the address, so a host route for it collides with nothing the operator runs, and reaching it discloses credentials or user data. A publicly routable address never qualifies however well it meets both — nothing in this set can be reopened, so blocking one here would leave the operator no escape hatch at all. Those belong in blockedNetworks, which an allowlist can override. Every entry is already inside the default blocklist, which is what makes it unconditional rather than newly blocked, with two exceptions that are the one behaviour change visible when the allowlist is unset: ::a9fe:a9fe and 64:ff9b::a9fe:a9fe, the IPv4-compatible and NAT64 spellings of 169.254.169.254, were reachable before and are refused now. net.IPNet.Contains normalises only the IPv4-mapped form via To4(), so 169.254.0.0/16 never matched those two. The ten it does cover now report a metadata error rather than the generic private-range one. The policy now lives in one function, Guard.checkIP, which both target-creation validation and the delivery dialer call. The two paths previously decided separately, which is how they came to disagree about a destination. The guard is built once from config and injected via fx into both the handlers and the delivery engine, so there is a single instance and a single answer. A set-but-unparseable value aborts startup naming the variable, reusing the existing envPrefixList parser. A non-empty list is logged at startup with the blocks spelled out, not counted, so the hole is visible in the log of any deployment that has one. Tests: an allowlisted loopback CIDR both validates and delivers to a live server (and the same URL still fails without the allowlist); a private address outside the listed block stays refused on both paths; every unconditionally blocked address stays refused on both paths under an allowlist that covers it, and the set itself is pinned entry by entry; public addresses are unaffected either way; and config coverage for parsing, startup abort, and the warning's contents.
2026-08-20 08:09:12 +00:00
10 changed files with 133 additions and 802 deletions
--- a/README.md
+++ b/README.md
@@ -118,10 +118,12 @@ TTY detection, and security headers are always applied.
 #### Allowing egress to your own network
-By default every delivery target must resolve to a public address. The
+By default every delivery target must resolve to a public address, and
-private and reserved ranges — RFC 1918, loopback, CGNAT, link-local and
+a handful of public ones are refused too. The private and reserved
-the rest — are refused, which stops a target from being used to make
+ranges — RFC 1918, loopback, CGNAT, link-local and the rest — are
-webhooker probe the network it sits in.
+refused, which stops a target from being used to make webhooker probe
 the network it sits in; so are the cloud metadata endpoints listed
 below that happen to live on public addresses.
 That default is also inconvenient for the thing webhooker is mostly
 for: taking a public webhook and forwarding it to something on your own
@@ -1160,53 +1162,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.
@@ -2496,9 +2451,9 @@ check, see [The login endpoint](#the-login-endpoint).
  [`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 —
+  metadata endpoints — several of which are ULAs or public addresses
-  stay blocked whatever is listed, though listing `0.0.0.0/0` or
+  outside link-local — stay blocked whatever is listed, though listing
-  `::/0` does open every other private range
+  `0.0.0.0/0` or `::/0` does open every other private range
 - **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
--- a/TODO.md
+++ b/TODO.md
@@ -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
+pre-1.0. No git tags exist. `main` (4f5ecb1) is a working webhook proxy
-ahead of `main` and a strict fast-forward. No git tags exist yet.
+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
+`next` is green — verified both by CI and by cache-defeated container
-use it in low-volume production". Every gap the deployability audit
+runs (`docker build --no-cache-filter=lint --no-cache-filter=builder`) —
-named against that bar is now closed:
+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
+The bar for 1.0 is not "the milestone is empty" but "sneak can deploy
-  (https://git.eeqj.de/sneak/webhooker/issues/201)
+this and use it in low-volume production". The audit found the gap
- shutdown on listener failure, rather than a live non-serving process
+between those two: two instances on one `DATA_DIR` both deliver
-  (https://git.eeqj.de/sneak/webhooker/issues/200)
+(reproduced), a failed listen leaves a live non-serving process that
- inbound signature verification
+restart policies never fire on, there is no inbound authentication of
-  (https://git.eeqj.de/sneak/webhooker/issues/67)
+any kind, delivery failures render as a bare word with no status code or
- per-attempt delivery detail in the event log
+error, a terminally failed delivery can never be replayed, the SSRF
-  (https://git.eeqj.de/sneak/webhooker/issues/202)
+blocklist has no escape hatch so the proxy cannot forward to your own
- replay of a terminally failed delivery
+network at all, and target credentials leak into the per-webhook event
-  (https://git.eeqj.de/sneak/webhooker/issues/203)
+databases.
 - `ALLOWED_EGRESS_CIDRS`, an allowlist escape hatch for the SSRF guard
  (https://git.eeqj.de/sneak/webhooker/issues/204)
 - the three credential exposures
  (https://git.eeqj.de/sneak/webhooker/issues/205,
  https://git.eeqj.de/sneak/webhooker/issues/206,
  https://git.eeqj.de/sneak/webhooker/issues/207)
-One caveat on reading a green check: a docs-only commit deliberately
+One caveat on reading a green check, narrower than it used to be. A
-replays from the layer cache
+docs-only commit deliberately replays from the layer cache (#119), so a
-(https://git.eeqj.de/sneak/webhooker/issues/119), so a green status on
+green status on such a commit evidences a replay rather than an executed
-such a commit evidences a replay rather than an executed run. A code
+run; a code commit invalidates the `COPY` layer and genuinely executes.
-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)
+Clear the reopened 1.0.0 milestone. The milestone PR
-and tag `v1.0.0`. It is `merge-ready` and assigned to sneak; nothing
+(https://git.eeqj.de/sneak/webhooker/pulls/111) is held: it carries a
-else gates it.
+`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:
+Three items belong to the owner, none of them blocking. #150 was decided
-https://git.eeqj.de/sneak/webhooker/issues/245,
+by the manager rather than left to stall the queue and is flagged on the
-https://git.eeqj.de/sneak/webhooker/issues/246,
+issue for reversal if that call was wrong. #112 (whether `Completed
-https://git.eeqj.de/sneak/webhooker/issues/247 and
+Steps` should exist at all, given it once conflicted on every unit) is
-https://git.eeqj.de/sneak/webhooker/issues/248. Also still open and
+unanswered; the provisional ruling in force is that issue branches do
-unmilestoned: https://git.eeqj.de/sneak/webhooker/issues/193 (a design
+not touch this file. #198 records that `make test` is past the org 20s
-question, not a defect), https://git.eeqj.de/sneak/webhooker/issues/198
+target — 46s of test execution inside a 62.8s CI layer — and turns on
-(`make test` is past the org 20s target) and
+which quantity the 60s hard cap governs; it is scoped as the improvement
-https://git.eeqj.de/sneak/webhooker/issues/212 (encrypting target config
+bug the 20-60s band requires, and should be milestoned instead if the
-at rest).
+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
+- Manual event redelivery from the web UI — the "Replay" capability the
-  failed delivery and per-attempt detail already landed
+  README describes as planned. No redelivery code exists anywhere in the
-  (https://git.eeqj.de/sneak/webhooker/issues/203,
+  tree; events are stored in full, which is all it would be built on
-  https://git.eeqj.de/sneak/webhooker/issues/202)
+- 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
+- Webhook signature verification for GitHub and Stripe HMAC formats
  landed with inbound verification
  (https://git.eeqj.de/sneak/webhooker/issues/67)
 - 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
+- Password reset flow for a forgotten password. The authenticated
-  authenticated password *change* flow already landed, and a lost
+  password *change* flow already landed on `main` (#65); reset does not
-  password is recoverable from the console with `webhooker resetpw`
+  exist
  (https://git.eeqj.de/sneak/webhooker/issues/208)
 - Later, nice to have
    - email delivery target type
    - SNS and S3 delivery targets
--- a/internal/delivery/client_ssrf_test.go
+++ b/internal/delivery/client_ssrf_test.go
@@ -26,12 +26,12 @@ func newSSRFTestEngine() *delivery.Engine {
 	return delivery.NewTestEngine(log, client, 1)
 }
-// TestClientForRequest_TimeoutKeepsSSRFGuard asserts that a
+// TestClientForConfig_TimeoutKeepsSSRFGuard asserts that a
-// client returned by clientForRequest for a config with 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()
@@ -51,7 +51,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 +92,10 @@ func TestClientForRequest_TimeoutKeepsSSRFGuard(t *testing.T) {
 	}
 }
-// TestClientForRequest_NoTimeoutUnchanged asserts that a
+// TestClientForConfig_NoTimeoutUnchanged asserts that with
-// request with neither a per-target timeout nor an origin-scoped
+// no per-target timeout the shared SSRF-safe client is
-// header gets the shared SSRF-safe client unchanged: there is then
+// returned unchanged.
-// nothing for a redirect policy to strip.
+func TestClientForConfig_NoTimeoutUnchanged(t *testing.T) {
 func TestClientForRequest_NoTimeoutUnchanged(t *testing.T) {
 	t.Parallel()
 	engine := newSSRFTestEngine()
@@ -105,46 +104,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",
 	)
 }
--- a/internal/delivery/export_test.go
+++ b/internal/delivery/export_test.go
@@ -6,7 +6,6 @@ import (
 	"net"
 	"net/http"
 	"net/netip"
 	"net/url"
 	"time"
 	"go.uber.org/fx"
@@ -71,14 +70,13 @@ func ExportIsForwardableHeader(name string) bool {
 }
 // ExportApplyRequestHeaders exposes applyRequestHeaders, so a test
-// can inspect the header set an outbound delivery actually carries
+// can inspect the header set an outbound delivery actually carries.
 // and the origin-scoped names it reports for the redirect policy.
 func ExportApplyRequestHeaders(
 	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 +186,12 @@ func (e *Engine) ExportDoHTTPRequest(
 	return e.httpTarget.doHTTPRequest(ctx, cfg, event)
 }
-// ExportClientForRequest exposes the http target's
+// ExportClientForConfig exposes the http target's
-// clientForRequest.
+// clientForConfig.
-func (e *Engine) ExportClientForRequest(
+func (e *Engine) ExportClientForConfig(
 	cfg *HTTPTargetConfig,
 	originScoped []string,
 ) *http.Client {
-	return e.httpTarget.clientForRequest(cfg, originScoped)
+	return e.httpTarget.clientForConfig(cfg)
 }
 // ErrExportTooManyRedirects exposes the sentinel the redirect
 // policy returns once a chain exceeds the hop cap. It carries the
 // Err prefix rather than this file's usual Export one because it
 // is a sentinel error.
 var ErrExportTooManyRedirects = errTooManyRedirects
 // ExportMaxDeliveryRedirects exposes the redirect hop cap.
 const ExportMaxDeliveryRedirects = maxDeliveryRedirects
 // ExportSameDeliveryOrigin exposes sameDeliveryOrigin.
 func ExportSameDeliveryOrigin(origin, dest *url.URL) bool {
 	return sameDeliveryOrigin(origin, dest)
 }
 // ExportClient returns the http target's shared HTTP client.
--- a/internal/delivery/redirect.go
+++ b/internal/delivery/redirect.go
@@ -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)
 }
--- a/internal/delivery/redirect_test.go
+++ b/internal/delivery/redirect_test.go
@@ -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",
 	)
 }
--- a/internal/delivery/target_headers.go
+++ b/internal/delivery/target_headers.go
@@ -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
+		return "", "", fmt.Errorf(
-		// a name if it parses as one; when it does not, it is as
+			"%w: %q", errHeaderNameInvalid, rawName,
-		// likely to be a pasted value whose own colon split the
+		)
 		// line, and half of a token would be echoed into the 400.
 		return "", "", errHeaderNameInvalid
 	}
 	name := http.CanonicalHeaderKey(rawName)
--- a/internal/delivery/target_headers_test.go
+++ b/internal/delivery/target_headers_test.go
@@ -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{
+	_, err := delivery.ParseTargetHeaders(
 		// The value, after the colon, in a duplicate name.
 		"X-A: " + secret + "\nx-a: " + secret,
-		// The value after the colon of an unusable name.
+	)
-		"X Bad Name: " + secret,
+	require.Error(t, err)
-		// The line splits on the value's own colon, so the
+	assert.NotContains(t, err.Error(), secret)
 		// 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",
 	}
-	for _, input := range inputs {
+	_, err = delivery.ParseTargetHeaders(
-		_, err := delivery.ParseTargetHeaders(input)
+		"X Bad Name: " + secret,
-		require.Error(t, err, input)
+	)
-		assert.NotContains(t, err.Error(), secret, input)
+	require.Error(t, err)
-	}
+	assert.NotContains(t, err.Error(), secret)
 }
 // Loading the edit form twice without saving must not reshuffle
--- a/internal/delivery/target_http.go
+++ b/internal/delivery/target_http.go
@@ -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.
+func (t *httpTarget) clientForConfig(
 // originScoped is the header set applyRequestHeaders built for that
 // attempt; a request with neither a per-target timeout nor an
 // origin-scoped header gets the shared client, because there is
 // then nothing for the redirect policy to strip and net/http's
 // default policy already withholds Authorization and Cookie across
 // hosts.
 func (t *httpTarget) clientForRequest(
 	cfg *HTTPTargetConfig,
 	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(
+		// Reuse the shared client's SSRF-safe transport so
-			cfg.Timeout,
+		// a per-target timeout does not drop the
-		) * time.Second
+		// 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 {
+	return t.client
 		client.CheckRedirect = offOriginHeaderPolicy(originScoped)
 	}
 	return 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
--- a/templates/target_edit.html
+++ b/templates/target_edit.html
@@ -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">