chore(mcp): validate outbound OAuth URLs and fail closed on missing client IP (#27575)

Squash-merged by litellm-agent from stuxf's PR.
This commit is contained in:
stuxf 2026-05-11 18:56:25 -07:00 committed by GitHub
parent 47c0dd2e11
commit ec5b9c2e40
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
17 changed files with 807 additions and 76 deletions

View file

@ -611,6 +611,7 @@ class AsyncHTTPHandler:
logging_obj: Optional[LiteLLMLoggingObject] = None,
files: Optional[RequestFiles] = None,
content: Any = None,
follow_redirects: Optional[bool] = None,
):
start_time = time.time()
try:
@ -633,7 +634,10 @@ class AsyncHTTPHandler:
files=files,
content=request_content,
)
response = await self.client.send(req, stream=stream)
send_kwargs: Dict[str, Any] = {"stream": stream}
if follow_redirects is not None:
send_kwargs["follow_redirects"] = follow_redirects
response = await self.client.send(req, **send_kwargs)
response.raise_for_status()
return response
except (httpx.RemoteProtocolError, httpx.ConnectError):
@ -650,6 +654,7 @@ class AsyncHTTPHandler:
params=params,
headers=headers,
stream=stream,
follow_redirects=follow_redirects,
)
finally:
await new_client.aclose()
@ -853,6 +858,7 @@ class AsyncHTTPHandler:
headers: Optional[dict] = None,
stream: bool = False,
content: Any = None,
follow_redirects: Optional[bool] = None,
):
"""
Making POST request for a single connection client.
@ -865,7 +871,10 @@ class AsyncHTTPHandler:
req = client.build_request(
"POST", url, data=request_data, json=json, params=params, headers=headers, content=request_content # type: ignore
)
response = await client.send(req, stream=stream)
send_kwargs: Dict[str, Any] = {"stream": stream}
if follow_redirects is not None:
send_kwargs["follow_redirects"] = follow_redirects
response = await client.send(req, **send_kwargs)
response.raise_for_status()
return response

View file

@ -213,6 +213,7 @@ class MCPRequestHandler:
# Inline imports avoid a circular dependency: mcp_server_manager imports
# from this module.
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
INTERNAL_REQUEST,
global_mcp_server_manager,
)
from litellm.types.mcp import MCPAuth
@ -229,7 +230,12 @@ class MCPRequestHandler:
return False
for name in target_names:
server = global_mcp_server_manager.get_mcp_server_by_name(name)
# Metadata lookup ("does this server use OAuth2?") used to decide
# whether anonymous OAuth2 fallback is allowed for this path.
# Not an access check — the IP gate doesn't apply.
server = global_mcp_server_manager.get_mcp_server_by_name(
name, client_ip=INTERNAL_REQUEST
)
if server is None or server.auth_type != MCPAuth.oauth2:
return False
return True

View file

@ -1,11 +1,13 @@
import json
from typing import Any, Dict, Optional
from typing import Any, Dict, Literal, Optional
from urllib.parse import parse_qsl, urlencode, urlparse, urlunparse
from fastapi import APIRouter, Form, HTTPException, Request
from fastapi.responses import HTMLResponse, JSONResponse, RedirectResponse
import litellm
from litellm._logging import verbose_logger
from litellm.litellm_core_utils.url_utils import SSRFError, validate_url
from litellm.llms.custom_httpx.http_handler import (
get_async_httpx_client,
httpxSpecialProvider,
@ -25,6 +27,43 @@ from litellm.proxy.utils import get_server_root_path
from litellm.types.mcp import MCPAuth
from litellm.types.mcp_server.mcp_server_manager import MCPServer
def _validate_mcp_oauth_outbound_url(
url: str, role: Literal["token", "registration"]
) -> tuple[str, str]:
"""Validate an admin-configured OAuth URL before the proxy makes a request to
it. The /token, /register and similar endpoints are reachable without a
LiteLLM API key (they sit in the middle of an OAuth handshake), so an
unauthenticated caller can trigger the proxy to POST to whatever host the
admin configured. Resolve and validate the host before sending so an
operator who points an MCP server at an internal IdP doesn't unintentionally
expose it as an SSRF probe via the proxy."""
if not getattr(litellm, "user_url_validation", True):
parsed = urlparse(url)
host = parsed.hostname or ""
host_header = f"{host}:{parsed.port}" if parsed.port else host
return url, host_header
try:
return validate_url(url)
except SSRFError as exc:
# The /token and /register endpoints are reachable without an API key,
# so the error response goes to an unauthenticated caller. The raw
# SSRFError message includes the resolved IP — leaking it would tell
# the caller exactly which internal address the operator's IdP lives at,
# which is the reconnaissance the SSRF guard is meant to deny. Log the
# real reason for operators and return a generic message to the caller.
verbose_logger.warning(
"MCP OAuth %s URL blocked by SSRF validation: %s", role, exc
)
raise HTTPException(
status_code=400,
detail=(
f"Configured MCP {role} URL is not safe to call: "
"the destination resolves to a blocked address."
),
)
router = APIRouter(
tags=["mcp"],
)
@ -393,10 +432,17 @@ async def exchange_token_with_server(
token_data["code_verifier"] = code_verifier
async_client = get_async_httpx_client(llm_provider=httpxSpecialProvider.Oauth2Check)
target_url, host_header = _validate_mcp_oauth_outbound_url(
mcp_server.token_url, role="token"
)
# Disable redirect-following so a malicious 30x from the validated host
# can't bounce the proxy to an internal target — validate_url only
# checked the initial URL.
response = await async_client.post(
mcp_server.token_url,
headers={"Accept": "application/json"},
target_url,
headers={"Accept": "application/json", "Host": host_header},
data=token_data,
follow_redirects=False,
)
response.raise_for_status()
@ -498,10 +544,17 @@ async def register_client_with_server(
async_client = get_async_httpx_client(
llm_provider=httpxSpecialProvider.Oauth2Register
)
target_url, host_header = _validate_mcp_oauth_outbound_url(
mcp_server.registration_url, role="registration"
)
# Disable redirect-following so a malicious 30x from the validated host
# can't bounce the proxy to an internal target — validate_url only
# checked the initial URL.
response = await async_client.post(
mcp_server.registration_url,
headers=headers,
target_url,
headers={**headers, "Host": host_header},
json=register_data,
follow_redirects=False,
)
response.raise_for_status()

View file

@ -300,6 +300,10 @@ class MCPDebug:
server_auth_type: Optional[str] = None
auth_resolution = "no-auth"
# Pass client_ip through unchanged. When IP extraction fails the gate
# fails closed for internal-only servers, so the debug response stays
# empty rather than leaking outbound_url / server_auth_type metadata
# for internal-only servers named in the request.
for server_name in mcp_servers or []:
server = global_mcp_server_manager.get_mcp_server_by_name(
server_name, client_ip=client_ip

View file

@ -166,6 +166,21 @@ def _deserialize_json_dict(data: Any) -> Optional[Dict[str, str]]:
return data
class _InternalRequest:
"""Sentinel passed in place of a client IP to indicate that the call site
is internal (background reload, registry maintenance, admin debug) and
should bypass IP-based access control. External request handlers must pass
a real IP string instead passing ``None`` now fails closed."""
__slots__ = ()
def __repr__(self) -> str: # pragma: no cover - trivial
return "INTERNAL_REQUEST"
INTERNAL_REQUEST = _InternalRequest()
class MCPServerManager:
_STDIO_ENV_TEMPLATE_PATTERN = re.compile(r"^\$\{(X-[^}]+)\}$")
@ -1045,28 +1060,56 @@ class MCPServerManager:
return toolset
def filter_server_ids_by_ip(
self, server_ids: List[str], client_ip: Optional[str]
self,
server_ids: List[str],
client_ip: Union[str, "_InternalRequest", None],
) -> List[str]:
"""
Filter server IDs by client IP external callers only see public servers.
Returns server_ids unchanged when client_ip is None (no filtering).
See ``filter_server_ids_by_ip_with_info`` for the contract on
``client_ip``: ``None`` fails closed, ``INTERNAL_REQUEST`` bypasses
gating, real IPs apply the existing rules.
"""
filtered, _ = self.filter_server_ids_by_ip_with_info(server_ids, client_ip)
return filtered
def _resolve_unknown_client_ip(
self, client_ip: Union[str, "_InternalRequest", None]
) -> Union[str, "_InternalRequest", None]:
"""Promote unknown ``client_ip=None`` to ``INTERNAL_REQUEST`` when the
operator has explicitly opted in via
``general_settings.mcp_allow_unknown_client_ip: true``. Lets
deployments behind ASGI middleware where ``request.client`` is
legitimately ``None`` opt back to the previous fail-open behavior."""
if client_ip is None:
general_settings = self._get_general_settings()
if general_settings.get("mcp_allow_unknown_client_ip", False):
return INTERNAL_REQUEST
return client_ip
def filter_server_ids_by_ip_with_info(
self, server_ids: List[str], client_ip: Optional[str]
self,
server_ids: List[str],
client_ip: Union[str, "_InternalRequest", None],
) -> Tuple[List[str], int]:
"""
Filter server IDs by client IP external callers only see public servers.
Returns (filtered_ids, ip_blocked_count) where ip_blocked_count is the number
of servers that were blocked because the client IP is not allowed to access them.
Returns server_ids unchanged (with 0 blocked) when client_ip is None.
Returns (filtered_ids, ip_blocked_count) where ip_blocked_count is the
number of servers that were blocked because the client IP is not
allowed to access them. ``None`` fails closed: external request
handlers must extract a real IP via
``IPAddressUtils.get_mcp_client_ip(request)``. Internal callers
(admin debug, registry maintenance) should pass
``INTERNAL_REQUEST`` to bypass IP gating.
"""
if client_ip is None:
client_ip = self._resolve_unknown_client_ip(client_ip)
if client_ip is INTERNAL_REQUEST:
return server_ids, 0
# Don't short-circuit on client_ip is None — public servers should
# still be reachable, only internal-only servers fail closed. The
# per-server gate enforces both halves.
allowed = []
blocked = 0
for sid in server_ids:
@ -3066,29 +3109,59 @@ class MCPServerManager:
return {}
def _is_server_accessible_from_ip(
self, server: MCPServer, client_ip: Optional[str]
self,
server: MCPServer,
client_ip: Union[str, _InternalRequest, None],
) -> bool:
"""
Check if a server is accessible from the given client IP.
- If client_ip is None, no IP filtering is applied (internal callers).
- If the server has available_on_public_internet=True, it's always accessible.
- Otherwise, only internal/private IPs can access it.
- ``INTERNAL_REQUEST`` bypasses IP filtering (internal callers, admin
debug, registry maintenance).
- If the server has ``available_on_public_internet=True`` (or is in
``litellm.public_mcp_servers``), it's always accessible — including
when ``client_ip`` is ``None``. Otherwise a request to a
legitimately-public OAuth endpoint behind ASGI middleware that
nulls ``request.client`` would 404 with no actionable diagnostic.
- For non-public servers, ``None`` fails closed: external request
handlers must extract a real IP via
``IPAddressUtils.get_mcp_client_ip(request)``. Earlier behaviour
treated ``None`` as "no filter," which let external callers reach
internal-only servers when IP extraction silently failed. Operators
behind ASGI middleware or load-balancer setups where
``request.client`` is legitimately ``None`` can opt back to the
previous fail-open behaviour by setting
``general_settings.mcp_allow_unknown_client_ip: true``.
- For non-public servers with a real IP, only internal/private IPs
can access it.
"""
if client_ip is None:
client_ip = self._resolve_unknown_client_ip(client_ip)
if client_ip is INTERNAL_REQUEST:
return True
# Public servers are reachable regardless of caller IP — and crucially
# even when IP extraction failed (client_ip is None). Otherwise a
# request to a legitimately-public OAuth endpoint behind an ASGI
# middleware that nulls request.client would 404 with no actionable
# diagnostic for operators.
if server.available_on_public_internet:
return True
# Check backwards compat: litellm.public_mcp_servers
public_ids = set(litellm.public_mcp_servers or [])
if server.server_id in public_ids:
return True
# Non-public server: only accessible from internal IPs
if client_ip is None:
# Non-public server with unknown caller IP fails closed. Earlier
# behaviour treated None as "no filter," letting external callers
# reach internal-only servers when IP extraction silently failed.
return False
# Non-public server: only accessible from internal IPs. The two
# early returns above narrow client_ip to ``str`` for the type
# checker; cast keeps mypy happy without a runtime assert.
general_settings = self._get_general_settings()
internal_networks = IPAddressUtils.parse_internal_networks(
general_settings.get("mcp_internal_ip_ranges")
)
return IPAddressUtils.is_internal_ip(client_ip, internal_networks)
return IPAddressUtils.is_internal_ip(cast(str, client_ip), internal_networks)
def get_mcp_server_by_id(self, server_id: str) -> Optional[MCPServer]:
"""
@ -3179,7 +3252,9 @@ class MCPServerManager:
return result
def get_mcp_server_by_name(
self, server_name: str, client_ip: Optional[str] = None
self,
server_name: str,
client_ip: Union[str, _InternalRequest, None] = None,
) -> Optional[MCPServer]:
"""
Get the MCP Server from the server name.
@ -3191,8 +3266,15 @@ class MCPServerManager:
Args:
server_name: The server name to look up.
client_ip: Optional client IP for access control. When provided,
non-public servers are hidden from external IPs.
client_ip: External request IP for IP-based access control, or
``INTERNAL_REQUEST`` for internal callers (admin debug,
registry maintenance) that intentionally bypass IP
gating. ``None`` fails closed: external request handlers
must extract a real IP via
``IPAddressUtils.get_mcp_client_ip(request)``. Earlier
behaviour silently bypassed gating on ``None``, which
let request handlers that forgot to pass an IP reach
internal-only servers.
"""
registry = self.get_registry()
# Pass 1: Match by alias (highest priority)
@ -3216,18 +3298,23 @@ class MCPServerManager:
return None
def get_filtered_registry(
self, client_ip: Optional[str] = None
self, client_ip: Union[str, "_InternalRequest", None] = None
) -> Dict[str, MCPServer]:
"""
Get registry filtered by client IP access control.
Args:
client_ip: Optional client IP. When provided, non-public servers
are hidden from external IPs. When None, returns all servers.
client_ip: Real client IP (filter applied), ``INTERNAL_REQUEST``
(return full registry, internal callers only), or
``None`` (fails closed: returns empty registry).
External request handlers must pass a real IP.
"""
registry = self.get_registry()
if client_ip is None:
client_ip = self._resolve_unknown_client_ip(client_ip)
if client_ip is INTERNAL_REQUEST:
return registry
# Don't short-circuit on client_ip is None — public servers should
# still be reachable, only internal-only servers fail closed.
return {
k: v
for k, v in registry.items()

View file

@ -380,7 +380,9 @@ if MCP_AVAILABLE:
# Resolve a server name to its UUID if needed
_name_resolved = None
if server_id not in allowed_server_ids:
_name_resolved = global_mcp_server_manager.get_mcp_server_by_name(server_id)
_name_resolved = global_mcp_server_manager.get_mcp_server_by_name(
server_id, client_ip=rest_client_ip
)
if _name_resolved is not None and _name_resolved.server_id in set(
allowed_server_ids
):
@ -393,11 +395,23 @@ if MCP_AVAILABLE:
)
if (
_server is not None
and rest_client_ip is not None
and not global_mcp_server_manager._is_server_accessible_from_ip(
_server, rest_client_ip
)
):
if rest_client_ip is None:
raise HTTPException(
status_code=403,
detail={
"error": "client_ip_unknown",
"message": (
"Cannot determine client IP for IP-based access "
"control. If the proxy is behind a load balancer "
"or reverse proxy, configure use_x_forwarded_for "
"and mcp_trusted_proxy_ranges in general_settings."
),
},
)
raise HTTPException(
status_code=403,
detail={
@ -470,7 +484,9 @@ if MCP_AVAILABLE:
# Resolve a server name to its UUID if needed
_name_resolved = None
if server_id not in allowed_server_ids:
_name_resolved = global_mcp_server_manager.get_mcp_server_by_name(server_id)
_name_resolved = global_mcp_server_manager.get_mcp_server_by_name(
server_id, client_ip=rest_client_ip
)
if _name_resolved is not None and _name_resolved.server_id in set(
allowed_server_ids
):
@ -483,11 +499,23 @@ if MCP_AVAILABLE:
)
if (
_server is not None
and rest_client_ip is not None
and not global_mcp_server_manager._is_server_accessible_from_ip(
_server, rest_client_ip
)
):
if rest_client_ip is None:
raise HTTPException(
status_code=403,
detail={
"error": "client_ip_unknown",
"message": (
"Cannot determine client IP for IP-based access "
"control. If the proxy is behind a load balancer "
"or reverse proxy, configure use_x_forwarded_for "
"and mcp_trusted_proxy_ranges in general_settings."
),
},
)
raise HTTPException(
status_code=403,
detail={

View file

@ -154,6 +154,7 @@ if MCP_AVAILABLE:
)
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
MCPServerManager,
_InternalRequest,
global_mcp_server_manager,
)
from litellm.proxy._experimental.mcp_server.openapi_to_mcp_generator import (
@ -850,27 +851,24 @@ if MCP_AVAILABLE:
async def _get_allowed_mcp_servers(
user_api_key_auth: Optional[UserAPIKeyAuth],
mcp_servers: Optional[List[str]],
client_ip: Optional[str] = None,
client_ip: Union[str, _InternalRequest, None] = None,
) -> List[MCPServer]:
"""Return allowed MCP servers for a request after applying filters.
Args:
user_api_key_auth: The authenticated user's API key info.
mcp_servers: Optional list of server names to filter to.
client_ip: Client IP for IP-based access control. If None, falls back to
auth context. Pass explicitly from request handlers for safety.
Note: If client_ip is None and auth context is not set, IP filtering is skipped.
This is intentional for internal callers but may indicate a bug if called
from a request handler without proper context setup.
client_ip: Client IP for IP-based access control. Pass an explicit
string from external request handlers, the
``INTERNAL_REQUEST`` sentinel from internal callers
(admin debug, registry maintenance, background work),
or ``None`` to fall back to the auth-context IP.
``None`` after the auth-context fallback fails closed
an external request that can't be attributed to an IP
will not reach internal-only servers.
"""
# Use explicit client_ip if provided, otherwise try auth context
if client_ip is None:
client_ip = _get_client_ip_from_context()
if client_ip is None:
verbose_logger.debug(
"MCP _get_allowed_mcp_servers called without client_ip and no auth context. "
"IP filtering will be skipped. This is expected for internal calls."
)
allowed_mcp_server_ids = (
await global_mcp_server_manager.get_allowed_mcp_servers(user_api_key_auth)
@ -887,15 +885,28 @@ if MCP_AVAILABLE:
allowed_mcp_server_ids,
)
if _ip_blocked > 0:
verbose_logger.debug(
"MCP IP filtering: %d server(s) are not accessible from client IP %s "
"because they are restricted to internal networks. "
"No tools from those servers will be returned. "
"To expose a server externally, set 'available_on_public_internet: true' "
"in its configuration.",
_ip_blocked,
client_ip,
)
if client_ip is None:
# IP extraction failed (no X-Forwarded-* header, missing
# trusted-proxy config, etc.). Fail-closed at the gate
# silently dropped the servers — tell the operator to fix
# IP forwarding, NOT to expose the server publicly.
verbose_logger.debug(
"MCP IP filtering: %d server(s) hidden because client IP "
"could not be determined for this request. Fix request-IP "
"extraction (X-Forwarded-For + trusted_proxies) so the "
"gate can evaluate access.",
_ip_blocked,
)
else:
verbose_logger.debug(
"MCP IP filtering: %d server(s) are not accessible from "
"client IP %s because they are restricted to internal "
"networks. No tools from those servers will be returned. "
"To expose a server externally, set "
"'available_on_public_internet: true' in its configuration.",
_ip_blocked,
client_ip,
)
allowed_mcp_servers: List[MCPServer] = []
for allowed_mcp_server_id in allowed_mcp_server_ids:
mcp_server = global_mcp_server_manager.get_mcp_server_by_id(

View file

@ -1557,14 +1557,35 @@ if MCP_AVAILABLE:
if server is None:
# Fall back to real DB/config server (e.g. for the user-side OAuth flow
# which calls these endpoints with a real server_id, not a temp session id).
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
INTERNAL_REQUEST,
)
from litellm.proxy.auth.ip_address_utils import IPAddressUtils
client_ip = IPAddressUtils.get_mcp_client_ip(request) if request else None
server = global_mcp_server_manager.get_mcp_server_by_id(
server_id
) or global_mcp_server_manager.get_mcp_server_by_name(
server_id, client_ip=client_ip
# Programmatic callers (no Request) bypass the IP gate explicitly.
# Real HTTP callers go through the normal extraction path.
client_ip = (
IPAddressUtils.get_mcp_client_ip(request)
if request
else INTERNAL_REQUEST
)
# get_mcp_server_by_id alone does not apply IP gating, so an
# external caller hitting /server/oauth/{server_id}/{authorize,
# token,register} with the UUID of an internal-only server would
# bypass the IP restriction. Apply the gate to the id-lookup
# result before falling back to the name lookup (which gates).
server = global_mcp_server_manager.get_mcp_server_by_id(server_id)
if (
server is not None
and not global_mcp_server_manager._is_server_accessible_from_ip(
server, client_ip
)
):
server = None
if server is None:
server = global_mcp_server_manager.get_mcp_server_by_name(
server_id, client_ip=client_ip
)
if server is None:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,

View file

@ -191,6 +191,7 @@ class LiteLLM_Proxy_MCP_Handler:
List names of allowed MCP servers
"""
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
INTERNAL_REQUEST,
global_mcp_server_manager,
)
from litellm.proxy._experimental.mcp_server.server import (
@ -216,7 +217,13 @@ class LiteLLM_Proxy_MCP_Handler:
resolved_mcp_servers: List[str] = []
resolved_toolset_ids: List[str] = []
for name in mcp_servers:
if not global_mcp_server_manager.get_mcp_server_by_name(name):
# Server-side disambiguation between MCP server names and toolset
# names. Access control is enforced downstream by
# _get_allowed_mcp_servers_from_mcp_server_names; this lookup is
# purely "is `name` a known server?", so bypass the IP gate.
if not global_mcp_server_manager.get_mcp_server_by_name(
name, client_ip=INTERNAL_REQUEST
):
try:
from litellm.proxy.proxy_server import prisma_client

View file

@ -798,3 +798,40 @@ def test_get_httpx_client_applies_httpx_timeout_object_without_mocking_handler()
assert handler.client.timeout == t
finally:
handler.close()
@pytest.mark.asyncio
async def test_post_retry_propagates_follow_redirects():
"""
AsyncHTTPHandler.post() retries connection errors via
single_connection_post_request(). When a caller passes
follow_redirects=False (e.g. the MCP OAuth SSRF guard), the retry
must honor it too otherwise a transient connection error followed
by a 30x on retry can bypass the redirect block and hit an internal
address.
"""
from unittest.mock import AsyncMock
handler = AsyncHTTPHandler()
try:
with (
patch.object(
handler.client,
"send",
side_effect=httpx.RemoteProtocolError("forced retry"),
),
patch.object(
handler,
"single_connection_post_request",
new=AsyncMock(return_value=MagicMock(status_code=200)),
) as mock_retry,
):
await handler.post("https://example.com/token", follow_redirects=False)
assert mock_retry.await_count == 1
kwargs = mock_retry.await_args.kwargs
assert kwargs.get("follow_redirects") is False, (
"follow_redirects must reach the retry path so the SSRF "
"redirect block holds across reconnect"
)
finally:
await handler.close()

View file

@ -6,19 +6,19 @@ import pytest
from fastapi import HTTPException
# Fixture to mock IP address check for all MCP tests
# This prevents tests from failing due to IP-based access control
# Fixture to bypass MCP IP-based access control for all OAuth flow tests.
# Mock requests don't carry a real client IP context; the bypass uses the
# explicit INTERNAL_REQUEST sentinel because passing None now fails closed
# in the gate function (see MCPServerManager._is_server_accessible_from_ip).
@pytest.fixture(autouse=True)
def mock_mcp_client_ip():
"""Mock IPAddressUtils.get_mcp_client_ip to return None for all tests.
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
INTERNAL_REQUEST,
)
This bypasses IP-based access control in tests, since the MCP server's
available_on_public_internet defaults to False and mock requests don't
have proper client IP context.
"""
with patch(
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.IPAddressUtils.get_mcp_client_ip",
return_value=None,
return_value=INTERNAL_REQUEST,
):
yield
@ -91,6 +91,7 @@ async def test_authorize_endpoint_includes_response_type():
# Mock request
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm.example.com/"
mock_request.headers = {}
@ -156,6 +157,7 @@ async def test_authorize_endpoint_preserves_existing_query_params():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm.example.com/"
mock_request.headers = {}
@ -223,6 +225,7 @@ async def test_authorize_endpoint_forwards_pkce_parameters():
# Mock request
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm-proxy.example.com/"
mock_request.headers = {}
@ -294,6 +297,7 @@ async def test_token_endpoint_forwards_code_verifier():
# Mock request
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm-proxy.example.com/"
mock_request.headers = {}
@ -371,6 +375,7 @@ async def test_register_client_without_mcp_server_name_returns_dummy():
global_mcp_server_manager.registry.clear()
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://proxy.litellm.example/"
mock_request.headers = {}
with patch(
@ -419,6 +424,7 @@ async def test_register_client_returns_existing_server_credentials():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://proxy.litellm.example/"
mock_request.headers = {}
@ -474,6 +480,7 @@ async def test_register_client_remote_registration_success():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://proxy.litellm.example/"
mock_request.headers = {}
@ -503,6 +510,13 @@ async def test_register_client_remote_registration_success():
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.get_async_httpx_client",
return_value=mock_async_client,
),
patch(
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.validate_url",
return_value=(
"https://provider.example/oauth/register",
"provider.example",
),
),
):
response = await register_client(
request=mock_request, mcp_server_name=oauth2_server.server_name
@ -522,6 +536,7 @@ async def test_register_client_remote_registration_success():
assert call_args.kwargs["headers"] == {
"Content-Type": "application/json",
"Accept": "application/json",
"Host": "provider.example",
}
assert call_args.kwargs["json"]["redirect_uris"] == [
"https://proxy.litellm.example/callback"
@ -573,6 +588,7 @@ async def test_authorize_endpoint_respects_x_forwarded_proto():
# Mock request with http base_url but X-Forwarded-Proto: https
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "http://litellm.example.com/" # HTTP
mock_request.headers = {"X-Forwarded-Proto": "https"} # Behind HTTPS proxy
@ -641,6 +657,7 @@ async def test_token_endpoint_respects_x_forwarded_proto():
# Mock request with http base_url but X-Forwarded-Proto: https
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "http://litellm-proxy.example.com/" # HTTP
mock_request.headers = {"X-Forwarded-Proto": "https"} # Behind HTTPS proxy
@ -719,6 +736,7 @@ async def test_oauth_protected_resource_respects_x_forwarded_proto():
# Mock request with http base_url but X-Forwarded-Proto: https
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "http://litellm.example.com/" # HTTP
mock_request.headers = {"X-Forwarded-Proto": "https"} # Behind HTTPS proxy
@ -774,6 +792,7 @@ async def test_oauth_authorization_server_respects_x_forwarded_proto():
# Mock request with http base_url but X-Forwarded-Proto: https
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "http://litellm.example.com/" # HTTP
mock_request.headers = {"X-Forwarded-Proto": "https"} # Behind HTTPS proxy
@ -812,6 +831,7 @@ async def test_register_client_respects_x_forwarded_proto():
# Mock request with http base_url but X-Forwarded-Proto: https
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "http://proxy.litellm.example/" # HTTP
mock_request.headers = {"X-Forwarded-Proto": "https"} # Behind HTTPS proxy
@ -871,6 +891,7 @@ async def test_authorize_endpoint_respects_x_forwarded_host():
# Internal: http://localhost:8888/github/mcp
# External: https://proxy.example.com/github/mcp
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "http://localhost:8888/github/mcp"
mock_request.headers = {
"X-Forwarded-Proto": "https",
@ -943,6 +964,7 @@ async def test_token_endpoint_respects_x_forwarded_host():
# Mock request simulating nginx proxy without port in host
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "http://localhost:8888/github/mcp"
mock_request.headers = {
"X-Forwarded-Proto": "https",
@ -1123,6 +1145,7 @@ def test_get_request_base_url_comprehensive(
pytest.skip("MCP discoverable endpoints not available")
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = base_url
headers = {}
@ -1209,6 +1232,7 @@ def test_get_request_base_url_xff_trust_gate(
pytest.skip("MCP discoverable endpoints not available")
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "http://localhost:4000/"
mock_request.client = MagicMock()
mock_request.client.host = direct_ip
@ -1253,6 +1277,7 @@ def test_xff_misconfig_warning_emitted_once(caplog):
ip_address_utils._warned_xff_without_trusted_ranges = False
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "http://localhost:4000/"
mock_request.client = MagicMock()
mock_request.client.host = "203.0.113.5"
@ -1323,6 +1348,7 @@ async def test_oauth_protected_resource_returns_empty_scopes_when_none():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm.example.com/"
mock_request.headers = {}
@ -1377,6 +1403,7 @@ async def test_oauth_authorization_server_returns_empty_scopes_when_none():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm.example.com/"
mock_request.headers = {}
@ -1445,6 +1472,7 @@ async def test_authorize_root_resolves_single_oauth2_server():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://llm.example.com/"
mock_request.headers = {}
@ -1498,6 +1526,7 @@ async def test_authorize_root_fails_with_multiple_oauth2_servers():
global_mcp_server_manager.registry[server2.server_id] = server2
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://llm.example.com/"
mock_request.headers = {}
@ -1536,6 +1565,7 @@ async def test_authorize_root_does_not_resolve_private_server_for_external_clien
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://llm.example.com/"
mock_request.headers = {}
@ -1577,6 +1607,7 @@ async def test_token_root_resolves_single_oauth2_server():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://llm.example.com/"
mock_request.headers = {}
@ -1642,6 +1673,7 @@ async def test_token_root_does_not_resolve_private_server_for_external_client():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://llm.example.com/"
mock_request.headers = {}
@ -1686,6 +1718,7 @@ async def test_register_root_resolves_single_oauth2_server():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://llm.example.com/"
mock_request.headers = {}
@ -1723,6 +1756,7 @@ async def test_register_root_does_not_resolve_private_server_for_external_client
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://llm.example.com/"
mock_request.headers = {}
@ -1765,6 +1799,7 @@ async def test_discovery_root_includes_server_name_prefix():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://llm.example.com/"
mock_request.headers = {}
@ -1805,6 +1840,7 @@ async def test_discovery_root_does_not_expose_private_server_for_external_client
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://llm.example.com/"
mock_request.headers = {}
@ -2006,6 +2042,7 @@ async def test_oauth_authorize_includes_scopes_from_server_config():
)
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm.example.com/"
mock_request.headers = {}
@ -2064,6 +2101,7 @@ async def test_oauth_authorize_prefers_request_scope_over_server_config():
)
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm.example.com/"
mock_request.headers = {}
@ -2133,6 +2171,7 @@ async def test_token_endpoint_refresh_token_grant():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://proxy.litellm.example/"
mock_request.headers = {}
@ -2267,6 +2306,7 @@ async def test_authorize_endpoint_rejects_non_loopback_redirect_uri():
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm.example.com/"
mock_request.headers = {}
@ -2314,6 +2354,7 @@ async def test_authorize_endpoint_accepts_ipv4_loopback_range_and_ipv6_full_form
global_mcp_server_manager.registry[oauth2_server.server_id] = oauth2_server
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm.example.com/"
mock_request.headers = {}
@ -2443,6 +2484,7 @@ async def test_token_endpoint_sets_no_store_cache_control():
token_url="https://provider.com/oauth/token",
)
mock_request = MagicMock(spec=Request)
mock_request.client = MagicMock(host="127.0.0.1")
mock_request.base_url = "https://litellm.example.com/"
mock_request.headers = {}
@ -2473,3 +2515,88 @@ async def test_token_endpoint_sets_no_store_cache_control():
assert response.headers["cache-control"] == "no-store"
assert response.headers["pragma"] == "no-cache"
class TestOutboundOAuthURLValidation:
"""
/token and /register can be hit without a LiteLLM API key (the caller is
mid-OAuth-handshake). They forward the request to whatever URL the admin
configured for the MCP server. Lock the new contract: the outbound URL is
SSRF-validated via litellm_core_utils.url_utils.validate_url before any
proxy POST, so an admin-configured internal IdP can't be probed by an
unauthenticated caller through the proxy.
"""
def test_validator_rejects_internal_url(self):
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
_validate_mcp_oauth_outbound_url,
)
with pytest.raises(HTTPException) as exc_info:
_validate_mcp_oauth_outbound_url(
"http://127.0.0.1:8080/token", role="token"
)
assert exc_info.value.status_code == 400
detail = str(exc_info.value.detail)
assert "token" in detail
# The /token endpoint is unauthenticated — leaking the resolved IP
# would hand reconnaissance to the caller. Detail must stay generic.
assert "127.0.0.1" not in detail
assert "blocked address" in detail
def test_validator_rejects_rfc1918_url(self):
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
_validate_mcp_oauth_outbound_url,
)
with pytest.raises(HTTPException) as exc_info:
_validate_mcp_oauth_outbound_url(
"http://192.168.1.10/oauth/register", role="registration"
)
detail = str(exc_info.value.detail)
assert "192.168.1.10" not in detail
assert "registration" in detail
def test_validator_passes_when_validation_disabled(self):
# Operators who explicitly opt out via litellm.user_url_validation = False
# still get the URL passed through unchanged.
import litellm
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
_validate_mcp_oauth_outbound_url,
)
original = getattr(litellm, "user_url_validation", True)
try:
litellm.user_url_validation = False
url, host = _validate_mcp_oauth_outbound_url(
"http://10.0.0.1:5000/token", role="token"
)
assert url == "http://10.0.0.1:5000/token"
# Host header includes the explicit port from the URL.
assert host == "10.0.0.1:5000"
# When the URL omits the port, the Host header omits it too.
url2, host2 = _validate_mcp_oauth_outbound_url(
"http://10.0.0.1/token", role="token"
)
assert host2 == "10.0.0.1"
finally:
litellm.user_url_validation = original
def test_validator_accepts_public_url(self):
# The public-URL acceptance path does live DNS resolution, which is
# flaky in CI. Patch validate_url to confirm the helper passes the
# validated tuple through.
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
_validate_mcp_oauth_outbound_url,
)
with patch(
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.validate_url",
return_value=("https://provider.example/token", "provider.example"),
):
url, host = _validate_mcp_oauth_outbound_url(
"https://provider.example/token", role="token"
)
assert url == "https://provider.example/token"
assert host == "provider.example"

View file

@ -265,3 +265,68 @@ class TestWrapSendWithDebugHeaders:
asyncio.run(wrapped(body_msg))
assert captured[0] == body_msg
class TestMaybeBuildDebugHeadersIPGate:
"""
The maybe_build_debug_headers entry point used to promote a missing
client_ip to INTERNAL_REQUEST so debug metadata wasn't silently dropped.
That bypass let an external caller (in deployments where IP extraction
fails) read x-mcp-debug-outbound-url / x-mcp-debug-server-auth-type for
internal-only servers named in the request the IP gate would
otherwise hide them. Lock the fixed contract: pass client_ip straight
through so the gate fails closed for internal-only servers when IP
extraction fails.
"""
def _debug_headers_for(self, client_ip, *, server_is_internal_only=True):
from unittest.mock import patch
server = MagicMock()
server.url = "https://internal.example/mcp"
server.auth_type = "oauth2"
server.alias = "alias"
server.server_name = "server"
server.has_client_credentials = False
server.authentication_token = None
manager = MagicMock()
# The fix forwards client_ip to get_mcp_server_by_name; we mirror the
# gate's contract: None on an internal-only server returns None,
# while a real internal IP returns the server.
def _by_name(name, client_ip):
if server_is_internal_only and client_ip is None:
return None
return server
manager.get_mcp_server_by_name.side_effect = _by_name
with patch(
"litellm.proxy._experimental.mcp_server.mcp_server_manager."
"global_mcp_server_manager",
manager,
):
return MCPDebug.maybe_build_debug_headers(
raw_headers={MCP_DEBUG_REQUEST_HEADER: "true"},
scope={"type": "http", "headers": []},
mcp_servers=["internal-only-server"],
mcp_auth_header=None,
mcp_server_auth_headers=None,
oauth2_headers=None,
client_ip=client_ip,
)
def test_unknown_ip_does_not_leak_internal_server_metadata(self):
headers = self._debug_headers_for(client_ip=None, server_is_internal_only=True)
# Outbound URL must NOT echo the internal-only server's URL.
assert headers.get("x-mcp-debug-outbound-url") == "(unknown)"
assert headers.get("x-mcp-debug-server-auth-type") == "(none)"
assert headers.get("x-mcp-debug-auth-resolution") == "no-auth"
def test_internal_ip_still_resolves_metadata(self):
headers = self._debug_headers_for(
client_ip="10.0.0.1", server_is_internal_only=True
)
assert headers.get("x-mcp-debug-outbound-url") == "https://internal.example/mcp"
assert headers.get("x-mcp-debug-server-auth-type") == "oauth2"

View file

@ -1154,6 +1154,12 @@ async def test_mcp_routing_with_conflicting_alias_and_group_name():
"litellm.proxy._experimental.mcp_server.server.global_mcp_server_manager._get_tools_from_server",
mock_get_tools_spy,
),
# Unit test has no HTTP request context — give the IP gate a real
# internal IP so it doesn't fail closed and filter every server out.
patch(
"litellm.proxy._experimental.mcp_server.server._get_client_ip_from_context",
return_value="127.0.0.1",
),
):
mcp_servers_from_path = _get_mcp_servers_in_path(test_path)

View file

@ -3311,5 +3311,148 @@ class TestOAuthDiscoverySSRFGuard:
mock_client.get.assert_not_called()
class TestIPGatingFailClosed:
"""
The IP-gating helper used to fail open when ``client_ip`` was ``None``,
which let request handlers that couldn't determine a client IP reach
internal-only MCP servers. Lock the new contract: ``None`` fails closed,
``INTERNAL_REQUEST`` bypasses gating, real IPs apply the existing rules.
"""
def _internal_server(self):
from litellm.types.mcp_server.mcp_server_manager import MCPServer
return MCPServer(
server_id="internal-1",
name="internal",
server_name="internal",
transport=MCPTransport.http,
url="https://internal.local/mcp",
available_on_public_internet=False,
)
def _public_server(self):
from litellm.types.mcp_server.mcp_server_manager import MCPServer
return MCPServer(
server_id="public-1",
name="public",
server_name="public",
transport=MCPTransport.http,
url="https://public.example/mcp",
available_on_public_internet=True,
)
@pytest.mark.parametrize(
"server_kind,client_ip_kind,expected",
[
# None fails closed for non-public servers — missing IP signals
# an external request that couldn't be attributed and must not
# reach internal-only servers.
("internal", "none", False),
# None still lets public servers through — otherwise a request
# to a public OAuth endpoint behind ASGI middleware that nulls
# request.client would 404 with no actionable diagnostic.
("public", "none", True),
# INTERNAL_REQUEST bypasses gating for both visibilities.
("internal", "internal_request", True),
("public", "internal_request", True),
# Real external IP applies the existing visibility rules.
("internal", "external", False),
("public", "external", True),
],
)
def test_gate_contract(self, server_kind, client_ip_kind, expected):
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
INTERNAL_REQUEST,
MCPServerManager,
)
manager = MCPServerManager()
server = (
self._internal_server()
if server_kind == "internal"
else self._public_server()
)
client_ip = {
"none": None,
"internal_request": INTERNAL_REQUEST,
"external": "8.8.8.8",
}[client_ip_kind]
assert manager._is_server_accessible_from_ip(server, client_ip) is expected
def test_get_filtered_registry_fails_closed_on_none(self):
# The wrapper used to return the full registry on None client_ip,
# which let unauth callers (e.g. _resolve_oauth2_server_for_root_endpoints
# when get_mcp_client_ip returned None) auto-select internal-only
# servers. Now fails closed; INTERNAL_REQUEST is the explicit bypass.
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
INTERNAL_REQUEST,
MCPServerManager,
)
manager = MCPServerManager()
server = self._internal_server()
manager.registry[server.server_id] = server
assert manager.get_filtered_registry() == {}
assert manager.get_filtered_registry(client_ip=None) == {}
assert manager.get_filtered_registry(client_ip="8.8.8.8") == {}
assert manager.get_filtered_registry(client_ip=INTERNAL_REQUEST) == {
server.server_id: server
}
def test_filter_server_ids_by_ip_fails_closed_on_none(self):
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
INTERNAL_REQUEST,
MCPServerManager,
)
manager = MCPServerManager()
server = self._internal_server()
manager.registry[server.server_id] = server
sids = [server.server_id]
# None fails closed: zero allowed, all reported as blocked.
allowed, blocked = manager.filter_server_ids_by_ip_with_info(sids, None)
assert (allowed, blocked) == ([], 1)
# External IP applies filter — internal server blocked.
allowed, blocked = manager.filter_server_ids_by_ip_with_info(sids, "8.8.8.8")
assert (allowed, blocked) == ([], 1)
# INTERNAL_REQUEST bypasses gating.
allowed, blocked = manager.filter_server_ids_by_ip_with_info(
sids, INTERNAL_REQUEST
)
assert (allowed, blocked) == (sids, 0)
def test_get_mcp_server_by_name_fails_closed_on_none(self):
# External request handlers must extract a real client IP. Passing
# None silently bypassed gating before; now it fails closed.
# Internal callers (admin debug, registry maintenance) must use
# INTERNAL_REQUEST explicitly.
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
INTERNAL_REQUEST,
MCPServerManager,
)
manager = MCPServerManager()
server = self._internal_server()
manager.registry[server.server_id] = server
# Default client_ip is None — fails closed for internal-only servers.
result = manager.get_mcp_server_by_name("internal")
assert result is None
# External IP can't reach a non-public server.
result = manager.get_mcp_server_by_name("internal", client_ip="8.8.8.8")
assert result is None
# INTERNAL_REQUEST sentinel bypasses gating for internal callers.
result = manager.get_mcp_server_by_name("internal", client_ip=INTERNAL_REQUEST)
assert result is server
if __name__ == "__main__":
pytest.main([__file__])

View file

@ -595,7 +595,9 @@ class TestListToolsRestAPI:
monkeypatch.setattr(
rest_endpoints.global_mcp_server_manager,
"get_mcp_server_by_name",
lambda name: stub_server if name == "my-server" else None,
lambda name, client_ip=None, **kwargs: (
stub_server if name == "my-server" else None
),
raising=False,
)
monkeypatch.setattr(
@ -658,7 +660,9 @@ class TestListToolsRestAPI:
monkeypatch.setattr(
rest_endpoints.global_mcp_server_manager,
"get_mcp_server_by_name",
lambda name: stub_server if name == "restricted-server" else None,
lambda name, client_ip=None, **kwargs: (
stub_server if name == "restricted-server" else None
),
raising=False,
)
monkeypatch.setattr(

View file

@ -85,12 +85,49 @@ class TestMCPServerIPFiltering:
@patch("litellm.public_mcp_servers", [])
@patch("litellm.proxy.proxy_server.general_settings", {})
def test_no_ip_means_no_filtering(self):
def test_no_ip_fails_closed(self):
# Missing client_ip on an external request fails closed; internal
# callers must pass INTERNAL_REQUEST explicitly to bypass IP gating.
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
INTERNAL_REQUEST,
)
priv = _make_server("priv", available_on_public_internet=False)
manager = _make_manager([priv])
result = manager.filter_server_ids_by_ip(["priv"], client_ip=None)
assert result == ["priv"]
assert manager.filter_server_ids_by_ip(["priv"], client_ip=None) == []
assert manager.filter_server_ids_by_ip(
["priv"], client_ip=INTERNAL_REQUEST
) == ["priv"]
@patch("litellm.public_mcp_servers", [])
@patch(
"litellm.proxy.proxy_server.general_settings",
{"mcp_allow_unknown_client_ip": True},
)
def test_unknown_client_ip_opt_out_restores_fail_open(self):
# Operators behind ASGI middleware where request.client is legitimately
# None can opt back to the pre-fix fail-open behavior with explicit
# consent via general_settings.mcp_allow_unknown_client_ip: true.
priv = _make_server("priv", available_on_public_internet=False)
manager = _make_manager([priv])
assert manager.filter_server_ids_by_ip(["priv"], client_ip=None) == ["priv"]
@patch("litellm.public_mcp_servers", [])
@patch("litellm.proxy.proxy_server.general_settings", {})
def test_public_server_reachable_when_ip_unknown(self):
# A legitimately-public server must stay reachable even when IP
# extraction failed (client_ip=None). Otherwise a request to a
# public OAuth endpoint behind ASGI middleware that nulls
# request.client would 404 with no actionable diagnostic.
pub = _make_server("pub", available_on_public_internet=True)
priv = _make_server("priv", available_on_public_internet=False)
manager = _make_manager([pub, priv])
assert manager.filter_server_ids_by_ip(["pub", "priv"], client_ip=None) == [
"pub"
]
class TestFilterServerIdsByIpWithInfo:
@ -124,15 +161,40 @@ class TestFilterServerIdsByIpWithInfo:
@patch("litellm.public_mcp_servers", [])
@patch("litellm.proxy.proxy_server.general_settings", {})
def test_no_ip_returns_all_with_zero_blocked(self):
def test_no_ip_fails_closed_reports_all_blocked(self):
# Missing client_ip on an external request fails closed: all servers
# are reported blocked. Internal callers pass INTERNAL_REQUEST.
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
INTERNAL_REQUEST,
)
priv = _make_server("priv", available_on_public_internet=False)
manager = _make_manager([priv])
allowed, blocked = manager.filter_server_ids_by_ip_with_info(
["priv"], client_ip=None
)
assert allowed == ["priv"]
assert blocked == 0
assert (allowed, blocked) == ([], 1)
allowed, blocked = manager.filter_server_ids_by_ip_with_info(
["priv"], client_ip=INTERNAL_REQUEST
)
assert (allowed, blocked) == (["priv"], 0)
@patch("litellm.public_mcp_servers", [])
@patch("litellm.proxy.proxy_server.general_settings", {})
def test_no_ip_lets_public_through_blocks_private(self):
# When IP extraction fails (client_ip=None), public servers stay
# reachable; only non-public servers are blocked.
pub = _make_server("pub", available_on_public_internet=True)
priv = _make_server("priv", available_on_public_internet=False)
manager = _make_manager([pub, priv])
allowed, blocked = manager.filter_server_ids_by_ip_with_info(
["pub", "priv"], client_ip=None
)
assert allowed == ["pub"]
assert blocked == 1
@patch("litellm.public_mcp_servers", [])
@patch("litellm.proxy.proxy_server.general_settings", {})

View file

@ -1365,6 +1365,67 @@ class TestTemporaryMCPSessionEndpoints:
assert exc_info.value.status_code == 404
@pytest.mark.asyncio
async def test_get_cached_temporary_mcp_server_id_lookup_applies_ip_gate(self):
"""
/server/oauth/{server_id}/{authorize,token,register} resolves the
server via _get_cached_temporary_mcp_server_or_404, which used to do
get_mcp_server_by_id(server_id) OR get_mcp_server_by_name(...). The
id-lookup branch did not apply IP gating, letting an external caller
hit an internal-only server by UUID. The fix gates the id-lookup
result before falling back to name lookup.
"""
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
_InternalRequest,
)
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
_get_cached_temporary_mcp_server_or_404,
)
internal_only_server = generate_mock_mcp_server_config_record(
server_id="internal-only", name="Internal Only"
)
internal_only_server.available_on_public_internet = False
admin_auth = generate_mock_user_api_key_auth(
user_role=LitellmUserRoles.PROXY_ADMIN,
)
mock_manager = MagicMock()
mock_manager.get_mcp_server_by_id.return_value = internal_only_server
mock_manager.get_mcp_server_by_name.return_value = None
# External IP: gate denies, both id and name lookups should miss → 404.
def _gate(server, client_ip):
if isinstance(client_ip, _InternalRequest):
return True
return False # external IP, server is internal-only
mock_manager._is_server_accessible_from_ip.side_effect = _gate
external_request = _make_mock_request(ip="8.8.8.8")
with (
patch(
"litellm.proxy.management_endpoints.mcp_management_endpoints.get_cached_temporary_mcp_server",
return_value=None,
),
patch(
"litellm.proxy.management_endpoints.mcp_management_endpoints.global_mcp_server_manager",
mock_manager,
),
):
with pytest.raises(HTTPException) as exc_info:
await _get_cached_temporary_mcp_server_or_404(
"internal-only", admin_auth, request=external_request
)
assert (
exc_info.value.status_code == 404
), "External caller must NOT be able to reach internal-only server by UUID"
# Gate must have been consulted on the id-lookup result.
mock_manager._is_server_accessible_from_ip.assert_any_call(
internal_only_server, "8.8.8.8"
)
@pytest.mark.asyncio
async def test_get_cached_temporary_mcp_server_non_admin_denied(self):
"""Non-admin without access to the server gets 403, not the server."""