Send only the Referer's scheme and host to Sentry (closes #548)
check / check (push) Waiting to run
check / check (push) Waiting to run
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
This commit is contained in:
@@ -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
|
body with `(redacted)`, drops cookies and the remote-address environment, and
|
||||||
reduces the headers to a fixed allowlist — `Accept`, `Content-Length`,
|
reduces the headers to a fixed allowlist — `Accept`, `Content-Length`,
|
||||||
`Content-Type`, `Host`, `Origin`, `Referer`, `User-Agent` and `X-Request-Id`.
|
`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
|
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
|
`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
|
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,
|
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
|
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
|
`Referer`; the latter two are kept (`Referer` as its scheme and host, which is
|
||||||
URL, because the SDK derives that scheme from
|
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
|
`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
|
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
|
service now uses everywhere it decides transport: the SDK reports `http` for the
|
||||||
|
|||||||
@@ -48,8 +48,8 @@ const (
|
|||||||
// pick the size of the line it writes.
|
// pick the size of the line it writes.
|
||||||
redactedQuery = "?(redacted)"
|
redactedQuery = "?(redacted)"
|
||||||
|
|
||||||
// redactedReferer stands in for a Referer the access log cannot
|
// redactedReferer stands in for a Referer that cannot be reduced
|
||||||
// reduce to its scheme and host; see accessLogReferer.
|
// to its scheme and host; see RefererSchemeAndHost.
|
||||||
redactedReferer = "(redacted)"
|
redactedReferer = "(redacted)"
|
||||||
|
|
||||||
// maxLogRequestIDBytes bounds the request id, which is also
|
// maxLogRequestIDBytes bounds the request id, which is also
|
||||||
@@ -342,15 +342,17 @@ func accessLogURL(r *http.Request, status int) string {
|
|||||||
return pattern
|
return pattern
|
||||||
}
|
}
|
||||||
|
|
||||||
// accessLogReferer returns the value for the access log's referer
|
// RefererSchemeAndHost returns a Referer's scheme and host, never its
|
||||||
// field: the Referer's scheme and host, never its path or query. The
|
// path or query. The path can be an entrypoint URL, which carries the
|
||||||
// path can be an entrypoint URL, which carries the UUID: a sender that
|
// UUID: a sender that follows a redirect from http:// to https://
|
||||||
// follows a redirect from http:// to https:// sends the URL it was
|
// sends the URL it was redirected from, and a browser sends the 404
|
||||||
// redirected from, and a browser sends the 404 page's URL under /h/
|
// page's URL under /h/ when it fetches that page's stylesheet. A
|
||||||
// when it fetches that page's stylesheet. A Referer without both a
|
// Referer without both a scheme and a host comes back as
|
||||||
// scheme and a host is logged as redactedReferer.
|
// redactedReferer.
|
||||||
func accessLogReferer(r *http.Request) string {
|
//
|
||||||
referer := r.Referer()
|
// 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 == "" {
|
if referer == "" {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
@@ -413,7 +415,8 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
|
|||||||
requestID, maxLogRequestIDBytes,
|
requestID, maxLogRequestIDBytes,
|
||||||
),
|
),
|
||||||
"referer", logfield.Truncate(
|
"referer", logfield.Truncate(
|
||||||
accessLogReferer(r), logfield.MaxBytes,
|
RefererSchemeAndHost(r.Referer()),
|
||||||
|
logfield.MaxBytes,
|
||||||
),
|
),
|
||||||
"proto", r.Proto,
|
"proto", r.Proto,
|
||||||
"remoteIP", RemoteIP(r),
|
"remoteIP", RemoteIP(r),
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import (
|
|||||||
|
|
||||||
"github.com/getsentry/sentry-go"
|
"github.com/getsentry/sentry-go"
|
||||||
"github.com/go-chi/chi"
|
"github.com/go-chi/chi"
|
||||||
|
"sneak.berlin/go/webhooker/internal/middleware"
|
||||||
)
|
)
|
||||||
|
|
||||||
// sentryRedacted stands in for a withheld field on every event shipped
|
// 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
|
// gorilla/csrf accepts in place of the form field — and the shared
|
||||||
// secrets senders put on the receiver route (X-Gitlab-Token and the
|
// secrets senders put on the receiver route (X-Gitlab-Token and the
|
||||||
// per-provider signature headers) would otherwise ship verbatim.
|
// 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 {
|
func keptSentryHeaders(headers map[string]string) map[string]string {
|
||||||
if len(headers) == 0 {
|
if len(headers) == 0 {
|
||||||
return headers
|
return headers
|
||||||
@@ -261,9 +263,15 @@ func keptSentryHeaders(headers map[string]string) map[string]string {
|
|||||||
kept := make(map[string]string, len(headers))
|
kept := make(map[string]string, len(headers))
|
||||||
|
|
||||||
for name, value := range headers {
|
for name, value := range headers {
|
||||||
if sentryKeepsHeader(name) {
|
if !sentryKeepsHeader(name) {
|
||||||
kept[name] = value
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if http.CanonicalHeaderKey(name) == "Referer" {
|
||||||
|
value = middleware.RefererSchemeAndHost(value)
|
||||||
|
}
|
||||||
|
|
||||||
|
kept[name] = value
|
||||||
}
|
}
|
||||||
|
|
||||||
return kept
|
return kept
|
||||||
@@ -271,11 +279,14 @@ func keptSentryHeaders(headers map[string]string) map[string]string {
|
|||||||
|
|
||||||
// sentryKeepsHeader reports whether a request header is routing or
|
// sentryKeepsHeader reports whether a request header is routing or
|
||||||
// content metadata rather than client-chosen payload. Referer is kept
|
// content metadata rather than client-chosen payload. Referer is kept
|
||||||
// on the reasoning that it is browser-set, that the only query
|
// only as its scheme and host, by the access log's own rule,
|
||||||
// parameters in this service's own URLs are the login page's `next`,
|
// middleware.RefererSchemeAndHost: its path can be an entrypoint URL,
|
||||||
// `notice` and the event log's `show`, and that Referrer-Policy is set to
|
// which carries the UUID, as when a sender follows a redirect from
|
||||||
// strict-origin-when-cross-origin. X-Request-Id ties the event to the
|
// http:// to https:// or a browser fetches the 404 page's stylesheet
|
||||||
// local access log line, which holds the rest of the detail.
|
// 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 {
|
func sentryKeepsHeader(name string) bool {
|
||||||
switch http.CanonicalHeaderKey(name) {
|
switch http.CanonicalHeaderKey(name) {
|
||||||
case "Accept",
|
case "Accept",
|
||||||
|
|||||||
@@ -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
|
// TestSentryScrub_KeepsTheRoutingContext checks the hook does not cost
|
||||||
// the debugging signal: the route, its scheme and host, the method and
|
// the debugging signal: the route, its scheme and host, the method and
|
||||||
// the metadata headers still identify what failed. On a static route
|
// the metadata headers still identify what failed. On a static route
|
||||||
|
|||||||
Reference in New Issue
Block a user