chore(mcp): tighten stdio server registration paths (#27570)

Squash-merged by litellm-agent from stuxf's PR.

litellm/proxy/_experimental/mcp_server/mcp_server_manager.py

@@ -806,6 +806,13 @@ class MCPServerManager:
                 self.initialize_tool_name_to_mcp_server_name_mapping()
 
     async def add_server(self, mcp_server: LiteLLM_MCPServerTable):
+        # The runtime registry is the allowlist for tool calls and health
+        # probes (which spawn the underlying transport, including stdio
+        # subprocesses). Match the eligibility set used by the bulk DB
+        # filter in reload_servers_from_database() — NULL is legacy and
+        # "approved" is a legacy alias for "active".
+        if mcp_server.approval_status not in (None, "active", "approved"):
+            return
         try:
             if mcp_server.server_id not in self.registry:
                 new_server = await self.build_mcp_server_from_table(mcp_server)
@@ -819,6 +826,13 @@ class MCPServerManager:
             raise e
 
     async def update_server(self, mcp_server: LiteLLM_MCPServerTable):
+        # If a previously-active server has been moved out of the active
+        # state, evict any stale registry entry so subsequent tool calls and
+        # health probes can't reach it.
+        if mcp_server.approval_status not in (None, "active", "approved"):
+            if mcp_server.server_id in self.registry:
+                del self.registry[mcp_server.server_id]
+            return
         try:
             if mcp_server.server_id in self.registry:
                 new_server = await self.build_mcp_server_from_table(mcp_server)

litellm/proxy/management_endpoints/mcp_management_endpoints.py

@@ -142,6 +142,7 @@ if MCP_AVAILABLE:
         MCPOAuthUserCredentialRequest,
         MCPOAuthUserCredentialStatus,
         MCPSubmissionsSummary,
+        MCPTransport,
         MCPUserCredentialListItem,
         MCPUserCredentialRequest,
         MCPUserCredentialResponse,
@@ -1070,6 +1071,24 @@ if MCP_AVAILABLE:
                 },
             )
 
+        # stdio servers spawn a local subprocess on the proxy host with the
+        # configured command + args, so accepting them from non-admin callers
+        # would let a team member propose a server config that an admin could
+        # rubber-stamp into local code execution. Restrict stdio submission to
+        # the admin POST /v1/mcp/server path or to config.yaml.
+        if payload.transport == MCPTransport.stdio:
+            raise HTTPException(
+                status_code=status.HTTP_400_BAD_REQUEST,
+                detail={
+                    "error": (
+                        "stdio MCP servers cannot be submitted via the user "
+                        "registration workflow. Ask a proxy admin to add this "
+                        "server via POST /v1/mcp/server or to declare it in "
+                        "config.yaml."
+                    )
+                },
+            )
+
         prisma_client = get_prisma_client_or_throw(
             "Database not connected. Connect a database to your proxy"
         )

tests/mcp_tests/test_mcp_server.py

@@ -1494,6 +1494,7 @@ async def test_add_update_server_with_alias():
     mock_mcp_server.created_at = None
     mock_mcp_server.updated_at = None
     mock_mcp_server.instructions = None
+    mock_mcp_server.approval_status = "active"
 
     # Add server to manager
     await test_manager.add_server(mock_mcp_server)
@@ -1551,6 +1552,7 @@ async def test_add_update_server_without_alias():
     mock_mcp_server.created_at = None
     mock_mcp_server.updated_at = None
     mock_mcp_server.instructions = None
+    mock_mcp_server.approval_status = "active"
 
     # Add server to manager
     await test_manager.add_server(mock_mcp_server)
@@ -1609,6 +1611,7 @@ async def test_add_update_server_fallback_to_server_id():
     mock_mcp_server.created_at = None
     mock_mcp_server.updated_at = None
     mock_mcp_server.instructions = None
+    mock_mcp_server.approval_status = "active"
     # Add server to manager
     await test_manager.add_server(mock_mcp_server)
 

tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py

@@ -29,7 +29,11 @@ from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
     MCPServerManager,
     _deserialize_json_dict,
 )
-from litellm.proxy._types import LiteLLM_MCPServerTable, MCPTransport
+from litellm.proxy._types import (
+    LiteLLM_MCPServerTable,
+    MCPApprovalStatus,
+    MCPTransport,
+)
 from litellm.types.mcp import MCPAuth
 from litellm.types.mcp_server.mcp_server_manager import MCPOAuthMetadata, MCPServer
 
@@ -3311,5 +3315,82 @@ class TestOAuthDiscoverySSRFGuard:
         mock_client.get.assert_not_called()
 
 
+class TestApprovalStatusGate:
+    """
+    Regression tests for GHSA-gm4g-h72v-jhc3.
+
+    The runtime registry must only contain servers an admin has approved.
+    A non-admin can submit a pending stdio MCP server with an attacker-chosen
+    command/args; before this gate, an admin opening the per-row endpoint
+    triggered ``add_server`` + ``health_check_server``, which spawned the
+    attacker's process under the proxy. The data-layer gate in
+    ``add_server`` / ``update_server`` blocks pending and rejected rows
+    from entering the registry regardless of which caller passes them in.
+    """
+
+    def _make_server(self, server_id: str, approval_status):
+        return LiteLLM_MCPServerTable(
+            server_id=server_id,
+            alias=f"server_{server_id}",
+            description="test",
+            url=None,
+            transport=MCPTransport.stdio,
+            command="python",
+            args=["-c", "print('attacker payload')"],
+            env={},
+            approval_status=approval_status,
+            created_at=datetime.now(),
+            updated_at=datetime.now(),
+        )
+
+    @pytest.mark.parametrize(
+        "approval_status,expect_in_registry",
+        [
+            (MCPApprovalStatus.pending_review, False),
+            (MCPApprovalStatus.rejected, False),
+            (MCPApprovalStatus.active, True),
+            # Legacy rows: NULL predates the approval workflow; "approved" is
+            # a legacy alias for "active" still present in older deployments.
+            # Both must continue to load to match the DB-level filter in
+            # reload_servers_from_database().
+            (None, True),
+            ("approved", True),
+        ],
+    )
+    async def test_add_server_respects_approval_status(
+        self, approval_status, expect_in_registry
+    ):
+        manager = MCPServerManager()
+        server_id = f"sid-{approval_status}"
+        await manager.add_server(self._make_server(server_id, approval_status))
+        assert (server_id in manager.registry) is expect_in_registry
+
+    async def test_update_server_evicts_when_transitioned_away_from_active(self):
+        # An admin updates a previously-active server to rejected (or pending).
+        # The stale registry entry must be evicted so subsequent tool calls
+        # and health probes can't reach it.
+        manager = MCPServerManager()
+        await manager.add_server(
+            self._make_server("evict-me", MCPApprovalStatus.active)
+        )
+        assert "evict-me" in manager.registry
+
+        await manager.update_server(
+            self._make_server("evict-me", MCPApprovalStatus.rejected)
+        )
+        assert "evict-me" not in manager.registry
+
+    async def test_update_server_noop_for_unregistered_pending(self):
+        # update_server called with a pending row that was never registered
+        # should silently return without adding it. Locks in the early-return
+        # so a future refactor can't accidentally route the pending row to
+        # build_mcp_server_from_table.
+        manager = MCPServerManager()
+        await manager.update_server(
+            self._make_server("never-seen", MCPApprovalStatus.pending_review)
+        )
+        assert "never-seen" not in manager.registry
+
+
 if __name__ == "__main__":
     pytest.main([__file__])

tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py

@@ -2491,6 +2491,32 @@ class TestMCPApprovalWorkflow:
         assert exc_info.value.status_code == 400
         assert "team" in str(exc_info.value.detail).lower()
 
+    @pytest.mark.asyncio
+    async def test_register_mcp_server_rejects_stdio_transport(self):
+        # stdio servers spawn a local subprocess on the proxy host. Accepting
+        # them from the non-admin submission endpoint would let a team member
+        # propose a config that an admin could rubber-stamp into local code
+        # execution. Admins use POST /v1/mcp/server or config.yaml instead.
+        from litellm.proxy.management_endpoints.mcp_management_endpoints import (
+            register_mcp_server,
+        )
+
+        payload = NewMCPServerRequest(
+            alias="local",
+            transport=MCPTransport.stdio,
+            command="python3",
+            args=["-m", "mcp_server_filesystem", "/tmp"],
+        )
+        user_auth = generate_mock_user_api_key_auth(
+            user_role=LitellmUserRoles.INTERNAL_USER,
+            team_id="team-123",
+            user_id="user-abc",
+        )
+        with pytest.raises(HTTPException) as exc_info:
+            await register_mcp_server(payload=payload, user_api_key_dict=user_auth)
+        assert exc_info.value.status_code == 400
+        assert "stdio" in str(exc_info.value.detail).lower()
+
     @pytest.mark.asyncio
     async def test_register_mcp_server_sets_pending_review(self):
         from litellm.proxy._types import MCPApprovalStatus