From 8f0129d4215997f1ef1cbab937ea3a27b8f36ba7 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 13:54:58 +0000 Subject: [PATCH] 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 | 8 +++- internal/middleware/middleware.go | 29 ++----------- internal/referer/referer.go | 32 +++++++++++++++ internal/referer/referer_test.go | 67 +++++++++++++++++++++++++++++++ internal/server/sentry.go | 25 ++++++++---- internal/server/sentry_test.go | 31 ++++++++++++++ 6 files changed, 157 insertions(+), 35 deletions(-) create mode 100644 internal/referer/referer.go create mode 100644 internal/referer/referer_test.go diff --git a/README.md b/README.md index 0a3894b..b24c744 100644 --- a/README.md +++ b/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 diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 9b5b842..183fc2b 100644 --- a/internal/middleware/middleware.go +++ b/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), diff --git a/internal/referer/referer.go b/internal/referer/referer.go new file mode 100644 index 0000000..4aac785 --- /dev/null +++ b/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 +} diff --git a/internal/referer/referer_test.go b/internal/referer/referer_test.go new file mode 100644 index 0000000..3f81cd8 --- /dev/null +++ b/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) + }) + } +} diff --git a/internal/server/sentry.go b/internal/server/sentry.go index fdd2dbd..fccdd1b 100644 --- a/internal/server/sentry.go +++ b/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", diff --git a/internal/server/sentry_test.go b/internal/server/sentry_test.go index cf1dcdc..7b844ea 100644 --- a/internal/server/sentry_test.go +++ b/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