feat(auth): gate /metrics/prometheus behind optional METRICS_TOKEN

[surpradhan]

Context

Both reviewers of #156 (fail-closed auth) flagged GET /metrics/prometheus as the one remaining always-open data-bearing surface in production and asked for an explicit decision: keep it open and document the scrape-network assumption, or add an optional metrics token.

Decision: option (b) — dedicated METRICS_TOKEN bearer auth

Rationale: #156 established that a forgotten token must never silently expose a surface in production. Documenting a scrape-network assumption would leave /metrics/prometheus as the single exception to that posture. A dedicated token matches the existing DASHBOARD_TOKEN idiom exactly, and Prometheus supports bearer credentials natively (authorization.credentials scrape config), so the operational cost is one line of scrape config.

Behavior matrix (all four live-probed against a real server)

METRICS_TOKEN	NODE_ENV	Result
unset	dev	200 — open scrape (unchanged, regression-locked)
unset	production	503 Metrics not configured (fail closed) + startup warning
set	any	401 without / with wrong bearer token
set	any	200 with correct Authorization: Bearer <token>

The payload remains aggregate and tenant-label-free either way (src/metrics.js); /health, /ready untouched.

Changes

• src/auth.js — new requireMetricsAuth (fourth auth layer), mirroring requireDashboardAuth's unset/dev/production branches and requireAdminAuth's bearer-only + constant-time comparison
• src/server.js — middleware wired onto the route; route comment rewritten; startup warning (dev-open vs production-503 variants, mirroring the DASHBOARD_TOKEN block)
• Docs: AUTH.md (overview table, env-vars table, security note), SECURITY.md (new §8 with prometheus.yml example), OPERATIONS.md (production checklist), docs/QUICK_REFERENCE.md (replaces the "intentionally unauthenticated" note), .env.example, docker-compose.yml (METRICS_TOKEN passthrough)

Tests

7 new integration tests (TDD — all four enforcement tests watched failing with actual: 200 before implementation):

• dev + unset → 200 (locks the open-scrape regression)
• set → 401 missing token, 401 wrong token, 200 correct token
• production + unset → 503 not configured
• production + set → 200 with token, 401 without (not 503)

Gates: npm run lint clean · unit 494/494 · integration 271/271.

Compatibility note

Production deployments that scrape /metrics/prometheus today must set METRICS_TOKEN on upgrade (endpoint 503s until then, loudly, with a hint + startup warning). This is the same intentional breaking posture #156 took for the dashboard, and the server is a run-from-source reference implementation with no release pipeline.

🤖 Generated with Claude Code

[surpradhan]

Independent validation report (empirical, run — not just read)

Validated at PR head e8f74a1 in an isolated worktree; gh pr diff 164 is byte-identical to the diff of e8f74a1 vs base 0e09eb7 (no uncommitted work missing from the PR). Node v24.11.1.

1. Gates — all green, counts match the PR's claims exactly

npm run lint          → exit 0 (eslint src tests, clean)
npm run test:unit     → tests 494 / pass 494 / fail 0
npm run test:integration → tests 271 / pass 271 / fail 0

2. Discriminator probe — the new tests are not vacuous

Reverted src/auth.js + src/server.js to pre-PR (0e09eb7) and re-ran only the METRICS_TOKEN suite:

7 tests: 3 pass, 4 FAIL against old code
  ✖ unauthenticated scrape returns 401            (old code: 200)
  ✖ wrong bearer token returns 401                (old code: 200)
  ✖ scrape returns 503 when unconfigured in prod  (old code: 200)
  ✖ unauth scrape returns 401 (not 503) when set  (old code: 200)

Exactly the 4 enforcement tests fail; the 3 that pass on old code are the dev-open regression lock and the two positive-path (correct-token → 200) tests, as expected. PR state restored; suite then re-run 3x at head: 7/7, 7/7, 7/7 (no flakiness).

3. Live-server matrix — real node src/server.js, ephemeral ports, temp DATABASE_PATH

Probe	Result	Expected
(a) NODE_ENV=production, unset → scrape	503 {"error":"Metrics not configured","hint":"Set the METRICS_TOKEN…"}	503 ✔
(a) startup log	1× METRICS_TOKEN not set — /metrics/prometheus returns 503 (production fails closed)	warn ✔
(a) /health, /ready	200, 200	unaffected ✔
(b) production + token: no token / wrong token / correct token	401 / 401 / 200	✔
(b) 200 payload	contains aep_events_received_total; grep -ci tenant = 0; only label keys used: method,route,status,le	aggregate, tenant-free ✔
(b) startup log	0 warnings (token set)	✔
(c) no NODE_ENV, unset	200 + …is open (dev mode) warning	dev-open ✔
(d) no NODE_ENV, token set: no token / correct	401 / 200; /health 200	✔

4. Edge probes

Probe	Result
NODE_ENV=prod (not exactly "production"), unset	200 — treated as dev, matches .env.example wording ✔
/v1/metrics/prometheus	404 — endpoint stays unversioned, consistent with VERSIONING.md ✔
METRICS_TOKEN= (empty string), production	503 + "not set" startup warning — empty ≡ unset ✔
METRICS_TOKEN= (empty string), dev	200 dev-open ✔ (re-verified on a second clean port after detecting a stale unrelated server on the first port — result held)
Authorization: <raw token> (no Bearer prefix)	401 ✔
Authorization: Basic <token>	401 ✔
JSON /metrics	untouched — still requireReadAccess (src/server.js:890), API-key path unaffected ✔

5. Docs spot-check

SECURITY.md §8 prometheus.yml snippet is correct: the authorization scrape-config block defaults type: Bearer, so credentials: <token> sends exactly Authorization: Bearer <token> — which extractBearer requires. Section numbering §1–§8 is clean. src/openapi.json does not document /metrics/prometheus (infra endpoint), so no spec drift there.

Findings

1. Minor — SETUP.md:544 — the endpoint reference table still reads | GET | /metrics/prometheus | none | … Unauthenticated — restrict at the network layer if needed. |. Now factually wrong (production: 503/401) and contradicts the PR's own updates to QUICK_REFERENCE/AUTH/SECURITY/OPERATIONS. Fix: change auth column to something like metrics token and the note to the METRICS_TOKEN behavior (legend on line 534 may need a [metrics] entry).
2. Nit — SECURITY.md:152–157 — the "§2 Set Strong Tokens" snippet generates only DASHBOARD_TOKEN/ADMIN_TOKEN; add METRICS_TOKEN=$(openssl rand -hex 32) for consistency with the updated checklist and §8.

No code-level findings: requireMetricsAuth mirrors the existing dashboard/admin idioms (per-request env read so tests can toggle, safeEqual timing-safe compare incl. length-mismatch path, bearer-only extraction), and CHANGELOG's historical "no auth" line is a changelog entry, correctly left as history.

Verdict: Approve (with the two doc touch-ups above — SETUP.md:544 is worth fixing before or immediately after merge since it's the endpoint reference table)

Every load-bearing claim in the PR body was reproduced empirically: gate counts (494/271) exact, all four enforcement tests proven discriminating against pre-change code, full 4-row behavior matrix + edge cases confirmed against live servers, payload confirmed aggregate and tenant-label-free, /health///ready/JSON-/metrics untouched, both startup-warning variants observed.

Posted by the independent validation context (shared gh identity with the author → comment, not formal approval).

[surpradhan]

Independent code review — PR #164 (feat(auth): gate /metrics/prometheus behind optional METRICS_TOKEN, head e8f74a1)

Reviewed the full diff against the parent commit; ran the gates and live-probed the behavior matrix and bypass variants against a real server.

Findings

Blockers: none.

Suggestion — SETUP.md still documents the endpoint as unauthenticated (stale doc the PR missed)

• SETUP.md:544 — the API-reference endpoint table still reads: | GET | /metrics/prometheus | none | Prometheus text format 0.0.4. Unauthenticated — restrict at the network layer if needed. |. This is now factually wrong on both counts (production 503s when METRICS_TOKEN is unset; token required when set) and contradicts AUTH.md/SECURITY.md §8. Suggested fix: Auth column → METRICS_TOKEN bearer, note → "Open in dev when METRICS_TOKEN is unset; 503 in production until it is set. Scrapers send Authorization: Bearer ."
• SETUP.md:600 — the §Observability paragraph ("Wire this into your existing Prometheus/Grafana stack for production monitoring") mentions no token; anyone following it in production hits a 503 with no pointer. Suggested fix: one sentence referencing METRICS_TOKEN + SECURITY.md §8.

Nit — src/openapi.json description omits the new gate

• src/openapi.json:6 — CLAUDE.md designates this spec the API source of truth. Its description names /metrics/prometheus among the infra endpoints and explicitly documents ADMIN_TOKEN for admin routes, but says nothing about the new METRICS_TOKEN requirement. Suggested fix: one sentence in the ## Authentication block (e.g. "The Prometheus scrape endpoint GET /metrics/prometheus is gated by the METRICS_TOKEN env var — bearer token when set; open in dev / 503 in production when unset."). Adding the path object itself is optional — it was never specced (like /ready), a pre-existing gap.

Observations (fine as-is, no action):

• No CHANGELOG entry — consistent with recent server-change precedent (#156/#163 added none).
• extractBearer trims whitespace, so Bearer <token> (double space) is accepted — pre-existing shared helper, same for admin auth; benign.
• Scope check: server is PARKED, but this closes the auth gap both #156 reviewers flagged — security fix, not feature growth. In-policy.
• Historical CHANGELOG lines (880, 1976) describing the old no-auth endpoint are release history and correctly left untouched.

What I verified

• Gates: npm run lint clean; unit 494/494; integration 271/271 — matches the PR's claims exactly.
• Behavior matrix live-probed (real server, not just the suite): dev+unset → 200; production+unset → 503 {error: "Metrics not configured", hint: …} + startup warning; set → 401 missing token, 401 wrong token, 200 correct bearer; production+set → 401 (not 503) without token. Both startup-warning variants observed in logs.
• Bypass probes (all negative): ?token= and X-API-Key rejected (bearer-only, as documented); trailing-slash and case variants (/Metrics/Prometheus) still route through the gate → 401; /x/../metrics/prometheus and //metrics/prometheus → 404; /v1/metrics/prometheus → 404 with and without a valid token (the v1 router never had this route, and tests/integration/server.test.js:4474 already regression-locks the 404).
• Timing safety: safeEqual → crypto.timingSafeEqual; the length-mismatch path does a dummy same-length compare whose duration depends only on attacker input length, never the secret. Comparison order (!provided || !safeEqual(...)) matches the admin middleware.
• Empty-token edge: .env.example's METRICS_TOKEN= / compose's ${METRICS_TOKEN:-} yield an empty string → falsy → treated as unset (prod 503); an empty string can never become an accepted credential. Same posture as DASHBOARD_TOKEN.
• Consistency: 503 envelope (error+hint) mirrors dashboard/admin unconfigured; 401 {error: "Invalid metrics token"} mirrors admin; header comment renumbered 1–4 correctly; module export added.
• Test hygiene: outer before/after save+restore with correct undefined guards; each inner describe fully re-specifies both env vars so no inter-describe leakage; node:test runs the suites serially (confirmed via per-request status logs in the run output); env restored before the file-level after. The four enforcement tests would demonstrably fail against the pre-change always-open route.
• Docs: AUTH.md (table, env vars, security note), SECURITY.md §8 (numbering flows §7→§8→checklist; authorization.credentials prometheus.yml syntax is correct — bearer is the default type), OPERATIONS.md checklist §8 cross-reference, QUICK_REFERENCE, docker-compose passthrough — all match the implemented behavior.
• Bonus hardening: unauthorized scrapes no longer reach db.getMetrics (auth precedes the DB call).
• Commit format feat(auth): … with no Co-Authored-By trailer; feature/ branch prefix is valid per CONTRIBUTING.

Verdict

Request Changes — docs-only: 2 actionable items (1 Suggestion: stale SETUP.md rows; 1 Nit: openapi.json description). The implementation, tests, and security posture are approve-quality; once SETUP.md and the spec description are aligned this is ready.

(Posted with --comment per repo convention — reviewer shares the author's gh identity.)

[surpradhan]

Round-1 findings from both reviews addressed in cb34d93:

• SETUP.md:544 (both reviews) — endpoint table row now shows metrics auth (METRICS_TOKEN bearer) with dev-open/production-503 semantics; added a [metrics] entry to the auth legend.
• SETUP.md:600 (code review) — Observability section now says production scraping requires METRICS_TOKEN via the Prometheus authorization scrape config, with a SECURITY.md §8 pointer.
• SECURITY.md §2 (validator nit) — token-minting snippet now includes METRICS_TOKEN=$(openssl rand -hex 32).
• src/openapi.json (code review nit) — Authentication description documents the metrics token and its unset behavior (path object intentionally not added — infra endpoints like /ready were never specced; pre-existing convention).

Observations acknowledged as no-change-needed per the reviews themselves: no CHANGELOG entry (matches #156/#163 precedent), extractBearer trim leniency (pre-existing, shared with admin auth), historical CHANGELOG lines untouched.

Gates re-run after the fixes: lint clean, unit 494/494, integration 271/271, openapi.json parses. Re-review round 2 incoming.

[surpradhan]

Independent code review — round 2 (PR #164, head cb34d93)

Re-reviewed the delta e8f74a1..cb34d93 (docs-only: SETUP.md, SECURITY.md, src/openapi.json) against my round-1 findings.

Round-1 findings — all resolved

• Suggestion (SETUP.md:544 stale table row) — fixed: Auth column now metrics, note reads "Open in dev when METRICS_TOKEN is unset; 503 in production until it is set. Scrapers send Authorization: Bearer <token>." Matches the live-verified behavior exactly, and the new [metrics] legend entry at SETUP.md:534 is consistent with the existing [admin]/[none] style.
• Suggestion (SETUP.md:600 Observability) — fixed: production scraping now points at METRICS_TOKEN + the Prometheus authorization scrape config with a SECURITY.md §8 cross-reference (section exists; relative link valid).
• Nit (src/openapi.json description) — fixed: the Authentication section now documents the metrics token with accurate unset semantics ("open in development and returns 503 in production (fail closed)"). Path object intentionally not added, consistent with the pre-existing infra-endpoint convention (/ready was never specced) — as I noted in round 1, that's fine.
• Validator's finding (SECURITY.md §2) — also fixed in this commit: METRICS_TOKEN=$(openssl rand -hex 32) minted and exported alongside the other two tokens, consistent with §8 and the checklist.

What I verified on cb34d93

• src/openapi.json parses as valid JSON (31 paths, description updated); the GET /openapi.json integration test that fetches and parses the served spec passes.
• No stale text remains: grep for "Unauthenticated" / "network layer" in SETUP.md → no hits; all remaining repo references to the old no-auth behavior are historical CHANGELOG entries (correctly untouched).
• No new inconsistencies introduced — every new doc claim matches the behavior I live-probed in round 1 (dev-open 200 / prod-unset 503 / 401 wrong-or-missing / 200 bearer).
• Gates re-run on the new head: npm run lint clean; unit 494/494; integration 271/271.
• Commit cb34d93 (docs: fix stale metrics-auth claims from PR #164 review round 1) follows the conventional format with no Co-Authored-By trailer.

Verdict

Approve — zero actionable items. All round-1 findings resolved cleanly; docs, spec, and implementation are now consistent.

(Posted with --comment per repo convention — reviewer shares the author's gh identity; treat this as my approval.)

[surpradhan]

Round-2 validation report (new head cb34d93)

Re-validated at the new PR head cb34d93 in the same isolated worktree. gh pr view confirms headRefOid = cb34d93…, and gh pr diff 164 is byte-identical to git diff 0e09eb7 cb34d93 locally (no uncommitted work missing from the PR).

Delta scope — docs-only, as claimed

git diff e8f74a1 cb34d93 touches exactly 3 files, +6/−5: SECURITY.md, SETUP.md, src/openapi.json. The openapi change is confined to the info.description string — no runtime code, no paths/security objects changed. On that basis I did not repeat the full live matrix; a spot-probe (below) confirms enforcement is unchanged.

Round-1 findings — all resolved (verified line-by-line)

Finding	Status
(validator #1) SETUP.md:544 stale "Unauthenticated — restrict at the network layer" row	Fixed — auth column now metrics, note states dev-open / production-503 / Authorization: Bearer <token>; legend (line 534) gained [metrics] = METRICS_TOKEN bearer
(validator #2) SECURITY.md §2 token snippet missing METRICS_TOKEN	Fixed — METRICS_TOKEN=$(openssl rand -hex 32) added and included in the export line
(code reviewer) SETUP.md Observability section	Fixed — now instructs setting METRICS_TOKEN, notes the production 503, points to SECURITY.md §8
(code reviewer) openapi.json Authentication description	Fixed — description now documents the metrics token, bearer header, and unset dev-open/production-503 semantics

Repo-wide re-grep: no remaining stale "unauthenticated" claims about /metrics/prometheus outside CHANGELOG.md (historical record, correctly untouched).

Gates at cb34d93 — green

npm run lint → exit 0 (clean)
npm test     → unit: 494/494 pass, 0 fail · integration: 271/271 pass, 0 fail (exit 0)

openapi.json integrity — parses and serves

• json.load('src/openapi.json') → parses OK; METRICS_TOKEN present in info.description.
• Live server (node src/server.js, ephemeral port, temp DATABASE_PATH, NODE_ENV=production METRICS_TOKEN=…): GET /openapi.json → 200, body re-parsed as valid JSON with the new description.
• Spot-probe on the same server confirms runtime behavior unchanged: /metrics/prometheus → 401 without token, 200 with correct bearer token.

Findings: zero actionable items.

Verdict: Approve — round-1 findings from both reviews are genuinely resolved, the delta is docs-only, gates are green, and the served OpenAPI document is intact.

Posted by the independent validation context (shared gh identity with the author → comment, not formal approval).