diff --git a/.env.example b/.env.example index 0a032c26..e490f7c9 100644 --- a/.env.example +++ b/.env.example @@ -335,6 +335,20 @@ MCP_DOCMOST_PASSWORD= # VictoriaMetrics/Prometheus reaching it as :/metrics. # METRICS_PORT=9464 # +# METRICS_BIND — interface the /metrics listener binds to. DEFAULT 127.0.0.1 +# (loopback only), so the unauthenticated endpoint is NOT exposed on all +# interfaces. If the scraper runs in a SEPARATE container and reaches this as +# docmost:9464, set METRICS_BIND=0.0.0.0 — but then also set METRICS_TOKEN +# and/or keep the port on a private network, since /metrics is otherwise open. +# METRICS_BIND=127.0.0.1 +# +# METRICS_TOKEN — optional Bearer token guarding /metrics. When set, every +# scrape MUST send `Authorization: Bearer ` (others get 401). Configure +# the scraper with the same bearer token (e.g. VictoriaMetrics/vmagent +# `bearer_token`, Prometheus `authorization.credentials`). Leave unset only +# when the endpoint is bound to loopback or an otherwise-trusted network. +# METRICS_TOKEN= +# # 2) CLIENT_TELEMETRY_ENABLED — the public client perf-telemetry sink. # OFF by default. When true, the unauthenticated POST /api/telemetry/vitals # endpoint is registered and browsers collect + send web-vitals / editor diff --git a/apps/server/src/integrations/metrics/metrics.server.spec.ts b/apps/server/src/integrations/metrics/metrics.server.spec.ts new file mode 100644 index 00000000..1589db31 --- /dev/null +++ b/apps/server/src/integrations/metrics/metrics.server.spec.ts @@ -0,0 +1,140 @@ +import { get as httpGet } from 'node:http'; +import { AddressInfo } from 'node:net'; +import { createServer } from 'node:http'; + +// Drive the metrics HTTP server without the load-time METRICS_PORT gate: mock the +// registry so isMetricsEnabled()/getMetricsRegistry() are always satisfied. What +// we assert is observed over a REAL socket (bind address, status codes), not on +// the mock. +jest.mock('./metrics.registry', () => ({ + isMetricsEnabled: () => true, + getMetricsRegistry: () => ({ + metrics: async () => '# HELP up test\nup 1\n', + contentType: 'text/plain; version=0.0.4', + }), +})); + +import { + startMetricsServer, + closeMetricsServer, + resolveMetricsBind, + resolveMetricsToken, +} from './metrics.server'; + +/** Find a free TCP port (the metrics server requires METRICS_PORT > 0). */ +function freePort(): Promise { + return new Promise((resolve, reject) => { + const s = createServer(); + s.once('error', reject); + s.listen(0, '127.0.0.1', () => { + const p = (s.address() as AddressInfo).port; + s.close(() => resolve(p)); + }); + }); +} + +/** Minimal GET against 127.0.0.1:port with optional Authorization header. */ +function req( + port: number, + headers: Record = {}, +): Promise<{ status: number; body: string }> { + return new Promise((resolve, reject) => { + const r = httpGet( + { host: '127.0.0.1', port, path: '/metrics', headers }, + (res) => { + let body = ''; + res.on('data', (c) => (body += c)); + res.on('end', () => + resolve({ status: res.statusCode ?? 0, body }), + ); + }, + ); + r.on('error', reject); + }); +} + +describe('metrics server bind + auth (#486)', () => { + const saved = { + bind: process.env.METRICS_BIND, + token: process.env.METRICS_TOKEN, + port: process.env.METRICS_PORT, + }; + + afterEach(async () => { + await closeMetricsServer(); + process.env.METRICS_BIND = saved.bind; + process.env.METRICS_TOKEN = saved.token; + process.env.METRICS_PORT = saved.port; + delete process.env.METRICS_BIND; + delete process.env.METRICS_TOKEN; + }); + + describe('resolveMetricsBind', () => { + it('defaults to loopback 127.0.0.1', () => { + delete process.env.METRICS_BIND; + expect(resolveMetricsBind()).toBe('127.0.0.1'); + }); + it('honours the METRICS_BIND override', () => { + process.env.METRICS_BIND = '0.0.0.0'; + expect(resolveMetricsBind()).toBe('0.0.0.0'); + }); + it('treats a blank override as unset (loopback)', () => { + process.env.METRICS_BIND = ' '; + expect(resolveMetricsBind()).toBe('127.0.0.1'); + }); + }); + + describe('resolveMetricsToken', () => { + it('is null when unset', () => { + delete process.env.METRICS_TOKEN; + expect(resolveMetricsToken()).toBeNull(); + }); + it('returns the trimmed token when set', () => { + process.env.METRICS_TOKEN = ' s3cret '; + expect(resolveMetricsToken()).toBe('s3cret'); + }); + }); + + it('binds to loopback by default and serves /metrics without auth when no token', async () => { + delete process.env.METRICS_BIND; + delete process.env.METRICS_TOKEN; + const port = await freePort(); + process.env.METRICS_PORT = String(port); + + const server = startMetricsServer(); + expect(server).not.toBeNull(); + await new Promise((resolve) => { + if (server!.listening) resolve(); + else server!.once('listening', () => resolve()); + }); + // OBSERVABLE: the listener bound to loopback, not 0.0.0.0. + expect((server!.address() as AddressInfo).address).toBe('127.0.0.1'); + + const res = await req(port); + expect(res.status).toBe(200); + expect(res.body).toContain('up 1'); + }); + + it('rejects unauthenticated scrapes with 401 and accepts the exact Bearer token', async () => { + delete process.env.METRICS_BIND; + process.env.METRICS_TOKEN = 'topsecret'; + const port = await freePort(); + process.env.METRICS_PORT = String(port); + + const server = startMetricsServer(); + expect(server).not.toBeNull(); + + // No auth -> 401. + const noAuth = await req(port); + expect(noAuth.status).toBe(401); + + // Wrong token -> 401. + const wrong = await req(port, { authorization: 'Bearer nope' }); + expect(wrong.status).toBe(401); + + // Correct token -> 200 with the metrics body. + const ok = await req(port, { authorization: 'Bearer topsecret' }); + expect(ok.status).toBe(200); + expect(ok.body).toContain('up 1'); + }); +}); diff --git a/apps/server/src/integrations/metrics/metrics.server.ts b/apps/server/src/integrations/metrics/metrics.server.ts index a5c4d3e8..577cbecd 100644 --- a/apps/server/src/integrations/metrics/metrics.server.ts +++ b/apps/server/src/integrations/metrics/metrics.server.ts @@ -16,6 +16,30 @@ import { getMetricsRegistry, isMetricsEnabled } from './metrics.registry'; */ let metricsServer: Server | null = null; +/** + * Interface the metrics endpoint binds to. Defaults to LOOPBACK (127.0.0.1) so + * the unauthenticated `/metrics` surface is NOT exposed on all interfaces by + * default — the old `0.0.0.0` bind put an auth-less endpoint on every interface. + * Deployments where the scraper runs in a SEPARATE container (and reaches this as + * `docmost:9464`) set `METRICS_BIND=0.0.0.0`, ideally together with METRICS_TOKEN + * and/or a private network so the port is not world-readable. + */ +export function resolveMetricsBind(): string { + const raw = (process.env.METRICS_BIND ?? '').trim(); + return raw.length > 0 ? raw : '127.0.0.1'; +} + +/** + * Optional Bearer token guarding `/metrics`. When `METRICS_TOKEN` is set, every + * scrape must present `Authorization: Bearer `; unset (default) leaves the + * endpoint open (safe when bound to loopback / a trusted network). Returns the + * trimmed token or null when unset/blank. + */ +export function resolveMetricsToken(): string | null { + const raw = (process.env.METRICS_TOKEN ?? '').trim(); + return raw.length > 0 ? raw : null; +} + export function startMetricsServer(): Server | null { if (!isMetricsEnabled()) return null; @@ -31,8 +55,22 @@ export function startMetricsServer(): Server | null { return null; } + const bind = resolveMetricsBind(); + const token = resolveMetricsToken(); + const server = createServer(async (req, res) => { if (req.method === 'GET' && req.url === '/metrics') { + // Optional Bearer auth: reject scrapes without the exact token when one is + // configured. This is the auth layer the old all-interfaces bind lacked. + if (token) { + const auth = req.headers['authorization']; + if (auth !== `Bearer ${token}`) { + res.statusCode = 401; + res.setHeader('WWW-Authenticate', 'Bearer'); + res.end(); + return; + } + } try { const body = await register.metrics(); res.setHeader('Content-Type', register.contentType); @@ -48,10 +86,14 @@ export function startMetricsServer(): Server | null { res.end(); }); - // Bind on all interfaces: the scraper (VictoriaMetrics) reaches this from - // another container as docmost:9464. The port is not published to the host. - server.listen(port, '0.0.0.0', () => { - logger.log(`Metrics endpoint listening on :${port}/metrics`); + // Bind to loopback by default so the auth-less endpoint is not exposed on all + // interfaces. Set METRICS_BIND=0.0.0.0 (ideally with METRICS_TOKEN) when the + // scraper runs in a separate container and reaches this as docmost:9464. + server.listen(port, bind, () => { + logger.log( + `Metrics endpoint listening on ${bind}:${port}/metrics` + + (token ? ' (Bearer auth required)' : ''), + ); }); server.on('error', (err) => {