From 47be6c8aeb578c71c13f56eea8d1db6a5b81ba3f Mon Sep 17 00:00:00 2001 From: yassin Date: Thu, 17 Sep 2026 18:34:36 +0000 Subject: [PATCH 01/19] feat(mcp): allowlist client applications for MCP gateway access Adds the mcp_allowed_clients general setting, enforced against the clientInfo.name each MCP client sends in its initialize request. A client not on the list, or one that does not identify itself, is rejected with 403 before any stateful session is created. The setting is configurable from config.yaml and from the Admin UI MCP network settings page Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../mcp_server/client_allowlist.py | 67 +++++ .../proxy/_experimental/mcp_server/server.py | 66 ++++- litellm/proxy/_types.py | 4 + litellm/proxy/proxy_server.py | 4 + .../mcp_server/test_client_allowlist.py | 105 ++++++++ .../mcp_server/test_mcp_server.py | 244 ++++++++++++++++++ tests/test_litellm/proxy/test_proxy_server.py | 165 +++++++++--- .../_components/MCPNetworkSettings.test.tsx | 66 ++++- .../_components/MCPNetworkSettings.tsx | 77 +++++- 9 files changed, 745 insertions(+), 53 deletions(-) create mode 100644 litellm/proxy/_experimental/mcp_server/client_allowlist.py create mode 100644 tests/test_litellm/proxy/_experimental/mcp_server/test_client_allowlist.py diff --git a/litellm/proxy/_experimental/mcp_server/client_allowlist.py b/litellm/proxy/_experimental/mcp_server/client_allowlist.py new file mode 100644 index 00000000000..57343c39565 --- /dev/null +++ b/litellm/proxy/_experimental/mcp_server/client_allowlist.py @@ -0,0 +1,67 @@ +""" +Gateway-level allowlist of MCP client applications, matched against the +``clientInfo.name`` a client sends in its JSON-RPC ``initialize`` request. The +name is client-supplied, so this is a policy control and not a security boundary. +""" + +import json +from dataclasses import dataclass +from typing import Final + +from pydantic import TypeAdapter, ValidationError + +from litellm._logging import verbose_logger + +MCP_ALLOWED_CLIENTS_SETTING: Final = "mcp_allowed_clients" + +_ALLOWED_CLIENTS_ADAPTER: Final = TypeAdapter(list[str]) + + +@dataclass(frozen=True, slots=True) +class MCPClientRejection: + client_name: str | None + + @property + def details(self) -> str: + if self.client_name is None: + return ( + "MCP initialize request did not identify the client application (clientInfo.name). " + f"This gateway only admits clients listed in {MCP_ALLOWED_CLIENTS_SETTING}." + ) + return f"MCP client '{self.client_name}' is not listed in this gateway's {MCP_ALLOWED_CLIENTS_SETTING}." + + +def parse_allowed_mcp_clients(raw_setting: object) -> frozenset[str] | None: + """None when the setting is absent (not enforced). A malformed setting admits nobody.""" + if raw_setting is None: + return None + try: + return frozenset(_ALLOWED_CLIENTS_ADAPTER.validate_python(raw_setting)) + except ValidationError: + verbose_logger.warning( + "%s is not a list of client names (%r); rejecting every MCP client until it is fixed", + MCP_ALLOWED_CLIENTS_SETTING, + raw_setting, + ) + return frozenset() + + +def extract_mcp_client_name(body: bytes) -> str | None: + try: + data: Final = json.loads(body) + except (json.JSONDecodeError, UnicodeDecodeError): + return None + params: Final = data.get("params") if isinstance(data, dict) else None + client_info: Final = params.get("clientInfo") if isinstance(params, dict) else None + name: Final = client_info.get("name") if isinstance(client_info, dict) else None + return name if isinstance(name, str) and name else None + + +def check_mcp_client_allowed(body: bytes, allowed_clients: frozenset[str] | None) -> MCPClientRejection | None: + """None when the initialize is admitted, otherwise the rejection to send back as a 403.""" + if allowed_clients is None: + return None + client_name: Final = extract_mcp_client_name(body) + if client_name is not None and client_name in allowed_clients: + return None + return MCPClientRejection(client_name=client_name) diff --git a/litellm/proxy/_experimental/mcp_server/server.py b/litellm/proxy/_experimental/mcp_server/server.py index 7feb1fd468d..cca867d4d2a 100644 --- a/litellm/proxy/_experimental/mcp_server/server.py +++ b/litellm/proxy/_experimental/mcp_server/server.py @@ -463,6 +463,11 @@ if MCP_AVAILABLE: from litellm.proxy._experimental.mcp_server.auth.litellm_auth_handler import ( MCPAuthenticatedUser, ) + from litellm.proxy._experimental.mcp_server.client_allowlist import ( + MCP_ALLOWED_CLIENTS_SETTING, + check_mcp_client_allowed, + parse_allowed_mcp_clients, + ) from litellm.proxy._experimental.mcp_server.faults.list_outcomes import ( SERVER_OUTCOMES_META_KEY, AggregateToolListing, @@ -3810,6 +3815,43 @@ if MCP_AVAILABLE: except (json.JSONDecodeError, TypeError): return False + def _load_allowed_mcp_clients() -> frozenset[str] | None: + from litellm.proxy.proxy_server import general_settings + + return parse_allowed_mcp_clients(general_settings.get(MCP_ALLOWED_CLIENTS_SETTING)) + + async def _reject_initialize_from_disallowed_client( + scope: Scope, + receive: Receive, + send: Send, + body: bytes, + client_ip: str | None, + ) -> bool: + """Send a 403 and return True when the initialize body names a client the gateway does not admit.""" + rejection: Final = check_mcp_client_allowed(body, _load_allowed_mcp_clients()) + if rejection is None: + return False + verbose_logger.warning( + "Rejecting MCP initialize from client %r (ip=%s): not listed in %s", + rejection.client_name, + client_ip, + MCP_ALLOWED_CLIENTS_SETTING, + ) + forbidden: Final = JSONResponse( + status_code=403, + content={"error": "Forbidden", "details": rejection.details}, + ) + await forbidden(scope, receive, send) + return True + + def _replay_consumed_messages(consumed_messages: list[Message], receive: Receive) -> Receive: + async def wrapped_receive() -> Message: + if consumed_messages: + return consumed_messages.pop(0) + return await receive() + + return wrapped_receive + async def _read_request_body_for_routing( receive: Receive, ) -> tuple[list[Message], bytes]: @@ -4510,6 +4552,10 @@ if MCP_AVAILABLE: if scope.get("method") == "POST": consumed_messages, body = await _read_request_body_for_routing(receive) is_initialize = _is_initialize_request(body) + if is_initialize and await _reject_initialize_from_disallowed_client( + scope, receive, send, body, _client_ip + ): + return use_stateful: Final = bool(session_id or is_initialize) target_manager: Final = session_manager_stateful if use_stateful else session_manager_stateless @@ -4540,15 +4586,8 @@ if MCP_AVAILABLE: return # Replay body messages if we consumed them for peeking - original_receive: Final = receive if consumed_messages: - - async def wrapped_receive(): - if consumed_messages: - return consumed_messages.pop(0) - return await original_receive() - - receive = wrapped_receive + receive = _replay_consumed_messages(consumed_messages, receive) # Serialize requests on the same stateful session so concurrent # callers don't clobber each other's auth context mid-flight. @@ -4785,6 +4824,15 @@ if MCP_AVAILABLE: await initialize_session_managers() await asyncio.sleep(0.1) + sse_consumed_messages, sse_body = ( + await _read_request_body_for_routing(receive) if scope.get("method") == "POST" else ([], b"") + ) + if _is_initialize_request(sse_body) and await _reject_initialize_from_disallowed_client( + scope, receive, send, sse_body, _sse_client_ip + ): + return + sse_receive: Final = _replay_consumed_messages(sse_consumed_messages, receive) + async with _gateway_initialize_instructions_request_scope( user_api_key_auth, mcp_servers, @@ -4792,7 +4840,7 @@ if MCP_AVAILABLE: scoped_server_endpoint=scoped_server_endpoint, is_initialize=scope.get("method") == "GET", ): - await sse_session_manager.handle_request(scope, receive, send) + await sse_session_manager.handle_request(scope, sse_receive, send) except MCPUpstreamAuthError as e: # Upstream delegated auth returned 401; surface it to the client so # standards-compliant MCP clients trigger the upstream OAuth flow. diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 680c63393e8..bbebb99055f 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -2853,6 +2853,10 @@ class ConfigGeneralSettings(LiteLLMPydanticObjectBase): None, description="Custom CIDR ranges that define internal/private networks for MCP access control. When set, only these ranges are treated as internal. Defaults to RFC 1918 private ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8).", ) + mcp_allowed_clients: list[str] | None = Field( + None, + description="MCP client applications admitted by the gateway, matched exactly against the clientInfo.name the client sends in its initialize request (for example 'claude-code'). When set, an initialize from any other client, or one that does not identify itself, is rejected with 403. Unset means every client is admitted. The name is client-supplied, so this is a policy control rather than a security boundary.", + ) mcp_trusted_proxy_ranges: list[str] | None = Field( None, description="CIDR ranges of trusted reverse proxies. When set, X-Forwarded-For and X-Forwarded-* origin headers are only trusted from these IPs.", diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index d7d8413d2ce..aa98491cf3b 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -7179,6 +7179,9 @@ class ProxyConfig: "enable_openai_websocket_passthrough" ) + if "mcp_allowed_clients" not in self._yaml_general_settings_keys: + general_settings["mcp_allowed_clients"] = _general_settings.get("mcp_allowed_clients") + if "user_api_key_cache_max_size" not in self._yaml_general_settings_keys: db_cache_max_size: Final = _general_settings.get("user_api_key_cache_max_size") try: @@ -17137,6 +17140,7 @@ _GENERAL_SETTINGS_CONFIG_LIST_FIELD_TYPES: Final[Mapping[str, str]] = MappingPro "maximum_spend_logs_cleanup_run_budget": "String", "maximum_spend_logs_cleanup_batch_timeout": "String", "mcp_internal_ip_ranges": "List", + "mcp_allowed_clients": "List", "mcp_trusted_proxy_ranges": "List", "mcp_xff_num_trusted_hops": "Integer", "always_include_stream_usage": "Boolean", diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_client_allowlist.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_client_allowlist.py new file mode 100644 index 00000000000..fce8a0a6c3b --- /dev/null +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_client_allowlist.py @@ -0,0 +1,105 @@ +import json +from typing import Final + +import pytest + +from litellm.proxy._experimental.mcp_server.client_allowlist import ( + MCP_ALLOWED_CLIENTS_SETTING, + MCPClientRejection, + check_mcp_client_allowed, + extract_mcp_client_name, + parse_allowed_mcp_clients, +) + + +def _initialize_body(client_info: object) -> bytes: + return json.dumps( + { + "jsonrpc": "2.0", + "id": 0, + "method": "initialize", + "params": {"protocolVersion": "2025-06-18", "capabilities": {}, "clientInfo": client_info}, + } + ).encode() + + +CLAUDE_CODE: Final = _initialize_body({"name": "claude-code", "version": "2.1.274"}) +ANTIGRAVITY: Final = _initialize_body({"name": "antigravity-cli", "version": "1.0.0"}) + + +@pytest.mark.parametrize( + ("raw_setting", "expected"), + ( + (None, None), + ([], frozenset()), + (["antigravity-cli"], frozenset({"antigravity-cli"})), + (["antigravity-cli", "codex-mcp-client"], frozenset({"antigravity-cli", "codex-mcp-client"})), + ("antigravity-cli", frozenset()), + ([1, "antigravity-cli"], frozenset()), + ({"name": "antigravity-cli"}, frozenset()), + ), +) +def test_parse_allowed_mcp_clients(raw_setting: object, expected: frozenset[str] | None) -> None: + assert parse_allowed_mcp_clients(raw_setting) == expected + + +@pytest.mark.parametrize( + ("body", "expected"), + ( + (CLAUDE_CODE, "claude-code"), + (_initialize_body({"name": "", "version": "1"}), None), + (_initialize_body({"version": "1"}), None), + (_initialize_body({"name": 7}), None), + (_initialize_body("claude-code"), None), + (b'{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}', None), + (b'{"jsonrpc":"2.0","id":1,"method":"initialize","params":[]}', None), + (b'["not", "an", "object"]', None), + (b'{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"clientInfo":{"name":"clau', None), + (b"\xff\xfe", None), + (b"", None), + ), +) +def test_extract_mcp_client_name(body: bytes, expected: str | None) -> None: + assert extract_mcp_client_name(body) == expected + + +def test_unconfigured_allowlist_admits_every_client_including_unidentified_ones() -> None: + assert check_mcp_client_allowed(CLAUDE_CODE, None) is None + assert check_mcp_client_allowed(b'{"method":"initialize","params":{}}', None) is None + assert check_mcp_client_allowed(b"garbage", None) is None + + +def test_listed_client_is_admitted_and_unlisted_client_is_rejected_by_name() -> None: + allowed: Final = frozenset({"antigravity-cli"}) + assert check_mcp_client_allowed(ANTIGRAVITY, allowed) is None + assert check_mcp_client_allowed(CLAUDE_CODE, allowed) == MCPClientRejection(client_name="claude-code") + + +def test_matching_is_exact_not_prefix_or_case_insensitive() -> None: + allowed: Final = frozenset({"claude-code"}) + assert check_mcp_client_allowed(_initialize_body({"name": "Claude-Code"}), allowed) is not None + assert check_mcp_client_allowed(_initialize_body({"name": "claude-code-sdk"}), allowed) is not None + assert check_mcp_client_allowed(_initialize_body({"name": " claude-code"}), allowed) is not None + + +def test_empty_allowlist_rejects_every_client() -> None: + assert check_mcp_client_allowed(ANTIGRAVITY, frozenset()) == MCPClientRejection(client_name="antigravity-cli") + assert check_mcp_client_allowed(CLAUDE_CODE, frozenset()) == MCPClientRejection(client_name="claude-code") + + +def test_missing_or_malformed_client_metadata_is_rejected_when_allowlist_is_set() -> None: + allowed: Final = frozenset({"antigravity-cli"}) + assert check_mcp_client_allowed(_initialize_body({"version": "1"}), allowed) == MCPClientRejection(None) + assert check_mcp_client_allowed(b'{"method":"initialize","params":{}}', allowed) == MCPClientRejection(None) + assert check_mcp_client_allowed(b"{not json", allowed) == MCPClientRejection(None) + + +def test_rejection_details_name_the_setting_and_the_offending_client() -> None: + named: Final = MCPClientRejection(client_name="claude-code").details + assert "claude-code" in named + assert MCP_ALLOWED_CLIENTS_SETTING in named + + anonymous: Final = MCPClientRejection(client_name=None).details + assert "clientInfo.name" in anonymous + assert MCP_ALLOWED_CLIENTS_SETTING in anonymous + assert "None" not in anonymous diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py index f5e4a420496..8550226e19d 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py @@ -1,4 +1,5 @@ import asyncio +import contextlib import contextvars import os from datetime import datetime, timedelta @@ -2035,6 +2036,249 @@ async def test_mcp_routing_initialize_to_stateful_no_session_to_stateless( assert not any(name.startswith(b"x-mcp-debug") for name in headers) +_CLAUDE_CODE_INITIALIZE: Final = ( + b'{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"protocolVersion":"2025-06-18",' + b'"capabilities":{},"clientInfo":{"name":"claude-code","version":"2.1.274"}}}' +) +_ANTIGRAVITY_INITIALIZE: Final = ( + b'{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"protocolVersion":"2025-06-18",' + b'"capabilities":{},"clientInfo":{"name":"antigravity-cli","version":"1.0.0"}}}' +) +_ANONYMOUS_INITIALIZE: Final = b'{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"capabilities":{}}}' +_TOOLS_LIST: Final = b'{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}' + + +async def _drain_body(receive) -> bytes: + chunks: list[bytes] = [] + while True: + message = await receive() + chunks.append(message.get("body", b"")) + if not message.get("more_body", False): + return b"".join(chunks) + + +def _forbidden_client_response(send: AsyncMock) -> tuple[int, dict[str, str]]: + import json as _json + + start: Final = send.call_args_list[0].args[0] + body: Final = b"".join(call.args[0].get("body", b"") for call in send.call_args_list[1:]) + return start["status"], _json.loads(body) + + +@contextlib.contextmanager +def _client_allowlist_patches(allowed_clients: object): + settings: Final = {} if allowed_clients is None else {"mcp_allowed_clients": allowed_clients} + with ( + patch( + "litellm.proxy._experimental.mcp_server.server.extract_mcp_auth_context", + new_callable=AsyncMock, + return_value=(UserAPIKeyAuth(user_id="allowlist-user"), None, None, None, None, {}), + ), + patch("litellm.proxy._experimental.mcp_server.server._SESSION_MANAGERS_INITIALIZED", True), + patch("litellm.proxy.proxy_server.general_settings", settings), + ): + yield + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("request_body", "expected_details"), + ( + ( + _CLAUDE_CODE_INITIALIZE, + "MCP client 'claude-code' is not listed in this gateway's mcp_allowed_clients.", + ), + ( + _ANONYMOUS_INITIALIZE, + "MCP initialize request did not identify the client application (clientInfo.name). " + "This gateway only admits clients listed in mcp_allowed_clients.", + ), + ), +) +async def test_streamable_http_rejects_initialize_from_unlisted_client_before_session_creation( + request_body: bytes, expected_details: str +) -> None: + from starlette.types import Scope + + from litellm.proxy._experimental.mcp_server import server as mcp_module + + scope: Final[Scope] = {"type": "http", "method": "POST", "path": "/mcp", "headers": []} + receive: Final = AsyncMock(return_value={"type": "http.request", "body": request_body, "more_body": False}) + send: Final = AsyncMock() + stateful_handle: Final = AsyncMock() + stateless_handle: Final = AsyncMock() + session_cap: Final = AsyncMock(return_value=True) + + with ( + _client_allowlist_patches(["antigravity-cli"]), + patch( + "litellm.proxy._experimental.mcp_server.server.session_manager_stateful", + SimpleNamespace(handle_request=stateful_handle), + ), + patch( + "litellm.proxy._experimental.mcp_server.server.session_manager_stateless", + SimpleNamespace(handle_request=stateless_handle), + ), + patch("litellm.proxy._experimental.mcp_server.server._enforce_stateful_session_cap_for_owner", session_cap), + ): + await mcp_module.handle_streamable_http_mcp(scope, receive, send) + + assert _forbidden_client_response(send) == (403, {"error": "Forbidden", "details": expected_details}) + stateful_handle.assert_not_awaited() + stateless_handle.assert_not_awaited() + session_cap.assert_not_awaited() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("allowed_clients", "request_body"), + ( + (["antigravity-cli"], _ANTIGRAVITY_INITIALIZE), + (["claude-code", "antigravity-cli"], _CLAUDE_CODE_INITIALIZE), + (None, _CLAUDE_CODE_INITIALIZE), + (None, _ANONYMOUS_INITIALIZE), + ), +) +async def test_streamable_http_admits_listed_or_unrestricted_initialize_and_replays_body( + allowed_clients: list[str] | None, request_body: bytes +) -> None: + from starlette.types import Receive, Scope, Send + + from litellm.proxy._experimental.mcp_server import server as mcp_module + + scope: Final[Scope] = {"type": "http", "method": "POST", "path": "/mcp", "headers": []} + receive: Final = AsyncMock( + side_effect=[ + {"type": "http.request", "body": request_body[:20], "more_body": True}, + {"type": "http.request", "body": request_body[20:], "more_body": False}, + ] + ) + send: Final = AsyncMock() + downstream_bodies: Final[list[bytes]] = [] + + async def handle_request(_: Scope, downstream_receive: Receive, __: Send) -> None: + downstream_bodies.append(await _drain_body(downstream_receive)) + + stateful_handle: Final = AsyncMock(side_effect=handle_request) + stateless_handle: Final = AsyncMock() + + with ( + _client_allowlist_patches(allowed_clients), + patch( + "litellm.proxy._experimental.mcp_server.server.session_manager_stateful", + SimpleNamespace(handle_request=stateful_handle), + ), + patch( + "litellm.proxy._experimental.mcp_server.server.session_manager_stateless", + SimpleNamespace(handle_request=stateless_handle), + ), + ): + await mcp_module.handle_streamable_http_mcp(scope, receive, send) + + assert downstream_bodies == [request_body] + stateless_handle.assert_not_awaited() + send.assert_not_awaited() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("allowed_clients", ([], "claude-code", [{"name": "claude-code"}])) +async def test_streamable_http_empty_or_malformed_allowlist_admits_nobody(allowed_clients: object) -> None: + from starlette.types import Scope + + from litellm.proxy._experimental.mcp_server import server as mcp_module + + scope: Final[Scope] = {"type": "http", "method": "POST", "path": "/mcp", "headers": []} + receive: Final = AsyncMock( + return_value={"type": "http.request", "body": _CLAUDE_CODE_INITIALIZE, "more_body": False} + ) + send: Final = AsyncMock() + stateful_handle: Final = AsyncMock() + + with ( + _client_allowlist_patches(allowed_clients), + patch( + "litellm.proxy._experimental.mcp_server.server.session_manager_stateful", + SimpleNamespace(handle_request=stateful_handle), + ), + ): + await mcp_module.handle_streamable_http_mcp(scope, receive, send) + + status, body = _forbidden_client_response(send) + assert status == 403 + assert body["details"] == "MCP client 'claude-code' is not listed in this gateway's mcp_allowed_clients." + stateful_handle.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_streamable_http_allowlist_only_inspects_initialize_requests() -> None: + from starlette.types import Receive, Scope, Send + + from litellm.proxy._experimental.mcp_server import server as mcp_module + + scope: Final[Scope] = {"type": "http", "method": "POST", "path": "/mcp", "headers": []} + receive: Final = AsyncMock(side_effect=[{"type": "http.request", "body": _TOOLS_LIST, "more_body": False}]) + send: Final = AsyncMock() + downstream_bodies: Final[list[bytes]] = [] + + async def handle_request(_: Scope, downstream_receive: Receive, __: Send) -> None: + downstream_bodies.append(await _drain_body(downstream_receive)) + + with ( + _client_allowlist_patches(["antigravity-cli"]), + patch( + "litellm.proxy._experimental.mcp_server.server.session_manager_stateless", + SimpleNamespace(handle_request=AsyncMock(side_effect=handle_request)), + ), + ): + await mcp_module.handle_streamable_http_mcp(scope, receive, send) + + assert downstream_bodies == [_TOOLS_LIST] + send.assert_not_awaited() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("request_body", "admitted"), + ((_ANTIGRAVITY_INITIALIZE, True), (_CLAUDE_CODE_INITIALIZE, False), (_ANONYMOUS_INITIALIZE, False)), +) +async def test_sse_endpoint_applies_the_same_client_allowlist(request_body: bytes, admitted: bool) -> None: + from starlette.types import Receive, Scope, Send + + from litellm.proxy._experimental.mcp_server import server as mcp_module + + scope: Final[Scope] = {"type": "http", "method": "POST", "path": "/mcp/sse", "headers": []} + receive: Final = AsyncMock(side_effect=[{"type": "http.request", "body": request_body, "more_body": False}]) + send: Final = AsyncMock() + downstream_bodies: Final[list[bytes]] = [] + + async def handle_request(_: Scope, downstream_receive: Receive, __: Send) -> None: + downstream_bodies.append(await _drain_body(downstream_receive)) + + with ( + _client_allowlist_patches(["antigravity-cli"]), + patch( + "litellm.proxy._experimental.mcp_server.server._raise_preemptive_401_for_unauthenticated_servers", + new_callable=AsyncMock, + ), + patch( + "litellm.proxy._experimental.mcp_server.server._check_passthrough_upstream_auth", + new_callable=AsyncMock, + ), + patch.object(mcp_module.sse_session_manager, "handle_request", side_effect=handle_request), + ): + await mcp_module.handle_sse_mcp(scope, receive, send) + + if admitted: + assert downstream_bodies == [request_body] + send.assert_not_awaited() + return + assert downstream_bodies == [] + status, body = _forbidden_client_response(send) + assert status == 403 + assert body["error"] == "Forbidden" + assert "mcp_allowed_clients" in body["details"] + + @pytest.mark.asyncio async def test_mcp_routing_chunked_initialize_to_stateful(): """ diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index 41c4956dba6..4a0f543dc38 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -7428,10 +7428,18 @@ async def test_update_general_settings_keeps_yaml_pass_through_endpoints_next_to request.query_params = {} return request - settings: Final = patch("litellm.proxy.proxy_server.general_settings", {"pass_through_endpoints": [yaml_endpoint]}) # test-quality-ok: the method reads this module global; no injection seam - yaml_endpoints: Final = patch("litellm.proxy.proxy_server.config_passthrough_endpoints", [yaml_endpoint]) # test-quality-ok: module global holding the YAML endpoints the fix merges in - initialize: Final = patch("litellm.proxy.proxy_server.initialize_pass_through_endpoints", AsyncMock()) # test-quality-ok: route registration needs the FastAPI app; auth is the observable here - master_key: Final = patch("litellm.proxy.proxy_server.master_key", "sk-master") # test-quality-ok: a set master key is what makes a missing Authorization header a 401 + settings: Final = patch( + "litellm.proxy.proxy_server.general_settings", {"pass_through_endpoints": [yaml_endpoint]} + ) # test-quality-ok: the method reads this module global; no injection seam + yaml_endpoints: Final = patch( + "litellm.proxy.proxy_server.config_passthrough_endpoints", [yaml_endpoint] + ) # test-quality-ok: module global holding the YAML endpoints the fix merges in + initialize: Final = patch( + "litellm.proxy.proxy_server.initialize_pass_through_endpoints", AsyncMock() + ) # test-quality-ok: route registration needs the FastAPI app; auth is the observable here + master_key: Final = patch( + "litellm.proxy.proxy_server.master_key", "sk-master" + ) # test-quality-ok: a set master key is what makes a missing Authorization header a 401 with settings, yaml_endpoints, initialize, master_key: await ProxyConfig()._update_general_settings(db_general_settings={"pass_through_endpoints": [db_endpoint]}) @@ -7479,10 +7487,18 @@ async def test_update_general_settings_db_pass_through_endpoint_overrides_yaml_e request.headers = {} request.query_params = {} - settings: Final = patch("litellm.proxy.proxy_server.general_settings", {"pass_through_endpoints": [yaml_endpoint]}) # test-quality-ok: the method reads this module global; no injection seam - yaml_endpoints: Final = patch("litellm.proxy.proxy_server.config_passthrough_endpoints", [yaml_endpoint]) # test-quality-ok: module global holding the YAML endpoints the fix merges in - initialize: Final = patch("litellm.proxy.proxy_server.initialize_pass_through_endpoints", AsyncMock()) # test-quality-ok: route registration needs the FastAPI app; auth is the observable here - master_key: Final = patch("litellm.proxy.proxy_server.master_key", "sk-master") # test-quality-ok: a set master key is what makes a missing Authorization header a 401 + settings: Final = patch( + "litellm.proxy.proxy_server.general_settings", {"pass_through_endpoints": [yaml_endpoint]} + ) # test-quality-ok: the method reads this module global; no injection seam + yaml_endpoints: Final = patch( + "litellm.proxy.proxy_server.config_passthrough_endpoints", [yaml_endpoint] + ) # test-quality-ok: module global holding the YAML endpoints the fix merges in + initialize: Final = patch( + "litellm.proxy.proxy_server.initialize_pass_through_endpoints", AsyncMock() + ) # test-quality-ok: route registration needs the FastAPI app; auth is the observable here + master_key: Final = patch( + "litellm.proxy.proxy_server.master_key", "sk-master" + ) # test-quality-ok: a set master key is what makes a missing Authorization header a 401 with settings, yaml_endpoints, initialize, master_key: await ProxyConfig()._update_general_settings(db_general_settings={"pass_through_endpoints": [db_endpoint]}) @@ -8597,9 +8613,7 @@ async def test_increment_spend_counters_finalizes_after_unreserved_increments(): async def assert_reservation_not_finalized_yet(**kwargs): assert budget_reservation["finalized"] is False incremented_counters.append(kwargs["counter_key"]) - return ps.PendingSpendIncrement( - counter_key=kwargs["counter_key"], increment=kwargs["increment"] - ) + return ps.PendingSpendIncrement(counter_key=kwargs["counter_key"], increment=kwargs["increment"]) import litellm.proxy.proxy_server as ps @@ -10144,9 +10158,15 @@ async def _lit6973_drive_realtime_session( side_effect=pre_call_error, return_value=({"model": "vertex_ai/gemini-live-2.5-flash"}, logging_obj) ) ws: Final = websocket if websocket is not None else _lit6973_fake_realtime_ws() - can_call = patch.object(ps, "can_key_call_resolved_model", new=AsyncMock(side_effect=model_access_error)) # test-quality-ok: no HTTP boundary; fakes in-process auth to reach the exit under test - pre = patch.object(ps.ProxyBaseLLMRequestProcessing, "common_processing_pre_call_logic", new=pre_call) # test-quality-ok: fakes phase-1 wiring; assertion checks observable reservation state - route = patch.object(ps, "route_request", new=AsyncMock(return_value=fake_llm_call())) # test-quality-ok: fakes the relay whose success/refusal outcome the endpoint reads off the logging object + can_call = patch.object( + ps, "can_key_call_resolved_model", new=AsyncMock(side_effect=model_access_error) + ) # test-quality-ok: no HTTP boundary; fakes in-process auth to reach the exit under test + pre = patch.object( + ps.ProxyBaseLLMRequestProcessing, "common_processing_pre_call_logic", new=pre_call + ) # test-quality-ok: fakes phase-1 wiring; assertion checks observable reservation state + route = patch.object( + ps, "route_request", new=AsyncMock(return_value=fake_llm_call()) + ) # test-quality-ok: fakes the relay whose success/refusal outcome the endpoint reads off the logging object with can_call, pre, route: await ps.realtime_websocket_endpoint( websocket=ws, @@ -10278,13 +10298,9 @@ async def _lit6463_drive_realtime_session_holding_a_max_parallel_slot( from litellm.proxy.utils import InternalUsageCache dual_cache: Final = DualCache() - await dual_cache.async_set_cache( - key=_LIT6463_COUNTER_KEY, value={"slot-1": 1.0, "slot-2": 2.0}, local_only=True - ) + await dual_cache.async_set_cache(key=_LIT6463_COUNTER_KEY, value={"slot-1": 1.0, "slot-2": 2.0}, local_only=True) limiter: Final = _PROXY_MaxParallelRequestsHandler_v3(internal_usage_cache=InternalUsageCache(dual_cache)) - stash: Final = RequestRateLimiterStash( - parallel_slot={"slot_id": "slot-1", "counter_keys": [_LIT6463_COUNTER_KEY]} - ) + stash: Final = RequestRateLimiterStash(parallel_slot={"slot_id": "slot-1", "counter_keys": [_LIT6463_COUNTER_KEY]}) reservation: Final = {"reserved_cost": 0.55, "input_cost": 0.0, "finalized": False, "entries": []} stash_token: Final = _request_stash.set(stash) @@ -10336,9 +10352,7 @@ async def test_successful_realtime_session_leaves_the_max_parallel_slot_for_the_ limiter's integer in-memory fallback, double-decrement the counter so the key admits more sessions than max_parallel_requests allows. With the success stamp present the route leaves the slot and the stash alone.""" - dual_cache, stash = await _lit6463_drive_realtime_session_holding_a_max_parallel_slot( - backend_logged_success=True - ) + dual_cache, stash = await _lit6463_drive_realtime_session_holding_a_max_parallel_slot(backend_logged_success=True) assert await dual_cache.async_get_cache(key=_LIT6463_COUNTER_KEY, local_only=True) == { "slot-1": 1.0, @@ -10384,8 +10398,12 @@ async def test_release_or_invalidate_falls_back_to_invalidating_the_counters(): async def _record(counter_key: str) -> None: invalidated.append(counter_key) - failing_release = patch.object(br, "release_budget_reservation", new=AsyncMock(side_effect=RuntimeError("counter store down"))) # test-quality-ok: forces the failure branch; assertion observes which counter key got invalidated - sink = patch.object(ps, "_invalidate_spend_counter", new=_record) # test-quality-ok: fakes the counter-store sink so the invalidated key is observable + failing_release = patch.object( + br, "release_budget_reservation", new=AsyncMock(side_effect=RuntimeError("counter store down")) + ) # test-quality-ok: forces the failure branch; assertion observes which counter key got invalidated + sink = patch.object( + ps, "_invalidate_spend_counter", new=_record + ) # test-quality-ok: fakes the counter-store sink so the invalidated key is observable with failing_release, sink: await br.release_or_invalidate_budget_reservation(budget_reservation=reservation) @@ -10401,8 +10419,12 @@ async def test_release_or_invalidate_finalizes_even_when_the_invalidate_fallback from litellm.proxy.spend_tracking import budget_reservation as br reservation: Final = {"reserved_cost": 0.55, "input_cost": 0.0, "finalized": False, "entries": []} - failing_release = patch.object(br, "release_budget_reservation", new=AsyncMock(side_effect=RuntimeError("counter store down"))) # test-quality-ok: forces the fallback branch - failing_invalidate = patch.object(br, "invalidate_budget_reservation_counters", new=AsyncMock(side_effect=RuntimeError("still down"))) # test-quality-ok: forces the fallback itself to fail + failing_release = patch.object( + br, "release_budget_reservation", new=AsyncMock(side_effect=RuntimeError("counter store down")) + ) # test-quality-ok: forces the fallback branch + failing_invalidate = patch.object( + br, "invalidate_budget_reservation_counters", new=AsyncMock(side_effect=RuntimeError("still down")) + ) # test-quality-ok: forces the fallback itself to fail with failing_release, failing_invalidate: await br.release_or_invalidate_budget_reservation(budget_reservation=reservation) @@ -12987,9 +13009,15 @@ async def test_moderations_response_carries_litellm_call_id_header(): user_api_key_dict = UserAPIKeyAuth(api_key="sk-test", spend=0.0) with ( - patch.object(proxy_server_module, "add_litellm_data_to_request", new=passthrough_add_litellm_data), # test-quality-ok: the route reads this module global, no injection point - patch.object(proxy_server_module, "route_request", new=AsyncMock(return_value=fake_llm_call())), # test-quality-ok: fakes the provider call so the response headers assembled by the real route are observable - patch.object(proxy_server_module, "proxy_logging_obj") as mock_logging, # test-quality-ok: module global, no injection point + patch.object( + proxy_server_module, "add_litellm_data_to_request", new=passthrough_add_litellm_data + ), # test-quality-ok: the route reads this module global, no injection point + patch.object( + proxy_server_module, "route_request", new=AsyncMock(return_value=fake_llm_call()) + ), # test-quality-ok: fakes the provider call so the response headers assembled by the real route are observable + patch.object( + proxy_server_module, "proxy_logging_obj" + ) as mock_logging, # test-quality-ok: module global, no injection point ): mock_logging.pre_call_hook = AsyncMock(side_effect=lambda user_api_key_dict, data, call_type: data) mock_logging.update_request_status = AsyncMock() @@ -13026,9 +13054,15 @@ async def test_moderations_failure_log_carries_the_callers_litellm_call_id(caplo verbose_proxy_logger.propagate = True try: with ( - patch.object(proxy_server_module, "add_litellm_data_to_request", new=passthrough_add_litellm_data), # test-quality-ok: the route reads this module global, no injection point - patch.object(proxy_server_module, "route_request", new=AsyncMock(side_effect=Exception("bad key"))), # test-quality-ok: fakes the provider failure so the real route's error log is observable - patch.object(proxy_server_module, "proxy_logging_obj", new=fake_logging), # test-quality-ok: module global, no injection point + patch.object( + proxy_server_module, "add_litellm_data_to_request", new=passthrough_add_litellm_data + ), # test-quality-ok: the route reads this module global, no injection point + patch.object( + proxy_server_module, "route_request", new=AsyncMock(side_effect=Exception("bad key")) + ), # test-quality-ok: fakes the provider failure so the real route's error log is observable + patch.object( + proxy_server_module, "proxy_logging_obj", new=fake_logging + ), # test-quality-ok: module global, no injection point caplog.at_level(logging.ERROR, logger="LiteLLM Proxy"), pytest.raises(ProxyException) as raised, ): @@ -13061,7 +13095,9 @@ async def test_moderations_unparseable_body_bills_the_callers_litellm_call_id(): fake_logging.post_call_failure_hook = AsyncMock() with ( - patch.object(proxy_server_module, "proxy_logging_obj", new=fake_logging), # test-quality-ok: module global, no injection point + patch.object( + proxy_server_module, "proxy_logging_obj", new=fake_logging + ), # test-quality-ok: module global, no injection point pytest.raises(ProxyException) as raised, ): await proxy_server_module.moderations( @@ -13089,8 +13125,12 @@ async def test_moderations_already_shaped_failure_answers_with_the_callers_litel fake_logging.post_call_failure_hook = AsyncMock() with ( - patch.object(proxy_server_module, "add_litellm_data_to_request", new=AsyncMock(side_effect=exc)), # test-quality-ok: the route reads this module global, no injection point - patch.object(proxy_server_module, "proxy_logging_obj", new=fake_logging), # test-quality-ok: module global, no injection point + patch.object( + proxy_server_module, "add_litellm_data_to_request", new=AsyncMock(side_effect=exc) + ), # test-quality-ok: the route reads this module global, no injection point + patch.object( + proxy_server_module, "proxy_logging_obj", new=fake_logging + ), # test-quality-ok: module global, no injection point pytest.raises(ProxyException) as raised, ): await proxy_server_module.moderations( @@ -13125,8 +13165,12 @@ async def test_audio_speech_already_shaped_failure_answers_with_the_callers_lite fake_logging.post_call_failure_hook = AsyncMock() with ( - patch.object(proxy_server_module, "add_litellm_data_to_request", new=AsyncMock(side_effect=exc)), # test-quality-ok: the route reads this module global, no injection point - patch.object(proxy_server_module, "proxy_logging_obj", new=fake_logging), # test-quality-ok: module global, no injection point + patch.object( + proxy_server_module, "add_litellm_data_to_request", new=AsyncMock(side_effect=exc) + ), # test-quality-ok: the route reads this module global, no injection point + patch.object( + proxy_server_module, "proxy_logging_obj", new=fake_logging + ), # test-quality-ok: module global, no injection point pytest.raises(type(exc)) as raised, ): await proxy_server_module.audio_speech( @@ -13814,6 +13858,43 @@ async def test_update_general_settings_keeps_yaml_openai_websocket_passthrough() assert ps.general_settings["enable_openai_websocket_passthrough"] is False +@pytest.mark.asyncio +@pytest.mark.parametrize( + "db_general_settings, expected", + [ + ({"mcp_allowed_clients": ["antigravity-cli"]}, ["antigravity-cli"]), + ({"mcp_allowed_clients": []}, []), + ({}, None), + ], +) +async def test_update_general_settings_propagates_mcp_allowed_clients(db_general_settings, expected): + from litellm.proxy.proxy_server import ProxyConfig + + proxy_config = ProxyConfig() + + with patch("litellm.proxy.proxy_server.general_settings", {"mcp_allowed_clients": ["claude-code"]}): + await proxy_config._update_general_settings(db_general_settings=db_general_settings) + + import litellm.proxy.proxy_server as ps + + assert ps.general_settings["mcp_allowed_clients"] == expected + + +@pytest.mark.asyncio +async def test_update_general_settings_keeps_yaml_mcp_allowed_clients(): + from litellm.proxy.proxy_server import ProxyConfig + + proxy_config = ProxyConfig() + proxy_config._yaml_general_settings_keys = {"mcp_allowed_clients"} + + with patch("litellm.proxy.proxy_server.general_settings", {"mcp_allowed_clients": ["claude-code"]}): + await proxy_config._update_general_settings(db_general_settings={"mcp_allowed_clients": ["codex-mcp-client"]}) + + import litellm.proxy.proxy_server as ps + + assert ps.general_settings["mcp_allowed_clients"] == ["claude-code"] + + async def test_token_counter_keeps_the_event_loop_free_during_a_huggingface_count(monkeypatch): from tests.large_text import text from tests.test_litellm.litellm_core_utils.event_loop_lag import ( @@ -13855,14 +13936,18 @@ async def test_token_counter_loads_a_custom_tokenizer_off_the_event_loop(monkeyp { "model_name": "self-hosted", "litellm_params": {"model": "openai/self-hosted-model", "api_base": "http://localhost:8080/v1"}, - "model_info": {"custom_tokenizer": {"identifier": "my-org/tokenizer", "revision": "main", "auth_token": None}}, + "model_info": { + "custom_tokenizer": {"identifier": "my-org/tokenizer", "revision": "main", "auth_token": None} + }, } ] ), ) response, took, lags = await timed_with_loop_lags( - lambda: proxy_server_module.token_counter(TokenCountRequest(model="self-hosted", prompt="count me off the loop")) + lambda: proxy_server_module.token_counter( + TokenCountRequest(model="self-hosted", prompt="count me off the loop") + ) ) assert response.tokenizer_type == "huggingface_tokenizer" diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx index 9526f5de074..b6521acd7e2 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx @@ -93,7 +93,7 @@ describe("MCPNetworkSettings", () => { await waitFor(() => expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges", ["10.0.0.0/8"]), ); - expect(deleteConfigFieldSetting).not.toHaveBeenCalled(); + expect(deleteConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges"); }); it("clears the setting instead of saving an empty list", async () => { @@ -103,4 +103,68 @@ describe("MCPNetworkSettings", () => { await waitFor(() => expect(deleteConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges")); expect(updateConfigFieldSetting).not.toHaveBeenCalled(); }); + + it("renders the stored allowed client names once settings load", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_allowed_clients", field_value: ["antigravity-cli", "codex-mcp-client"] }, + ]); + + renderSettings(); + + expect(await screen.findByText("antigravity-cli")).toBeInTheDocument(); + expect(screen.getByText("codex-mcp-client")).toBeInTheDocument(); + }); + + it("adds typed client names on Enter and saves them under mcp_allowed_clients", async () => { + renderSettings(); + const input = await screen.findByRole("textbox", { name: "Allowed client names" }); + + await userEvent.type(input, "antigravity-cli, codex-mcp-client{Enter}"); + + expect(screen.getByText("antigravity-cli")).toBeInTheDocument(); + expect(screen.getByText("codex-mcp-client")).toBeInTheDocument(); + expect(input).toHaveValue(""); + + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ + "antigravity-cli", + "codex-mcp-client", + ]), + ); + expect(deleteConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_allowed_clients"); + }); + + it("removes a client name and clears the setting when the list becomes empty", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_allowed_clients", field_value: ["claude-code"] }, + ]); + + renderSettings(); + await userEvent.click(await screen.findByRole("button", { name: "Remove claude-code" })); + + expect(screen.queryByText("claude-code")).not.toBeInTheDocument(); + + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => expect(deleteConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients")); + expect(updateConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_allowed_clients", expect.anything()); + }); + + it("keeps the private ranges and the allowed clients as independent settings on save", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_internal_ip_ranges", field_value: ["10.0.0.0/8"] }, + { field_name: "mcp_allowed_clients", field_value: ["antigravity-cli"] }, + ]); + + renderSettings(); + await userEvent.click(await screen.findByRole("button", { name: /Save/ })); + + await waitFor(() => + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", ["antigravity-cli"]), + ); + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges", ["10.0.0.0/8"]); + expect(deleteConfigFieldSetting).not.toHaveBeenCalled(); + }); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx index 8b4d2a58652..18377a4bb82 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx @@ -30,8 +30,10 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) const [loading, setLoading] = useState(true); const [saving, setSaving] = useState(false); const [privateRanges, setPrivateRanges] = useState([]); + const [allowedClients, setAllowedClients] = useState([]); const [currentIp, setCurrentIp] = useState(null); const [rangeDraft, setRangeDraft] = useState(""); + const [clientDraft, setClientDraft] = useState(""); useEffect(() => { loadSettings(); @@ -47,6 +49,9 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) if (field.field_name === "mcp_internal_ip_ranges" && field.field_value) { setPrivateRanges(field.field_value); } + if (field.field_name === "mcp_allowed_clients" && field.field_value) { + setAllowedClients(field.field_value); + } } } catch (error) { console.error("Failed to load MCP network settings:", error); @@ -72,6 +77,11 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) } else { await deleteConfigFieldSetting(accessToken, "mcp_internal_ip_ranges"); } + if (allowedClients.length > 0) { + await updateConfigFieldSetting(accessToken, "mcp_allowed_clients", allowedClients); + } else { + await deleteConfigFieldSetting(accessToken, "mcp_allowed_clients"); + } } catch (error) { console.error("Failed to save MCP network settings:", error); } finally { @@ -86,17 +96,28 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) }; // Commas separate entries, matching the old tokenised input. - const commitDraft = () => { - const added = rangeDraft + const splitDraft = (draft: string, existing: string[]) => + draft .split(",") .map((r) => r.trim()) - .filter((r) => r !== "" && !privateRanges.includes(r)); + .filter((r) => r !== "" && !existing.includes(r)); + + const commitDraft = () => { + const added = splitDraft(rangeDraft, privateRanges); if (added.length > 0) { setPrivateRanges([...privateRanges, ...added]); } setRangeDraft(""); }; + const commitClientDraft = () => { + const added = splitDraft(clientDraft, allowedClients); + if (added.length > 0) { + setAllowedClients([...allowedClients, ...added]); + } + setClientDraft(""); + }; + if (loading) { return (
@@ -178,6 +199,56 @@ const MCPNetworkSettings: React.FC = ({ accessToken })

+
+

Allowed Client Applications

+

+ Only the MCP client applications listed here can connect to the gateway. Names are matched exactly against + the clientInfo.name each client sends in its MCP initialize request (for example claude-code or + codex-mcp-client). Leave empty to allow every client. Clients choose the name they send, so treat this as a + policy control rather than a security boundary. +

+
+ + +
+

Allowed Client Names

+
+ {allowedClients.length > 0 && ( +
+ {allowedClients.map((client) => ( + + {client} + + + ))} +
+ )} + setClientDraft(e.target.value)} + onBlur={commitClientDraft} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === ",") { + e.preventDefault(); + commitClientDraft(); + } + }} + /> +

+ Enter the clientInfo.name values to admit. Any other client, or one that does not identify itself, gets a + 403 on its MCP initialize request. +

+
+
- + + + ))}
)} - setClientDraft(e.target.value)} - onBlur={commitClientDraft} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === ",") { - e.preventDefault(); - commitClientDraft(); - } - }} - /> +

- Enter the exact JWT claim or header values to admit. Every MCP request from any other client, or from one with - no resolvable identity, gets a 403. + The alias is the name shown here and in gateway logs. The value is the exact JWT claim or header value that + identifies the client, such as the OAuth client ID your identity provider issues. Leave the list empty to + allow every client. Every MCP request from an unlisted client, or from one with no resolvable identity, gets a + 403.

diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index 19c0e29f17c..45a11aff0bc 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -26849,9 +26849,9 @@ export interface components { maximum_spend_logs_retention_period?: string | null; /** * Mcp Allowed Clients - * @description MCP client applications admitted by the gateway. When set, every MCP request must carry a client identity that matches one of these values exactly: a JWT caller is identified by the claim named in litellm_jwtauth.mcp_client_id_jwt_field, any other caller by the header named in mcp_client_id_header. A request with no resolvable identity, or an unlisted one, is rejected with 403. Unset means every client is admitted. + * @description MCP client applications admitted by the gateway, each an {alias, value} pair where alias is the name shown in the dashboard and logs and value is the identity that must match exactly. When set, every MCP request must carry a client identity equal to one of the values: a JWT caller is identified by the claim named in litellm_jwtauth.mcp_client_id_jwt_field, any other caller by the header named in mcp_client_id_header. A request with no resolvable identity, or an unlisted one, is rejected with 403. Unset means every client is admitted. */ - mcp_allowed_clients?: string[] | null; + mcp_allowed_clients?: components["schemas"]["MCPAllowedClient"][] | null; /** * Mcp Client Id Header * @description Request header whose value names the calling MCP client application (for example 'x-mcp-client') for callers that did not authenticate with a JWT, used only while mcp_allowed_clients is set. The client picks this value itself, so it is a policy control rather than a security boundary; prefer litellm_jwtauth.mcp_client_id_jwt_field where callers use JWTs. @@ -32491,6 +32491,22 @@ export interface components { */ status?: "healthy" | "unhealthy"; }; + /** + * MCPAllowedClient + * @description One entry of `general_settings.mcp_allowed_clients`. + */ + MCPAllowedClient: { + /** + * Alias + * @description Human-readable name for this client application, shown in the dashboard and in gateway logs. + */ + alias: string; + /** + * Value + * @description Exact value of the JWT claim named in litellm_jwtauth.mcp_client_id_jwt_field, or of the mcp_client_id_header header, that identifies this client application. Matched case-sensitively. + */ + value: string; + }; /** MCPConnectorEntry */ MCPConnectorEntry: { /** Args */ From da603c629ba465b8e81709847506580623450cb7 Mon Sep 17 00:00:00 2001 From: yassin Date: Fri, 18 Sep 2026 22:37:30 +0000 Subject: [PATCH 17/19] fix(ui): surface a malformed stored MCP allowlist as deny-all and let Save replace or remove it Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../_components/MCPNetworkSettings.test.tsx | 29 +++++++++- .../_components/MCPNetworkSettings.tsx | 55 +++++++++++++------ 2 files changed, 65 insertions(+), 19 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx index 92f6f7554d4..d27c18c5ae3 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx @@ -154,16 +154,39 @@ describe("MCPNetworkSettings", () => { expect(screen.getByRole("textbox", { name: "Client 2 value" })).toHaveValue("codex-mcp-client"); }); - it("ignores a stored allowlist in the old plain-string shape instead of rendering it", async () => { + it("warns that a stored allowlist in the old plain-string shape denies every client and lets Save remove it", async () => { vi.mocked(getGeneralSettingsCall).mockResolvedValue([ { field_name: "mcp_allowed_clients", field_value: ["antigravity-cli"] }, ]); renderSettings(); - await screen.findByText("Allowed Clients"); + expect(await screen.findByText(/stored allowlist is not a list of alias and value pairs/)).toBeVisible(); expect(screen.queryByRole("textbox", { name: "Client 1 value" })).not.toBeInTheDocument(); - expect(screen.queryByText(/every client is denied/)).not.toBeInTheDocument(); + + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => expect(deleteConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients")); + expect(updateConfigFieldSetting).not.toHaveBeenCalled(); + await waitFor(() => expect(screen.queryByText(/stored allowlist is not a list/)).not.toBeInTheDocument()); + }); + + it("replaces a stored allowlist in the old plain-string shape with the clients the admin adds", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_allowed_clients", field_value: ["antigravity-cli"] }, + ]); + + renderSettings(); + + await screen.findByText(/stored allowlist is not a list of alias and value pairs/); + await addClient(ANTIGRAVITY.alias, ANTIGRAVITY.value); + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ANTIGRAVITY]), + ); + expect(deleteConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_allowed_clients"); + await waitFor(() => expect(screen.queryByText(/stored allowlist is not a list/)).not.toBeInTheDocument()); }); it("adds clients as alias and value pairs and saves them under mcp_allowed_clients", async () => { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx index 2ef3ee8707d..ae1fad36599 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx @@ -42,10 +42,20 @@ const isAllowedClient = (entry: unknown): entry is AllowedClient => { return typeof alias === "string" && typeof value === "string"; }; -const parseStoredClients = (fieldValue: unknown): AllowedClient[] | null => - Array.isArray(fieldValue) && fieldValue.every(isAllowedClient) - ? fieldValue.map(({ alias, value }) => ({ alias, value })) - : null; +type StoredAllowlist = + | { readonly kind: "absent" } + | { readonly kind: "clients"; readonly clients: AllowedClient[] } + | { readonly kind: "malformed" }; + +const ABSENT: StoredAllowlist = { kind: "absent" }; + +const parseStoredClients = (fieldValue: unknown): StoredAllowlist => { + if (fieldValue === null || fieldValue === undefined) return ABSENT; + if (Array.isArray(fieldValue) && fieldValue.every(isAllowedClient)) { + return { kind: "clients", clients: fieldValue.map(({ alias, value }) => ({ alias, value })) }; + } + return { kind: "malformed" }; +}; let nextRowKey = 0; const newRow = (client: AllowedClient = { alias: "", value: "" }): AllowedClientRow => ({ @@ -66,8 +76,16 @@ const sameClients = (a: AllowedClient[], b: AllowedClient[]) => const unchangedSinceLoad = (value: string[], stored: string[] | null) => stored === null ? value.length === 0 : value.length > 0 && sameList(value, stored); -const clientsUnchangedSinceLoad = (value: AllowedClient[], stored: AllowedClient[] | null) => - stored === null ? value.length === 0 : value.length > 0 && sameClients(value, stored); +const clientsUnchangedSinceLoad = (value: AllowedClient[], stored: StoredAllowlist) => { + switch (stored.kind) { + case "absent": + return value.length === 0; + case "clients": + return value.length > 0 && sameClients(value, stored.clients); + case "malformed": + return false; + } +}; const headerUnchangedSinceLoad = (value: string, stored: string | null) => stored === null ? value === "" : value !== "" && value === stored; @@ -79,7 +97,7 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) const [allowedClients, setAllowedClients] = useState([]); const [clientIdHeader, setClientIdHeader] = useState(""); const [storedRanges, setStoredRanges] = useState(null); - const [storedClients, setStoredClients] = useState(null); + const [storedClients, setStoredClients] = useState(ABSENT); const [storedClientIdHeader, setStoredClientIdHeader] = useState(null); const [currentIp, setCurrentIp] = useState(null); const [rangeDraft, setRangeDraft] = useState(""); @@ -100,11 +118,9 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) setStoredRanges(field.field_value); } if (field.field_name === "mcp_allowed_clients") { - const clients = parseStoredClients(field.field_value); - if (clients !== null) { - setAllowedClients(clients.map(newRow)); - setStoredClients(clients); - } + const stored = parseStoredClients(field.field_value); + setAllowedClients(stored.kind === "clients" ? stored.clients.map(newRow) : []); + setStoredClients(stored); } if (field.field_name === "mcp_client_id_header" && typeof field.field_value === "string") { setClientIdHeader(field.field_value); @@ -145,11 +161,11 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) if (clientsUnchangedSinceLoad(clients, storedClients)) return; if (clients.length > 0) { await updateConfigFieldSetting(token, "mcp_allowed_clients", clients); - setStoredClients(clients); + setStoredClients({ kind: "clients", clients }); return; } await deleteConfigFieldSetting(token, "mcp_allowed_clients"); - setStoredClients(null); + setStoredClients(ABSENT); }; const persistClientIdHeader = async (token: string) => { @@ -216,7 +232,8 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) } const suggestedRange = currentIp ? ipToSlash24(currentIp) : null; - const storedAllowlistDeniesEveryone = storedClients !== null && storedClients.length === 0; + const storedAllowlistIsMalformed = storedClients.kind === "malformed"; + const storedAllowlistIsEmpty = storedClients.kind === "clients" && storedClients.clients.length === 0; return (
@@ -303,7 +320,13 @@ const MCPNetworkSettings: React.FC = ({ accessToken })

Allowed Clients

- {storedAllowlistDeniesEveryone && ( + {storedAllowlistIsMalformed && ( +

+ The stored allowlist is not a list of alias and value pairs, so every client is denied. Add the clients you + want and save to replace it, or save with the list empty to remove it and allow every client again. +

+ )} + {storedAllowlistIsEmpty && (

An empty allowlist is currently stored, so every client is denied. Save with the list empty to remove it and allow every client again. From c32309fb2de108768fa8704ee0696b29d383733c Mon Sep 17 00:00:00 2001 From: yassin Date: Sat, 19 Sep 2026 00:13:44 +0000 Subject: [PATCH 18/19] feat(ui): show MCP allowed clients as cards edited in a dialog Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../_components/MCPNetworkSettings.test.tsx | 114 +++++++++--- .../_components/MCPNetworkSettings.tsx | 167 ++++++++++++------ 2 files changed, 201 insertions(+), 80 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx index d27c18c5ae3..4cc87f1455c 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi, beforeEach } from "vitest"; import MCPNetworkSettings from "./MCPNetworkSettings"; @@ -26,12 +26,20 @@ const renderSettings = () => render(); const ANTIGRAVITY = { alias: "Antigravity CLI", value: "antigravity-cli" }; const CODEX = { alias: "Codex", value: "codex-mcp-client" }; +const clientCard = (alias: string) => screen.getByRole("button", { name: new RegExp(`^${alias}`) }); + +const fillClientDialog = async (alias: string, value: string) => { + const dialog = await screen.findByRole("dialog"); + fireEvent.change(within(dialog).getByRole("textbox", { name: "Alias" }), { target: { value: alias } }); + fireEvent.change(within(dialog).getByRole("textbox", { name: "Value" }), { target: { value } }); + return dialog; +}; + const addClient = async (alias: string, value: string) => { await userEvent.click(screen.getByRole("button", { name: "Add client" })); - const aliases = screen.getAllByRole("textbox", { name: /^Client \d+ alias$/ }); - const values = screen.getAllByRole("textbox", { name: /^Client \d+ value$/ }); - fireEvent.change(aliases[aliases.length - 1], { target: { value: alias } }); - fireEvent.change(values[values.length - 1], { target: { value } }); + const dialog = await fillClientDialog(alias, value); + await userEvent.click(within(dialog).getByRole("button", { name: "Add" })); + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); }; describe("MCPNetworkSettings", () => { @@ -139,7 +147,7 @@ describe("MCPNetworkSettings", () => { expect(updateConfigFieldSetting).not.toHaveBeenCalled(); }); - it("labels the section Allowed Clients and renders each stored client as an alias and value row", async () => { + it("labels the section Allowed Clients and renders each stored client as a card showing alias and value", async () => { vi.mocked(getGeneralSettingsCall).mockResolvedValue([ { field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY, CODEX] }, ]); @@ -148,10 +156,24 @@ describe("MCPNetworkSettings", () => { expect(await screen.findByText("Allowed Clients")).toBeVisible(); expect(screen.queryByText(/Allowed Client IDs/)).not.toBeInTheDocument(); - expect(screen.getByRole("textbox", { name: "Client 1 alias" })).toHaveValue("Antigravity CLI"); - expect(screen.getByRole("textbox", { name: "Client 1 value" })).toHaveValue("antigravity-cli"); - expect(screen.getByRole("textbox", { name: "Client 2 alias" })).toHaveValue("Codex"); - expect(screen.getByRole("textbox", { name: "Client 2 value" })).toHaveValue("codex-mcp-client"); + expect(screen.queryByText(/Allowed Client Applications/)).not.toBeInTheDocument(); + expect(clientCard("Antigravity CLI")).toHaveTextContent("antigravity-cli"); + expect(clientCard("Codex")).toHaveTextContent("codex-mcp-client"); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + it("opens an edit dialog when a client card is clicked, prefilled with that client's alias and value", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY, CODEX] }, + ]); + + renderSettings(); + await screen.findByText("Allowed Clients"); + await userEvent.click(clientCard("Codex")); + + const dialog = await screen.findByRole("dialog", { name: "Edit client" }); + expect(within(dialog).getByRole("textbox", { name: "Alias" })).toHaveValue("Codex"); + expect(within(dialog).getByRole("textbox", { name: "Value" })).toHaveValue("codex-mcp-client"); }); it("warns that a stored allowlist in the old plain-string shape denies every client and lets Save remove it", async () => { @@ -162,7 +184,7 @@ describe("MCPNetworkSettings", () => { renderSettings(); expect(await screen.findByText(/stored allowlist is not a list of alias and value pairs/)).toBeVisible(); - expect(screen.queryByRole("textbox", { name: "Client 1 value" })).not.toBeInTheDocument(); + expect(screen.queryByText("antigravity-cli")).not.toBeInTheDocument(); await userEvent.click(screen.getByRole("button", { name: /Save/ })); @@ -203,15 +225,22 @@ describe("MCPNetworkSettings", () => { expect(deleteConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_allowed_clients"); }); - it("edits a stored client's value in place and saves the new value", async () => { + it("edits a stored client's value through its dialog and saves the new value", async () => { vi.mocked(getGeneralSettingsCall).mockResolvedValue([ { field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY] }, ]); renderSettings(); - fireEvent.change(await screen.findByRole("textbox", { name: "Client 1 value" }), { - target: { value: "0oa1b2c3d4e5f6g7h8i9" }, + await screen.findByText("Allowed Clients"); + await userEvent.click(clientCard("Antigravity CLI")); + const dialog = await screen.findByRole("dialog"); + fireEvent.change(within(dialog).getByRole("textbox", { name: "Value" }), { + target: { value: " 0oa1b2c3d4e5f6g7h8i9 " }, }); + await userEvent.click(within(dialog).getByRole("button", { name: "Done" })); + + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + expect(clientCard("Antigravity CLI")).toHaveTextContent("0oa1b2c3d4e5f6g7h8i9"); await userEvent.click(screen.getByRole("button", { name: /Save/ })); await waitFor(() => @@ -221,22 +250,37 @@ describe("MCPNetworkSettings", () => { ); }); - it("refuses to save a client that has an alias but no value, and reports why", async () => { + it("keeps a stored client untouched when its dialog is cancelled", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY] }, + ]); + + renderSettings(); + await screen.findByText("Allowed Clients"); + await userEvent.click(clientCard("Antigravity CLI")); + const dialog = await screen.findByRole("dialog"); + fireEvent.change(within(dialog).getByRole("textbox", { name: "Value" }), { target: { value: "changed" } }); + await userEvent.click(within(dialog).getByRole("button", { name: "Cancel" })); + + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + expect(clientCard("Antigravity CLI")).toHaveTextContent("antigravity-cli"); + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => expect(toast.success).toHaveBeenCalledWith("MCP network settings saved")); + expect(updateConfigFieldSetting).not.toHaveBeenCalled(); + }); + + it("will not add a client that has an alias but no value", async () => { renderSettings(); await screen.findByText("Allowed Clients"); - await addClient("Antigravity CLI", ""); - await userEvent.click(screen.getByRole("button", { name: /Save/ })); + await userEvent.click(screen.getByRole("button", { name: "Add client" })); + const dialog = await fillClientDialog("Antigravity CLI", " "); - await waitFor(() => - expect(toast.fromError).toHaveBeenCalledWith(new Error("Every allowed client needs both an alias and a value")), - ); - expect(updateConfigFieldSetting).not.toHaveBeenCalled(); - expect(deleteConfigFieldSetting).not.toHaveBeenCalled(); - expect(toast.success).not.toHaveBeenCalled(); + expect(within(dialog).getByRole("button", { name: "Add" })).toBeDisabled(); }); - it("drops rows left completely blank instead of saving or failing on them", async () => { + it("adds nothing when the add dialog is cancelled", async () => { vi.mocked(getGeneralSettingsCall).mockResolvedValue([ { field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY] }, ]); @@ -244,6 +288,11 @@ describe("MCPNetworkSettings", () => { renderSettings(); await screen.findByText("Allowed Clients"); await userEvent.click(screen.getByRole("button", { name: "Add client" })); + const dialog = await fillClientDialog("Codex", "codex-mcp-client"); + await userEvent.click(within(dialog).getByRole("button", { name: "Cancel" })); + + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + expect(screen.queryByText("Codex")).not.toBeInTheDocument(); await userEvent.click(screen.getByRole("button", { name: /Save/ })); await waitFor(() => expect(toast.success).toHaveBeenCalledWith("MCP network settings saved")); @@ -260,13 +309,17 @@ describe("MCPNetworkSettings", () => { ]); renderSettings(); - await userEvent.click(await screen.findByRole("button", { name: "Remove client Claude Code" })); + await screen.findByText("Allowed Clients"); + await userEvent.click(clientCard("Claude Code")); + await userEvent.click(within(await screen.findByRole("dialog")).getByRole("button", { name: "Remove client" })); + + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + expect(screen.queryByText("claude-code")).not.toBeInTheDocument(); await userEvent.click(screen.getByRole("button", { name: /Save/ })); await waitFor(() => expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ANTIGRAVITY, CODEX]), ); - expect(screen.queryByDisplayValue("claude-code")).not.toBeInTheDocument(); }); it("removes a client and clears the setting when the list becomes empty", async () => { @@ -275,9 +328,12 @@ describe("MCPNetworkSettings", () => { ]); renderSettings(); - await userEvent.click(await screen.findByRole("button", { name: "Remove client Claude Code" })); + await screen.findByText("Allowed Clients"); + await userEvent.click(clientCard("Claude Code")); + await userEvent.click(within(await screen.findByRole("dialog")).getByRole("button", { name: "Remove client" })); - expect(screen.queryByDisplayValue("claude-code")).not.toBeInTheDocument(); + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + expect(screen.queryByText("claude-code")).not.toBeInTheDocument(); await userEvent.click(screen.getByRole("button", { name: /Save/ })); @@ -304,7 +360,7 @@ describe("MCPNetworkSettings", () => { renderSettings(); - await screen.findByText("Allowed Client Applications"); + await screen.findByText("Allowed Clients"); expect(screen.queryByText(/every client is denied/)).not.toBeInTheDocument(); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx index ae1fad36599..db45cfdedd1 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx @@ -1,9 +1,18 @@ -import React, { useState, useEffect } from "react"; +import React, { useState, useEffect, useId } from "react"; import { Save, Plus, X } from "lucide-react"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { Card } from "@/components/ui/card"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; import { UiLoadingSpinner } from "@/components/ui/ui-loading-spinner"; import { DeprecationBanner } from "@/components/DeprecationBanner"; import { toast } from "@/lib/toast"; @@ -36,6 +45,10 @@ interface AllowedClientRow extends AllowedClient { readonly key: string; } +interface ClientDraft extends AllowedClient { + readonly key: string | null; +} + const isAllowedClient = (entry: unknown): entry is AllowedClient => { if (typeof entry !== "object" || entry === null) return false; const { alias, value } = entry as Partial>; @@ -58,14 +71,10 @@ const parseStoredClients = (fieldValue: unknown): StoredAllowlist => { }; let nextRowKey = 0; -const newRow = (client: AllowedClient = { alias: "", value: "" }): AllowedClientRow => ({ - ...client, - key: `client-${nextRowKey++}`, -}); +const newRow = (client: AllowedClient): AllowedClientRow => ({ ...client, key: `client-${nextRowKey++}` }); const trimClient = ({ alias, value }: AllowedClient): AllowedClient => ({ alias: alias.trim(), value: value.trim() }); -const isBlank = ({ alias, value }: AllowedClient) => alias === "" && value === ""; const isIncomplete = ({ alias, value }: AllowedClient) => alias === "" || value === ""; const sameList = (a: string[], b: string[]) => a.length === b.length && a.every((value, i) => value === b[i]); @@ -90,6 +99,67 @@ const clientsUnchangedSinceLoad = (value: AllowedClient[], stored: StoredAllowli const headerUnchangedSinceLoad = (value: string, stored: string | null) => stored === null ? value === "" : value !== "" && value === stored; +interface AllowedClientDialogProps { + readonly draft: ClientDraft | null; + readonly onChange: (draft: ClientDraft) => void; + readonly onCommit: () => void; + readonly onRemove: () => void; + readonly onClose: () => void; +} + +const AllowedClientDialog: React.FC = ({ draft, onChange, onCommit, onRemove, onClose }) => { + const aliasId = useId(); + const valueId = useId(); + if (draft === null) return null; + return ( +

!open && onClose()}> + + + {draft.key === null ? "Add client" : "Edit client"} + + The alias is the name shown in the dashboard and gateway logs. The value is the exact JWT claim or header + value that identifies the client, such as the OAuth client ID your identity provider issues. + + +
+
+ + onChange({ ...draft, alias: e.target.value })} + /> +
+
+ + onChange({ ...draft, value: e.target.value })} + /> +
+
+ + {draft.key !== null && ( + + )} + + + +
+
+ ); +}; + const MCPNetworkSettings: React.FC = ({ accessToken }) => { const [loading, setLoading] = useState(true); const [saving, setSaving] = useState(false); @@ -101,6 +171,7 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) const [storedClientIdHeader, setStoredClientIdHeader] = useState(null); const [currentIp, setCurrentIp] = useState(null); const [rangeDraft, setRangeDraft] = useState(""); + const [clientDraft, setClientDraft] = useState(null); useEffect(() => { loadSettings(); @@ -154,10 +225,7 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) }; const persistAllowedClients = async (token: string) => { - const clients = allowedClients.map(trimClient).filter((client) => !isBlank(client)); - if (clients.some(isIncomplete)) { - throw new Error("Every allowed client needs both an alias and a value"); - } + const clients = allowedClients.map(({ alias, value }) => ({ alias, value })); if (clientsUnchangedSinceLoad(clients, storedClients)) return; if (clients.length > 0) { await updateConfigFieldSetting(token, "mcp_allowed_clients", clients); @@ -218,10 +286,22 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) setRangeDraft(""); }; - const updateClient = (key: string, patch: Partial) => - setAllowedClients(allowedClients.map((row) => (row.key === key ? { ...row, ...patch } : row))); + const commitClientDraft = () => { + if (clientDraft === null) return; + const client = trimClient(clientDraft); + setAllowedClients( + clientDraft.key === null + ? [...allowedClients, newRow(client)] + : allowedClients.map((row) => (row.key === clientDraft.key ? { ...row, ...client } : row)), + ); + setClientDraft(null); + }; - const removeClient = (key: string) => setAllowedClients(allowedClients.filter((row) => row.key !== key)); + const removeDraftedClient = () => { + if (clientDraft === null) return; + setAllowedClients(allowedClients.filter((row) => row.key !== clientDraft.key)); + setClientDraft(null); + }; if (loading) { return ( @@ -307,7 +387,7 @@ const MCPNetworkSettings: React.FC = ({ accessToken })
-

Allowed Client Applications

+

Allowed Clients

Only the MCP client applications listed here can use the gateway. Leave empty to allow every client. A client that authenticates with a JWT is identified by the claim named in litellm_jwtauth.mcp_client_id_jwt_field in @@ -317,9 +397,6 @@ const MCPNetworkSettings: React.FC = ({ accessToken })

-
-

Allowed Clients

-
{storedAllowlistIsMalformed && (

The stored allowlist is not a list of alias and value pairs, so every client is denied. Add the clients you @@ -333,35 +410,17 @@ const MCPNetworkSettings: React.FC = ({ accessToken })

)} {allowedClients.length > 0 && ( -
-

Alias

-

Value

- - {allowedClients.map((row, index) => ( - - updateClient(row.key, { alias: e.target.value })} - /> - updateClient(row.key, { value: e.target.value })} - /> - - +
+ {allowedClients.map((row) => ( + ))}
)} @@ -369,16 +428,14 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) type="button" variant="outline" size="sm" - onClick={() => setAllowedClients([...allowedClients, newRow()])} + onClick={() => setClientDraft({ key: null, alias: "", value: "" })} > Add client

- The alias is the name shown here and in gateway logs. The value is the exact JWT claim or header value that - identifies the client, such as the OAuth client ID your identity provider issues. Leave the list empty to - allow every client. Every MCP request from an unlisted client, or from one with no resolvable identity, gets a - 403. + Click a client to edit or remove it. Leave the list empty to allow every client. Every MCP request from an + unlisted client, or from one with no resolvable identity, gets a 403.

@@ -403,6 +460,14 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) Save
+ + setClientDraft(null)} + />
); }; From e2141da81ea059c6946c7c7c674babd7ef62443a Mon Sep 17 00:00:00 2001 From: yassin Date: Sat, 19 Sep 2026 00:23:11 +0000 Subject: [PATCH 19/19] fix(ui): treat MCP allowed clients with an empty alias or value as malformed Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../_components/MCPNetworkSettings.test.tsx | 11 +++++++++++ .../mcp-servers/_components/MCPNetworkSettings.tsx | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx index 4cc87f1455c..4a486bfc648 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.test.tsx @@ -211,6 +211,17 @@ describe("MCPNetworkSettings", () => { await waitFor(() => expect(screen.queryByText(/stored allowlist is not a list/)).not.toBeInTheDocument()); }); + it("treats a stored entry with an empty alias or value as denying every client, like the gateway does", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY, { alias: "", value: "claude-code" }] }, + ]); + + renderSettings(); + + expect(await screen.findByText(/stored allowlist is not a list of alias and value pairs/)).toBeVisible(); + expect(screen.queryByRole("button", { name: /^Antigravity CLI/ })).not.toBeInTheDocument(); + }); + it("adds clients as alias and value pairs and saves them under mcp_allowed_clients", async () => { renderSettings(); await screen.findByText("Allowed Clients"); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx index db45cfdedd1..2fd62c7f1ef 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPNetworkSettings.tsx @@ -52,7 +52,7 @@ interface ClientDraft extends AllowedClient { const isAllowedClient = (entry: unknown): entry is AllowedClient => { if (typeof entry !== "object" || entry === null) return false; const { alias, value } = entry as Partial>; - return typeof alias === "string" && typeof value === "string"; + return typeof alias === "string" && typeof value === "string" && !isIncomplete({ alias, value }); }; type StoredAllowlist =