Harden operator-set target headers (closes #233)

Three findings from the review of the per-target request headers
feature, plus the follow-up they raised about the inbound headers
the same delivery path forwards.

One rule now governs every header a delivery carries on someone
else's behalf: a redirect hop that leaves the origin the target
names carries none of them. That covers the operator's configured
headers and the inbound event headers forwarded from the sender
alike. net/http withholds only Authorization and Cookie across a
host change, so an operator's X-Api-Key or a sender's
X-Hub-Signature would otherwise follow a 302 to a host nobody
configured. Redirects are still followed — refusing them would
break every destination that legitimately redirects and would
record the 3xx as the delivery's result — but a hop to another
host, another port, or down from https to http drops the lot. The
shared SSRF-safe transport is kept on that client, so each hop is
still dialled through the private-IP guard.

The set to strip is not a name list. applyRequestHeaders now
returns the canonical names of everything it applied on the
sender's or operator's behalf, and the redirect policy strips
exactly that, so a header added to the forward set is covered
without a second edit. Content-Type and User-Agent are the
delivery path's own and always travel; a 307 preserves the body
across hosts and it has to stay typed.

The origin comparison no longer collapses two IPv6 origins into
one. Hostname() unwraps a literal's brackets, so re-appending the
port with a bare colon rendered https://[2001:db8::1]:8080 and
https://[2001:db8::1:8080] identically — a different address on a
different port passing as the same origin. The port is joined with
net.JoinHostPort, and both spellings are in TestSameDeliveryOrigin.

The ten-hop cap gains a regression test. Installing a CheckRedirect
is precisely what discards net/http's own limit, so a
self-redirecting destination is driven through the policy and
asserted to stop after exactly ten requests with the sentinel
surfacing to the caller.

Trailer joins the reserved names. net/http strips it from the
request it writes, so a configured one was accepted, stored, and
provably never sent.

The invalid-header-name error no longer quotes the text before the
first colon. That text is only a name if it parses as one; when it
does not, a pasted value whose own colon split the line put half a
token into a 400 body. TestParseTargetHeaders_ErrorsNeverQuoteAValue
asserted this invariant while only exercising the after-the-colon
case, and now covers the before-the-colon one.

README documents the http target's config keys, the 300-second
timeout ceiling, the reserved-header list and the redirect
behaviour as one rule over both header classes, including that the
drop is per hop rather than permanent: net/http re-copies the
initial request's headers each hop, so a chain returning to the
configured origin carries them again, exactly as it treats
Authorization. The edit form's hint gains Trailer and the redirect
note.

Closes #243

README.md

@@ -1049,6 +1049,48 @@ 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. 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.

internal/delivery/client_ssrf_test.go

@@ -25,12 +25,12 @@ func newSSRFTestEngine() *delivery.Engine {
 	return delivery.NewTestEngine(log, client, 1)
 }
 
-// TestClientForConfig_TimeoutKeepsSSRFGuard asserts that a
-// client returned by clientForConfig for a config with a
+// TestClientForRequest_TimeoutKeepsSSRFGuard asserts that a
+// client returned by clientForRequest 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 TestClientForConfig_TimeoutKeepsSSRFGuard(t *testing.T) {
+func TestClientForRequest_TimeoutKeepsSSRFGuard(t *testing.T) {
 	t.Parallel()
 
 	engine := newSSRFTestEngine()
@@ -50,7 +50,7 @@ func TestClientForConfig_TimeoutKeepsSSRFGuard(t *testing.T) {
 				Timeout: 5,
 			}
 
-			client := engine.ExportClientForConfig(cfg)
+			client := engine.ExportClientForRequest(cfg, nil)
 
 			require.NotSame(t, engine.ExportClient(), client,
 				"a per-target timeout must yield a "+
@@ -91,10 +91,11 @@ func TestClientForConfig_TimeoutKeepsSSRFGuard(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) {
+// 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) {
 	t.Parallel()
 
 	engine := newSSRFTestEngine()
@@ -103,10 +104,46 @@ func TestClientForConfig_NoTimeoutUnchanged(t *testing.T) {
 		URL: "https://example.com/hook",
 	}
 
-	client := engine.ExportClientForConfig(cfg)
+	client := engine.ExportClientForRequest(cfg, nil)
 
 	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",
+	)
+}

internal/delivery/export_test.go

@@ -5,6 +5,7 @@ import (
 	"log/slog"
 	"net"
 	"net/http"
+	"net/url"
 	"time"
 
 	"go.uber.org/fx"
@@ -49,13 +50,14 @@ 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,
-) {
-	applyRequestHeaders(req, event, cfg)
+) []string {
+	return applyRequestHeaders(req, event, cfg)
 }
 
 // ExportTruncate exposes truncate for testing.
@@ -165,12 +167,27 @@ func (e *Engine) ExportDoHTTPRequest(
 	return e.httpTarget.doHTTPRequest(ctx, cfg, event)
 }
 
-// ExportClientForConfig exposes the http target's
-// clientForConfig.
-func (e *Engine) ExportClientForConfig(
+// ExportClientForRequest exposes the http target's
+// clientForRequest.
+func (e *Engine) ExportClientForRequest(
 	cfg *HTTPTargetConfig,
+	originScoped []string,
 ) *http.Client {
-	return e.httpTarget.clientForConfig(cfg)
+	return e.httpTarget.clientForRequest(cfg, originScoped)
+}
+
+// ErrExportTooManyRedirects exposes the sentinel the redirect
+// policy returns once a chain exceeds the hop cap. It carries the
+// Err prefix rather than this file's usual Export one because it
+// is a sentinel error.
+var ErrExportTooManyRedirects = errTooManyRedirects
+
+// ExportMaxDeliveryRedirects exposes the redirect hop cap.
+const ExportMaxDeliveryRedirects = maxDeliveryRedirects
+
+// ExportSameDeliveryOrigin exposes sameDeliveryOrigin.
+func ExportSameDeliveryOrigin(origin, dest *url.URL) bool {
+	return sameDeliveryOrigin(origin, dest)
 }
 
 // ExportClient returns the http target's shared HTTP client.

internal/delivery/redirect.go

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

internal/delivery/redirect_test.go

@@ -0,0 +1,380 @@
+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 Content-Type is deliberately excluded: it
+// describes the body, which a 307 carries across hosts.
+func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
+	t.Parallel()
+
+	inbound, err := json.Marshal(map[string][]string{
+		inboundHeaderName: {inboundHeaderValue},
+		"Content-Type":    {testContentType},
+		"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",
+	)
+}

internal/delivery/target_headers.go

@@ -65,6 +65,11 @@ 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
 	}
@@ -118,9 +123,11 @@ func parseHeaderLine(line string) (string, string, error) {
 
 	rawName = strings.TrimSpace(rawName)
 	if !validHeaderName(rawName) {
-		return "", "", fmt.Errorf(
-			"%w: %q", errHeaderNameInvalid, 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
 	}
 
 	name := http.CanonicalHeaderKey(rawName)

internal/delivery/target_headers_test.go

@@ -82,6 +82,16 @@ func TestParseTargetHeaders_Rejects(t *testing.T) {
 	}
 }
 
+// net/http strips Trailer from the request it writes, so accepting
+// one would store a header that never reaches the target.
+func TestParseTargetHeaders_RejectsTrailer(t *testing.T) {
+	t.Parallel()
+
+	_, err := delivery.ParseTargetHeaders("Trailer: X-Checksum")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "Trailer")
+}
+
 // A header value is routinely a bearer token and these errors are
 // rendered into a 400 body, so no message may quote one.
 func TestParseTargetHeaders_ErrorsNeverQuoteAValue(t *testing.T) {
@@ -89,17 +99,26 @@ func TestParseTargetHeaders_ErrorsNeverQuoteAValue(t *testing.T) {
 
 	const secret = "QQNEVERINAMESSAGEQQ"
 
-	_, err := delivery.ParseTargetHeaders(
+	inputs := []string{
+		// The value, after the colon, in a duplicate name.
 		"X-A: " + secret + "\nx-a: " + secret,
-	)
-	require.Error(t, err)
-	assert.NotContains(t, err.Error(), secret)
-
-	_, err = delivery.ParseTargetHeaders(
+		// The value after the colon of an unusable name.
 		"X Bad Name: " + secret,
-	)
-	require.Error(t, err)
-	assert.NotContains(t, err.Error(), 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",
+	}
+
+	for _, input := range inputs {
+		_, err := delivery.ParseTargetHeaders(input)
+		require.Error(t, err, input)
+		assert.NotContains(t, err.Error(), secret, input)
+	}
 }
 
 // Loading the edit form twice without saving must not reshuffle

internal/delivery/target_http.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"sort"
 	"sync"
 	"time"
 
@@ -404,9 +405,9 @@ func (t *httpTarget) doHTTPRequest(
 		)
 	}
 
-	applyRequestHeaders(req, event, cfg)
+	originScoped := applyRequestHeaders(req, event, cfg)
 
-	client := t.clientForConfig(cfg)
+	client := t.clientForRequest(cfg, originScoped)
 
 	resp, doErr := executeHTTPRequest(client, req)
 
@@ -432,23 +433,41 @@ func (t *httpTarget) doHTTPRequest(
 	return resp.StatusCode, string(body), dur, nil
 }
 
-func (t *httpTarget) clientForConfig(
+// clientForRequest returns the client for one delivery attempt.
+// originScoped is the header set applyRequestHeaders built for that
+// attempt; a request with neither a per-target timeout nor an
+// origin-scoped header gets the shared client, because there is
+// then nothing for the redirect policy to strip and net/http's
+// default policy already withholds Authorization and Cookie across
+// hosts.
+func (t *httpTarget) clientForRequest(
 	cfg *HTTPTargetConfig,
+	originScoped []string,
 ) *http.Client {
-	if cfg.Timeout > 0 {
-		// 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 cfg.Timeout <= 0 && len(originScoped) == 0 {
+		return t.client
 	}
 
-	return t.client
+	// Reuse the shared client's SSRF-safe transport so neither a
+	// per-target timeout nor the redirect policy drops the
+	// request-time private-IP guard — which, being a dial hook,
+	// also covers every redirect hop.
+	client := &http.Client{
+		Timeout:   t.client.Timeout,
+		Transport: t.client.Transport,
+	}
+
+	if cfg.Timeout > 0 {
+		client.Timeout = time.Duration(
+			cfg.Timeout,
+		) * time.Second
+	}
+
+	if len(originScoped) > 0 {
+		client.CheckRedirect = offOriginHeaderPolicy(originScoped)
+	}
+
+	return client
 }
 
 func parseHTTPConfig(
@@ -490,40 +509,82 @@ 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,
 		)
 	}
 
-	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)
-					}
-				}
-			}
-		}
-	}
+	originScoped := forwardEventHeaders(req, event)
 
 	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")
+
+	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

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> and <code>User-Agent</code> are set by the delivery engine and are rejected here rather than silently ignored.</p>
+                <p class="text-xs text-gray-500 mt-1">One <code>Name: value</code> per line, sent with every delivery. Leave blank for none. <code>Host</code>, <code>Content-Length</code>, <code>Transfer-Encoding</code>, <code>Connection</code>, <code>Trailer</code> and <code>User-Agent</code> are set by the delivery engine and are rejected here rather than silently ignored. Headers set here are dropped if a redirect leaves the destination's own origin, so a credential cannot follow one to another host.</p>
             </div>
 
             <div class="form-group">