From d855e5633363d7a90b3022bb854b38492241fd02 Mon Sep 17 00:00:00 2001 From: Sameer Kankute Date: Fri, 15 May 2026 10:05:35 +0530 Subject: [PATCH] chore(mcp): warn on internal + upstream PKCE delegate Log verbose_logger.warning when loading oauth2 interactive servers with available_on_public_internet=false and delegate_auth_to_upstream=true (config + DB). Dashboard Alert for the same combo. CLAUDE note for operators. Tests for log and M2M skip. --- CLAUDE.md | 1 + .../mcp_server/mcp_server_manager.py | 26 ++++++++ .../mcp_server/test_mcp_server_manager.py | 61 +++++++++++++++++++ .../mcp_tools/MCPPermissionManagement.tsx | 18 +++++- 4 files changed, 105 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 938801df7c..baf23c9014 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -117,6 +117,7 @@ LiteLLM is a unified interface for 100+ LLM providers with two main components: - **Always use `antd` for new UI components** — we are migrating off of `@tremor/react`. Do not introduce new `Badge`, `Text`, `Card`, `Grid`, `Title`, or other imports from `@tremor/react` in any new or modified file. Use `antd` equivalents: `Tag` for labels, `Typography.Text` / `Typography.Title` / `Typography.Paragraph` for textual content (avoid plain text-only ``, `

`, `` when Typography fits), and `Card` from `antd`. Note that `antd` has no `"yellow"` Tag color — use `"gold"` for amber/yellow. ### MCP OAuth / OpenAPI Transport Mapping +- **`available_on_public_internet: false` with `delegate_auth_to_upstream: true` (oauth2, interactive — not `client_credentials`)** — LiteLLM still allows the anonymous upstream PKCE path (no proxy API key for `/authorize` and matching MCP routes). The internal-only flag mainly affects other surfaces (e.g. IP-based discovery). Rely on the upstream IdP and network policy; the dashboard shows a warning when both are set, and the proxy logs a warning when the server is loaded from config or the database. - `TRANSPORT.OPENAPI` is a UI-only concept. The backend only accepts `"http"`, `"sse"`, or `"stdio"`. Always map it to `"http"` before any API call (including pre-OAuth temp-session calls). - FastAPI validation errors return `detail` as an array of `{loc, msg, type}` objects. Error extractors must handle: array (map `.msg`), string, nested `{error: string}`, and fallback. - When an MCP server already has `authorization_url` stored, skip OAuth discovery (`_discovery_metadata`) — the server URL for OpenAPI MCPs is the spec file, not the API base, and fetching it causes timeouts. diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 2ed0894207..fd7bcf2fb5 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -145,6 +145,30 @@ def _warn_on_server_name_fields( _warn("server_name", server_name) +def _warn_internal_delegate_pkce_if_applicable( + server: MCPServer, *, source: str +) -> None: + """Surface internal + upstream PKCE delegate in logs for operators.""" + if server.auth_type != MCPAuth.oauth2: + return + if getattr(server, "delegate_auth_to_upstream", False) is not True: + return + if getattr(server, "available_on_public_internet", True): + return + if server.has_client_credentials: + return + label = get_server_prefix(server) + verbose_logger.warning( + "MCP server %r (id=%s, source=%s): internal-only (available_on_public_internet=false) " + "with delegate_auth_to_upstream=true. Anonymous callers can reach the upstream OAuth2 " + "/authorize flow and complete PKCE without a LiteLLM API key session; ensure the " + "upstream IdP and network enforce your access policy.", + label, + server.server_id, + source, + ) + + def _deserialize_json_dict(data: Any) -> Optional[Dict[str, str]]: """ Deserialize optional JSON mappings stored in the database. @@ -425,6 +449,7 @@ class MCPServerManager: ), ) self._assign_unique_short_prefix(new_server) + _warn_internal_delegate_pkce_if_applicable(new_server, source="config") self.config_mcp_servers[server_id] = new_server # Check if this is an OpenAPI-based server @@ -834,6 +859,7 @@ class MCPServerManager: ) or "urn:ietf:params:oauth:token-type:access_token", ) + _warn_internal_delegate_pkce_if_applicable(new_server, source="database") return new_server async def _maybe_register_openapi_tools( 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 794864f658..ef1c09aa81 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 @@ -2633,6 +2633,67 @@ class TestMCPServerTimestamps: assert rebuilt_table.updated_at == updated +class TestInternalDelegatePkceWarningLog: + @pytest.mark.asyncio + async def test_build_mcp_server_logs_on_internal_delegate_interactive(self, caplog): + caplog.set_level(logging.WARNING, logger="LiteLLM") + manager = MCPServerManager() + table_record = LiteLLM_MCPServerTable( + server_id="warn-del-1", + server_name="warn_server", + url="https://example.com/mcp", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + authorization_url="https://idp.example.com/authorize", + token_url="https://idp.example.com/token", + available_on_public_internet=False, + delegate_auth_to_upstream=True, + ) + await manager.build_mcp_server_from_table(table_record) + combined = " ".join(r.getMessage() for r in caplog.records) + assert "internal-only" in combined + assert "delegate_auth_to_upstream=true" in combined + + @pytest.mark.asyncio + async def test_build_mcp_server_no_internal_delegate_log_when_public(self, caplog): + caplog.set_level(logging.WARNING, logger="LiteLLM") + manager = MCPServerManager() + table_record = LiteLLM_MCPServerTable( + server_id="warn-del-2", + server_name="warn_server_pub", + url="https://example.com/mcp", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + authorization_url="https://idp.example.com/authorize", + token_url="https://idp.example.com/token", + available_on_public_internet=True, + delegate_auth_to_upstream=True, + ) + await manager.build_mcp_server_from_table(table_record) + combined = " ".join(r.getMessage() for r in caplog.records) + assert "internal-only" not in combined + + def test_warn_skipped_for_client_credentials(self, caplog): + caplog.set_level(logging.WARNING, logger="LiteLLM") + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + _warn_internal_delegate_pkce_if_applicable, + ) + + server = MCPServer( + server_id="m2m-1", + name="x", + url="https://example.com/mcp", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + oauth2_flow="client_credentials", + available_on_public_internet=False, + delegate_auth_to_upstream=True, + ) + _warn_internal_delegate_pkce_if_applicable(server, source="test") + combined = " ".join(r.getMessage() for r in caplog.records) + assert "internal-only" not in combined + + class TestHasClientCredentialsOAuth2Flow: """ Regression tests for the M2M auto-detection bug. diff --git a/ui/litellm-dashboard/src/components/mcp_tools/MCPPermissionManagement.tsx b/ui/litellm-dashboard/src/components/mcp_tools/MCPPermissionManagement.tsx index 8f60e50d2b..58848df39a 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/MCPPermissionManagement.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/MCPPermissionManagement.tsx @@ -1,5 +1,5 @@ import React, { useEffect } from "react"; -import { Form, Select, Tooltip, Collapse, Input, Space, Button, Switch } from "antd"; +import { Alert, Form, Select, Tooltip, Collapse, Input, Space, Button, Switch } from "antd"; import { InfoCircleOutlined, MinusCircleOutlined, PlusOutlined } from "@ant-design/icons"; import { MCPServer, AUTH_TYPE } from "./types"; const { Panel } = Collapse; @@ -25,6 +25,12 @@ const MCPPermissionManagement: React.FC = ({ const form = Form.useFormInstance(); const watchedAuthType = Form.useWatch("auth_type", form); const isOAuth2 = watchedAuthType === AUTH_TYPE.OAUTH2; + const watchedDelegateAuth = Form.useWatch("delegate_auth_to_upstream", form); + const watchedPublicInternet = Form.useWatch("available_on_public_internet", form); + const showInternalDelegatePkceWarning = + isOAuth2 && + watchedDelegateAuth === true && + watchedPublicInternet === false; // Set initial values when mcpServer changes useEffect(() => { @@ -144,6 +150,16 @@ const MCPPermissionManagement: React.FC = ({ )} + {showInternalDelegatePkceWarning && ( + + )} +