From 8041f0d28f962d72174feda709e0faf8fdf47799 Mon Sep 17 00:00:00 2001 From: adefemiesther1-debug Date: Sat, 17 Oct 2026 11:48:33 +0100 Subject: [PATCH] feat(api): add ETag caching, correlation ID tracing, standardized error codes, and scoped API credentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - #985: strong SHA-256 ETags + If-None-Match → 304 on event and statistics GET endpoints - #986: validated X-Correlation-Id propagated via AsyncLocalStorage into logs, error envelopes and EventProcessingQueue jobs - #987: central error-code catalog; error envelope always carries details and correlation_id - #988: scoped API credentials (scrypt-hashed, secret returned only on create), server-side scope checks returning 403 INVALID_PERMISSIONS Co-Authored-By: Claude Opus 5.5 --- listener/API_ERROR_REFERENCE.md | 50 ++- listener/docs/API_HARDENING.md | 98 ++++++ listener/src/api/api-key-auth.ts | 116 +++++++ listener/src/api/credentials-api.test.ts | 284 ++++++++++++++++++ listener/src/api/credentials-api.ts | 160 ++++++++++ listener/src/api/error-handler.ts | 14 +- listener/src/api/events-server.ts | 167 +++++----- listener/src/errors/api-error-codes.test.ts | 193 ++++++++++++ listener/src/errors/api-error-codes.ts | 100 ++++++ listener/src/index.ts | 2 + listener/src/middleware/correlation.test.ts | 227 ++++++++++++++ listener/src/middleware/correlation.ts | 36 +++ listener/src/middleware/etag.test.ts | 174 +++++++++++ listener/src/middleware/etag.ts | 104 +++++++ .../services/api-credential-service.test.ts | 126 ++++++++ .../src/services/api-credential-service.ts | 263 ++++++++++++++++ .../src/services/event-processing-queue.ts | 11 +- listener/src/types/api-credential.ts | 58 ++++ listener/src/utils/correlation-context.ts | 57 ++++ listener/src/utils/logger.ts | 23 +- listener/src/utils/request-id.ts | 32 +- listener/src/utils/response.ts | 50 ++- 22 files changed, 2212 insertions(+), 133 deletions(-) create mode 100644 listener/docs/API_HARDENING.md create mode 100644 listener/src/api/credentials-api.test.ts create mode 100644 listener/src/api/credentials-api.ts create mode 100644 listener/src/errors/api-error-codes.test.ts create mode 100644 listener/src/errors/api-error-codes.ts create mode 100644 listener/src/middleware/correlation.test.ts create mode 100644 listener/src/middleware/correlation.ts create mode 100644 listener/src/middleware/etag.test.ts create mode 100644 listener/src/middleware/etag.ts create mode 100644 listener/src/services/api-credential-service.test.ts create mode 100644 listener/src/services/api-credential-service.ts create mode 100644 listener/src/types/api-credential.ts create mode 100644 listener/src/utils/correlation-context.ts diff --git a/listener/API_ERROR_REFERENCE.md b/listener/API_ERROR_REFERENCE.md index b96f9bb3..6a1279d2 100644 --- a/listener/API_ERROR_REFERENCE.md +++ b/listener/API_ERROR_REFERENCE.md @@ -1,11 +1,57 @@ # Notification API Error Reference -This reference lists listener API error responses, common causes, and operator or client actions. Error bodies use JSON and generally follow: +This reference lists listener API error responses, common causes, and operator or client actions. + +## Standard Error Envelope (#987) + +Errors produced through the shared response helpers use this envelope: ```json -{ "error": "Human readable error message" } +{ + "success": false, + "error": { + "code": "INVALID_PERMISSIONS", + "message": "The provided credentials lack the necessary scope for this action.", + "details": [], + "correlation_id": "req-12345" + } +} ``` +| Field | Type | Notes | +|-------|------|-------| +| `code` | string | Stable machine-readable code from the table below. **Branch on this, not on `message`.** | +| `message` | string | Human-readable; wording may change between releases. | +| `details` | array \| object | Validation issues or extra context. `[]` when there is nothing to add. | +| `correlation_id` | string | Same value as the `X-Correlation-Id` response header. Quote it when reporting problems. | + +### Error Codes + +Codes are never renamed or reused once published. The source of truth is +`src/errors/api-error-codes.ts` (`ERROR_CODE_CATALOG`). + +| Code | HTTP | Meaning | +|------|------|---------| +| `BAD_REQUEST` | 400 | Malformed request or invalid parameters. | +| `VALIDATION_ERROR` | 400 | Body failed schema validation; `details` lists `{ field, message }` issues. | +| `PARSE_ERROR` | 400 | Body is not valid JSON. | +| `UNAUTHORIZED` | 401 | API key missing, unknown, revoked, or mismatched. | +| `INVALID_PERMISSIONS` | 403 | Valid credential lacking a required scope; `details` lists `requiredScopes` / `missingScopes`. | +| `FORBIDDEN` | 403 | Action not permitted for this caller. | +| `NOT_FOUND` | 404 | Unknown route or resource. | +| `METHOD_NOT_ALLOWED` | 405 | Resource does not support the method; see `Allow`. | +| `CONFLICT` | 409 | Conflicts with current resource state. | +| `PAYLOAD_TOO_LARGE` | 413 | Body exceeds the size limit. | +| `UNSUPPORTED_MEDIA_TYPE` | 415 | `Content-Type` not accepted. | +| `UNPROCESSABLE` | 422 | Well-formed but cannot be processed. | +| `RATE_LIMITED` | 429 | Rate limit exceeded; honour `Retry-After`. | +| `INTERNAL_ERROR` | 500 | Unexpected server failure. | +| `SERVICE_UNAVAILABLE` | 503 | A required optional service is not configured. | + +> Some older endpoints documented below (e.g. `POST /api/schedule`) still +> write ad-hoc bodies such as `{ "error": "...", "code": "..." }`. They will be +> migrated to the standard envelope; the codes they emit are unchanged. + Batch validation endpoints return structured validation results instead: ```json diff --git a/listener/docs/API_HARDENING.md b/listener/docs/API_HARDENING.md new file mode 100644 index 00000000..935da0fb --- /dev/null +++ b/listener/docs/API_HARDENING.md @@ -0,0 +1,98 @@ +# API Caching, Tracing and Scoped Credentials + +Covers issues #985 (ETags), #986 (correlation IDs), #987 (error codes) and +#988 (scoped credentials). Error codes are documented in +[`API_ERROR_REFERENCE.md`](../API_ERROR_REFERENCE.md). + +## Conditional GET with ETags (#985) + +These endpoints return a strong `ETag` (SHA-256 of the response body) and +`Cache-Control: private, no-cache`: + +- `GET /api/events`, `GET /api/events/history` +- `GET /api/analytics` (not when `?reset=true`), `GET /api/analytics/history` +- `GET /api/schedule/stats`, `/execution-metrics`, `/retry-distribution`, `/retry-statistics` + +Send the tag back in `If-None-Match`. If the data is unchanged the server +replies `304 Not Modified` with an empty body: + +```bash +etag=$(curl -s -D - -o /dev/null localhost:8787/api/events | grep -i etag | cut -d' ' -f2 | tr -d '\r') +curl -i -H "If-None-Match: $etag" localhost:8787/api/events # → 304 +``` + +The tag is computed from the bytes that would be sent, so it changes as soon as +the underlying data changes; there is no separate cache to invalidate. +`W/"…"`, comma-separated lists and `*` are honoured. + +## Correlation IDs (#986) + +- Every response carries `X-Correlation-Id`. +- A client-supplied `X-Correlation-Id` is reused only if it is 1–64 characters + of `[A-Za-z0-9._:-]` and starts with a letter or digit. Anything else + (whitespace, control characters, quotes, overlong values) is replaced with a + new UUID v4, so untrusted header content never reaches the logs. +- The ID is held in AsyncLocalStorage for the life of the request. Every + `logger.*` call made while serving it gets `correlationId` / `requestId` + added automatically, and error envelopes include it as `correlation_id`. +- `EventProcessingQueue` captures the ID at enqueue time and restores it when + the job runs. For other deferred work, wrap the callback with + `bindCorrelationContext()` from `src/utils/correlation-context.ts`. + +## Scoped API Credentials (#988) + +### Scopes + +| Scope | Grants | +|-------|--------| +| `events:read` | Read event data (needed with `notifications:read` for `/api/export`) | +| `events:write` | Reserved for event write endpoints (none exist yet) | +| `stats:read` | Reserved for statistics endpoints (currently public) | +| `notifications:read` | `/api/notifications/history`, `/api/notifications/:id/receipts` | +| `notifications:write` | `POST /api/notifications/import` | +| `webhooks:manage` | Reserved for webhook management endpoints (none exist yet) | +| `credentials:manage` | Manage credentials via `/api/credentials` | + +Static keys from the `apiKeys` config keep full access, so existing +integrations keep working. Credentials issued through the API are limited to +their scopes; a missing scope returns `403 INVALID_PERMISSIONS`. + +### Managing credentials + +All routes need `credentials:manage`. On a deployment with no static keys +they always return `401`, so an open dev server can't be used to create the +first credential. + +```bash +# Create: the only response that ever includes apiKey +curl -X POST localhost:8787/api/credentials \ + -H "X-API-Key: $ADMIN_KEY" -H "Content-Type: application/json" \ + -d '{"name":"grafana","scopes":["stats:read","events:read"]}' +# → 201 { "success": true, "data": { "id": "…", "apiKey": "nck__", … } } + +curl localhost:8787/api/credentials -H "X-API-Key: $ADMIN_KEY" # list (no secrets) +curl localhost:8787/api/credentials/ -H "X-API-Key: $ADMIN_KEY" # get (no secrets) +curl -X DELETE localhost:8787/api/credentials/ -H "X-API-Key: $ADMIN_KEY" # revoke +``` + +A scoped caller holding `credentials:manage` can only grant scopes it already +holds. + +### Secret handling + +- Keys look like `nck_<16-hex id>_<43-char secret>`. The id is a lookup handle. + The secret is 256 bits from `crypto.randomBytes`. +- Only an scrypt hash (N=16384, r=8, p=1, 16-byte per-key salt) is stored. + Comparisons use `timingSafeEqual`, and unknown ids still do the same scrypt + work so response timing doesn't reveal which ids exist. +- Successful verifications are cached for 60 s under a SHA-256 of the key, so + scrypt doesn't run on every request. Revoking a key clears its cache entry + right away. + +### Limitations + +- The default `InMemoryApiCredentialStore` loses credentials when the process + restarts. For durable storage, implement `ApiCredentialStore` and pass it to + `new ApiCredentialService({ store })`. +- Scope checks apply only to endpoints that already require authentication. + Public read endpoints such as `GET /api/events` stay unauthenticated. diff --git a/listener/src/api/api-key-auth.ts b/listener/src/api/api-key-auth.ts index 84dfdfb1..b091b291 100644 --- a/listener/src/api/api-key-auth.ts +++ b/listener/src/api/api-key-auth.ts @@ -1,5 +1,11 @@ import crypto from 'crypto'; import http from 'http'; +import { ErrorCode } from '../errors/api-error-codes'; +import { + looksLikeScopedApiKey, + type ApiCredentialService, +} from '../services/api-credential-service'; +import type { ApiScope } from '../types/api-credential'; export interface ApiKeyEntry { key: string; @@ -63,3 +69,113 @@ export const API_KEY_AUTH_MESSAGES: Record<'missing' | 'invalid', string> = { missing: 'Unauthorized: missing API key', invalid: 'Unauthorized: invalid API key', }; + +// --------------------------------------------------------------------------- +// Scoped authorization (#988) +// --------------------------------------------------------------------------- + +export type AuthPrincipal = + /** No keys configured anywhere: open dev/local mode (backward compatible). */ + | { type: 'anonymous' } + /** A key from the static `apiKeys` config: full access, as before scopes existed. */ + | { type: 'legacy'; keyName?: string } + /** A scoped credential issued through the credentials API. */ + | { type: 'credential'; credentialId: string; name: string; scopes: readonly ApiScope[] }; + +export type AuthorizationResult = + | { allowed: true; principal: AuthPrincipal } + | { + allowed: false; + status: 401 | 403; + code: typeof ErrorCode.UNAUTHORIZED | typeof ErrorCode.INVALID_PERMISSIONS; + message: string; + details: unknown[]; + }; + +export interface AuthorizeOptions { + apiKeys?: ApiKeyEntry[]; + credentialService?: ApiCredentialService | null; + /** Every listed scope must be granted. Legacy keys implicitly hold all scopes. */ + requiredScopes?: readonly ApiScope[]; + /** + * Reject even when no keys are configured. Used for credential management so + * an unconfigured deployment cannot be used to mint the first key. + */ + requireAuthentication?: boolean; +} + +export const INVALID_PERMISSIONS_MESSAGE = + 'The provided credentials lack the necessary scope for this action.'; + +function unauthorized(message: string): AuthorizationResult { + return { allowed: false, status: 401, code: ErrorCode.UNAUTHORIZED, message, details: [] }; +} + +/** True when `principal` holds every scope in `required`. */ +export function principalHasScopes( + principal: AuthPrincipal, + required: readonly ApiScope[], +): boolean { + if (principal.type !== 'credential') return true; + return required.every((scope) => principal.scopes.includes(scope)); +} + +/** + * Authenticates the `X-API-Key` header and enforces endpoint scopes + * server-side. + * + * - `nck_…` keys are verified against the credential service and must hold + * every `requiredScopes` entry, else `403 INVALID_PERMISSIONS`. + * - Other keys are checked against the static `apiKeys` config and keep the + * full access they always had. + * - With no static keys and no active credentials, requests are allowed + * (unless `requireAuthentication`), preserving the existing dev behaviour. + */ +export async function authorizeRequest( + req: http.IncomingMessage, + options: AuthorizeOptions, +): Promise { + const required = options.requiredScopes ?? []; + const header = req.headers['x-api-key']; + if (Array.isArray(header)) return unauthorized(API_KEY_AUTH_MESSAGES.invalid); + + const hasStaticKeys = Boolean(options.apiKeys && options.apiKeys.length > 0); + const hasCredentials = options.credentialService?.hasActiveCredentials() ?? false; + + if (typeof header === 'string' && header.length > 0 && looksLikeScopedApiKey(header)) { + const credential = options.credentialService + ? await options.credentialService.verify(header) + : null; + if (!credential) return unauthorized(API_KEY_AUTH_MESSAGES.invalid); + + const principal: AuthPrincipal = { + type: 'credential', + credentialId: credential.id, + name: credential.name, + scopes: credential.scopes, + }; + if (!principalHasScopes(principal, required)) { + const missing = required.filter((scope) => !credential.scopes.includes(scope)); + return { + allowed: false, + status: 403, + code: ErrorCode.INVALID_PERMISSIONS, + message: INVALID_PERMISSIONS_MESSAGE, + details: [{ requiredScopes: [...required], missingScopes: missing }], + }; + } + return { allowed: true, principal }; + } + + if (!hasStaticKeys) { + if (!hasCredentials && !options.requireAuthentication) { + return { allowed: true, principal: { type: 'anonymous' } }; + } + const presented = typeof header === 'string' && header.length > 0; + return unauthorized(API_KEY_AUTH_MESSAGES[presented ? 'invalid' : 'missing']); + } + + const legacy = authenticateApiKey(req, options.apiKeys); + if (!legacy.authenticated) return unauthorized(API_KEY_AUTH_MESSAGES[legacy.reason]); + return { allowed: true, principal: { type: 'legacy', keyName: legacy.keyName } }; +} diff --git a/listener/src/api/credentials-api.test.ts b/listener/src/api/credentials-api.test.ts new file mode 100644 index 00000000..049821bb --- /dev/null +++ b/listener/src/api/credentials-api.test.ts @@ -0,0 +1,284 @@ +import http from 'http'; +import type { AddressInfo } from 'net'; +import { handleCredentialRoutes } from './credentials-api'; +import { authorizeRequest, type ApiKeyEntry } from './api-key-auth'; +import { ApiCredentialService } from '../services/api-credential-service'; +import { runWithRequestCorrelation } from '../middleware/correlation'; +import { sendErr, sendOk, ErrorCode } from '../utils/response'; +import type { ApiScope } from '../types/api-credential'; + +jest.mock('../utils/logger', () => { + const actual = jest.requireActual('../utils/logger'); + return { + __esModule: true, + ...actual, + default: { debug: jest.fn(), info: jest.fn(), warn: jest.fn(), error: jest.fn() }, + }; +}); + +const ADMIN_KEY = 'legacy-admin-key-123'; + +interface Res { + status: number; + headers: http.IncomingHttpHeaders; + body: any; +} + +/** + * Minimal server mirroring the events server's wiring: credential routes plus + * two scope-protected endpoints. + */ +function buildServer( + credentialService: ApiCredentialService, + apiKeys: ApiKeyEntry[] | undefined, +): http.Server { + const protectedRoutes: Record = { + '/api/stats': ['stats:read'], + '/api/export': ['events:read', 'notifications:read'], + }; + + return http.createServer((req, res) => + runWithRequestCorrelation(req, res, async () => { + const url = new URL(req.url ?? '/', 'http://localhost'); + if (await handleCredentialRoutes(req, res, url, { credentialService, apiKeys })) return; + + const scopes = protectedRoutes[url.pathname]; + if (!scopes) { + sendErr(res, 404, 'Not found', ErrorCode.NOT_FOUND); + return; + } + const auth = await authorizeRequest(req, { + apiKeys, + credentialService, + requiredScopes: scopes, + }); + if (!auth.allowed) { + sendErr(res, auth.status, auth.message, auth.code, auth.details); + return; + } + sendOk(res, 200, { ok: true, principal: auth.principal.type }); + }), + ); +} + +describe('scoped API credentials', () => { + let server: http.Server; + let port: number; + let credentialService: ApiCredentialService; + let apiKeys: ApiKeyEntry[] | undefined; + + async function start(keys: ApiKeyEntry[] | undefined): Promise { + apiKeys = keys; + credentialService = new ApiCredentialService(); + server = buildServer(credentialService, apiKeys); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + port = (server.address() as AddressInfo).port; + } + + afterEach(async () => { + await new Promise((resolve) => server.close(() => resolve())); + }); + + function call( + method: string, + path: string, + options: { key?: string; body?: unknown; correlationId?: string } = {}, + ): Promise { + return new Promise((resolve, reject) => { + const headers: Record = {}; + if (options.key) headers['X-API-Key'] = options.key; + if (options.correlationId) headers['X-Correlation-Id'] = options.correlationId; + const payload = options.body === undefined ? undefined : JSON.stringify(options.body); + if (payload) headers['Content-Type'] = 'application/json'; + + const req = http.request({ hostname: '127.0.0.1', port, path, method, headers }, (res) => { + let data = ''; + res.on('data', (c) => (data += c)); + res.on('end', () => + resolve({ + status: res.statusCode ?? 0, + headers: res.headers, + body: data ? JSON.parse(data) : null, + }), + ); + }); + req.on('error', reject); + if (payload) req.write(payload); + req.end(); + }); + } + + async function issue(scopes: ApiScope[], name = 'integration'): Promise { + return call('POST', '/api/credentials', { key: ADMIN_KEY, body: { name, scopes } }); + } + + describe('with a static admin key configured', () => { + beforeEach(() => start([{ key: ADMIN_KEY, name: 'admin' }])); + + it('creates a credential and returns the secret key once with 201', async () => { + const res = await issue(['stats:read']); + expect(res.status).toBe(201); + expect(res.headers['cache-control']).toBe('no-store'); + expect(res.body.success).toBe(true); + expect(res.body.data.apiKey).toMatch(/^nck_/); + expect(res.body.data.scopes).toEqual(['stats:read']); + }); + + it('never returns the secret from list or get', async () => { + const created = (await issue(['stats:read'])).body.data; + + const list = await call('GET', '/api/credentials', { key: ADMIN_KEY }); + const one = await call('GET', `/api/credentials/${created.id}`, { key: ADMIN_KEY }); + + expect(list.status).toBe(200); + expect(list.body.data.count).toBe(1); + expect(one.status).toBe(200); + expect(one.body.data.id).toBe(created.id); + for (const view of [list.body.data.credentials[0], one.body.data]) { + expect(view).not.toHaveProperty('apiKey'); + expect(view).not.toHaveProperty('secretHash'); + } + expect(JSON.stringify(list.body)).not.toContain(created.apiKey); + expect(JSON.stringify(one.body)).not.toContain(created.apiKey); + }); + + it('allows a request whose credential holds the required scope', async () => { + const { apiKey } = (await issue(['stats:read'])).body.data; + const res = await call('GET', '/api/stats', { key: apiKey }); + expect(res.status).toBe(200); + expect(res.body.data.principal).toBe('credential'); + }); + + it('rejects a credential missing the required scope with 403 INVALID_PERMISSIONS', async () => { + const { apiKey } = (await issue(['events:read'])).body.data; + const res = await call('GET', '/api/stats', { key: apiKey, correlationId: 'req-403' }); + + expect(res.status).toBe(403); + expect(res.body.error.code).toBe('INVALID_PERMISSIONS'); + expect(res.body.error.correlation_id).toBe('req-403'); + expect(res.body.error.details).toEqual([ + { requiredScopes: ['stats:read'], missingScopes: ['stats:read'] }, + ]); + }); + + it('requires every scope when an endpoint lists several', async () => { + const partial = (await issue(['events:read'])).body.data.apiKey; + const full = (await issue(['events:read', 'notifications:read'])).body.data.apiKey; + + const denied = await call('GET', '/api/export', { key: partial }); + expect(denied.status).toBe(403); + expect(denied.body.error.details[0].missingScopes).toEqual(['notifications:read']); + + expect((await call('GET', '/api/export', { key: full })).status).toBe(200); + }); + + it('keeps full access for static (legacy) keys', async () => { + const res = await call('GET', '/api/export', { key: ADMIN_KEY }); + expect(res.status).toBe(200); + expect(res.body.data.principal).toBe('legacy'); + }); + + it('rejects a forged scoped key with 401 UNAUTHORIZED', async () => { + const forged = `nck_${'a'.repeat(16)}_${'B'.repeat(43)}`; + const res = await call('GET', '/api/stats', { key: forged }); + expect(res.status).toBe(401); + expect(res.body.error.code).toBe('UNAUTHORIZED'); + }); + + it('rejects a revoked credential with 401', async () => { + const created = (await issue(['stats:read'])).body.data; + expect((await call('GET', '/api/stats', { key: created.apiKey })).status).toBe(200); + + const del = await call('DELETE', `/api/credentials/${created.id}`, { key: ADMIN_KEY }); + expect(del.status).toBe(200); + expect(del.body.data.revokedAt).not.toBeNull(); + expect(del.body.data).not.toHaveProperty('apiKey'); + + expect((await call('GET', '/api/stats', { key: created.apiKey })).status).toBe(401); + }); + + it('requires credentials:manage to manage credentials', async () => { + const { apiKey } = (await issue(['stats:read'])).body.data; + const res = await call('GET', '/api/credentials', { key: apiKey }); + expect(res.status).toBe(403); + expect(res.body.error.code).toBe('INVALID_PERMISSIONS'); + }); + + it('prevents a scoped manager from granting scopes it does not hold', async () => { + const manager = (await issue(['credentials:manage', 'events:read'])).body.data.apiKey; + + const escalate = await call('POST', '/api/credentials', { + key: manager, + body: { name: 'escalate', scopes: ['webhooks:manage'] }, + }); + expect(escalate.status).toBe(403); + expect(escalate.body.error.code).toBe('INVALID_PERMISSIONS'); + + const allowed = await call('POST', '/api/credentials', { + key: manager, + body: { name: 'reader', scopes: ['events:read'] }, + }); + expect(allowed.status).toBe(201); + }); + + it('returns VALIDATION_ERROR with per-field details for bad input', async () => { + const res = await call('POST', '/api/credentials', { + key: ADMIN_KEY, + body: { name: '', scopes: ['nope'] }, + }); + expect(res.status).toBe(400); + expect(res.body.error.code).toBe('VALIDATION_ERROR'); + expect(res.body.error.details).toEqual( + expect.arrayContaining([ + expect.objectContaining({ field: 'name' }), + expect.objectContaining({ field: 'scopes' }), + ]), + ); + }); + + it('returns 404 NOT_FOUND for unknown credential ids', async () => { + const res = await call('GET', `/api/credentials/${'0'.repeat(16)}`, { key: ADMIN_KEY }); + expect(res.status).toBe(404); + expect(res.body.error.code).toBe('NOT_FOUND'); + }); + + it('returns 405 METHOD_NOT_ALLOWED for unsupported methods', async () => { + const res = await call('PUT', '/api/credentials', { key: ADMIN_KEY, body: {} }); + expect(res.status).toBe(405); + expect(res.body.error.code).toBe('METHOD_NOT_ALLOWED'); + expect(res.headers.allow).toBe('GET, POST'); + }); + + it('rejects unauthenticated management requests with 401', async () => { + const res = await call('GET', '/api/credentials'); + expect(res.status).toBe(401); + expect(res.headers['www-authenticate']).toContain('X-API-Key'); + }); + }); + + describe('with no static keys configured (dev mode)', () => { + beforeEach(() => start(undefined)); + + it('refuses to mint credentials without authentication', async () => { + const res = await call('POST', '/api/credentials', { + body: { name: 'x', scopes: ['events:read'] }, + }); + expect(res.status).toBe(401); + expect(credentialService.hasActiveCredentials()).toBe(false); + }); + + it('keeps protected endpoints open while no credentials exist', async () => { + const res = await call('GET', '/api/stats'); + expect(res.status).toBe(200); + expect(res.body.data.principal).toBe('anonymous'); + }); + + it('starts enforcing once a scoped credential exists', async () => { + const created = await credentialService.create({ name: 'ci', scopes: ['events:read'] }); + + expect((await call('GET', '/api/stats')).status).toBe(401); + expect((await call('GET', '/api/stats', { key: created.apiKey })).status).toBe(403); + expect((await call('GET', '/api/export', { key: 'some-random-key' })).status).toBe(401); + }); + }); +}); diff --git a/listener/src/api/credentials-api.ts b/listener/src/api/credentials-api.ts new file mode 100644 index 00000000..e071cfa7 --- /dev/null +++ b/listener/src/api/credentials-api.ts @@ -0,0 +1,160 @@ +/** + * Scoped API credential management endpoints (#988) + * + * POST /api/credentials issue a key — the only response that ever contains `apiKey` + * GET /api/credentials list credentials (no secret material) + * GET /api/credentials/:id fetch one credential (no secret material) + * DELETE /api/credentials/:id revoke a credential + * + * Every route requires the `credentials:manage` scope (static admin keys hold + * it implicitly) and authentication even on deployments with no keys + * configured, so an open dev server cannot be used to mint credentials. + * A scoped caller may only grant scopes it holds itself. + */ + +import http from 'http'; +import logger from '../utils/logger'; +import { ok, sendErr, sendJson, sendOk, ErrorCode } from '../utils/response'; +import { + authorizeRequest, + principalHasScopes, + INVALID_PERMISSIONS_MESSAGE, + type ApiKeyEntry, +} from './api-key-auth'; +import { + CredentialValidationError, + validateCreateCredentialInput, + type ApiCredentialService, +} from '../services/api-credential-service'; +import type { CreateApiCredentialInput } from '../types/api-credential'; + +export interface CredentialRoutesDeps { + credentialService: ApiCredentialService | null | undefined; + apiKeys?: ApiKeyEntry[]; +} + +const COLLECTION_PATH = '/api/credentials'; +const ITEM_PATTERN = /^\/api\/credentials\/([0-9a-f]{16})$/; + +function readJsonBody(req: http.IncomingMessage): Promise { + return new Promise((resolve, reject) => { + let body = ''; + req.on('data', (chunk) => { + body += chunk.toString(); + }); + req.on('end', () => { + try { + resolve(body ? JSON.parse(body) : {}); + } catch (error) { + reject(error); + } + }); + req.on('error', reject); + }); +} + +/** + * Handles `/api/credentials` routes. Resolves true when the request was + * handled (a response has been or will be sent). + */ +export async function handleCredentialRoutes( + req: http.IncomingMessage, + res: http.ServerResponse, + url: URL, + deps: CredentialRoutesDeps, +): Promise { + const path = url.pathname; + const itemMatch = ITEM_PATTERN.exec(path); + if (path !== COLLECTION_PATH && !itemMatch && !path.startsWith(`${COLLECTION_PATH}/`)) { + return false; + } + + // Secrets may appear in these responses; never let an intermediary cache them. + res.setHeader('Cache-Control', 'no-store'); + + const auth = await authorizeRequest(req, { + apiKeys: deps.apiKeys, + credentialService: deps.credentialService, + requiredScopes: ['credentials:manage'], + requireAuthentication: true, + }); + if (!auth.allowed) { + if (auth.status === 401) res.setHeader('WWW-Authenticate', 'ApiKey header="X-API-Key"'); + sendErr(res, auth.status, auth.message, auth.code, auth.details); + return true; + } + + const service = deps.credentialService; + if (!service) { + sendErr(res, 503, 'Credential management is not enabled', ErrorCode.SERVICE_UNAVAILABLE); + return true; + } + + if (path === COLLECTION_PATH && req.method === 'POST') { + let input: CreateApiCredentialInput; + try { + input = validateCreateCredentialInput(await readJsonBody(req)); + } catch (error) { + if (error instanceof CredentialValidationError) { + sendErr(res, 400, error.message, ErrorCode.VALIDATION_ERROR, error.issues); + } else { + sendErr(res, 400, 'Request body must be valid JSON.', ErrorCode.PARSE_ERROR); + } + return true; + } + + // No privilege escalation: a scoped key cannot mint a broader key. + if (!principalHasScopes(auth.principal, input.scopes)) { + sendErr(res, 403, INVALID_PERMISSIONS_MESSAGE, ErrorCode.INVALID_PERMISSIONS, [ + { reason: 'Cannot grant scopes the calling credential does not hold' }, + ]); + return true; + } + + const created = await service.create(input); + logger.info('API credential created', { + credentialId: created.id, + name: created.name, + scopes: created.scopes, + createdBy: auth.principal.type, + }); + sendJson(res, 201, ok(created)); + return true; + } + + if (path === COLLECTION_PATH && req.method === 'GET') { + const credentials = service.list(); + sendOk(res, 200, { count: credentials.length, credentials }); + return true; + } + + if (itemMatch && req.method === 'GET') { + const credential = service.get(itemMatch[1]); + if (!credential) { + sendErr(res, 404, 'Credential not found', ErrorCode.NOT_FOUND); + return true; + } + sendOk(res, 200, credential); + return true; + } + + if (itemMatch && req.method === 'DELETE') { + const revoked = service.revoke(itemMatch[1]); + if (!revoked) { + sendErr(res, 404, 'Credential not found', ErrorCode.NOT_FOUND); + return true; + } + logger.info('API credential revoked', { credentialId: revoked.id }); + sendOk(res, 200, revoked); + return true; + } + + if (!itemMatch && path !== COLLECTION_PATH) { + sendErr(res, 404, 'Credential not found', ErrorCode.NOT_FOUND); + return true; + } + + res.setHeader('Allow', path === COLLECTION_PATH ? 'GET, POST' : 'GET, DELETE'); + sendErr(res, 405, `Method ${req.method} not allowed`, ErrorCode.METHOD_NOT_ALLOWED); + return true; +} diff --git a/listener/src/api/error-handler.ts b/listener/src/api/error-handler.ts index 5ac65875..94f6923f 100644 --- a/listener/src/api/error-handler.ts +++ b/listener/src/api/error-handler.ts @@ -29,6 +29,13 @@ export class ApiError extends Error { return new ApiError(message, 401, ErrorCode.UNAUTHORIZED); } + static invalidPermissions( + message = 'The provided credentials lack the necessary scope for this action.', + details?: unknown, + ): ApiError { + return new ApiError(message, 403, ErrorCode.INVALID_PERMISSIONS, details); + } + static notFound(message: string): ApiError { return new ApiError(message, 404, ErrorCode.NOT_FOUND); } @@ -76,7 +83,7 @@ export function handleApiError( errorCode: error.errorCode, message: error.message, }); - sendErr(res, error.statusCode, error.message, error.errorCode, error.details); + sendErr(res, error.statusCode, error.message, error.errorCode, error.details, correlationId); return; } @@ -91,7 +98,8 @@ export function handleApiError( 400, `Validation failed: ${error.message}`, ErrorCode.BAD_REQUEST, - error.issues + error.issues, + correlationId, ); return; } @@ -102,7 +110,7 @@ export function handleApiError( correlationId, error, }); - sendErr(res, 500, message, ErrorCode.INTERNAL_ERROR); + sendErr(res, 500, message, ErrorCode.INTERNAL_ERROR, undefined, correlationId); } export function wrapAsyncHandler( diff --git a/listener/src/api/events-server.ts b/listener/src/api/events-server.ts index b9475d07..98e0ef69 100644 --- a/listener/src/api/events-server.ts +++ b/listener/src/api/events-server.ts @@ -14,7 +14,13 @@ import { handleTemplateRoutes } from './template-routes'; import { sendOk, sendErr, sendJson, ErrorCode } from '../utils/response'; import { normalizePaginationParams } from '../utils/pagination'; import { handleApiError, ApiError } from './error-handler'; -import { applyRequestIdMiddleware } from '../middleware/request-id'; +import { type RequestContext } from '../middleware/request-id'; +import { runWithRequestCorrelation } from '../middleware/correlation'; +import { sendOkWithETag } from '../middleware/etag'; +import { handleCredentialRoutes } from './credentials-api'; +import { authorizeRequest } from './api-key-auth'; +import { ApiCredentialService } from '../services/api-credential-service'; +import type { ApiScope } from '../types/api-credential'; import { TemplateService } from '../services/template-service'; import { handleTemplateRoutes } from './template-routes'; import { sendOk, sendErr, sendJson, ErrorCode } from '../utils/response'; @@ -83,6 +89,12 @@ export interface EventsServerOptions { discordWebhookUrl?: string; webhookSecrets?: WebhookSecret[]; apiKeys?: Array<{ key: string; name?: string }>; + /** + * Issues and verifies scoped integration credentials (#988). Static + * `apiKeys` keep full access; credentials from this service are limited to + * their scopes. + */ + credentialService?: ApiCredentialService | null; notificationAPI?: NotificationAPI | null; /** Idempotency-Key replay protection for mutating endpoints (webhooks, schedule). */ idempotencyService?: IdempotencyKeyService | null; @@ -461,15 +473,16 @@ export function createEventsServer(options: EventsServerOptions): http.Server { ? options.responseTimeMiddleware : new ResponseTimeMiddleware({ slowRequestThresholdMs: options.slowRequestThresholdMs }); - const server = http.createServer(async (req, res) => { + const handleRequest = async ( + req: http.IncomingMessage, + res: http.ServerResponse, + requestContext: RequestContext, + ): Promise => { addSecurityHeaders(res, { isProduction: options.isProduction ?? process.env.NODE_ENV === 'production', }); - // Request-ID middleware (#686): assigns (or validates+reuses) a requestId - // and resolves a correlationId for every request, stamping both onto the - // response headers. See listener/src/middleware/request-id.ts. - const { requestId, correlationId } = applyRequestIdMiddleware(req, res); + const { requestId, correlationId } = requestContext; const startTime = Date.now(); // Start response-time tracking for this request (#491) @@ -503,11 +516,11 @@ export function createEventsServer(options: EventsServerOptions): http.Server { res.setHeader('Access-Control-Allow-Methods', 'GET, POST, PUT, DELETE, OPTIONS'); res.setHeader( 'Access-Control-Allow-Headers', - 'Content-Type, X-API-Key, Authorization, X-Correlation-Id, X-Request-Id', + 'Content-Type, X-API-Key, Authorization, X-Correlation-Id, X-Request-Id, If-None-Match', ); res.setHeader( 'Access-Control-Expose-Headers', - 'X-Request-Id, X-Correlation-Id, X-Response-Time', + 'X-Request-Id, X-Correlation-Id, X-Response-Time, ETag', ); const url = new URL(req.url ?? '/', 'http://localhost'); @@ -529,21 +542,27 @@ export function createEventsServer(options: EventsServerOptions): http.Server { res.setHeader('Content-Type', 'application/json'); } /** - * Enforces X-API-Key auth for protected endpoints. Sends a 401 and returns - * false when the request is not authenticated. + * Enforces X-API-Key auth and scope checks (#988) for protected endpoints. + * Sends 401 UNAUTHORIZED or 403 INVALID_PERMISSIONS and resolves false + * when the request may not proceed. */ - const requireApiKey = (): boolean => { - const auth = authenticateApiKey(req, options.apiKeys); - if (auth.authenticated) return true; - logger.warn('API key authentication failed', { + const requireApiKey = async (requiredScopes: readonly ApiScope[] = []): Promise => { + const auth = await authorizeRequest(req, { + apiKeys: options.apiKeys, + credentialService: options.credentialService, + requiredScopes, + }); + if (auth.allowed) return true; + logger.warn('API key authorization failed', { requestId, correlationId, method: req.method, path: url.pathname, - reason: auth.reason, + status: auth.status, + code: auth.code, }); - res.setHeader('WWW-Authenticate', 'ApiKey header="X-API-Key"'); - sendErr(res, 401, API_KEY_AUTH_MESSAGES[auth.reason], ErrorCode.UNAUTHORIZED); + if (auth.status === 401) res.setHeader('WWW-Authenticate', 'ApiKey header="X-API-Key"'); + sendErr(res, auth.status, auth.message, auth.code, auth.details); return false; }; @@ -565,6 +584,15 @@ export function createEventsServer(options: EventsServerOptions): http.Server { return; } + // Scoped API credential management (#988) + if (url.pathname === '/api/credentials' || url.pathname.startsWith('/api/credentials/')) { + await handleCredentialRoutes(req, res, url, { + credentialService: options.credentialService, + apiKeys: options.apiKeys, + }); + return; + } + // Template API routes (handled first for priority) // Note: route matching is handled inline below for NotificationTemplateService compatibility. // url.pathname is already rewritten from /api/v1/* → /api/* above @@ -649,7 +677,7 @@ export function createEventsServer(options: EventsServerOptions): http.Server { returned: events.length, }); - sendOk(res, 200, { + sendOkWithETag(req, res, { count: events.length, startLedger: range.startLedger, endLedger: range.endLedger, @@ -684,7 +712,8 @@ export function createEventsServer(options: EventsServerOptions): http.Server { logger.info('Handling GET /api/events', { requestId, correlationId, limit: paginationParams.limit }); - sendOk(res, 200, { count: eventRegistry.count(), events }); + // ETag (#985): pollers that already hold the current feed get a 304. + sendOkWithETag(req, res, { count: eventRegistry.count(), events }); logger.info('GET /api/events complete', { requestId, @@ -753,12 +782,15 @@ export function createEventsServer(options: EventsServerOptions): http.Server { durationMs: Date.now() - startTime, }); - sendOk(res, 200, snapshot); - - if (reset) { - aggregator.reset(); - logger.info('Analytics snapshot reset after read', { requestId, correlationId }); + // A resetting read has a side effect, so it always returns the full body. + if (!reset) { + sendOkWithETag(req, res, snapshot); + return; } + + sendOk(res, 200, snapshot); + aggregator.reset(); + logger.info('Analytics snapshot reset after read', { requestId, correlationId }); return; } @@ -775,7 +807,7 @@ export function createEventsServer(options: EventsServerOptions): http.Server { options.metricsStore .getHistory(limit, since) .then((snapshots) => { - sendOk(res, 200, { snapshots }); + sendOkWithETag(req, res, { snapshots }); }) .catch((error) => { logger.error('Failed to fetch metrics history', { error, requestId, correlationId }); @@ -995,7 +1027,7 @@ export function createEventsServer(options: EventsServerOptions): http.Server { // POST /api/notifications/import — bulk import from JSON or CSV if (req.method === 'POST' && url.pathname === '/api/notifications/import') { // Authenticate before revealing anything about service availability. - if (!requireApiKey()) return; + if (!(await requireApiKey(['notifications:write']))) return; if (!options.notificationAPI) { sendErr(res, 503, 'Scheduler not enabled', ErrorCode.SERVICE_UNAVAILABLE); @@ -1006,16 +1038,6 @@ export function createEventsServer(options: EventsServerOptions): http.Server { return; } - const apiKeyHeader = req.headers['x-api-key']; - if (options.apiKeys && options.apiKeys.length > 0) { - const provided = Array.isArray(apiKeyHeader) ? apiKeyHeader[0] : apiKeyHeader; - const allowed = options.apiKeys.some((k) => k.key === provided); - if (!allowed) { - sendErr(res, 401, 'Unauthorized', ErrorCode.UNAUTHORIZED); - return; - } - } - let body = ''; req.on('data', (chunk) => { body += chunk.toString(); @@ -1045,15 +1067,7 @@ export function createEventsServer(options: EventsServerOptions): http.Server { (req.method === 'GET' || req.method === 'POST') && (url.pathname === '/api/admin/export' || url.pathname === '/api/export') ) { - const apiKeyHeader = req.headers['x-api-key']; - if (options.apiKeys && options.apiKeys.length > 0) { - const provided = Array.isArray(apiKeyHeader) ? apiKeyHeader[0] : apiKeyHeader; - const allowed = options.apiKeys.some((k) => k.key === provided); - if (!allowed) { - sendErr(res, 401, 'Unauthorized', ErrorCode.UNAUTHORIZED); - return; - } - } + if (!(await requireApiKey(['events:read', 'notifications:read']))) return; const processExport = async (rawFilters: Record) => { try { @@ -1289,7 +1303,7 @@ export function createEventsServer(options: EventsServerOptions): http.Server { options.notificationAPI .getStatistics() .then((stats) => { - sendOk(res, 200, stats); + sendOkWithETag(req, res, stats); }) .catch((error) => { logger.error('Failed to get scheduler stats', { error, requestId, correlationId }); @@ -1307,7 +1321,7 @@ export function createEventsServer(options: EventsServerOptions): http.Server { (options.notificationAPI as any) .getExecutionMetrics() .then((metrics: unknown) => { - sendOk(res, 200, metrics); + sendOkWithETag(req, res, metrics); }) .catch((error: Error) => { logger.error('Failed to get execution metrics', { error, requestId, correlationId }); @@ -1325,7 +1339,7 @@ export function createEventsServer(options: EventsServerOptions): http.Server { (options.notificationAPI as any) .getRetryDistribution() .then((distribution: unknown) => { - sendOk(res, 200, distribution); + sendOkWithETag(req, res, distribution); }) .catch((error: Error) => { logger.error('Failed to get retry distribution', { error, requestId, correlationId }); @@ -1369,7 +1383,7 @@ export function createEventsServer(options: EventsServerOptions): http.Server { options.notificationAPI .getRetryStatistics() .then((stats) => { - sendOk(res, 200, stats); + sendOkWithETag(req, res, stats); }) .catch((error) => { logger.error('Failed to get retry statistics', { error, requestId, correlationId }); @@ -1515,29 +1529,10 @@ export function createEventsServer(options: EventsServerOptions): http.Server { return; } - function isValidApiKey(apiKey: string | undefined, allowedKeys: Array<{ key: string; name?: string }> | undefined): boolean { - function isValidApiKey( - apiKey: string | undefined, - allowedKeys: Array<{ key: string; name?: string }> | undefined, - ): boolean { - if (!allowedKeys || allowedKeys.length === 0) { - // If no API keys are configured, allow unauthenticated access is allowed (for backward compatibility) - return true; - } - if (!apiKey) { - return false; - } - return allowedKeys.some((k) => k.key === apiKey); - } - // Get delivery receipts for a scheduled notification const receiptPath = url.pathname.match(/^\/api\/notifications\/(\d+)\/receipts$/); if (req.method === 'GET' && receiptPath) { - const apiKey = req.headers['x-api-key'] as string | undefined; - if (!isValidApiKey(apiKey, options.apiKeys)) { - sendErr(res, 401, 'Unauthorized: Invalid or missing API key', ErrorCode.UNAUTHORIZED); - return; - } + if (!(await requireApiKey(['notifications:read']))) return; if (!options.deliveryReceiptRepository) { sendErr(res, 503, 'Delivery receipt storage is not configured', ErrorCode.SERVICE_UNAVAILABLE); @@ -1561,26 +1556,11 @@ export function createEventsServer(options: EventsServerOptions): http.Server { return; } - // Get notification delivery history endpoint - if (req.method === 'GET' && req.url?.startsWith('/api/notifications/history')) { - const apiKey = req.headers['x-api-key'] as string | undefined; - if (!isValidApiKey(apiKey, options.apiKeys)) { - sendErr(res, 401, 'Unauthorized: Invalid or missing API key', ErrorCode.UNAUTHORIZED); - return; - } - - const url = new URL(req.url, 'http://localhost'); - const limit = url.searchParams.get('limit') - ? parseInt(url.searchParams.get('limit')!, 10) - : undefined; - const offset = url.searchParams.get('offset') - ? parseInt(url.searchParams.get('offset')!, 10) - : undefined; // Get notification delivery history endpoint. // Matched on the rewritten pathname so /api/v1/notifications/history is // routed (and authenticated) the same as the unversioned path. if (req.method === 'GET' && url.pathname === '/api/notifications/history') { - if (!requireApiKey()) return; + if (!(await requireApiKey(['notifications:read']))) return; const limit = url.searchParams.get('limit') ? parseInt(url.searchParams.get('limit')!, 10) : undefined; const offset = url.searchParams.get('offset') ? parseInt(url.searchParams.get('offset')!, 10) : undefined; const cursor = url.searchParams.get('cursor') || undefined; @@ -2121,6 +2101,23 @@ export function createEventsServer(options: EventsServerOptions): http.Server { }); sendErr(res, 404, 'Not found', ErrorCode.NOT_FOUND); responseTime.finish(req, res, requestId, 404); + }; + + const server = http.createServer((req, res) => { + // Request-ID + correlation middleware (#686, #986): assigns (or + // validates+reuses) a requestId and correlationId, stamps both onto the + // response headers, and runs the handler inside an AsyncLocalStorage + // context so logs, error envelopes and queued jobs inherit them. + runWithRequestCorrelation(req, res, (requestContext) => + handleRequest(req, res, requestContext), + ).catch((error: unknown) => { + logger.error('Unhandled error in events server request handler', { error }); + if (!res.headersSent) { + sendErr(res, 500, 'Internal server error', ErrorCode.INTERNAL_ERROR); + } else { + res.end(); + } + }); }); if (rateLimiter) { diff --git a/listener/src/errors/api-error-codes.test.ts b/listener/src/errors/api-error-codes.test.ts new file mode 100644 index 00000000..538c9603 --- /dev/null +++ b/listener/src/errors/api-error-codes.test.ts @@ -0,0 +1,193 @@ +import http from 'http'; +import type { AddressInfo } from 'net'; +import { ERROR_CODE_CATALOG, ErrorCode, isKnownErrorCode } from './api-error-codes'; +import { err, sendErr } from '../utils/response'; +import { ApiError, handleApiError } from '../api/error-handler'; +import { ValidationError } from '../utils/validation'; +import { runWithRequestCorrelation } from '../middleware/correlation'; +import { runWithCorrelationContext } from '../utils/correlation-context'; + +jest.mock('../utils/logger', () => { + const actual = jest.requireActual('../utils/logger'); + return { + __esModule: true, + ...actual, + default: { debug: jest.fn(), info: jest.fn(), warn: jest.fn(), error: jest.fn() }, + }; +}); + +const STABLE_CODE = /^[A-Z][A-Z0-9_]*$/; + +describe('error code catalog', () => { + it('documents every code with an HTTP status and description', () => { + for (const code of Object.values(ErrorCode)) { + const entry = ERROR_CODE_CATALOG[code]; + expect(entry).toBeDefined(); + expect(entry.status).toBeGreaterThanOrEqual(400); + expect(entry.status).toBeLessThan(600); + expect(entry.description.length).toBeGreaterThan(0); + } + }); + + it('uses SCREAMING_SNAKE_CASE values equal to their keys', () => { + for (const [key, value] of Object.entries(ErrorCode)) { + expect(value).toBe(key); + expect(value).toMatch(STABLE_CODE); + } + }); + + it('includes the codes required by the API contract', () => { + for (const code of [ + 'UNAUTHORIZED', + 'INVALID_PERMISSIONS', + 'NOT_FOUND', + 'RATE_LIMITED', + 'VALIDATION_ERROR', + 'INTERNAL_ERROR', + ]) { + expect(isKnownErrorCode(code)).toBe(true); + } + }); + + it('maps INVALID_PERMISSIONS to 403 and UNAUTHORIZED to 401', () => { + expect(ERROR_CODE_CATALOG.INVALID_PERMISSIONS.status).toBe(403); + expect(ERROR_CODE_CATALOG.UNAUTHORIZED.status).toBe(401); + }); + + it('rejects unknown codes', () => { + expect(isKnownErrorCode('SOMETHING_ELSE')).toBe(false); + expect(isKnownErrorCode('toString')).toBe(false); + }); +}); + +describe('err() envelope', () => { + it('always includes details, defaulting to an empty array', () => { + expect(err('Nope', ErrorCode.NOT_FOUND, undefined, undefined)).toEqual({ + success: false, + error: { code: 'NOT_FOUND', message: 'Nope', details: [] }, + }); + }); + + it('carries supplied details through unchanged', () => { + const details = [{ field: 'name', message: 'required' }]; + expect(err('Bad', ErrorCode.VALIDATION_ERROR, details).error.details).toBe(details); + }); + + it('picks up the active correlation id', () => { + const envelope = runWithCorrelationContext({ correlationId: 'req-12345' }, () => + err( + 'The provided credentials lack the necessary scope for this action.', + ErrorCode.INVALID_PERMISSIONS, + ), + ); + expect(envelope).toEqual({ + success: false, + error: { + code: 'INVALID_PERMISSIONS', + message: 'The provided credentials lack the necessary scope for this action.', + details: [], + correlation_id: 'req-12345', + }, + }); + }); + + it('prefers an explicit correlation id over the context', () => { + const envelope = runWithCorrelationContext({ correlationId: 'ctx' }, () => + err('x', ErrorCode.CONFLICT, undefined, 'explicit'), + ); + expect(envelope.error.correlation_id).toBe('explicit'); + }); + + it('omits correlation_id when none is known', () => { + expect(err('x', ErrorCode.CONFLICT)).not.toHaveProperty('error.correlation_id'); + }); +}); + +describe('error paths over HTTP', () => { + let server: http.Server; + let port: number; + + const routes: Record void> = { + '/api-error/not-found': (res) => handleApiError(res, ApiError.notFound('Thing not found')), + '/api-error/unauthorized': (res) => handleApiError(res, ApiError.unauthorized('No key')), + '/api-error/forbidden': (res) => + handleApiError( + res, + ApiError.invalidPermissions(undefined, [{ missingScopes: ['stats:read'] }]), + ), + '/api-error/rate-limited': (res) => handleApiError(res, ApiError.rateLimited('Slow down')), + '/api-error/bad-request': (res) => handleApiError(res, ApiError.badRequest('Bad input')), + '/api-error/unavailable': (res) => handleApiError(res, ApiError.serviceUnavailable('Off')), + '/validation': (res) => + handleApiError(res, new ValidationError([{ field: 'name', message: 'name is required' }])), + '/unknown-throw': (res) => handleApiError(res, new Error('boom')), + '/send-err': (res) => sendErr(res, 404, 'Not found', ErrorCode.NOT_FOUND), + }; + + beforeAll(async () => { + server = http.createServer((req, res) => + runWithRequestCorrelation(req, res, () => routes[req.url ?? '']?.(res)), + ); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + port = (server.address() as AddressInfo).port; + }); + + afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())); + }); + + function get(path: string, correlationId: string) { + return new Promise<{ status: number; headers: http.IncomingHttpHeaders; body: any }>( + (resolve, reject) => { + const req = http.request( + { hostname: '127.0.0.1', port, path, headers: { 'X-Correlation-Id': correlationId } }, + (res) => { + let data = ''; + res.on('data', (c) => (data += c)); + res.on('end', () => + resolve({ + status: res.statusCode ?? 0, + headers: res.headers, + body: JSON.parse(data), + }), + ); + }, + ); + req.on('error', reject); + req.end(); + }, + ); + } + + it.each([ + ['/api-error/not-found', 404, 'NOT_FOUND'], + ['/api-error/unauthorized', 401, 'UNAUTHORIZED'], + ['/api-error/forbidden', 403, 'INVALID_PERMISSIONS'], + ['/api-error/rate-limited', 429, 'RATE_LIMITED'], + ['/api-error/bad-request', 400, 'BAD_REQUEST'], + ['/api-error/unavailable', 503, 'SERVICE_UNAVAILABLE'], + ['/validation', 400, 'BAD_REQUEST'], + ['/unknown-throw', 500, 'INTERNAL_ERROR'], + ['/send-err', 404, 'NOT_FOUND'], + ])('%s → %i %s in the standard envelope', async (path, status, code) => { + const correlationId = `req-${status}-${code.toLowerCase()}`; + const res = await get(path, correlationId); + + expect(res.status).toBe(status); + expect(res.headers['x-correlation-id']).toBe(correlationId); + expect(res.body.success).toBe(false); + expect(Object.keys(res.body.error).sort()).toEqual( + ['code', 'correlation_id', 'details', 'message'].sort(), + ); + expect(res.body.error.code).toBe(code); + expect(isKnownErrorCode(res.body.error.code)).toBe(true); + expect(typeof res.body.error.message).toBe('string'); + expect(res.body.error.details).toBeDefined(); + expect(res.body.error.correlation_id).toBe(correlationId); + }); + + it('includes scope details for INVALID_PERMISSIONS', async () => { + const res = await get('/api-error/forbidden', 'req-scope'); + expect(res.body.error.details).toEqual([{ missingScopes: ['stats:read'] }]); + }); +}); diff --git a/listener/src/errors/api-error-codes.ts b/listener/src/errors/api-error-codes.ts new file mode 100644 index 00000000..7f374373 --- /dev/null +++ b/listener/src/errors/api-error-codes.ts @@ -0,0 +1,100 @@ +/** + * Stable, machine-readable API error codes (#987) + * + * Every error response produced by the listener API carries one of these codes + * in `error.code`. Clients should branch on the code, never on `message`, which + * is human-readable and may change between releases. + * + * Codes are part of the public API contract: once published a code is never + * renamed or repurposed. New failure modes get new codes. + * + * Envelope: + * + * { + * "success": false, + * "error": { + * "code": "INVALID_PERMISSIONS", + * "message": "The provided credentials lack the necessary scope for this action.", + * "details": [], + * "correlation_id": "4d1c3c1e-..." + * } + * } + * + * See listener/API_ERROR_REFERENCE.md for the client-facing documentation. + */ + +export const ErrorCode = { + BAD_REQUEST: 'BAD_REQUEST', + VALIDATION_ERROR: 'VALIDATION_ERROR', + PARSE_ERROR: 'PARSE_ERROR', + UNAUTHORIZED: 'UNAUTHORIZED', + INVALID_PERMISSIONS: 'INVALID_PERMISSIONS', + FORBIDDEN: 'FORBIDDEN', + NOT_FOUND: 'NOT_FOUND', + METHOD_NOT_ALLOWED: 'METHOD_NOT_ALLOWED', + CONFLICT: 'CONFLICT', + PAYLOAD_TOO_LARGE: 'PAYLOAD_TOO_LARGE', + UNSUPPORTED_MEDIA_TYPE: 'UNSUPPORTED_MEDIA_TYPE', + UNPROCESSABLE: 'UNPROCESSABLE', + RATE_LIMITED: 'RATE_LIMITED', + INTERNAL_ERROR: 'INTERNAL_ERROR', + SERVICE_UNAVAILABLE: 'SERVICE_UNAVAILABLE', +} as const; + +export type ErrorCodeValue = (typeof ErrorCode)[keyof typeof ErrorCode]; + +export interface ErrorCodeDefinition { + /** HTTP status the code is normally returned with. */ + status: number; + /** One-line description for documentation and tooling. */ + description: string; +} + +/** + * Catalogue of every published error code. Kept exhaustive by the + * `Record` type so adding a code without documenting it is a + * compile error. + */ +export const ERROR_CODE_CATALOG: Readonly> = { + BAD_REQUEST: { status: 400, description: 'The request is malformed or has invalid parameters.' }, + VALIDATION_ERROR: { + status: 400, + description: 'The request body failed schema validation; see `details` for each issue.', + }, + PARSE_ERROR: { status: 400, description: 'The request body is not valid JSON.' }, + UNAUTHORIZED: { + status: 401, + description: 'Credentials are missing, unknown, revoked, or do not match.', + }, + INVALID_PERMISSIONS: { + status: 403, + description: 'The credentials are valid but lack the scope required for this action.', + }, + FORBIDDEN: { status: 403, description: 'The action is not permitted for this caller.' }, + NOT_FOUND: { status: 404, description: 'The route or requested resource does not exist.' }, + METHOD_NOT_ALLOWED: { + status: 405, + description: 'The resource exists but does not support this HTTP method; see the Allow header.', + }, + CONFLICT: { status: 409, description: 'The request conflicts with the current resource state.' }, + PAYLOAD_TOO_LARGE: { status: 413, description: 'The request body exceeds the size limit.' }, + UNSUPPORTED_MEDIA_TYPE: { + status: 415, + description: 'The request Content-Type is not accepted by this endpoint.', + }, + UNPROCESSABLE: { + status: 422, + description: + 'The request is well-formed but cannot be processed (e.g. template render failure).', + }, + RATE_LIMITED: { status: 429, description: 'Rate limit exceeded; honour the Retry-After header.' }, + INTERNAL_ERROR: { status: 500, description: 'An unexpected server-side failure occurred.' }, + SERVICE_UNAVAILABLE: { + status: 503, + description: 'A required optional service (scheduler, templates, …) is not configured.', + }, +}; + +export function isKnownErrorCode(code: string): code is ErrorCodeValue { + return Object.prototype.hasOwnProperty.call(ERROR_CODE_CATALOG, code); +} diff --git a/listener/src/index.ts b/listener/src/index.ts index 69ec3f0c..62debbca 100644 --- a/listener/src/index.ts +++ b/listener/src/index.ts @@ -34,6 +34,7 @@ import { NotificationHealthMonitor } from './services/notification-health-monito import { getWorkerManager } from './services/worker-manager'; import { EventDeduplicationService } from './services/event-deduplication-service'; import { DeliveryReceiptRepository } from './services/delivery-receipt-repository'; +import { ApiCredentialService } from './services/api-credential-service'; dotenv.config(); @@ -180,6 +181,7 @@ async function main() { schedulerTemplateService: legacyTemplateService, webhookSecrets: config.webhookSecrets, apiKeys: config.apiKeys, + credentialService: new ApiCredentialService(), rateLimit: config.rateLimit, archiveStore, archiveService, diff --git a/listener/src/middleware/correlation.test.ts b/listener/src/middleware/correlation.test.ts new file mode 100644 index 00000000..12545774 --- /dev/null +++ b/listener/src/middleware/correlation.test.ts @@ -0,0 +1,227 @@ +import http from 'http'; +import type { AddressInfo } from 'net'; +import * as StellarSDK from '@stellar/stellar-sdk'; +import { runWithRequestCorrelation } from './correlation'; +import { + bindCorrelationContext, + getCorrelationContext, + getCorrelationId, + runWithCorrelationContext, +} from '../utils/correlation-context'; +import { + isValidCorrelationId, + MAX_CORRELATION_ID_LENGTH, + resolveCorrelationId, +} from '../utils/request-id'; +import { withCorrelationMeta } from '../utils/logger'; +import { EventProcessingQueue } from '../services/event-processing-queue'; +import { sendErr, ErrorCode } from '../utils/response'; + +const UUID_V4 = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/; + +describe('isValidCorrelationId', () => { + it.each(['req-12345', 'abc', 'svc:orders.v2_trace-1', '0b5f3c1e-6d2a-4f7e-9a1b-2c3d4e5f6a7b'])( + 'accepts %s', + (value) => { + expect(isValidCorrelationId(value)).toBe(true); + }, + ); + + it('accepts exactly the maximum length', () => { + expect(isValidCorrelationId('a'.repeat(MAX_CORRELATION_ID_LENGTH))).toBe(true); + }); + + it.each([ + ['too long', 'a'.repeat(MAX_CORRELATION_ID_LENGTH + 1)], + ['newline (log injection)', 'abc\nlevel=error msg=forged'], + ['carriage return', 'abc\rdef'], + ['null byte', 'abc\0def'], + ['space', 'abc def'], + ['quote', 'abc"def'], + ['braces', '{"a":1}'], + ['leading separator', '-abc'], + ['empty', ''], + ['non-ascii', 'café'], + ])('rejects %s', (_label, value) => { + expect(isValidCorrelationId(value)).toBe(false); + }); +}); + +describe('resolveCorrelationId', () => { + it('generates a UUID v4 when the header is omitted', () => { + expect(resolveCorrelationId(undefined)).toMatch(UUID_V4); + }); + + it('generates distinct ids per call', () => { + expect(resolveCorrelationId(undefined)).not.toBe(resolveCorrelationId(undefined)); + }); + + it('reuses a valid client id', () => { + expect(resolveCorrelationId('req-12345')).toBe('req-12345'); + }); + + it('replaces an invalid client id with a fresh UUID instead of echoing it', () => { + const malicious = 'id\r\nX-Injected: yes'; + const resolved = resolveCorrelationId(malicious); + expect(resolved).not.toBe(malicious); + expect(resolved).toMatch(UUID_V4); + }); + + it('replaces an oversized client id', () => { + expect(resolveCorrelationId('a'.repeat(200))).toMatch(UUID_V4); + }); +}); + +describe('correlation context', () => { + it('is undefined outside a request', () => { + expect(getCorrelationContext()).toBeUndefined(); + }); + + it('survives awaits inside the context', async () => { + await runWithCorrelationContext({ correlationId: 'c-1', requestId: 'r-1' }, async () => { + await new Promise((resolve) => setTimeout(resolve, 1)); + expect(getCorrelationId()).toBe('c-1'); + }); + }); + + it('isolates concurrent contexts', async () => { + const seen: string[] = []; + await Promise.all( + ['a', 'b', 'c'].map((id, i) => + runWithCorrelationContext({ correlationId: id }, async () => { + await new Promise((resolve) => setTimeout(resolve, 5 - i)); + seen.push(`${id}=${getCorrelationId()}`); + }), + ), + ); + expect(seen.sort()).toEqual(['a=a', 'b=b', 'c=c']); + }); + + it('bindCorrelationContext restores the captured context later', () => { + const bound = runWithCorrelationContext({ correlationId: 'captured' }, () => + bindCorrelationContext(() => getCorrelationId()), + ); + expect(getCorrelationId()).toBeUndefined(); + expect(bound()).toBe('captured'); + }); +}); + +describe('logger metadata propagation', () => { + it('adds correlationId and requestId from the active context', () => { + const meta = runWithCorrelationContext({ correlationId: 'c-9', requestId: 'r-9' }, () => + withCorrelationMeta({ count: 3 }), + ); + expect(meta).toEqual({ count: 3, correlationId: 'c-9', requestId: 'r-9' }); + }); + + it('keeps ids the caller supplied explicitly', () => { + const meta = runWithCorrelationContext({ correlationId: 'ctx', requestId: 'ctx-r' }, () => + withCorrelationMeta({ correlationId: 'explicit', requestId: 'explicit-r' }), + ); + expect(meta).toEqual({ correlationId: 'explicit', requestId: 'explicit-r' }); + }); + + it('leaves metadata untouched outside a context', () => { + const original = { a: 1 }; + expect(withCorrelationMeta(original)).toBe(original); + expect(withCorrelationMeta(undefined)).toBeUndefined(); + }); +}); + +describe('background job propagation', () => { + it('runs queued work under the enqueuing request correlation id', async () => { + let seenArg: string | undefined; + let seenContext: string | undefined; + const queue = new EventProcessingQueue( + async (_event, _config, requestId) => { + seenArg = requestId; + seenContext = getCorrelationId(); + return true; + }, + { baseDelayMs: 0, pollIntervalMs: 5 }, + ); + + runWithCorrelationContext({ correlationId: 'req-from-api' }, () => { + queue.enqueue( + { id: 'evt-1' } as unknown as StellarSDK.rpc.Api.EventResponse, + { address: 'CA1', events: [] } as never, + ); + }); + + queue.start(); + try { + for (let i = 0; i < 100 && seenArg === undefined; i++) { + await new Promise((resolve) => setTimeout(resolve, 5)); + } + } finally { + queue.stop(); + } + + expect(seenArg).toBe('req-from-api'); + expect(seenContext).toBe('req-from-api'); + }); +}); + +describe('runWithRequestCorrelation over HTTP', () => { + let server: http.Server; + let port: number; + + beforeAll(async () => { + server = http.createServer((req, res) => + runWithRequestCorrelation(req, res, async (ctx) => { + await new Promise((resolve) => setTimeout(resolve, 1)); + if (req.url === '/fail') { + sendErr(res, 404, 'Not found', ErrorCode.NOT_FOUND); + return; + } + res.setHeader('Content-Type', 'application/json'); + res.end(JSON.stringify({ fromContext: getCorrelationId(), fromArg: ctx.correlationId })); + }), + ); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + port = (server.address() as AddressInfo).port; + }); + + afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())); + }); + + function get(path: string, headers: Record = {}) { + return new Promise<{ status: number; headers: http.IncomingHttpHeaders; body: any }>( + (resolve, reject) => { + const req = http.request({ hostname: '127.0.0.1', port, path, headers }, (res) => { + let data = ''; + res.on('data', (c) => (data += c)); + res.on('end', () => + resolve({ status: res.statusCode ?? 0, headers: res.headers, body: JSON.parse(data) }), + ); + }); + req.on('error', reject); + req.end(); + }, + ); + } + + it('generates and returns a UUID when the client sends none', async () => { + const res = await get('/'); + expect(res.headers['x-correlation-id']).toMatch(UUID_V4); + expect(res.body.fromContext).toBe(res.headers['x-correlation-id']); + }); + + it('echoes a valid client id and exposes it to the handler context', async () => { + const res = await get('/', { 'X-Correlation-Id': 'req-12345' }); + expect(res.headers['x-correlation-id']).toBe('req-12345'); + expect(res.body).toEqual({ fromContext: 'req-12345', fromArg: 'req-12345' }); + }); + + it('replaces an invalid client id', async () => { + const res = await get('/', { 'X-Correlation-Id': 'bad id with spaces' }); + expect(res.headers['x-correlation-id']).toMatch(UUID_V4); + }); + + it('stamps the correlation id into error envelopes', async () => { + const res = await get('/fail', { 'X-Correlation-Id': 'req-err-1' }); + expect(res.status).toBe(404); + expect(res.body.error.correlation_id).toBe('req-err-1'); + }); +}); diff --git a/listener/src/middleware/correlation.ts b/listener/src/middleware/correlation.ts new file mode 100644 index 00000000..67eeb0ae --- /dev/null +++ b/listener/src/middleware/correlation.ts @@ -0,0 +1,36 @@ +/** + * Correlation-ID middleware (#986) + * + * Resolves the request's `X-Correlation-Id` (validating any client-supplied + * value and minting a UUID otherwise), echoes it on the response, and runs the + * request handler inside an AsyncLocalStorage context so the ID reaches: + * + * - every structured log line (see `withCorrelationMeta` in utils/logger), + * - every standardized error envelope (`error.correlation_id`), + * - background jobs enqueued while serving the request + * (EventProcessingQueue captures it at enqueue time). + * + * Usage: + * + * http.createServer((req, res) => + * runWithRequestCorrelation(req, res, (ctx) => handleRequest(req, res, ctx)), + * ); + */ + +import type { IncomingMessage, ServerResponse } from 'http'; +import { applyRequestContext, type RequestContext } from '../utils/request-id'; +import { runWithCorrelationContext } from '../utils/correlation-context'; + +export const CORRELATION_ID_HEADER = 'X-Correlation-Id'; + +export function runWithRequestCorrelation( + req: IncomingMessage, + res: ServerResponse, + handler: (context: RequestContext) => T, +): T { + const context = applyRequestContext(req, res); + return runWithCorrelationContext( + { correlationId: context.correlationId, requestId: context.requestId }, + () => handler(context), + ); +} diff --git a/listener/src/middleware/etag.test.ts b/listener/src/middleware/etag.test.ts new file mode 100644 index 00000000..dec11bc4 --- /dev/null +++ b/listener/src/middleware/etag.test.ts @@ -0,0 +1,174 @@ +import http from 'http'; +import type { AddressInfo } from 'net'; +import { + computeETag, + ETAG_CACHE_CONTROL, + ifNoneMatchSatisfied, + sendJsonWithETag, + sendOkWithETag, +} from './etag'; + +interface TestResponse { + status: number; + headers: http.IncomingHttpHeaders; + body: string; +} + +function request( + port: number, + options: { method?: string; headers?: Record } = {}, +): Promise { + return new Promise((resolve, reject) => { + const req = http.request( + { + hostname: '127.0.0.1', + port, + path: '/', + method: options.method ?? 'GET', + headers: options.headers, + }, + (res) => { + let body = ''; + res.on('data', (chunk) => (body += chunk)); + res.on('end', () => resolve({ status: res.statusCode ?? 0, headers: res.headers, body })); + }, + ); + req.on('error', reject); + req.end(); + }); +} + +describe('computeETag', () => { + it('produces a quoted strong tag', () => { + expect(computeETag('{"a":1}')).toMatch(/^"[A-Za-z0-9_-]{43}"$/); + }); + + it('is deterministic for identical bodies', () => { + expect(computeETag('{"events":[]}')).toBe(computeETag('{"events":[]}')); + }); + + it('changes when the body changes', () => { + expect(computeETag('{"count":1}')).not.toBe(computeETag('{"count":2}')); + }); +}); + +describe('ifNoneMatchSatisfied', () => { + const etag = '"abc"'; + + it.each([ + ['exact match', '"abc"', true], + ['weak form of same tag', 'W/"abc"', true], + ['list containing the tag', '"x", "abc" , "y"', true], + ['wildcard', '*', true], + ['different tag', '"def"', false], + ['unquoted value', 'abc', false], + ['empty header', '', false], + ['whitespace header', ' ', false], + ])('%s', (_label, header, expected) => { + expect(ifNoneMatchSatisfied(header, etag)).toBe(expected); + }); + + it('returns false when the header is absent', () => { + expect(ifNoneMatchSatisfied(undefined, etag)).toBe(false); + }); + + it('accepts repeated header values', () => { + expect(ifNoneMatchSatisfied(['"x"', '"abc"'], etag)).toBe(true); + }); +}); + +describe('conditional responses over HTTP', () => { + let server: http.Server; + let port: number; + let data: { count: number; events: string[] }; + let statusCode: number; + + beforeAll(async () => { + server = http.createServer((req, res) => { + if (statusCode === 200) { + sendOkWithETag(req, res, data); + } else { + sendJsonWithETag(req, res, statusCode, { success: false }); + } + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + port = (server.address() as AddressInfo).port; + }); + + afterAll(async () => { + await new Promise((resolve) => server.close(() => resolve())); + }); + + beforeEach(() => { + data = { count: 1, events: ['e1'] }; + statusCode = 200; + }); + + it('includes an ETag and revalidation Cache-Control on GET', async () => { + const res = await request(port); + expect(res.status).toBe(200); + expect(res.headers.etag).toBe(computeETag(res.body)); + expect(res.headers['cache-control']).toBe(ETAG_CACHE_CONTROL); + expect(JSON.parse(res.body)).toEqual({ success: true, data }); + }); + + it('returns the same ETag for unchanged data', async () => { + const first = await request(port); + const second = await request(port); + expect(second.headers.etag).toBe(first.headers.etag); + }); + + it('short-circuits with an empty 304 when If-None-Match matches', async () => { + const { headers } = await request(port); + const res = await request(port, { headers: { 'If-None-Match': headers.etag as string } }); + expect(res.status).toBe(304); + expect(res.body).toBe(''); + expect(res.headers.etag).toBe(headers.etag); + expect(res.headers['content-type']).toBeUndefined(); + }); + + it('returns 304 for a weak validator of the current tag', async () => { + const { headers } = await request(port); + const res = await request(port, { headers: { 'If-None-Match': `W/${headers.etag}` } }); + expect(res.status).toBe(304); + }); + + it('returns the full body when If-None-Match does not match', async () => { + const res = await request(port, { headers: { 'If-None-Match': '"stale"' } }); + expect(res.status).toBe(200); + expect(JSON.parse(res.body).data).toEqual(data); + }); + + it('invalidates the cached tag when the underlying data changes', async () => { + const before = await request(port); + data = { count: 2, events: ['e1', 'e2'] }; + + const after = await request(port, { + headers: { 'If-None-Match': before.headers.etag as string }, + }); + expect(after.status).toBe(200); + expect(after.headers.etag).not.toBe(before.headers.etag); + expect(JSON.parse(after.body).data.count).toBe(2); + }); + + it('serves HEAD with the ETag but no body', async () => { + const get = await request(port); + const head = await request(port, { method: 'HEAD' }); + expect(head.status).toBe(200); + expect(head.headers.etag).toBe(get.headers.etag); + expect(head.body).toBe(''); + }); + + it('does not make non-200 responses conditional', async () => { + statusCode = 503; + const res = await request(port, { headers: { 'If-None-Match': '*' } }); + expect(res.status).toBe(503); + expect(res.headers.etag).toBeUndefined(); + }); + + it('does not make non-GET requests conditional', async () => { + const res = await request(port, { method: 'POST', headers: { 'If-None-Match': '*' } }); + expect(res.status).toBe(200); + expect(res.headers.etag).toBeUndefined(); + }); +}); diff --git a/listener/src/middleware/etag.ts b/listener/src/middleware/etag.ts new file mode 100644 index 00000000..2c9c6249 --- /dev/null +++ b/listener/src/middleware/etag.ts @@ -0,0 +1,104 @@ +/** + * ETag / conditional GET support (#985) + * + * Event feeds and statistics are polled far more often than they change. For + * those endpoints the serialized response body is hashed into a strong ETag; + * a client that sends the tag back in `If-None-Match` gets an empty + * `304 Not Modified` instead of the full payload. + * + * The tag is derived from the exact bytes that would be sent, so it changes + * whenever the underlying data does — there is no separate cache to invalidate. + * `Cache-Control: no-cache` tells clients and proxies they may store the + * response but must revalidate it on every use. + * + * sendOkWithETag(req, res, { count, events }); + */ + +import crypto from 'crypto'; +import type { IncomingMessage, ServerResponse } from 'http'; +import { ok } from '../utils/response'; + +export const ETAG_CACHE_CONTROL = 'private, no-cache'; + +/** Strong ETag: quoted base64url SHA-256 of the representation. */ +export function computeETag(body: string | Buffer): string { + const digest = crypto.createHash('sha256').update(body).digest('base64url'); + return `"${digest}"`; +} + +/** Drops a weak-validator prefix so `W/"x"` and `"x"` compare equal. */ +function opaqueTag(tag: string): string { + return tag.startsWith('W/') ? tag.slice(2) : tag; +} + +/** + * True when `If-None-Match` matches `etag`. Uses the weak comparison RFC 9110 + * §13.1.2 prescribes for this header, and honours `*` and comma-separated + * lists. Absent or empty headers never match. + */ +export function ifNoneMatchSatisfied( + ifNoneMatch: string | string[] | undefined, + etag: string, +): boolean { + const raw = Array.isArray(ifNoneMatch) ? ifNoneMatch.join(',') : ifNoneMatch; + if (!raw || !raw.trim()) return false; + if (raw.trim() === '*') return true; + + const target = opaqueTag(etag); + return raw + .split(',') + .map((candidate) => candidate.trim()) + .filter(Boolean) + .some((candidate) => opaqueTag(candidate) === target); +} + +/** + * Writes `body` as JSON with an ETag, or a bodiless 304 when the client's + * `If-None-Match` already holds the current tag. Only 200 responses to GET / + * HEAD are made conditional; anything else is sent unchanged. + * + * @returns true when a 304 was sent. + */ +export function sendJsonWithETag( + req: IncomingMessage, + res: ServerResponse, + statusCode: number, + body: unknown, +): boolean { + const serialized = JSON.stringify(body); + const method = req.method ?? 'GET'; + const conditional = statusCode === 200 && (method === 'GET' || method === 'HEAD'); + + if (!conditional) { + res.writeHead(statusCode, { 'Content-Type': 'application/json' }); + res.end(serialized); + return false; + } + + const etag = computeETag(serialized); + res.setHeader('ETag', etag); + res.setHeader('Cache-Control', ETAG_CACHE_CONTROL); + + if (ifNoneMatchSatisfied(req.headers['if-none-match'], etag)) { + // A 304 carries no body, so no representation headers either. + res.removeHeader('Content-Type'); + res.removeHeader('Content-Length'); + res.writeHead(304); + res.end(); + return true; + } + + res.writeHead(statusCode, { 'Content-Type': 'application/json' }); + res.end(method === 'HEAD' ? undefined : serialized); + return false; +} + +/** `sendOk` counterpart that adds ETag / If-None-Match handling. */ +export function sendOkWithETag( + req: IncomingMessage, + res: ServerResponse, + data: T, + meta?: Record, +): boolean { + return sendJsonWithETag(req, res, 200, ok(data, meta)); +} diff --git a/listener/src/services/api-credential-service.test.ts b/listener/src/services/api-credential-service.test.ts new file mode 100644 index 00000000..5157123e --- /dev/null +++ b/listener/src/services/api-credential-service.test.ts @@ -0,0 +1,126 @@ +import { + ApiCredentialService, + CredentialValidationError, + InMemoryApiCredentialStore, + parseApiKey, +} from './api-credential-service'; + +describe('ApiCredentialService', () => { + let store: InMemoryApiCredentialStore; + let service: ApiCredentialService; + + beforeEach(() => { + store = new InMemoryApiCredentialStore(); + service = new ApiCredentialService({ store }); + }); + + describe('create', () => { + it('returns the full api key exactly once, in the creation result', async () => { + const created = await service.create({ name: 'CI', scopes: ['events:read'] }); + + expect(created.apiKey).toMatch(/^nck_[0-9a-f]{16}_[A-Za-z0-9_-]{43}$/); + expect(parseApiKey(created.apiKey)?.id).toBe(created.id); + expect(created.keyPrefix).toBe(`nck_${created.id}`); + expect(created.scopes).toEqual(['events:read']); + }); + + it('stores only a salted scrypt hash, never the secret', async () => { + const created = await service.create({ name: 'CI', scopes: ['events:read'] }); + const secret = parseApiKey(created.apiKey)!.secret; + const record = store.get(created.id)!; + + expect(record.secretHash).toMatch(/^scrypt\$16384\$8\$1\$[A-Za-z0-9_-]+\$[A-Za-z0-9_-]+$/); + expect(JSON.stringify(record)).not.toContain(secret); + }); + + it('uses a unique salt per credential', async () => { + const a = await service.create({ name: 'a', scopes: ['events:read'] }); + const b = await service.create({ name: 'b', scopes: ['events:read'] }); + const saltOf = (id: string) => store.get(id)!.secretHash.split('$')[4]; + expect(saltOf(a.id)).not.toBe(saltOf(b.id)); + }); + + it('de-duplicates scopes', async () => { + const created = await service.create({ + name: 'dup', + scopes: ['stats:read', 'stats:read', 'events:read'], + }); + expect(created.scopes).toEqual(['stats:read', 'events:read']); + }); + + it.each([ + ['missing name', { scopes: ['events:read'] }], + ['blank name', { name: ' ', scopes: ['events:read'] }], + ['overlong name', { name: 'x'.repeat(101), scopes: ['events:read'] }], + ['missing scopes', { name: 'n' }], + ['empty scopes', { name: 'n', scopes: [] }], + ['unknown scope', { name: 'n', scopes: ['admin:*'] }], + ])('rejects %s', async (_label, input) => { + await expect(service.create(input as never)).rejects.toBeInstanceOf( + CredentialValidationError, + ); + }); + }); + + describe('read paths never expose secret material', () => { + it('list() and get() omit apiKey and secretHash', async () => { + const created = await service.create({ name: 'CI', scopes: ['events:read'] }); + const secret = parseApiKey(created.apiKey)!.secret; + + for (const view of [...service.list(), service.get(created.id)!]) { + expect(view).not.toHaveProperty('apiKey'); + expect(view).not.toHaveProperty('secretHash'); + expect(JSON.stringify(view)).not.toContain(secret); + } + }); + + it('get() returns null for unknown ids', () => { + expect(service.get('0000000000000000')).toBeNull(); + }); + }); + + describe('verify', () => { + it('accepts the issued key and records last use', async () => { + const created = await service.create({ name: 'CI', scopes: ['stats:read'] }); + const verified = await service.verify(created.apiKey); + expect(verified?.id).toBe(created.id); + expect(verified?.scopes).toEqual(['stats:read']); + expect(verified?.lastUsedAt).not.toBeNull(); + }); + + it('rejects a key with the right id but a wrong secret', async () => { + const created = await service.create({ name: 'CI', scopes: ['stats:read'] }); + const forged = `nck_${created.id}_${'A'.repeat(43)}`; + expect(await service.verify(forged)).toBeNull(); + }); + + it('rejects unknown ids and malformed keys', async () => { + expect(await service.verify(`nck_${'f'.repeat(16)}_${'A'.repeat(43)}`)).toBeNull(); + expect(await service.verify('nck_short')).toBeNull(); + expect(await service.verify('')).toBeNull(); + }); + + it('rejects a revoked key even after a cached successful verification', async () => { + const created = await service.create({ name: 'CI', scopes: ['stats:read'] }); + expect(await service.verify(created.apiKey)).not.toBeNull(); + + const revoked = service.revoke(created.id); + expect(revoked?.revokedAt).not.toBeNull(); + expect(await service.verify(created.apiKey)).toBeNull(); + }); + }); + + describe('hasActiveCredentials', () => { + it('tracks non-revoked credentials', async () => { + expect(service.hasActiveCredentials()).toBe(false); + const created = await service.create({ name: 'CI', scopes: ['events:read'] }); + expect(service.hasActiveCredentials()).toBe(true); + service.revoke(created.id); + expect(service.hasActiveCredentials()).toBe(false); + }); + }); + + it('revoke() returns null for unknown ids', () => { + expect(service.revoke('0000000000000000')).toBeNull(); + }); +}); diff --git a/listener/src/services/api-credential-service.ts b/listener/src/services/api-credential-service.ts new file mode 100644 index 00000000..f9d77ad7 --- /dev/null +++ b/listener/src/services/api-credential-service.ts @@ -0,0 +1,263 @@ +/** + * Scoped API credential service (#988) + * + * Issues, verifies and revokes integration API keys that carry explicit + * permission scopes. + * + * Secret handling + * --------------- + * - Keys have the form `nck_<16 hex id>_<43 char base64url secret>`. The id is + * a lookup handle; only the secret is sensitive. + * - The secret is stored solely as an scrypt hash (memory-hard KDF from Node's + * crypto module) with a per-credential random salt, so a leaked store does + * not yield usable keys. + * - The full key is returned only from `create()`. Every other read returns + * the public `ApiCredential` view, which has no secret or hash field. + * + * Because scrypt is deliberately expensive, a successful verification is + * cached briefly under a SHA-256 of the presented key. Revocation clears the + * cache entry immediately. + */ + +import crypto from 'crypto'; +import { promisify } from 'util'; +import { + isApiScope, + type ApiCredential, + type ApiCredentialRecord, + type ApiScope, + type CreateApiCredentialInput, + type CreatedApiCredential, +} from '../types/api-credential'; + +const scryptAsync = promisify(crypto.scrypt) as ( + password: crypto.BinaryLike, + salt: crypto.BinaryLike, + keylen: number, + options: crypto.ScryptOptions, +) => Promise; + +export const API_KEY_PREFIX = 'nck'; +const API_KEY_PATTERN = /^nck_([0-9a-f]{16})_([A-Za-z0-9_-]{43})$/; + +const SCRYPT_PARAMS = { N: 16384, r: 8, p: 1 } as const; +const SCRYPT_KEYLEN = 32; +const SALT_BYTES = 16; +const DEFAULT_VERIFY_CACHE_TTL_MS = 60_000; +export const MAX_CREDENTIAL_NAME_LENGTH = 100; + +/** Pluggable persistence. The default keeps records in process memory. */ +export interface ApiCredentialStore { + get(id: string): ApiCredentialRecord | undefined; + list(): ApiCredentialRecord[]; + save(record: ApiCredentialRecord): void; +} + +export class InMemoryApiCredentialStore implements ApiCredentialStore { + private readonly records = new Map(); + + get(id: string): ApiCredentialRecord | undefined { + const record = this.records.get(id); + return record ? { ...record, scopes: [...record.scopes] } : undefined; + } + + list(): ApiCredentialRecord[] { + return [...this.records.values()].map((r) => ({ ...r, scopes: [...r.scopes] })); + } + + save(record: ApiCredentialRecord): void { + this.records.set(record.id, { ...record, scopes: [...record.scopes] }); + } +} + +export class CredentialValidationError extends Error { + constructor( + message: string, + public readonly issues: Array<{ field: string; message: string }>, + ) { + super(message); + this.name = 'CredentialValidationError'; + } +} + +export interface ApiCredentialServiceOptions { + store?: ApiCredentialStore; + verifyCacheTtlMs?: number; + now?: () => Date; +} + +export function parseApiKey(value: string): { id: string; secret: string } | null { + const match = API_KEY_PATTERN.exec(value); + return match ? { id: match[1], secret: match[2] } : null; +} + +export function looksLikeScopedApiKey(value: string): boolean { + return value.startsWith(`${API_KEY_PREFIX}_`); +} + +async function hashSecret(secret: string): Promise { + const salt = crypto.randomBytes(SALT_BYTES); + const derived = await scryptAsync(secret, salt, SCRYPT_KEYLEN, SCRYPT_PARAMS); + const { N, r, p } = SCRYPT_PARAMS; + return `scrypt$${N}$${r}$${p}$${salt.toString('base64url')}$${derived.toString('base64url')}`; +} + +async function secretMatches(secret: string, encoded: string): Promise { + const [algo, n, r, p, saltB64, hashB64] = encoded.split('$'); + if (algo !== 'scrypt' || !saltB64 || !hashB64) return false; + const expected = Buffer.from(hashB64, 'base64url'); + const derived = await scryptAsync(secret, Buffer.from(saltB64, 'base64url'), expected.length, { + N: Number(n), + r: Number(r), + p: Number(p), + }); + return derived.length === expected.length && crypto.timingSafeEqual(derived, expected); +} + +function toPublic(record: ApiCredentialRecord): ApiCredential { + return { + id: record.id, + name: record.name, + scopes: [...record.scopes], + keyPrefix: `${API_KEY_PREFIX}_${record.id}`, + createdAt: record.createdAt, + lastUsedAt: record.lastUsedAt, + revokedAt: record.revokedAt, + }; +} + +export function validateCreateCredentialInput(input: unknown): CreateApiCredentialInput { + const issues: Array<{ field: string; message: string }> = []; + const body = (input ?? {}) as Record; + + const name = typeof body.name === 'string' ? body.name.trim() : ''; + if (!name) { + issues.push({ field: 'name', message: 'name is required' }); + } else if (name.length > MAX_CREDENTIAL_NAME_LENGTH) { + issues.push({ + field: 'name', + message: `name must be at most ${MAX_CREDENTIAL_NAME_LENGTH} characters`, + }); + } + + const scopes: ApiScope[] = []; + if (!Array.isArray(body.scopes) || body.scopes.length === 0) { + issues.push({ field: 'scopes', message: 'scopes must be a non-empty array' }); + } else { + for (const scope of body.scopes) { + if (!isApiScope(scope)) { + issues.push({ field: 'scopes', message: `unknown scope: ${String(scope)}` }); + } else if (!scopes.includes(scope)) { + scopes.push(scope); + } + } + } + + if (issues.length > 0) { + throw new CredentialValidationError('Invalid credential request', issues); + } + return { name, scopes }; +} + +export class ApiCredentialService { + private readonly store: ApiCredentialStore; + private readonly verifyCacheTtlMs: number; + private readonly now: () => Date; + /** sha256(presented key) → { credential id, expiry } for recently verified keys. */ + private readonly verifyCache = new Map(); + /** Burns equivalent scrypt work for unknown ids so timing does not reveal them. */ + private dummyHash: Promise | null = null; + + constructor(options: ApiCredentialServiceOptions = {}) { + this.store = options.store ?? new InMemoryApiCredentialStore(); + this.verifyCacheTtlMs = options.verifyCacheTtlMs ?? DEFAULT_VERIFY_CACHE_TTL_MS; + this.now = options.now ?? (() => new Date()); + } + + /** Issues a new credential. The returned `apiKey` is never retrievable again. */ + async create(input: CreateApiCredentialInput): Promise { + const { name, scopes } = validateCreateCredentialInput(input); + let id = crypto.randomBytes(8).toString('hex'); + while (this.store.get(id)) id = crypto.randomBytes(8).toString('hex'); + const secret = crypto.randomBytes(32).toString('base64url'); + + const record: ApiCredentialRecord = { + id, + name, + scopes, + secretHash: await hashSecret(secret), + createdAt: this.now().toISOString(), + lastUsedAt: null, + revokedAt: null, + }; + this.store.save(record); + + return { ...toPublic(record), apiKey: `${API_KEY_PREFIX}_${id}_${secret}` }; + } + + list(): ApiCredential[] { + return this.store.list().map(toPublic); + } + + get(id: string): ApiCredential | null { + const record = this.store.get(id); + return record ? toPublic(record) : null; + } + + /** Revokes a credential. Returns the updated view, or null when unknown. */ + revoke(id: string): ApiCredential | null { + const record = this.store.get(id); + if (!record) return null; + if (!record.revokedAt) { + record.revokedAt = this.now().toISOString(); + this.store.save(record); + } + for (const [key, entry] of this.verifyCache) { + if (entry.id === id) this.verifyCache.delete(key); + } + return toPublic(record); + } + + /** True when at least one non-revoked credential exists. */ + hasActiveCredentials(): boolean { + return this.store.list().some((r) => !r.revokedAt); + } + + /** + * Resolves a presented API key to its credential. Returns null for + * malformed, unknown, revoked, or mismatching keys. + */ + async verify(apiKey: string): Promise { + const parsed = parseApiKey(apiKey); + if (!parsed) return null; + + const cacheKey = crypto.createHash('sha256').update(apiKey).digest('base64url'); + const cached = this.verifyCache.get(cacheKey); + const nowMs = this.now().getTime(); + if (cached && cached.expiresAt > nowMs && cached.id === parsed.id) { + return this.touch(parsed.id); + } + if (cached) this.verifyCache.delete(cacheKey); + + const record = this.store.get(parsed.id); + if (!record) { + this.dummyHash ??= hashSecret('dummy-secret-for-timing'); + await secretMatches(parsed.secret, await this.dummyHash); + return null; + } + + const matches = await secretMatches(parsed.secret, record.secretHash); + if (!matches || record.revokedAt) return null; + + this.verifyCache.set(cacheKey, { id: record.id, expiresAt: nowMs + this.verifyCacheTtlMs }); + return this.touch(record.id); + } + + private touch(id: string): ApiCredential | null { + const record = this.store.get(id); + if (!record || record.revokedAt) return null; + record.lastUsedAt = this.now().toISOString(); + this.store.save(record); + return toPublic(record); + } +} diff --git a/listener/src/services/event-processing-queue.ts b/listener/src/services/event-processing-queue.ts index 20df7735..66f5dce5 100644 --- a/listener/src/services/event-processing-queue.ts +++ b/listener/src/services/event-processing-queue.ts @@ -2,6 +2,7 @@ import * as StellarSDK from '@stellar/stellar-sdk'; import { ContractConfig } from '../types'; import logger from '../utils/logger'; import { generateCorrelationId } from '../utils/request-id'; +import { getCorrelationId, runWithCorrelationId } from '../utils/correlation-context'; import { RetryFailureType, RetryPolicy, @@ -102,7 +103,9 @@ export class EventProcessingQueue { requestId?: string, priority: Priority = Priority.Medium ): boolean { - const correlationId = requestId ?? generateCorrelationId(); + // Inherit the enqueuing request's correlation ID (#986) so the background + // processing of this event is traceable back to the call that queued it. + const correlationId = requestId ?? getCorrelationId() ?? generateCorrelationId(); const fingerprint = buildEventFingerprint(event, contractConfig.address); if (this.queuedFingerprints.has(fingerprint)) { @@ -233,7 +236,11 @@ export class EventProcessingQueue { const attempt = item.retryCount + 1; try { - const success = await this.processor(item.event, item.contractConfig, item.requestId); + // The queue drains on its own timer, outside the enqueuing request's + // async chain, so restore the item's correlation context explicitly. + const success = await runWithCorrelationId(item.requestId, () => + this.processor(item.event, item.contractConfig, item.requestId), + ); const duration = Date.now() - startTime; if (success) { diff --git a/listener/src/types/api-credential.ts b/listener/src/types/api-credential.ts new file mode 100644 index 00000000..acdbc3ce --- /dev/null +++ b/listener/src/types/api-credential.ts @@ -0,0 +1,58 @@ +/** + * Scoped API credentials (#988) + * + * Integration keys carry an explicit list of permission scopes instead of + * implicit full administrative access. Endpoints declare the scopes they need + * and the server rejects a credential lacking any of them with + * `403 INVALID_PERMISSIONS`. + */ + +export const API_SCOPES = [ + 'events:read', + 'events:write', + 'stats:read', + 'notifications:read', + 'notifications:write', + 'webhooks:manage', + 'credentials:manage', +] as const; + +export type ApiScope = (typeof API_SCOPES)[number]; + +export function isApiScope(value: unknown): value is ApiScope { + return typeof value === 'string' && (API_SCOPES as readonly string[]).includes(value); +} + +/** Stored form. `secretHash` never leaves the service layer. */ +export interface ApiCredentialRecord { + id: string; + name: string; + scopes: ApiScope[]; + /** scrypt-derived hash of the secret, `scrypt$N$r$p$salt$hash` (base64url fields). */ + secretHash: string; + createdAt: string; + lastUsedAt: string | null; + revokedAt: string | null; +} + +/** Public view returned by list / get endpoints — no secret material. */ +export interface ApiCredential { + id: string; + name: string; + scopes: ApiScope[]; + /** Non-secret key prefix (`nck_`) so operators can recognise a key. */ + keyPrefix: string; + createdAt: string; + lastUsedAt: string | null; + revokedAt: string | null; +} + +/** Returned exactly once, by the create call. `apiKey` is not recoverable afterwards. */ +export interface CreatedApiCredential extends ApiCredential { + apiKey: string; +} + +export interface CreateApiCredentialInput { + name: string; + scopes: ApiScope[]; +} diff --git a/listener/src/utils/correlation-context.ts b/listener/src/utils/correlation-context.ts new file mode 100644 index 00000000..cfad8e71 --- /dev/null +++ b/listener/src/utils/correlation-context.ts @@ -0,0 +1,57 @@ +/** + * Correlation-ID context propagation (#986) + * + * Holds the active request's correlation ID in AsyncLocalStorage so that every + * log line, error envelope and background job started while serving a request + * is tagged with the same ID — without threading it through every call site. + * + * runWithCorrelationContext({ correlationId, requestId }, () => handle(req, res)); + * + * // anywhere downstream, including after awaits: + * getCorrelationId(); // → the request's correlation ID + * + * Work that is deferred onto a queue or timer and runs outside the request's + * async chain should either capture the ID explicitly or wrap its callback + * with `bindCorrelationContext`. + */ + +import { AsyncLocalStorage } from 'async_hooks'; + +export interface CorrelationContext { + correlationId: string; + requestId?: string; +} + +const storage = new AsyncLocalStorage(); + +/** Run `fn` with `context` as the active correlation context. */ +export function runWithCorrelationContext(context: CorrelationContext, fn: () => T): T { + return storage.run(context, fn); +} + +/** Run `fn` tagged with `correlationId`, keeping any active requestId. */ +export function runWithCorrelationId(correlationId: string, fn: () => T): T { + const current = storage.getStore(); + return storage.run({ ...current, correlationId }, fn); +} + +export function getCorrelationContext(): CorrelationContext | undefined { + return storage.getStore(); +} + +export function getCorrelationId(): string | undefined { + return storage.getStore()?.correlationId; +} + +/** + * Captures the current correlation context and returns a function that + * restores it when invoked — for callbacks handed to timers, emitters, or job + * queues that run outside the originating async chain. + */ +export function bindCorrelationContext( + fn: (...args: A) => R, +): (...args: A) => R { + const captured = storage.getStore(); + if (!captured) return fn; + return (...args: A) => storage.run(captured, () => fn(...args)); +} diff --git a/listener/src/utils/logger.ts b/listener/src/utils/logger.ts index be925189..147ea154 100644 --- a/listener/src/utils/logger.ts +++ b/listener/src/utils/logger.ts @@ -5,6 +5,7 @@ import { redactObject, redactString, } from './redact'; +import { getCorrelationContext } from './correlation-context'; // --------------------------------------------------------------------------- // Types @@ -21,6 +22,8 @@ export interface FormattedError { export interface LogContext { /** Identifier scoped to a single poll/request cycle for end-to-end tracing. */ requestId?: string; + /** Cross-service trace ID; filled from the active request context when omitted (#986). */ + correlationId?: string; /** Elapsed milliseconds for timed operations (RPC calls, webhook delivery, etc.). */ durationMs?: number; /** Any additional structured fields the caller wants to attach. */ @@ -243,14 +246,32 @@ function formatMeta(meta: LogContext): LogContext { return redactObject(normalized as Record) as LogContext; } +/** + * Adds the active request's `correlationId` / `requestId` (#986) to the log + * metadata unless the caller already supplied them, so every line emitted while + * serving a request — or running a job bound to it — can be joined in the + * aggregator without each call site threading the IDs through by hand. + */ +export function withCorrelationMeta(meta?: LogContext): LogContext | undefined { + const context = getCorrelationContext(); + if (!context) return meta; + const merged: LogContext = { ...meta }; + if (merged.correlationId === undefined) merged.correlationId = context.correlationId; + if (merged.requestId === undefined && context.requestId !== undefined) { + merged.requestId = context.requestId; + } + return merged; +} + function logWithMeta( level: LogLevel, message: string, - meta?: LogContext + rawMeta?: LogContext ): void { // The message itself is redacted too: interpolated strings such as // `Posting to ${webhookUrl}` would otherwise bypass metadata redaction. const safeMessage = typeof message === 'string' ? redactString(message) : message; + const meta = withCorrelationMeta(rawMeta); if (meta && Object.keys(meta).length > 0) { baseLogger[level](safeMessage, formatMeta(meta)); } else { diff --git a/listener/src/utils/request-id.ts b/listener/src/utils/request-id.ts index ef670efc..40881af4 100644 --- a/listener/src/utils/request-id.ts +++ b/listener/src/utils/request-id.ts @@ -40,23 +40,35 @@ export function resolveRequestId(incomingHeader: string | string[] | undefined): return generateRequestId(); } +/** Longest client-supplied correlation ID that will be reused (#986). */ +export const MAX_CORRELATION_ID_LENGTH = 64; + +/** + * Client-supplied correlation IDs must be 1–64 characters drawn from + * `[A-Za-z0-9._:-]` and start with an alphanumeric. That admits UUIDs and the + * usual `req-12345` / `svc:abc.1` styles while excluding whitespace, control + * characters, quotes and braces — anything that could forge a log line or + * break out of a structured log field (#986). + */ +const CORRELATION_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._:-]{0,63}$/; + +export function isValidCorrelationId(value: string): boolean { + return CORRELATION_ID_PATTERN.test(value); +} + /** * Resolves a correlation ID for a request. - * Honours an incoming X-Correlation-Id header if present and non-blank, - * otherwise generates a new UUID. Correlation IDs are intentionally more - * permissive than request IDs (callers may use longer opaque tokens). + * Honours an incoming X-Correlation-Id header when it passes + * `isValidCorrelationId`; a missing, blank, or invalid value is replaced with + * a freshly generated UUID rather than echoed back or logged. */ export function resolveCorrelationId(incomingHeader: string | string[] | undefined): string { const incoming = Array.isArray(incomingHeader) ? incomingHeader[0] : incomingHeader; const trimmed = incoming?.trim() ?? ''; - if (!trimmed) { - return randomUUID(); - } - // Bound length / charset so log aggregators stay safe - if (trimmed.length > 200 || /[\r\n\0]/.test(trimmed)) { - return randomUUID(); + if (trimmed && isValidCorrelationId(trimmed)) { + return trimmed; } - return trimmed; + return randomUUID(); } export interface RequestContext { diff --git a/listener/src/utils/response.ts b/listener/src/utils/response.ts index 77488b69..85fbcb9d 100644 --- a/listener/src/utils/response.ts +++ b/listener/src/utils/response.ts @@ -12,14 +12,15 @@ * "meta": * } * - * Error + * Error (#987) * ----- * { * "success": false, * "error": { - * "code": "", - * "message": "", - * "details": + * "code": "", – stable, see errors/api-error-codes.ts + * "message": "", + * "details": , + * "correlation_id": "" – present when known * } * } * @@ -34,6 +35,11 @@ */ import http from 'http'; +import { ErrorCode, type ErrorCodeValue } from '../errors/api-error-codes'; +import { getCorrelationId } from './correlation-context'; + +export { ErrorCode, ERROR_CODE_CATALOG, isKnownErrorCode } from '../errors/api-error-codes'; +export type { ErrorCodeValue } from '../errors/api-error-codes'; // --------------------------------------------------------------------------- // Types @@ -48,7 +54,8 @@ export interface ApiSuccess { export interface ApiErrorBody { code: string; message: string; - details?: unknown; + details: unknown; + correlation_id?: string; } export interface ApiError { @@ -58,27 +65,6 @@ export interface ApiError { export type ApiResponse = ApiSuccess | ApiError; -// --------------------------------------------------------------------------- -// Well-known error codes -// --------------------------------------------------------------------------- - -export const ErrorCode = { - BAD_REQUEST: 'BAD_REQUEST', - UNAUTHORIZED: 'UNAUTHORIZED', - FORBIDDEN: 'FORBIDDEN', - NOT_FOUND: 'NOT_FOUND', - CONFLICT: 'CONFLICT', - UNPROCESSABLE: 'UNPROCESSABLE', - PAYLOAD_TOO_LARGE: 'PAYLOAD_TOO_LARGE', - RATE_LIMITED: 'RATE_LIMITED', - INTERNAL_ERROR: 'INTERNAL_ERROR', - SERVICE_UNAVAILABLE: 'SERVICE_UNAVAILABLE', - PARSE_ERROR: 'PARSE_ERROR', - UNSUPPORTED_MEDIA_TYPE: 'UNSUPPORTED_MEDIA_TYPE', -} as const; - -export type ErrorCodeValue = (typeof ErrorCode)[keyof typeof ErrorCode]; - // --------------------------------------------------------------------------- // Builders // --------------------------------------------------------------------------- @@ -97,15 +83,18 @@ export function ok(data: T, meta?: Record): ApiSuccess { * * @param message Human-readable description forwarded to the client. * @param code Machine-readable code; defaults to `INTERNAL_ERROR`. - * @param details Optional extra context (validation errors, field names, …). + * @param details Optional extra context (validation errors, field names, …); + * defaults to `[]` so clients can always iterate it. + * @param correlationId Defaults to the active request's correlation ID. */ export function err( message: string, code: ErrorCodeValue | string = ErrorCode.INTERNAL_ERROR, details?: unknown, + correlationId: string | undefined = getCorrelationId(), ): ApiError { - const body: ApiErrorBody = { code, message }; - if (details !== undefined) body.details = details; + const body: ApiErrorBody = { code, message, details: details ?? [] }; + if (correlationId) body.correlation_id = correlationId; return { success: false, error: body }; } @@ -151,6 +140,7 @@ export function sendErr( message: string, code: ErrorCodeValue | string = ErrorCode.INTERNAL_ERROR, details?: unknown, + correlationId?: string, ): void { - sendJson(res, statusCode, err(message, code, details)); + sendJson(res, statusCode, err(message, code, details, correlationId ?? getCorrelationId())); }