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/engine_integration_test.go

@@ -1201,6 +1201,83 @@ 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(

internal/delivery/export_test.go

@@ -5,6 +5,7 @@ import (
 	"log/slog"
 	"net"
 	"net/http"
+	"net/url"
 	"time"
 
 	"go.uber.org/fx"
@@ -25,6 +26,12 @@ 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.
@@ -43,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.
@@ -159,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

internal/delivery/target_redact.go

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

internal/delivery/target_redact_test.go

@@ -0,0 +1,284 @@
+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,
+		)
+	}
+}

internal/handlers/delivery_replay_test.go

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

internal/handlers/delivery_result_view.go

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

internal/handlers/delivery_result_view_test.go

@@ -0,0 +1,515 @@
+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",
+	)
+}

internal/handlers/export_test.go

@@ -19,6 +19,14 @@ 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
@@ -43,7 +51,9 @@ 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
 }

internal/handlers/source_logs_test.go

@@ -15,6 +15,11 @@ 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.
@@ -32,7 +37,7 @@ func seedDeliveredEvent(
 		WebhookID:   webhookID,
 		Method:      http.MethodPost,
 		Body:        `{"test":true}`,
-		ContentType: "application/json",
+		ContentType: contentTypeJSON,
 	}
 
 	require.NoError(t, webhookDB.Omit(

internal/handlers/source_management.go

@@ -4,11 +4,13 @@ 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"
@@ -101,6 +103,32 @@ 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.
@@ -769,12 +797,24 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 			return
 		}
 
-		targets := h.loadTargetMap(webhook.ID)
+		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
+		}
+
 		page := h.parsePage(r)
 
-		evts, total := h.loadEventsWithDeliveries(
+		evts, total, ok := h.loadEventsWithDeliveries(
 			w, webhook, targets, page,
 		)
+		if !ok {
+			return
+		}
 
 		totalPages := int(total) / paginationPerPage
 		if int(total)%paginationPerPage != 0 {
@@ -807,29 +847,54 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 }
 
 // loadTargetMap loads targets into a map of display-safe
-// 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.
+// 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.
 func (h *Handlers) loadTargetMap(
 	webhookID string,
-) map[string]delivery.TargetView {
+) (map[string]eventLogTarget, error) {
 	var targets []database.Target
 
-	h.db.DB().Where(
+	err := h.db.DB().Unscoped().Where(
 		"webhook_id = ?", webhookID,
-	).Find(&targets)
-
-	views := delivery.NewTargetViews(targets)
-
-	targetMap := make(
-		map[string]delivery.TargetView, len(views),
-	)
-
-	for _, v := range views {
-		targetMap[v.ID] = v
+	).Find(&targets).Error
+	if err != nil {
+		return nil, err
 	}
 
-	return targetMap
+	targetMap := make(
+		map[string]eventLogTarget, len(targets),
+	)
+	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])
+		}
+	}
+
+	// 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
 }
 
 // parsePage extracts a page number from the query string.
@@ -850,18 +915,22 @@ 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]delivery.TargetView,
+	targetMap map[string]eventLogTarget,
 	page int,
-) ([]EventLogView, int64) {
+) ([]EventLogView, int64, bool) {
 	var totalEvents int64
 
 	var result []EventLogView
 
 	if !h.dbMgr.DBExists(webhook.ID) {
-		return result, totalEvents
+		return result, totalEvents, true
 	}
 
 	webhookDB, err := h.dbMgr.GetDB(webhook.ID)
@@ -870,7 +939,7 @@ func (h *Handlers) loadEventsWithDeliveries(
 			w, "failed to get webhook database", err,
 		)
 
-		return nil, 0
+		return nil, 0, false
 	}
 
 	webhookDB.Model(&database.Event{}).Where(
@@ -890,43 +959,170 @@ 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(&deliveries)
+		).Find(&eventDeliveries[i])
 
+		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(
-			deliveries, targetMap,
+			eventDeliveries[i], targetMap, attempts,
 		)
 	}
 
-	return result, totalEvents
+	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
 }
 
 // newDeliveryViews projects deliveries for rendering,
-// resolving each one's target to its display-safe view.
+// resolving each one's target to its display-safe view and
+// each one's attempts through that target's redactor.
 func newDeliveryViews(
 	deliveries []database.Delivery,
-	targetMap map[string]delivery.TargetView,
+	targetMap map[string]eventLogTarget,
+	attempts map[string][]deliveryResultRow,
 ) []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: targetMap[deliveries[i].TargetID],
+			Target:          target.View,
+			Results:         results,
+			AttemptCount:    len(rows),
+			AttemptsOmitted: omitted,
 		}
 	}
 
 	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) {

templates/source_logs.html

@@ -46,20 +46,65 @@
                     {{end}}
 
                     {{if .Deliveries}}
-                    <div class="mt-3 pt-4 border-t border-gray-200">
+                    <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="flex items-center justify-between py-2 text-xs text-gray-700">
-                            <span><span class="font-medium">{{.Target.Name}}</span>: {{.Status}}</span>
+                            <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">
+                                        <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>
                             {{end}}
                         </div>
+                    </div>
                     {{end}}
                 </div>
             </div>

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">