From 5d777c16d9e59c690886d978f7546b130bd2432d Mon Sep 17 00:00:00 2001 From: tin-berri Date: Sat, 26 Sep 2026 16:53:02 -0700 Subject: [PATCH] fix(mcp): align hub publication status and controls (#43241) * fix(mcp): align hub publication status and controls * refactor(mcp): keep hub visibility guard outside table rendering --------- Co-authored-by: Joshua Valluru <326636767+joshua-berri@users.noreply.github.com> --- cookbook/litellm_proxy_server/mcp/README.md | 37 ++++ .../mcp_server/mcp_server_manager.py | 24 +-- .../mcp_management_endpoints.py | 35 ++-- .../public_endpoints/public_endpoints.py | 14 +- .../mcp_server/test_mcp_server_manager.py | 54 ++++++ .../test_mcp_management_endpoints.py | 156 ++++++++++++++++ .../public_endpoints/test_public_endpoints.py | 66 +++++-- .../_components/MCPPermissionManagement.tsx | 4 +- .../_components/MCPServerCard.test.tsx | 12 ++ .../mcp-servers/_components/MCPServerCard.tsx | 19 +- .../_components/mcp_server_view.test.tsx | 11 +- .../_components/mcp_server_view.tsx | 21 +-- .../mcp-servers/_components/utils.test.tsx | 29 +++ .../mcp-servers/_components/utils.tsx | 35 +++- .../AIHub/MCPHubTableColumns.test.tsx | 19 +- .../components/AIHub/MCPHubTableColumns.tsx | 6 +- .../components/AIHub/ModelHubTable.test.tsx | 30 ++- .../src/components/AIHub/ModelHubTable.tsx | 15 +- .../AIHub/forms/MakeMCPPublicForm.test.tsx | 174 +++++++++++++----- .../AIHub/forms/MakeMCPPublicForm.tsx | 121 ++++++++---- .../src/components/mcp_tools/types.tsx | 2 + 21 files changed, 720 insertions(+), 164 deletions(-) create mode 100644 cookbook/litellm_proxy_server/mcp/README.md diff --git a/cookbook/litellm_proxy_server/mcp/README.md b/cookbook/litellm_proxy_server/mcp/README.md new file mode 100644 index 00000000000..aeee0719019 --- /dev/null +++ b/cookbook/litellm_proxy_server/mcp/README.md @@ -0,0 +1,37 @@ +# Publish MCP servers in the AI Hub + +Set `litellm_settings.public_mcp_servers` to the concrete IDs of the servers you want listed in the public AI Hub. Pin `server_id` in each configuration entry so the publication list stays stable across deployments + +```yaml +mcp_servers: + documentation: + server_id: documentation-mcp + url: https://mcp.example.com/mcp + transport: http + available_on_public_internet: true + +litellm_settings: + public_mcp_hub_strict_whitelist: true + public_mcp_servers: + - documentation-mcp +``` + +Use `documentation-mcp`, the `server_id`, in the publication list. The configuration key `documentation`, display names, and aliases are not publication IDs. Database-created servers use the ID returned by `/v1/mcp/server` + +The dashboard's **AI Hub > MCP Hub > Manage MCP Hub Visibility** dialog edits this same list. Its YAML example includes the selected server IDs. With database-backed configuration (`store_model_in_db: true`), a value declared in YAML is owned by that file: edit the file and reload, or remove that key from YAML to let the dashboard manage it in the database. File-backed deployments can save the list directly to their configuration file + +To remove all explicit entries, save an empty selection in the dialog or configure: + +```yaml +litellm_settings: + public_mcp_hub_strict_whitelist: true + public_mcp_servers: [] +``` + +## Hub listing and network access + +The **Hub listing** column in AI Hub identifies servers that appear in `/public/mcp_hub`. The dashboard derives this status from the current registry and publication settings. Setting `mcp_info.is_public` on a server does not publish it; that response field is derived metadata. `mcp_info.is_public_explicit` identifies registered servers included in the explicit publication list + +Gateway cards and server details show **All Networks** when `available_on_public_internet` is enabled or the server is explicitly published in `public_mcp_servers`. They show **Internal Only** when both are false. The per-server flag defaults to `true`; explicit publication overrides a disabled flag for compatibility. Older proxies that omit the metadata needed to determine access show **Unknown**. These labels describe allowed client IPs; authentication and tool permissions still apply + +The default `public_mcp_hub_strict_whitelist: true` lists only registered servers in `public_mcp_servers`. Legacy mode (`false`) additionally lists registered servers with `available_on_public_internet: true`. In legacy mode, clearing the explicit publication list leaves these automatically listed servers visible. Enable strict mode when the publication list should fully determine hub visibility diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index d0d9100971d..31896d9ddc5 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -6799,6 +6799,16 @@ class MCPServerManager: return server return None + @staticmethod + def _is_public_mcp_server(server: MCPServer, public_ids: Container[str]) -> bool: + return server.server_id in public_ids or ( + not litellm.public_mcp_hub_strict_whitelist and server.available_on_public_internet + ) + + def is_mcp_server_public(self, server_id: str) -> bool: + server: Final = self.registry.get(server_id) or self.config_mcp_servers.get(server_id) + return server is not None and self._is_public_mcp_server(server, litellm.public_mcp_servers or ()) + def get_public_mcp_servers(self) -> list[MCPServer]: """ Return the MCP servers published to the AI Hub via /v1/mcp/make_public. @@ -6816,18 +6826,8 @@ class MCPServerManager: deployments that relied on the OR-with-default semantics; will be removed in a future release. """ - if litellm.public_mcp_hub_strict_whitelist: - if litellm.public_mcp_servers is None: - return [] - public_ids = set(litellm.public_mcp_servers) - return [server for server in self.get_registry().values() if server.server_id in public_ids] - - public_ids = set(litellm.public_mcp_servers or []) - return [ - server - for server in self.get_registry().values() - if server.available_on_public_internet or server.server_id in public_ids - ] + public_ids: Final = frozenset(litellm.public_mcp_servers or ()) + return [server for server in self.get_registry().values() if self._is_public_mcp_server(server, public_ids)] def expand_permission_list(self, identifiers: list[str]) -> list[str]: """ diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 346b1a75ac6..deb0e00ff9b 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -650,7 +650,16 @@ if MCP_AVAILABLE: if hasattr(redacted_server, "credentials"): setattr(redacted_server, "credentials", _preserved_admin_config_credentials(redacted_server.credentials)) - return redacted_server + is_public: Final = global_mcp_server_manager.is_mcp_server_public(redacted_server.server_id) + return redacted_server.model_copy( + update={ + "mcp_info": { + **(redacted_server.mcp_info or {}), + "is_public": is_public, + "is_public_explicit": is_public and redacted_server.server_id in (litellm.public_mcp_servers or ()), + } + } + ) def _preserved_admin_config_credentials( credentials: "MCPCredentials | str | None", @@ -832,10 +841,10 @@ if MCP_AVAILABLE: sanitized.updated_at = None # `mcp_info` is arbitrary metadata; keep only an explicit safe subset. - is_public = False - if isinstance(sanitized.mcp_info, dict): - is_public = bool(sanitized.mcp_info.get("is_public")) - sanitized.mcp_info = {"is_public": True} if is_public else None + sanitized.mcp_info = { + "is_public": (sanitized.mcp_info or {}).get("is_public") is True, + "is_public_explicit": (sanitized.mcp_info or {}).get("is_public_explicit") is True, + } return sanitized @@ -1260,14 +1269,6 @@ if MCP_AVAILABLE: for server in redacted_mcp_servers: server.connected_app_reachable = server.server_id in reachable_ids - # augment the mcp servers with public status - if litellm.public_mcp_servers is not None: - for server in redacted_mcp_servers: - if server.server_id in litellm.public_mcp_servers: - if server.mcp_info is None: - server.mcp_info = {} - server.mcp_info["is_public"] = True - # Annotate has_user_credential for BYOK servers (single batched query) from litellm.proxy.proxy_server import prisma_client as _byok_prisma_client @@ -3041,9 +3042,6 @@ if MCP_AVAILABLE: }, ) - if litellm.public_mcp_servers is None: - litellm.public_mcp_servers = [] - for server_id in request.mcp_server_ids: server = global_mcp_server_manager.get_mcp_server_by_id(server_id=server_id) if server is None: @@ -3052,16 +3050,15 @@ if MCP_AVAILABLE: detail=f"MCP Server with ID {server_id} not found", ) - litellm.public_mcp_servers = request.mcp_server_ids - # Update config with new settings if "litellm_settings" not in config or config["litellm_settings"] is None: config["litellm_settings"] = {} - config["litellm_settings"]["public_mcp_servers"] = litellm.public_mcp_servers + config["litellm_settings"]["public_mcp_servers"] = request.mcp_server_ids # Save the updated config await proxy_config.save_config(new_config=config) + litellm.public_mcp_servers = request.mcp_server_ids verbose_proxy_logger.debug( "Updated public mcp servers to: %s by user: %s", litellm.public_mcp_servers, user_api_key_dict.user_id diff --git a/litellm/proxy/public_endpoints/public_endpoints.py b/litellm/proxy/public_endpoints/public_endpoints.py index 26a5c44fce1..bba5ef681d0 100644 --- a/litellm/proxy/public_endpoints/public_endpoints.py +++ b/litellm/proxy/public_endpoints/public_endpoints.py @@ -300,7 +300,19 @@ async def get_mcp_servers(): ) public_mcp_servers: Final = global_mcp_server_manager.get_public_mcp_servers() - return [MCPPublicServer.model_validate(server.model_dump()) for server in public_mcp_servers] + return [ + MCPPublicServer.model_validate( + { + **server.model_dump(), + "mcp_info": { + **(server.mcp_info or {}), + "is_public": True, + "is_public_explicit": server.server_id in (litellm.public_mcp_servers or ()), + }, + } + ) + for server in public_mcp_servers + ] @router.get( 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 db476e86043..70ef4312f4c 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 @@ -9566,6 +9566,60 @@ class TestGetPublicMCPServers: manager.config_mcp_servers[s.server_id] = s return manager + @pytest.mark.parametrize("registered_in", ("config", "database", "both", "neither")) + @pytest.mark.parametrize("public_ids", (None, [], ["server-id"], ["server-alias"], ["Server Name"])) + @pytest.mark.parametrize( + "strict,network_access,implicitly_public", + ((True, True, False), (True, False, False), (False, True, True), (False, False, False)), + ) + def test_public_status_agrees_with_hub_membership( + self, + registered_in: Literal["config", "database", "both", "neither"], + public_ids: list[str] | None, + strict: bool, + network_access: bool, + implicitly_public: bool, + ) -> None: + manager: Final = MCPServerManager() + server: Final = MCPServer( + server_id="server-id", + name="server-alias", + alias="server-alias", + server_name="Server Name", + transport=MCPTransport.http, + available_on_public_internet=network_access, + mcp_info={"is_public": True, "description": "Preserve custom metadata"}, + ) + config_server: Final = ( + server.model_copy(update={"available_on_public_internet": not network_access}) + if registered_in == "both" + else server + ) + manager.config_mcp_servers = ( + {server.server_id: config_server} if registered_in in ("config", "both") else {} + ) + manager.registry = {server.server_id: server} if registered_in in ("database", "both") else {} + original_server: Final = server.model_dump() + original_config_server: Final = config_server.model_dump() + expected_public: Final = registered_in != "neither" and ( + public_ids == [server.server_id] or implicitly_public + ) + + with ( + patch("litellm.public_mcp_servers", public_ids), + patch("litellm.public_mcp_hub_strict_whitelist", strict), + ): + public_servers: Final = manager.get_public_mcp_servers() + assert manager.is_mcp_server_public(server.server_id) is expected_public + assert [item.server_id for item in public_servers] == ( + [server.server_id] if expected_public else [] + ) + assert manager.is_mcp_server_public("server-alias") is False + assert manager.is_mcp_server_public("missing-server") is False + + assert server.model_dump() == original_server + assert config_server.model_dump() == original_config_server + @patch("litellm.public_mcp_servers", None) def test_returns_empty_when_whitelist_is_none(self): """No /make_public call yet → hub returns nothing, regardless of diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index 8aa16b817fc..11b3dcf54bc 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -33,6 +33,7 @@ from litellm.proxy._types import ( LiteLLM_ObjectPermissionTable, LiteLLM_MCPServerTable, LitellmUserRoles, + MakeMCPServersPublicRequest, MCPTransport, MCPUserCredentialResponse, NewMCPServerRequest, @@ -154,6 +155,161 @@ def patch_proxy_general_settings(settings: dict): ) +@pytest.mark.asyncio +@pytest.mark.parametrize("from_db", (False, True)) +@pytest.mark.parametrize( + "strict,explicit,expected_public", + ((True, True, True), (True, False, False), (False, False, True)), +) +async def test_mcp_publication_list_and_detail_derive_current_status( + from_db: bool, strict: bool, explicit: bool, expected_public: bool +) -> None: + from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager + + manager: Final = MCPServerManager() + server: Final = MCPServer( + server_id="publication-server", + name="publication-server", + transport=MCPTransport.http, + auth_type=MCPAuth.api_key, + available_on_public_internet=True, + mcp_info={ + "is_public": not expected_public, + "is_public_explicit": not explicit, + "description": "Keep this description", + }, + ) + manager.registry = {server.server_id: server} if from_db else {} + manager.config_mcp_servers = {} if from_db else {server.server_id: server} + record: Final = manager._build_mcp_server_table(server) + original_metadata: Final = dict(server.mcp_info or {}) + admin: Final = generate_mock_user_api_key_auth() + + with ( + patch("litellm.public_mcp_servers", [server.server_id] if explicit else []), + patch("litellm.public_mcp_hub_strict_whitelist", strict), + patch.object(mgmt_endpoints, "global_mcp_server_manager", manager), + patch.object(mgmt_endpoints, "get_prisma_client_or_throw", return_value=MagicMock()), + patch.object(mgmt_endpoints, "get_mcp_server", AsyncMock(return_value=record if from_db else None)), + patch("litellm.proxy.proxy_server.prisma_client", None), + patch("litellm.proxy.proxy_server.general_settings", {"user_mcp_management_mode": "view_all"}), + ): + listing: Final = await mgmt_endpoints.fetch_all_mcp_servers( + user_api_key_dict=admin, team_id=None, connected_app_view=False + ) + detail: Final = await mgmt_endpoints.fetch_mcp_server( + request=_make_mock_request(), server_id=server.server_id, user_api_key_dict=admin + ) + assert len(listing) == 1 + for projected in (listing[0], detail): + assert projected.mcp_info == { + "is_public": expected_public, + "is_public_explicit": explicit, + "description": "Keep this description", + } + assert bool(manager.get_public_mcp_servers()) is expected_public + + assert server.mcp_info == original_metadata + assert record.mcp_info == original_metadata + + +@pytest.mark.parametrize("approval_status", ("pending_review", "rejected", "draft", "active")) +@pytest.mark.parametrize("strict", (False, True)) +def test_mcp_publication_projection_excludes_unregistered_lifecycle_records( + approval_status: str, strict: bool +) -> None: + from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager + + record: Final = LiteLLM_MCPServerTable( + server_id="unregistered-server", + transport=MCPTransport.http, + approval_status=approval_status, + credentials={"auth_value": "test-secret"}, + available_on_public_internet=True, + mcp_info={"is_public": True, "is_public_explicit": True}, + ) + original: Final = record.model_dump() + with ( + patch("litellm.public_mcp_servers", [record.server_id]), + patch("litellm.public_mcp_hub_strict_whitelist", strict), + patch.object(mgmt_endpoints, "global_mcp_server_manager", MCPServerManager()), + ): + for project in ( + mgmt_endpoints._redact_mcp_credentials, + mgmt_endpoints._sanitize_mcp_server_for_non_admin, + mgmt_endpoints._sanitize_mcp_server_for_virtual_key, + ): + projected: Final = project(record) + assert projected.mcp_info == {"is_public": False, "is_public_explicit": False} + assert projected.credentials is None + assert record.model_dump() == original + + +@pytest.mark.asyncio +@pytest.mark.parametrize("previous_ids", (None, ["old-server"])) +@pytest.mark.parametrize( + "selected_ids,save_error,role,error_status", + ( + (["new-server"], None, LitellmUserRoles.PROXY_ADMIN, None), + ([], None, LitellmUserRoles.PROXY_ADMIN, None), + (["new-server"], HTTPException(400, "Owned by config file"), LitellmUserRoles.PROXY_ADMIN, 400), + (["new-server"], RuntimeError("Database write failed"), LitellmUserRoles.PROXY_ADMIN, 500), + (["missing-server"], None, LitellmUserRoles.PROXY_ADMIN, 404), + (["new-server"], None, LitellmUserRoles.INTERNAL_USER, 403), + ), +) +async def test_mcp_publication_updates_runtime_only_after_successful_save( + previous_ids: list[str] | None, + selected_ids: list[str], + save_error: HTTPException | RuntimeError | None, + role: LitellmUserRoles, + error_status: int | None, +) -> None: + import litellm + from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager + + manager: Final = MCPServerManager() + server: Final = generate_mock_mcp_server_config_record(server_id="new-server") + manager.config_mcp_servers = {server.server_id: server} + expected_config: Final = {"litellm_settings": {"drop_params": True, "public_mcp_servers": selected_ids}} + + async def save_config(new_config: Mapping[str, object]) -> None: + assert litellm.public_mcp_servers is previous_ids + assert new_config == expected_config + if save_error is not None: + raise save_error + + save: Final = AsyncMock(side_effect=save_config) + proxy_config: Final = SimpleNamespace( + get_config=AsyncMock(return_value={"litellm_settings": {"drop_params": True}}), + save_config=save, + ) + request: Final = MakeMCPServersPublicRequest(mcp_server_ids=selected_ids) + caller: Final = generate_mock_user_api_key_auth(user_role=role) + with ( + patch("litellm.public_mcp_servers", previous_ids), + patch("litellm.proxy.proxy_server.proxy_config", proxy_config), + patch( + "litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager", + manager, + ), + ): + if error_status is None: + response: Final = await mgmt_endpoints.make_mcp_servers_public(request, caller) + assert response["public_mcp_servers"] == selected_ids + assert litellm.public_mcp_servers == selected_ids + else: + with pytest.raises(HTTPException) as error: + await mgmt_endpoints.make_mcp_servers_public(request, caller) + assert error.value.status_code == error_status + assert litellm.public_mcp_servers is previous_ids + + if error_status in (403, 404): + save.assert_not_awaited() + else: + save.assert_awaited_once_with(new_config=expected_config) + + class TestMCPCredentialsTokenExchangeProfile: """token_exchange_profile must be a declared MCPCredentials field so the management API can persist the entra_obo profile. An undeclared key is silently stripped by pydantic when the diff --git a/tests/test_litellm/proxy/public_endpoints/test_public_endpoints.py b/tests/test_litellm/proxy/public_endpoints/test_public_endpoints.py index 0dec44af402..18839a65d62 100644 --- a/tests/test_litellm/proxy/public_endpoints/test_public_endpoints.py +++ b/tests/test_litellm/proxy/public_endpoints/test_public_endpoints.py @@ -1086,43 +1086,73 @@ def test_clean_display_name_passthrough_when_no_suffix(): assert _clean_display_name("") == "" -def test_public_mcp_hub_returns_only_whitelisted_servers(): - """Regression: /public/mcp_hub must gate strictly on - litellm.public_mcp_servers, mirroring /public/model_hub and - /public/agent_hub. Servers with available_on_public_internet=True that - are not on the whitelist must not leak.""" +@pytest.mark.parametrize( + "strict,explicit,expected_listed", + ((True, True, True), (True, False, False), (False, True, True), (False, False, True)), +) +@pytest.mark.parametrize("stored_public", (None, False, True)) +def test_public_mcp_hub_derives_publication_metadata_without_mutating_registry( + strict: bool, + explicit: bool, + expected_listed: bool, + stored_public: bool | None, +) -> None: + from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager from litellm.types.mcp_server.mcp_server_manager import MCPServer from litellm.proxy._types import MCPTransport - app = FastAPI() + app: Final = FastAPI() app.include_router(router) - app.dependency_overrides[user_api_key_auth] = lambda: MagicMock() - client = TestClient(app) + client: Final = TestClient(app) - listed = MCPServer( + server: Final = MCPServer( server_id="listed", name="listed", server_name="listed", transport=MCPTransport.http, available_on_public_internet=True, + mcp_info=( + { + "is_public": stored_public, + "is_public_explicit": not explicit, + "description": "Preserve custom metadata", + } + if stored_public is not None + else None + ), ) - - mock_manager = MagicMock() - mock_manager.get_public_mcp_servers.return_value = [listed] + unlisted: Final = MCPServer( + server_id="unlisted", + name="unlisted", + transport=MCPTransport.http, + available_on_public_internet=False, + mcp_info={"is_public": True, "is_public_explicit": True}, + ) + manager: Final = MCPServerManager() + manager.config_mcp_servers = {server.server_id: server} + manager.registry = {unlisted.server_id: unlisted} + original_registry: Final = {key: value.model_dump() for key, value in manager.get_registry().items()} with ( - patch("litellm.public_mcp_servers", ["listed"]), + patch("litellm.public_mcp_servers", [server.server_id] if explicit else []), + patch("litellm.public_mcp_hub_strict_whitelist", strict), patch( "litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager", - mock_manager, + manager, ), ): - response = client.get("/public/mcp_hub") + response: Final = client.get("/public/mcp_hub") assert response.status_code == 200 - data = response.json() - assert [item["server_id"] for item in data] == ["listed"] - app.dependency_overrides.clear() + data: Final = response.json() + assert [item["server_id"] for item in data] == ([server.server_id] if expected_listed else []) + if expected_listed: + assert data[0]["mcp_info"] == { + **({"description": "Preserve custom metadata"} if stored_public is not None else {}), + "is_public": True, + "is_public_explicit": explicit, + } + assert {key: value.model_dump() for key, value in manager.get_registry().items()} == original_registry def test_public_mcp_hub_returns_empty_when_whitelist_unset(): diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.tsx index cb423b435ae..a48c991bc7a 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPPermissionManagement.tsx @@ -217,12 +217,12 @@ const MCPPermissionManagement: React.FC = ({
Internal network only - +

- Turn on to restrict access to callers within your internal network only. + Turn on to restrict public IPs. Explicitly published server IDs remain accessible from public IPs.

diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.test.tsx index 100b0ea93d3..d298d9d8145 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.test.tsx @@ -126,3 +126,15 @@ describe("MCPServerCard per-user credentials", () => { expect(screen.queryByRole("button", { name: "Set" })).not.toBeInTheDocument(); }); }); + +describe("MCPServerCard network access", () => { + it("shows effective network access without a hub listing badge", () => { + renderCard({ + available_on_public_internet: false, + mcp_info: { server_name: "demo_server", is_public: true, is_public_explicit: true }, + }); + + expect(screen.getByText("All Networks")).toBeInTheDocument(); + expect(screen.queryByText(/^Hub:/)).not.toBeInTheDocument(); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.tsx index 775809e3670..bb153f94665 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPServerCard.tsx @@ -13,7 +13,7 @@ import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/comp import { cn } from "@/lib/cva.config"; import { AUTH_TYPE, MCP_REACHABLE_DESCRIPTION, type MCPServer } from "@/components/mcp_tools/types"; import { Logo } from "@/components/molecules/logo/Logo"; -import { getMaskedAndFullUrl } from "./utils"; +import { getMaskedAndFullUrl, getMCPNetworkAccess } from "./utils"; interface MCPServerCardProps { server: MCPServer; @@ -70,7 +70,7 @@ const MCPServerCard: FC = ({ server.auth_type === AUTH_TYPE.OAUTH2 && !server.oauth2_flow && !server.delegate_auth_to_upstream; const status = server.status || "unknown"; const healthTone = HEALTH_TONE[status] ?? HEALTH_TONE.unknown; - const isPublic = server.available_on_public_internet; + const networkAccess = getMCPNetworkAccess(server); const accessGroups = (server.mcp_access_groups ?? []).filter((g): g is string => typeof g === "string"); const missing = missingUserFields ?? []; @@ -236,10 +236,17 @@ const MCPServerCard: FC = ({ )} - - - {isPublic ? "Public" : "Internal"} - + + + + {networkAccess.label} + + } + /> + {networkAccess.description} + {accessGroups.slice(0, 2).map((g) => ( { }); it("shows the read-only settings summary before editing", async () => { - renderView({ allow_all_keys: true, available_on_public_internet: false }); + renderView({ + allow_all_keys: true, + available_on_public_internet: false, + mcp_info: { server_name: "demo server", is_public: true, is_public_explicit: true }, + }); await userEvent.click(screen.getByRole("tab", { name: "Settings" })); expect(await screen.findByText("MCP Server Settings")).toBeInTheDocument(); expect(screen.getByText("Allow All Keys")).toBeInTheDocument(); expect(screen.getByText("Enabled")).toBeInTheDocument(); - expect(screen.getByText("Internal only")).toBeInTheDocument(); + expect(screen.getByText("Network access")).toBeInTheDocument(); + expect(screen.getByText("All Networks")).toBeInTheDocument(); + expect(screen.queryByText("MCP Hub")).not.toBeInTheDocument(); + expect(screen.queryByText("Listed")).not.toBeInTheDocument(); expect(screen.queryByText("edit form")).not.toBeInTheDocument(); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.tsx index a7ff34301a0..c97596ce0f6 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.tsx @@ -13,7 +13,7 @@ import { MCPServerUserCredentialsPanel } from "./MCPServerUserCredentialsPanel"; import { getSecureItem } from "@/utils/secureStorage"; import { isProxyAdminRole, isProxyAdminTierRole } from "@/utils/roles"; import MCPServerCostDisplay from "./mcp_server_cost_display"; -import { getMaskedAndFullUrl } from "./utils"; +import { getMaskedAndFullUrl, getMCPNetworkAccess } from "./utils"; import { copyToClipboard as utilCopyToClipboard } from "@/utils/dataUtils"; import { CheckIcon, CopyIcon } from "lucide-react"; @@ -68,6 +68,7 @@ export const MCPServerView: React.FC = ({ const returningFromEditOAuth = isReturningFromEditOAuth(canEdit, mcpServer.server_id); const [editing, setEditing] = useState(isEditing || returningFromEditOAuth); const [showFullUrl, setShowFullUrl] = useState(false); + const networkAccess = getMCPNetworkAccess(mcpServer); const [copiedStates, setCopiedStates] = useState>({}); const [selectedTabIndex, setSelectedTabIndex] = useState(returningFromEditOAuth ? 2 : initialTabIndex); const canViewUserCredentials = userRole !== null && isProxyAdminTierRole(userRole); @@ -318,19 +319,13 @@ export const MCPServerView: React.FC = ({
-

Network Access

+

Network access

- {mcpServer.available_on_public_internet ? ( - - - Public - - ) : ( - - - Internal only - - )} + + + {networkAccess.label} + +

{networkAccess.description}

{handleAuth(mcpServer.auth_type) === "oauth2" && ( diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/utils.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/utils.test.tsx index 3b4fda400c2..bf30821d73a 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/utils.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/utils.test.tsx @@ -3,11 +3,40 @@ import { extractMCPToken, maskUrl, getMaskedAndFullUrl, + getMCPNetworkAccess, validateMCPServerUrl, validateMCPServerName, normalizeToolOverrideMap, } from "./utils"; +describe("getMCPNetworkAccess", () => { + it.each([ + { publicIp: true, explicit: false, label: "All Networks" }, + { publicIp: false, explicit: true, label: "All Networks" }, + { publicIp: true, explicit: true, label: "All Networks" }, + { publicIp: false, explicit: false, label: "Internal Only" }, + { publicIp: true, explicit: undefined, label: "All Networks" }, + { publicIp: false, explicit: undefined, label: "Unknown" }, + { publicIp: undefined, explicit: false, label: "Unknown" }, + ])("reports $label for network=$publicIp and publication=$explicit", ({ publicIp, explicit, label }) => { + expect( + getMCPNetworkAccess({ + available_on_public_internet: publicIp, + mcp_info: { server_name: "demo", is_public: true, is_public_explicit: explicit }, + }).label, + ).toBe(label); + }); + + it("explains when hub publication permits public IPs", () => { + expect( + getMCPNetworkAccess({ + available_on_public_internet: false, + mcp_info: { server_name: "demo", is_public_explicit: true }, + }).description, + ).toContain("because this server is published in MCP Hub"); + }); +}); + describe("extractMCPToken", () => { it("should extract token after /mcp/", () => { const result = extractMCPToken("https://example.com/mcp/abc123"); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/utils.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/utils.tsx index 4738e1e8fba..bb72831d92e 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/utils.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/utils.tsx @@ -1,4 +1,37 @@ -import { MCPEnvVar, MCPEnvVarScope } from "@/components/mcp_tools/types"; +import { MCPEnvVar, MCPEnvVarScope, type MCPServer } from "@/components/mcp_tools/types"; + +export const getMCPNetworkAccess = ( + server: Pick, +): { + readonly label: "All Networks" | "Internal Only" | "Unknown"; + readonly dotClassName: string; + readonly description: string; +} => { + const explicitlyPublished = server.mcp_info?.is_public_explicit; + if (server.available_on_public_internet === true || explicitlyPublished === true) { + return { + label: "All Networks", + dotClassName: "bg-success", + description: + server.available_on_public_internet === true + ? "Allows requests from public and internal IPs. Authentication and access permissions still apply" + : "Allows requests from public and internal IPs because this server is published in MCP Hub. Authentication and access permissions still apply", + }; + } + if (server.available_on_public_internet === false && explicitlyPublished === false) { + return { + label: "Internal Only", + dotClassName: "bg-warning", + description: + "Allows requests only from internal/private IP ranges. Authentication and access permissions still apply", + }; + } + return { + label: "Unknown", + dotClassName: "bg-border", + description: "The proxy did not report enough network and publication settings to determine allowed client IPs", + }; +}; export const extractMCPToken = (url: string): { token: string | null; baseUrl: string } => { try { diff --git a/ui/litellm-dashboard/src/components/AIHub/MCPHubTableColumns.test.tsx b/ui/litellm-dashboard/src/components/AIHub/MCPHubTableColumns.test.tsx index 1032b03a3ce..fee58e10fda 100644 --- a/ui/litellm-dashboard/src/components/AIHub/MCPHubTableColumns.test.tsx +++ b/ui/litellm-dashboard/src/components/AIHub/MCPHubTableColumns.test.tsx @@ -1,4 +1,4 @@ -import { render, screen } from "@testing-library/react"; +import { render, screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import { DataTable } from "@/components/shared/DataTable"; @@ -63,6 +63,23 @@ describe("getMCPHubTableColumns", () => { expect(screen.getByText("Auth Type")).toBeInTheDocument(); }); + it("shows hub membership separately from the network setting", () => { + renderTable(vi.fn(), [ + { ...mockServer, available_on_public_internet: false, mcp_info: { is_public: true } }, + { + ...mockServer, + server_id: "network-only", + server_name: "Network-only server", + available_on_public_internet: true, + mcp_info: { is_public: false }, + }, + ]); + + expect(screen.getByText("Hub listing")).toBeInTheDocument(); + expect(within(screen.getByRole("row", { name: /exa_test/ })).getByText("Listed")).toBeInTheDocument(); + expect(within(screen.getByRole("row", { name: /Network-only server/ })).getByText("Unlisted")).toBeInTheDocument(); + }); + it("does not expose a URL column", () => { renderTable(); expect(screen.queryByText("URL")).not.toBeInTheDocument(); diff --git a/ui/litellm-dashboard/src/components/AIHub/MCPHubTableColumns.tsx b/ui/litellm-dashboard/src/components/AIHub/MCPHubTableColumns.tsx index 20a14bcb476..db53e97569b 100644 --- a/ui/litellm-dashboard/src/components/AIHub/MCPHubTableColumns.tsx +++ b/ui/litellm-dashboard/src/components/AIHub/MCPHubTableColumns.tsx @@ -203,8 +203,8 @@ export const getMCPHubTableColumns = ({ onServerClick }: MCPHubTableColumnsDeps) { id: "is_public", accessorFn: (row) => row.mcp_info?.is_public === true, - meta: { title: "Public", skeleton: "badge", className: "hidden md:table-cell" }, - header: ({ column }) => , + meta: { title: "Hub listing", skeleton: "badge", className: "hidden md:table-cell" }, + header: ({ column }) => , size: 100, enableSorting: true, sortingFn: (rowA, rowB) => { @@ -214,7 +214,7 @@ export const getMCPHubTableColumns = ({ onServerClick }: MCPHubTableColumnsDeps) }, cell: ({ row }) => { const isPublic = row.original.mcp_info?.is_public === true; - return ; + return ; }, }, { diff --git a/ui/litellm-dashboard/src/components/AIHub/ModelHubTable.test.tsx b/ui/litellm-dashboard/src/components/AIHub/ModelHubTable.test.tsx index 27fe2330acd..1f052b8932a 100644 --- a/ui/litellm-dashboard/src/components/AIHub/ModelHubTable.test.tsx +++ b/ui/litellm-dashboard/src/components/AIHub/ModelHubTable.test.tsx @@ -1,5 +1,7 @@ import * as networking from "@/components/networking"; import userEvent from "@testing-library/user-event"; +import { act } from "@testing-library/react"; +import type { MCPServerData } from "./MCPHubTableColumns"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { renderWithProviders, screen, waitFor } from "../../../tests/test-utils"; import ModelHubTable from "./ModelHubTable"; @@ -18,6 +20,7 @@ vi.mock("@/components/networking", () => ({ getProxyBaseUrl: vi.fn(() => "http://localhost:4000"), getAgentsList: vi.fn(), fetchMCPServers: vi.fn(), + makeMCPPublicCall: vi.fn(), getUiSettings: vi.fn(), getClaudeCodePluginsList: vi.fn(() => Promise.resolve({ plugins: [] })), })); @@ -202,13 +205,13 @@ describe("ModelHubTable", () => { }); describe("hub tabs", () => { - const renderHub = async (agents: object[] = []) => { + const renderHub = async (agents: object[] = [], mcpServers: Promise = Promise.resolve([])) => { vi.mocked(networking.modelHubCall).mockResolvedValue({ data: [{ model_group: "claude-opus-4-8", providers: ["anthropic"], mode: "chat" }], }); vi.mocked(networking.getConfigFieldSetting).mockResolvedValue({ field_value: false }); vi.mocked(networking.getAgentsList).mockResolvedValue({ agents }); - vi.mocked(networking.fetchMCPServers).mockResolvedValue([]); + vi.mocked(networking.fetchMCPServers).mockReturnValue(mcpServers); vi.mocked(networking.getUiSettings).mockResolvedValue({ values: {} }); mockUseUISettings.mockReturnValue({ data: { values: {} }, isLoading: false }); @@ -219,6 +222,29 @@ describe("ModelHubTable", () => { return { user, search: await screen.findByPlaceholderText("Search model names...") }; }; + it("requires a fresh MCP publication list before and after saving", async () => { + const servers = Promise.withResolvers(); + const { user } = await renderHub([], servers.promise); + await user.click(screen.getByRole("tab", { name: "MCP Hub" })); + + const manageVisibility = screen.getByRole("button", { name: "Manage MCP Hub Visibility" }); + expect(manageVisibility).toBeDisabled(); + await act(async () => servers.resolve([])); + expect(manageVisibility).toBeEnabled(); + + const refresh = Promise.withResolvers(); + vi.mocked(networking.makeMCPPublicCall).mockResolvedValueOnce({}); + vi.mocked(networking.fetchMCPServers).mockReturnValueOnce(refresh.promise); + await user.click(manageVisibility); + await user.click(screen.getByRole("button", { name: "Next" })); + await user.click(screen.getByRole("button", { name: "Save Publication List" })); + + expect(networking.makeMCPPublicCall).toHaveBeenCalledWith("test-token", []); + expect(manageVisibility).toBeDisabled(); + await act(async () => refresh.reject(new Error("Unable to reload the publication list"))); + expect(manageVisibility).toBeDisabled(); + }); + it("keeps the model filter typed on the Model Hub tab after visiting another hub", async () => { const { user, search } = await renderHub(); diff --git a/ui/litellm-dashboard/src/components/AIHub/ModelHubTable.tsx b/ui/litellm-dashboard/src/components/AIHub/ModelHubTable.tsx index 063850b3e72..c4d776ea2fb 100644 --- a/ui/litellm-dashboard/src/components/AIHub/ModelHubTable.tsx +++ b/ui/litellm-dashboard/src/components/AIHub/ModelHubTable.tsx @@ -49,6 +49,10 @@ interface ModelHubTableProps { userRole: string | null; } +function isMCPHubVisibilityDisabled(isLoading: boolean, servers: readonly MCPServerData[] | null): boolean { + return isLoading || servers === null; +} + function HubEmptyState({ title, body }: { title: string; body: string }) { return (
@@ -359,10 +363,14 @@ const ModelHubTable: React.FC = ({ accessToken, publicPage, if (accessToken) { const fetchMcpData = async () => { try { + setMcpLoading(true); const response = await fetchMCPServers(accessToken); setMcpHubData(response); } catch (error) { + setMcpHubData(null); console.error("Error refreshing MCP server data:", error); + } finally { + setMcpLoading(false); } }; fetchMcpData(); @@ -567,7 +575,12 @@ const ModelHubTable: React.FC = ({ accessToken, publicPage, {/* Header with Make Public Button */} {publicPage == false && canModify && (
- +
)} diff --git a/ui/litellm-dashboard/src/components/AIHub/forms/MakeMCPPublicForm.test.tsx b/ui/litellm-dashboard/src/components/AIHub/forms/MakeMCPPublicForm.test.tsx index 5b96e9ad194..881711c1668 100644 --- a/ui/litellm-dashboard/src/components/AIHub/forms/MakeMCPPublicForm.test.tsx +++ b/ui/litellm-dashboard/src/components/AIHub/forms/MakeMCPPublicForm.test.tsx @@ -1,6 +1,8 @@ import { render, screen, fireEvent, act, waitFor } from "@testing-library/react"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import MakeMCPPublicForm from "./MakeMCPPublicForm"; +import userEvent from "@testing-library/user-event"; +import { toast } from "@/lib/toast"; import { MCPServerData } from "@/components/AIHub/MCPHubTableColumns"; // Mock the networking function @@ -8,6 +10,10 @@ vi.mock("../../networking", () => ({ makeMCPPublicCall: vi.fn(), })); +vi.mock("@/lib/toast", () => ({ + toast: { success: vi.fn(), fromError: vi.fn() }, +})); + // Import the mocked function import { makeMCPPublicCall } from "../../networking"; const mockMakeMCPPublicCall = vi.mocked(makeMCPPublicCall); @@ -28,7 +34,7 @@ describe("MakeMCPPublicForm", () => { url: "http://example.com/server1", transport: "http", status: "active", - mcp_info: { is_public: false }, + mcp_info: { is_public: false, is_public_explicit: false }, allowed_tools: ["tool-1", "tool-2"], auth_type: "bearer", credentials: {}, @@ -50,7 +56,7 @@ describe("MakeMCPPublicForm", () => { url: "http://example.com/server2", transport: "websocket", status: "inactive", - mcp_info: { is_public: true }, + mcp_info: { is_public: true, is_public_explicit: true }, allowed_tools: [], auth_type: "none", credentials: {}, @@ -80,16 +86,16 @@ describe("MakeMCPPublicForm", () => { it("should render the component", () => { render(); - expect(screen.getByText("Make MCP Servers Public")).toBeInTheDocument(); - expect(screen.getByText("Select MCP Servers to Make Public")).toBeInTheDocument(); + expect(screen.getByText("Manage MCP Hub Visibility")).toBeInTheDocument(); + expect(screen.getByText("Select MCP Servers for the Hub")).toBeInTheDocument(); }); it("should initialize with correct state", () => { render(); // Check that the component renders with the correct title and content - expect(screen.getByText("Make MCP Servers Public")).toBeInTheDocument(); - expect(screen.getByText("Select MCP Servers to Make Public")).toBeInTheDocument(); + expect(screen.getByText("Manage MCP Hub Visibility")).toBeInTheDocument(); + expect(screen.getByText("Select MCP Servers for the Hub")).toBeInTheDocument(); // Check that all server checkboxes are present const checkboxes = screen.getAllByRole("checkbox"); @@ -104,7 +110,7 @@ describe("MakeMCPPublicForm", () => { render(); // Initially on step 1 - expect(screen.getByText("Select MCP Servers to Make Public")).toBeInTheDocument(); + expect(screen.getByText("Select MCP Servers for the Hub")).toBeInTheDocument(); // Select all servers using the select all checkbox const selectAllCheckbox = screen.getByRole("checkbox", { name: "Select All (2)" }); @@ -123,7 +129,7 @@ describe("MakeMCPPublicForm", () => { // Should move to step 2 await waitFor(() => { - expect(screen.getByText("Confirm Making MCP Servers Public")).toBeInTheDocument(); + expect(screen.getByText("Confirm MCP Hub Publication")).toBeInTheDocument(); }); }); @@ -145,10 +151,10 @@ describe("MakeMCPPublicForm", () => { // Wait for navigation to complete await waitFor(() => { - expect(screen.getByText("Confirm Making MCP Servers Public")).toBeInTheDocument(); + expect(screen.getByText("Confirm MCP Hub Publication")).toBeInTheDocument(); }); - const submitButton = screen.getByRole("button", { name: "Make Public" }); + const submitButton = screen.getByRole("button", { name: "Save Publication List" }); await act(async () => { fireEvent.click(submitButton); }); @@ -187,29 +193,105 @@ describe("MakeMCPPublicForm", () => { expect(checkboxes[2]).not.toBeChecked(); }); - it("should show error when no servers selected", async () => { + it("submits an empty publication list after the last server is deselected", async () => { + mockMakeMCPPublicCall.mockResolvedValueOnce({}); render(); - // Deselect all servers first - const checkboxes = screen.getAllByRole("checkbox"); - await act(async () => { - fireEvent.click(checkboxes[0]); // Click select all to select all - }); - await act(async () => { - fireEvent.click(checkboxes[0]); // Click select all again to deselect all - }); + fireEvent.click(screen.getAllByRole("checkbox")[2]); + expect(screen.getByRole("button", { name: "Next" })).toBeEnabled(); + fireEvent.click(screen.getByRole("button", { name: "Next" })); + fireEvent.click(screen.getByRole("button", { name: "Save Publication List" })); - // Try to go to next step - const nextButton = screen.getByRole("button", { name: "Next" }); - await act(async () => { - fireEvent.click(nextButton); - }); - - // Should stay on same step - expect(screen.getByText("Select MCP Servers to Make Public")).toBeInTheDocument(); + await waitFor(() => expect(mockMakeMCPPublicCall).toHaveBeenCalledWith("test-token", [])); + expect(mockProps.onSuccess).toHaveBeenCalled(); }); - it("should display empty state when no servers are available", () => { + it("keeps legacy listings separate from explicitly published selections", () => { + render( + , + ); + + expect(screen.getAllByRole("checkbox")[1]).not.toBeChecked(); + expect(screen.getAllByRole("checkbox")[2]).toBeChecked(); + expect(screen.getByText("Listed by legacy mode")).toBeInTheDocument(); + }); + + it.each([ + { mode: "all missing, stale true", info: { is_public: true }, mixed: false }, + { mode: "all missing, stale false", info: { is_public: false }, mixed: false }, + { mode: "mixed, stale true", info: { is_public: true }, mixed: true }, + { mode: "mixed, stale false", info: { is_public: false }, mixed: true }, + { mode: "null explicit status", info: { is_public: true, is_public_explicit: null }, mixed: true }, + { mode: "nonboolean explicit status", info: { is_public: true, is_public_explicit: "true" }, mixed: true }, + ])("blocks unknown explicit publication metadata: $mode", ({ info, mixed }) => { + const unknownServer = { ...mockProps.mcpHubData[0], mcp_info: info }; + const catalog = mixed ? [unknownServer, mockProps.mcpHubData[1]] : [unknownServer]; + render(); + + expect(screen.getByRole("alert")).toHaveTextContent("explicit publication status"); + expect(screen.queryByRole("checkbox")).not.toBeInTheDocument(); + expect(screen.queryByText("Configure in YAML")).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Copy code" })).not.toBeInTheDocument(); + const nextButton = screen.getByRole("button", { name: "Next" }); + expect(nextButton).toBeDisabled(); + fireEvent.click(nextButton); + expect(screen.queryByText("Confirm MCP Hub Publication")).not.toBeInTheDocument(); + expect(mockMakeMCPPublicCall).not.toHaveBeenCalled(); + }); + + it.each([true, false])("blocks confirmation when explicit metadata disappears with stale listing %s", (listed) => { + const { rerender } = render(); + fireEvent.click(screen.getByRole("button", { name: "Next" })); + expect(screen.getByRole("button", { name: "Save Publication List" })).toBeEnabled(); + + const catalog = [{ ...mockProps.mcpHubData[0], mcp_info: { is_public: listed } }, mockProps.mcpHubData[1]]; + rerender(); + + expect(screen.getByRole("alert")).toHaveTextContent("explicit publication status"); + expect(screen.queryByText("Confirm MCP Hub Publication")).not.toBeInTheDocument(); + const saveButton = screen.getByRole("button", { name: "Save Publication List" }); + expect(saveButton).toBeDisabled(); + fireEvent.click(saveButton); + expect(mockMakeMCPPublicCall).not.toHaveBeenCalled(); + expect(screen.queryByRole("button", { name: "Copy code" })).not.toBeInTheDocument(); + + const refreshedCatalog = [ + { ...mockProps.mcpHubData[0], mcp_info: { is_public: true, is_public_explicit: true } }, + { ...mockProps.mcpHubData[1], mcp_info: { is_public: false, is_public_explicit: false } }, + ]; + rerender(); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Next" })).toBeEnabled(); + expect(screen.getByRole("checkbox", { name: "Publish Test Server 1" })).toBeChecked(); + expect(screen.getByRole("checkbox", { name: "Publish Test Server 2" })).not.toBeChecked(); + }); + + it("copies publication YAML using the selected server IDs", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByText("Configure in YAML")); + await user.click(screen.getByRole("button", { name: "Copy code" })); + + expect(await navigator.clipboard.readText()).toBe( + 'litellm_settings:\n public_mcp_hub_strict_whitelist: true\n public_mcp_servers:\n - "server-2"', + ); + + await user.click(screen.getByRole("checkbox", { name: "Publish Test Server 2" })); + await user.click(screen.getByRole("button", { name: "Copy code" })); + expect(await navigator.clipboard.readText()).toBe( + "litellm_settings:\n public_mcp_hub_strict_whitelist: true\n public_mcp_servers: []", + ); + }); + + it("allows clearing publication IDs when the loaded server catalog is empty", async () => { + mockMakeMCPPublicCall.mockResolvedValueOnce({}); const emptyProps = { ...mockProps, mcpHubData: [] as MCPServerData[], @@ -223,9 +305,13 @@ describe("MakeMCPPublicForm", () => { const selectAllCheckbox = screen.getByRole("checkbox", { name: "Select All" }); expectDisabledControl(selectAllCheckbox); - // Next button should be disabled const nextButton = screen.getByRole("button", { name: "Next" }); - expect(nextButton).toBeDisabled(); + expect(nextButton).toBeEnabled(); + fireEvent.click(nextButton); + fireEvent.click(screen.getByRole("button", { name: "Save Publication List" })); + + await waitFor(() => expect(mockMakeMCPPublicCall).toHaveBeenCalledWith("test-token", [])); + expect(mockProps.onSuccess).toHaveBeenCalled(); }); it("should handle Cancel button functionality", async () => { @@ -252,7 +338,7 @@ describe("MakeMCPPublicForm", () => { // Verify we're on step 1 await waitFor(() => { - expect(screen.getByText("Confirm Making MCP Servers Public")).toBeInTheDocument(); + expect(screen.getByText("Confirm MCP Hub Publication")).toBeInTheDocument(); }); // Click Previous button @@ -262,7 +348,7 @@ describe("MakeMCPPublicForm", () => { }); // Should go back to step 0 - expect(screen.getByText("Select MCP Servers to Make Public")).toBeInTheDocument(); + expect(screen.getByText("Select MCP Servers for the Hub")).toBeInTheDocument(); }); it("should handle individual server selection", async () => { @@ -322,8 +408,8 @@ describe("MakeMCPPublicForm", () => { }); it("should handle submit error properly", async () => { - const errorMessage = "Network error"; - mockMakeMCPPublicCall.mockRejectedValueOnce(new Error(errorMessage)); + const error = new Error("Update litellm_settings.public_mcp_servers in your YAML configuration"); + mockMakeMCPPublicCall.mockRejectedValueOnce(error); render(); @@ -333,10 +419,10 @@ describe("MakeMCPPublicForm", () => { }); await waitFor(() => { - expect(screen.getByText("Confirm Making MCP Servers Public")).toBeInTheDocument(); + expect(screen.getByText("Confirm MCP Hub Publication")).toBeInTheDocument(); }); - const submitButton = screen.getByRole("button", { name: "Make Public" }); + const submitButton = screen.getByRole("button", { name: "Save Publication List" }); await act(async () => { fireEvent.click(submitButton); }); @@ -346,6 +432,8 @@ describe("MakeMCPPublicForm", () => { expect(mockMakeMCPPublicCall).toHaveBeenCalledWith("test-token", ["server-2"]); }); + expect(toast.fromError).toHaveBeenCalledWith(error); + // Should not call onSuccess or onClose on error expect(mockProps.onSuccess).not.toHaveBeenCalled(); expect(mockProps.onClose).not.toHaveBeenCalled(); @@ -366,10 +454,10 @@ describe("MakeMCPPublicForm", () => { }); await waitFor(() => { - expect(screen.getByText("Confirm Making MCP Servers Public")).toBeInTheDocument(); + expect(screen.getByText("Confirm MCP Hub Publication")).toBeInTheDocument(); }); - const submitButton = screen.getByRole("button", { name: "Make Public" }); + const submitButton = screen.getByRole("button", { name: "Save Publication List" }); await act(async () => { fireEvent.click(submitButton); }); @@ -381,7 +469,7 @@ describe("MakeMCPPublicForm", () => { expect(mockMakeMCPPublicCall).toHaveBeenCalledTimes(1); expect(mockProps.onSuccess).not.toHaveBeenCalled(); expect(mockProps.onClose).not.toHaveBeenCalled(); - expect(screen.getByText("Confirm Making MCP Servers Public")).toBeInTheDocument(); + expect(screen.getByText("Confirm MCP Hub Publication")).toBeInTheDocument(); resolvePromise({}); await waitFor(() => { @@ -400,7 +488,7 @@ describe("MakeMCPPublicForm", () => { // Modal should not be rendered expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(screen.queryByText("Make MCP Servers Public")).not.toBeInTheDocument(); + expect(screen.queryByText("Manage MCP Hub Visibility")).not.toBeInTheDocument(); }); it("should preselect already public servers when modal opens", () => { @@ -415,7 +503,7 @@ describe("MakeMCPPublicForm", () => { url: "http://example.com/server1", transport: "http", status: "active", - mcp_info: { is_public: false }, // Not public + mcp_info: { is_public: false, is_public_explicit: false }, // Not public allowed_tools: [], auth_type: "bearer", credentials: {}, @@ -437,7 +525,7 @@ describe("MakeMCPPublicForm", () => { url: "http://example.com/server2", transport: "websocket", status: "inactive", - mcp_info: { is_public: true }, // Already public + mcp_info: { is_public: true, is_public_explicit: true }, // Already public allowed_tools: [], auth_type: "none", credentials: {}, @@ -459,7 +547,7 @@ describe("MakeMCPPublicForm", () => { url: "http://example.com/server3", transport: "sse", status: "healthy", - mcp_info: { is_public: true }, // Already public + mcp_info: { is_public: true, is_public_explicit: true }, // Already public allowed_tools: [], auth_type: "oauth", credentials: {}, diff --git a/ui/litellm-dashboard/src/components/AIHub/forms/MakeMCPPublicForm.tsx b/ui/litellm-dashboard/src/components/AIHub/forms/MakeMCPPublicForm.tsx index 8287cf47f1a..2448732a236 100644 --- a/ui/litellm-dashboard/src/components/AIHub/forms/MakeMCPPublicForm.tsx +++ b/ui/litellm-dashboard/src/components/AIHub/forms/MakeMCPPublicForm.tsx @@ -1,5 +1,6 @@ import React, { useState, useEffect } from "react"; import { Loader2 } from "lucide-react"; +import CodeBlock from "@/components/CodeBlock"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { Checkbox } from "@/components/ui/checkbox"; @@ -29,6 +30,11 @@ interface MakeMCPPublicFormProps { onSuccess: () => void; } +interface PublicationSelection { + readonly catalog: MCPServerData[]; + readonly serverIds: Set; +} + const MakeMCPPublicForm: React.FC = ({ visible, onClose, @@ -37,21 +43,28 @@ const MakeMCPPublicForm: React.FC = ({ onSuccess, }) => { const [currentStep, setCurrentStep] = useState(0); - const [selectedServers, setSelectedServers] = useState>(new Set()); + const [selection, setSelection] = useState(null); const [loading, setLoading] = useState(false); + const selectedServers = selection?.serverIds ?? new Set(); + const hasPublicationMetadata = mcpHubData.every((server) => typeof server.mcp_info?.is_public_explicit === "boolean"); + const canManagePublication = hasPublicationMetadata && selection?.catalog === mcpHubData; + const publicationYaml = [ + "litellm_settings:", + " public_mcp_hub_strict_whitelist: true", + selectedServers.size === 0 + ? " public_mcp_servers: []" + : ` public_mcp_servers:\n${Array.from(selectedServers, (id) => ` - ${JSON.stringify(id)}`).join("\n")}`, + ].join("\n"); const handleClose = () => { setCurrentStep(0); - setSelectedServers(new Set()); + setSelection(null); onClose(); }; const handleNext = () => { + if (!canManagePublication) return; if (currentStep === 0) { - if (selectedServers.size === 0) { - toast.fromError("Please select at least one MCP server to make public"); - return; - } setCurrentStep(1); } }; @@ -69,37 +82,32 @@ const MakeMCPPublicForm: React.FC = ({ } else { newSelection.delete(serverId); } - setSelectedServers(newSelection); + setSelection({ catalog: mcpHubData, serverIds: newSelection }); }; const handleSelectAll = (checked: boolean) => { if (checked) { const allServerIds = mcpHubData.map((server) => server.server_id); - setSelectedServers(new Set(allServerIds)); + setSelection({ catalog: mcpHubData, serverIds: new Set(allServerIds) }); } else { - setSelectedServers(new Set()); + setSelection({ catalog: mcpHubData, serverIds: new Set() }); } }; - // Initialize and preselect already public servers when modal opens useEffect(() => { - if (visible && mcpHubData.length > 0) { - // Extract server IDs from servers that are already public - const publicServerIds = mcpHubData - .filter((server) => server.mcp_info?.is_public === true) - .map((server) => server.server_id); - - // Preselect servers that are already public - setSelectedServers(new Set(publicServerIds)); - } - }, [visible]); // Only re-run when modal visibility changes, not when mcpHubData updates - - const handleSubmit = async () => { - if (selectedServers.size === 0) { - toast.fromError("Please select at least one MCP server to make public"); + if (!visible || !hasPublicationMetadata) { + setSelection(null); return; } + const publicServerIds = mcpHubData + .filter((server) => server.mcp_info.is_public_explicit === true) + .map((server) => server.server_id); + setSelection({ catalog: mcpHubData, serverIds: new Set(publicServerIds) }); + setCurrentStep(0); + }, [visible, mcpHubData, hasPublicationMetadata]); + const handleSubmit = async () => { + if (!canManagePublication) return; setLoading(true); try { const serverIdsToMakePublic = Array.from(selectedServers); @@ -107,12 +115,12 @@ const MakeMCPPublicForm: React.FC = ({ // Make batch API call for all servers await makeMCPPublicCall(accessToken, serverIdsToMakePublic); - toast.success(`Successfully made ${serverIdsToMakePublic.length} MCP server(s) public!`); + toast.success("MCP Hub publication list updated"); handleClose(); onSuccess(); } catch (error) { console.error("Error making MCP servers public:", error); - toast.fromError("Failed to make MCP servers public. Please try again."); + toast.fromError(error); } finally { setLoading(false); } @@ -126,7 +134,7 @@ const MakeMCPPublicForm: React.FC = ({ return (
-

Select MCP Servers to Make Public

+

Select MCP Servers for the Hub

- Select the MCP servers you want to be visible on the public model hub. Users will still require a valid - Virtual Key to use these servers. + Select the complete list of MCP servers to publish on the public hub. Uncheck a server to remove it from this + list, or uncheck all to clear it. Authentication and access permissions still apply +

+ +

+ Legacy mode also lists servers with public IP access enabled. Set public_mcp_hub_strict_whitelist to true in + your configuration to use only the publication list

@@ -160,16 +173,22 @@ const MakeMCPPublicForm: React.FC = ({ className="flex items-center space-x-3 p-3 border rounded-lg hover:bg-accent" > handleServerSelection(server.server_id, checked === true)} />

{server.server_name}

- {isPublic && Public} + {isPublic && ( + + {server.mcp_info?.is_public_explicit === false ? "Listed by legacy mode" : "Listed"} + + )} {server.transport} {server.status || "unknown"}
+

{server.server_id}

{server.description || server.url}

@@ -193,6 +212,18 @@ const MakeMCPPublicForm: React.FC = ({
+
+ Configure in YAML +
+

+ Merge these settings into your proxy configuration and reload it. Entries use the server IDs shown above, + not names or aliases. For servers defined in YAML, pin server_id in each existing mcp_servers entry so the + publication list stays stable +

+ +
+
+ {selectedServers.size > 0 && (

@@ -207,19 +238,20 @@ const MakeMCPPublicForm: React.FC = ({ const renderStep2Content = () => { return (

-

Confirm Making MCP Servers Public

+

Confirm MCP Hub Publication

- Warning: Once you make these MCP servers public, anyone who can go to the{" "} - /ui/model_hub_table will be able to know they exist on the proxy. + Anyone who can open /ui/model_hub_table can discover published servers. Explicitly published + server IDs also allow requests from public IPs. Authentication and access permissions still apply

-

MCP Servers to be made public:

+

MCP servers in the publication list:

+ {selectedServers.size === 0 &&

No explicitly published servers

} {Array.from(selectedServers).map((serverId) => { const server = mcpHubData.find((s) => s.server_id === serverId); return ( @@ -248,8 +280,8 @@ const MakeMCPPublicForm: React.FC = ({

- Total: {selectedServers.size} MCP server{selectedServers.size !== 1 ? "s" : ""} will be - made public + Saving replaces the publication list with {selectedServers.size} MCP server + {selectedServers.size !== 1 ? "s" : ""}. Legacy mode may still list servers with public IP access enabled

@@ -257,6 +289,15 @@ const MakeMCPPublicForm: React.FC = ({ }; const renderStepContent = () => { + if (!hasPublicationMetadata) { + return ( +
+ This proxy does not provide explicit publication status for every MCP server. Update the proxy to manage + visibility here, or edit litellm_settings.public_mcp_servers in its existing configuration +
+ ); + } + if (!canManagePublication) return

Loading publication settings

; switch (currentStep) { case 0: return renderStep1Content(); @@ -276,15 +317,15 @@ const MakeMCPPublicForm: React.FC = ({
{currentStep === 0 && ( - )} {currentStep === 1 && ( - )}
@@ -296,7 +337,7 @@ const MakeMCPPublicForm: React.FC = ({ !open && handleClose()} disablePointerDismissal> - Make MCP Servers Public + Manage MCP Hub Visibility
diff --git a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx index be7d39616ca..afeff869b7a 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx @@ -321,6 +321,8 @@ export interface MCPServerCostInfo { // Define MCP provider info export interface MCPInfo { server_name: string; + is_public?: boolean; + is_public_explicit?: boolean; description?: string; logo_url?: string; mcp_server_cost_info?: MCPServerCostInfo | null;