From 386e3d16fc23c0fafbaa2870c6208074e16bb946 Mon Sep 17 00:00:00 2001 From: Oluwaseyitan Animasaun Date: Thu, 30 Jul 2026 11:16:46 +0100 Subject: [PATCH 1/4] feat: add ETag/304 caching to GET /api/apis --- docs/etag-caching.md | 151 ++++++++++++ src/middleware/etagCache.test.ts | 183 +++++++++++++++ src/middleware/etagCache.ts | 150 ++++++++++++ src/routes/apis.etag.test.ts | 382 +++++++++++++++++++++++++++++++ src/routes/apis.ts | 42 +++- 5 files changed, 906 insertions(+), 2 deletions(-) create mode 100644 docs/etag-caching.md create mode 100644 src/middleware/etagCache.test.ts create mode 100644 src/middleware/etagCache.ts create mode 100644 src/routes/apis.etag.test.ts diff --git a/docs/etag-caching.md b/docs/etag-caching.md new file mode 100644 index 00000000..0cb36404 --- /dev/null +++ b/docs/etag-caching.md @@ -0,0 +1,151 @@ +# ETag / 304 Caching — GET /api/apis + +**Issue:** #866 +**Added in:** `src/middleware/etagCache.ts`, `src/routes/apis.ts` + +## Overview + +`GET /api/apis` and `GET /api/apis/:id` support HTTP conditional requests via +strong ETags and `304 Not Modified` responses. Clients that cache the response +can supply an `If-None-Match` header on subsequent requests; when the response +content has not changed, the server replies with `304` and an empty body, +saving bandwidth and client-side parsing time. + +## How It Works + +1. On every successful `GET /api/apis` or `GET /api/apis/:id` response, the + server serialises the response body to JSON, computes a 32-character + SHA-256 hex digest, and emits it as a strong `ETag` header: + + ``` + ETag: "a7ffc6f8bf1ed76651c14756a061d662" + ``` + +2. The client stores the ETag alongside the cached response body. + +3. On the next request, the client sends the stored ETag in `If-None-Match`: + + ``` + GET /api/apis HTTP/1.1 + If-None-Match: "a7ffc6f8bf1ed76651c14756a061d662" + ``` + +4. If the current response body would hash to the same digest, the server + returns `304 Not Modified` with an empty body (saving the transfer cost of + the JSON payload). + +5. If the data has changed (new APIs added, existing ones updated), the hash + differs and the server returns the full `200` response with the updated body + and the new ETag. + +## Routes Covered + +| Route | ETag? | Notes | +|---|---|---| +| `GET /api/apis` | ✅ | Covers all query params (limit, offset, category, search). Different params → different ETags. | +| `GET /api/apis/:id` | ✅ | Per-resource ETag, includes endpoint list in the hash. | +| `POST /api/apis` | ❌ | Write operation — no caching. | +| `POST /api/apis/:id/endpoints/bulk` | ❌ | Write operation — no caching. | + +## Response Headers + +| Header | Example | Description | +|---|---|---| +| `ETag` | `"a7ffc6f8bf1ed76651c14756a061d662"` | Strong ETag. Always present on 200 responses from the covered GET routes. | + +## Request Headers + +| Header | Example | Description | +|---|---|---| +| `If-None-Match` | `"a7ffc6f8bf1ed76651c14756a061d662"` | Single ETag, comma-separated list, weak ETag (`W/"..."`), or wildcard (`*`). | + +## ETag Format + +ETags are **strong** (no `W/` prefix). The value is the first 32 hex characters +of the SHA-256 digest of the JSON-serialized response body, wrapped in +double-quotes. + +- **Strong** because the digest changes if and only if the response bytes + change — precise byte-level equivalence, not just semantic equivalence. +- **Body-derived** (not timestamp- or version-derived) so two requests that + return the same data always produce the same ETag, regardless of when they + are made. + +## Express Built-in ETag + +Express 4.x generates **weak** ETags by default (`app.set('etag', 'weak')` is +the implicit default). The Callora backend does **not** disable Express's +default ETag generation globally, because that would affect all other routes. + +Instead, `apis.ts` sets the `ETag` header explicitly before calling +`res.json()`. When an `ETag` header is already present at response finalisation +time, Express skips its own ETag generation for that response. + +## Interaction with the ListingsCache + +`GET /api/apis` already uses an in-process `ListingsCache` (30-second TTL by +default) to skip DB reads on repeated requests. ETag evaluation happens on top +of this layer: + +- **Cache hit + matching ETag:** Both the DB read *and* the HTTP body transfer + are skipped. This is the fully-optimised path. +- **Cache hit + stale/absent ETag:** The DB read is skipped (ListingsCache + hit), but the full 200 body is returned. +- **Cache miss + matching ETag:** The DB read happens (cache miss), the + response is built, the ETag is computed, and the 304 shortcut is applied. + The body transfer is saved even on cache misses with a warm client. + +## Example with curl + +**First request — get the ETag:** + +```bash +curl -si https://api.callora.io/api/apis | grep -E '^(HTTP|etag|ETag)' +``` + +``` +HTTP/2 200 +etag: "a7ffc6f8bf1ed76651c14756a061d662" +``` + +**Subsequent request — 304 when data is unchanged:** + +```bash +curl -si https://api.callora.io/api/apis \ + -H 'If-None-Match: "a7ffc6f8bf1ed76651c14756a061d662"' +``` + +``` +HTTP/2 304 +etag: "a7ffc6f8bf1ed76651c14756a061d662" +``` + +**Subsequent request — 200 when data has changed:** + +```bash +curl -si https://api.callora.io/api/apis \ + -H 'If-None-Match: "a7ffc6f8bf1ed76651c14756a061d662"' +``` + +``` +HTTP/2 200 +etag: "b3d2ae19f7c4e0a5d8f92c6b1e4a8305" +content-type: application/json; charset=utf-8 + +{"data":[...],"meta":{...}} +``` + +## Implementation Notes + +The ETag computation uses the **compute-then-compare** approach: the route +builds the full response object before computing the ETag and issuing a 304. +For `GET /api/apis`, skipping the full response build on 304 would require +restructuring the cache layer significantly. Given that the ListingsCache +already skips the DB on cache hits, and that `JSON.stringify` + SHA-256 on a +20-item listing is sub-millisecond, the compute-then-compare cost is +negligible. + +The implementation is in `src/middleware/etagCache.ts` and exports three pure +functions: `computeStrongETag`, `parseIfNoneMatch`, and `isETagMatch`. No new +npm dependencies were introduced — only Node.js's built-in `node:crypto` module +is used. diff --git a/src/middleware/etagCache.test.ts b/src/middleware/etagCache.test.ts new file mode 100644 index 00000000..b24074df --- /dev/null +++ b/src/middleware/etagCache.test.ts @@ -0,0 +1,183 @@ +/** + * @file src/middleware/etagCache.test.ts + * @description Focused unit tests for the ETag / 304 caching utilities in + * src/middleware/etagCache.ts. + * + * These tests cover the pure helper functions in isolation. The integration + * tests that exercise the full HTTP layer (ETag on GET /api/apis, 304 response, + * body absent on 304, etc.) live in src/routes/apis.etag.test.ts. + */ + +import { computeStrongETag, parseIfNoneMatch, isETagMatch } from './etagCache.js'; + +// ──────────────────────────────────────────────────────────────────────────── +// computeStrongETag +// ──────────────────────────────────────────────────────────────────────────── + +describe('computeStrongETag', () => { + it('returns a quoted string', () => { + const tag = computeStrongETag({ data: [], meta: {} }); + expect(tag).toMatch(/^"[0-9a-f]+"$/); + }); + + it('returns the same ETag for identical payloads', () => { + const body = { data: [{ id: 1, name: 'Test' }], meta: { total: 1 } }; + expect(computeStrongETag(body)).toBe(computeStrongETag(body)); + }); + + it('returns different ETags for different payloads', () => { + const a = computeStrongETag({ data: [{ id: 1 }] }); + const b = computeStrongETag({ data: [{ id: 2 }] }); + expect(a).not.toBe(b); + }); + + it('is sensitive to field order in the JSON representation', () => { + // JSON.stringify preserves insertion order; objects with different key + // orders produce different JSON strings and thus different ETags. + const a = computeStrongETag({ a: 1, b: 2 }); + const b = computeStrongETag({ b: 2, a: 1 }); + // These MAY differ because insertion order differs. + // We just verify both are valid quoted strings. + expect(a).toMatch(/^"[0-9a-f]+"$/); + expect(b).toMatch(/^"[0-9a-f]+"$/); + }); + + it('produces a 34-character string (32 hex chars + 2 quotes)', () => { + const tag = computeStrongETag({ x: 1 }); + expect(tag).toHaveLength(34); // '"' + 32 hex + '"' + }); + + it('handles an empty object', () => { + const tag = computeStrongETag({}); + expect(tag).toMatch(/^"[0-9a-f]{32}"$/); + }); + + it('handles null', () => { + const tag = computeStrongETag(null); + expect(tag).toMatch(/^"[0-9a-f]{32}"$/); + }); + + it('handles arrays', () => { + const tag = computeStrongETag([1, 2, 3]); + expect(tag).toMatch(/^"[0-9a-f]{32}"$/); + }); + + it('handles deeply nested structures', () => { + const tag = computeStrongETag({ a: { b: { c: [1, 2, { d: 'e' }] } } }); + expect(tag).toMatch(/^"[0-9a-f]{32}"$/); + }); +}); + +// ──────────────────────────────────────────────────────────────────────────── +// parseIfNoneMatch +// ──────────────────────────────────────────────────────────────────────────── + +describe('parseIfNoneMatch', () => { + it('returns an empty set for undefined', () => { + expect(parseIfNoneMatch(undefined).size).toBe(0); + }); + + it('returns an empty set for empty string', () => { + expect(parseIfNoneMatch('').size).toBe(0); + }); + + it('returns a Set containing "*" for the wildcard', () => { + const tags = parseIfNoneMatch('*'); + expect(tags.has('*')).toBe(true); + expect(tags.size).toBe(1); + }); + + it('parses a single quoted ETag', () => { + const tags = parseIfNoneMatch('"abc123"'); + expect(tags.has('abc123')).toBe(true); + expect(tags.size).toBe(1); + }); + + it('parses multiple comma-separated ETags', () => { + const tags = parseIfNoneMatch('"aaa", "bbb", "ccc"'); + expect(tags.has('aaa')).toBe(true); + expect(tags.has('bbb')).toBe(true); + expect(tags.has('ccc')).toBe(true); + expect(tags.size).toBe(3); + }); + + it('strips the W/ prefix from weak ETags', () => { + const tags = parseIfNoneMatch('W/"weaketag"'); + expect(tags.has('weaketag')).toBe(true); + }); + + it('strips W/ prefix (case-insensitive)', () => { + const tags = parseIfNoneMatch('w/"weaketag"'); + expect(tags.has('weaketag')).toBe(true); + }); + + it('handles a mix of strong and weak ETags', () => { + const tags = parseIfNoneMatch('"strong", W/"weak"'); + expect(tags.has('strong')).toBe(true); + expect(tags.has('weak')).toBe(true); + }); + + it('ignores empty segments from trailing commas', () => { + const tags = parseIfNoneMatch('"abc",'); + expect(tags.has('abc')).toBe(true); + expect(tags.size).toBe(1); + }); + + it('handles extra whitespace around tags', () => { + const tags = parseIfNoneMatch(' "abc" , "def" '); + expect(tags.has('abc')).toBe(true); + expect(tags.has('def')).toBe(true); + }); + + it('returns an empty set for a completely malformed header (no quotes)', () => { + // "notquoted" without surrounding double-quotes is still handled: + // the regex strips quotes only if present, leaving the bare value. + const tags = parseIfNoneMatch('notquoted'); + // The value is returned as-is after stripping (no quotes to strip). + expect(tags.has('notquoted')).toBe(true); + }); +}); + +// ──────────────────────────────────────────────────────────────────────────── +// isETagMatch +// ──────────────────────────────────────────────────────────────────────────── + +describe('isETagMatch', () => { + const currentETag = '"abc123def456abc123def456abc12345"'; // 32-char hex digest + + it('returns false when If-None-Match header is absent', () => { + expect(isETagMatch(currentETag, undefined)).toBe(false); + }); + + it('returns false when If-None-Match is empty string', () => { + expect(isETagMatch(currentETag, '')).toBe(false); + }); + + it('returns true when If-None-Match contains the matching ETag', () => { + expect(isETagMatch(currentETag, '"abc123def456abc123def456abc12345"')).toBe(true); + }); + + it('returns true for wildcard "*"', () => { + expect(isETagMatch(currentETag, '*')).toBe(true); + }); + + it('returns false when If-None-Match contains a different ETag', () => { + expect(isETagMatch(currentETag, '"different000000000000000000000000"')).toBe(false); + }); + + it('returns true when the matching ETag is one of many in a list', () => { + const header = '"other0000000000000000000000000000", "abc123def456abc123def456abc12345", "another00000000000000000000000000"'; + expect(isETagMatch(currentETag, header)).toBe(true); + }); + + it('returns true when the matching ETag is supplied as a weak ETag in If-None-Match', () => { + // Per RFC 9110 §13.1.2, If-None-Match uses weak comparison — a weak client + // tag matching the strong server tag is still a match. + expect(isETagMatch(currentETag, 'W/"abc123def456abc123def456abc12345"')).toBe(true); + }); + + it('returns false when none of the listed ETags match', () => { + const header = '"aaa00000000000000000000000000000", "bbb00000000000000000000000000000"'; + expect(isETagMatch(currentETag, header)).toBe(false); + }); +}); diff --git a/src/middleware/etagCache.ts b/src/middleware/etagCache.ts new file mode 100644 index 00000000..891a2ab5 --- /dev/null +++ b/src/middleware/etagCache.ts @@ -0,0 +1,150 @@ +/** + * @file src/middleware/etagCache.ts + * @description Strong ETag / 304 Not Modified support for GET /api/apis routes. + * + * ── Why strong, not weak? ──────────────────────────────────────────────────── + * Express's default ETag mode generates *weak* ETags ("W/...") using a + * combination of the response body length and a timestamp from res.getHeader. + * Weak ETags signal that two representations are semantically equivalent but + * not byte-for-byte identical — they are unsuitable for byte-range requests. + * More importantly, weak ETags are computed from length + time, so two + * responses with identical bodies but different timestamps get different ETags, + * defeating caching for repeat reads within the same second. + * + * Strong ETags are derived from a SHA-256 digest of the actual serialized body, + * so they change if and only if the response content changes. This is the + * correct semantics for an API that returns a deterministic JSON response. + * + * Express's built-in ETag generation is NOT disabled globally — doing so would + * affect all other routes. Instead, this middleware sets the ETag header + * explicitly before `res.json()` sends the response, which causes Express to + * skip its own ETag generation for that response (Express only generates an + * ETag if the ETag header is not already set by the time the response is + * finalized). + * + * ── Approach: compute-then-compare ────────────────────────────────────────── + * The route already uses an in-process ListingsCache that skips the DB on + * cache hits, so the most expensive work (DB query + pagination formatting) is + * already avoided on cache hits. The ETag is computed from the serialized JSON + * body after the response object is ready but before it is sent over the wire. + * + * Skipping serialization on 304 responses would require restructuring the route + * handler to build the response object in a separate step, which would touch + * unrelated logic and increase complexity. The current approach: + * 1. Build the response object normally (cache hit avoids the DB). + * 2. Serialize it to JSON and compute a SHA-256 digest. + * 3. If the digest matches If-None-Match, respond 304 with no body. + * 4. Otherwise, set ETag header and send the JSON body. + * + * The cost of JSON.stringify + SHA-256 on a typical paginated API listing + * (~20 items) is sub-millisecond and is negligible compared to any network RTT. + * + * ── If-None-Match parsing ──────────────────────────────────────────────────── + * Per RFC 9110 §13.1.2, If-None-Match may contain: + * - A comma-separated list of ETags, each optionally quoted: "abc", "def" + * - A wildcard: * + * - Weak ETags prefixed with W/: W/"abc" + * + * This implementation normalises incoming ETags to their unquoted digest value + * before comparison. Weak ETags from If-None-Match are accepted and compared + * against the strong ETag value (per RFC 9110 §13.1.2 the weak comparison + * function is used for If-None-Match). + * + * ── Security ───────────────────────────────────────────────────────────────── + * SHA-256 digests do not leak information about the content structure beyond + * what the response body itself would reveal. The ETag is truncated to the + * first 32 hex characters (128 bits) — sufficient for collision resistance in + * this context while keeping headers compact. + */ + +import { createHash } from 'node:crypto'; + +/** + * Compute a strong ETag value for an arbitrary serialisable payload. + * + * The value is the first 32 hex chars of the SHA-256 digest of the + * JSON-serialized body, wrapped in double-quotes as required by RFC 9110. + * + * @example + * computeStrongETag({ data: [], meta: { total: 0, limit: 20, offset: 0 } }) + * // => '"a7ffc6f8bf1ed76651c14756a061d662"' (illustrative, not real) + */ +export function computeStrongETag(body: unknown): string { + const json = JSON.stringify(body); + const digest = createHash('sha256').update(json, 'utf8').digest('hex').slice(0, 32); + return `"${digest}"`; +} + +/** + * Parse an If-None-Match header value into a set of normalised ETag strings. + * + * Handles: + * - Wildcard: "*" + * - List: `"abc", "def"`, `W/"abc", "def"` + * - Malformed: returns an empty set so the caller falls back to a 200 + * + * Each tag in the returned set is the bare digest string without quotes or + * the W/ prefix, allowing a single `Set.has(digest)` lookup. + * + * @returns A Set of normalised tag strings, or the singleton Set(['*']) for + * the wildcard case. + */ +export function parseIfNoneMatch(headerValue: string | undefined): Set { + if (!headerValue) { + return new Set(); + } + + const trimmed = headerValue.trim(); + + // Wildcard — matches any ETag + if (trimmed === '*') { + return new Set(['*']); + } + + const tags = new Set(); + + // Split on commas, then strip quotes and the optional W/ prefix + for (const part of trimmed.split(',')) { + const raw = part.trim(); + if (!raw) continue; + + // Strip optional weak prefix: W/"..." or w/"..." + const withoutWeak = raw.replace(/^W\//i, ''); + + // Strip surrounding double-quotes + const unquoted = withoutWeak.replace(/^"(.*)"$/, '$1'); + + if (unquoted) { + tags.add(unquoted); + } + } + + return tags; +} + +/** + * Determine whether a computed ETag matches the client's If-None-Match header. + * + * `currentETag` must be the quoted form returned by `computeStrongETag`, + * e.g. `'"abc123"'`. + * + * Matching rules (RFC 9110 §13.1.2 weak comparison for If-None-Match): + * - Wildcard "*" always matches. + * - Otherwise the bare digest of `currentETag` is compared against every + * tag extracted from `ifNoneMatchHeader`. + */ +export function isETagMatch(currentETag: string, ifNoneMatchHeader: string | undefined): boolean { + const tags = parseIfNoneMatch(ifNoneMatchHeader); + + if (tags.size === 0) { + return false; + } + + if (tags.has('*')) { + return true; + } + + // Strip quotes from the current strong ETag to get the bare digest + const currentDigest = currentETag.replace(/^"(.*)"$/, '$1'); + return tags.has(currentDigest); +} diff --git a/src/routes/apis.etag.test.ts b/src/routes/apis.etag.test.ts new file mode 100644 index 00000000..5c5badb3 --- /dev/null +++ b/src/routes/apis.etag.test.ts @@ -0,0 +1,382 @@ +/** + * @file src/routes/apis.etag.test.ts + * @description Integration tests for ETag / 304 Not Modified support on + * GET /api/apis and GET /api/apis/:id (issue #866). + * + * Style deliberately mirrors src/routes/apis.test.ts: same mocking approach, + * same InMemoryApiRepository, same express + supertest setup. + */ + +jest.mock('better-sqlite3', () => { + return class MockDatabase { + prepare() { return { get: () => null }; } + exec() { return undefined; } + close() { return undefined; } + }; +}); + +import express from 'express'; +import request from 'supertest'; +import { errorHandler } from '../middleware/errorHandler.js'; +import { InMemoryApiRepository } from '../repositories/apiRepository.js'; +import { createApisRouter } from './apis.js'; +import { ListingsCache } from '../lib/listingsCache.js'; + +// ── Shared test data ───────────────────────────────────────────────────────── + +const ACTIVE_API = { + id: 1, + name: 'Weather API', + description: 'Provides weather data', + base_url: 'https://api.weather.test', + logo_url: null, + category: 'weather', + status: 'active' as const, + developer: { + name: 'Acme Corp', + website: 'https://acme.test', + description: 'Leading data provider', + }, +}; + +const ENDPOINTS_MAP = new Map([ + [ + 1, + [ + { + path: '/current', + method: 'GET' as const, + price_per_call_usdc: '0.01', + description: 'Current weather', + }, + ], + ], +]); + +// ── App builder ────────────────────────────────────────────────────────────── + +function buildApp(overrideApis = [ACTIVE_API], endpointsMap = ENDPOINTS_MAP) { + const repo = new InMemoryApiRepository(overrideApis, endpointsMap); + // Fresh cache per test so tests are fully isolated + const cache = new ListingsCache({ ttlMs: 30_000 }); + + const app = express(); + app.use( + '/api/apis', + createApisRouter({ apiRepository: repo, cache }), + ); + app.use(errorHandler); + return { app, repo, cache }; +} + +// ──────────────────────────────────────────────────────────────────────────── +// GET /api/apis — ETag + 304 +// ──────────────────────────────────────────────────────────────────────────── + +describe('GET /api/apis — ETag caching', () => { + it('returns 200 with an ETag header on the first request', async () => { + const { app } = buildApp(); + const res = await request(app).get('/api/apis'); + + expect(res.status).toBe(200); + expect(res.headers['etag']).toBeDefined(); + // Must be a strong ETag (no W/ prefix) wrapped in double-quotes + expect(res.headers['etag']).toMatch(/^"[0-9a-f]+"$/); + }); + + it('returns 304 with no body when If-None-Match matches the current ETag', async () => { + const { app } = buildApp(); + + // First request — get the ETag + const first = await request(app).get('/api/apis'); + expect(first.status).toBe(200); + const etag = first.headers['etag']; + expect(etag).toBeDefined(); + + // Second request with matching If-None-Match + const second = await request(app) + .get('/api/apis') + .set('If-None-Match', etag); + + expect(second.status).toBe(304); + // 304 must have no body (text representation of body should be empty) + expect(second.text).toBe(''); + }); + + it('the 304 response still carries the ETag header', async () => { + const { app } = buildApp(); + + const first = await request(app).get('/api/apis'); + const etag = first.headers['etag']; + + const second = await request(app) + .get('/api/apis') + .set('If-None-Match', etag); + + expect(second.status).toBe(304); + expect(second.headers['etag']).toBe(etag); + }); + + it('returns 200 with the current body when If-None-Match is stale', async () => { + const { app } = buildApp(); + + const staleETag = '"0000000000000000000000000000abcd"'; + const res = await request(app) + .get('/api/apis') + .set('If-None-Match', staleETag); + + expect(res.status).toBe(200); + expect(res.body.data).toBeDefined(); + // The response must carry the current (new) ETag, not the stale one + expect(res.headers['etag']).not.toBe(staleETag); + expect(res.headers['etag']).toMatch(/^"[0-9a-f]+"$/); + }); + + it('ETag changes when the underlying data changes', async () => { + // Build two separate apps — one with one API, one with two APIs — + // both using an empty cache to ensure DB reads happen. + const cacheA = new ListingsCache({ ttlMs: 30_000 }); + const repoA = new InMemoryApiRepository([ACTIVE_API], ENDPOINTS_MAP); + const appA = express(); + appA.use('/api/apis', createApisRouter({ apiRepository: repoA, cache: cacheA })); + appA.use(errorHandler); + + const cacheB = new ListingsCache({ ttlMs: 30_000 }); + const repoB = new InMemoryApiRepository( + [ + ACTIVE_API, + { + id: 2, + name: 'Translate API', + description: null, + base_url: 'https://api.translate.test', + logo_url: null, + category: 'language', + status: 'active' as const, + developer: { name: 'New Dev', website: null, description: null }, + }, + ], + ENDPOINTS_MAP, + ); + const appB = express(); + appB.use('/api/apis', createApisRouter({ apiRepository: repoB, cache: cacheB })); + appB.use(errorHandler); + + const first = await request(appA).get('/api/apis'); + const etagBefore = first.headers['etag']; + + const second = await request(appB).get('/api/apis'); + const etagAfter = second.headers['etag']; + + expect(etagBefore).toBeDefined(); + expect(etagAfter).toBeDefined(); + expect(etagBefore).not.toBe(etagAfter); + }); + + it('handles a malformed If-None-Match header gracefully (returns 200, not 500)', async () => { + const { app } = buildApp(); + + // A header value that is not a valid quoted ETag + const res = await request(app) + .get('/api/apis') + .set('If-None-Match', '!!!not-valid!!!'); + + // Must fall back to a normal 200 response — never a 500 + expect(res.status).toBe(200); + expect(res.body.data).toBeDefined(); + }); + + it('returns 304 when If-None-Match contains the matching ETag in a list', async () => { + const { app } = buildApp(); + + const first = await request(app).get('/api/apis'); + const etag = first.headers['etag']; + + // Client sends a list of ETags including the current one + const second = await request(app) + .get('/api/apis') + .set('If-None-Match', `"stale0000000000000000000000000000", ${etag}`); + + expect(second.status).toBe(304); + }); + + it('returns 304 for wildcard If-None-Match: *', async () => { + const { app } = buildApp(); + + const res = await request(app) + .get('/api/apis') + .set('If-None-Match', '*'); + + expect(res.status).toBe(304); + }); + + it('returns 304 when If-None-Match contains the matching weak ETag', async () => { + const { app } = buildApp(); + + const first = await request(app).get('/api/apis'); + const etag = first.headers['etag']; // e.g. "abc..." + // Client sends a weak version of the same ETag + const weakETag = `W/${etag}`; + + const second = await request(app) + .get('/api/apis') + .set('If-None-Match', weakETag); + + // Weak comparison is used for If-None-Match — must still match + expect(second.status).toBe(304); + }); + + it('ETag is identical for two requests with the same data (deterministic)', async () => { + const { app } = buildApp(); + + const first = await request(app).get('/api/apis'); + const second = await request(app).get('/api/apis'); + + expect(first.headers['etag']).toBe(second.headers['etag']); + }); + + it('ETag is different for different pagination params', async () => { + const repo = new InMemoryApiRepository( + [ + ACTIVE_API, + { + id: 3, + name: 'Second API', + description: null, + base_url: 'https://api2.test', + logo_url: null, + category: 'other', + status: 'active' as const, + developer: { name: 'Dev B', website: null, description: null }, + }, + ], + ENDPOINTS_MAP, + ); + const cache = new ListingsCache({ ttlMs: 30_000 }); + const app = express(); + app.use('/api/apis', createApisRouter({ apiRepository: repo, cache })); + app.use(errorHandler); + + const page1 = await request(app).get('/api/apis?limit=1&offset=0'); + const page2 = await request(app).get('/api/apis?limit=1&offset=1'); + + expect(page1.status).toBe(200); + expect(page2.status).toBe(200); + expect(page1.headers['etag']).not.toBe(page2.headers['etag']); + }); + + it('cache-hit path still returns 304 on matching If-None-Match', async () => { + const { app } = buildApp(); + + // First request — populates the in-process ListingsCache + const first = await request(app).get('/api/apis'); + const etag = first.headers['etag']; + + // Second request hits the ListingsCache (no DB) and still evaluates ETag + const second = await request(app) + .get('/api/apis') + .set('If-None-Match', etag); + + expect(second.status).toBe(304); + expect(second.text).toBe(''); + }); +}); + +// ──────────────────────────────────────────────────────────────────────────── +// GET /api/apis/:id — ETag + 304 +// ──────────────────────────────────────────────────────────────────────────── + +describe('GET /api/apis/:id — ETag caching', () => { + it('returns 200 with a strong ETag header', async () => { + const { app } = buildApp(); + const res = await request(app).get('/api/apis/1'); + + expect(res.status).toBe(200); + expect(res.headers['etag']).toMatch(/^"[0-9a-f]+"$/); + }); + + it('returns 304 with no body when If-None-Match matches', async () => { + const { app } = buildApp(); + + const first = await request(app).get('/api/apis/1'); + const etag = first.headers['etag']; + + const second = await request(app) + .get('/api/apis/1') + .set('If-None-Match', etag); + + expect(second.status).toBe(304); + expect(second.text).toBe(''); + expect(second.headers['etag']).toBe(etag); + }); + + it('returns 200 when If-None-Match does not match', async () => { + const { app } = buildApp(); + + const res = await request(app) + .get('/api/apis/1') + .set('If-None-Match', '"stale0000000000000000000000000000"'); + + expect(res.status).toBe(200); + expect(res.body.id).toBe(1); + expect(res.headers['etag']).toMatch(/^"[0-9a-f]+"$/); + }); + + it('handles a malformed If-None-Match header gracefully (returns 200)', async () => { + const { app } = buildApp(); + + const res = await request(app) + .get('/api/apis/1') + .set('If-None-Match', '!!!bad!!!'); + + expect(res.status).toBe(200); + expect(res.body.id).toBe(1); + }); + + it('returns 304 for wildcard If-None-Match: *', async () => { + const { app } = buildApp(); + + const res = await request(app) + .get('/api/apis/1') + .set('If-None-Match', '*'); + + expect(res.status).toBe(304); + }); + + it('does not emit an ETag on 404 responses', async () => { + const { app } = buildApp(); + + const res = await request(app).get('/api/apis/9999'); + + expect(res.status).toBe(404); + // 404 responses come from the error handler and must not carry an ETag + expect(res.headers['etag']).toBeUndefined(); + }); + + it('ETag differs between different APIs', async () => { + const repo = new InMemoryApiRepository( + [ + ACTIVE_API, + { + id: 2, + name: 'Different API', + description: null, + base_url: 'https://different.test', + logo_url: null, + category: 'other', + status: 'active' as const, + developer: { name: 'Other Dev', website: null, description: null }, + }, + ], + new Map([[1, []], [2, []]]), + ); + const app = express(); + app.use('/api/apis', createApisRouter({ apiRepository: repo })); + app.use(errorHandler); + + const api1 = await request(app).get('/api/apis/1'); + const api2 = await request(app).get('/api/apis/2'); + + expect(api1.headers['etag']).not.toBe(api2.headers['etag']); + }); +}); diff --git a/src/routes/apis.ts b/src/routes/apis.ts index 2bd5208c..5afd2eb9 100644 --- a/src/routes/apis.ts +++ b/src/routes/apis.ts @@ -5,6 +5,7 @@ import { buildCacheKey, listingsCache, type ListingsCache } from '../lib/listing import { recordCacheHit, recordCacheMiss } from '../metrics.js'; import { requireAuth, type AuthenticatedLocals } from '../middleware/requireAuth.js'; import { bodyValidator } from '../middleware/validate.js'; +import { computeStrongETag, isETagMatch } from '../middleware/etagCache.js'; import { defaultApiRepository, type ApiRepository, @@ -41,6 +42,18 @@ export function createApisRouter(deps: ApisRouterDeps = {}): Router { if (cached !== undefined) { // Serve from cache and record a hit metric. recordCacheHit(); + + // ── Strong ETag / 304 (cache-hit path) ─────────────────────────── + // The response body is already available in `cached`, so we can + // compute the ETag without touching the DB. This is the fast path: + // both the DB and the full HTTP body are skipped on a 304. + const etag = computeStrongETag(cached); + if (isETagMatch(etag, req.headers['if-none-match'])) { + res.status(304).set('ETag', etag).end(); + return; + } + + res.set('ETag', etag); res.json(cached); return; } @@ -54,6 +67,17 @@ export function createApisRouter(deps: ApisRouterDeps = {}): Router { // the TTL window skip the DB entirely. cache.set(cacheKey, response); + // ── Strong ETag / 304 (cache-miss path) ────────────────────────────── + // Approach: compute-then-compare. The response is built before the ETag + // check because the cache miss already required the DB read. The cost + // of JSON serialisation + SHA-256 is sub-millisecond and negligible. + const etag = computeStrongETag(response); + if (isETagMatch(etag, req.headers['if-none-match'])) { + res.status(304).set('ETag', etag).end(); + return; + } + + res.set('ETag', etag); res.json(response); } catch (error) { next(error); @@ -77,7 +101,7 @@ export function createApisRouter(deps: ApisRouterDeps = {}): Router { const endpoints = await apiRepository.getEndpoints(id); - res.json({ + const responseBody = { id: api.id, name: api.name, description: api.description, @@ -87,7 +111,21 @@ export function createApisRouter(deps: ApisRouterDeps = {}): Router { status: api.status, developer: api.developer, endpoints, - }); + }; + + // ── Strong ETag / 304 ───────────────────────────────────────────────── + // Approach: compute-then-compare. The DB reads for findById and + // getEndpoints are required to build the response, so they cannot be + // skipped. The ETag is computed from the assembled response object and + // the 304 shortcut avoids sending the JSON body over the wire. + const etag = computeStrongETag(responseBody); + if (isETagMatch(etag, req.headers['if-none-match'])) { + res.status(304).set('ETag', etag).end(); + return; + } + + res.set('ETag', etag); + res.json(responseBody); } catch (error) { next(error); } From 8b96b7c77ae5bde7e8479930713e4e950ec2cfca Mon Sep 17 00:00:00 2001 From: Haneefah Sanni <121475253+MissHarah@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:44:53 +0100 Subject: [PATCH 2/4] feat: add Zod-validated /api/tenants schema with structured 400 errors and OpenAPI docs (#1134) - Validators (createTenantSchema, updateTenantSchema, tenantParamsSchema) with .strict() mode, field trimming, slug lowercasing, and metadata key limits were already in place in src/validators/tenants.ts - Routes (POST, PATCH, GET) were already wired to bodyValidator() and validate({ params, body }) in src/routes/tenants.ts producing structured ValidationError 400 envelopes with per-field details arrays - Add /api/tenants and /api/tenants/{tenantId} to src/openapi.yaml with: - Typed component schemas: TenantPlan, TenantMetadata, TenantRecord, TenantCreateRequest, TenantUpdateRequest, TenantResponse, TenantListResponse - Full request/response examples for all operations (create, update, list) - Structured 400 error examples: missingName, invalidEmail, unknownKey, invalidPlan, invalidParamAndEmptyBody with VALIDATION_ERROR code and per-field details array - ETag / 304 conditional-GET documentation for GET /api/tenants - 401 examples for all three operations - Add src/routes/tenants.openapi.test.ts covering: - OpenAPI YAML contract assertions (path presence, schema names, example keys) - POST integration: 400 per-field details, UNRECOGNIZED_KEYS, enum rejection, 201 success, whitespace trim, unauthenticated 401 - PATCH integration: combined param+body error collection, strict-schema unknown-key rejection, 200 success, 401 - GET integration: 200 list envelope, strong ETag header, 304 on match, 401 Closes # --- src/openapi.yaml | 499 +++++++++++++++++++++++++++++ src/routes/tenants.openapi.test.ts | 450 ++++++++++++++++++++++++++ 2 files changed, 949 insertions(+) create mode 100644 src/routes/tenants.openapi.test.ts diff --git a/src/openapi.yaml b/src/openapi.yaml index f8227595..9c12320a 100644 --- a/src/openapi.yaml +++ b/src/openapi.yaml @@ -830,6 +830,367 @@ paths: message: Error definition 999 not found requestId: req-errors-delete-404 timestamp: "2026-07-27T09:15:00.000Z" + /api/tenants: + get: + summary: List tenants + description: > + Returns all tenants visible to the authenticated user. Responses are + ETag-stamped so clients can use conditional GET (If-None-Match) to avoid + redundant transfers. + security: + - bearerAuth: [] + parameters: + - name: x-user-id + in: header + required: true + description: Authenticated user identifier (set by auth middleware). + schema: + type: string + examples: + developer: + summary: Authenticated developer + value: dev-1 + - name: If-None-Match + in: header + required: false + description: > + Strong ETag from a prior response. Returns 304 if the tenant list + has not changed. + schema: + type: string + examples: + etag: + summary: Strong ETag from prior response + value: '"a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3f4a5b6c7d8e9f0a1b2"' + responses: + "200": + description: Tenant list retrieved successfully. + headers: + ETag: + description: Strong ETag of the current tenant list payload. + schema: + type: string + content: + application/json: + schema: + $ref: "#/components/schemas/TenantListResponse" + examples: + withTenants: + summary: Two tenants returned + value: + success: true + data: + - id: ten_a1b2c3d4-e5f6-7890-abcd-ef1234567890 + name: GrantFox Ops + slug: grantfox-ops + contactEmail: ops@grantfox.test + plan: growth + metadata: + campaign: fwc26 + createdBy: dev-1 + createdAt: "2026-07-28T00:00:00.000Z" + updatedAt: "2026-07-28T00:00:00.000Z" + - id: ten_b2c3d4e5-f6a7-8901-bcde-f12345678901 + name: GrantFox Stadium + slug: grantfox-stadium + plan: enterprise + createdBy: dev-1 + createdAt: "2026-07-28T00:00:00.000Z" + updatedAt: "2026-07-28T00:00:00.000Z" + requestId: req-tenants-list-1 + timestamp: "2026-07-28T10:00:00.000Z" + empty: + summary: No tenants exist yet + value: + success: true + data: [] + requestId: req-tenants-list-2 + timestamp: "2026-07-28T10:01:00.000Z" + "304": + description: > + Tenant list has not changed since the ETag supplied in + If-None-Match. + "401": + description: Authentication is required. + content: + application/json: + schema: + $ref: "#/components/schemas/StandardErrorEnvelope" + examples: + unauthorized: + summary: Missing or invalid authentication + value: + success: false + error: + code: UNAUTHORIZED + message: Unauthorized + requestId: req-tenants-list-401 + timestamp: "2026-07-28T10:02:00.000Z" + post: + summary: Create a tenant + description: > + Creates a new tenant. The request body is Zod-validated at the route + boundary; unrecognised keys are rejected with a structured 400 error + envelope so clients receive per-field details without any internal + information leaking. + security: + - bearerAuth: [] + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/TenantCreateRequest" + examples: + createFull: + summary: Full tenant payload with optional fields + value: + name: GrantFox Ops + slug: grantfox-ops + contactEmail: ops@grantfox.test + plan: growth + metadata: + campaign: fwc26 + priority: 1 + active: true + createMinimal: + summary: Minimal tenant — only name is required + value: + name: GrantFox Stadium + responses: + "201": + description: Tenant created successfully. + content: + application/json: + schema: + $ref: "#/components/schemas/TenantResponse" + examples: + created: + summary: Newly created tenant + value: + success: true + data: + id: ten_a1b2c3d4-e5f6-7890-abcd-ef1234567890 + name: GrantFox Ops + slug: grantfox-ops + contactEmail: ops@grantfox.test + plan: growth + metadata: + campaign: fwc26 + priority: 1 + active: true + createdBy: dev-1 + createdAt: "2026-07-28T10:00:00.000Z" + updatedAt: "2026-07-28T10:00:00.000Z" + requestId: req-tenants-create-1 + timestamp: "2026-07-28T10:00:00.000Z" + "400": + description: > + Request body failed Zod validation. The error envelope includes a + per-field `details` array so clients can surface precise messages + without any server internals leaking. + content: + application/json: + schema: + $ref: "#/components/schemas/StandardErrorEnvelope" + examples: + missingName: + summary: Required field missing + value: + success: false + error: + code: VALIDATION_ERROR + message: Request validation failed + details: + - field: body.name + message: name is required + code: INVALID_TYPE + requestId: req-tenants-create-400-name + timestamp: "2026-07-28T10:01:00.000Z" + invalidEmail: + summary: Invalid contactEmail format + value: + success: false + error: + code: VALIDATION_ERROR + message: Request validation failed + details: + - field: body.contactEmail + message: contactEmail must be a valid email address + code: INVALID_STRING + requestId: req-tenants-create-400-email + timestamp: "2026-07-28T10:02:00.000Z" + unknownKey: + summary: Unrecognised key rejected by strict schema + value: + success: false + error: + code: VALIDATION_ERROR + message: Request validation failed + details: + - field: body + message: "Unrecognized key(s) in object: 'unsafeRole'" + code: UNRECOGNIZED_KEYS + requestId: req-tenants-create-400-unknown + timestamp: "2026-07-28T10:03:00.000Z" + invalidPlan: + summary: Plan value outside the allowed enum + value: + success: false + error: + code: VALIDATION_ERROR + message: Request validation failed + details: + - field: body.plan + message: "Invalid option: expected one of 'starter' | 'growth' | 'enterprise'" + code: INVALID_ENUM_VALUE + requestId: req-tenants-create-400-plan + timestamp: "2026-07-28T10:04:00.000Z" + "401": + description: Authentication is required. + content: + application/json: + schema: + $ref: "#/components/schemas/StandardErrorEnvelope" + examples: + unauthorized: + summary: Missing or invalid authentication + value: + success: false + error: + code: UNAUTHORIZED + message: Unauthorized + requestId: req-tenants-create-401 + timestamp: "2026-07-28T10:05:00.000Z" + /api/tenants/{tenantId}: + patch: + summary: Update a tenant + description: > + Partially updates an existing tenant. Both the route parameter and the + request body are Zod-validated; all validation errors are collected in a + single pass and returned together in the structured 400 envelope so the + client can fix all issues in one round-trip. + security: + - bearerAuth: [] + parameters: + - name: tenantId + in: path + required: true + description: > + Tenant identifier — 3–64 characters, letters/numbers/underscores/hyphens. + schema: + type: string + pattern: "^[A-Za-z0-9][A-Za-z0-9_-]{2,63}$" + examples: + valid: + summary: Valid tenant ID + value: tenant_123 + tooShort: + summary: ID too short — triggers 400 + value: no + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/TenantUpdateRequest" + examples: + updatePlan: + summary: Upgrade the plan + value: + plan: enterprise + updateContactEmail: + summary: Update contact email only + value: + contactEmail: stadium@grantfox.test + updateMultiple: + summary: Update name, plan, and metadata together + value: + name: GrantFox Stadium Ops + plan: enterprise + metadata: + campaign: fwc26-final + responses: + "200": + description: Tenant updated successfully. + content: + application/json: + schema: + $ref: "#/components/schemas/TenantResponse" + examples: + updated: + summary: Updated tenant record + value: + success: true + data: + id: tenant_123 + name: GrantFox Stadium Ops + slug: grantfox-stadium + contactEmail: stadium@grantfox.test + plan: enterprise + metadata: + campaign: fwc26-final + createdBy: dev-1 + createdAt: "2026-07-28T00:00:00.000Z" + updatedAt: "2026-07-28T11:00:00.000Z" + requestId: req-tenants-update-1 + timestamp: "2026-07-28T11:00:00.000Z" + "400": + description: > + Route parameter or body failed Zod validation. All errors are + collected in one pass and returned in the `details` array. + content: + application/json: + schema: + $ref: "#/components/schemas/StandardErrorEnvelope" + examples: + invalidParamAndEmptyBody: + summary: Invalid tenantId param and empty body collected together + value: + success: false + error: + code: VALIDATION_ERROR + message: Request validation failed + details: + - field: body + message: At least one tenant field must be provided + code: CUSTOM + - field: params.tenantId + message: >- + tenantId must be 3-64 characters using letters, + numbers, underscores, or hyphens + code: INVALID_STRING + requestId: req-tenants-update-400 + timestamp: "2026-07-28T11:01:00.000Z" + unknownKey: + summary: Unrecognised key in update body + value: + success: false + error: + code: VALIDATION_ERROR + message: Request validation failed + details: + - field: body + message: "Unrecognized key(s) in object: 'slug'" + code: UNRECOGNIZED_KEYS + requestId: req-tenants-update-400-key + timestamp: "2026-07-28T11:02:00.000Z" + "401": + description: Authentication is required. + content: + application/json: + schema: + $ref: "#/components/schemas/StandardErrorEnvelope" + examples: + unauthorized: + summary: Missing or invalid authentication + value: + success: false + error: + code: UNAUTHORIZED + message: Unauthorized + requestId: req-tenants-update-401 + timestamp: "2026-07-28T11:03:00.000Z" /api/webhooks: post: summary: Register a webhook @@ -1397,6 +1758,144 @@ components: description: type: [string, "null"] maxLength: 1000 + # --------------------------------------------------------------------------- + # Tenant schemas + # --------------------------------------------------------------------------- + TenantPlan: + type: string + enum: [starter, growth, enterprise] + description: Billing plan for the tenant. + TenantMetadata: + type: object + description: > + Arbitrary key/value pairs attached to the tenant. Keys must be 1–64 + characters; values may be strings (≤ 256 chars), finite numbers, or + booleans. Maximum 20 keys. + maxProperties: 20 + additionalProperties: + oneOf: + - type: string + maxLength: 256 + - type: number + - type: boolean + TenantRecord: + type: object + required: [id, name, slug, plan, createdBy, createdAt, updatedAt] + properties: + id: + type: string + description: Tenant identifier prefixed with `ten_`. + example: ten_a1b2c3d4-e5f6-7890-abcd-ef1234567890 + name: + type: string + minLength: 1 + maxLength: 120 + description: Human-readable tenant name (leading/trailing whitespace is trimmed). + slug: + type: string + pattern: "^[a-z0-9](?:[a-z0-9-]{1,61}[a-z0-9])?$" + description: > + URL-safe lowercase identifier derived from the name when omitted. + 3–63 characters; letters, numbers, and internal hyphens only. + contactEmail: + type: string + format: email + maxLength: 254 + description: Optional primary contact address for the tenant. + plan: + $ref: "#/components/schemas/TenantPlan" + metadata: + $ref: "#/components/schemas/TenantMetadata" + createdBy: + type: string + description: ID of the user who created the tenant. + createdAt: + type: string + format: date-time + updatedAt: + type: string + format: date-time + TenantResponse: + type: object + required: [success, data, requestId, timestamp] + description: Success envelope wrapping a single TenantRecord. + properties: + success: + type: boolean + enum: [true] + data: + $ref: "#/components/schemas/TenantRecord" + requestId: + type: string + timestamp: + type: string + format: date-time + TenantListResponse: + type: object + required: [success, data, requestId, timestamp] + description: Success envelope wrapping an array of TenantRecords. + properties: + success: + type: boolean + enum: [true] + data: + type: array + items: + $ref: "#/components/schemas/TenantRecord" + requestId: + type: string + timestamp: + type: string + format: date-time + TenantCreateRequest: + type: object + required: [name] + description: > + Zod-validated at the route boundary using `createTenantSchema`. The + schema is `.strict()` — any unrecognised key triggers a 400 with + code UNRECOGNIZED_KEYS in the details array. + properties: + name: + type: string + minLength: 1 + maxLength: 120 + description: Tenant name. Leading/trailing whitespace is trimmed by the validator. + slug: + type: string + pattern: "^[a-z0-9](?:[a-z0-9-]{1,61}[a-z0-9])?$" + description: > + Optional URL-safe slug. Converted to lowercase by the validator. + Auto-derived from `name` when omitted. + contactEmail: + type: string + format: email + maxLength: 254 + plan: + $ref: "#/components/schemas/TenantPlan" + metadata: + $ref: "#/components/schemas/TenantMetadata" + additionalProperties: false + TenantUpdateRequest: + type: object + description: > + Zod-validated at the route boundary using `updateTenantSchema`. The + schema is `.strict()` — unrecognised keys (e.g. `slug`) are rejected. + At least one field must be present; an empty body triggers a 400. + minProperties: 1 + properties: + name: + type: string + minLength: 1 + maxLength: 120 + contactEmail: + type: string + format: email + maxLength: 254 + plan: + $ref: "#/components/schemas/TenantPlan" + metadata: + $ref: "#/components/schemas/TenantMetadata" + additionalProperties: false # Distinct from the pre-existing flat ErrorResponse — this matches the # actual envelope produced by errorHandler.ts / buildErrorEnvelope(): # { success: false, error: { code, message, details? }, requestId, timestamp } diff --git a/src/routes/tenants.openapi.test.ts b/src/routes/tenants.openapi.test.ts new file mode 100644 index 00000000..d31c51bb --- /dev/null +++ b/src/routes/tenants.openapi.test.ts @@ -0,0 +1,450 @@ +/** + * Contract tests for src/openapi.yaml — /api/tenants surface. + * + * Validates that the examples added for GrantFox FWC26 cover: + * - POST /api/tenants (create with Zod validation + structured 400s) + * - GET /api/tenants (list with ETag support) + * - PATCH /api/tenants/{tenantId} (update with per-field + param errors) + * + * Follows the same pattern used by src/routes/spike.openapi.test.ts: + * read the YAML file as a string and assert the presence of key strings so + * the tests remain stable without a full YAML parse dependency. + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import express from 'express'; +import request from 'supertest'; +import { errorHandler } from '../middleware/errorHandler.js'; +import { requestIdMiddleware } from '../middleware/requestId.js'; +import { logger } from '../logger.js'; +import { createTenantsRouter, type TenantRecord, type TenantRepository } from './tenants.js'; +import type { CreateTenantInput, UpdateTenantInput } from '../validators/tenants.js'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const yamlPath = path.join(process.cwd(), 'src', 'openapi.yaml'); + +function readOpenApiYaml(): string { + return fs.readFileSync(yamlPath, 'utf8'); +} + +class MockTenantRepository implements TenantRepository { + list = jest.fn(async (): Promise => [ + { + id: 'ten_a1b2c3d4-e5f6-7890-abcd-ef1234567890', + name: 'GrantFox Ops', + slug: 'grantfox-ops', + contactEmail: 'ops@grantfox.test', + plan: 'growth', + metadata: { campaign: 'fwc26' }, + createdBy: 'dev-1', + createdAt: '2026-07-28T00:00:00.000Z', + updatedAt: '2026-07-28T00:00:00.000Z', + }, + ]); + + create = jest.fn(async (input: CreateTenantInput, actorId: string): Promise => ({ + id: 'ten_a1b2c3d4-e5f6-7890-abcd-ef1234567890', + name: input.name, + slug: input.slug ?? 'grantfox-ops', + contactEmail: input.contactEmail, + plan: input.plan, + metadata: input.metadata, + createdBy: actorId, + createdAt: '2026-07-28T10:00:00.000Z', + updatedAt: '2026-07-28T10:00:00.000Z', + })); + + update = jest.fn(async (tenantId: string, input: UpdateTenantInput, actorId: string): Promise => ({ + id: tenantId, + name: input.name ?? 'GrantFox Ops', + slug: 'grantfox-ops', + contactEmail: input.contactEmail, + plan: input.plan ?? 'starter', + metadata: input.metadata, + createdBy: actorId, + createdAt: '2026-07-28T00:00:00.000Z', + updatedAt: '2026-07-28T11:00:00.000Z', + })); +} + +function buildApp(repository = new MockTenantRepository()) { + const app = express(); + app.use(express.json()); + app.use(requestIdMiddleware); + app.use('/api/tenants', createTenantsRouter({ tenantRepository: repository })); + app.use(errorHandler); + return { app, repository }; +} + +// --------------------------------------------------------------------------- +// Group 1 — OpenAPI YAML contract (string-presence assertions) +// --------------------------------------------------------------------------- + +describe('src/openapi.yaml — /api/tenants surface', () => { + test('documents /api/tenants and /api/tenants/{tenantId} paths', () => { + const content = readOpenApiYaml(); + + expect(content).toContain('/api/tenants:'); + expect(content).toContain('/api/tenants/{tenantId}:'); + }); + + test('documents GET /api/tenants list and ETag examples', () => { + const content = readOpenApiYaml(); + + expect(content).toContain('List tenants'); + expect(content).toContain('withTenants:'); + expect(content).toContain('empty:'); + // ETag conditional-GET + expect(content).toContain('If-None-Match'); + expect(content).toContain('304'); + }); + + test('documents POST /api/tenants create request and success example', () => { + const content = readOpenApiYaml(); + + expect(content).toContain('Create a tenant'); + expect(content).toContain('createFull:'); + expect(content).toContain('createMinimal:'); + expect(content).toContain('created:'); + // Zod-validation callout in description + expect(content).toContain('Zod-validated'); + }); + + test('documents structured 400 validation-error examples for POST', () => { + const content = readOpenApiYaml(); + + expect(content).toContain('missingName:'); + expect(content).toContain('VALIDATION_ERROR'); + expect(content).toContain('name is required'); + expect(content).toContain('invalidEmail:'); + expect(content).toContain('contactEmail must be a valid email address'); + expect(content).toContain('unknownKey:'); + expect(content).toContain('UNRECOGNIZED_KEYS'); + expect(content).toContain('invalidPlan:'); + expect(content).toContain('INVALID_ENUM_VALUE'); + }); + + test('documents PATCH /api/tenants/{tenantId} update request and success example', () => { + const content = readOpenApiYaml(); + + expect(content).toContain('Update a tenant'); + expect(content).toContain('updatePlan:'); + expect(content).toContain('updateContactEmail:'); + expect(content).toContain('updateMultiple:'); + expect(content).toContain('updated:'); + }); + + test('documents combined param + body 400 example for PATCH', () => { + const content = readOpenApiYaml(); + + expect(content).toContain('invalidParamAndEmptyBody:'); + expect(content).toContain('At least one tenant field must be provided'); + expect(content).toContain('params.tenantId'); + expect(content).toContain('unknownKey:'); + }); + + test('documents 401 examples for all tenant operations', () => { + const content = readOpenApiYaml(); + + // Multiple 401 blocks — one per operation + const matches = [...content.matchAll(/code: UNAUTHORIZED/g)]; + // At minimum POST, GET, and PATCH each contribute one UNAUTHORIZED block + expect(matches.length).toBeGreaterThanOrEqual(3); + }); + + test('defines typed tenant schemas in components', () => { + const content = readOpenApiYaml(); + + expect(content).toContain('TenantRecord:'); + expect(content).toContain('TenantCreateRequest:'); + expect(content).toContain('TenantUpdateRequest:'); + expect(content).toContain('TenantResponse:'); + expect(content).toContain('TenantListResponse:'); + expect(content).toContain('TenantPlan:'); + expect(content).toContain('TenantMetadata:'); + // Plan enum values + expect(content).toContain('enum: [starter, growth, enterprise]'); + }); +}); + +// --------------------------------------------------------------------------- +// Group 2 — HTTP integration: Zod validation produces structured 400s +// --------------------------------------------------------------------------- + +describe('POST /api/tenants — Zod validation integration', () => { + let infoSpy: jest.SpyInstance; + + beforeEach(() => { + infoSpy = jest.spyOn(logger, 'info').mockImplementation(() => undefined); + }); + + afterEach(() => { + infoSpy.mockRestore(); + }); + + it('returns 400 with VALIDATION_ERROR and per-field details when name is missing', async () => { + const { app } = buildApp(); + + const res = await request(app) + .post('/api/tenants') + .set('x-user-id', 'dev-1') + .set('x-request-id', 'req-missing-name') + .send({ contactEmail: 'bad-email' }); + + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ + success: false, + error: { code: 'VALIDATION_ERROR', message: 'Request validation failed' }, + requestId: 'req-missing-name', + }); + expect(res.body.error.details).toEqual( + expect.arrayContaining([ + expect.objectContaining({ field: 'body.name' }), + expect.objectContaining({ field: 'body.contactEmail' }), + ]), + ); + }); + + it('returns 400 with UNRECOGNIZED_KEYS when unknown fields are sent', async () => { + const { app } = buildApp(); + + const res = await request(app) + .post('/api/tenants') + .set('x-user-id', 'dev-1') + .set('x-request-id', 'req-unknown-key') + .send({ name: 'GrantFox Ops', unsafeRole: 'admin' }); + + expect(res.status).toBe(400); + expect(res.body.error.details).toEqual( + expect.arrayContaining([ + expect.objectContaining({ field: 'body', code: 'UNRECOGNIZED_KEYS' }), + ]), + ); + }); + + it('returns 400 when plan is outside the allowed enum', async () => { + const { app } = buildApp(); + + const res = await request(app) + .post('/api/tenants') + .set('x-user-id', 'dev-1') + .set('x-request-id', 'req-bad-plan') + .send({ name: 'GrantFox Ops', plan: 'premium' }); + + expect(res.status).toBe(400); + expect(res.body.error.details).toEqual( + expect.arrayContaining([ + expect.objectContaining({ field: 'body.plan' }), + ]), + ); + }); + + it('returns 201 success envelope for a valid minimal create request', async () => { + const { app, repository } = buildApp(); + + const res = await request(app) + .post('/api/tenants') + .set('x-user-id', 'dev-1') + .set('x-request-id', 'req-create-ok') + .send({ name: 'GrantFox Stadium' }); + + expect(res.status).toBe(201); + expect(res.body).toMatchObject({ + success: true, + requestId: 'req-create-ok', + data: expect.objectContaining({ name: 'GrantFox Stadium' }), + }); + expect(repository.create).toHaveBeenCalledWith( + expect.objectContaining({ name: 'GrantFox Stadium', plan: 'starter' }), + 'dev-1', + ); + }); + + it('returns 201 with trimmed name and lowercased slug', async () => { + const { app, repository } = buildApp(); + + const res = await request(app) + .post('/api/tenants') + .set('x-user-id', 'dev-1') + .send({ name: ' GrantFox Ops ', slug: 'GrantFox-Ops', plan: 'growth' }); + + expect(res.status).toBe(201); + expect(repository.create).toHaveBeenCalledWith( + expect.objectContaining({ name: 'GrantFox Ops', slug: 'grantfox-ops' }), + 'dev-1', + ); + }); + + it('returns 401 before validation when request is unauthenticated', async () => { + const { app, repository } = buildApp(); + + const res = await request(app) + .post('/api/tenants') + .send({ name: 'GrantFox Ops' }); + + expect(res.status).toBe(401); + expect(res.body.error.code).toBe('UNAUTHORIZED'); + expect(repository.create).not.toHaveBeenCalled(); + }); +}); + +// --------------------------------------------------------------------------- +// Group 3 — HTTP integration: PATCH validation collects param + body errors +// --------------------------------------------------------------------------- + +describe('PATCH /api/tenants/:tenantId — Zod validation integration', () => { + let infoSpy: jest.SpyInstance; + + beforeEach(() => { + infoSpy = jest.spyOn(logger, 'info').mockImplementation(() => undefined); + }); + + afterEach(() => { + infoSpy.mockRestore(); + }); + + it('returns 400 collecting param and body errors in one pass', async () => { + const { app } = buildApp(); + + const res = await request(app) + .patch('/api/tenants/no') + .set('x-user-id', 'dev-1') + .set('x-request-id', 'req-patch-multi-error') + .send({}); + + expect(res.status).toBe(400); + expect(res.body.requestId).toBe('req-patch-multi-error'); + expect(res.body.error.code).toBe('VALIDATION_ERROR'); + expect(res.body.error.details).toEqual( + expect.arrayContaining([ + expect.objectContaining({ field: 'body' }), + expect.objectContaining({ field: 'params.tenantId' }), + ]), + ); + }); + + it('returns 400 when slug is sent (strict schema — not an update field)', async () => { + const { app } = buildApp(); + + const res = await request(app) + .patch('/api/tenants/tenant_123') + .set('x-user-id', 'dev-1') + .send({ slug: 'new-slug' }); + + expect(res.status).toBe(400); + expect(res.body.error.details).toEqual( + expect.arrayContaining([ + expect.objectContaining({ code: 'UNRECOGNIZED_KEYS' }), + ]), + ); + }); + + it('returns 200 success envelope for a valid update', async () => { + const { app, repository } = buildApp(); + + const res = await request(app) + .patch('/api/tenants/tenant_123') + .set('x-user-id', 'dev-1') + .set('x-request-id', 'req-patch-ok') + .send({ plan: 'enterprise' }); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + success: true, + requestId: 'req-patch-ok', + data: expect.objectContaining({ id: 'tenant_123', plan: 'enterprise' }), + }); + expect(repository.update).toHaveBeenCalledWith( + 'tenant_123', + expect.objectContaining({ plan: 'enterprise' }), + 'dev-1', + ); + }); + + it('returns 401 before validation when unauthenticated', async () => { + const { app, repository } = buildApp(); + + const res = await request(app) + .patch('/api/tenants/tenant_123') + .send({ plan: 'enterprise' }); + + expect(res.status).toBe(401); + expect(res.body.error.code).toBe('UNAUTHORIZED'); + expect(repository.update).not.toHaveBeenCalled(); + }); +}); + +// --------------------------------------------------------------------------- +// Group 4 — HTTP integration: GET /api/tenants +// --------------------------------------------------------------------------- + +describe('GET /api/tenants — list integration', () => { + let infoSpy: jest.SpyInstance; + + beforeEach(() => { + infoSpy = jest.spyOn(logger, 'info').mockImplementation(() => undefined); + }); + + afterEach(() => { + infoSpy.mockRestore(); + }); + + it('returns 200 success envelope with tenant array', async () => { + const { app } = buildApp(); + + const res = await request(app) + .get('/api/tenants') + .set('x-user-id', 'dev-1') + .set('x-request-id', 'req-list-ok'); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + success: true, + requestId: 'req-list-ok', + }); + expect(Array.isArray(res.body.data)).toBe(true); + }); + + it('sets a strong ETag header on the list response', async () => { + const { app } = buildApp(); + + const res = await request(app) + .get('/api/tenants') + .set('x-user-id', 'dev-1'); + + expect(res.status).toBe(200); + expect(res.headers.etag).toBeDefined(); + expect(res.headers.etag).toMatch(/^"[0-9a-f]{64}"$/); + }); + + it('returns 304 when If-None-Match matches the ETag', async () => { + const { app } = buildApp(); + + const first = await request(app) + .get('/api/tenants') + .set('x-user-id', 'dev-1'); + + expect(first.status).toBe(200); + const etag = first.headers.etag as string; + + const second = await request(app) + .get('/api/tenants') + .set('x-user-id', 'dev-1') + .set('If-None-Match', etag); + + expect(second.status).toBe(304); + }); + + it('returns 401 when unauthenticated', async () => { + const { app } = buildApp(); + + const res = await request(app).get('/api/tenants'); + + expect(res.status).toBe(401); + expect(res.body.error.code).toBe('UNAUTHORIZED'); + }); +}); From bb39fd0ba33608e94074649ee6d9d09edeaf644e Mon Sep 17 00:00:00 2001 From: Isihaq123 <165170348+Isihaq123@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:44:58 +0100 Subject: [PATCH 3/4] feat: ETag on /api/apis (#1136) - Wire strong SHA-256 ETag middleware (etagMiddleware) to GET /api/apis for RFC 7232 conditional GET support - Remove etagMiddleware from GET /:id (uses Express built-in weak ETag) - Add ?status query param validation to GET /api/apis with 400 on unknown values - Include status param in listings cache key to avoid cross-filter cache collision - Enrich GET /api/apis listing rows with developer info and endpoints - Extend ListingsCacheKeyParams and buildCacheKey to accept optional status field Closes #682 --- src/lib/listingsCache.ts | 5 ++++ src/routes/apis.ts | 53 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/lib/listingsCache.ts b/src/lib/listingsCache.ts index aac7d475..bdcd4260 100644 --- a/src/lib/listingsCache.ts +++ b/src/lib/listingsCache.ts @@ -46,6 +46,8 @@ export interface ListingsCacheKeyParams { search?: string; /** Opaque cursor string for keyset pagination. When present, offset is ignored. */ cursor?: string; + /** Optional status filter. When absent, the route returns active APIs by default. */ + status?: string; } // ── Cache key builder ───────────────────────────────────────────────────────── @@ -67,6 +69,9 @@ export function buildCacheKey(params: ListingsCacheKeyParams): string { category: params.category ?? null, search: params.search ?? null, cursor: params.cursor ?? null, + // Status is included so ?status=draft and ?status=active are cached + // independently and never serve each other's data. + status: params.status ?? null, }); } diff --git a/src/routes/apis.ts b/src/routes/apis.ts index c6ab0ad7..4a637c1b 100644 --- a/src/routes/apis.ts +++ b/src/routes/apis.ts @@ -4,6 +4,7 @@ import { NotFoundError, UnauthorizedError, } from "../errors/index.js"; +import { apiStatusEnum, type ApiStatus } from "../db/schema.js"; import { parseCursorPagination, decodeCursor, @@ -139,6 +140,23 @@ export function createApisRouter(deps: ApisRouterDeps = {}): Router { const search = typeof req.query.search === "string" ? req.query.search : undefined; + // Validate optional ?status filter against the known enum values. + // The public listing only returns active APIs by default; callers may + // explicitly request a different status (e.g. draft) but unknown values + // are rejected early to avoid silent no-result responses. + const statusParam = + typeof req.query.status === "string" ? req.query.status : undefined; + if (statusParam !== undefined) { + if (!apiStatusEnum.includes(statusParam as ApiStatus)) { + next( + new BadRequestError( + `status must be one of: ${apiStatusEnum.join(", ")}`, + ), + ); + return; + } + } + const { limit, cursor: rawCursor } = parseCursorPagination(query); let cursorDate: Date | undefined; @@ -164,6 +182,9 @@ export function createApisRouter(deps: ApisRouterDeps = {}): Router { category, search, cursor: rawCursor, + // Include status in the key so different status filters are cached + // independently and never collide. + status: statusParam, }); const cached = cache.get(cacheKey); if (cached !== undefined) { @@ -179,6 +200,7 @@ export function createApisRouter(deps: ApisRouterDeps = {}): Router { const fetchLimit = rawCursor ? limit : limit + 1; const rows = await apiRepository.listPublic({ limit: fetchLimit, + status: statusParam as ApiStatus | undefined, category, search, cursor: @@ -199,7 +221,34 @@ export function createApisRouter(deps: ApisRouterDeps = {}): Router { ); } - const response = cursorPaginatedResponse(pageRows, { + // Enrich each row with developer info and endpoints. + // findById returns the full ApiDetails (including joined developer). + // getEndpoints is a lightweight indexed lookup per API. + const enrichedRows = await Promise.all( + pageRows.map(async (api) => { + const [details, endpoints] = await Promise.all([ + apiRepository.findById(api.id), + apiRepository.getEndpoints(api.id), + ]); + return { + id: api.id, + name: api.name, + description: api.description, + base_url: api.base_url, + logo_url: api.logo_url, + category: api.category, + status: api.status, + developer: details?.developer ?? { + name: null, + website: null, + description: null, + }, + endpoints, + }; + }), + ); + + const response = cursorPaginatedResponse(enrichedRows, { limit, nextCursor, hasMore, @@ -212,7 +261,7 @@ export function createApisRouter(deps: ApisRouterDeps = {}): Router { } }); - router.get("/:id", etagMiddleware, async (req, res, next) => { + router.get("/:id", async (req, res, next) => { try { const id = Number(req.params.id); From a16a946f724cfae6a2e72bce9bdb8094bae09e8e Mon Sep 17 00:00:00 2001 From: dfwbigcharlie Date: Thu, 30 Jul 2026 19:45:01 +0100 Subject: [PATCH 4/4] feat(rate-limit): add structured JSON access logs for /api/rate-limit (#1137) Adds rateLimitAccessLogMiddleware that emits channel:rate_limit entries with req-id, latency, status, response size, and actor for every /api/rate-limit/* request. Closes #767 --- src/middleware/rateLimitAccessLog.test.ts | 537 ++++++++++++++++++++++ src/middleware/rateLimitAccessLog.ts | 163 +++++++ src/routes/rate-limit.ts | 5 + 3 files changed, 705 insertions(+) create mode 100644 src/middleware/rateLimitAccessLog.test.ts create mode 100644 src/middleware/rateLimitAccessLog.ts diff --git a/src/middleware/rateLimitAccessLog.test.ts b/src/middleware/rateLimitAccessLog.test.ts new file mode 100644 index 00000000..081f796c --- /dev/null +++ b/src/middleware/rateLimitAccessLog.test.ts @@ -0,0 +1,537 @@ +import { EventEmitter } from 'node:events'; +import type { Request, Response } from 'express'; + +import { + createRateLimitAccessLogMiddleware, + rateLimitAccessLogger, + RATE_LIMIT_LOG_REDACTED_VALUE, + rateLimitAccessLogMiddleware, +} from './rateLimitAccessLog.js'; + +type FakeReq = EventEmitter & + Request & { + headers: Record; + id?: string; + }; + +type FakeRes = EventEmitter & + Response & { + statusCode: number; + writableEnded: boolean; + locals: Record; + write: jest.Mock; + end: jest.Mock; + setHeader: jest.Mock; + }; + +function makeReq(overrides: Partial = {}): FakeReq { + return Object.assign(new EventEmitter(), { + method: 'GET', + path: '/api/rate-limit/health', + headers: {}, + id: undefined, + ...overrides, + }) as FakeReq; +} + +function makeRes(overrides: Partial = {}): FakeRes { + return Object.assign(new EventEmitter(), { + statusCode: 200, + writableEnded: true, + locals: {}, + setHeader: jest.fn(), + write: jest.fn(() => true), + end: jest.fn(() => true), + ...overrides, + }) as FakeRes; +} + +describe('createRateLimitAccessLogMiddleware', () => { + test('emits a structured log with all base fields on 2xx', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + + const req = makeReq({ + method: 'GET', + path: '/api/rate-limit/health', + headers: { 'x-request-id': 'req-rl-1' }, + id: 'req-rl-1', + }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(infoSpy).toHaveBeenCalledTimes(1); + const [payload, msg] = infoSpy.mock.calls[0]; + expect(payload).toEqual( + expect.objectContaining({ + correlationId: 'req-rl-1', + requestId: 'req-rl-1', + method: 'GET', + path: '/api/rate-limit/health', + status: 200, + statusCode: 200, + ms: expect.any(Number), + durationMs: expect.any(Number), + responseBytes: 0, + }), + ); + expect(msg).toBe('rate-limit request completed'); + } finally { + infoSpy.mockRestore(); + } + }); + + test('includes actor from res.locals.authenticatedUser', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + + const req = makeReq({ + headers: { 'x-request-id': 'req-actor-1' }, + id: 'req-actor-1', + }); + const res = makeRes({ + statusCode: 200, + locals: { authenticatedUser: { id: 'dev-xyz' } }, + }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(infoSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ actor: 'dev-xyz' }), + ); + } finally { + infoSpy.mockRestore(); + } + }); + + test('omits actor when unauthenticated', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-unauth' }, id: 'req-unauth' }); + const res = makeRes({ statusCode: 200, locals: {} }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + const payload = infoSpy.mock.calls[0][0] as Record; + expect(payload).not.toHaveProperty('actor'); + } finally { + infoSpy.mockRestore(); + } + }); + + test('counts responseBytes from res.write and res.end', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-bytes' }, id: 'req-bytes' }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.write('hello'); + res.end(Buffer.from(' world')); + res.emit('finish'); + + expect(infoSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ responseBytes: 11 }), + ); + } finally { + infoSpy.mockRestore(); + } + }); + + test('logs at warn level for 4xx responses', () => { + const warnSpy = jest.spyOn(rateLimitAccessLogger, 'warn').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-4xx' }, id: 'req-4xx' }); + const res = makeRes({ statusCode: 404 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ status: 404, statusCode: 404 }), + ); + } finally { + warnSpy.mockRestore(); + } + }); + + test('logs at error level for 5xx responses', () => { + const errorSpy = jest.spyOn(rateLimitAccessLogger, 'error').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-5xx' }, id: 'req-5xx' }); + const res = makeRes({ statusCode: 500 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(errorSpy).toHaveBeenCalledTimes(1); + expect(errorSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ status: 500, statusCode: 500 }), + ); + } finally { + errorSpy.mockRestore(); + } + }); + + test('logs at info level for 201 responses', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-201' }, id: 'req-201' }); + const res = makeRes({ statusCode: 201 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(infoSpy).toHaveBeenCalledTimes(1); + expect(infoSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ status: 201 }), + ); + } finally { + infoSpy.mockRestore(); + } + }); + + test('logs at info level for 204 responses', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-204' }, id: 'req-204' }); + const res = makeRes({ statusCode: 204 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(infoSpy).toHaveBeenCalledTimes(1); + expect(infoSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ status: 204 }), + ); + } finally { + infoSpy.mockRestore(); + } + }); + + test('logs at warn level for 401 responses', () => { + const warnSpy = jest.spyOn(rateLimitAccessLogger, 'warn').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-401' }, id: 'req-401' }); + const res = makeRes({ statusCode: 401 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ status: 401, statusCode: 401 }), + ); + } finally { + warnSpy.mockRestore(); + } + }); + + test('emits log on close when response is not writableEnded', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-close' }, id: 'req-close' }); + const res = makeRes({ statusCode: 200, writableEnded: false }); + + middleware(req, res, jest.fn()); + res.emit('close'); + + expect(infoSpy).toHaveBeenCalledTimes(1); + expect(infoSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ requestId: 'req-close' }), + ); + } finally { + infoSpy.mockRestore(); + } + }); + + test('emits log only once when both finish and close fire', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-once' }, id: 'req-once' }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + res.emit('close'); + + expect(infoSpy).toHaveBeenCalledTimes(1); + } finally { + infoSpy.mockRestore(); + } + }); + + test('does not emit a second log when close fires after finish on a writableEnded response', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-ws-end' }, id: 'req-ws-end' }); + const res = makeRes({ statusCode: 200, writableEnded: true }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + res.emit('close'); + + expect(infoSpy).toHaveBeenCalledTimes(1); + } finally { + infoSpy.mockRestore(); + } + }); + + test('prefers x-correlation-id over x-request-id for correlationId', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ + headers: { + 'x-request-id': 'req-id-1', + 'x-correlation-id': 'corr-id-1', + }, + id: 'req-id-1', + }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(infoSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ + correlationId: 'corr-id-1', + requestId: 'req-id-1', + }), + ); + } finally { + infoSpy.mockRestore(); + } + }); + + test('falls back to x-request-id for both correlationId and requestId when no x-correlation-id', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ + headers: { 'x-request-id': 'req-fallback' }, + id: 'req-fallback', + }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(infoSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ + correlationId: 'req-fallback', + requestId: 'req-fallback', + }), + ); + } finally { + infoSpy.mockRestore(); + } + }); + + test('generates a UUID requestId when no id is present and no headers', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: {}, id: undefined }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(infoSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ + requestId: expect.stringMatching( + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/, + ), + }), + ); + } finally { + infoSpy.mockRestore(); + } + }); + + test('accepts array-valued x-request-id header and uses the first element', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ + headers: { 'x-request-id': ['arr-id-1', 'arr-id-2'] }, + id: undefined, + }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(infoSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ requestId: 'arr-id-1' }), + ); + } finally { + infoSpy.mockRestore(); + } + }); + + test('redacts configured fields (case-insensitive)', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware({ + redactFields: ['path', 'requestId'], + }); + + const req = makeReq({ + method: 'GET', + path: '/api/rate-limit/health', + headers: { 'x-request-id': 'req-redact' }, + id: 'req-redact', + }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(infoSpy.mock.calls[0][0]).toEqual( + expect.objectContaining({ + path: RATE_LIMIT_LOG_REDACTED_VALUE, + requestId: RATE_LIMIT_LOG_REDACTED_VALUE, + }), + ); + } finally { + infoSpy.mockRestore(); + } + }); + + test('redacts fields regardless of key case in options', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware({ + redactFields: ['PATH', 'CORRELATIONID'], + }); + const req = makeReq({ + headers: { 'x-request-id': 'req-case' }, + id: 'req-case', + }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + const payload = infoSpy.mock.calls[0][0] as Record; + expect(payload.path).toBe(RATE_LIMIT_LOG_REDACTED_VALUE); + expect(payload.correlationId).toBe(RATE_LIMIT_LOG_REDACTED_VALUE); + } finally { + infoSpy.mockRestore(); + } + }); + + test('uses a custom logger when provided', () => { + const customInfo = jest.fn(); + const customLogger = { info: customInfo, warn: jest.fn(), error: jest.fn() }; + + const middleware = createRateLimitAccessLogMiddleware({ logger: customLogger }); + const req = makeReq({ headers: { 'x-request-id': 'req-custom' }, id: 'req-custom' }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(customInfo).toHaveBeenCalledTimes(1); + }); + + test('custom logger receives warn for 4xx with correct payload', () => { + const customWarn = jest.fn(); + const customLogger = { info: jest.fn(), warn: customWarn, error: jest.fn() }; + + const middleware = createRateLimitAccessLogMiddleware({ logger: customLogger }); + const req = makeReq({ headers: { 'x-request-id': 'req-custom-4xx' }, id: 'req-custom-4xx' }); + const res = makeRes({ statusCode: 400 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + expect(customWarn).toHaveBeenCalledTimes(1); + expect(customWarn.mock.calls[0][0]).toEqual( + expect.objectContaining({ status: 400, requestId: 'req-custom-4xx' }), + ); + }); + + test('calls next() immediately', () => { + const next = jest.fn(); + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: {}, id: 'req-next' }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, next); + + expect(next).toHaveBeenCalledTimes(1); + }); + + test('reports non-negative latency values', () => { + const infoSpy = jest.spyOn(rateLimitAccessLogger, 'info').mockImplementation(() => rateLimitAccessLogger); + + try { + const middleware = createRateLimitAccessLogMiddleware(); + const req = makeReq({ headers: { 'x-request-id': 'req-latency' }, id: 'req-latency' }); + const res = makeRes({ statusCode: 200 }); + + middleware(req, res, jest.fn()); + res.emit('finish'); + + const payload = infoSpy.mock.calls[0][0] as Record; + expect(typeof payload.ms).toBe('number'); + expect(payload.ms as number).toBeGreaterThanOrEqual(0); + expect(payload.durationMs).toBe(payload.ms); + } finally { + infoSpy.mockRestore(); + } + }); +}); + +describe('rateLimitAccessLogger', () => { + test('has channel=rate_limit', () => { + expect(rateLimitAccessLogger).toBeDefined(); + expect(rateLimitAccessLogger.bindings?.()).toEqual( + expect.objectContaining({ channel: 'rate_limit' }), + ); + }); +}); + +describe('rateLimitAccessLogMiddleware', () => { + test('is a function', () => { + expect(typeof rateLimitAccessLogMiddleware).toBe('function'); + }); +}); diff --git a/src/middleware/rateLimitAccessLog.ts b/src/middleware/rateLimitAccessLog.ts new file mode 100644 index 00000000..cb9231af --- /dev/null +++ b/src/middleware/rateLimitAccessLog.ts @@ -0,0 +1,163 @@ +import type { NextFunction, Request, Response } from 'express'; +import { v4 as uuidv4 } from 'uuid'; +import { getClientIp } from '../lib/clientIp.js'; +import { getRequestId } from '../utils/asyncContext.js'; +import { logger } from './logging.js'; +import { sanitizeRequestId } from './requestId.js'; + +export const RATE_LIMIT_LOG_REDACTED_VALUE = '[REDACTED]'; + +export const rateLimitAccessLogger = logger.child({ channel: 'rate_limit' }); + +export interface RateLimitAccessLogPayload { + correlationId: string; + requestId: string; + method: string; + path: string; + status: number; + statusCode: number; + ms: number; + durationMs: number; + responseBytes: number; + actor?: string; + clientIp?: string; +} + +export interface RateLimitAccessLogOptions { + redactFields?: readonly string[]; + logger?: Pick; +} + +const TRUST_PROXY = process.env.TRUST_PROXY_HEADERS === 'true'; + +function byteLength(chunk: unknown, encoding?: BufferEncoding): number { + if (chunk === null || chunk === undefined) return 0; + if (Buffer.isBuffer(chunk)) return chunk.length; + if (typeof chunk === 'string') return Buffer.byteLength(chunk, encoding); + return Buffer.byteLength(String(chunk)); +} + +function extractUserId(res: Response): string | undefined { + const locals = res.locals as Record; + const user = locals.authenticatedUser as { id?: string } | undefined; + return user?.id; +} + +export function createRateLimitAccessLogMiddleware(options: RateLimitAccessLogOptions = {}) { + const redactFieldsLower = options.redactFields?.map((f) => f.toLowerCase()) ?? []; + const loggerInstance = options.logger ?? rateLimitAccessLogger; + + return function rateLimitAccessLogMiddleware(req: Request, res: Response, next: NextFunction): void { + const startAt = process.hrtime.bigint(); + + const requestId = + sanitizeRequestId(req.id) ?? + getRequestId() ?? + sanitizeRequestId( + Array.isArray(req.headers['x-request-id']) + ? req.headers['x-request-id'][0] + : req.headers['x-request-id'], + ) ?? + uuidv4(); + + const correlationId = + sanitizeRequestId( + Array.isArray(req.headers['x-correlation-id']) + ? req.headers['x-correlation-id'][0] + : req.headers['x-correlation-id'], + ) ?? + sanitizeRequestId( + Array.isArray(req.headers['x-request-id']) + ? req.headers['x-request-id'][0] + : req.headers['x-request-id'], + ) ?? + requestId; + + const clientIp = getClientIp(req, TRUST_PROXY); + + let responseBytes = 0; + let emitted = false; + + const originalWrite = typeof res.write === 'function' ? res.write.bind(res) : undefined; + const originalEnd = typeof res.end === 'function' ? res.end.bind(res) : undefined; + + if (originalWrite) { + res.write = ((chunk: unknown, encoding?: unknown, callback?: unknown) => { + responseBytes += byteLength( + chunk, + typeof encoding === 'string' ? (encoding as BufferEncoding) : undefined, + ); + return originalWrite(chunk as never, encoding as never, callback as never); + }) as typeof res.write; + } + + if (originalEnd) { + res.end = ((chunk?: unknown, encoding?: unknown, callback?: unknown) => { + responseBytes += byteLength( + chunk, + typeof encoding === 'string' ? (encoding as BufferEncoding) : undefined, + ); + return originalEnd(chunk as never, encoding as never, callback as never); + }) as typeof res.end; + } + + const emitLog = (): void => { + if (emitted) return; + emitted = true; + + const elapsedMs = Number(process.hrtime.bigint() - startAt) / 1_000_000; + const status = res.statusCode; + const userId = extractUserId(res); + + const payload: RateLimitAccessLogPayload = { + correlationId, + requestId, + method: req.method, + path: req.path, + status, + statusCode: status, + ms: Number(elapsedMs.toFixed(3)), + durationMs: Number(elapsedMs.toFixed(3)), + responseBytes, + ...(userId ? { actor: userId } : {}), + ...(clientIp ? { clientIp } : {}), + }; + + if (redactFieldsLower.length > 0) { + const lowerToActual = Object.keys(payload).reduce>( + (map, key) => { + map[key.toLowerCase()] = key; + return map; + }, + {}, + ); + for (const field of redactFieldsLower) { + const actualKey = lowerToActual[field]; + if (actualKey) { + (payload as unknown as Record)[actualKey] = + RATE_LIMIT_LOG_REDACTED_VALUE; + } + } + } + + if (status >= 500) { + loggerInstance.error({ ...payload }, 'rate-limit request completed'); + } else if (status >= 400) { + loggerInstance.warn({ ...payload }, 'rate-limit request completed'); + } else { + loggerInstance.info({ ...payload }, 'rate-limit request completed'); + } + }; + + res.once('finish', emitLog); + res.once('close', () => { + if (!res.writableEnded) { + emitLog(); + } + }); + + next(); + }; +} + +export const rateLimitAccessLogMiddleware = createRateLimitAccessLogMiddleware(); diff --git a/src/routes/rate-limit.ts b/src/routes/rate-limit.ts index 49597fc2..7e1fd5d3 100644 --- a/src/routes/rate-limit.ts +++ b/src/routes/rate-limit.ts @@ -11,6 +11,7 @@ import { Router } from 'express'; import { correlationMiddleware } from '../middleware/correlation.js'; +import { rateLimitAccessLogMiddleware } from '../middleware/rateLimitAccessLog.js'; import { createRateLimitHealthRouter, type RateLimitHealthDeps } from './rate-limit/health.js'; export interface RateLimitRouterDeps extends Partial { @@ -36,6 +37,10 @@ export function createRateLimitRouter(deps: RateLimitRouterDeps = {}): Router { // handlers, structured logging, and outbound HTTP calls. router.use(correlationMiddleware); + // Structured JSON access log for every /api/rate-limit request. + // Emits req-id, latency, status, response size, and actor when available. + router.use(rateLimitAccessLogMiddleware); + // Rate-limit health dependency probe router.use( '/health',