Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -756,14 +756,30 @@ def filter_server_ids_by_ip(

Returns server_ids unchanged when client_ip is None (no filtering).
"""
filtered, _ = self.filter_server_ids_by_ip_with_info(server_ids, client_ip)
return filtered

def filter_server_ids_by_ip_with_info(
self, server_ids: List[str], client_ip: Optional[str]
) -> 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.
"""
if client_ip is None:
return server_ids
return [
sid
for sid in server_ids
if (s := self.get_mcp_server_by_id(sid)) is not None
and self._is_server_accessible_from_ip(s, client_ip)
]
return server_ids, 0
allowed = []
blocked = 0
for sid in server_ids:
s = self.get_mcp_server_by_id(sid)
if s is not None and self._is_server_accessible_from_ip(s, client_ip):
allowed.append(sid)
elif s is not None:
blocked += 1
return allowed, blocked

async def get_tools_for_server(self, server_id: str) -> List[MCPTool]:
"""
Expand Down
41 changes: 38 additions & 3 deletions litellm/proxy/_experimental/mcp_server/rest_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
)
from litellm.proxy._experimental.mcp_server.utils import merge_mcp_headers
from litellm.proxy._types import UserAPIKeyAuth
from litellm.proxy.common_utils.http_parsing_utils import _safe_get_request_headers
from litellm.proxy.auth.ip_address_utils import IPAddressUtils
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
from litellm.proxy.common_utils.http_parsing_utils import _safe_get_request_headers
from litellm.types.mcp import MCPAuth
from litellm.types.utils import CallTypes

Expand Down Expand Up @@ -283,8 +283,10 @@ async def list_tool_rest_api(
)
allowed_server_ids_set.update(servers)

allowed_server_ids = global_mcp_server_manager.filter_server_ids_by_ip(
list(allowed_server_ids_set), _rest_client_ip
allowed_server_ids, _ip_blocked_count = (
global_mcp_server_manager.filter_server_ids_by_ip_with_info(
list(allowed_server_ids_set), _rest_client_ip
)
)

list_tools_result = []
Expand All @@ -293,6 +295,26 @@ async def list_tool_rest_api(
# If server_id is specified, only query that specific server
if server_id:
if server_id not in allowed_server_ids:
_server = global_mcp_server_manager.get_mcp_server_by_id(server_id)
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
)
):
raise HTTPException(
status_code=403,
detail={
"error": "ip_filtering",
"message": (
f"MCP server '{server_id}' is not accessible from your IP address "
f"({_rest_client_ip}). This server is restricted to internal "
"networks only. To make it externally accessible, set "
"'available_on_public_internet: true' in the server configuration."
),
},
Comment on lines +306 to +316

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error message exposes internal server configuration details

The error detail includes the client IP address, states that the server is "restricted to internal networks only", and tells the caller how to change the server configuration (available_on_public_internet: true). This is appropriate for operator-facing logs but may reveal more than desired to external API consumers, especially the configuration key name. A similar concern applies to the all-servers case on line 361.

Consider returning a more generic message to the API consumer (e.g., "Access denied for this server") and keeping the configuration hints in server-side logs only.

)
Comment on lines +298 to +317

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant server lookup and private method access

This block re-fetches the server via get_mcp_server_by_id(server_id) and then calls the private method _is_server_accessible_from_ip directly from outside the MCPServerManager class. Both operations were already performed inside filter_server_ids_by_ip_with_info a few lines above.

Consider instead checking whether the server_id was in the original allowed_server_ids_set (before IP filtering) to determine whether the denial is due to IP filtering vs. key-based access. This avoids the redundant lookup and calling a private method from outside its class.

raise HTTPException(
status_code=403,
detail={
Expand Down Expand Up @@ -330,6 +352,19 @@ async def list_tool_rest_api(
}
else:
if not allowed_server_ids:
if _ip_blocked_count > 0:
raise HTTPException(
status_code=403,
detail={
"error": "ip_filtering",
"message": (
f"No MCP tools are available for your IP address ({_rest_client_ip}). "
f"{_ip_blocked_count} server(s) are restricted to internal networks only. "
"To make servers externally accessible, set "
"'available_on_public_internet: true' in the server configuration."
),
},
)
raise HTTPException(
status_code=403,
detail={
Expand Down
14 changes: 12 additions & 2 deletions litellm/proxy/_experimental/mcp_server/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -771,15 +771,25 @@
user_api_key_auth
)
)
allowed_mcp_server_ids = (
global_mcp_server_manager.filter_server_ids_by_ip(
allowed_mcp_server_ids, _ip_blocked = (
global_mcp_server_manager.filter_server_ids_by_ip_with_info(
allowed_mcp_server_ids, client_ip
)
)
verbose_logger.debug(
"MCP IP filter: client_ip=%s, allowed_server_ids=%s",
client_ip, 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,

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expression logs
sensitive data (secret)
as clear text.
This expression logs
sensitive data (password)
as clear text.
This expres

Copilot Autofix

AI 8 months ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

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(
Expand Down
55 changes: 55 additions & 0 deletions tests/test_litellm/proxy/auth/test_mcp_ip_filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,58 @@ def test_no_ip_means_no_filtering(self):

result = manager.filter_server_ids_by_ip(["priv"], client_ip=None)
assert result == ["priv"]


class TestFilterServerIdsByIpWithInfo:
"""Tests that filter_server_ids_by_ip_with_info returns accurate block counts."""

@patch("litellm.public_mcp_servers", [])
@patch("litellm.proxy.proxy_server.general_settings", {})
def test_external_ip_reports_blocked_count(self):
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="8.8.8.8"
)
assert allowed == ["pub"]
assert blocked == 1

@patch("litellm.public_mcp_servers", [])
@patch("litellm.proxy.proxy_server.general_settings", {})
def test_internal_ip_reports_zero_blocked(self):
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="192.168.1.1"
)
assert allowed == ["pub", "priv"]
assert blocked == 0

@patch("litellm.public_mcp_servers", [])
@patch("litellm.proxy.proxy_server.general_settings", {})
def test_no_ip_returns_all_with_zero_blocked(self):
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

@patch("litellm.public_mcp_servers", [])
@patch("litellm.proxy.proxy_server.general_settings", {})
def test_all_private_external_ip_reports_all_blocked(self):
priv1 = _make_server("priv1", available_on_public_internet=False)
priv2 = _make_server("priv2", available_on_public_internet=False)
manager = _make_manager([priv1, priv2])

allowed, blocked = manager.filter_server_ids_by_ip_with_info(
["priv1", "priv2"], client_ip="1.2.3.4"
)
assert allowed == []
assert blocked == 2
Loading