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.
This commit is contained in:
parent
5aabfccf57
commit
d855e56333
@ -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 `<span>`, `<p>`, `<h*>` 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.
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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.
|
||||
|
||||
@ -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<MCPPermissionManagementProps> = ({
|
||||
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<MCPPermissionManagementProps> = ({
|
||||
</div>
|
||||
)}
|
||||
|
||||
{showInternalDelegatePkceWarning && (
|
||||
<Alert
|
||||
type="warning"
|
||||
showIcon
|
||||
className="mb-2"
|
||||
message="Internal server with upstream OAuth delegation"
|
||||
description="This MCP server is configured as internal-only but delegates auth to upstream. Anonymous users will be able to reach the upstream OAuth2 /authorize flow without a LiteLLM session. Ensure your upstream provider and network enforce access controls."
|
||||
/>
|
||||
)}
|
||||
|
||||
<Form.Item
|
||||
label={
|
||||
<span className="text-sm font-medium text-gray-700 flex items-center">
|
||||
|
||||
Loading…
Reference in New Issue
Block a user