fix(security): allow /metrics to require a bearer token - #138
Open
fjaeckel wants to merge 1 commit into
Open
Conversation
The Prometheus endpoint was registered with no authentication. It exposes Go runtime stats, DB pool stats, per-path request counters and auth attempt/failure counters -- useful reconnaissance for anyone who can reach the port, and the user/registration counters make it a slow enumeration oracle. Live-confirmed: GET /metrics with no credentials returned 200 with 340 metric lines. METRICS_TOKEN now gates the endpoint behind a bearer token, compared in constant time so response timing cannot leak it prefix-wise. Both 'Bearer <token>' and a raw token are accepted, since Prometheus' bearer_token_file sends the former while simple scrape configs often send the latter. When METRICS_TOKEN is unset the endpoint stays open and a startup warning names the setting. That keeps existing deployments working: the intended topology scrapes over the internal Docker network with the API port unpublished, where the endpoint is already unreachable from outside. Failing closed would silently break those scrapes on upgrade, so this is opt-in with a loud default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GukWfyJMY28qv2CJjxFvKF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
GET /metricsis served without any authentication. Against a locally-run instance it returned 340 lines of Prometheus output to an unauthenticated client, including per-route request counts and latency histograms, Go runtime and GC internals, process memory/FD counts, and DB connection-pool stats. Route-level counters leak usage volume and endpoint inventory;go_info/process_*leak build and host details useful for fingerprinting.Change
internal/api/middleware/metrics_auth.go—MetricsAuthMiddleware(token)compares the presented credential withsubtle.ConstantTimeCompare. It accepts eitherAuthorization: Bearer <token>or the bare token, and rejects everything else with a bodyless401.cmd/api/main.goreadsMETRICS_TOKENand applies the middleware to the/metricsroute when it is set..env.exampledocuments the variable.internal/api/middleware/metrics_auth_test.go— 7 sub-tests covering the bearer form, the bare form, a missing header, a wrong token, a case-mismatched scheme, an empty presented value, and a token that is a prefix-extension of the expected value.Deliberate design decision
When
METRICS_TOKENis unset the endpoint stays open, and the server logs a warning at startup:Failing closed by default would silently break existing scrapers on upgrade — a deployment whose Prometheus is on an internal network would start returning 401 with no code change on their side, and the failure surfaces as a gap in dashboards rather than an error anyone reads. The warning makes the exposure visible in logs while leaving the operator in control of when to turn the gate on. If you would rather fail closed and accept the upgrade break, that is a one-line change and I am happy to make it.
Verification
go test ./...— 24 packages passgofmt -lcleanMETRICS_TOKENset,curl /metrics→401;curl -H 'Authorization: Bearer $TOKEN' /metrics→200with the metric bodyGenerated by Claude Code