From f79bc578c8bb0999d636a053de51d66b218b5302 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, through the access log's own reduction, which internal/middleware now exports as RefererSchemeAndHost. The comment above sentryKeepsHeader and the README's Sentry passage say so. Model: opus-5-5 --- README.md | 7 +++++-- internal/middleware/middleware.go | 27 +++++++++++++++------------ internal/server/sentry.go | 25 ++++++++++++++++++------- internal/server/sentry_test.go | 31 +++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index d5febdb..feb16ff 100644 --- a/README.md +++ b/README.md @@ -2491,6 +2491,8 @@ 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, by the same rule as the access +log's `referer` field, because its path can be an entrypoint URL. 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 @@ -2561,8 +2563,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 41e65ac..afba083 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -48,8 +48,8 @@ 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 stands in for a Referer that cannot be reduced + // to its scheme and host; see RefererSchemeAndHost. redactedReferer = "(redacted)" // maxLogRequestIDBytes bounds the request id, which is also @@ -342,15 +342,17 @@ 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() +// RefererSchemeAndHost 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 +// redactedReferer. +// +// It is the access log's referer field, and the Referer header the +// Sentry hook in internal/server sends. +func RefererSchemeAndHost(referer string) string { if referer == "" { return "" } @@ -413,7 +415,8 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler { requestID, maxLogRequestIDBytes, ), "referer", logfield.Truncate( - accessLogReferer(r), logfield.MaxBytes, + RefererSchemeAndHost(r.Referer()), + logfield.MaxBytes, ), "proto", r.Proto, "remoteIP", RemoteIP(r), diff --git a/internal/server/sentry.go b/internal/server/sentry.go index fdd2dbd..71dd2e1 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/middleware" ) // 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 = middleware.RefererSchemeAndHost(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, by the access log's own rule, +// middleware.RefererSchemeAndHost: 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