Skip to content

Commit

Permalink
feat: strip user-agent from otel tracing (#1309)
Browse files Browse the repository at this point in the history
Strips the `User-Agent` header so that it's not traced by OpenTelemetry,
while making it available for the rest of the middlewares.

Fixes: https://github.com/supabase/gotrue/security/dependabot/11
  • Loading branch information
hf authored Nov 14, 2023
1 parent 93e5f82 commit d76f439
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
8 changes: 8 additions & 0 deletions internal/api/opentelemetry-tracer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ func (ts *OpenTelemetryTracerTestSuite) TestOpenTelemetryTracer_Spans() {

w := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodPost, "http://localhost/something1", nil)
req.Header.Set("User-Agent", "whatever")
ts.API.handler.ServeHTTP(w, req)

req = httptest.NewRequest(http.MethodGet, "http://localhost/something2", nil)
req.Header.Set("User-Agent", "whatever")
ts.API.handler.ServeHTTP(w, req)

spanStubs := exporter.GetSpans()
Expand All @@ -80,12 +82,18 @@ func (ts *OpenTelemetryTracerTestSuite) TestOpenTelemetryTracer_Spans() {
statusCode1 := getAttribute(attributes1, semconv.HTTPStatusCodeKey)
assert.Equal(ts.T(), int64(404), statusCode1.AsInt64())

userAgent1 := getAttribute(attributes1, semconv.HTTPUserAgentKey)
assert.Equal(ts.T(), "stripped", userAgent1.AsString())

attributes2 := spans[1].Attributes()
method2 := getAttribute(attributes2, semconv.HTTPMethodKey)
assert.Equal(ts.T(), "GET", method2.AsString())
url2 := getAttribute(attributes2, semconv.HTTPTargetKey)
assert.Equal(ts.T(), "http://localhost/something2", url2.AsString())
statusCode2 := getAttribute(attributes2, semconv.HTTPStatusCodeKey)
assert.Equal(ts.T(), int64(404), statusCode2.AsInt64())

userAgent2 := getAttribute(attributes2, semconv.HTTPUserAgentKey)
assert.Equal(ts.T(), "stripped", userAgent2.AsString())
}
}
29 changes: 28 additions & 1 deletion internal/observability/request-tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,36 @@ func RequestTracing() func(http.Handler) http.Handler {
defer traceChiRouteURLParamsSafely(r)
defer countStatusCodesSafely(&writer, r, statusCodes)

originalUserAgent := r.Header.Get("X-Gotrue-Original-User-Agent")
if originalUserAgent != "" {
r.Header.Set("User-Agent", originalUserAgent)
}

next.ServeHTTP(&writer, r)

if originalUserAgent != "" {
r.Header.Set("X-Gotrue-Original-User-Agent", originalUserAgent)
r.Header.Set("User-Agent", "stripped")
}
}

return otelhttp.NewHandler(http.HandlerFunc(fn), "api")
otelHandler := otelhttp.NewHandler(http.HandlerFunc(fn), "api")

return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// there is a vulnerability with otelhttp where
// User-Agent strings are kept in RAM indefinitely and
// can be used as an easy way to resource exhaustion;
// so this code strips the User-Agent header before
// it's passed to be traced by otelhttp, and then is
// returned back to the middleware
// https://github.com/supabase/gotrue/security/dependabot/11
userAgent := r.UserAgent()
if userAgent != "" {
r.Header.Set("X-Gotrue-Original-User-Agent", userAgent)
r.Header.Set("User-Agent", "stripped")
}

otelHandler.ServeHTTP(w, r)
})
}
}

0 comments on commit d76f439

Please sign in to comment.