From 0d6948241e97fbd9e30743174d5f6f6799f7897c Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Wed, 2 Sep 2026 17:35:51 -0700 Subject: [PATCH] fix(mcp): attribute the agent veto for differently cased server names --- .../proxy/_experimental/mcp_server/server.py | 27 +++++------ .../mcp_server/test_mcp_server.py | 45 +++++++++++++++---- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index 1100b986f15..2f6a05481e3 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -1414,13 +1414,10 @@ if MCP_AVAILABLE: server_name_matched = False for server in allowed_mcp_servers: - if server: - match_list = [s.lower() for s in iter_known_server_prefixes(server) if s] - - if server_or_group.lower() in match_list: - filtered_server[server.server_id] = server - server_name_matched = True - break + if server and _server_answers_to(server, server_or_group): + filtered_server[server.server_id] = server + server_name_matched = True + break if not server_name_matched: try: @@ -1450,6 +1447,10 @@ if MCP_AVAILABLE: return allowed_mcp_servers + def _server_answers_to(server: MCPServer, name: str) -> bool: + requested: Final = name.lower() + return any(requested == known.lower() for known in iter_known_server_prefixes(server) if known) + class _McpDeniedDetail(TypedDict): error: ReadOnly[str] @@ -1470,15 +1471,11 @@ if MCP_AVAILABLE: mcp_servers=requested_names, client_ip=client_ip, ) - resolved_ids: Final = frozenset(server.server_id for server in resolved_without_agent) - def _registered_server_id(name: str) -> str | None: - server: Final = global_mcp_server_manager.get_mcp_server_by_name(name, client_ip=client_ip) - return server.server_id if server is not None else None + def _resolved_to_server(name: str) -> bool: + return any(_server_answers_to(server, name) for server in resolved_without_agent) - vetoed_server: Final = next( - (name for name in requested_names if _registered_server_id(name) in resolved_ids), None - ) + vetoed_server: Final = next((name for name in requested_names if _resolved_to_server(name)), None) if vetoed_server is not None: agent_denial: Final[_McpDeniedDetail] = { "error": ( @@ -1493,7 +1490,7 @@ if MCP_AVAILABLE: ( name for name in requested_names - if _registered_server_id(name) is None + if not _resolved_to_server(name) and any(name in (server.access_groups or ()) for server in resolved_without_agent) ), None, diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py index 52b8fa3d849..db911364ef3 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py @@ -2,6 +2,7 @@ import asyncio import contextvars import os from datetime import datetime, timedelta +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -1336,12 +1337,19 @@ def _denied_scope_manager(known_server_names_to_ids: dict[str, str]) -> MagicMoc return manager -def _scope_resolver(resolved_without_agent: list[str], access_groups: tuple[str, ...] = ()) -> AsyncMock: +def _scope_resolver(resolved_without_agent: dict[str, str], access_groups: tuple[str, ...] = ()) -> AsyncMock: async def resolve(user_api_key_auth, mcp_servers, client_ip=None): if user_api_key_auth is not None and user_api_key_auth.agent_id: return [] return [ - MagicMock(server_id=server_id, access_groups=list(access_groups)) for server_id in resolved_without_agent + SimpleNamespace( + server_id=server_id, + server_name=server_name, + alias=None, + short_prefix=None, + access_groups=list(access_groups), + ) + for server_name, server_id in resolved_without_agent.items() ] return AsyncMock(side_effect=resolve) @@ -1380,7 +1388,7 @@ async def test_scoped_list_denied_by_agent_binding_raises_403_naming_agent(): pytest.importorskip("litellm.proxy._experimental.mcp_server.server") user_api_key_auth = UserAPIKeyAuth(api_key="test_key", user_id="test_user", agent_id="agent-123") - resolver = _scope_resolver(resolved_without_agent=["srv-github"]) + resolver = _scope_resolver(resolved_without_agent={"github": "srv-github"}) denial = await _denied_scoped_list( user_api_key_auth, ["github"], _denied_scope_manager({"github": "srv-github"}), resolver @@ -1452,13 +1460,13 @@ async def test_scoped_list_unknown_name_raises_same_generic_403_as_unauthorized( user_api_key_auth = UserAPIKeyAuth(api_key="test_key", user_id="test_user", agent_id="agent-123") unknown = await _denied_scoped_list( - user_api_key_auth, ["github"], _denied_scope_manager({}), _scope_resolver(resolved_without_agent=[]) + user_api_key_auth, ["github"], _denied_scope_manager({}), _scope_resolver(resolved_without_agent={}) ) unauthorized = await _denied_scoped_list( user_api_key_auth, ["github"], _denied_scope_manager({"github": "srv-github"}), - _scope_resolver(resolved_without_agent=[]), + _scope_resolver(resolved_without_agent={}), ) assert unknown.status_code == unauthorized.status_code == 403 @@ -1479,7 +1487,7 @@ async def test_scoped_list_access_group_vetoed_by_agent_names_agent_and_group(): user_api_key_auth, ["prod-group"], _denied_scope_manager({}), - _scope_resolver(resolved_without_agent=["srv-github"], access_groups=("prod-group",)), + _scope_resolver(resolved_without_agent={"github": "srv-github"}, access_groups=("prod-group",)), ) assert denial.status_code == 403 @@ -1502,7 +1510,7 @@ async def test_scoped_list_mixed_unknown_and_vetoed_group_names_the_group_that_r user_api_key_auth, ["no-such-group", "prod-group"], _denied_scope_manager({}), - _scope_resolver(resolved_without_agent=["srv-github"], access_groups=("prod-group",)), + _scope_resolver(resolved_without_agent={"github": "srv-github"}, access_groups=("prod-group",)), ) assert denial.status_code == 403 @@ -1519,7 +1527,7 @@ async def test_scoped_list_agent_key_denied_by_key_grants_raises_generic_403(): pytest.importorskip("litellm.proxy._experimental.mcp_server.server") user_api_key_auth = UserAPIKeyAuth(api_key="test_key", user_id="test_user", agent_id="agent-123") - resolver = _scope_resolver(resolved_without_agent=[]) + resolver = _scope_resolver(resolved_without_agent={}) denial = await _denied_scoped_list( user_api_key_auth, ["github"], _denied_scope_manager({"github": "srv-github"}), resolver @@ -1532,6 +1540,27 @@ async def test_scoped_list_agent_key_denied_by_key_grants_raises_generic_403(): assert resolver.await_count == 2 +@pytest.mark.asyncio +async def test_scoped_list_agent_veto_attributed_for_differently_cased_server_name(): + """The scope filter matches `/mcp/GitHub` to a server named `github` case-insensitively, so the + agent-attributed 403 must match the same way instead of falling back to the generic denial.""" + pytest.importorskip("litellm.proxy._experimental.mcp_server.server") + + user_api_key_auth = UserAPIKeyAuth(api_key="test_key", user_id="test_user", agent_id="agent-123") + + denial = await _denied_scoped_list( + user_api_key_auth, + ["GitHub"], + _denied_scope_manager({"github": "srv-github"}), + _scope_resolver(resolved_without_agent={"github": "srv-github"}), + ) + + assert denial.status_code == 403 + message = denial.detail["error"] + assert "MCP server 'GitHub'" in message + assert "agent 'agent-123'" in message + + @pytest.mark.asyncio async def test_handle_list_tools_converts_permission_httpexception_to_mcp_error(): """The MCP protocol handler surfaces a permission HTTPException as a clean JSON-RPC error