Send only the Referer's scheme and host to Sentry (closes #548)

A Sentry event kept the Referer header verbatim, and a Referer can be
an entrypoint URL: a sender that follows the README's http:// to
https:// redirect sends the URL it was redirected from, and a browser
sends the 404 page's URL under /h/ when it fetches that page's
stylesheet. The Sentry hook now keeps only the Referer's scheme and
host. The access log's reduction moves, unchanged, into a new package,
internal/referer, as SchemeAndHost; the access log and the Sentry hook
both call it there. The comment above sentryKeepsHeader and the
README's Sentry passage say so.

Model: opus-5-5

README.md

@@ -2496,6 +2496,9 @@ are both taken unconditionally, the body precisely because these handlers call
 body with `(redacted)`, drops cookies and the remote-address environment, and
 reduces the headers to a fixed allowlist — `Accept`, `Content-Length`,
 `Content-Type`, `Host`, `Origin`, `Referer`, `User-Agent` and `X-Request-Id`.
+`Referer` is sent as its scheme and host only, because its path can be an
+entrypoint URL; `internal/referer.SchemeAndHost` does that reduction for both
+the access log's `referer` field and this hook.
 
 The same hook rewrites the request URL. The SDK builds it as
 `scheme://host/path` from the concrete path, which on the receiver route is
@@ -2566,8 +2569,9 @@ receiver route. What survives still names the failing route — scheme, host,
 route pattern, method — and `X-Request-Id` ties the event to the local access
 log line that holds the rest. Nothing dropped is needed for the likeliest use,
 debugging a CSRF rejection. Its three inputs are the TLS decision, `Origin` and
-`Referer`; the latter two are kept, and the first is the scheme of the retained
-URL, because the SDK derives that scheme from
+`Referer`; the latter two are kept (`Referer` as its scheme and host, which is
+all the check compares), and the first is the scheme of the retained URL,
+because the SDK derives that scheme from
 `r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https"`. That predicate
 is the SDK's own and is stricter than `internal/reqtls.IsTLS`, which this
 service now uses everywhere it decides transport: the SDK reports `http` for the

internal/middleware/middleware.go

@@ -22,6 +22,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/globals"
 	"sneak.berlin/go/webhooker/internal/logfield"
+	"sneak.berlin/go/webhooker/internal/referer"
 	"sneak.berlin/go/webhooker/internal/session"
 )
 
@@ -48,10 +49,6 @@ const (
 	// pick the size of the line it writes.
 	redactedQuery = "?(redacted)"
 
-	// redactedReferer stands in for a Referer the access log cannot
-	// reduce to its scheme and host; see accessLogReferer.
-	redactedReferer = "(redacted)"
-
 	// maxLogRequestIDBytes bounds the request id, which is also
 	// client-supplied: chi's RequestID middleware passes an inbound
 	// X-Request-Id header through verbatim. Its generated form is an
@@ -342,27 +339,6 @@ func accessLogURL(r *http.Request, status int) string {
 	return pattern
 }
 
-// accessLogReferer returns the value for the access log's referer
-// field: the Referer's scheme and host, never its path or query. The
-// path can be an entrypoint URL, which carries the UUID: a sender that
-// follows a redirect from http:// to https:// sends the URL it was
-// redirected from, and a browser sends the 404 page's URL under /h/
-// when it fetches that page's stylesheet. A Referer without both a
-// scheme and a host is logged as redactedReferer.
-func accessLogReferer(r *http.Request) string {
-	referer := r.Referer()
-	if referer == "" {
-		return ""
-	}
-
-	parsed, err := url.Parse(referer)
-	if err != nil || parsed.Scheme == "" || parsed.Host == "" {
-		return redactedReferer
-	}
-
-	return parsed.Scheme + "://" + parsed.Host
-}
-
 // Logging returns middleware that logs each HTTP request with
 // timing and metadata.
 func (s *Middleware) Logging() func(http.Handler) http.Handler {
@@ -413,7 +389,8 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
 						requestID, maxLogRequestIDBytes,
 					),
 					"referer", logfield.Truncate(
-						accessLogReferer(r), logfield.MaxBytes,
+						referer.SchemeAndHost(r.Referer()),
+						logfield.MaxBytes,
 					),
 					"proto", r.Proto,
 					"remoteIP", RemoteIP(r),

internal/referer/referer.go

@@ -0,0 +1,32 @@
+// Package referer reduces a Referer header to its scheme and host, so
+// that a URL path, which on the receiver route carries the entrypoint
+// UUID, never reaches a log line or an error report.
+package referer
+
+import "net/url"
+
+// redacted stands in for a Referer that cannot be reduced to its
+// scheme and host.
+const redacted = "(redacted)"
+
+// SchemeAndHost returns a Referer's scheme and host, never its path or
+// query. The path can be an entrypoint URL, which carries the UUID: a
+// sender that follows a redirect from http:// to https:// sends the
+// URL it was redirected from, and a browser sends the 404 page's URL
+// under /h/ when it fetches that page's stylesheet. A Referer without
+// both a scheme and a host comes back as "(redacted)".
+//
+// It is the access log's referer field, and the Referer header the
+// Sentry hook sends.
+func SchemeAndHost(referer string) string {
+	if referer == "" {
+		return ""
+	}
+
+	parsed, err := url.Parse(referer)
+	if err != nil || parsed.Scheme == "" || parsed.Host == "" {
+		return redacted
+	}
+
+	return parsed.Scheme + "://" + parsed.Host
+}

internal/referer/referer_test.go

@@ -0,0 +1,67 @@
+package referer_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"sneak.berlin/go/webhooker/internal/referer"
+)
+
+// entrypointUUID is the secret part of an entrypoint URL; no case may
+// return it.
+const entrypointUUID = "0b6f7d3e-6a39-4e3c-9f0a-2c4d8e1b5a77"
+
+// redacted is what SchemeAndHost returns for a Referer it cannot
+// reduce to a scheme and host.
+const redacted = "(redacted)"
+
+func TestSchemeAndHost(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name    string
+		referer string
+		want    string
+	}{
+		{
+			name:    "no Referer",
+			referer: "",
+			want:    "",
+		},
+		{
+			name: "entrypoint URL with a query",
+			referer: "http://webhooker.example/h/" + entrypointUUID +
+				"?src=ci",
+			want: "http://webhooker.example",
+		},
+		{
+			name:    "host with a port",
+			referer: "https://webhooker.example:8443/h/" + entrypointUUID,
+			want:    "https://webhooker.example:8443",
+		},
+		{
+			name:    "path without a scheme or host",
+			referer: "/h/" + entrypointUUID,
+			want:    redacted,
+		},
+		{
+			name:    "host without a scheme",
+			referer: "webhooker.example/h/" + entrypointUUID,
+			want:    redacted,
+		},
+		{
+			name:    "not a URL",
+			referer: "http://[::1/h/" + entrypointUUID,
+			want:    redacted,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := referer.SchemeAndHost(tc.referer)
+
+			assert.Equal(t, tc.want, got)
+			assert.NotContains(t, got, entrypointUUID)
+		})
+	}
+}

internal/server/sentry.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/getsentry/sentry-go"
 	"github.com/go-chi/chi"
+	"sneak.berlin/go/webhooker/internal/referer"
 )
 
 // sentryRedacted stands in for a withheld field on every event shipped
@@ -253,6 +254,7 @@ func sentryTransactionName(name, pattern string) string {
 // gorilla/csrf accepts in place of the form field — and the shared
 // secrets senders put on the receiver route (X-Gitlab-Token and the
 // per-provider signature headers) would otherwise ship verbatim.
+// The Referer it keeps is cut down to its scheme and host.
 func keptSentryHeaders(headers map[string]string) map[string]string {
 	if len(headers) == 0 {
 		return headers
@@ -261,9 +263,15 @@ func keptSentryHeaders(headers map[string]string) map[string]string {
 	kept := make(map[string]string, len(headers))
 
 	for name, value := range headers {
-		if sentryKeepsHeader(name) {
-			kept[name] = value
+		if !sentryKeepsHeader(name) {
+			continue
 		}
+
+		if http.CanonicalHeaderKey(name) == "Referer" {
+			value = referer.SchemeAndHost(value)
+		}
+
+		kept[name] = value
 	}
 
 	return kept
@@ -271,11 +279,14 @@ func keptSentryHeaders(headers map[string]string) map[string]string {
 
 // sentryKeepsHeader reports whether a request header is routing or
 // content metadata rather than client-chosen payload. Referer is kept
-// on the reasoning that it is browser-set, that the only query
-// parameters in this service's own URLs are the login page's `next`,
-// `notice` and the event log's `show`, and that Referrer-Policy is set to
-// strict-origin-when-cross-origin. X-Request-Id ties the event to the
-// local access log line, which holds the rest of the detail.
+// only as its scheme and host, through referer.SchemeAndHost, which
+// the access log uses too: its path can be an entrypoint URL, which
+// carries the UUID, as when a sender follows a redirect from http://
+// to https:// or a browser fetches the 404 page's stylesheet from
+// under /h/. The scheme and host are all gorilla/csrf compares a
+// Referer by, so a CSRF rejection is still readable from the event.
+// X-Request-Id ties the event to the local access log line, which
+// holds the rest of the detail.
 func sentryKeepsHeader(name string) bool {
 	switch http.CanonicalHeaderKey(name) {
 	case "Accept",

internal/server/sentry_test.go

@@ -325,6 +325,37 @@ func TestSentryScrub_ReplacesTheCapabilityPathWithTheRoutePattern(
 	)
 }
 
+// TestSentryScrub_KeepsOnlyTheRefererSchemeAndHost sends a receiver
+// request whose Referer is the entrypoint URL, as a sender's is after
+// it follows the README's redirect from http:// to https://. The event
+// keeps the Referer's scheme and host, and carries the UUID nowhere.
+func TestSentryScrub_KeepsOnlyTheRefererSchemeAndHost(t *testing.T) {
+	t.Parallel()
+
+	event := onlyEvent(t, sentryCase{
+		scrub:  true,
+		panics: true,
+		request: func(client *sentry.Client) *http.Request {
+			req := sentryReceiverRequest(client)
+			req.Header.Set(
+				"Referer",
+				"http://webhooker.example/h/"+sentryReceiverUUID,
+			)
+
+			return req
+		},
+	}.capture(t))
+
+	assert.NotContains(
+		t, marshalEvent(t, event), sentryReceiverUUID,
+	)
+	assert.Equal(
+		t,
+		"http://webhooker.example",
+		event.Request.Headers["Referer"],
+	)
+}
+
 // TestSentryScrub_KeepsTheRoutingContext checks the hook does not cost
 // the debugging signal: the route, its scheme and host, the method and
 // the metadata headers still identify what failed. On a static route