diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 285e7b26104..0462e76c2ad 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -118,6 +118,7 @@ from litellm.proxy._experimental.mcp_server.utils import ( is_short_mcp_tool_prefix_enabled, iter_known_server_prefixes, iter_known_tool_name_spellings, + match_known_tool_name, match_known_server_prefix, merge_mcp_headers, normalize_server_name, @@ -4262,26 +4263,19 @@ class MCPServerManager: """ Check if the tool is allowed or banned for the given server. - ``tool_name`` is bare: every caller resolves the boundary against the - server's registered prefixes before dispatch (``server.py``'s - ``original_tool_name``, the Responses handler's ``sanitized_tool_name``). - Stored entries are matched by deriving the spellings routing accepts - rather than by stripping the entries, which would cut a second boundary - out of a native name that itself opens with the server prefix. + ``tool_name`` is bare: every caller resolves the boundary against the server's + registered prefixes before dispatch (``server.py``'s ``original_tool_name``, the + Responses handler's ``sanitized_tool_name``). Configured entries are matched by + deriving the spellings routing accepts, never by stripping the entry, which would + cut a second boundary out of a native name that opens with the server prefix. """ from litellm.proxy._experimental.mcp_server.utils import ( server_applies_tool_allowlist, ) - spellings = tuple(iter_known_tool_name_spellings(tool_name, server)) - if server_applies_tool_allowlist(server): - if not server.allowed_tools: - return False - return any(spelling in server.allowed_tools for spelling in spellings) - if server.disallowed_tools: - return all(spelling not in server.disallowed_tools for spelling in spellings) - return True + return match_known_tool_name(tool_name, server, server.allowed_tools or ()) is not None + return match_known_tool_name(tool_name, server, server.disallowed_tools or ()) is None def validate_allowed_params(self, tool_name: str, arguments: dict[str, Any], server: MCPServer) -> None: """ @@ -4299,18 +4293,12 @@ class MCPServerManager: Raises: HTTPException: If allowed_params is configured for this tool but arguments contain disallowed params """ - # If no allowed_params configured, return all arguments - if not server.allowed_params: + allowed_params = server.allowed_params or {} + matched = match_known_tool_name(tool_name, server, allowed_params) + if matched is None: return - spellings = iter_known_tool_name_spellings(tool_name, server) - allowed_params_list = next( - (server.allowed_params[name] for name in spellings if name in server.allowed_params), None - ) - - # If this tool doesn't have allowed_params specified, allow all params - if allowed_params_list is None: - return None + allowed_params_list = allowed_params[matched] # Filter arguments to only include allowed parameters disallowed_params = [param for param in arguments.keys() if param not in allowed_params_list] diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index 5181f3a2676..2c774224098 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -65,7 +65,7 @@ from litellm.proxy._experimental.mcp_server.utils import ( extract_mcp_tool_result_error_message, get_server_prefix, iter_known_server_prefixes, - iter_known_tool_name_spellings, + match_known_tool_name, ) from litellm.proxy._types import ( ProxyException, @@ -1421,28 +1421,13 @@ if MCP_AVAILABLE: """ Check if a tool name matches any name in the filter list. - Matches via the same ``iter_known_tool_name_spellings`` the server-level - permission checks use, so discovery hides exactly what dispatch refuses; - covering fewer spellings here leaves a blocked tool advertised in - ``tools/list``. Comparison is case-insensitive to handle OpenAPI - operationIds that may be in camelCase. - - Args: - tool_name: The tool name to check (may be prefixed like "server-tool_name") - filter_list: List of tool names to match against - mcp_server: The server the tool belongs to, whose registered prefixes - locate the boundary exactly. Required: guessing the boundary at - the first separator silently mismatches every tool on a server - whose prefix contains the separator. - - Returns: - True if any spelling of the tool name is in the filter list + Reads the same owner the server-level permission checks use, so discovery hides + exactly what dispatch refuses. ``mcp_server`` is required: guessing the boundary + at the first separator mismatches every tool on a server whose prefix contains + the separator. """ - filter_list_lower = {f.lower() for f in filter_list} bare_name = strip_known_server_prefix(tool_name, mcp_server) - spellings = (tool_name, *iter_known_tool_name_spellings(bare_name, mcp_server)) - - return any(spelling.lower() in filter_list_lower for spelling in spellings) + return match_known_tool_name(bare_name, mcp_server, filter_list) is not None def filter_tools_by_allowed_tools( tools: list[MCPTool], diff --git a/litellm/proxy/_experimental/mcp_server/utils.py b/litellm/proxy/_experimental/mcp_server/utils.py index e8f8c08188e..900d6259a5a 100644 --- a/litellm/proxy/_experimental/mcp_server/utils.py +++ b/litellm/proxy/_experimental/mcp_server/utils.py @@ -23,6 +23,8 @@ import importlib import os from urllib.parse import quote +from litellm.types.mcp_server.mcp_server_manager import MCPServer + # Constants # # NOTE: The environment-backed values below are read once, when this module is @@ -326,24 +328,31 @@ def iter_known_server_prefixes(server: Any) -> Iterator[str]: yield from _emit(server_id) -def iter_known_tool_name_spellings(tool_name: str, server: Any) -> Iterator[str]: - """Yield every name that denotes the bare ``tool_name`` on ``server``. - - The bare name, then its wire spelling under each prefix from - ``iter_known_server_prefixes``. Routing resolves an inbound name against that - whole set, so anything keyed by tool name (the routing map, the allow/deny - lists, ``allowed_params``) must cover it too or it answers for fewer names - than are reachable, which fails open on ``disallowed_tools``. - ``get_server_prefix`` alone covers only the published spelling, and that moves - with the alias and with ``LITELLM_USE_SHORT_MCP_TOOL_PREFIX``. These are - spellings of one tool on one server, so honoring all of them normalizes the - entry rather than widening a grant. +def iter_known_tool_name_spellings(tool_name: str, server: MCPServer) -> Iterator[str]: + """Yield every name that denotes the bare ``tool_name`` on ``server``: the bare name, + then its wire spelling under each prefix ``iter_known_server_prefixes`` accepts. + ``get_server_prefix`` covers only the currently published one, and that moves with the + alias and with ``LITELLM_USE_SHORT_MCP_TOOL_PREFIX``. """ yield tool_name for prefix in iter_known_server_prefixes(server): yield add_server_prefix_to_name(tool_name, prefix) +def match_known_tool_name(tool_name: str, server: MCPServer, names: Iterable[str]) -> str | None: + """Return the entry of ``names`` that denotes ``tool_name`` on ``server``, else ``None``. + + The single question every tool-name-keyed site asks: the allow list, the deny list, + ``allowed_params`` and the discovery filter. Matching spans every spelling routing + accepts and ignores case, so discovery hides exactly what dispatch refuses. Callers + read the returned entry rather than testing a container's values, which is what stops + an explicitly empty ``allowed_params`` list from reading as "nothing configured". + """ + entries = {name.casefold(): name for name in names} + spellings = map(str.casefold, iter_known_tool_name_spellings(tool_name, server)) + return next((entries[spelling] for spelling in spellings if spelling in entries), None) + + def split_server_prefix_from_name(prefixed_name: str) -> Tuple[str, str]: """Return the unprefixed name plus the server name used as prefix. 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 a3577c85621..5115fde2687 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 @@ -8146,10 +8146,11 @@ class TestListFiltersHonorThePrefixBoundary: published = MCPTool(name="eiG-read_wiki_contents", description="", inputSchema={"type": "object"}) for spelling in registered: - server = _server(disallowed_tools=[spelling]) + for entry in (spelling, spelling.upper()): + server = _server(disallowed_tools=[entry]) - refused = not manager.check_allowed_or_banned_tools("read_wiki_contents", server) - hidden = filter_tools_by_allowed_tools([published], server) == [] + refused = not manager.check_allowed_or_banned_tools("read_wiki_contents", server) + hidden = filter_tools_by_allowed_tools([published], server) == [] - assert refused, spelling - assert hidden, spelling + assert refused, entry + assert hidden, entry diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py index 2977f13caf1..134a5d89225 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py @@ -9511,6 +9511,21 @@ class TestServerToolListsHonorThePrefixBoundary: assert exc_info.value.status_code == 403 assert "include_internal" in exc_info.value.detail["error"] + @pytest.mark.asyncio + async def test_a_case_variant_blocklist_entry_still_blocks(self): + server = self._aliased_server(disallowed_tools=["PetStore-DeletePet"]) + + with pytest.raises(HTTPException) as exc_info: + await self._run_check(server, "deletepet") + + assert exc_info.value.status_code == 403 + + @pytest.mark.asyncio + async def test_a_case_variant_allowlist_entry_grants_the_tool(self): + server = self._aliased_server(allowed_tools=["PetStore-GetPetById"]) + + await self._run_check(server, "getpetbyid") + @pytest.mark.asyncio async def test_an_explicitly_empty_allowed_params_list_refuses_every_parameter(self): server = self._alias_less_server(allowed_params={"read_wiki_contents": []})