Compare commits

1 Commits

Author SHA1 Message Date
fb5a203896 Send the chi route pattern to Sentry, not the concrete path (closes #179)
All checks were successful
check / check (push) Successful in 2m49s
The Sentry SDK builds Request.URL as scheme://host/path from the
concrete path, which on the receiver route is /webhook/<uuid> in full.
That UUID is a write capability, not an identifier: anyone holding it
can post events this service accepts and its targets then deliver. A
third-party tracker has its own retention, access control and deletion
policy, so the rule the local access log follows does not carry across
that boundary.

The BeforeSend hook now rebuilds the URL from the chi route pattern,
on every route rather than by route list, since a route-conditional
rule leaks on any route someone forgets to add. Scheme and host are
kept and everything else in the URL is discarded rather than edited:
the scheme is the CSRF TLS decision the header allowlist relies on,
and the host is whatever the client's Host header carried, which the
allowlisted Host header already sends anyway.

The pattern is reachable only on the error dispatch, where sentryhttp
puts the request on the context the client copies onto the hint. A
finished span captures with a nil hint, so BeforeSendTransaction sees
no context; there and wherever else the pattern is missing the path
falls back to the literal /(redacted), never to the concrete path, and
a URL that will not parse into a scheme is withheld whole. A
transaction event's SDK-built "METHOD /path" name carries the same
capability and is rewritten on the same terms.
2026-08-18 00:35:35 +00:00
3 changed files with 496 additions and 81 deletions

101
README.md
View File

@@ -1037,38 +1037,77 @@ reduces the headers to a fixed allowlist — `Accept`, `Content-Length`,
`Content-Type`, `Host`, `Origin`, `Referer`, `User-Agent` and `Content-Type`, `Host`, `Origin`, `Referer`, `User-Agent` and
`X-Request-Id`. `X-Request-Id`.
The body is replaced on every route rather than filtered by route, and The same hook rewrites the request URL. The SDK builds it as
that is a choice rather than a limitation: the route is reachable from `scheme://host/path` from the concrete path, which on the receiver
the hook. `sentryhttp`'s recover path puts the request on the context route is `/webhook/<uuid>` in full — and that UUID is a write
it hands to `RecoverWithContext`, and the SDK carries that context capability, not an identifier: anyone holding it can post events this
through to `BeforeSend` as `hint.Context`, so service accepts and its targets then deliver. A tracker has its own
`hint.Context.Value(sentry.RequestContextKey)` yields the live request retention, access control and deletion policy, so the rule the access
and chi's `RoutePattern()` yields the matched pattern off it. There log follows above does not carry across that boundary. What is sent is
are two reasons to redact unconditionally anyway. Nothing debuggable the chi route pattern instead: `http://host/webhook/{uuid}`.
is lost:
every handler reads its fields with `PostFormValue`, so the body is
exactly where the credentials are — the target destination URL, the
login password, both password-change fields — and the one route whose
body is genuine signal is the receiver, whose body is already stored
on the event and served from the UI, so a tracker is not where anyone
reads it. And an unconditional rule cannot leak on a route somebody
forgets to add to it, which a route-conditional one can.
The headers are an allowlist for that second reason: the SDK's own The scheme and the host are kept, and everything else in the URL is
filter removes four names and passes everything else, which would ship discarded rather than edited, so a future SDK version that starts
`X-CSRF-Token` and the shared secrets senders put on the receiver appending a query string cannot widen this. The scheme has to survive
route. What survives still names the failing route — scheme, host, for the reason given below. The host is whatever the request's `Host`
path, method — and `X-Request-Id` ties the event to the local access header carried — this service validates no hostname, so on a directly
log line that holds the rest. Nothing dropped is needed for the exposed deployment a client sets it — and that same header is on the
likeliest use, debugging a CSRF rejection. Its three inputs are the allowlist above, so scrubbing the host out of the URL would withhold
TLS decision, `Origin` and `Referer`; the latter two are kept, and the nothing that is not sent anyway.
first is already in the retained URL, because the SDK derives that
URL's scheme from `r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https"` The body, the query string and the URL are all handled on every route
byte for byte the predicate `internal/middleware/csrf.go` uses to rather than filtered by route. For the URL that is also what keeps the
choose between the `csrf.Secure(true)` and `csrf.Secure(false)` event locatable: an error event is grouped by its exception and stack
handlers. So dropping `X-Forwarded-Proto` costs nothing. The dropped trace, not by its URL, so replacing the path with the pattern costs no
provider headers (`X-GitHub-Event`, `X-Gitlab-Event` and the like) are grouping and the pattern still names the route in the UI. And an
real signal but are recorded locally on the event, and unconditional rule cannot leak on a route somebody forgets to add to
it, which a route-conditional one can. For the body there is a second
reason: nothing debuggable is lost, because every handler reads its
fields with `PostFormValue`, so the body is exactly where the
credentials are — the target destination URL, the login password, both
password-change fields — and the one route whose body is genuine
signal is the receiver, whose body is already stored on the event and
served from the UI, so a tracker is not where anyone reads it.
The route is reachable from the hook only on the error dispatch.
`sentryhttp`'s recover path puts the request on the context it hands
to `RecoverWithContext`, and the SDK carries that context through to
`BeforeSend` as `hint.Context`, so
`hint.Context.Value(sentry.RequestContextKey)` yields the live request
and chi's `RoutePattern()` yields the matched pattern off it. The
transaction dispatch has no such request: a finished span captures
with a nil hint, which the client replaces with an empty one, so
`BeforeSendTransaction` sees no context at all. Tracing is off in this
service, so no transaction event is produced today, but the hook is
installed on both dispatches as a floor.
Where the pattern is out of reach — the transaction dispatch, an event
captured outside the router, or a request that matched no route — the
fallback is never the concrete path. The path becomes the literal
`/(redacted)`, so the URL reads `http://host/(redacted)`; a URL the
rewrite cannot parse into a scheme is withheld whole. A transaction
event additionally carries the SDK's own `METHOD /path` name, built
from the concrete path as well; it is rewritten on the same terms, to
`POST /webhook/{uuid}` where the pattern is known and `POST
/(redacted)` where it is not.
The headers are an allowlist for the same reason the rules above are
unconditional: the SDK's own filter removes four names and passes
everything else, which would ship `X-CSRF-Token` and the shared
secrets senders put on the 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
`r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https"` — byte
for byte the predicate `internal/middleware/csrf.go` uses to choose
between the `csrf.Secure(true)` and `csrf.Secure(false)` handlers.
That is what the rewrite above preserves it for, and it is why
dropping `X-Forwarded-Proto` costs nothing. The dropped provider
headers (`X-GitHub-Event`, `X-Gitlab-Event` and the like) are real
signal but are recorded locally on the event, and
`Sentry-Trace`/`Baggage` are already reflected in the event's trace `Sentry-Trace`/`Baggage` are already reflected in the event's trace
context. context.

View File

@@ -2,8 +2,11 @@ package server
import ( import (
"net/http" "net/http"
"net/url"
"strings"
"github.com/getsentry/sentry-go" "github.com/getsentry/sentry-go"
"github.com/go-chi/chi"
) )
// sentryRedacted stands in for a withheld field on every event shipped // sentryRedacted stands in for a withheld field on every event shipped
@@ -11,6 +14,12 @@ import (
// can tell a suppressed value from an absent one. // can tell a suppressed value from an absent one.
const sentryRedacted = "(redacted)" const sentryRedacted = "(redacted)"
// sentryRedactedPath is what stands in for the request path when the
// route pattern is not reachable. It is deliberately not the concrete
// path: on the receiver route that path carries the entrypoint UUID,
// which is a write capability rather than an identifier.
const sentryRedactedPath = "/" + sentryRedacted
// sentryClientOptions builds the options the SDK is initialised with. // sentryClientOptions builds the options the SDK is initialised with.
// It is its own function so a test can stand up a client wired exactly // It is its own function so a test can stand up a client wired exactly
// as production is, with only the transport swapped. // as production is, with only the transport swapped.
@@ -44,18 +53,43 @@ func sentryClientOptions(dsn, release string) sentry.ClientOptions {
// login password and both password-change fields. None of that may // login password and both password-change fields. None of that may
// reach a third-party service. // reach a third-party service.
// //
// URL is the third such field. NewRequest builds it as
// scheme://host/path (interfaces.go:183), and on the receiver route
// that path is /webhook/<uuid> in full — a write capability, not an
// identifier. It is rebuilt here from the chi route pattern, on every
// route, keeping the scheme and the host.
//
// This hook is a floor, not a default: the fields it clears stay // This hook is a floor, not a default: the fields it clears stay
// cleared even if SendDefaultPII is ever turned on. // cleared even if SendDefaultPII is ever turned on.
func scrubSentryRequest( func scrubSentryRequest(
event *sentry.Event, event *sentry.Event,
_ *sentry.EventHint, hint *sentry.EventHint,
) *sentry.Event { ) *sentry.Event {
if event == nil || event.Request == nil { if event == nil {
return event
}
pattern := sentryRoutePattern(hint)
// Only transaction events carry a Transaction name, and the SDK
// builds it from the concrete path too (sentryhttp.go:105 via
// tracing.go:553). Rewritten on the same terms.
if event.Transaction != "" {
event.Transaction = sentryTransactionName(
event.Transaction, pattern,
)
}
if event.Request == nil {
return event return event
} }
req := event.Request req := event.Request
if req.URL != "" {
req.URL = sentryRouteURL(req.URL, pattern)
}
if req.QueryString != "" { if req.QueryString != "" {
req.QueryString = sentryRedacted req.QueryString = sentryRedacted
} }
@@ -71,6 +105,91 @@ func scrubSentryRequest(
return event return event
} }
// sentryRoutePattern returns the chi route pattern for the request the
// hint carries, or "" when it is not reachable.
//
// The request is reachable on the error dispatch only. sentryhttp's
// recover path calls RecoverWithContext with the request on the
// context under sentry.RequestContextKey (sentryhttp.go:124-125), and
// the client copies that context onto the hint (client.go:484-485)
// before handing it to BeforeSend (client.go:631). chi's routing
// context is a pointer placed on the request context before the
// middleware chain runs (chi mux.go:84) and filled in as the mux
// routes, so by the time a handler panics it names the matched route.
//
// The transaction dispatch has no such request: Span.doFinish calls
// hub.CaptureEvent (tracing.go:356), which passes a nil hint that the
// client replaces with an empty one (client.go:620-622). The pattern
// is therefore always "" there, and the callers fall back.
func sentryRoutePattern(hint *sentry.EventHint) string {
if hint == nil || hint.Context == nil {
return ""
}
req, ok := hint.Context.Value(
sentry.RequestContextKey,
).(*http.Request)
if !ok || req == nil {
return ""
}
rctx := chi.RouteContext(req.Context())
if rctx == nil {
return ""
}
// Empty when no route matched, which is the fallback case too.
return rctx.RoutePattern()
}
// sentryRouteURL rebuilds an event's request URL with the route
// pattern in place of the concrete path.
//
// The scheme is load-bearing and is kept: the SDK derives it from
// r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https"
// (interfaces.go:180), byte for byte the predicate
// internal/middleware/csrf.go uses, so it is the CSRF TLS decision and
// the reason dropping X-Forwarded-Proto from the header allowlist
// costs nothing. The host is parsed.Host of the SDK's
// scheme://r.Host/path, so it is whatever the client's Host header
// carried: this service validates no hostname. It is kept because that
// same header is on the allowlist, so scrubbing it here would withhold
// nothing that is not sent anyway.
//
// Everything else in the URL is discarded rather than edited, so a
// future SDK that starts appending a query string cannot widen this.
func sentryRouteURL(rawURL, pattern string) string {
parsed, err := url.Parse(rawURL)
if err != nil || parsed.Scheme == "" {
// Not a shape this can safely take apart.
return sentryRedacted
}
if pattern == "" {
pattern = sentryRedactedPath
}
return parsed.Scheme + "://" + parsed.Host + pattern
}
// sentryTransactionName rebuilds the SDK's "METHOD /path" transaction
// name with the route pattern in place of the concrete path. Method is
// kept for the same reason Request.Method is: net/http admits only a
// bounded token there. A name in any other shape is withheld whole,
// since nothing can be said about which part of it is a path.
func sentryTransactionName(name, pattern string) string {
method, _, found := strings.Cut(name, " ")
if !found {
return sentryRedacted
}
if pattern == "" {
pattern = sentryRedactedPath
}
return method + " " + pattern
}
// keptSentryHeaders returns the subset of headers an event may carry // keptSentryHeaders returns the subset of headers an event may carry
// off-host. Dropping by allowlist rather than by blocklist is what // off-host. Dropping by allowlist rather than by blocklist is what
// makes an unrecognised header safe: the SDK's own filter removes four // makes an unrecognised header safe: the SDK's own filter removes four

View File

@@ -13,12 +13,13 @@ import (
"github.com/getsentry/sentry-go" "github.com/getsentry/sentry-go"
sentryhttp "github.com/getsentry/sentry-go/http" sentryhttp "github.com/getsentry/sentry-go/http"
"github.com/go-chi/chi"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/server" "sneak.berlin/go/webhooker/internal/server"
) )
// The three markers below are the credentials a captured event could // The four markers below are the credentials a captured event could
// carry off-host, one per field of sentry.Request that the SDK fills // carry off-host, one per field of sentry.Request that the SDK fills
// from the request without a SendDefaultPII guard. // from the request without a SendDefaultPII guard.
const ( const (
@@ -33,6 +34,12 @@ const (
// sentryHeaderMarker rides X-Csrf-Token, which gorilla/csrf // sentryHeaderMarker rides X-Csrf-Token, which gorilla/csrf
// accepts in place of the form field. // accepts in place of the form field.
sentryHeaderMarker = "QQSENTRYHEADERMARKERQQ" sentryHeaderMarker = "QQSENTRYHEADERMARKERQQ"
// sentryReceiverUUID is the entrypoint identifier in the path of
// a receiver request. It is a write capability: anyone holding
// it can POST events this service accepts and its targets then
// deliver, so it may not reach a third-party tracker.
sentryReceiverUUID = "6d1f9c2a-3b7e-4f58-9a0d-c0ffeebadc0d"
) )
// sentryKeptUserAgent is a non-secret header value planted so the // sentryKeptUserAgent is a non-secret header value planted so the
@@ -58,19 +65,41 @@ func (c *captureTransport) SendEvent(event *sentry.Event) {
c.events = append(c.events, event) c.events = append(c.events, event)
} }
// captureThroughSentryHTTP panics inside a form handler wrapped in the // sentryCase drives one request through the real sentryhttp middleware
// real sentryhttp middleware and returns the event the SDK produced. // inside a real chi router and returns the events the SDK produced.
// //
// This is the only construction path on which Request.Data appears: // Routing through a chi mux is load-bearing, not decoration. chi puts
// sentryhttp calls Scope.SetRequest, which tees r.Body into a 10 KiB // its routing context on the request context before the middleware
// buffer, ParseForm drains the tee, and Scope.ApplyToEvent copies the // chain runs and fills it in as it matches, so a hand-built request
// buffer into the event inside prepareEvent — before BeforeSend runs. // carries no route pattern at all and could not distinguish the hook
// A hand-built sentry.NewRequest never reads the body and so cannot // working from the hook falling back.
// regress-test any of it.
// //
// scrub selects whether the production BeforeSend hooks are installed, // This is also the only construction path on which Request.Data
// so the same path shows both what the SDK collects and what survives. // appears: sentryhttp calls Scope.SetRequest, which tees r.Body into a
func captureThroughSentryHTTP(t *testing.T, scrub bool) *sentry.Event { // 10 KiB buffer, ParseForm drains the tee, and Scope.ApplyToEvent
// copies the buffer into the event inside prepareEvent — before
// BeforeSend runs. A hand-built sentry.NewRequest never reads the body
// and so cannot regress-test any of it.
type sentryCase struct {
// scrub selects whether the production BeforeSend hooks are
// installed, so the same path shows both what the SDK collects
// and what survives.
scrub bool
// tracing enables the transaction dispatch, which the service
// leaves off. With it on, a served request produces a
// transaction event through BeforeSendTransaction.
tracing bool
// panics selects the error dispatch, via BeforeSend.
panics bool
// request builds the request to serve, given the client whose
// hub it must carry.
request func(*sentry.Client) *http.Request
}
func (c sentryCase) capture(t *testing.T) []*sentry.Event {
t.Helper() t.Helper()
transport := &captureTransport{} transport := &captureTransport{}
@@ -80,57 +109,107 @@ func captureThroughSentryHTTP(t *testing.T, scrub bool) *sentry.Event {
) )
opts.Transport = transport opts.Transport = transport
if !scrub { if !c.scrub {
opts.BeforeSend = nil opts.BeforeSend = nil
opts.BeforeSendTransaction = nil opts.BeforeSendTransaction = nil
} }
if c.tracing {
opts.EnableTracing = true
opts.TracesSampleRate = 1.0
}
client, err := sentry.NewClient(opts) client, err := sentry.NewClient(opts)
require.NoError(t, err) require.NoError(t, err)
handler := sentryhttp.New(sentryhttp.Options{}).Handle( c.router().ServeHTTP(httptest.NewRecorder(), c.request(client))
http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
// This call is what drains the tee and fills the
// buffer. Its success is asserted by the unscrubbed
// case below, which sees the body in the event.
_ = r.ParseForm()
panic("boom") return transport.events
}),
)
handler.ServeHTTP(
httptest.NewRecorder(),
sentryLoginRequest(client),
)
require.Len(t, transport.events, 1)
return transport.events[0]
} }
// sentryLoginRequest builds the password POST the capture above drives, // router mirrors setupGlobalMiddleware's ordering over the two route
// with a credential planted in the body, the query and a header. // patterns these tests need: a recovering middleware first, then the
// sentryhttp handler registered with Use and Repanic set, exactly as
// routes.go registers it. The local recover stands in for chi's
// middleware.Recoverer, which holds that slot in production; it is
// here only to keep panic stacks out of the test output.
func (c sentryCase) router() http.Handler {
handler := func(_ http.ResponseWriter, r *http.Request) {
// This call is what drains the body tee and fills the
// buffer. Its success is asserted by the unscrubbed case
// below, which sees the body in the event.
_ = r.ParseForm()
if c.panics {
panic("boom")
}
}
router := chi.NewRouter()
router.Use(recoveringMiddleware)
router.Use(
sentryhttp.New(sentryhttp.Options{Repanic: true}).Handle,
)
router.HandleFunc("/pages/login", handler)
router.HandleFunc("/webhook/{uuid}", handler)
return router
}
func recoveringMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
defer func() { _ = recover() }()
next.ServeHTTP(w, r)
},
)
}
// sentryLoginRequest builds the password POST most cases drive, with a
// credential planted in the body, the query and a header.
func sentryLoginRequest(client *sentry.Client) *http.Request { func sentryLoginRequest(client *sentry.Client) *http.Request {
form := url.Values{} form := url.Values{}
form.Set("username", "admin") form.Set("username", "admin")
form.Set("password", sentryBodyMarker) form.Set("password", sentryBodyMarker)
req := sentryRequest(
client,
"/pages/login?url=https://hooks.slack.com/services/"+
sentryQueryMarker,
form.Encode(),
)
req.Header.Set("X-Csrf-Token", sentryHeaderMarker)
return req
}
// sentryReceiverRequest builds a POST to the receiver route, whose
// concrete path carries the entrypoint capability.
func sentryReceiverRequest(client *sentry.Client) *http.Request {
return sentryRequest(
client, "/webhook/"+sentryReceiverUUID, "payload=hello",
)
}
func sentryRequest(
client *sentry.Client,
target, body string,
) *http.Request {
req := httptest.NewRequestWithContext( req := httptest.NewRequestWithContext(
sentry.SetHubOnContext( sentry.SetHubOnContext(
context.Background(), context.Background(),
sentry.NewHub(client, sentry.NewScope()), sentry.NewHub(client, sentry.NewScope()),
), ),
http.MethodPost, http.MethodPost,
"/pages/login?url=https://hooks.slack.com/services/"+ target,
sentryQueryMarker, strings.NewReader(body),
strings.NewReader(form.Encode()),
) )
req.Header.Set( req.Header.Set(
"Content-Type", "application/x-www-form-urlencoded", "Content-Type", "application/x-www-form-urlencoded",
) )
req.Header.Set("X-Csrf-Token", sentryHeaderMarker)
req.Header.Set("User-Agent", sentryKeptUserAgent) req.Header.Set("User-Agent", sentryKeptUserAgent)
return req return req
@@ -146,15 +225,27 @@ func marshalEvent(t *testing.T, event *sentry.Event) string {
return string(encoded) return string(encoded)
} }
// onlyEvent asserts a single event was captured and returns it.
func onlyEvent(t *testing.T, events []*sentry.Event) *sentry.Event {
t.Helper()
require.Len(t, events, 1)
require.NotNil(t, events[0].Request)
return events[0]
}
// TestSentryScrub_SDKCollectsTheRequestUnscrubbed pins the premise the // TestSentryScrub_SDKCollectsTheRequestUnscrubbed pins the premise the
// hook exists for. Without it the SDK ships the whole POST body, the // hook exists for. Without it the SDK ships the whole POST body, the
// raw query and the CSRF header, none of which SendDefaultPII=false // raw query, the CSRF header and the concrete request path, none of
// suppresses. // which SendDefaultPII=false suppresses.
func TestSentryScrub_SDKCollectsTheRequestUnscrubbed(t *testing.T) { func TestSentryScrub_SDKCollectsTheRequestUnscrubbed(t *testing.T) {
t.Parallel() t.Parallel()
event := captureThroughSentryHTTP(t, false) event := onlyEvent(t, sentryCase{
require.NotNil(t, event.Request) panics: true,
request: sentryLoginRequest,
}.capture(t))
assert.Contains( assert.Contains(
t, event.Request.Data, sentryBodyMarker, t, event.Request.Data, sentryBodyMarker,
@@ -165,6 +256,18 @@ func TestSentryScrub_SDKCollectsTheRequestUnscrubbed(t *testing.T) {
assert.Contains( assert.Contains(
t, marshalEvent(t, event), sentryHeaderMarker, t, marshalEvent(t, event), sentryHeaderMarker,
) )
receiver := onlyEvent(t, sentryCase{
panics: true,
request: sentryReceiverRequest,
}.capture(t))
assert.Contains(
t, receiver.Request.URL, sentryReceiverUUID,
"the SDK is expected to build Request.URL from the "+
"concrete path; if it no longer does, the route "+
"pattern rewrite's premise changed",
)
} }
// TestSentryScrub_RedactsTheCapturedRequest is the regression test: no // TestSentryScrub_RedactsTheCapturedRequest is the regression test: no
@@ -173,8 +276,11 @@ func TestSentryScrub_SDKCollectsTheRequestUnscrubbed(t *testing.T) {
func TestSentryScrub_RedactsTheCapturedRequest(t *testing.T) { func TestSentryScrub_RedactsTheCapturedRequest(t *testing.T) {
t.Parallel() t.Parallel()
event := captureThroughSentryHTTP(t, true) event := onlyEvent(t, sentryCase{
require.NotNil(t, event.Request) scrub: true,
panics: true,
request: sentryLoginRequest,
}.capture(t))
encoded := marshalEvent(t, event) encoded := marshalEvent(t, event)
@@ -189,16 +295,45 @@ func TestSentryScrub_RedactsTheCapturedRequest(t *testing.T) {
assert.Empty(t, event.Request.Env) assert.Empty(t, event.Request.Env)
} }
// TestSentryScrub_ReplacesTheCapabilityPathWithTheRoutePattern is the
// regression test for the receiver URL: the entrypoint UUID is a write
// capability and may not reach the tracker, while the route it names
// must still be readable there.
func TestSentryScrub_ReplacesTheCapabilityPathWithTheRoutePattern(
t *testing.T,
) {
t.Parallel()
event := onlyEvent(t, sentryCase{
scrub: true,
panics: true,
request: sentryReceiverRequest,
}.capture(t))
assert.NotContains(
t, marshalEvent(t, event), sentryReceiverUUID,
)
assert.Equal(
t, "http://example.com/webhook/{uuid}", event.Request.URL,
)
}
// TestSentryScrub_KeepsTheRoutingContext checks the hook does not cost // TestSentryScrub_KeepsTheRoutingContext checks the hook does not cost
// the debugging signal: the route, the method and the metadata headers // the debugging signal: the route, its scheme and host, the method and
// still identify what failed. // the metadata headers still identify what failed. On a static route
// the pattern is the path, so the URL is unchanged there.
func TestSentryScrub_KeepsTheRoutingContext(t *testing.T) { func TestSentryScrub_KeepsTheRoutingContext(t *testing.T) {
t.Parallel() t.Parallel()
event := captureThroughSentryHTTP(t, true) event := onlyEvent(t, sentryCase{
require.NotNil(t, event.Request) scrub: true,
panics: true,
request: sentryLoginRequest,
}.capture(t))
assert.Contains(t, event.Request.URL, "/pages/login") assert.Equal(
t, "http://example.com/pages/login", event.Request.URL,
)
assert.Equal(t, http.MethodPost, event.Request.Method) assert.Equal(t, http.MethodPost, event.Request.Method)
assert.Equal( assert.Equal(
t, t,
@@ -212,6 +347,127 @@ func TestSentryScrub_KeepsTheRoutingContext(t *testing.T) {
) )
} }
// TestSentryScrub_RedactsTheTransactionDispatch covers the other hook.
// Span.doFinish captures with a nil hint, so BeforeSendTransaction
// gets one with no context and no request: the route pattern is out of
// reach and both the URL and the SDK-built transaction name have to
// fall back. Tracing is off in this service, so no transaction event
// is produced today; the hook is a floor against that changing.
func TestSentryScrub_RedactsTheTransactionDispatch(t *testing.T) {
t.Parallel()
events := sentryCase{
scrub: true,
tracing: true,
request: sentryReceiverRequest,
}.capture(t)
event := onlyEvent(t, events)
require.Equal(t, "transaction", event.Type)
assert.NotContains(
t, marshalEvent(t, event), sentryReceiverUUID,
)
assert.Equal(
t, "http://example.com/(redacted)", event.Request.URL,
)
assert.Equal(t, "POST /(redacted)", event.Transaction)
}
// TestSentryScrub_TransactionDispatchIsUnscrubbedWithoutTheHook pins
// that dispatch's premise the same way, since it is the one the
// service does not exercise today.
func TestSentryScrub_TransactionDispatchIsUnscrubbedWithoutTheHook(
t *testing.T,
) {
t.Parallel()
event := onlyEvent(t, sentryCase{
tracing: true,
request: sentryReceiverRequest,
}.capture(t))
require.Equal(t, "transaction", event.Type)
assert.Contains(t, event.Request.URL, sentryReceiverUUID)
assert.Contains(t, event.Transaction, sentryReceiverUUID)
}
// TestSentryScrub_FallsBackWithoutARoutePattern covers every way the
// pattern can be missing. None of them may fall back to the concrete
// path, and all of them keep the scheme, which is the CSRF TLS
// decision.
func TestSentryScrub_FallsBackWithoutARoutePattern(t *testing.T) {
t.Parallel()
concrete := "https://example.com/webhook/" + sentryReceiverUUID
// A request with no chi routing context on it at all, which is
// what an event captured outside the router would carry.
unrouted := httptest.NewRequestWithContext(
context.Background(), http.MethodPost, concrete, nil,
)
for name, hint := range map[string]*sentry.EventHint{
"no hint": nil,
"no context": {},
"no request": {Context: context.Background()},
"unrouted request": {
Context: context.WithValue(
context.Background(),
sentry.RequestContextKey,
unrouted,
),
},
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
event := sentry.NewEvent()
event.Request = &sentry.Request{URL: concrete}
event.Transaction = "POST /webhook/" +
sentryReceiverUUID
scrubbed := server.ScrubSentryRequestForTest(
event, hint,
)
require.NotNil(t, scrubbed)
assert.Equal(
t,
"https://example.com/(redacted)",
scrubbed.Request.URL,
)
assert.Equal(
t, "POST /(redacted)", scrubbed.Transaction,
)
assert.NotContains(
t,
marshalEvent(t, scrubbed),
sentryReceiverUUID,
)
})
}
}
// TestSentryScrub_WithholdsUnparseableValues covers the shapes the
// rewrite cannot take apart. Withholding them whole is the safe
// answer, since nothing can be said about which part is a path.
func TestSentryScrub_WithholdsUnparseableValues(t *testing.T) {
t.Parallel()
event := sentry.NewEvent()
event.Request = &sentry.Request{
URL: "/webhook/" + sentryReceiverUUID,
}
event.Transaction = "/webhook/" + sentryReceiverUUID
scrubbed := server.ScrubSentryRequestForTest(event, nil)
require.NotNil(t, scrubbed)
assert.Equal(t, "(redacted)", scrubbed.Request.URL)
assert.Equal(t, "(redacted)", scrubbed.Transaction)
}
// TestSentryScrub_ToleratesEventsWithoutARequest covers the events the // TestSentryScrub_ToleratesEventsWithoutARequest covers the events the
// hook sees outside an HTTP handler, where no request is attached. // hook sees outside an HTTP handler, where no request is attached.
func TestSentryScrub_ToleratesEventsWithoutARequest(t *testing.T) { func TestSentryScrub_ToleratesEventsWithoutARequest(t *testing.T) {
@@ -223,5 +479,6 @@ func TestSentryScrub_ToleratesEventsWithoutARequest(t *testing.T) {
require.NotNil(t, scrubbed) require.NotNil(t, scrubbed)
assert.Nil(t, scrubbed.Request) assert.Nil(t, scrubbed.Request)
assert.Empty(t, scrubbed.Transaction)
assert.Nil(t, server.ScrubSentryRequestForTest(nil, nil)) assert.Nil(t, server.ScrubSentryRequestForTest(nil, nil))
} }