diff --git a/litellm/models/mcp_server.py b/litellm/models/mcp_server.py index 7ccff9434a7..9125d708e79 100644 --- a/litellm/models/mcp_server.py +++ b/litellm/models/mcp_server.py @@ -50,6 +50,7 @@ class LiteLLM_MCPServerTable(LiteLLMPydanticObjectBase): """Represents a LiteLLM_MCPServerTable record""" server_id: str + is_config: bool = Field(default=False, description="Whether this server is defined in config and is read-only.") server_name: str | None = None alias: str | None = None description: str | None = None diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 4a2713cb19c..20c114a2f3e 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -7081,6 +7081,7 @@ class MCPServerManager: def _build_mcp_server_table(self, server: MCPServer) -> LiteLLM_MCPServerTable: return LiteLLM_MCPServerTable( server_id=server.server_id, + is_config=self.is_config_declared_server(server.server_id) and server.server_id not in self.registry, server_name=server.server_name, alias=server.alias, description=(server.mcp_info.get("description") if server.mcp_info else None), diff --git a/litellm/proxy/_lazy_openapi_snapshot.json b/litellm/proxy/_lazy_openapi_snapshot.json index 03122f25870..4dbca14b917 100644 --- a/litellm/proxy/_lazy_openapi_snapshot.json +++ b/litellm/proxy/_lazy_openapi_snapshot.json @@ -24767,6 +24767,12 @@ "title": "Is Byok", "type": "boolean" }, + "is_config": { + "default": false, + "description": "Whether this server is defined in config and is read-only.", + "title": "Is Config", + "type": "boolean" + }, "issuer": { "anyOf": [ { @@ -27817,6 +27823,12 @@ "title": "Is Byok", "type": "boolean" }, + "is_config": { + "default": false, + "description": "Whether this server is defined in config and is read-only.", + "title": "Is Config", + "type": "boolean" + }, "issuer": { "anyOf": [ { 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 9f42a523350..7725aca1948 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 @@ -14470,6 +14470,21 @@ async def test_request_selected_during_guardrail_runs_concurrently_with_tool(mon assert result.content[0].text == "executed" +@pytest.mark.asyncio +@pytest.mark.parametrize("in_config,in_db,expected", [(True, False, True), (False, True, False), (True, True, False)]) +async def test_server_response_identifies_read_only_config(in_config, in_db, expected): + manager = MCPServerManager() + server = MCPServer(server_id="source-server", name="source_server", transport=MCPTransport.http) + manager.config_mcp_servers = {server.server_id: server} if in_config else {} + manager.registry = {server.server_id: server} if in_db else {} + + listed = await manager.get_all_mcp_servers_unfiltered() + + assert len(listed) == 1 + assert listed[0].model_dump().get("is_config") is expected + assert manager._build_mcp_server_table(server).model_dump().get("is_config") is expected + + @pytest.mark.asyncio @pytest.mark.parametrize("with_caller,legacy_factory", [(True, False), (False, False), (True, True)]) async def test_client_sampling_does_not_fill_explicit_context_from_another_ambient_caller(with_caller, legacy_factory): 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 47540eb5d6d..53645e62034 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 @@ -7900,3 +7900,43 @@ class TestGetMcpToolsWireShape: assert tool["outputSchema"] == {"type": "integer"} assert "_meta" in tool assert not {"input_schema", "output_schema", "meta"} & tool.keys() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("role,expected_status", [ + (LitellmUserRoles.PROXY_ADMIN, 404), + (LitellmUserRoles.INTERNAL_USER, 403), +]) +async def test_config_server_edit_preserves_api_contract_without_creating_rows(role, expected_status): + from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager + + manager = MCPServerManager() + server = generate_mock_mcp_server_config_record(server_id="read-only-config") + manager.config_mcp_servers = {server.server_id: server} + original = server.model_dump() + prisma = MagicMock() + prisma.db.litellm_mcpservertable.find_unique = AsyncMock(return_value=None) + prisma.db.litellm_mcpservertable.update = AsyncMock(return_value=None) + with ( + patch.object(mgmt_endpoints, "global_mcp_server_manager", manager), + patch.object(mgmt_endpoints, "get_prisma_client_or_throw", return_value=prisma), + ): + with pytest.raises(HTTPException) as exc: + await mgmt_endpoints.edit_mcp_server( + payload=UpdateMCPServerRequest(server_id=server.server_id, description="UI edit"), + user_api_key_dict=UserAPIKeyAuth(user_id="actor", user_role=role), + ) + + assert exc.value.status_code == expected_status + if role == LitellmUserRoles.PROXY_ADMIN: + assert exc.value.detail == { + "error": f"MCP Server not found, passed server_id={server.server_id}" + } + prisma.db.litellm_mcpservertable.update.assert_awaited_once() + else: + prisma.db.litellm_mcpservertable.update.assert_not_awaited() + prisma.db.litellm_mcpservertable.create.assert_not_called() + prisma.db.litellm_mcpservertable.create_many.assert_not_called() + prisma.tx.assert_not_called() + assert server.model_dump() == original + assert manager.registry == {} diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.test.tsx index da564f23de5..58e1e9bcce9 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/mcp_server_view.test.tsx @@ -4,6 +4,8 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { MCPServerView } from "./mcp_server_view"; import * as networking from "@/components/networking"; +import { setSecureItem } from "@/utils/secureStorage"; +import { EDIT_OAUTH_UI_STATE_KEY } from "./mcp_server_edit"; import type { MCPServer } from "@/components/mcp_tools/types"; vi.mock(".", () => ({ @@ -68,6 +70,7 @@ const openUserCredentials = async (props: Record) => { describe("MCPServerView", () => { beforeEach(() => { vi.clearAllMocks(); + sessionStorage.clear(); }); // Name, alias and description each label the header and a Settings row, so @@ -146,6 +149,37 @@ describe("MCPServerView", () => { expect(screen.queryByRole("button", { name: "Edit Settings" })).not.toBeInTheDocument(); }); + it.each([false, true])("keeps config settings read-only with isEditing=%s", async (isEditing) => { + renderView({ is_config: true }, { isEditing }); + + await userEvent.click(screen.getByRole("tab", { name: "Settings" })); + + expect(screen.getByRole("button", { name: "Edit Settings" })).toBeDisabled(); + expect(screen.getByText("Defined in config. Edit your YAML configuration to make changes")).toBeVisible(); + expect(screen.queryByText("edit form")).not.toBeInTheDocument(); + }); + + it.each([true, false])("honors config read-only state on OAuth return: %s", async (isConfig) => { + setSecureItem(EDIT_OAUTH_UI_STATE_KEY, JSON.stringify({ serverId: "srv-1" })); + renderView({ is_config: isConfig }); + + await userEvent.click(screen.getByRole("tab", { name: "Settings" })); + + if (isConfig) { + expect(screen.queryByText("edit form")).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Edit Settings" })).toBeDisabled(); + } else { + expect(screen.getByText("edit form")).toBeVisible(); + } + }); + + it("does not open the editor for a view-only admin", async () => { + renderView({}, { isViewOnly: true, isEditing: true }); + await userEvent.click(screen.getByRole("tab", { name: "Settings" })); + expect(screen.getByRole("button", { name: "Edit Settings" })).toBeDisabled(); + expect(screen.queryByText("edit form")).not.toBeInTheDocument(); + }); + it("opens on the tab named by initialTabIndex", async () => { renderView({}, { initialTabIndex: 1 }); 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 a346c7d986b..6045be4607d 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 @@ -62,7 +62,8 @@ export const MCPServerView: React.FC = ({ }) => { // Open the editing Settings tab on first render when returning from the edit OAuth // redirect, so the "token fetched" feedback shows where the user left off (Settings=2). - const returningFromEditOAuth = isReturningFromEditOAuth(isProxyAdmin, mcpServer.server_id); + const canEdit = isProxyAdmin && !isViewOnly && !mcpServer.is_config; + const returningFromEditOAuth = isReturningFromEditOAuth(canEdit, mcpServer.server_id); const [editing, setEditing] = useState(isEditing || returningFromEditOAuth); const [showFullUrl, setShowFullUrl] = useState(false); const [copiedStates, setCopiedStates] = useState>({}); @@ -224,13 +225,18 @@ export const MCPServerView: React.FC = ({

MCP Server Settings

- {editing ? null : ( - )}
- {editing ? ( + {mcpServer.is_config && ( +

+ Defined in config. Edit your YAML configuration to make changes +

+ )} + {editing && canEdit ? (