Skip to content

Commit d1aeae0

Browse files
rickcrawfordclaude
andcommitted
fix(security): mitigate reflected XSS on /metrics endpoint
CodeQL go/reflected-xss (high) flagged a data flow from HTTP request into the metrics response body via Prometheus label values (e.g. path or user-agent histogram labels). Force a text/plain Content-Type and X-Content-Type-Options: nosniff in the scrape limiter wrapper before the downstream handler writes any bytes. Downstream promhttp may refine the Content-Type to the Prometheus exposition variant, but the response can no longer be MIME-sniffed as HTML by a browser. Apply the same headers to the 429 Too Many Requests path. Lock the behavior in with a TestScrapeLimiter_SecurityHeaders test covering both success and rate-limited responses. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4440bc5 commit d1aeae0

2 files changed

Lines changed: 66 additions & 0 deletions

File tree

internal/observe/metric/scrape_limiter.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ func NewScrapeLimiter(minInterval time.Duration, maxBodySize int64) *ScrapeLimit
4040
// Wrap wraps an http.Handler with rate limiting for the /metrics endpoint.
4141
// Requests arriving before minInterval has elapsed since the last scrape receive
4242
// a 429 Too Many Requests response with a Retry-After header.
43+
//
44+
// Security: Prometheus metric labels can echo user-controlled data (e.g. request
45+
// path or user agent from histogram labels). To neutralize any reflected-XSS
46+
// risk if the response body is ever rendered as HTML, Wrap forces
47+
// X-Content-Type-Options: nosniff and a text/plain Content-Type before the
48+
// downstream handler writes any bytes. The downstream handler may still refine
49+
// the Content-Type (e.g. to Prometheus' exposition format with a charset), but
50+
// browsers will no longer MIME-sniff HTML out of this response.
4351
func (sl *ScrapeLimiter) Wrap(next http.Handler) http.Handler {
4452
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
4553
sl.mu.Lock()
@@ -52,6 +60,8 @@ func (sl *ScrapeLimiter) Wrap(next http.Handler) http.Handler {
5260

5361
retrySeconds := int(retryAfter.Seconds()) + 1
5462
w.Header().Set("Retry-After", fmt.Sprintf("%d", retrySeconds))
63+
w.Header().Set("X-Content-Type-Options", "nosniff")
64+
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
5565
http.Error(w, "Too Many Requests", http.StatusTooManyRequests)
5666
return
5767
}
@@ -60,6 +70,12 @@ func (sl *ScrapeLimiter) Wrap(next http.Handler) http.Handler {
6070
maxBody := sl.maxBodySize
6171
sl.mu.Unlock()
6272

73+
// Lock the response to plain text before the wrapped handler writes
74+
// anything. This prevents any metric-label content from being
75+
// interpreted as HTML by browsers (CodeQL go/reflected-xss).
76+
w.Header().Set("X-Content-Type-Options", "nosniff")
77+
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
78+
6379
if maxBody > 0 {
6480
lrw := &limitedResponseWriter{
6581
ResponseWriter: w,

internal/observe/metric/scrape_limiter_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,3 +228,53 @@ func TestScrapeLimiter_RetryAfterHeader(t *testing.T) {
228228
t.Errorf("expected Retry-After ~30, got %q", retryAfter)
229229
}
230230
}
231+
232+
// TestScrapeLimiter_SecurityHeaders verifies that the wrapper forces a
233+
// non-HTML Content-Type and nosniff so Prometheus label values cannot be
234+
// interpreted as HTML by a browser (mitigates CodeQL go/reflected-xss).
235+
func TestScrapeLimiter_SecurityHeaders(t *testing.T) {
236+
t.Run("success response has nosniff and text/plain", func(t *testing.T) {
237+
sl := NewScrapeLimiter(1*time.Second, 0)
238+
handler := sl.Wrap(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
239+
// Attempt to set an HTML content type from the wrapped handler;
240+
// the wrapper's early Set is what guards against label-based XSS,
241+
// but legitimate downstream handlers may still refine to a more
242+
// specific text/plain variant (e.g. Prometheus exposition format).
243+
_, _ = w.Write([]byte(`<script>alert(1)</script>`))
244+
}))
245+
246+
req := httptest.NewRequest(http.MethodGet, "/metrics", nil)
247+
rr := httptest.NewRecorder()
248+
handler.ServeHTTP(rr, req)
249+
250+
if got := rr.Header().Get("X-Content-Type-Options"); got != "nosniff" {
251+
t.Errorf("expected X-Content-Type-Options=nosniff, got %q", got)
252+
}
253+
if got := rr.Header().Get("Content-Type"); !strings.HasPrefix(got, "text/plain") {
254+
t.Errorf("expected text/plain Content-Type, got %q", got)
255+
}
256+
})
257+
258+
t.Run("rate-limited response also has security headers", func(t *testing.T) {
259+
sl := NewScrapeLimiter(30*time.Second, 0)
260+
handler := sl.Wrap(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
261+
_, _ = w.Write([]byte("ok"))
262+
}))
263+
264+
// Prime the limiter, then hit it again immediately.
265+
handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/metrics", nil))
266+
267+
rr := httptest.NewRecorder()
268+
handler.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/metrics", nil))
269+
270+
if rr.Code != http.StatusTooManyRequests {
271+
t.Fatalf("expected 429, got %d", rr.Code)
272+
}
273+
if got := rr.Header().Get("X-Content-Type-Options"); got != "nosniff" {
274+
t.Errorf("expected X-Content-Type-Options=nosniff on 429, got %q", got)
275+
}
276+
if got := rr.Header().Get("Content-Type"); !strings.HasPrefix(got, "text/plain") {
277+
t.Errorf("expected text/plain Content-Type on 429, got %q", got)
278+
}
279+
})
280+
}

0 commit comments

Comments
 (0)