diff --git a/README.md b/README.md index be755d9..46ca400 100644 --- a/README.md +++ b/README.md @@ -403,6 +403,7 @@ All configuration is managed through `src/config/` and validated at startup usin | `ACTIVE_COLOR` | `blue` | Active backend color for blue-green routing | | `BLUE_PORT` | `3001` | Port for the 'blue' backend | | `GREEN_PORT` | `3002` | Port for the 'green' backend | +| `HTTP_METRICS_ROUTE_LABEL_LIMIT` | `100` | Maximum distinct HTTP route template labels before new routes are recorded as `other` | ## API Endpoints @@ -499,6 +500,17 @@ Each probe reports one of three statuses: In production (NODE_ENV=production), probe details are stripped to prevent topology leakage to unauthenticated callers. +### Metrics Cardinality Guard + +HTTP metrics use Express route templates for the `route` label, for example +`/api/v1/contracts/:id`, instead of concrete request paths with embedded user +or resource identifiers. Unmatched requests are recorded as `unmatched`. + +`HTTP_METRICS_ROUTE_LABEL_LIMIT` caps the number of distinct route template +labels retained by `http_requests_total` and `http_request_duration_seconds`. +After the cap is reached, newly observed route templates are recorded as +`other`, while existing route labels, `method`, and `status_code` remain intact. + ### Contracts - `GET /api/v1/contracts` - List contracts (placeholder) diff --git a/docs/backend/environment-variables.md b/docs/backend/environment-variables.md index 1e7629f..2eb7789 100644 --- a/docs/backend/environment-variables.md +++ b/docs/backend/environment-variables.md @@ -227,6 +227,7 @@ Defined in: `src/logger.ts`, `src/middleware/metricsAuth.ts`, `src/middleware/ht | `METRICS_AUTH_TOKEN` | No | _(none)_ | Bearer token required to access the `/metrics` endpoint. If unset, the endpoint is open (acceptable in development, not in production). **Treat as a secret.** | | `TRUST_PROXY` | No | `false` | Set to `true` to trust the `X-Forwarded-For` header for client IP resolution. Enable only when running behind a trusted reverse proxy. | | `SERVICE_NAME` | No | `talenttrust-backend` | Service name label attached to Prometheus metrics. | +| `HTTP_METRICS_ROUTE_LABEL_LIMIT` | No | `100` | Caps distinct HTTP route template labels for `http_requests_total` and `http_request_duration_seconds`; new routes beyond the cap are recorded as `other`. | --- diff --git a/docs/backend/observability.md b/docs/backend/observability.md index 9805467..467637a 100644 --- a/docs/backend/observability.md +++ b/docs/backend/observability.md @@ -61,6 +61,7 @@ If auth is missing/invalid, route returns `401`. | `SERVICE_NAME` | `talenttrust-backend` | Name used in health payload and metrics labels | | `METRICS_ENABLED` | `true` | Set to `false` to return `404` on `/metrics` | | `METRICS_AUTH_TOKEN` | _unset_ | Enables bearer-token protection for `/metrics` | +| `HTTP_METRICS_ROUTE_LABEL_LIMIT` | `100` | Maximum distinct HTTP route template labels before new routes are recorded as `other` | ## Exported Prometheus Metrics @@ -87,6 +88,9 @@ Mitigation: Mitigation: - Route labels use bounded path values from Express route templates. +- Route labels include the static mount path plus the matched template, for example `/api/v1/contracts/:id`. +- Unmatched requests collapse into the shared `unmatched` bucket. +- `HTTP_METRICS_ROUTE_LABEL_LIMIT` caps distinct route labels; routes first seen after the cap are recorded as `other`. - No request payload, IDs, or user-provided fields are added as labels. - Rate limit metrics use redacted provider IDs (first 4 chars + `****`) to bound cardinality. diff --git a/src/app.ts b/src/app.ts index 88568c9..680c15c 100644 --- a/src/app.ts +++ b/src/app.ts @@ -64,6 +64,8 @@ export function createApp(options?: AppFactoryOptions): express.Application { const metricsService = new MetricsService( process.env['SERVICE_NAME'] ?? 'talenttrust-backend', + undefined, + { httpRouteLabelLimit: env.HTTP_METRICS_ROUTE_LABEL_LIMIT }, ); // ── Middleware ──────────────────────────────────────────────────────────── diff --git a/src/config/env.schema.ts b/src/config/env.schema.ts index 00ea95c..9bf5506 100644 --- a/src/config/env.schema.ts +++ b/src/config/env.schema.ts @@ -144,6 +144,11 @@ export const envSchema = z.object({ }) .pipe(z.record(z.string(), z.number()).optional()), + HTTP_METRICS_ROUTE_LABEL_LIMIT: z.string() + .default('100') + .transform((val) => parseInt(val, 10)) + .pipe(z.number().int().positive().max(10000)), + // Reputation Scoring Configuration REPUTATION_DECAY_LAMBDA: z.string() .default('0.005') diff --git a/src/config/environment.test.ts b/src/config/environment.test.ts index 5455869..c98a02f 100644 --- a/src/config/environment.test.ts +++ b/src/config/environment.test.ts @@ -255,6 +255,25 @@ describe('Environment Configuration', () => { }); }); + it('should parse HTTP_METRICS_ROUTE_LABEL_LIMIT with a safe default', () => { + delete process.env.HTTP_METRICS_ROUTE_LABEL_LIMIT; + expect(validateEnv(process.env).HTTP_METRICS_ROUTE_LABEL_LIMIT).toBe(100); + + process.env.HTTP_METRICS_ROUTE_LABEL_LIMIT = '250'; + expect(validateEnv(process.env).HTTP_METRICS_ROUTE_LABEL_LIMIT).toBe(250); + }); + + it('should reject invalid HTTP_METRICS_ROUTE_LABEL_LIMIT values', () => { + process.env.HTTP_METRICS_ROUTE_LABEL_LIMIT = '0'; + expect(() => validateEnv(process.env)).toThrow(); + + process.env.HTTP_METRICS_ROUTE_LABEL_LIMIT = '10001'; + expect(() => validateEnv(process.env)).toThrow(); + + process.env.HTTP_METRICS_ROUTE_LABEL_LIMIT = 'not-a-number'; + expect(() => validateEnv(process.env)).toThrow(); + }); + it('should reject malformed ROUTE_BODY_LIMITS', () => { // Missing path prefix '/' process.env.ROUTE_BODY_LIMITS = 'api/upload:1024'; @@ -307,4 +326,4 @@ describe('Environment Configuration', () => { expect(() => validateEnv(process.env)).toThrow(); }); }); -}); \ No newline at end of file +}); diff --git a/src/observability/metrics-service.test.ts b/src/observability/metrics-service.test.ts index 8fccb4c..fe72eb8 100644 --- a/src/observability/metrics-service.test.ts +++ b/src/observability/metrics-service.test.ts @@ -1,12 +1,45 @@ import { Registry } from 'prom-client'; +import { EventEmitter } from 'events'; +import { NextFunction, Request, Response } from 'express'; import { MetricsService } from './metrics-service'; -function makeService() { +function makeService(httpRouteLabelLimit?: number) { const register = new Registry(); - const service = new MetricsService('test', register); + const service = new MetricsService('test', register, { httpRouteLabelLimit }); return { service, register }; } +function recordHttpRequest( + service: MetricsService, + request: { + method?: string; + baseUrl?: string; + routePath?: string; + statusCode?: number; + }, +) { + const response = new EventEmitter() as Response & EventEmitter; + response.statusCode = request.statusCode ?? 200; + + const req = { + method: request.method ?? 'GET', + baseUrl: request.baseUrl ?? '', + route: request.routePath === undefined ? undefined : { path: request.routePath }, + } as unknown as Request; + + const next = jest.fn() as NextFunction; + + service.trackHttpRequest(req, response, next); + expect(next).toHaveBeenCalledTimes(1); + response.emit('finish'); +} + +async function routeLabels(register: Registry): Promise { + const metrics = await register.getMetricsAsJSON(); + const counter = metrics.find((m) => m.name === 'http_requests_total'); + return ((counter?.values ?? []) as any[]).map((value) => value.labels.route); +} + describe('MetricsService — webhook metrics', () => { it('increments webhook_deliveries_total with outcome=success', async () => { const { service, register } = makeService(); @@ -65,3 +98,53 @@ describe('MetricsService — webhook metrics', () => { expect((gauge!.values as any[])[0].value).toBe(5); }); }); + +describe('MetricsService — HTTP route labels', () => { + it('uses the mounted Express route template instead of concrete paths', async () => { + const { service, register } = makeService(); + + recordHttpRequest(service, { + method: 'GET', + baseUrl: '/api/v1/contracts', + routePath: '/:id/metadata/:metadataId', + }); + + expect(await routeLabels(register)).toContain('/api/v1/contracts/:id/metadata/:metadataId'); + }); + + it('collapses unmatched requests into one bucket', async () => { + const { service, register } = makeService(); + + recordHttpRequest(service, { routePath: undefined, statusCode: 404 }); + recordHttpRequest(service, { routePath: undefined, statusCode: 404 }); + + expect(await routeLabels(register)).toEqual(['unmatched']); + }); + + it('keeps new route labels through the configured cap boundary', async () => { + const { service, register } = makeService(2); + + recordHttpRequest(service, { routePath: '/health' }); + recordHttpRequest(service, { routePath: '/metrics' }); + + expect(await routeLabels(register)).toEqual(expect.arrayContaining(['/health', '/metrics'])); + }); + + it('routes excess distinct templates to other under a high-cardinality flood', async () => { + const { service, register } = makeService(3); + + for (let index = 0; index < 20; index += 1) { + recordHttpRequest(service, { + baseUrl: '/api/v1', + routePath: `/resource-${index}/:id`, + }); + } + + const labels = await routeLabels(register); + const distinctLabels = new Set(labels); + + expect(distinctLabels.size).toBe(4); + expect(labels).toContain('other'); + expect(labels).not.toContain('/api/v1/resource-19/:id'); + }); +}); diff --git a/src/observability/metrics-service.ts b/src/observability/metrics-service.ts index 9e18be3..cc23488 100644 --- a/src/observability/metrics-service.ts +++ b/src/observability/metrics-service.ts @@ -28,6 +28,14 @@ const HEALTH_STATUS_VALUE: Record = { down: 0, }; +const DEFAULT_HTTP_ROUTE_LABEL_LIMIT = 100; +const OTHER_ROUTE_LABEL = 'other'; +const UNMATCHED_ROUTE_LABEL = 'unmatched'; + +export interface MetricsServiceOptions { + httpRouteLabelLimit?: number; +} + /** * Manages Prometheus metrics registration and request instrumentation. */ @@ -50,10 +58,19 @@ export class MetricsService implements MetricsServiceLike { private readonly webhookRateLimitQueueDepth: Gauge; + private readonly httpRouteLabelLimit: number; + + private readonly observedHttpRouteLabels = new Set(); + private rateLimitStopSampling: (() => void) | null = null; - constructor(private readonly serviceName: string, register?: Registry) { + constructor( + private readonly serviceName: string, + register?: Registry, + options: MetricsServiceOptions = {}, + ) { this.register = register ?? new Registry(); + this.httpRouteLabelLimit = options.httpRouteLabelLimit ?? DEFAULT_HTTP_ROUTE_LABEL_LIMIT; collectDefaultMetrics({ register: this.register, prefix: `${sanitizeMetricPrefix(serviceName)}_`, @@ -117,7 +134,7 @@ export class MetricsService implements MetricsServiceLike { res.on('finish', () => { const duration = Number(process.hrtime.bigint() - start) / 1_000_000_000; - const route = extractRoute(req); + const route = this.boundRouteLabel(extractRoute(req)); const labels = { method: req.method, route, @@ -169,6 +186,19 @@ export class MetricsService implements MetricsServiceLike { getMetrics(): Promise { return this.register.metrics(); } + + private boundRouteLabel(route: string): string { + if (this.observedHttpRouteLabels.has(route)) { + return route; + } + + if (this.observedHttpRouteLabels.size < this.httpRouteLabelLimit) { + this.observedHttpRouteLabels.add(route); + return route; + } + + return OTHER_ROUTE_LABEL; + } } function sanitizeMetricPrefix(input: string): string { @@ -176,12 +206,61 @@ function sanitizeMetricPrefix(input: string): string { return sanitized.length > 0 ? sanitized : 'service'; } +/** + * Returns a bounded, non-user-controlled route label for HTTP metrics. + * + * Express exposes the matched route template at `req.route.path`; joining it + * with the static mount point in `req.baseUrl` preserves useful labels such as + * `/api/v1/contracts/:id` without using concrete request paths that may contain + * attacker-controlled identifiers. Requests that never match a route collapse + * into one shared bucket. + */ function extractRoute(req: Request): string { - if (req.route?.path) { - return String(req.route.path); + const routePath = formatExpressPath(req.route?.path); + if (routePath === null) { + return UNMATCHED_ROUTE_LABEL; + } + + const baseUrl = normalizeRoutePart(req.baseUrl); + const route = joinRouteParts(baseUrl, routePath); + return route.length > 0 ? route : '/'; +} + +function formatExpressPath(path: unknown): string | null { + if (typeof path === 'string') { + return normalizeRoutePart(path); + } + + if (path instanceof RegExp) { + return path.toString(); + } + + if (Array.isArray(path)) { + const parts = path.map(formatExpressPath).filter((part): part is string => part !== null); + return parts.length > 0 ? parts.join('|') : null; + } + + return null; +} + +function normalizeRoutePart(part: string | undefined): string { + if (!part || part === '/') { + return ''; + } + + return part.startsWith('/') ? part : `/${part}`; +} + +function joinRouteParts(baseUrl: string, routePath: string): string { + if (!baseUrl) { + return routePath; + } + + if (!routePath) { + return baseUrl; } - return 'unmatched'; + return `${baseUrl}${routePath}`; }