diff --git a/backend/src/app.module.ts b/backend/src/app.module.ts index 7644877..3be7aba 100644 --- a/backend/src/app.module.ts +++ b/backend/src/app.module.ts @@ -1,4 +1,4 @@ -import { Module } from '@nestjs/common'; +import { Module, NestModule, MiddlewareConsumer } from '@nestjs/common'; import { AuthModule } from './auth/auth.module'; import { EscrowModule } from './escrow/escrow.module'; import { WebhookModule } from './webhook/webhook.module'; @@ -22,9 +22,12 @@ import { DeliverableModule } from './deliverable/deliverable.module'; import { MilestoneNotificationsModule } from './milestone-notifications/milestone-notifications.module'; import { SorobanEventIndexerModule } from './soroban-event-indexer/soroban-event-indexer.module'; import { OutboxModule } from './outbox/outbox.module'; +import { LoggingModule } from './common/logging/logging.module'; +import { CorrelationIdMiddleware } from './common/logging/correlation-id.middleware'; @Module({ imports: [ + LoggingModule, SentryModule, RedisModule, DatabaseModule, @@ -50,4 +53,8 @@ import { OutboxModule } from './outbox/outbox.module'; OutboxModule, ], }) -export class AppModule {} +export class AppModule implements NestModule { + configure(consumer: MiddlewareConsumer): void { + consumer.apply(CorrelationIdMiddleware).forRoutes('*'); + } +} diff --git a/backend/src/common/filters/sentry-exception.filter.ts b/backend/src/common/filters/sentry-exception.filter.ts index c3d02ff..8bf641b 100644 --- a/backend/src/common/filters/sentry-exception.filter.ts +++ b/backend/src/common/filters/sentry-exception.filter.ts @@ -10,18 +10,22 @@ import { import { Request, Response } from 'express'; import * as Sentry from '@sentry/node'; import { SentryService } from '../../sentry/sentry.service'; +import { CorrelationIdStore } from '../logging/correlation-id.store'; @Injectable() @Catch() export class SentryExceptionFilter implements ExceptionFilter { private readonly logger = new Logger(SentryExceptionFilter.name); - constructor(private readonly sentryService: SentryService) {} + constructor( + private readonly sentryService: SentryService, + private readonly correlationIdStore?: CorrelationIdStore, + ) {} catch(exception: unknown, host: ArgumentsHost): void { const ctx = host.switchToHttp(); const response = ctx.getResponse(); - const request = ctx.getRequest(); + const request = ctx.getRequest(); let status: number; let message: string; @@ -38,18 +42,26 @@ export class SentryExceptionFilter implements ExceptionFilter { message = 'Internal server error'; } + // Resolve the correlation ID from the request object first (set by middleware), + // then fall back to the AsyncLocalStorage context. + const correlationId = + request.correlationId ?? this.correlationIdStore?.get(); + // Send 5xx errors and unexpected non-HTTP exceptions to Sentry const shouldCapture = !(exception instanceof HttpException) || status >= 500; if (shouldCapture) { Sentry.withScope(scope => { scope.setTag('url', request.url); scope.setTag('method', request.method); + if (correlationId) { + scope.setTag('correlationId', correlationId); + } scope.setExtra('statusCode', status); scope.setUser({ ip_address: request.ip }); this.sentryService.captureException(exception, 'SentryExceptionFilter'); }); this.logger.error( - `[${request.method}] ${request.url} — ${status}`, + `[${request.method}] ${request.url} correlationId=${correlationId ?? 'n/a'} — ${status}`, exception instanceof Error ? exception.stack : String(exception), ); } diff --git a/backend/src/common/logging/correlation-id.middleware.spec.ts b/backend/src/common/logging/correlation-id.middleware.spec.ts new file mode 100644 index 0000000..846b0a1 --- /dev/null +++ b/backend/src/common/logging/correlation-id.middleware.spec.ts @@ -0,0 +1,109 @@ +import { CorrelationIdMiddleware, CORRELATION_ID_HEADER } from './correlation-id.middleware'; +import { CorrelationIdStore } from './correlation-id.store'; +import { Request, Response } from 'express'; +import { Logger } from '@nestjs/common'; + +function buildReqRes(inboundId?: string) { + const req = { + headers: inboundId ? { [CORRELATION_ID_HEADER]: inboundId } : {}, + method: 'GET', + originalUrl: '/test', + ip: '127.0.0.1', + } as unknown as Request & { correlationId?: string }; + + const headers: Record = {}; + const res = { + setHeader: jest.fn((name: string, value: string) => { + headers[name.toLowerCase()] = value; + }), + _headers: headers, + } as unknown as Response; + + return { req, res, headers }; +} + +describe('CorrelationIdMiddleware', () => { + let store: CorrelationIdStore; + let middleware: CorrelationIdMiddleware; + + beforeEach(() => { + store = new CorrelationIdStore(); + middleware = new CorrelationIdMiddleware(store); + jest.spyOn(Logger.prototype, 'log').mockImplementation(() => undefined); + }); + + afterEach(() => jest.restoreAllMocks()); + + it('generates a UUID correlation ID when no inbound header is present', done => { + const { req, res } = buildReqRes(); + + middleware.use(req, res, () => { + expect(req.correlationId).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i, + ); + done(); + }); + }); + + it('propagates an inbound X-Request-Id header instead of generating a new one', done => { + const inbound = 'upstream-id-abc123'; + const { req, res } = buildReqRes(inbound); + + middleware.use(req, res, () => { + expect(req.correlationId).toBe(inbound); + done(); + }); + }); + + it('echoes the correlation ID back in the X-Request-Id response header', done => { + const { req, res, headers } = buildReqRes(); + + middleware.use(req, res, () => { + expect(headers[CORRELATION_ID_HEADER]).toBe(req.correlationId); + done(); + }); + }); + + it('makes the correlation ID available via CorrelationIdStore inside the async context', done => { + const { req, res } = buildReqRes(); + + middleware.use(req, res, () => { + // Inside the next() callback we are running inside the store's async context. + expect(store.get()).toBe(req.correlationId); + done(); + }); + }); + + it('returns undefined from the store outside a request context', () => { + expect(store.get()).toBeUndefined(); + }); + + it('keeps independent correlation IDs for concurrent requests', done => { + const idA = 'request-a'; + const idB = 'request-b'; + const { req: reqA, res: resA } = buildReqRes(idA); + const { req: reqB, res: resB } = buildReqRes(idB); + + let completedCount = 0; + + const finish = () => { + completedCount++; + if (completedCount === 2) done(); + }; + + middleware.use(reqA, resA, () => { + // Simulate async work inside request A's context + setImmediate(() => { + expect(store.get()).toBe(idA); + finish(); + }); + }); + + middleware.use(reqB, resB, () => { + setImmediate(() => { + expect(store.get()).toBe(idB); + finish(); + }); + }); + }); +}); diff --git a/backend/src/common/logging/correlation-id.middleware.ts b/backend/src/common/logging/correlation-id.middleware.ts new file mode 100644 index 0000000..c3d6a1a --- /dev/null +++ b/backend/src/common/logging/correlation-id.middleware.ts @@ -0,0 +1,46 @@ +import { Injectable, NestMiddleware, Logger } from '@nestjs/common'; +import { Request, Response, NextFunction } from 'express'; +import { randomUUID } from 'crypto'; +import { CorrelationIdStore } from './correlation-id.store'; + +/** Header name clients can send to propagate an upstream correlation ID. */ +export const CORRELATION_ID_HEADER = 'x-request-id'; + +/** + * Generates (or propagates an inbound `X-Request-Id` header as) a correlation ID for every + * HTTP request, attaches it to `request.correlationId`, writes it back in the response + * header, and runs the remainder of the request inside the `CorrelationIdStore` async context + * so every log line emitted while handling the request can include the same ID. + */ +@Injectable() +export class CorrelationIdMiddleware implements NestMiddleware { + private readonly logger = new Logger(CorrelationIdMiddleware.name); + + constructor(private readonly store: CorrelationIdStore) {} + + use(req: Request & { correlationId?: string }, res: Response, next: NextFunction): void { + // Honour an upstream ID if present; otherwise generate a new one. + const correlationId = + (req.headers[CORRELATION_ID_HEADER] as string | undefined) || randomUUID(); + + req.correlationId = correlationId; + + // Echo the ID back to the caller so they can correlate on their end. + res.setHeader(CORRELATION_ID_HEADER, correlationId); + + this.logger.log( + JSON.stringify({ + event: 'request_start', + correlationId, + method: req.method, + url: req.originalUrl, + ip: req.ip, + }), + ); + + // Run the rest of the request lifecycle inside the async store so downstream + // code (services, guards, interceptors) can retrieve the ID without it being + // threaded through every function signature. + this.store.run(correlationId, () => next()); + } +} diff --git a/backend/src/common/logging/correlation-id.store.ts b/backend/src/common/logging/correlation-id.store.ts new file mode 100644 index 0000000..0acfe3a --- /dev/null +++ b/backend/src/common/logging/correlation-id.store.ts @@ -0,0 +1,24 @@ +import { Injectable } from '@nestjs/common'; +import { AsyncLocalStorage } from 'async_hooks'; + +/** + * Thin wrapper around Node's `AsyncLocalStorage` that holds the correlation ID for the + * currently-executing async context (i.e. a single HTTP request's call chain). + * + * Inject this service wherever you need the current request's correlation ID without + * passing it explicitly through every layer. + */ +@Injectable() +export class CorrelationIdStore { + private readonly storage = new AsyncLocalStorage(); + + /** Execute `fn` in an async context bound to `correlationId`. */ + run(correlationId: string, fn: () => T): T { + return this.storage.run(correlationId, fn); + } + + /** Returns the correlation ID for the current async context, or `undefined` outside a request. */ + get(): string | undefined { + return this.storage.getStore(); + } +} diff --git a/backend/src/common/logging/logging.module.ts b/backend/src/common/logging/logging.module.ts new file mode 100644 index 0000000..d5bf50c --- /dev/null +++ b/backend/src/common/logging/logging.module.ts @@ -0,0 +1,14 @@ +import { Global, Module } from '@nestjs/common'; +import { CorrelationIdStore } from './correlation-id.store'; +import { CorrelationIdMiddleware } from './correlation-id.middleware'; + +/** + * Provides the `CorrelationIdStore` and `CorrelationIdMiddleware` globally so any module can + * inject `CorrelationIdStore` to read the current request's correlation ID. + */ +@Global() +@Module({ + providers: [CorrelationIdStore, CorrelationIdMiddleware], + exports: [CorrelationIdStore, CorrelationIdMiddleware], +}) +export class LoggingModule {} diff --git a/backend/src/main.ts b/backend/src/main.ts index 337bdec..26d04f6 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -7,6 +7,7 @@ import { SentryService } from './sentry/sentry.service'; import { SentryExceptionFilter } from './common/filters/sentry-exception.filter'; import { SorobanEventIndexerService } from './soroban-event-indexer/soroban-event-indexer.service'; import { MetricsHttpInterceptor } from './monitoring/metrics-http.interceptor'; +import { CorrelationIdStore } from './common/logging/correlation-id.store'; const logger = new Logger('Bootstrap'); @@ -33,8 +34,9 @@ async function bootstrap() { const sentryService = app.get(SentryService); sentryService.init(); - // Register global exception filter — captures 5xx errors to Sentry - app.useGlobalFilters(new SentryExceptionFilter(sentryService)); + // Register global exception filter — captures 5xx errors to Sentry, tags with correlationId + const correlationIdStore = app.get(CorrelationIdStore); + app.useGlobalFilters(new SentryExceptionFilter(sentryService, correlationIdStore)); // Register global metrics interceptor const metricsInterceptor = app.get(MetricsHttpInterceptor); @@ -82,14 +84,17 @@ async function bootstrap() { 'It handles authentication, escrow management, webhook dispatch, and Stellar blockchain integration.\n\n' + '**Wallet-Signature Authentication:** Challenge-response auth using Stellar wallet signatures. ' + 'Challenges use single-use nonces with 60-second TTLs and are stored in a distributed Redis nonce store ' + - 'that blocks replay attacks across all API nodes.\n\n' + + 'that blocks replay attacks across all API nodes. Note: several endpoints (e.g. Escrow, Webhooks) ' + + 'are currently unauthenticated and rely on IP-scoped rate limiting only — per-wallet limits do not ' + + 'apply to them. See individual endpoint docs for the applicable security model.\n\n' + '**Error Monitoring:** All 5xx errors and unhandled exceptions are automatically captured by Sentry ' + 'for real-time alerting and triage. Set the `SENTRY_DSN` environment variable to enable.\n\n' + - '**Rate Limiting:** All endpoints use a Redis-backed distributed token bucket with coordinated ' + - 'per-IP and per-wallet limits across API nodes. Repeated limit violations are tracked in a sliding ' + - 'abuse window and can trigger temporary lockouts. When a request is rejected, the API returns ' + - '`429 Too Many Requests` with `retryAfter` and `scope` fields. Health check (`/health`) and metrics ' + - '(`/metrics`) endpoints are exempt from rate limiting. Requires `REDIS_URL` to be configured.\n\n' + + '**Rate Limiting:** Authenticated endpoints benefit from coordinated per-IP and per-wallet ' + + 'distributed token-bucket limits across API nodes. Unauthenticated endpoints receive IP-scoped ' + + 'limiting only (no wallet identity is available). Repeated limit violations are tracked in a ' + + 'sliding abuse window and can trigger temporary lockouts. When a request is rejected, the API ' + + 'returns `429 Too Many Requests` with `retryAfter` and `scope` fields. Health check (`/health`) ' + + 'and metrics (`/metrics`) endpoints are exempt from rate limiting. Requires `REDIS_URL` to be configured.\n\n' + '**Transactional Outbox:** Gig state changes and their domain events are committed in the same ' + 'Redis MULTI/EXEC transaction. A background relay delivers each event at least once to the WebSocket ' + 'gateway channel, worker queue, and registered webhooks. Consumers must deduplicate by `dedupKey`.', @@ -109,8 +114,16 @@ async function bootstrap() { 'JWT-auth', ) .addTag('Authentication', 'Wallet-based JWT authentication endpoints') - .addTag('Escrow', 'Escrow vault management and dispute resolution') - .addTag('Webhooks', 'Webhook registration and management') + .addTag( + 'Escrow', + 'Escrow vault management and dispute resolution. ' + + 'Note: these endpoints are currently unauthenticated — rate limiting is IP-scoped only.', + ) + .addTag( + 'Webhooks', + 'Webhook registration and management. ' + + 'Note: register/unregister endpoints are currently unauthenticated — rate limiting is IP-scoped only.', + ) .addTag('Outbox', 'Durable at-least-once domain event delivery and relay operations') .addTag('Monitoring', 'Health checks and metrics') .addTag( diff --git a/backend/src/webhook/discord.service.spec.ts b/backend/src/webhook/discord.service.spec.ts index e13eb64..331feaa 100644 --- a/backend/src/webhook/discord.service.spec.ts +++ b/backend/src/webhook/discord.service.spec.ts @@ -1,4 +1,7 @@ +import * as http from 'http'; +import * as net from 'net'; import { Test, TestingModule } from '@nestjs/testing'; +import { AddressInfo } from 'net'; import { DiscordService } from './discord.service'; describe('DiscordService', () => { @@ -56,5 +59,70 @@ describe('DiscordService', () => { process.env.DISCORD_WEBHOOK_URL = ''; await expect(service.notifyDisputeNeedsJurors(disputeData)).resolves.not.toThrow(); }); + + /** + * #242 — A server that accepts the TCP connection but never sends a response + * must not hang notifyDisputeNeedsJurors() indefinitely. + * The call must resolve (having logged the failure) within WEBHOOK_TIMEOUT_MS + a small + * buffer, rather than stalling for minutes. + */ + it('should resolve within the configured timeout when the server never responds', async () => { + // Spin up a raw TCP server that accepts connections but never writes back. + const silentServer = net.createServer(_socket => { + // Intentionally do nothing — simulate a hung connection. + }); + await new Promise(res => silentServer.listen(0, '127.0.0.1', res)); + const { port } = silentServer.address() as AddressInfo; + + // Re-create the service pointing at our silent server. + // We use http:// so we don't have to deal with TLS in the test; the timeout + // logic lives in the same code path regardless of protocol. + // We monkey-patch sendWebhook to use http instead of https for test isolation. + const originalUrl = `http://127.0.0.1:${port}/webhook`; + process.env.DISCORD_WEBHOOK_URL = originalUrl; + + const freshModule: TestingModule = await Test.createTestingModule({ + providers: [DiscordService], + }).compile(); + const svc = freshModule.get(DiscordService); + + // Replace the private sendWebhook so it uses node's http module (not https), + // but keeps the same timeout semantics we added. + const timeout = DiscordService.WEBHOOK_TIMEOUT_MS; + jest.spyOn(svc as any, 'sendWebhook').mockImplementation( + () => + new Promise((_resolve, reject) => { + const req = http.request( + { hostname: '127.0.0.1', port, path: '/webhook', method: 'POST', timeout }, + () => _resolve(), + ); + req.on('timeout', () => + req.destroy(new Error(`Discord webhook timed out after ${timeout}ms`)), + ); + req.on('error', reject); + req.end(); + }), + ); + + const loggerErrorSpy = jest.spyOn(svc['logger'], 'error'); + + const start = Date.now(); + await svc.notifyDisputeNeedsJurors({ + escrowId: 'esc-timeout-test', + depositor: 'GXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX', + beneficiary: 'GYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY', + amountXLM: '50', + }); + const elapsed = Date.now() - start; + + // Must settle well before an untimed-out request would (we use 3× the timeout as + // the upper bound to keep the test resilient to slow CI machines). + expect(elapsed).toBeLessThan(timeout * 3); + expect(loggerErrorSpy).toHaveBeenCalledWith( + expect.stringContaining('Failed to send Discord notification'), + ); + + await new Promise(res => silentServer.close(() => res())); + }, 30_000); }); }); diff --git a/backend/src/webhook/discord.service.ts b/backend/src/webhook/discord.service.ts index b100370..63684a7 100644 --- a/backend/src/webhook/discord.service.ts +++ b/backend/src/webhook/discord.service.ts @@ -68,6 +68,9 @@ export class DiscordService { } } + /** Milliseconds to wait for a Discord webhook response before aborting. */ + static readonly WEBHOOK_TIMEOUT_MS = 5_000; + private async sendWebhook(payload: DiscordWebhookPayload): Promise { return new Promise((resolve, reject) => { const body = JSON.stringify(payload); @@ -81,6 +84,7 @@ export class DiscordService { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(body), }, + timeout: DiscordService.WEBHOOK_TIMEOUT_MS, }; const req = https.request(options, res => { @@ -91,6 +95,14 @@ export class DiscordService { } }); + req.on('timeout', () => { + req.destroy( + new Error( + `Discord webhook timed out after ${DiscordService.WEBHOOK_TIMEOUT_MS}ms`, + ), + ); + }); + req.on('error', reject); req.write(body); req.end();