Harden operator-set target headers (closes #233) (#242)

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,88 @@ func isForwardableHeader(name string) bool {
 	}
 }
 
+// applyRequestHeaders builds one outbound delivery's header set and
+// returns the canonical names of every header in it that is scoped
+// to the configured origin: the inbound event headers this delivery
+// forwarded, plus the operator's configured headers. The redirect
+// policy strips exactly that set on a hop that leaves the origin,
+// so the forward set is decided here and only here — a header added
+// to it is covered off-origin without a second edit elsewhere.
 func applyRequestHeaders(
 	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")
+
+	// User-Agent is overwritten just above, so an inbound one never
+	// reaches the wire and the value that does identifies this
+	// delivery path rather than the sender. Reporting it would strip
+	// it off-origin and leave net/http's own default in its place.
+	delete(originScoped, "User-Agent")
+
+	names := make([]string, 0, len(originScoped))
+	for name := range originScoped {
+		names = append(names, name)
+	}
+
+	sort.Strings(names)
+
+	return names
+}
+
+// forwardEventHeaders copies the inbound event's forwardable
+// headers onto the outbound request and returns the canonical names
+// it forwarded. Headers the event never carried are absent from the
+// result, so the redirect policy strips what was actually sent.
+func forwardEventHeaders(
+	req *http.Request,
+	event *database.Event,
+) map[string]struct{} {
+	forwarded := make(map[string]struct{})
+
+	if event.Headers == "" {
+		return forwarded
+	}
+
+	var inbound map[string][]string
+
+	if json.Unmarshal([]byte(event.Headers), &inbound) != nil {
+		return forwarded
+	}
+
+	for k, vals := range inbound {
+		if !isForwardableHeader(k) || len(vals) == 0 {
+			continue
+		}
+
+		for _, v := range vals {
+			req.Header.Add(k, v)
+		}
+
+		forwarded[http.CanonicalHeaderKey(k)] = struct{}{}
+	}
+
+	return forwarded
 }
 
 // executeHTTPRequest sends an HTTP request using the provided