diff --git a/backend/.env.example b/backend/.env.example index 2a77a906..2239951e 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -51,6 +51,18 @@ DATABASE_PASSWORD=your_database_password DATABASE_USERNAME=your_database_username DATABASE_PORT=5432 DATABASE_HOST=your_database_host +# Connection pool bounds (issue #1778) — tune per environment (e.g. lower in +# staging, higher under prod load) without a code change. Defaults match +# node-postgres's own defaults (min 2, max 10) when unset. +DB_POOL_MIN=2 +DB_POOL_MAX=10 + +# Metrics endpoint access control (issue #1781) +# Shared secret the /metrics scrape endpoint requires as +# `Authorization: Bearer ` (the standard shape for a Prometheus +# bearer_token scrape config). Required outside local development — the +# endpoint refuses all requests in production if this is unset. +METRICS_ACCESS_TOKEN=change-me-in-every-environment # Auth / JWT # JWT_SECRET is read by src/auth/auth.module.ts. diff --git a/backend/src/app.module.ts b/backend/src/app.module.ts index 968b0a62..c0a06459 100644 --- a/backend/src/app.module.ts +++ b/backend/src/app.module.ts @@ -1,5 +1,5 @@ import { MiddlewareConsumer, Module, NestModule } from '@nestjs/common'; -import { APP_GUARD } from '@nestjs/core'; +import { APP_GUARD, APP_INTERCEPTOR } from '@nestjs/core'; import { BullModule } from '@nestjs/bull'; import { CacheModule } from '@nestjs/cache-manager'; import { ScheduleModule } from '@nestjs/schedule'; @@ -18,6 +18,7 @@ import { HealthModule } from './health/health.module'; import { MetricsService } from './common/metrics.service'; import { MetricsController } from './common/metrics.controller'; import { RequestContextMiddleware } from './common/request-context.middleware'; +import { RequestDurationInterceptor } from './common/request-duration.interceptor'; import { TracingMiddleware } from './common/tracing.middleware'; @Module({ @@ -47,6 +48,13 @@ import { TracingMiddleware } from './common/tracing.middleware'; database: config.get('DATABASE_NAME'), autoLoadEntities: true, synchronize: false, + // Env-configurable pool bounds (issue #1778) — see + // src/database/data-source.ts for the matching CLI/migration + // DataSource and the defaults these fall back to. + extra: { + min: config.get('DB_POOL_MIN', 2), + max: config.get('DB_POOL_MAX', 10), + }, }), }), // Backs the Soroban escrow submission queue (issue #1574) — see @@ -88,6 +96,10 @@ import { TracingMiddleware } from './common/tracing.middleware'; provide: APP_GUARD, useClass: ThrottlerGuard, }, + { + provide: APP_INTERCEPTOR, + useClass: RequestDurationInterceptor, + }, ], }) export class AppModule implements NestModule { diff --git a/backend/src/common/http-exception.filter.spec.ts b/backend/src/common/http-exception.filter.spec.ts new file mode 100644 index 00000000..8f196007 --- /dev/null +++ b/backend/src/common/http-exception.filter.spec.ts @@ -0,0 +1,87 @@ +import { ArgumentsHost, HttpException, HttpStatus } from '@nestjs/common'; +import { AllExceptionsFilter, redactSensitive } from './http-exception.filter'; + +describe('redactSensitive (issue #1779)', () => { + it('redacts key=value style secrets and tokens', () => { + expect(redactSensitive('auth failed: token=abc123.def456')).toBe( + 'auth failed: token=[REDACTED]', + ); + expect(redactSensitive('bad request, password: "hunter2"')).toBe( + 'bad request, password=[REDACTED]', + ); + expect(redactSensitive('Authorization=Bearer sk_live_abcdef')).toBe( + 'Authorization=[REDACTED]', + ); + }); + + it('redacts credentials inside a connection string, keeping scheme/host', () => { + expect( + redactSensitive( + 'connect ECONNREFUSED postgres://appuser:s3cr3t@db.internal:5432/app', + ), + ).toBe('connect ECONNREFUSED postgres://[REDACTED]@db.internal:5432/app'); + }); + + it('redacts a bare JWT-shaped token with no key name attached', () => { + const jwt = + 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U'; + expect(redactSensitive(`invalid session for ${jwt}`)).toBe( + 'invalid session for [REDACTED]', + ); + }); + + it('leaves ordinary messages untouched', () => { + expect(redactSensitive('Booking not found')).toBe('Booking not found'); + }); +}); + +describe('AllExceptionsFilter', () => { + function buildHost(): { + host: ArgumentsHost; + json: jest.Mock; + status: jest.Mock; + } { + const json = jest.fn(); + const status = jest.fn().mockReturnValue({ json }); + const response = { status, getHeader: jest.fn() }; + const request = { method: 'GET', url: '/wallets/link' }; + const host = { + switchToHttp: () => ({ + getResponse: () => response, + getRequest: () => request, + }), + } as unknown as ArgumentsHost; + return { host, json, status }; + } + + it('redacts sensitive content out of the client-facing message', () => { + const filter = new AllExceptionsFilter(); + const { host, json, status } = buildHost(); + + filter.catch( + new HttpException( + 'upstream rejected token=abc.def.ghi', + HttpStatus.BAD_GATEWAY, + ), + host, + ); + + expect(status).toHaveBeenCalledWith(HttpStatus.BAD_GATEWAY); + expect(json).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'upstream rejected token=[REDACTED]', + }), + ); + }); + + it('never leaks a raw non-HttpException message', () => { + const filter = new AllExceptionsFilter(); + const { host, json } = buildHost(); + + filter.catch(new Error('secret=topsecret while connecting'), host); + + expect(json).toHaveBeenCalledWith( + expect.objectContaining({ message: 'Internal server error' }), + ); + }); +}); diff --git a/backend/src/common/http-exception.filter.ts b/backend/src/common/http-exception.filter.ts index b495df19..336c85cb 100644 --- a/backend/src/common/http-exception.filter.ts +++ b/backend/src/common/http-exception.filter.ts @@ -9,6 +9,39 @@ import { import { Request, Response } from 'express'; import { currentRequestId } from './request-context'; +// Field names whose value should never reach the client, wherever they show +// up inside a `key: value`, `key=value`, or `key="value"` style substring of +// an error message (issue #1779) — e.g. a raw driver error that happens to +// echo back part of the connection options it was given. +const SENSITIVE_KEY_PATTERN = + /\b((?:api[_-]?)?(?:secret|token|password|passwd|pwd|auth(?:orization)?)[a-z_]*)\s*[:=]\s*("[^"]*"|'[^']*'|(?:Bearer\s+)?\S+)/gi; + +// `scheme://user:pass@host` connection strings (Postgres, MySQL, MongoDB, +// Redis, AMQP, ...) — the credential portion is redacted, the scheme/host +// are left intact since they're useful for debugging and not secret. +const CONNECTION_STRING_PATTERN = + /([a-z][a-z0-9+.-]*:\/\/)[^:/?#\s]+:[^@/?#\s]+@/gi; + +// A standalone bearer/JWT-shaped token (three dot-separated base64url +// segments), independent of any surrounding key name. +const JWT_PATTERN = + /\b[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\b/g; + +/** + * Strips tokens, secrets, and connection-string credentials out of a + * message before it can reach an HTTP response (issue #1779). Applied + * unconditionally (not just in non-dev environments) since a client-facing + * error payload should never carry these regardless of environment; the + * full, unredacted detail is still available server-side in the log line + * this filter writes below. + */ +export function redactSensitive(message: string): string { + return message + .replace(CONNECTION_STRING_PATTERN, '$1[REDACTED]@') + .replace(SENSITIVE_KEY_PATTERN, '$1=[REDACTED]') + .replace(JWT_PATTERN, '[REDACTED]'); +} + @Catch() export class AllExceptionsFilter implements ExceptionFilter { private readonly logger = new Logger(AllExceptionsFilter.name); @@ -21,15 +54,19 @@ export class AllExceptionsFilter implements ExceptionFilter { exception instanceof HttpException ? exception.getStatus() : HttpStatus.INTERNAL_SERVER_ERROR; - const message = + const message = redactSensitive( exception instanceof HttpException ? exception.message - : 'Internal server error'; + : 'Internal server error', + ); + // Stack traces (which can contain far more than the message, e.g. full + // query text) are only ever logged server-side here, never sent to the + // client in the response body below. this.logger.error( `${request.method} ${request.url} failed${ currentRequestId() ? ` requestId=${currentRequestId()}` : '' - }: ${exception instanceof Error ? exception.stack ?? exception.message : String(exception)}`, + }: ${exception instanceof Error ? (exception.stack ?? exception.message) : String(exception)}`, ); response.status(status).json({ diff --git a/backend/src/common/metrics-auth.guard.spec.ts b/backend/src/common/metrics-auth.guard.spec.ts new file mode 100644 index 00000000..1d59a105 --- /dev/null +++ b/backend/src/common/metrics-auth.guard.spec.ts @@ -0,0 +1,68 @@ +import { ExecutionContext, UnauthorizedException } from '@nestjs/common'; +import { MetricsAuthGuard } from './metrics-auth.guard'; + +describe('MetricsAuthGuard (issue #1781)', () => { + const ORIGINAL_ENV = { ...process.env }; + + afterEach(() => { + process.env = { ...ORIGINAL_ENV }; + }); + + function buildContext(authorizationHeader?: string): ExecutionContext { + const request = { + headers: authorizationHeader + ? { authorization: authorizationHeader } + : {}, + }; + return { + switchToHttp: () => ({ + getRequest: () => request, + }), + } as unknown as ExecutionContext; + } + + it('allows a request bearing the correct token', () => { + process.env.METRICS_ACCESS_TOKEN = 'correct-horse-battery-staple'; + const guard = new MetricsAuthGuard(); + + expect( + guard.canActivate(buildContext('Bearer correct-horse-battery-staple')), + ).toBe(true); + }); + + it('rejects a request with the wrong token', () => { + process.env.METRICS_ACCESS_TOKEN = 'correct-horse-battery-staple'; + const guard = new MetricsAuthGuard(); + + expect(() => guard.canActivate(buildContext('Bearer wrong-token'))).toThrow( + UnauthorizedException, + ); + }); + + it('rejects a request with no Authorization header at all', () => { + process.env.METRICS_ACCESS_TOKEN = 'correct-horse-battery-staple'; + const guard = new MetricsAuthGuard(); + + expect(() => guard.canActivate(buildContext())).toThrow( + UnauthorizedException, + ); + }); + + it('refuses every request in production when no token is configured', () => { + delete process.env.METRICS_ACCESS_TOKEN; + process.env.NODE_ENV = 'production'; + const guard = new MetricsAuthGuard(); + + expect(() => guard.canActivate(buildContext())).toThrow( + UnauthorizedException, + ); + }); + + it('allows requests through in development when no token is configured', () => { + delete process.env.METRICS_ACCESS_TOKEN; + process.env.NODE_ENV = 'development'; + const guard = new MetricsAuthGuard(); + + expect(guard.canActivate(buildContext())).toBe(true); + }); +}); diff --git a/backend/src/common/metrics-auth.guard.ts b/backend/src/common/metrics-auth.guard.ts new file mode 100644 index 00000000..54e4a2ac --- /dev/null +++ b/backend/src/common/metrics-auth.guard.ts @@ -0,0 +1,79 @@ +import { + CanActivate, + ExecutionContext, + Injectable, + Logger, + UnauthorizedException, +} from '@nestjs/common'; +import { timingSafeEqual } from 'crypto'; +import { Request } from 'express'; + +/** + * Restricts the Prometheus scrape endpoint (issue #1781). Previously + * MetricsController served request volumes and error rates by route with + * no access control at all, relying entirely on the operator to restrict + * it at the network level. + * + * Requires an `Authorization: Bearer ` header + * matching the METRICS_ACCESS_TOKEN env var (the standard shape for a + * Prometheus `bearer_token` scrape config). Network-level restriction + * (e.g. only allowing the endpoint from an internal subnet) remains a + * valid, complementary option — this guard covers the case where that + * alone isn't in place. + * + * With no token configured: refuses every request outside development, + * since shipping to staging/prod without ever setting the token should + * fail closed rather than silently stay world-readable. In development + * it allows the request through (logging a one-time warning) so local + * `curl localhost:PORT/metrics` keeps working without extra setup. + */ +@Injectable() +export class MetricsAuthGuard implements CanActivate { + private readonly logger = new Logger(MetricsAuthGuard.name); + private warnedNoTokenConfigured = false; + + canActivate(context: ExecutionContext): boolean { + const configuredToken = process.env.METRICS_ACCESS_TOKEN; + + if (!configuredToken) { + if (process.env.NODE_ENV === 'production') { + throw new UnauthorizedException( + 'Metrics endpoint is not configured for access (METRICS_ACCESS_TOKEN unset)', + ); + } + if (!this.warnedNoTokenConfigured) { + this.warnedNoTokenConfigured = true; + this.logger.warn( + 'METRICS_ACCESS_TOKEN is not set — /metrics is unauthenticated. ' + + 'Set it before deploying outside local development.', + ); + } + return true; + } + + const request = context.switchToHttp().getRequest(); + const presentedToken = this.extractToken(request); + + if (!presentedToken || !this.matches(presentedToken, configuredToken)) { + throw new UnauthorizedException('Invalid or missing metrics token'); + } + + return true; + } + + private extractToken(request: Request): string | undefined { + const [type, token] = request.headers.authorization?.split(' ') ?? []; + return type === 'Bearer' ? token : undefined; + } + + /** Constant-time comparison so a wrong-length/wrong-value guess can't be + * timed to narrow down the correct token byte by byte. */ + private matches(presented: string, configured: string): boolean { + const presentedBuf = Buffer.from(presented); + const configuredBuf = Buffer.from(configured); + if (presentedBuf.length !== configuredBuf.length) { + return false; + } + return timingSafeEqual(presentedBuf, configuredBuf); + } +} diff --git a/backend/src/common/metrics.controller.spec.ts b/backend/src/common/metrics.controller.spec.ts index 44bddca9..378d8861 100644 --- a/backend/src/common/metrics.controller.spec.ts +++ b/backend/src/common/metrics.controller.spec.ts @@ -1,6 +1,8 @@ import { Test } from '@nestjs/testing'; +import { Reflector } from '@nestjs/core'; import { MetricsController } from './metrics.controller'; import { MetricsService } from './metrics.service'; +import { MetricsAuthGuard } from './metrics-auth.guard'; describe('MetricsController', () => { let controller: MetricsController; @@ -37,4 +39,12 @@ describe('MetricsController', () => { expect(body).toContain('managehub_manual_review_queue_depth 3'); expect(service.renderPrometheus).toHaveBeenCalled(); }); + + it('is gated by MetricsAuthGuard (issue #1781)', () => { + const guards = new Reflector().get('__guards__', MetricsController) as + | unknown[] + | undefined; + expect(guards).toBeDefined(); + expect(guards).toContainEqual(MetricsAuthGuard); + }); }); diff --git a/backend/src/common/metrics.controller.ts b/backend/src/common/metrics.controller.ts index 9f5c2be0..370cfbd2 100644 --- a/backend/src/common/metrics.controller.ts +++ b/backend/src/common/metrics.controller.ts @@ -1,20 +1,24 @@ -import { Controller, Get, Header } from '@nestjs/common'; +import { Controller, Get, Header, UseGuards } from '@nestjs/common'; import { ApiExcludeController } from '@nestjs/swagger'; import { SkipThrottle } from '@nestjs/throttler'; import { MetricsService } from './metrics.service'; +import { MetricsAuthGuard } from './metrics-auth.guard'; /** * Prometheus scrape endpoint (issue BE-141). Serves every gauge/counter/ - * summary the app records in OpenMetrics text format, including the - * `managehub_manual_review_queue_depth` gauge maintained by the + * summary/histogram the app records in OpenMetrics text format, including + * the `managehub_manual_review_queue_depth` gauge maintained by the * reconciliation cron — so the manual-review queue depth can be alerted on * externally (e.g. Prometheus + Alertmanager) rather than only via log - * lines. Deliberately unauthenticated (a scrape target has no bearer - * token) and exempt from rate limiting; network-level access control is the - * operator's responsibility. + * lines. Exempt from rate limiting (a scrape happens on its own fixed + * interval, not user traffic) but gated by MetricsAuthGuard (issue #1781) + * — see that guard for the token requirement and the METRICS_ACCESS_TOKEN + * env var. Network-level access control remains a valid complementary + * layer on top of the token. */ @ApiExcludeController() @SkipThrottle() +@UseGuards(MetricsAuthGuard) @Controller('metrics') export class MetricsController { constructor(private readonly metrics: MetricsService) {} diff --git a/backend/src/common/metrics.service.spec.ts b/backend/src/common/metrics.service.spec.ts new file mode 100644 index 00000000..a2d81d7e --- /dev/null +++ b/backend/src/common/metrics.service.spec.ts @@ -0,0 +1,47 @@ +import { MetricsService } from './metrics.service'; + +describe('MetricsService — per-endpoint latency histogram (issue #1780)', () => { + let service: MetricsService; + + beforeEach(() => { + service = new MetricsService(); + }); + + it('renders a histogram type line, buckets, sum and count keyed by route', () => { + service.recordHttpRequestDuration('/wallets/:id', 'GET', 200, 42); + service.recordHttpRequestDuration('/wallets/:id', 'GET', 200, 9); + + const output = service.renderPrometheus(); + + expect(output).toContain( + '# TYPE managehub_http_request_duration_seconds histogram', + ); + expect(output).toContain( + 'managehub_http_request_duration_seconds_count{route="/wallets/:id",method="GET",status_code="200"} 2', + ); + // 42ms observation falls in the 0.05s bucket and above, not below. + expect(output).toContain( + 'managehub_http_request_duration_seconds_bucket{route="/wallets/:id",method="GET",status_code="200",le="0.025"} 1', + ); + expect(output).toContain( + 'managehub_http_request_duration_seconds_bucket{route="/wallets/:id",method="GET",status_code="200",le="0.05"} 2', + ); + expect(output).toContain( + 'managehub_http_request_duration_seconds_bucket{route="/wallets/:id",method="GET",status_code="200",le="+Inf"} 2', + ); + }); + + it('keeps separate series for different routes', () => { + service.recordHttpRequestDuration('/wallets/:id', 'GET', 200, 10); + service.recordHttpRequestDuration('/payments', 'POST', 201, 500); + + const output = service.renderPrometheus(); + + expect(output).toContain( + 'managehub_http_request_duration_seconds_count{route="/wallets/:id",method="GET",status_code="200"} 1', + ); + expect(output).toContain( + 'managehub_http_request_duration_seconds_count{route="/payments",method="POST",status_code="201"} 1', + ); + }); +}); diff --git a/backend/src/common/metrics.service.ts b/backend/src/common/metrics.service.ts index fe9ddbe5..7126c406 100644 --- a/backend/src/common/metrics.service.ts +++ b/backend/src/common/metrics.service.ts @@ -5,6 +5,23 @@ interface MetricSample { value: number; } +interface HistogramSample { + labels: Record; + count: number; + sum: number; + // Cumulative bucket counts, one per HISTOGRAM_BUCKETS_SECONDS entry plus a + // trailing +Inf bucket — Prometheus's `le` (less-than-or-equal) convention. + buckets: number[]; +} + +// Bucket upper bounds, in seconds, for the per-endpoint latency histogram +// (issue #1780). Covers fast in-process handlers (5ms) up to slow +// downstream-dependent ones (10s); a request slower than that only counts +// against the +Inf bucket. +const HISTOGRAM_BUCKETS_SECONDS = [ + 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10, +]; + @Injectable() export class MetricsService { private readonly counters = new Map(); @@ -13,6 +30,7 @@ export class MetricsService { string, { count: number; sum: number; labels: Record } >(); + private readonly histograms = new Map(); recordPaymentTransition(from: string, to: string): void { this.incrementCounter('managehub_payment_status_transitions_total', { @@ -42,6 +60,24 @@ export class MetricsService { this.incrementCounter('managehub_settlement_payout_failures_total', {}); } + /** + * Records one HTTP request's duration for the per-route/method latency + * histogram (issue #1780) — lets p95/p99 regressions on a specific + * endpoint be caught, which a plain request counter can't show. + */ + recordHttpRequestDuration( + route: string, + method: string, + statusCode: number, + durationMs: number, + ): void { + this.observeHistogram( + 'managehub_http_request_duration_seconds', + { route, method, status_code: String(statusCode) }, + durationMs / 1000, + ); + } + /** Counts a detected tenant credit-usage anomaly by metered resource. */ recordCreditUsageSpike(resource: string): void { this.incrementCounter('managehub_credit_usage_spikes_total', { resource }); @@ -52,6 +88,7 @@ export class MetricsService { this.appendCounters(lines); this.appendGauges(lines); this.appendSummaries(lines); + this.appendHistograms(lines); return `${lines.join('\n')}\n`; } @@ -102,6 +139,34 @@ export class MetricsService { }); } + private observeHistogram( + name: string, + labels: Record, + valueSeconds: number, + ): void { + const series = this.histograms.get(name) ?? []; + let sample = series.find((item) => this.sameLabels(item.labels, labels)); + if (!sample) { + sample = { + labels, + count: 0, + sum: 0, + buckets: new Array(HISTOGRAM_BUCKETS_SECONDS.length + 1).fill(0), + }; + series.push(sample); + this.histograms.set(name, series); + } + sample.count += 1; + sample.sum += valueSeconds; + HISTOGRAM_BUCKETS_SECONDS.forEach((bound, index) => { + if (valueSeconds <= bound) { + sample!.buckets[index] += 1; + } + }); + // +Inf bucket always includes every observation. + sample.buckets[HISTOGRAM_BUCKETS_SECONDS.length] += 1; + } + private appendCounters(lines: string[]): void { for (const [name, series] of this.counters.entries()) { lines.push(`# TYPE ${name} counter`); @@ -128,6 +193,28 @@ export class MetricsService { } } + private appendHistograms(lines: string[]): void { + for (const [name, series] of this.histograms.entries()) { + lines.push(`# TYPE ${name} histogram`); + for (const sample of series) { + HISTOGRAM_BUCKETS_SECONDS.forEach((bound, index) => { + lines.push( + `${name}_bucket${this.formatLabels({ ...sample.labels, le: String(bound) })} ${sample.buckets[index]}`, + ); + }); + lines.push( + `${name}_bucket${this.formatLabels({ ...sample.labels, le: '+Inf' })} ${sample.buckets[HISTOGRAM_BUCKETS_SECONDS.length]}`, + ); + lines.push( + `${name}_sum${this.formatLabels(sample.labels)} ${sample.sum}`, + ); + lines.push( + `${name}_count${this.formatLabels(sample.labels)} ${sample.count}`, + ); + } + } + } + private formatLabels(labels: Record): string { const entries = Object.entries(labels); if (entries.length === 0) { diff --git a/backend/src/common/request-duration.interceptor.spec.ts b/backend/src/common/request-duration.interceptor.spec.ts new file mode 100644 index 00000000..e7a7f672 --- /dev/null +++ b/backend/src/common/request-duration.interceptor.spec.ts @@ -0,0 +1,72 @@ +import { of, throwError } from 'rxjs'; +import { RequestDurationInterceptor } from './request-duration.interceptor'; +import { MetricsService } from './metrics.service'; + +describe('RequestDurationInterceptor (issue #1780)', () => { + function buildContext(overrides: Partial> = {}) { + const request = { + method: 'GET', + url: '/wallets/abc-123', + path: '/wallets/abc-123', + route: { path: '/wallets/:id' }, + baseUrl: '', + ...overrides, + }; + const response = { statusCode: 200 }; + return { + getType: () => 'http', + switchToHttp: () => ({ + getRequest: () => request, + getResponse: () => response, + }), + } as any; + } + + it('records the route pattern (not the raw URL) on success', (done) => { + const recordHttpRequestDuration = jest.fn(); + const metrics = { recordHttpRequestDuration } as unknown as MetricsService; + const interceptor = new RequestDurationInterceptor(metrics); + const context = buildContext(); + + interceptor.intercept(context, { handle: () => of('ok') }).subscribe(() => { + expect(recordHttpRequestDuration).toHaveBeenCalledTimes(1); + const [route, method, statusCode, durationMs] = + recordHttpRequestDuration.mock.calls[0]; + expect(route).toBe('/wallets/:id'); + expect(method).toBe('GET'); + expect(statusCode).toBe(200); + expect(typeof durationMs).toBe('number'); + done(); + }); + }); + + it('still records duration when the handler throws', (done) => { + const recordHttpRequestDuration = jest.fn(); + const metrics = { recordHttpRequestDuration } as unknown as MetricsService; + const interceptor = new RequestDurationInterceptor(metrics); + const context = buildContext({ route: undefined }); + + interceptor + .intercept(context, { handle: () => throwError(() => new Error('boom')) }) + .subscribe({ + error: () => { + expect(recordHttpRequestDuration).toHaveBeenCalledTimes(1); + const [route] = recordHttpRequestDuration.mock.calls[0]; + // Falls back to the raw path when no route pattern was matched. + expect(route).toBe('/wallets/abc-123'); + done(); + }, + }); + }); + + it('skips non-HTTP contexts (e.g. RPC/WS) without recording', () => { + const recordHttpRequestDuration = jest.fn(); + const metrics = { recordHttpRequestDuration } as unknown as MetricsService; + const interceptor = new RequestDurationInterceptor(metrics); + const context = { getType: () => 'rpc' } as any; + + interceptor.intercept(context, { handle: () => of('ok') }); + + expect(recordHttpRequestDuration).not.toHaveBeenCalled(); + }); +}); diff --git a/backend/src/common/request-duration.interceptor.ts b/backend/src/common/request-duration.interceptor.ts new file mode 100644 index 00000000..c0c5ffda --- /dev/null +++ b/backend/src/common/request-duration.interceptor.ts @@ -0,0 +1,48 @@ +import { + CallHandler, + ExecutionContext, + Injectable, + NestInterceptor, +} from '@nestjs/common'; +import { Request, Response } from 'express'; +import { Observable } from 'rxjs'; +import { tap } from 'rxjs/operators'; +import { MetricsService } from './metrics.service'; + +/** + * Records every HTTP request's duration into the per-route latency + * histogram (issue #1780). Registered globally in app.module.ts. + */ +@Injectable() +export class RequestDurationInterceptor implements NestInterceptor { + constructor(private readonly metrics: MetricsService) {} + + intercept(context: ExecutionContext, next: CallHandler): Observable { + if (context.getType() !== 'http') { + return next.handle(); + } + + const start = process.hrtime.bigint(); + const httpContext = context.switchToHttp(); + const request = httpContext.getRequest(); + const response = httpContext.getResponse(); + + const record = () => { + const durationMs = Number(process.hrtime.bigint() - start) / 1_000_000; + // Prefer the matched Express route pattern (e.g. "/wallets/:id") over + // request.url so per-endpoint labels don't explode into one series + // per distinct path param value. + const route = request.route?.path + ? `${request.baseUrl ?? ''}${request.route.path}` + : (request.path ?? request.url); + this.metrics.recordHttpRequestDuration( + route, + request.method, + response.statusCode, + durationMs, + ); + }; + + return next.handle().pipe(tap({ next: record, error: record })); + } +} diff --git a/backend/src/database/data-source.spec.ts b/backend/src/database/data-source.spec.ts new file mode 100644 index 00000000..02dccf74 --- /dev/null +++ b/backend/src/database/data-source.spec.ts @@ -0,0 +1,39 @@ +describe('AppDataSource pool configuration (issue #1778)', () => { + const ORIGINAL_ENV = { ...process.env }; + + afterEach(() => { + process.env = { ...ORIGINAL_ENV }; + jest.resetModules(); + }); + + function loadDataSource() { + let mod: typeof import('./data-source'); + jest.isolateModules(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + mod = require('./data-source'); + }); + return mod!; + } + + it('defaults DB_POOL_MIN/MAX to node-postgres defaults when unset', () => { + delete process.env.DB_POOL_MIN; + delete process.env.DB_POOL_MAX; + + const { DB_POOL_MIN, DB_POOL_MAX, AppDataSource } = loadDataSource(); + + expect(DB_POOL_MIN).toBe(2); + expect(DB_POOL_MAX).toBe(10); + expect(AppDataSource.options.extra).toEqual({ min: 2, max: 10 }); + }); + + it('reads DB_POOL_MIN/MAX from the environment when set', () => { + process.env.DB_POOL_MIN = '5'; + process.env.DB_POOL_MAX = '25'; + + const { DB_POOL_MIN, DB_POOL_MAX, AppDataSource } = loadDataSource(); + + expect(DB_POOL_MIN).toBe(5); + expect(DB_POOL_MAX).toBe(25); + expect(AppDataSource.options.extra).toEqual({ min: 5, max: 25 }); + }); +}); diff --git a/backend/src/database/data-source.ts b/backend/src/database/data-source.ts index 0ef735e9..b1a40bf0 100644 --- a/backend/src/database/data-source.ts +++ b/backend/src/database/data-source.ts @@ -1,6 +1,17 @@ import 'dotenv/config'; import { DataSource } from 'typeorm'; +// Pool bounds are env-configurable (issue #1778) so staging/prod can be +// tuned without a code change. node-postgres (the underlying driver) reads +// `min`/`max` from the `extra` block below; DB_POOL_MIN defaults to 2 and +// DB_POOL_MAX to 10, matching node-postgres's own defaults. +export const DB_POOL_MIN = process.env.DB_POOL_MIN + ? parseInt(process.env.DB_POOL_MIN, 10) + : 2; +export const DB_POOL_MAX = process.env.DB_POOL_MAX + ? parseInt(process.env.DB_POOL_MAX, 10) + : 10; + export const AppDataSource = new DataSource({ type: 'postgres', host: process.env.DATABASE_HOST, @@ -13,6 +24,10 @@ export const AppDataSource = new DataSource({ entities: [__dirname + '/../**/*.entity{.ts,.js}'], migrations: [__dirname + '/migrations/*{.ts,.js}'], synchronize: false, + extra: { + min: DB_POOL_MIN, + max: DB_POOL_MAX, + }, }); export default AppDataSource;