Skip to content

feat: migrate HTTP platform from Express to Fastify#3190

Open
PooyaRaki wants to merge 7 commits into
mainfrom
feat/fastify
Open

feat: migrate HTTP platform from Express to Fastify#3190
PooyaRaki wants to merge 7 commits into
mainfrom
feat/fastify

Conversation

@PooyaRaki

Copy link
Copy Markdown
Contributor

What

Migrates the gateway's HTTP platform from Express to Fastify: swaps @nestjs/platform-express@nestjs/platform-fastify, replaces cookie-parser with @fastify/cookie, and adds @fastify/static for static assets.

Why

Fastify offers lower per-request overhead and a more modern plugin model. This is an internal infrastructure change intended to be behaviour-preserving for clients.

Key changes

  • Adapter built from config at bootstrap. Fastify needs trustProxy and the JSON bodyLimit at construction time — before the DI container exists — so main.ts reads the raw configuration and builds the adapter directly (createFastifyAdapterFromConfiguration). This replaces the old Express app.set('trust proxy') / app.use(json(...)) post-construction setup. - Request access abstracted behind src/routes/common/http/http-request.utils.ts (getHeader / getRoutePath / getRouteParam / getRouteUrl), so guards, interceptors, filters and decorators no longer depend on Express types.

  • Response API migrated: res.json()res.send(), res.cookie()res.setCookie(), res.set()/.header()reply.header(), res.headersSentreply.sent, res.redirect(code, url)res.redirect(url, code).

  • Cookie maxAge is now in seconds (Fastify/@fastify/cookie) rather than milliseconds (Express) — auth and OIDC flows updated accordingly (getSecondsUntil, ms→s conversions).

  • Test harness: createTestApplication / initTestApplication call Fastify's ready() after init() so supertest can reach the server; all boot specs (integration + e2e) use them.

  • Removed the AppProvider / DefaultAppProvider abstraction — the production bootstrap is inlined into main.ts, and TestAppProvider is now a standalone test helper (its public provide() API is unchanged, so consumer specs were untouched).

    Testing

  • ✅ Unit suite: 5363 passing, 0 failing.

  • ✅ Production build typecheck (tsconfig.build.json): clean.

  • ✅ Biome: clean.

  • Integration & e2e require live Redis/Postgres/AMQP and must be verified in CI / locally.

Reviewer notes

  • Please confirm yarn test:integration and yarn test:e2e are green in CI.
  • A quick yarn start smoke test is worthwhile to confirm Fastify boots with Swagger (/api), static assets, and cookie auth.
  • The config namespace is still named express.* (jsonLimit, trustProxy) — left as-is to avoid touching deployed env vars; can be renamed in a follow-up.

🤖 Generated with Claude Code

@datadog-official

This comment has been minimized.

Replace @nestjs/platform-express with @nestjs/platform-fastify and swap
supporting middleware: cookie-parser -> @fastify/cookie, plus @fastify/static
for static assets.

Key changes:
- Build the Fastify adapter from configuration at bootstrap (trustProxy and
  JSON body limit), since Fastify requires these at construction time, before
  the DI container exists.
- Abstract request access behind src/routes/common/http/http-request.utils.ts
  (getHeader / getRoutePath / getRouteParam / getRouteUrl).
- Response API: res.json() -> res.send(), res.cookie() -> res.setCookie(),
  res.set()/.header() -> reply.header(), res.headersSent -> reply.sent,
  res.redirect(code, url) -> res.redirect(url, code).
- Cookie maxAge is now expressed in seconds (Fastify) instead of milliseconds
  (Express).
- Tests: createTestApplication / initTestApplication call Fastify's ready()
  after init() so supertest can reach the server; all boot specs use them.
- Inline the production bootstrap into main.ts and remove the AppProvider /
  DefaultAppProvider abstraction; TestAppProvider is now standalone.

Behavioural parity intended; unit suite green (5363 passing). Integration and
e2e suites require live Redis/Postgres/AMQP to verify.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@PooyaRaki

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown

Claude finished @PooyaRaki's task in 4m 37s —— View job


Claude PR Review — Express → Fastify migration

  • Load review tools
  • Read key changed files
  • Identify bugs, regressions, security & test coverage gaps
  • Post review feedback

Overall, this is a clean, well-scoped migration. Below are the things worth tightening before merge.


🐛 Potential bugs

  1. Math.floor(stateMaxAge / 1_000) can round a sub-second TTL to 0src/modules/auth/oidc/routes/oidc-auth.controller.ts:110. If auth.stateTtlMs is ever configured below 1000, the OIDC state cookie is written with maxAge: 0 and is effectively unset, breaking the state check in the callback. Today's defaults make this unlikely, but it's an easy footgun. Prefer Math.max(1, Math.ceil(stateMaxAge / 1_000)) and consider asserting the lower bound at startup. Fix this →

  2. parseBodyLimit is stricter than the old bytes parser it replacessrc/app.provider.ts:38-56. The previous Express config used body-parser which delegates to the bytes library and accepts e.g. tb, pb, and looser whitespace/casing rules. The new regex only matches b|kb|mb|gb and \d+(?:\.\d+)?. A previously valid EXPRESS_JSON_LIMIT=1tb (or '1 KiB') now throws at bootstrap. Worth either:

    • documenting the supported units in the env-var docstring at src/config/entities/configuration.ts:386, and/or
    • reusing the bytes package (already a transitive dep) to preserve compatibility.
  3. NotFoundLoggerMiddleware types the request as raw IncomingMessagesrc/middleware/not-found-logger.middleware.ts:25. This works because @fastify/middie (used by NestJS' Fastify middleware adapter) does set originalUrl, but it isn't part of the Node type. Type drift may make a future change silently fall back to req.url. Consider importing the shared RequestLike from http-request.utils.ts for consistency.

  4. createTestApplication skips production-equivalent adapter configsrc/__tests__/test-app.provider.ts:18. It calls new FastifyAdapter() with no trustProxy/bodyLimit, while TestAppProvider.provide() correctly uses createFastifyAdapter(configurationService). Tests that exercise either should standardize on the configured adapter, otherwise behaviour-drift between integration tests and production is invisible.

🔐 Security / robustness

  1. getRouteUrl silently falls back to 'localhost'src/routes/common/http/http-request.utils.ts:69-74. If the Host header is missing (unusual but possible from internal probes), pagination cursors and any audit logs that include this URL will contain a misleading authority. Previously Express would throw because request.get('Host') returned undefined and the URL constructor failed. Recommend throwing here too, or at minimum logging a warning, to surface misconfigured callers rather than masking them.

  2. TenderlySignatureGuard HMAC over re-stringified bodysrc/modules/alerts/routes/guards/tenderly-signature.guard.ts:57 (pre-existing, but now even more relevant under Fastify). Re-serializing the parsed JSON to compute the HMAC depends on Fastify's parser preserving the exact byte representation that the sender signed. Insertion-order is preserved by secure-json-parse, but whitespace, number formatting, and Unicode escapes are not guaranteed. Long-term, prefer Fastify's rawBody / a preParsing hook to verify against the actual received bytes.

🧪 Test coverage

  1. No assertions on the new seconds-vs-milliseconds cookie units. The maxAge/getSecondsUntil change is exactly the kind of unit conversion that warrants a regression test (e.g. an integration test verifying the Set-Cookie: Max-Age=... value falls in the expected seconds-range). I didn't find one — src/app.provider.spec.ts covers trustProxy/bodyLimit well, but cookie units are uncovered.

  2. Datadog reports the integration-tests job failed for commit 61942e7. The PR description already flags integration/e2e as pending — please make sure those are green before merge, since the Fastify ready() change in initTestApplication and the cookie-unit changes are the most likely culprits if anything regressed.

🧹 Quality / nits

  1. The config namespace express.* and the docstring "bytes library for parsing" at src/config/entities/configuration.ts:385-387 are now stale. The PR description calls this out as a deferred rename; an inline TODO or comment update would prevent confusion for the next reader.

  2. parseBodyLimit("1.5b") returns Math.floor(1.5 * 1) = 1 — harmless but indicates the regex permits decimals where they're meaningless. A minor tweak (b not allowing decimals) would surface bad configs earlier.

  3. @Res({ passthrough: true }) + Promise<void> handlers (e.g. auth.controller.ts:127, oidc-auth.controller.ts:94) — under Fastify, NestJS will auto-send undefined. Behaviour is fine, but a quick e2e check that POST /v1/auth/verify returns 200 "" (not 204) is worth confirming during smoke testing.

✅ What looks good

  • The createFastifyAdapterFromConfiguration split keeping adapter creation pure & testable is the right call given Fastify's construction-time config requirement.
  • RequestLike abstraction in http-request.utils.ts is a nice insulation layer — guards/decorators/filters look largely portable now.
  • @fastify/cookie is registered as a Fastify plugin (app.register(...)) instead of a Nest middleware — correct path.
  • Cookie unit conversion (ms → s) appears applied consistently at every cookie call-site I audited.

Nothing here is blocking on its own; #1 and #2 are the two I'd want addressed (or explicitly accepted) before merging.


· Branch: feat/fastify

The Express->Fastify platform swap left several parity gaps that failed
the integration-tests CI job:

- Re-enable trailing-slash matching (Fastify defaults it off) via
  routerOptions.ignoreTrailingSlash, fixing 404s on `/path/` routes.
- Raise routerOptions.maxParamLength to 1024 (Fastify defaults to 100) so
  composite ids like `multisig_<address>_<safeTxHash>` (~118 chars) match
  instead of 404ing.
- Preserve the Set-Cookie contract: derive `Expires` from `maxAge` on set
  and clear without `Max-Age=0`, via getSetCookieOptions/getClearCookieOptions
  (auth + oidc controllers).
- Pass an explicit 302 to the oidc redirects; Fastify reuses the
  NestJS-set 200 status when no code is supplied.
- Add NullResponseInterceptor so a top-level `null` controller return
  serializes to an empty body (Express behaviour) instead of literal `null`.
- Enable versioning in createTestApplication so the v1/v2 `/chains`
  controllers no longer collide ("GET already declared for route") under
  Fastify, and fix hooks.controller spec to the real `/v1/hooks/events` path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@PooyaRaki PooyaRaki self-assigned this Jun 24, 2026
PooyaRaki and others added 3 commits June 24, 2026 11:17
- guard the OIDC state cookie against a sub-second TTL collapsing to
  Max-Age=0 (Math.max(1, floor(stateMaxAge/1000)))
- restore body-parser parity by accepting tb/pb units in parseBodyLimit
- share FASTIFY_ROUTER_OPTIONS so createTestApplication matches production
  route matching (trailing slash, long composite ids)
- type the 404 middleware request as the shared RequestLike
- refresh the stale jsonLimit docstring (parser, units, default, namespace)
- add runnable unit specs for auth cookie options and parseBodyLimit

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@PooyaRaki PooyaRaki marked this pull request as ready for review June 24, 2026 13:17
@PooyaRaki PooyaRaki requested a review from a team as a code owner June 24, 2026 13:17
@PooyaRaki

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Jun 24, 2026

Copy link
Copy Markdown

Claude finished @PooyaRaki's task in 3m 34s —— View job


Claude PR Review — Express → Fastify migration

  • Read key changed files
  • Identify bugs, regressions, security, performance & test coverage gaps
  • Post review feedback

Most of the prior review feedback has been addressed (sub-second TTL guard at oidc-auth.controller.ts:116, tb/pb units in parseBodyLimit, RequestLike in NotFoundLoggerMiddleware, cookie-unit regression tests in auth-cookie.utils.spec.ts). Remaining items below — none are blocking on their own.


🐛 Bugs / correctness

  1. targeted-messaging.controller.ts:86-90 returns 204 No Content with a body. Under Fastify, a 204 reply is required by spec to have no body; the framework strips it (or warns) — but the intent of send({}) is unclear and the behaviour changes vs. the Express era which would happily emit {} at 204. If clients ever parsed the empty object, this is a silent contract change. Either return 200 with {} or drop the body and return 204 with .send(). Fix this →

  2. getRouteUrl still silently falls back to 'localhost'src/routes/common/http/http-request.utils.ts:69-74. Used by PaginationDataDecorator and RouteUrlDecorator, so a missing Host header (rare but possible from internal probes or misbehaving load-balancers) bakes a misleading authority into pagination cursors that clients then send back. Express would have thrown via URL(...) because it returned undefined. Prefer throwing/logging here over masking. Fix this →

  3. parseBodyLimit('1.5b') silently returns 1src/app.provider.ts:73. The regex permits decimals for the b unit where they're meaningless; Math.floor(1.5 * 1) = 1 masks a bad config. Restrict the bytes group to integers: ^(\d+)\s*b$|^(\d+(?:\.\d+)?)\s*(kb|mb|gb|tb|pb)$ or assert no decimal for b.

🔐 Security / robustness

  1. TenderlySignatureGuard HMAC over re-stringified bodysrc/modules/alerts/routes/guards/tenderly-signature.guard.ts:57 (pre-existing, but Fastify makes it worth flagging). JSON.stringify(args.body) depends on the Fastify parser preserving the exact byte representation that the sender signed. Insertion order is preserved by secure-json-parse, but whitespace, number formatting, and Unicode escapes are not guaranteed. Long-term, register Fastify's rawBody plugin / a preParsing hook and HMAC the actual received bytes.

  2. OidcAuthService.stateTtlMs has no startup assertion against sub-second values. The cookie path is now bounded (max(1, …)), but a configured stateTtlMs < 1000 silently becomes a 1-second TTL — likely unreachable for the user, breaking the callback state check. Assert >= 1_000 at module init (or in configuration.ts) so misconfiguration fails fast rather than degrading behaviour.

🧪 Test coverage

  1. createTestApplication diverges from prod adapter configsrc/__tests__/test-app.provider.ts:26. The comment explains the trade-off, but specs using this helper won't catch a regression in body-limit or trust-proxy parsing. Worth either funneling all helpers through createFastifyAdapter or adding an integration test that asserts the production adapter is constructed with the configured options end-to-end.

  2. No coverage of the targeted-messaging 204-body behaviour. Given Configure E2E tests #1 above, an integration assertion on the actual Set-Cookie/Content-Length of the not-found branch would lock the contract down.

🧹 Quality / nits

  1. Stale express.* namespace and docstringsrc/config/entities/configuration.ts:382-397. The PR notes this is a deferred rename. A TODO(fastify-rename) marker would help the next reader.

  2. @app.module.ts:151ServeStaticModule.forRoot({ exclude: ['{*any}'] }). Fastify pattern syntax; please verify in a yarn start smoke test that /favicon.png / /logo.svg (consumed by the Swagger UI at /api) are still served correctly — the prior Express * exclude semantics aren't 1-to-1.

  3. @Res({ passthrough: true }) with Promise<void> handlers (e.g. auth.controller.ts:127, 155, oidc-auth.controller.ts:94, 161) — under Fastify with NestJS auto-send, response will be an empty body. Behaviour-correct, but an e2e assertion on POST /v1/auth/verify / POST /v1/auth/logout returning 200 with empty body would prevent silent drift.

✅ What looks good

  • Clean split between createFastifyAdapterFromConfiguration (pure) and createFastifyAdapter (DI-bound), letting main.ts build the adapter before the container exists.
  • FASTIFY_ROUTER_OPTIONS (ignoreTrailingSlash + raised maxParamLength) restores two non-obvious Express behaviours that would have produced spurious 404s.
  • NullResponseInterceptor preserves the Express empty-body contract for null returns — a subtle Fastify gotcha caught well.
  • RequestLike abstraction in http-request.utils.ts keeps guards/decorators/filters portable.
  • Cookie unit conversion (ms → s) appears applied consistently at every cookie call-site I audited, and getSetCookieOptions adds an explicit expires to preserve the previous Express Max-Age+Expires pair.

· Branch: feat/fastify

- Reject sub-second AUTH_STATE_TTL_MILLISECONDS (min 1s) so a misconfigured
  OIDC state cookie fails fast instead of collapsing to a 1s TTL
- Send no body on the targeted-messaging 204 branch instead of an empty
  object (Fastify strips 204 payloads either way)
- Mark the deferred express.* config rename with TODO(fastify-rename)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@PooyaRaki

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Jun 24, 2026

Copy link
Copy Markdown

Claude PR Review — Express → Fastify migration

  • Load review tools
  • Read key changed files
  • Identify issues to improve
  • Post review feedback

View job run

@PooyaRaki

Copy link
Copy Markdown
Contributor Author

All prior review feedback has been addressed. Latest commit (49ef8cb) covers the remaining items:

  • AUTH_STATE_TTL_MILLISECONDS now rejects sub-second values at startup (.min(1_000)) — misconfiguration fails fast instead of silently collapsing the OIDC state cookie TTL.
  • targeted-messaging 204 branch now calls .send() with no body, consistent with HTTP spec and Fastify's own behaviour.
  • Deferred express.* config rename marked TODO(fastify-rename) for greppability.

Items not actioned (with rationale):

  • getRouteUrl localhost fallback — Express did not throw on a missing Host; it produced http://undefined/…. No regression.
  • parseBodyLimit('1.5b')→1 — deliberate bytes-library parity, documented and tested.
  • Tenderly rawBody HMAC — pre-existing, behaviour-affecting; out of scope for this PR.
  • ServeStaticModule exclude — unchanged from main, not a migration artifact.
  • Test coverage items — all three scenarios are already exercised by existing integration specs.

@PooyaRaki

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Jun 24, 2026

Copy link
Copy Markdown

Claude finished @PooyaRaki's task in 4m 1s —— View job


Claude PR Review — Express → Fastify migration

  • Load review tools
  • Read key changed files
  • Identify issues to improve
  • Post review feedback

Prior review rounds have addressed the bulk of issues. The migration is in good shape — the remaining items are minor and none are blocking.


🐛 Bugs / correctness

  1. maxParamLength: 1024 is broader than requiredsrc/app.provider.ts:44. The justification (composite ids like multisig_<address>_<safeTxHash>) caps out around ~118 chars. 1024 lets clients pass kilobyte-long path segments that then flow through every ValidationPipe/zod parser downstream — gratuitous work and a mild DoS vector. Tightening to 256 (~2× the longest legitimate id) preserves the fix without the surface. Fix this →

  2. getRoutePath falls back to the resolved request pathsrc/routes/common/http/http-request.utils.ts:50-57. When routeOptions.url/route.path/path are all absent, the function returns the actual URL (e.g. /v1/chains/1/safes/0xabc…) instead of the route template (/v1/chains/:chainId/safes/:safeAddress). Under Fastify routeOptions.url should always be present on a matched route, but if anything goes through this path (e.g. 404 logging), it will silently explode log cardinality in Datadog. An assertion or explicit 'unknown' sentinel would surface the regression instead of masking it.

  3. logging/utils.ts:43request.method ?? 'UNKNOWN' is unreachable under Fastify (method is always set on the request). Either drop the fallback or, if the intent is to support the raw Node request from NotFoundLoggerMiddleware, document it — right now it reads like defensive code with no triggering case.

🔐 Security / robustness

  1. TenderlySignatureGuard HMAC over re-stringified bodysrc/modules/alerts/routes/guards/tenderly-signature.guard.ts:57. Pre-existing and intentionally out of scope per the author, but flagged again because Fastify's parser behaviour (whitespace, number formatting, Unicode escaping) is what now binds the HMAC contract. Worth tracking as a follow-up: register a preParsing hook / fastify-raw-body and HMAC the actual received bytes.

  2. @fastify/middie is used implicitly via MiddlewareConsumer.apply(...) but not a direct dependency in package.json. NestJS pulls it transitively today; pinning it explicitly insulates the bootstrap from upstream peer-dep churn.

🧪 Test coverage

  1. No regression test for the Fastify route-options. app.provider.spec.ts covers trustProxy/bodyLimit well, but ignoreTrailingSlash and maxParamLength (both load-bearing for not 404-ing legitimate traffic) aren't asserted. A 2-line supertest case each would prevent silent reversion.

  2. No coverage of the OIDC state cookie TTL clampoidc-auth.controller.ts:116. The Math.max(1, Math.floor(stateMaxAge / 1_000)) guard is exactly the kind of arithmetic edge case worth a unit test (stateMaxAge = 500 → maxAge = 1).

🧹 Quality / nits

  1. auth.controller.ts:155logout declared void, logoutWithRedirect declared void, verify declared Promise<void> — under Fastify with @Res({ passthrough: true }), NestJS auto-sends. Functionally fine, but a one-line e2e assertion on POST /v1/auth/verify returning 200 with no body would lock the contract.

  2. NullResponseInterceptor only maps top-level nullnull nested inside an object (e.g. { value: null }) is preserved, which is correct. The docstring already notes this; consider adding a brief unit test for the nested case to make the contract explicit.

✅ What looks good

  • createFastifyAdapterFromConfiguration / createFastifyAdapter split keeps main.ts able to build the adapter before the DI container — clean separation.
  • FASTIFY_ROUTER_OPTIONS restores Express' tolerant routing without scattering config across the codebase.
  • parseBodyLimit mirrors the bytes library's unit set (b…pb), and the parser tests cover both the integer and decimal paths plus whitespace/case normalisation.
  • RequestLike abstraction lets guards/decorators/filters/middleware share one shape across FastifyRequest and the raw Node IncomingMessage that middie hands to NotFoundLoggerMiddleware.
  • getSetCookieOptions reasserting expires alongside Max-Age preserves the Express Set-Cookie shape that callers/proxies may already depend on.
  • AUTH_STATE_TTL_MILLISECONDS .min(1_000) guard at the schema layer is the correct place to fail fast on misconfiguration.

Nothing here blocks merge; #1 (maxParamLength) is the one I'd most encourage tightening before this lands.


· Branch: feat/fastify ·

- Tighten maxParamLength from 1024 to 256 (~2× the longest real composite
  id, ~118 chars) to avoid admitting kilobyte-long path segments through
  ValidationPipe/zod on every request
- Return 'unknown' sentinel from getRoutePath when no route template is
  available (unmatched/404 requests) instead of the actual URL, preventing
  log cardinality explosion in Datadog
- Document request.method ?? 'UNKNOWN' fallback as a type-required sentinel
  (unreachable at runtime under both FastifyRequest and raw Node IncomingMessage)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@PooyaRaki

Copy link
Copy Markdown
Contributor Author

Reduced maxParamLength from 1024256 based on review feedback.

The longest real composite id in the API is multisig_<address>_<safeTxHash>, which caps out around ~118 chars. 256 is ~2× that, giving a comfortable safety margin for any legitimate value while closing the gap that previously let clients send kilobyte-long path segments — those would flow through ValidationPipe/zod on every matched request, adding gratuitous overhead and creating a mild DoS surface. 1024 was set conservatively during the initial migration; 256 is the right long-term value.

@PooyaRaki

Copy link
Copy Markdown
Contributor Author

@claude do a final review, all the previous remarks are fixed.

@PooyaRaki

Copy link
Copy Markdown
Contributor Author

@claude review all the remarks are fixed.

@claude

claude Bot commented Jun 24, 2026

Copy link
Copy Markdown

Claude finished @PooyaRaki's task in 1m 57s —— View job


Claude PR Review — Express → Fastify migration (final pass)

  • Load review tools
  • Read latest changed files
  • Verify prior remarks are addressed
  • Post final review feedback

Confirmed the latest changes against prior review rounds. The migration is in a good state to merge — nothing blocking remains.


✅ Prior remarks addressed

  • FASTIFY_ROUTER_OPTIONS.maxParamLength tightened from 1024256 (src/app.provider.ts:44). Comment correctly justifies "~2× the longest legitimate id".
  • getRoutePath sentinelsrc/routes/common/http/http-request.utils.ts:50-60 now returns 'unknown' on the unmatched path instead of the actual URL, preventing log-cardinality explosion in Datadog. The inline comment explains the contract.
  • AUTH_STATE_TTL_MILLISECONDS startup guardsrc/config/entities/schemas/configuration.schema.ts:66 rejects sub-second values via .min(1_000), so misconfiguration fails fast. The cookie-side Math.max(1, Math.floor(...)) at oidc-auth.controller.ts:116 is a sound second layer of defence.
  • Targeted-messaging 204 bodysrc/modules/targeted-messaging/routes/targeted-messaging.controller.ts:89-92 now calls .send() with no body, matching the HTTP spec and Fastify's own behaviour.
  • request.method ?? 'UNKNOWN' documented at src/logging/utils.ts:43-45 as a type-required sentinel — fine.
  • TODO(fastify-rename) marker added at src/config/entities/configuration.ts:387-389.

🟡 Items intentionally deferred (rationale accepted)

  • getRouteUrl localhost fallback — Express produced http://undefined/… rather than throwing, so this is not a regression.
  • parseBodyLimit('1.5b') → 1 — deliberate bytes-library parity; covered by app.provider.spec.ts:133-135.
  • Tenderly rawBody HMAC — pre-existing and behaviour-affecting; out of scope.
  • ServeStaticModule.forRoot({ exclude: ['{*any}'] }) — unchanged from main.

🧹 Minor nits (non-blocking, optional)

  1. createTestApplication adapter divergence is acknowledged in code (src/__tests__/test-app.provider.ts:22-26) — leaving Fastify defaults for trustProxy/bodyLimit here is reasonable, but if a future spec needs to assert production-equivalent body-size handling, funneling through createFastifyAdapter (as TestAppProvider.provide() already does) is the safer choice.

  2. getRoutePath fallback chain still keeps request.route?.path ?? request.path — under Fastify, routeOptions.url is the canonical source on a matched route, so the intermediate fallbacks are unreachable in practice. Not worth ripping out, but a one-line comment that they exist only to satisfy the shared RequestLike shape used by the raw IncomingMessage middie path would prevent confusion.

✅ What looks good

  • createFastifyAdapterFromConfiguration / createFastifyAdapter split keeps main.ts able to build the adapter before the DI container exists — clean separation.
  • FASTIFY_ROUTER_OPTIONS (ignoreTrailingSlash, maxParamLength: 256) restores Express' tolerant routing without admitting kilobyte-long path segments.
  • parseBodyLimit mirrors the bytes library's unit set (b…pb); tests cover integer, decimal, whitespace, and case-insensitive paths.
  • RequestLike abstraction (src/routes/common/http/http-request.utils.ts:12-25) keeps guards/decorators/filters/middleware portable across FastifyRequest and the raw Node IncomingMessage that middie hands to NotFoundLoggerMiddleware.
  • getSetCookieOptions reasserting expires alongside Max-Age preserves the Express Set-Cookie shape callers/proxies may depend on, and auth-cookie.utils.spec.ts covers the seconds-vs-milliseconds contract.
  • AUTH_STATE_TTL_MILLISECONDS .min(1_000) at the schema layer is the right place to fail fast on misconfiguration.

LGTM — no further changes needed from my side. Suggest confirming yarn test:integration and yarn test:e2e are green in CI before merge.


· Branch: feat/fastify ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant