feat(security): add rate limiting and transport hardening - #3
Open
fondomp wants to merge 1 commit into
Open
Conversation
Harden the API before it handles real traffic. Every protection is applied in one place (configure-security.ts) that both main.ts and the e2e suite use, so the tests exercise the real configuration. Rate limiting - Three stacked windows (1s/10s/60s) enforced on every route via a global guard. One window cannot be both tight enough to stop a burst and loose enough to allow normal sustained traffic. - ProxyAwareThrottlerGuard normalizes the tracker (IPv6-mapped IPv4 is collapsed so one client cannot occupy two buckets), emits a canonical Retry-After (the stock guard only emits the suffixed variants when throttlers are named), and logs rejections. - TRUST_PROXY_HOPS defaults to 0, so X-Forwarded-For is ignored unless a proxy is declared. Getting this wrong allows rate-limit bypass, so both behaviours are covered by e2e tests. Transport security - Helmet with an API-appropriate CSP (every fetch directive denied), nosniff, frame-ancestors none, no-referrer and HSTS. - CORS disabled unless CORS_ORIGINS declares an exact allowlist; a wildcard combined with credentials throws at boot. - Body parsers capped at BODY_LIMIT (default 100kb). - Global ValidationPipe strips and then rejects unknown properties. - AllExceptionsFilter returns a fixed envelope and never leaks stack traces or driver messages. Client-error statuses raised by Express middleware are preserved but their messages are replaced. - Validated X-Request-Id on every request; client values are reused only when they match a UUID shape. Configuration - All settings are environment variables validated at startup, so an invalid security knob aborts the boot instead of degrading silently. A blank value is treated as unset. Tests: 124 unit + 22 e2e. Coverage 100% statements / 93% branches over non-wiring code, with an 85% threshold now enforced in CI. Known limitation: the throttler uses in-memory storage, so limits are per replica. docs/security.md documents the shared-storage migration path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Hardens the API before it handles real traffic: rate limiting plus a transport-security layer (Helmet, CORS, payload limits, input validation, non-leaking errors).
Every protection is applied in one place —
configure-security.ts— which bothmain.tsand the e2e suite call, so the tests exercise the real configuration rather than an approximation of it.Rate limiting
Three stacked windows, all of which must pass:
shortmediumlongA single window can either be tight enough to stop a burst or loose enough to allow normal sustained traffic, but not both.
ProxyAwareThrottlerGuardis registered as anAPP_GUARD(every route by default; opt out with@SkipThrottle(), tighten with@Throttle()). It adds three things the stock guard lacks:::ffff:1.2.3.4) is collapsed so one client cannot occupy two buckets;Retry-Afterheader — the base guard only emits the suffixedRetry-After-<window>variants when throttlers are named;warnlog per rejection, so abuse is visible.Transport security
'none'),nosniff,frame-ancestors 'none',no-referrer, HSTS,X-Powered-Byremoved.CORS_ORIGINSdeclares an exact allowlist.*with credentials throws at boot; browsers reject that pairing anyway, and failing early beats debugging silently dropped credentialed requests.BODY_LIMIT(default100kb); oversized payloads get413.ValidationPipewithwhitelist+forbidNonWhitelisted, so mass-assignment and unexpected-field attacks stop at the boundary.AllExceptionsFilterreturns a fixed envelope and never leaks stack traces, ORM errors or driver messages. Client-error statuses raised by Express middleware (body parser, CORS) are preserved, but the library's own message is replaced with the standard reason phrase; a library5xxis reported as a plain500.X-Request-Idon every request. A client value is reused only when it matches a UUID shape; an unvalidated header would be echoed into responses and log lines, which is a log-injection path.Configuration
All settings are environment variables validated at startup by
env.validation.ts— an invalid security knob aborts the boot instead of degrading silently. A blank value is treated as unset, so the declared default applies. See.env.example.Validation runs in a provider factory rather than
ConfigModule.forRoot({ validate }), because the latter executes at module-import time and pins the configuration before the process is set up — which also made it impossible to boot the module twice with different settings in tests. Fail-fast behaviour is unchanged.Two decisions needed per environment
TRUST_PROXY_HOPS(default0). Set it to the real number of proxies in front of the service. Too high and a client can prepend a forgedX-Forwarded-For, get a fresh bucket every request and bypass rate limiting entirely. Both behaviours are covered by e2e tests.docs/security.mddocuments the shared-storage migration path; the options already use the object form, so adding a Redis adapter is a one-line change.Test plan
pnpm run lint— cleanpnpm run build— cleanpnpm run test— 124 unit tests passpnpm run test:e2e— 22 e2e tests passpnpm run coverage— 100% statements / 93% branches over non-wiring code; an 85% threshold is now enforced (main.tsand*.module.tsexcluded as DI wiring, covered by e2e)429withRetry-After, expected security headers present,413on an oversized body, no server errors in the loge2e coverage includes the security headers, rate-limit headers and
429envelope,X-Forwarded-Forspoofing both with and without a declared proxy, CORS on/off/rejected-origin, the body limit, and the404envelope.Notes
@nestjs/throttler,@nestjs/config,helmet,compression,class-validator,class-transformer..env.examplecontains placeholders and comments only.auth-middleware-tsstill needs to be wired before the service handles real data.mkdocs.yml,catalog-info.yaml,publish-techdocs.yml) is intentionally out of scope; the repo is not yet catalogued in kira.docs/security.mdis written to slot straight into adocs/tree when it is.