diff --git a/litellm/proxy/_experimental/mcp_server/utils.py b/litellm/proxy/_experimental/mcp_server/utils.py index 900d6259a5a..4c0690082d5 100644 --- a/litellm/proxy/_experimental/mcp_server/utils.py +++ b/litellm/proxy/_experimental/mcp_server/utils.py @@ -343,14 +343,30 @@ def match_known_tool_name(tool_name: str, server: MCPServer, names: Iterable[str """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". + ``allowed_params`` and the discovery filter, so discovery hides exactly what dispatch + refuses. It spans every spelling routing accepts and no more. A tool's identity is its + exact name, because routing dispatches two names differing only in case as two tools, + and folding case here would let one policy decide both. + + OpenAPI servers are the exception, and not a fuzzy one. ``_register_openapi_tools`` + rewrites every operationId through ``sanitize_openapi_tool_name``, so configuration + holding the spec's own spelling never equals the registered name; replaying that exact + rewrite on the entry recovers the link. It cannot merge identities, because every name + it produces is lowercased, so no two tools on such a server differ only in case. + + 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) + from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import ( + sanitize_openapi_tool_name, + ) + + spellings = set(iter_known_tool_name_spellings(tool_name, server)) + exact = next((name for name in names if name in spellings), None) + if exact is not None or not getattr(server, "spec_path", None): + return exact + sanitized = {sanitize_openapi_tool_name(spelling) for spelling in spellings} + return next((name for name in names if sanitize_openapi_tool_name(name) in sanitized), None) def split_server_prefix_from_name(prefixed_name: str) -> Tuple[str, str]: 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 b2cdcd51294..3a84428add1 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 @@ -4519,6 +4519,7 @@ def test_tool_name_matches_case_insensitive(): server_name="per_store", url="http://127.0.0.1:5115/mcp", transport=MCPTransport.http, + spec_path="/specs/petstore.yaml", ) # Test case 1: Unprefixed tool name with camelCase in filter list @@ -4597,6 +4598,7 @@ def test_filter_tools_by_allowed_tools_case_insensitive(): name="per_store", server_name="per_store", transport=MCPTransport.http, + spec_path="/specs/petstore.yaml", allowed_tools=["addPet", "updatePet", "findPetsByStatus"], ) @@ -8080,12 +8082,19 @@ class TestListFiltersHonorThePrefixBoundary: assert not _tool_name_matches(f"{self.SERVER_ID}-read_wiki_contents", ["read_wiki_structure"], server) - def test_match_is_still_case_insensitive(self): + def test_case_folding_applies_to_openapi_servers_and_not_to_native_ones(self): + # Registration rewrites operationIds through sanitize_openapi_tool_name, so + # folding recovers a spec-spelled entry on an OpenAPI server. A native server + # gets none of it: routing dispatches two names differing only in case as two + # tools, so one policy must not decide both. from litellm.proxy._experimental.mcp_server.server import _tool_name_matches - server = self._alias_less_server() + native = self._alias_less_server() + openapi = self._alias_less_server(spec_path="/specs/petstore.yaml") - assert _tool_name_matches(f"{self.SERVER_ID}-findPetsByStatus", ["findpetsbystatus"], server) + assert _tool_name_matches(f"{self.SERVER_ID}-findPetsByStatus", ["findpetsbystatus"], openapi) + assert not _tool_name_matches(f"{self.SERVER_ID}-findPetsByStatus", ["findpetsbystatus"], native) + assert _tool_name_matches(f"{self.SERVER_ID}-findpetsbystatus", ["findpetsbystatus"], native) def test_alias_form_entry_matches_a_tool_published_under_the_short_prefix(self, monkeypatch): # Routing accepts the alias form, so an entry stored before short @@ -8111,6 +8120,10 @@ class TestListFiltersHonorThePrefixBoundary: A spelling the blocklist enforces but the filter misses leaves a blocked tool advertised; the reverse hides a tool that would have been callable. + Every spelling routing registers bans, and its upper-cased form bans nothing, + because a tool's identity is its exact name; asserting the verdict and not only + the agreement is what keeps this from passing on a matcher that answers wrongly + but consistently. """ from mcp.types import Tool as MCPTool @@ -8146,14 +8159,14 @@ class TestListFiltersHonorThePrefixBoundary: published = MCPTool(name="eiG-read_wiki_contents", description="", inputSchema={"type": "object"}) for spelling in registered: - for entry in (spelling, spelling.upper()): + for entry, expected in ((spelling, True), (spelling.upper(), False)): 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) == [] - assert refused, entry - assert hidden, entry + assert refused == hidden, entry + assert refused is expected, entry @pytest.mark.asyncio @pytest.mark.parametrize( 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 134a5d89225..cdef50ad6b5 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 @@ -9512,19 +9512,25 @@ class TestServerToolListsHonorThePrefixBoundary: 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"]) + async def test_a_blocklist_entry_does_not_reach_a_case_variant_sibling_tool(self): + server = self._aliased_server(disallowed_tools=["petstore-getPet"]) with pytest.raises(HTTPException) as exc_info: - await self._run_check(server, "deletepet") + await self._run_check(server, "getPet") assert exc_info.value.status_code == 403 + await self._run_check(server, "getpet") @pytest.mark.asyncio - async def test_a_case_variant_allowlist_entry_grants_the_tool(self): - server = self._aliased_server(allowed_tools=["PetStore-GetPetById"]) + async def test_an_allowlist_entry_does_not_grant_a_case_variant_sibling_tool(self): + server = self._aliased_server(allowed_tools=["petstore-getPet"]) - await self._run_check(server, "getpetbyid") + await self._run_check(server, "getPet") + + with pytest.raises(HTTPException) as exc_info: + await self._run_check(server, "getpet") + + assert exc_info.value.status_code == 403 @pytest.mark.asyncio async def test_an_explicitly_empty_allowed_params_list_refuses_every_parameter(self):