fix(mcp): reject oauth2+user_fields admin config to prevent user deadlock

The exclusivity guard already rejected is_byok+user_fields because both
serialize into the single credential_b64 row of LiteLLM_MCPUserCredentials
and the write paths refuse to overwrite a foreign payload type. The same
mutual exclusion applies to auth_type='oauth2' (interactive) — whichever
credential the user saves first permanently locks the other, and the user
has no way to escape without admin intervention.

Extend the check to also reject auth_type=oauth2 combined with non-empty
user_fields, and rename the helper to reflect that it covers more than
the BYOK case. oauth2_token_exchange remains allowed because its tokens
are cached in-memory (token_exchange.py) and never touch credential_b64.
This commit is contained in:
mateo-berri 2026-05-19 09:41:57 +00:00
parent 56790e43eb
commit b7fd2ec550
No known key found for this signature in database
2 changed files with 68 additions and 12 deletions

View file

@ -216,22 +216,24 @@ if MCP_AVAILABLE:
def validate_and_normalize_mcp_server_payload(payload: Any) -> None:
_base_validate_and_normalize_mcp_server_payload(payload)
_validate_mcp_server_name_fields(payload)
_validate_mcp_user_fields_byok_exclusive(payload)
_validate_mcp_user_fields_exclusive(payload)
def _validate_mcp_user_fields_byok_exclusive(payload: Any) -> None:
"""Reject server configurations that combine is_byok with user_fields.
def _validate_mcp_user_fields_exclusive(payload: Any) -> None:
"""Reject server configurations that combine user_fields with another
credential type that shares the same per-user storage row.
Both credential types share the same (user_id, server_id) row in
LiteLLM_MCPUserCredentials, and the store paths refuse to overwrite
the other type — so a user can save BYOK or user-fields for a given
server, never both. Allowing this combination at admin time would
trap end-users in an unresolvable state: every tool call would 401
on whichever check the user has not (and cannot) satisfy.
BYOK strings, OAuth2 access tokens, and user_fields blobs all encode
into the single ``credential_b64`` column of LiteLLM_MCPUserCredentials,
and the write paths refuse to overwrite a different type. Mixing
``is_byok=True`` or ``auth_type=oauth2`` with non-empty ``user_fields``
would let whichever credential the user saves first permanently block
the other — every tool call would 401 on the unsatisfied check, and
the user has no path to resolve it without admin intervention.
"""
if not getattr(payload, "is_byok", False):
return
user_fields = getattr(payload, "user_fields", None) or []
if user_fields:
if not user_fields:
return
if getattr(payload, "is_byok", False):
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={
@ -243,6 +245,20 @@ if MCP_AVAILABLE:
)
},
)
if getattr(payload, "auth_type", None) == MCPAuth.oauth2:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={
"error": (
"MCP servers cannot combine auth_type='oauth2' with "
"user_fields. The interactive OAuth2 token and "
"user_fields share the same per-user credential row, "
"so saving one would block saving the other. Use "
"user_fields for per-user secrets or oauth2 for the "
"primary upstream auth, not both."
)
},
)
_VALID_MCP_REQUIRED_FIELDS: frozenset = frozenset(NewMCPServerRequest.model_fields)

View file

@ -2468,6 +2468,46 @@ class TestManagementPayloadValidation:
assert payload.alias == "valid_server"
def test_rejects_is_byok_with_user_fields(self):
payload = SimpleNamespace(
server_name="valid_server",
alias=None,
is_byok=True,
user_fields=[{"field_key": "TOKEN", "header_name": "Authorization"}],
)
with pytest.raises(HTTPException) as exc_info:
mgmt_endpoints.validate_and_normalize_mcp_server_payload(payload)
assert exc_info.value.status_code == 400
assert "is_byok and user_fields" in exc_info.value.detail["error"]
def test_rejects_oauth2_with_user_fields(self):
payload = SimpleNamespace(
server_name="valid_server",
alias=None,
is_byok=False,
auth_type=MCPAuth.oauth2,
user_fields=[{"field_key": "WORKSPACE", "header_name": "X-Workspace"}],
)
with pytest.raises(HTTPException) as exc_info:
mgmt_endpoints.validate_and_normalize_mcp_server_payload(payload)
assert exc_info.value.status_code == 400
assert "oauth2" in exc_info.value.detail["error"]
def test_accepts_oauth2_token_exchange_with_user_fields(self):
payload = SimpleNamespace(
server_name="valid_server",
alias=None,
is_byok=False,
auth_type=MCPAuth.oauth2_token_exchange,
user_fields=[{"field_key": "WORKSPACE", "header_name": "X-Workspace"}],
)
mgmt_endpoints.validate_and_normalize_mcp_server_payload(payload)
@pytest.mark.asyncio
async def test_health_check_view_all_mode(self):
"""view_all mode should return health info for all MCP servers."""