google-gemini / google-gemini/gemini-cli
bug: concurrent MCP OAuth refreshes race, and any refresh failure deletes valid credentials (forced re-auth)
- Dominant language
- TypeScript
- Stars
- 107k
- Forks
- 14.6k
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 45
Description
## What happened?
Two related defects in `MCPOAuthTokenStorage`-backed refresh flow of `MCPOAuthProvider`:
1. **No concurrency control around token refresh.** Two MCP tool calls hitting an expired access token concurrently both read the same refresh token and both POST to the token endpoint. Servers that rotate refresh tokens (standard per RFC 6749 §10.4 / OAuth BCP) invalidate the old refresh token on first use — the loser gets `invalid_grant`, enters the catch, and calls `deleteCredentials(serverName)`, **deleting the fresh valid credentials just saved by the winner**. Result: forced full browser re-authentication despite a successful refresh.
2. **Any error deletes stored credentials.** The same catch wipes credentials for *every* failure — including transient network errors on the refresh POST. An offline blip at expiry time permanently logs the user out even though the refresh token was perfectly valid.
## Affected code
`packages/core/src/mcp/oauth-provider.ts:574-617` (`getValidToken`):
```ts
const newTokenResponse = await this.refreshAccessToken(
{ ...config, clientId }, token.refreshToken, ...);
// ...
await this.tokenStorage.saveToken(serverName, newToken, ...);
return newToken.accessToken;
} catch (error) {
coreEvents.emitFeedback('error', 'Failed to refresh auth token.', error);
// Remove invalid token
await this.tokenStorage.deleteCredentials(serverName); // ANY error -> wipe
}
```
There is no mutex / in-flight-promise dedup / re-read-after-refresh anywhere; the underlying storage is plain read-modify-write per operation.
## How can this be reproduced?
1. Connect an MCP server that rotates refresh tokens; let the access token expire.
2. Fire two tool calls concurrently.
3. Observe two refresh POSTs; the second fails `invalid_grant`; stored credentials are deleted; next call demands interactive login.
(Or simpler: kill network connectivity during a single refresh → credentials wiped.)
## What did you expect to happen?
- Single-flight refresh (share one in-flight promise across callers).
- On refresh failure, distinguish `invalid_grant` (credentials truly dead → delete) from transient errors (keep credentials, surface retryable error).
## Impact
Forced re-authentication loops for MCP servers; worst case repeated browser popups in automation contexts.
---
*Found by source audit on current `main` (commit `5411f113c`); platform-independent. No open issue/PR covering this was found (searched: MCP OAuth refresh token deleted).*
Contributor guide
Research direction
Start in packages/core/src/mcp/oauth-provider.ts:574-617, especially getValidToken, then inspect refreshAccessToken and the MCPOAuthTokenStorage operations it uses. Reproduce the concurrent expired-token case and a network failure, then verify that refreshes are single-flight, invalid_grant removes dead credentials, and transient failures preserve valid credentials.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- authentication, cli
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100