scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-22 00:31:44 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 51

fix(mcp): keep public servers reachable when client IP is unknown

Browse source 
The fail-closed gate previously denied access for any server when
client_ip was None — including legitimately-public servers. A real
HTTP request whose IP couldn't be extracted (ASGI middleware nulling
request.client, broken X-Forwarded-* parsing, etc.) would 404 against
a public OAuth endpoint with no actionable diagnostic for operators.

Reorder _is_server_accessible_from_ip to check the public-server
short-circuits (available_on_public_internet and
litellm.public_mcp_servers) before failing closed on None. Non-public
servers still fail closed when client_ip is None; the
mcp_allow_unknown_client_ip opt-out still works as a wider escape
hatch for operators who can't fix IP extraction.

Also remove the None short-circuits in filter_server_ids_by_ip_with_info
and get_filtered_registry so they delegate to the per-server gate,
which now handles None correctly.

Update test_gate_contract to expect public+None → True and add
test_public_server_reachable_when_ip_unknown and
test_no_ip_lets_public_through_blocks_private for the new semantics.
This commit is contained in:
user 2026-05-11 04:19:49 +00:00
parent 0ef0b709fa
commit 1037f471c0
No known key found for this signature in database
3 changed files with 67 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

48
litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
View file

@ -1107,10 +1107,9 @@ class MCPServerManager:
client_ip = self._resolve_unknown_client_ip(client_ip)
if client_ip is INTERNAL_REQUEST:
return server_ids, 0
if client_ip is None:
# Fail closed: don't expose internal-only servers when the IP
# couldn't be attributed.
return [], len(server_ids)
# 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:
@ -3119,29 +3118,42 @@ class MCPServerManager:
- ``INTERNAL_REQUEST`` bypasses IP filtering (internal callers, admin
debug, registry maintenance).
- ``None`` fails closed: external request handlers must extract a real
IP via ``IPAddressUtils.get_mcp_client_ip(request)`` and reject the
request when extraction fails. 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
- 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``.
- If the server has ``available_on_public_internet=True``, it's
always accessible.
- Otherwise, only internal/private IPs can access it.
- For non-public servers with a real IP, only internal/private IPs
can access it.
"""
client_ip = self._resolve_unknown_client_ip(client_ip)
if client_ip is INTERNAL_REQUEST:
return True
if client_ip is None:
return False
# 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
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.
@ -3301,8 +3313,8 @@ class MCPServerManager:
client_ip = self._resolve_unknown_client_ip(client_ip)
if client_ip is INTERNAL_REQUEST:
return registry
if client_ip is None:
return {}
# 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()

10
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py
View file

@ -3346,10 +3346,14 @@ class TestIPGatingFailClosed:
@pytest.mark.parametrize(
"server_kind,client_ip_kind,expected",
[
# None fails closed regardless of server visibility — missing IP
# signals an external request that couldn't be attributed.
# 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),
("public", "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),

30
tests/test_litellm/proxy/auth/test_mcp_ip_filtering.py
View file

@ -114,6 +114,21 @@ class TestMCPServerIPFiltering:
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:
"""Tests that filter_server_ids_by_ip_with_info returns accurate block counts."""
@ -166,6 +181,21 @@ class TestFilterServerIdsByIpWithInfo:
)
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", {})
def test_all_private_external_ip_reports_all_blocked(self):