conceptadev / conceptadev/rockets
audit: opt-in requireRateLimitGuard — @RateLimit with no guard reaching the route is silently unlimited
- Dominant language
- TypeScript
- Stars
- 1
- Forks
- 2
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 23
Description
### Use case
`@RateLimit(...)` on a route that no `RateLimitGuard` ever reaches is
silently unlimited. The decorator only writes metadata; the guard is what
enforces it, and nothing checks that the two meet.
Every `@RateLimit` site in this repo pairs with `@UseGuards(RateLimitGuard)`
**by convention**. A consumer — or a future controller here — that writes the
decorator and forgets the guard gets a route that looks rate-limited in the
source, documents nothing, and enforces nothing. The same is true of the
reverse shape already covered elsewhere: `@Body({ schema })` with no pipe
reaching it fails the boot under `requireSchemaPipe`, and `@AuthSession()`
with no CSRF guard fails under `requireCsrf`. Rate limiting is the member of
that family with no check.
Related but separate: `@RateLimit({})` — an empty policy — in an app that
registers no `RATE_LIMIT_DEFAULTS_TOKEN` produces zero dimensions, and the
guard returns `true`. Four auth controllers use the empty policy, so this is
the shape a consumer is most likely to copy. Documented in `CONFIGURATION.md`
§7d as of #105; a check would be better than a paragraph.
Both are **fail-open**: the route serves unlimited traffic and nothing says
so. That is why they are worth more than the fail-closed case #105 already
handles (an incomplete dimension throws loudly on the first request).
### Proposal
Add an **opt-in** policy rule — `requireRateLimitGuard` — to
`RouteAuditService`, alongside `requireCsrf`:
- A route carrying `ROCKETS_RATE_LIMIT_TOKEN` metadata (handler or class) with
no recognised limiter guard reaching it is a violation.
- Guards are read the way the audit already reads them: `GUARDS_METADATA` on
the class and handler plus `ApplicationConfig.getGlobalGuards()`. This needs
no DI value, so it does not have the scoping problem that made the
boot-time *dimension completeness* check unsound (`CONFIGURATION.md` §7d
records that decision) — guard identity is static, unlike
`RATE_LIMIT_DEFAULTS_TOKEN`, which the guard resolves per declaring module.
- Recognition list `rateLimitGuards?: Type[]`, same shape as the existing
`csrfGuards`, so an app whose limiter is its own guard class can name it.
- **Opt-in, not always-on**: a consumer's own limiter would otherwise
false-positive, and this repo has already had to fix one audit rule that
aborted correct apps.
Optionally, a second rule for the empty-policy case: a route whose only
`@RateLimit` is `{}` in an app whose `RATE_LIMIT_DEFAULTS_TOKEN` registers no
dimensions. That one does read a DI value and therefore inherits the scoping
problem above, so it should be evaluated separately rather than bundled in.
### Out of scope
- Boot-time checking of dimension completeness — considered and rejected in
#105, reasons in `CONFIGURATION.md` §7d.
- Anything on #105 itself.
Contributor guide
Research direction
Start in RouteAuditService by tracing the existing requireCsrf audit and how it reads GUARDS_METADATA and ApplicationConfig.getGlobalGuards(). Review CONFIGURATION.md §7d and #105 for the rejected dimension-completeness approach. Done means an opt-in requireRateLimitGuard rule recognizes configured rate-limit guards and reports routes carrying rate-limit metadata when none reaches them.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend-api-design, security
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100