PR-1: OAuth/session security hardening (P0)#230
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| if (expiredAccessToken || staleAlias || expiredRefreshToken) { | ||
| this.sessions.delete(token); | ||
| } | ||
| } |
There was a problem hiding this comment.
Cleanup deletes sessions with valid refresh tokens prematurely
High Severity
cleanupExpiredState deletes sessions when expiredAccessToken is true, even if the refresh token is still valid. Since handleRefreshTokenGrant calls cleanupExpiredState() as its first action, a client trying to refresh after access-token expiry will find its session already garbage-collected and receive invalid_grant. This completely breaks the standard OAuth refresh flow—refresh tokens become unusable the moment the access token expires, despite potentially having hours of validity remaining. The same deletion occurs in validateBearerToken when someone presents an expired access token.
Additional Locations (1)
| private async handleAuthorizationCodeGrant( | ||
| params: TokenRequest, | ||
| ): Promise<TokenResponse> { | ||
| this.cleanupExpiredState(); |
There was a problem hiding this comment.
Redundant double cleanup on every token request
Low Severity
handleTokenRequest calls cleanupExpiredState(), then dispatches to handleAuthorizationCodeGrant, handleRefreshTokenGrant, or handleClientCredentialsGrant, each of which also calls cleanupExpiredState() as its first action. This results in every token request performing the full cleanup scan (iterating all sessions and authorization codes) twice with no state change in between, adding unnecessary overhead.
Additional Locations (2)
1 participant
Summary
Files
Gate
Note
Medium Risk
Touches authentication/token issuance and validation logic, so mistakes could cause unintended logouts or refresh failures. Changes are localized and covered by new unit tests plus configurable limits to reduce operational risk.
Overview
OAuth token/session hardening:
OAuth2Servernow tracksissuedAtand optionalrefreshTokenExpiresAt, enforces refresh-token expiry, and invalidates old access tokens on refresh by revoking all sessions tied to the refresh token and deleting stale aliases during validation/cleanup.Memory/state bounding: Adds periodic expiry sweeps plus size caps for in-memory OAuth sessions and authorization codes (configurable via new env vars in
mcp-streamable-http.ts), and makes the streamable HTTP in-memory event store bounded by TTL and max events. Adds tests covering refresh rotation invalidation and refresh-token expiry rejection.Written by Cursor Bugbot for commit 5ab4299. This will update automatically on new commits. Configure here.