fix: address 3/5 Greptile issues: double-error toast, cache sentinel inconsistency, vestigial master_key check

- Remove handleError() from getMcpOAuth2ConnectUrl: caller (OAuth2ConnectButton)
  already calls setError(), so double notification is now eliminated
- Fix _check_byok_credential to treat "" sentinel as cache miss: consistent with
  _get_byok_credential, preventing a race where deleted credentials pass auth check
  but return None from token lookup, causing silent 401 to backend
- Remove vestigial master_key import/check from openapi_oauth2_connect: state
  tokens are now pure random (secrets.token_urlsafe), prisma_client check already
  rejects unconfigured deployments
This commit is contained in:
Ishaan Jaffer 2026-03-07 13:12:20 -08:00
parent ad8898b8d3
commit 493ff1a6d1
3 changed files with 11 additions and 6 deletions

View file

@ -167,8 +167,6 @@ async def openapi_oauth2_connect(
request: Request,
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
) -> JSONResponse:
from litellm.proxy.proxy_server import master_key
server = global_mcp_server_manager.get_mcp_server_by_id(server_id)
if server is None:
raise HTTPException(status_code=404, detail=f"MCP server '{server_id}' not found")
@ -193,8 +191,6 @@ async def openapi_oauth2_connect(
status_code=400,
detail=f"Server '{server_id}' has no client_secret configured",
)
if master_key is None:
raise HTTPException(status_code=500, detail="Master key not configured")
# Fail early if the DB is unavailable: without it the callback cannot store
# the token, so sending the user through the provider consent flow is wasted effort.

View file

@ -1654,6 +1654,11 @@ if MCP_AVAILABLE:
)
# Check shared credential cache before hitting the DB.
# Note: the status endpoint writes "" as a sentinel for "connected per latest
# status poll". We treat "" identically to _get_byok_credential (cache miss)
# so that both functions always verify from DB when only the sentinel is present.
# This prevents a race where a deleted credential passes the auth check but
# returns None from _get_byok_credential, causing a silent 401 to the backend.
cache_key = (user_id, mcp_server.server_id)
cached = _byok_cred_cache.get(cache_key)
if cached is not None:
@ -1675,7 +1680,10 @@ if MCP_AVAILABLE:
"WWW-Authenticate": 'Bearer resource_metadata="/.well-known/oauth-protected-resource"'
},
)
return
# Only return early for real cached credentials; treat "" as a miss
# so we always verify against the DB when only the status sentinel exists.
if cached_cred:
return
from litellm.proxy._experimental.mcp_server.db import get_user_credential
from litellm.proxy.proxy_server import prisma_client

View file

@ -6414,7 +6414,8 @@ export const getMcpOAuth2ConnectUrl = async (
if (!response.ok) {
const errorData = await response.json().catch(() => ({}));
const errorMessage = deriveErrorMessage(errorData);
handleError(errorMessage);
// Don't call handleError here: the caller (OAuth2ConnectButton) already
// renders an inline error via setError(), avoiding duplicate notifications.
throw new Error(errorMessage);
}