mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-23 00:41:40 +00:00
fix(mcp): remove auto-promote of unknown client IP to internal sentinel
Previously _get_allowed_mcp_servers fell back to INTERNAL_REQUEST when both the explicit client_ip and the auth-context IP were missing, defeating the new fail-closed gate at the wrapper layer. An external request with no attributable client IP could reach internal-only MCP servers via tool/list/resource calls. Drop the auto-promote: pass None through, which now fails closed in filter_server_ids_by_ip_with_info. Internal callers must opt in to INTERNAL_REQUEST explicitly. Update test_mcp_routing_with_conflicting_alias_and_group_name to mock _get_client_ip_from_context with a real internal IP — the unit test has no HTTP request context so it would otherwise be filtered out by the fail-closed gate. Update test_mcp_ip_filtering tests to assert the new fail-closed behavior on missing IP and the INTERNAL_REQUEST escape hatch.
This commit is contained in:
parent
c99ac0124c
commit
bf935a556c
3 changed files with 43 additions and 31 deletions
|
|
@ -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,37 +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 neither is available the call is treated as internal (admin
|
||||
# debug, registry maintenance, background work) and uses
|
||||
# INTERNAL_REQUEST to bypass IP gating explicitly. The wrapper
|
||||
# otherwise fails closed on None.
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
INTERNAL_REQUEST,
|
||||
)
|
||||
|
||||
gate_arg: Union[str, "_InternalRequest", None] = client_ip
|
||||
if gate_arg is None:
|
||||
gate_arg = _get_client_ip_from_context()
|
||||
if gate_arg 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."
|
||||
)
|
||||
gate_arg = INTERNAL_REQUEST
|
||||
if client_ip is None:
|
||||
client_ip = _get_client_ip_from_context()
|
||||
|
||||
allowed_mcp_server_ids = (
|
||||
await global_mcp_server_manager.get_allowed_mcp_servers(user_api_key_auth)
|
||||
|
|
@ -889,7 +877,7 @@ if MCP_AVAILABLE:
|
|||
allowed_mcp_server_ids,
|
||||
_ip_blocked,
|
||||
) = global_mcp_server_manager.filter_server_ids_by_ip_with_info(
|
||||
allowed_mcp_server_ids, gate_arg
|
||||
allowed_mcp_server_ids, client_ip
|
||||
)
|
||||
verbose_logger.debug(
|
||||
"MCP IP filter: client_ip=%s, allowed_server_ids=%s",
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -85,12 +85,20 @@ 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"]
|
||||
|
||||
|
||||
class TestFilterServerIdsByIpWithInfo:
|
||||
|
|
@ -124,15 +132,25 @@ 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", {})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue