feat(auth): add login rate limiting to prevent brute-force attacks#977
Merged
feat(auth): add login rate limiting to prevent brute-force attacks#977
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
nhopeatall
approved these changes
Mar 22, 2026
Collaborator
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
LGTM — well-implemented in-memory rate limiter with correct logic, good test coverage, and clean integration into the login handler.
Code Issues
Should Fix
- src/api/auth/login.ts:26 — The
'unknown'sentinel fallback means every client that connects without a reverse proxy (nox-forwarded-for) shares a single rate-limit bucket. A burst of unauthenticated requests from any direct-connect IP would lock out all other direct-connect users. Hono's Node.js adapter shipsgetConnInfofrom@hono/node-server/conninfowhich returns the TCP remote address — using it as a fallback before'unknown'would give real per-client rate limiting in non-proxied environments (dev, Docker direct-connect) without affecting the proxy path.
Notes (non-blocking)
- The design choice to reset the counter on successful login is a reasonable tradeoff: it favors legitimate users at the cost of a small brute-force window for attackers sharing a NAT IP. This is well-documented and standard practice.
- Deletion during
Mapiteration in the cleanup sweep is safe per the JS spec — verified. - The count logic is correct: 10 attempts are allowed (calls 1-10), and the 11th triggers the block — matches
MAX_ATTEMPTS = 10.
🕵️ claude-code · claude-opus-4-6 · run details
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
src/api/auth/rateLimiter.ts) that tracks login attempts per IP addresssrc/api/auth/login.ts) — checks before any DB/bcrypt work, resets counter on successRetry-Afterheader after 10 failed attempts from the same IP within 1 minute.unref()'d) evicts expired entries to prevent memory leaksTechnical Details
x-forwarded-forheader (first value for comma-separated lists), falls back to'unknown'recordSuccessfulLogin) so they are not counted against the limitsetIntervalevery 5 minutes sweeps expired entries; timer is.unref()'d so it won't prevent process exitTest plan
tests/unit/api/auth/rateLimiter.test.ts— 14 tests covering: under limit, at limit, over limit, window reset, successful-login reset, cleanup sweeptests/unit/api/auth/login.test.ts— extended with 7 new rate-limiting tests: 429 response, Retry-After header, IP extraction fromx-forwarded-for, comma-separated list,recordSuccessfulLogincalled/not-calledbiome) passes with zero errorsRelated
Trello: https://trello.com/c/8y9Sd1Xa/498-as-a-developer-i-want-login-rate-limiting-so-that-brute-force-attacks-against-the-dashboard-are-prevented
Card ID: 69c00fbbb87d445a13b03294
🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details