Skip to content

feat(rate-limit): implement three-tier rate limiting with burst limits - #162

Open
samjay8 wants to merge 7 commits into
ASTROIDX556:mainfrom
samjay8:fix/issue-78
Open

feat(rate-limit): implement three-tier rate limiting with burst limits#162
samjay8 wants to merge 7 commits into
ASTROIDX556:mainfrom
samjay8:fix/issue-78

Conversation

@samjay8

@samjay8 samjay8 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Closes #78

Summary

  • Added a third rate-limit tier webhook alongside the existing api and auth tiers, with its own configurable limit (default 30 req/min) and burst ceiling (default 5 req/sec)
  • Added per-tier burst rate limiting — a per-second spike ceiling that rejects requests immediately when exceeded, independent of the steady-state per-minute limit
  • Added rate-limit response headers (X-RateLimit-Limit, X-RateLimit-Reset, Retry-After) from the NestJS ThrottlerGuard, so clients can see their remaining budget
  • Applied @ThrottleTierDecorator('webhook') to WebhookController to scope webhook management endpoints to the webhook tier

Scope

Does not touch the SlidingWindowThrottlerGuard (Redis-backed sliding window), the sliding-window spec, or any domain modules beyond the webhook controller tier annotation.

Testing

  • npm run typecheck (type safety)
  • npm run lint (no new lint errors)
  • npm test (new throttler guard spec + existing sliding-window spec)
  • npm run build (successful build)

Files changed

  • src/common/decorators/throttle-tier.decorator.ts — Extended ThrottleTier union to include 'webhook'
  • src/common/guards/throttler.guard.ts — Added burst limiting, rate-limit headers, and webhook tier support
  • src/common/guards/throttler.guard.spec.ts — New tests for tier routing, burst limits, tracker scoping
  • src/config/env.validation.ts — Added THROTTLE_WEBHOOK_LIMIT, THROTTLE_*_BURST env vars
  • src/config/queue.config.ts — Added webhookLimit, apiBurst, authBurst, webhookBurst to config type
  • src/app.module.ts — Registered the webhook named throttler
  • src/modules/webhooks/webhook.controller.ts — Applied @ThrottleTierDecorator('webhook')
  • .env.example — Documented new rate-limit env vars

- Add 'webhook' tier alongside existing 'api' and 'auth' tiers
- Add per-tier burst rate limiting (per-second spike ceiling)
- Add rate limit response headers (X-RateLimit-Limit, X-RateLimit-Reset,
  Retry-After) from the NestJS ThrottlerGuard
- Configure webhook tier limits via THROTTLE_WEBHOOK_LIMIT env var
- Configure burst limits via THROTTLE_API_BURST, THROTTLE_AUTH_BURST,
  THROTTLE_WEBHOOK_BURST env vars
- Apply @ThrottleTierDecorator('webhook') to WebhookController
- Add unit tests for tier routing, burst limiting, and tracker scoping
@drips-wave

drips-wave Bot commented Aug 30, 2026

Copy link
Copy Markdown

@samjay8 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@mergekeeper

mergekeeper Bot commented Aug 30, 2026

Copy link
Copy Markdown

Needs changes

AstroidThrottlerGuard hardcodes burst limit defaults instead of reading the injected ConfigService values.

  • src/common/guards/throttler.guard.ts:138: getBurstLimit uses hardcoded defaults (api: 10, auth: 3, webhook: 5) instead of reading throttle burst configuration from the injected ConfigService.

Reviewed commit: f594be1d27cd9cc3601c34ed7e6af08117bfa392.

@mergekeeper mergekeeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs changes

AstroidThrottlerGuard hardcodes burst limit defaults instead of reading the injected ConfigService values.

  • src/common/guards/throttler.guard.ts:138: getBurstLimit uses hardcoded defaults (api: 10, auth: 3, webhook: 5) instead of reading throttle burst configuration from the injected ConfigService.

Reviewed commit: f594be1d27cd9cc3601c34ed7e6af08117bfa392.

@mergekeeper

mergekeeper Bot commented Aug 30, 2026

Copy link
Copy Markdown

MergeKeeper review

Scope: in scope for linked issue #78.
Verdict: clean

Successfully implemented three-tier rate limiting with webhook tier, burst limits, and corresponding tests.

Reviewed commit: 0af4181ce2ae0c5dac056cb99df31e7cbc8b7723.
CI and merge eligibility are checked separately.

@mergekeeper

mergekeeper Bot commented Aug 30, 2026

Copy link
Copy Markdown

MergeKeeper merge status

Status: blocked
PR state: open
Mergeability: mergeable
Checked commit: 0af4181ce2ae0c5dac056cb99df31e7cbc8b7723.

Reason:
GitHub pull request merge request failed with 405: 4 of 4 required status checks are expected.

Next steps:
This required status check is enforced directly by the repository's branch rules, independent of MergeKeeper's own CI evaluation — a maintainer merge override cannot bypass it unless MergeKeeper is authorized to.

  1. Wait for the check to pass, or fix the underlying failure.
  2. To let an explicit override bypass this check going forward, add the MergeKeeper GitHub App as a Bypass actor on the ruleset (Settings → Rules → Rulesets → the ruleset → Bypass list).

@mergekeeper mergekeeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

Successfully implemented three-tier rate limiting with webhook tier, burst limits, and corresponding tests.

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.

Implement multi-tier rate limiting strategy using NestJS Throttler

1 participant