diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index f107b829a90..784b2385d19 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -4883,6 +4883,10 @@ class MCPServerManager: scoped_server = self.get_mcp_server_by_id(next(iter(in_scope))) if scoped_server is not None: return MCPToolRouteResolved(server=scoped_server) + # The tool exists but no owner is reachable by this caller. Fail closed + # rather than fall through to a scope-blind lookup that could hand back a + # server outside the caller's scope. + return MCPToolRouteNotFound(tool_name=tool_name) server = self._get_mcp_server_from_tool_name(tool_name) if server is None: return MCPToolRouteNotFound(tool_name=tool_name) diff --git a/tests/mcp_tests/test_mcp_logging.py b/tests/mcp_tests/test_mcp_logging.py index d8193c3074f..ba53b0777b1 100644 --- a/tests/mcp_tests/test_mcp_logging.py +++ b/tests/mcp_tests/test_mcp_logging.py @@ -126,7 +126,9 @@ async def test_mcp_cost_tracking(): ) # Manually add the tool mapping to ensure it's available (since mocking might not capture it properly) - zapier_server_ids = frozenset(local_mcp_server_manager.get_all_mcp_server_ids()) + zapier_server = local_mcp_server_manager.get_mcp_server_by_name("zapier_gmail_server") + assert zapier_server is not None + zapier_server_ids = frozenset({zapier_server.server_id}) local_mcp_server_manager.tool_name_to_mcp_server_ids_mapping[ "add_tools" ] = zapier_server_ids @@ -241,7 +243,9 @@ async def test_mcp_cost_tracking_per_tool(): await local_mcp_server_manager._initialize_tool_name_to_mcp_server_ids_mapping() # Manually add the tool mapping to ensure it's available (since mocking might not capture it properly) - test_server_ids = frozenset(local_mcp_server_manager.get_all_mcp_server_ids()) + test_server = local_mcp_server_manager.get_mcp_server_by_name("test_server") + assert test_server is not None + test_server_ids = frozenset({test_server.server_id}) local_mcp_server_manager.tool_name_to_mcp_server_ids_mapping[ "expensive_tool" ] = test_server_ids @@ -406,7 +410,9 @@ async def test_mcp_tool_call_hook(): await local_mcp_server_manager._initialize_tool_name_to_mcp_server_ids_mapping() # Manually add the tool mapping to ensure it's available (since mocking might not capture it properly) - zapier_server_ids = frozenset(local_mcp_server_manager.get_all_mcp_server_ids()) + zapier_server = local_mcp_server_manager.get_mcp_server_by_name("zapier_gmail_server") + assert zapier_server is not None + zapier_server_ids = frozenset({zapier_server.server_id}) local_mcp_server_manager.tool_name_to_mcp_server_ids_mapping["add_tools"] = ( zapier_server_ids ) 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 1116e3efaf1..cfb360f30ac 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 @@ -3937,6 +3937,23 @@ class TestMCPServerManager: assert manager._get_mcp_server_from_tool_name("shared-echo") is None + def test_resolve_tool_route_is_not_found_when_no_owner_is_in_scope(self): + """A tool whose only owner is outside the caller's scope must fail closed. + + Falling through to the scope-blind lookup would hand back a server the caller + cannot reach, and the downstream name-based permission check would misroute it + to a reachable same-named server. + """ + manager = MCPServerManager() + alpha = MCPServer(server_id="id-alpha", name="echo_alpha", transport=MCPTransport.http) + zulu = MCPServer(server_id="id-zulu", name="echo_zulu", transport=MCPTransport.http) + manager.registry = {"id-alpha": alpha, "id-zulu": zulu} + manager._register_tool_route("secret_tool", "id-zulu") + + route = manager.resolve_tool_route("secret_tool", allowed_server_ids=frozenset({"id-alpha"})) + + assert route.kind == "not_found" + def test_resolve_tool_route_names_every_ambiguous_owner(self): """The ambiguous route carries all owners so callers can report the real candidates.""" manager = MCPServerManager()