fix(auth): rate limit the password reset endpoints - #6553
Merged
Conversation
`/api/auth/forget-password` calls `auth.api.requestPasswordReset` without headers, which bypasses Better Auth's rate limiter entirely — that limiter lives in the HTTP router, not the endpoint. The route was an unthrottled email-send amplifier keyed on any address a caller chose. Add two dimensions via the existing route-helper family: a per-IP budget before parsing (cheap pre-parse gate), and a per-recipient budget, since no per-IP limit can stop a distributed attempt to bomb one mailbox. The recipient key is normalized and hashed, so the bucket store never holds an address and long inputs cannot inflate key cardinality. It is enforced before any user lookup and identically whether or not the account exists, so a 429 is not an account-existence oracle. Also throttle `/api/auth/reset-password`, which had none and is an online token-guessing surface. Passing headers to `auth.api.*` is deliberately not the fix: Better Auth's limiter throws an APIError that these routes' catch blocks project as a 500, and it cannot express the per-recipient dimension.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryCursor Bugbot is generating a summary for commit d2df8b3. Configure here. |
Contributor
Greptile SummaryAdds rate limiting to the unauthenticated password-reset flows to limit email amplification and reset-token guessing.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/auth/forget-password/route.ts | Adds early per-IP throttling and a normalized, hashed per-recipient budget before requesting a reset email. |
| apps/sim/app/api/auth/reset-password/route.ts | Adds an early endpoint-specific per-IP budget before parsing or consuming a reset token. |
| apps/sim/lib/core/rate-limiter/route-helpers.ts | Introduces a reusable recipient limiter that stores only a normalized email hash in the bucket key. |
| apps/sim/app/api/auth/forget-password/route.test.ts | Tests recipient exhaustion, normalization, hashed keys, and IP-limit short-circuiting. |
| apps/sim/app/api/auth/reset-password/route.test.ts | Tests that exhausted IP budgets return 429 without invoking password reset. |
| apps/sim/lib/core/rate-limiter/index.ts | Exports the new recipient rate-limit helper through the package entry point. |
Sequence Diagram
sequenceDiagram
participant Client
participant Forget as Forget-password route
participant Limiter as Rate limiter
participant Auth as Better Auth
Client->>Forget: POST email
Forget->>Limiter: Check per-IP budget
alt IP budget exhausted
Limiter-->>Client: 429 + Retry-After
else IP budget available
Forget->>Forget: Parse and validate request
Forget->>Limiter: Check hashed recipient budget
alt Recipient budget exhausted
Limiter-->>Client: 429 + Retry-After
else Recipient budget available
Forget->>Auth: Request password-reset email
Auth-->>Client: Success response
end
end
Reviews (2): Last reviewed commit: "fix(auth): rate limit the password reset..." | Re-trigger Greptile
Collaborator
Author
Collaborator
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d2df8b3. Configure here.
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
1 participant
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
/api/auth/forget-passwordcallsauth.api.requestPasswordResetwithout headers, which bypasses Better Auth's rate limiter entirely (it lives in the HTTP router, not the endpoint). The route was an unthrottled email-send amplifier keyed on any address a caller chose/api/auth/reset-password, which had no rate limiting and is an online token-guessing surfaceWhy not just pass headers to
auth.api.*That looks like the deeper fix but regresses behavior: Better Auth's limiter throws an
APIError, which these routes' catch blocks project as a 500 with the raw message, not a 429. It is also per-IP only, so it cannot express the per-recipient dimension that mailbox bombing actually requires.internalRateLimitsalso cannot cover these routes — itsenforceruns afterauthenticateand takes aPrincipal, anddefineInternalJsonRoutehas no unauthenticated path. Imperativeenforce*is the established pattern here (19 route files already use it; zero use a declarative throttling policy, because none exists).Type of Change
Testing
bun run type-checkclean; 230 tests across 16 files inapp/api/auth+lib/core/rate-limiterpass;bun run lintandcheck:api-validationclean. Both new rate-limit tests verified to fail when their guard is stubbed out.Follow-up (not in this PR)
rate_limit_buckethas no TTL or cleanup job on the Postgres adapter, so the recipient dimension is attacker-controlled key space that grows unbounded on Postgres-backed deployments. Redis-backed deployments are unaffected (redis-token-bucket.ts:41setsEXPIRE).app/api/contact,app/api/demo-requests, andapp/api/help/integration-requeststill hand-roll whatroute-helpersdoes, including a byte-identical copy ofDEFAULT_PUBLIC_IP_ROUTE_LIMIT. Unifying them changes the user-visible 429 string on three public forms, so it is left out of this change.Checklist