fix: make rate-limit windows atomic across instances - #1296
Merged
Baskarayelu merged 1 commit intoAug 30, 2026
Merged
Baskarayelu merged 1 commit into
Baskarayelu merged 1 commit into
Conversation
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.
Summary
503for sensitive configurations, while opt-in fail-open remains available for non-sensitive routes.Acceptance criteria
Security and correctness note
The client-controlled leftmost
X-Forwarded-Forvalue is never used. The limiter uses the socket peer or trusted authenticated identity, and Redis performs the counter decision atomically. A client cannot create a second tenant bucket by changing separators or route text because the complete identity is hashed before key construction.The compatibility command path is retained only for minimal adapters/tests. The production Redis client uses
EVAL; the deployment checklist documents verifying that capability. Sensitive routes should usefailOpen: falseso a dependency outage cannot silently remove abuse protection.Compatibility and rollback
The public middleware factory and legacy
rateLimithelper remain compatible. Existing auth behavior continues to share the tenant bucket across login and refresh. The application API opts into route scope explicitly. Keys use the existing namespace prefixes with a window ID, so old counters expire naturally and no database migration is required. Rollback is schema-safe but may restore the prior weaker race behavior and should be monitored.Validation
npx vitest run tests/routes/rateLimit.test.ts tests/routes/authRateLimit.test.ts tests/routes/rateLimitDistributed.test.ts— 62 passing, 0 failingnpx eslint src/middleware/rateLimit.ts src/app.ts— 0 errors; 4 pre-existing logger-schema warnings insrc/app.tsgit diff --check— passednpm run build— blocked by pre-existing invalid-character errors in unrelatedsrc/db/repositories/auditChainVerificationRepository.tsandsrc/db/repositories/baseRepository.tsRelated issue
Closes #1240