feat(portal-bff): rate limiting + structured error filter #123

Merged
julien merged 1 commits from feat/portal-bff/security-rate-limit-error-filter into main 2026-05-13 21:34:33 +02:00
Owner

Summary

Closes the phase-2 hardening list that main.ts has been advertising since the security PR (#122). Two new middlewares + one alignment pass on the response shape so every BFF error follows a single contract.

Structured error filter

A global ExceptionFilter (registered via app.useGlobalFilters(...) at the top of bootstrap()) normalises every 4xx/5xx response to a single envelope :

{
  "error": {
    "code": "csrf",
    "message": "CSRF token missing or invalid",
    "traceId": "abc123…"
  }
}
  • code — stable token the SPA can switch on. Either explicit on the HttpException's response object (new UnauthorizedException({ code: 'unauthenticated', message: '...' })) or derived from the status (STATUS_CODE_MAP for the common cases, 'http_error' fallback). 500s always use 'internal'.
  • message — safe human-readable text. 500s never leak the underlying exception (the full message + stack go to the Pino error log line as err: exception — Pino's stack-serialiser does the rest).
  • traceId — current OTel trace id (or null when no span is active). Makes cross-correlation with the audit log + Pino lines trivial.

An exported errorResponse(code, message) helper produces the same envelope for code paths that write the response directly (raw Express middlewares like the CSRF one, the rate-limit handler) — single contract everywhere.

Rate limiting

express-rate-limit mounted after the session middleware:

  • Dynamic max per request: 10/min on /api/auth/login + /api/auth/callback (RATE_LIMIT_AUTH_PER_MINUTE env), 120/min everywhere else (RATE_LIMIT_PER_MINUTE).
  • Bucket key = session id when the request carries an active session, remote IP otherwise. A single attacker can't dodge the limit by rotating sessions; an authenticated user gets per-account fairness regardless of source IP.
  • /api/health is skipped so orchestrator polls don't burn the user quota.
  • 429 response uses the same envelope as everything else ({ error: { code: 'rate_limited', … } }) via the shared errorResponse() helper.
  • In-memory store (single-instance v1 per ADR-0015). Redis-backed store is a one-line config change when we scale out.

Alignment pass

  • CSRF middleware previously returned { error: 'csrf' }. Now returns the full envelope via errorResponse('csrf', 'CSRF token missing or invalid').
  • /auth/me 401 previously wrote { error: 'unauthenticated' } directly. Now throws UnauthorizedException({ code: 'unauthenticated', message: 'Unauthenticated' }) so the filter formats it. Identical response shape on the wire as the CSRF path.

Both spec assertions updated to the new shape.

Type-resolution fix (transitive)

@types/express@4.17.25 was being pulled in transitively by http-proxy-middleware (Nx's webpack-dev-server). express-rate-limit's .d.ts files import 'express' and the type resolver was matching the v4 copy, causing Request type mismatches with our v5-based code. Added "@types/express": "^5.0.6" to pnpm.overrides so the workspace pins a single version everywhere.

Notable choices

StructuredErrorFilter is the source of truth, but raw middlewares are still allowed to write responses directly (rate-limit, CSRF). The reason: Nest's filter chain only handles exceptions thrown from controllers/guards/interceptors. Express middleware short-circuits before that. Both paths now use the same envelope shape through the errorResponse() helper.

No traceId in non-5xx responses? It IS included. The filter writes it on every status — useful for any client-server debugging conversation ("send me your traceId from the 403 you got").

500s strip the exception message. Even if a developer accidentally surfaces a sensitive detail via throw new Error('connection to postgres://user:secret@host failed'), the response body just says "Internal server error". The full message goes to the log — visible to ops, never to clients. This is the standard secure-by-default for unhandled errors.

Dynamic max per request, not two separate rateLimit() instances. Two instances would each maintain a separate store, so the /auth/login bucket would be independent of the general one for the same IP. A single instance with a path-conditional max gives consistent bucket accounting.

Out of scope

  • Redis-backed rate-limit store. v1 ships in-memory; the BFF runs as a single instance. The migration is new RedisStore({ ... }) when we scale out (ADR-0015 mentions this).
  • Per-user override of RATE_LIMIT_PER_MINUTE (e.g. admins / service accounts with higher quotas). No code path for this in v1.
  • CSP fine-tuning for portal-shell + portal-admin once Caddy serves them.

Test plan

  • pnpm nx test portal-bff (clean env) → 199/199 pass (+25 specs: StructuredErrorFilter, rate-limit middleware, CSRF + /me alignments).
  • pnpm nx test feature-auth (clean env) → 28/28 pass.
  • pnpm nx test portal-shell (clean env) → 34/34 pass.
  • pnpm nx run-many -t lint build --projects=portal-bff,feature-auth,portal-shell → clean.
  • Prettier-clean.
  • CI clean-env repro: every env var unset (including new RATE_LIMIT_*) → 261/261 pass.
  • Manual smoke against running BFF:
    • Throw any error from a controller → response is { error: { code, message, traceId } }. Pino log has the full exception under err.
    • Curl /api/auth/me without a session cookie → 401 + same envelope, code: 'unauthenticated'.
    • Hit /api/auth/login 11 times in a minute → 11th returns 429 + code: 'rate_limited'. /api/health hit 100 times → all 200.
    • POST without X-CSRF-Token → 403 + code: 'csrf'.
## Summary Closes the phase-2 hardening list that `main.ts` has been advertising since the security PR (#122). Two new middlewares + one alignment pass on the response shape so every BFF error follows a single contract. ### Structured error filter A global `ExceptionFilter` (registered via `app.useGlobalFilters(...)` at the top of `bootstrap()`) normalises every 4xx/5xx response to a single envelope : ```json { "error": { "code": "csrf", "message": "CSRF token missing or invalid", "traceId": "abc123…" } } ``` - `code` — stable token the SPA can `switch` on. Either explicit on the `HttpException`'s response object (`new UnauthorizedException({ code: 'unauthenticated', message: '...' })`) or derived from the status (`STATUS_CODE_MAP` for the common cases, `'http_error'` fallback). 500s always use `'internal'`. - `message` — safe human-readable text. **500s never leak the underlying exception** (the full message + stack go to the Pino `error` log line as `err: exception` — Pino's stack-serialiser does the rest). - `traceId` — current OTel trace id (or `null` when no span is active). Makes cross-correlation with the audit log + Pino lines trivial. An exported `errorResponse(code, message)` helper produces the same envelope for code paths that write the response directly (raw Express middlewares like the CSRF one, the rate-limit handler) — single contract everywhere. ### Rate limiting `express-rate-limit` mounted after the session middleware: - **Dynamic max per request**: 10/min on `/api/auth/login` + `/api/auth/callback` (`RATE_LIMIT_AUTH_PER_MINUTE` env), 120/min everywhere else (`RATE_LIMIT_PER_MINUTE`). - **Bucket key** = session id when the request carries an active session, remote IP otherwise. A single attacker can't dodge the limit by rotating sessions; an authenticated user gets per-account fairness regardless of source IP. - **`/api/health` is skipped** so orchestrator polls don't burn the user quota. - 429 response uses the same envelope as everything else (`{ error: { code: 'rate_limited', … } }`) via the shared `errorResponse()` helper. - In-memory store (single-instance v1 per ADR-0015). Redis-backed store is a one-line config change when we scale out. ### Alignment pass - **CSRF middleware** previously returned `{ error: 'csrf' }`. Now returns the full envelope via `errorResponse('csrf', 'CSRF token missing or invalid')`. - **`/auth/me` 401** previously wrote `{ error: 'unauthenticated' }` directly. Now throws `UnauthorizedException({ code: 'unauthenticated', message: 'Unauthenticated' })` so the filter formats it. Identical response shape on the wire as the CSRF path. Both spec assertions updated to the new shape. ### Type-resolution fix (transitive) `@types/express@4.17.25` was being pulled in transitively by `http-proxy-middleware` (Nx's webpack-dev-server). `express-rate-limit`'s `.d.ts` files import `'express'` and the type resolver was matching the v4 copy, causing `Request` type mismatches with our v5-based code. Added `"@types/express": "^5.0.6"` to `pnpm.overrides` so the workspace pins a single version everywhere. ## Notable choices **`StructuredErrorFilter` is the source of truth, but raw middlewares are still allowed to write responses directly** (rate-limit, CSRF). The reason: Nest's filter chain only handles exceptions thrown from controllers/guards/interceptors. Express middleware short-circuits before that. Both paths now use the same envelope shape through the `errorResponse()` helper. **No `traceId` in non-5xx responses?** It IS included. The filter writes it on every status — useful for any client-server debugging conversation ("send me your traceId from the 403 you got"). **500s strip the exception message.** Even if a developer accidentally surfaces a sensitive detail via `throw new Error('connection to postgres://user:secret@host failed')`, the response body just says "Internal server error". The full message goes to the log — visible to ops, never to clients. This is the standard secure-by-default for unhandled errors. **Dynamic `max` per request, not two separate `rateLimit()` instances.** Two instances would each maintain a separate store, so the `/auth/login` bucket would be independent of the general one for the same IP. A single instance with a path-conditional max gives consistent bucket accounting. ## Out of scope - Redis-backed rate-limit store. v1 ships in-memory; the BFF runs as a single instance. The migration is `new RedisStore({ ... })` when we scale out (ADR-0015 mentions this). - Per-user override of `RATE_LIMIT_PER_MINUTE` (e.g. admins / service accounts with higher quotas). No code path for this in v1. - CSP fine-tuning for portal-shell + portal-admin once Caddy serves them. ## Test plan - [x] `pnpm nx test portal-bff` (clean env) → **199/199 pass** (+25 specs: StructuredErrorFilter, rate-limit middleware, CSRF + /me alignments). - [x] `pnpm nx test feature-auth` (clean env) → **28/28 pass**. - [x] `pnpm nx test portal-shell` (clean env) → **34/34 pass**. - [x] `pnpm nx run-many -t lint build --projects=portal-bff,feature-auth,portal-shell` → clean. - [x] Prettier-clean. - [x] CI clean-env repro: every env var unset (including new `RATE_LIMIT_*`) → 261/261 pass. - [ ] Manual smoke against running BFF: - [ ] Throw any error from a controller → response is `{ error: { code, message, traceId } }`. Pino log has the full exception under `err`. - [x] Curl `/api/auth/me` without a session cookie → 401 + same envelope, `code: 'unauthenticated'`. - [ ] Hit `/api/auth/login` 11 times in a minute → 11th returns 429 + `code: 'rate_limited'`. `/api/health` hit 100 times → all 200. - [ ] POST without `X-CSRF-Token` → 403 + `code: 'csrf'`.
julien added 1 commit 2026-05-13 21:32:53 +02:00
feat(portal-bff): rate limiting + structured error filter
CI / scan (pull_request) Successful in 2m8s
CI / commits (pull_request) Successful in 2m9s
CI / check (pull_request) Successful in 3m31s
CI / a11y (pull_request) Successful in 1m25s
CI / perf (pull_request) Successful in 4m34s
2454186cc4
closes the phase-2 hardening list main.ts has been advertising since
#122 (helmet + cors + csrf). two new middlewares + an alignment
pass so every bff error follows a single response contract.

structured error filter:
  global ExceptionFilter (registered via app.useGlobalFilters at
  the top of bootstrap()) normalises every 4xx/5xx response to:
    { error: { code, message, traceId } }
  - code: stable token the spa can switch on. either explicit on
    the HttpException's response object
    (UnauthorizedException({ code: 'unauthenticated', message }))
    or derived from status. 500s always 'internal'.
  - message: safe text. 500s NEVER leak the underlying exception
    (full message + stack go to pino's err: field, never to the
    response body).
  - traceId: otel trace id for cross-correlation.
  exported errorResponse(code, message) helper for raw-middleware
  callers (csrf, rate-limit) so the envelope is identical whether
  the response came through nest's filter or a short-circuit
  middleware.

rate limiting:
  express-rate-limit mounted after the session middleware.
  - dynamic max per request: 10/min on /api/auth/login + /api/auth/
    callback (RATE_LIMIT_AUTH_PER_MINUTE), 120/min elsewhere
    (RATE_LIMIT_PER_MINUTE).
  - bucket key = session id when auth, remote ip otherwise. rotating
    sessions doesn't dodge the limit; auth'd users get per-account
    fairness regardless of source ip.
  - /api/health skipped. orchestrator polls don't burn user quota.
  - 429 uses the shared errorResponse envelope.
  - in-memory store. single-instance v1 per adr-0015. redis-backed
    store is one config arg the day we scale out horizontally.

alignment pass on existing error responses:
  - csrf middleware used to return { error: 'csrf' }. now uses
    errorResponse('csrf', 'CSRF token missing or invalid').
  - /auth/me used to res.status(401).json({ error: 'unauthenticated' })
    directly. now throws UnauthorizedException({ code: ...,
    message: ... }) so the filter formats it. identical shape on
    the wire.

type-resolution fix (transitive):
  @types/express@4.17.25 was being pulled in by http-proxy-middleware
  (nx's webpack-dev-server). express-rate-limit's .d.ts files import
  'express', and tsc was matching the v4 copy — causing Request type
  mismatches with our v5-based code. added "@types/express": "^5.0.6"
  to pnpm.overrides so the workspace pins one version everywhere.

specs:
  - 199/199 portal-bff (was 174 before; +25 specs across
    StructuredErrorFilter, rate-limit middleware, csrf alignment,
    /me alignment).
  - feature-auth 28/28, portal-shell 34/34 unchanged.
  - clean-env repro: env -u every config var → 261/261 pass.

out of scope:
  - redis-backed rate-limit store (single-instance v1).
  - per-user rate-limit overrides (admin / service accounts).
  - csp fine-tuning when caddy serves the static spas.
julien merged commit 0e6c114ba7 into main 2026-05-13 21:34:33 +02:00
julien deleted branch feat/portal-bff/security-rate-limit-error-filter 2026-05-13 21:34:35 +02:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: julien/apf_portal#123