From 2231a3ca433dbe7eed4d77167f93955833d989fd Mon Sep 17 00:00:00 2001 From: yassin Date: Fri, 18 Sep 2026 22:29:17 +0000 Subject: [PATCH] feat(mcp): give each allowed MCP client an alias and a value mcp_allowed_clients entries become {alias, value} objects: the value is what the JWT claim or header must equal, the alias is the name the dashboard and logs show. The Network Settings section is renamed Allowed Clients with one alias/value row per client Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../mcp_server/client_allowlist.py | 36 ++-- litellm/proxy/_types.py | 5 +- litellm/proxy/proxy_server.py | 2 +- litellm/types/mcp.py | 16 ++ .../mcp_server/test_client_allowlist.py | 88 ++++++--- .../mcp_server/test_mcp_server.py | 2 +- .../mcp_server/test_rest_endpoints.py | 2 +- tests/test_litellm/proxy/test_proxy_server.py | 8 +- .../_components/MCPNetworkSettings.test.tsx | 144 +++++++++++--- .../_components/MCPNetworkSettings.tsx | 183 +++++++++++------- ui/litellm-dashboard/src/lib/http/schema.d.ts | 20 +- 11 files changed, 356 insertions(+), 150 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/client_allowlist.py b/litellm/proxy/_experimental/mcp_server/client_allowlist.py index 1b8bf7aa9ba..524642600b3 100644 --- a/litellm/proxy/_experimental/mcp_server/client_allowlist.py +++ b/litellm/proxy/_experimental/mcp_server/client_allowlist.py @@ -1,6 +1,8 @@ """ Gateway-level allowlist of MCP client applications (``general_settings.mcp_allowed_clients``). +Each entry pairs an admin-chosen ``alias`` (shown in the dashboard and logs) with the ``value`` that +identifies the client. Only the value is compared, exactly and case-sensitively. A caller that authenticated with a JWT is identified by the claim named in ``litellm_jwtauth.mcp_client_id_jwt_field``, a value asserted by the identity provider. Every other caller is identified by the header named in ``general_settings.mcp_client_id_header``, @@ -10,6 +12,7 @@ While the allowlist is set, a caller with no usable identity source is rejected. from collections.abc import Mapping from dataclasses import dataclass +from types import MappingProxyType from typing import Final, Literal from pydantic import TypeAdapter, ValidationError @@ -17,15 +20,17 @@ from typing_extensions import ReadOnly, TypedDict from litellm._logging import verbose_logger from litellm.litellm_core_utils.dot_notation_indexing import get_nested_value +from litellm.types.mcp import MCPAllowedClient MCP_ALLOWED_CLIENTS_SETTING: Final = "mcp_allowed_clients" MCP_CLIENT_ID_HEADER_SETTING: Final = "mcp_client_id_header" MCP_CLIENT_ID_JWT_FIELD_SETTING: Final = "mcp_client_id_jwt_field" _JWT_AUTH_SETTING: Final = "litellm_jwtauth" -_ALLOWED_CLIENTS_ADAPTER: Final[TypeAdapter[list[str]]] = TypeAdapter(list[str]) +_ALLOWED_CLIENTS_ADAPTER: Final[TypeAdapter[list[MCPAllowedClient]]] = TypeAdapter(list[MCPAllowedClient]) _OPTIONAL_NAME_ADAPTER: Final[TypeAdapter[str | None]] = TypeAdapter(str | None) _OPTIONAL_MAPPING_ADAPTER: Final[TypeAdapter[dict[str, object] | None]] = TypeAdapter(dict[str, object] | None) +_NOBODY: Final[Mapping[str, str]] = MappingProxyType({}) class MCPClientForbiddenBody(TypedDict): @@ -35,7 +40,9 @@ class MCPClientForbiddenBody(TypedDict): @dataclass(frozen=True, slots=True) class MCPClientAllowlist: - allowed_clients: frozenset[str] + """``aliases_by_value`` maps each admitted identity value to the alias the admin gave it.""" + + aliases_by_value: Mapping[str, str] jwt_field: str | None header: str | None @@ -67,19 +74,20 @@ def _unidentified_rejection(reason: str) -> MCPClientRejection: ) -def parse_allowed_mcp_clients(raw_setting: object) -> frozenset[str] | None: - """None when the setting is absent (not enforced). A malformed setting admits nobody.""" +def parse_allowed_mcp_clients(raw_setting: object) -> Mapping[str, str] | None: + """Value-to-alias mapping; 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)) + clients: Final = _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", + "%s is not a list of {alias, value} entries (%r); rejecting every MCP client until it is fixed", MCP_ALLOWED_CLIENTS_SETTING, raw_setting, ) - return frozenset() + return _NOBODY + return MappingProxyType({client.value: client.alias for client in clients}) def _parse_optional_name(setting_name: str, raw_setting: object) -> str | None: @@ -112,7 +120,7 @@ def load_mcp_client_allowlist(general_settings: Mapping[str, object]) -> MCPClie MCP_CLIENT_ID_HEADER_SETTING, general_settings.get(MCP_CLIENT_ID_HEADER_SETTING) ) return MCPClientAllowlist( - allowed_clients=allowed_clients, + aliases_by_value=allowed_clients, jwt_field=_jwt_field_from_general_settings(general_settings), header=header.lower() if header is not None else None, ) @@ -154,8 +162,10 @@ def check_mcp_client_allowed( identity: Final = resolve_mcp_client_identity(allowlist, jwt_claims, headers) if isinstance(identity, MCPClientRejection): return identity - if identity.client_id in allowlist.allowed_clients: - return None - return MCPClientRejection( - details=f"MCP client {identity.description} is not listed in this gateway's {MCP_ALLOWED_CLIENTS_SETTING}." - ) + alias: Final = allowlist.aliases_by_value.get(identity.client_id) + if alias is None: + return MCPClientRejection( + details=f"MCP client {identity.description} is not listed in this gateway's {MCP_ALLOWED_CLIENTS_SETTING}." + ) + verbose_logger.debug("Admitted MCP client '%s' identified as %s", alias, identity.description) + return None diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 17e8387aceb..32bf59ce4bd 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -34,6 +34,7 @@ from litellm.types.llms.openai import ( ResponsesAPIResponse, ) from litellm.types.mcp import ( + MCPAllowedClient, MCPAuth, MCPAuthType, MCPCredentials, @@ -2899,9 +2900,9 @@ 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( + mcp_allowed_clients: list[MCPAllowedClient] | None = Field( None, - 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_client_id_header: str | None = Field( None, diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index d5857a8fe1a..182077ed565 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -17169,7 +17169,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_allowed_clients": "TypedDictionary", "mcp_client_id_header": "String", "mcp_trusted_proxy_ranges": "List", "mcp_xff_num_trusted_hops": "Integer", diff --git a/litellm/types/mcp.py b/litellm/types/mcp.py index c5a26c997b7..e0fd3e9a69d 100644 --- a/litellm/types/mcp.py +++ b/litellm/types/mcp.py @@ -91,6 +91,22 @@ class MCPPublicServer(BaseModel): mcp_info: dict[str, Any] | None = None +class MCPAllowedClient(BaseModel): + """One entry of `general_settings.mcp_allowed_clients`.""" + + model_config = ConfigDict(frozen=True) + + alias: str = Field( + min_length=1, + description="Human-readable name for this client application, shown in the dashboard and in gateway logs.", + ) + value: str = Field( + min_length=1, + 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.", + ) + + class MCPToolSearchSettings(BaseModel): """`litellm_settings.mcp_tool_search`: how the native `mcp_tool_search` virtual tool ranks the caller's tools.""" 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 index d8b52b7b348..d1f852e504e 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_client_allowlist.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_client_allowlist.py @@ -16,30 +16,43 @@ from litellm.proxy._experimental.mcp_server.client_allowlist import ( resolve_mcp_client_identity, ) -JWT_ONLY: Final = MCPClientAllowlist(allowed_clients=frozenset({"antigravity-cli"}), jwt_field="azp", header=None) -HEADER_ONLY: Final = MCPClientAllowlist( - allowed_clients=frozenset({"antigravity-cli"}), jwt_field=None, header="x-mcp-client" -) -JWT_AND_HEADER: Final = MCPClientAllowlist( - allowed_clients=frozenset({"antigravity-cli"}), jwt_field="azp", header="x-mcp-client" -) -NO_SOURCE: Final = MCPClientAllowlist(allowed_clients=frozenset({"antigravity-cli"}), jwt_field=None, header=None) +ANTIGRAVITY: Final = {"alias": "Antigravity CLI", "value": "antigravity-cli"} +CODEX: Final = {"alias": "Codex", "value": "codex-mcp-client"} +ANTIGRAVITY_ONLY: Final[Mapping[str, str]] = {"antigravity-cli": "Antigravity CLI"} +JWT_ONLY: Final = MCPClientAllowlist(aliases_by_value=ANTIGRAVITY_ONLY, jwt_field="azp", header=None) +HEADER_ONLY: Final = MCPClientAllowlist(aliases_by_value=ANTIGRAVITY_ONLY, jwt_field=None, header="x-mcp-client") +JWT_AND_HEADER: Final = MCPClientAllowlist(aliases_by_value=ANTIGRAVITY_ONLY, jwt_field="azp", header="x-mcp-client") +NO_SOURCE: Final = MCPClientAllowlist(aliases_by_value=ANTIGRAVITY_ONLY, jwt_field=None, header=None) NO_HEADERS: Final[Mapping[str, str]] = {} -_ALLOWLIST_SETTING_CASES: Final[tuple[tuple[object, frozenset[str] | None], ...]] = ( +_ALLOWLIST_SETTING_CASES: Final[tuple[tuple[object, Mapping[str, str] | None], ...]] = ( (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()), + ([], {}), + ([ANTIGRAVITY], ANTIGRAVITY_ONLY), + ([ANTIGRAVITY, CODEX], {"antigravity-cli": "Antigravity CLI", "codex-mcp-client": "Codex"}), + ( + [ANTIGRAVITY, {"alias": "Antigravity (prod)", "value": "antigravity-cli"}], + {"antigravity-cli": "Antigravity (prod)"}, + ), + ( + [ANTIGRAVITY, {"alias": "Antigravity CLI", "value": "antigravity-prod"}], + {**ANTIGRAVITY_ONLY, "antigravity-prod": "Antigravity CLI"}, + ), + (["antigravity-cli"], {}), + ("antigravity-cli", {}), + ([ANTIGRAVITY, 1], {}), + ([{"alias": "Antigravity CLI"}], {}), + ([{"value": "antigravity-cli"}], {}), + ([{"alias": "", "value": "antigravity-cli"}], {}), + ([{"alias": "Antigravity CLI", "value": ""}], {}), + ([{"alias": "Antigravity CLI", "value": ["antigravity-cli"]}], {}), + (ANTIGRAVITY, {}), ) @pytest.mark.parametrize(("raw_setting", "expected"), _ALLOWLIST_SETTING_CASES) -def test_parse_allowed_mcp_clients(raw_setting: object, expected: frozenset[str] | None) -> None: +def test_parse_allowed_mcp_clients(raw_setting: object, expected: Mapping[str, str] | None) -> None: assert parse_allowed_mcp_clients(raw_setting) == expected @@ -50,12 +63,12 @@ def test_load_returns_none_when_the_allowlist_setting_is_absent_even_if_identity def test_load_reads_the_jwt_field_from_litellm_jwtauth_and_lowercases_the_header_name() -> None: settings: Final = { - "mcp_allowed_clients": ["antigravity-cli", "codex-mcp-client"], + "mcp_allowed_clients": [ANTIGRAVITY, CODEX], "litellm_jwtauth": {"user_id_jwt_field": "sub", "mcp_client_id_jwt_field": "resource_access.mcp.client"}, "mcp_client_id_header": "X-MCP-Client", } assert load_mcp_client_allowlist(settings) == MCPClientAllowlist( - allowed_clients=frozenset({"antigravity-cli", "codex-mcp-client"}), + aliases_by_value={"antigravity-cli": "Antigravity CLI", "codex-mcp-client": "Codex"}, jwt_field="resource_access.mcp.client", header="x-mcp-client", ) @@ -64,10 +77,10 @@ def test_load_reads_the_jwt_field_from_litellm_jwtauth_and_lowercases_the_header @pytest.mark.parametrize( "settings", ( - {"mcp_allowed_clients": ["antigravity-cli"]}, - {"mcp_allowed_clients": ["antigravity-cli"], "litellm_jwtauth": {}, "mcp_client_id_header": ""}, - {"mcp_allowed_clients": ["antigravity-cli"], "litellm_jwtauth": {"mcp_client_id_jwt_field": ""}}, - {"mcp_allowed_clients": ["antigravity-cli"], "litellm_jwtauth": "azp", "mcp_client_id_header": ["x"]}, + {"mcp_allowed_clients": [ANTIGRAVITY]}, + {"mcp_allowed_clients": [ANTIGRAVITY], "litellm_jwtauth": {}, "mcp_client_id_header": ""}, + {"mcp_allowed_clients": [ANTIGRAVITY], "litellm_jwtauth": {"mcp_client_id_jwt_field": ""}}, + {"mcp_allowed_clients": [ANTIGRAVITY], "litellm_jwtauth": "azp", "mcp_client_id_header": ["x"]}, ), ) def test_load_without_a_usable_identity_source_keeps_the_allowlist_but_no_source( @@ -76,10 +89,31 @@ def test_load_without_a_usable_identity_source_keeps_the_allowlist_but_no_source assert load_mcp_client_allowlist(settings) == NO_SOURCE -def test_load_malformed_allowlist_admits_nobody() -> None: - loaded: Final = load_mcp_client_allowlist({"mcp_allowed_clients": "antigravity-cli"}) +@pytest.mark.parametrize("raw_setting", ("antigravity-cli", ["antigravity-cli"], [{"alias": "Antigravity CLI"}])) +def test_load_malformed_allowlist_admits_nobody(raw_setting: object) -> None: + loaded: Final = load_mcp_client_allowlist({"mcp_allowed_clients": raw_setting}) assert loaded is not None - assert loaded.allowed_clients == frozenset() + assert loaded.aliases_by_value == {} + assert check_mcp_client_allowed(loaded, {"azp": "antigravity-cli"}, {"x-mcp-client": "antigravity-cli"}) is not None + + +def test_only_the_value_identifies_a_client_never_its_alias() -> None: + assert check_mcp_client_allowed(JWT_ONLY, {"azp": "Antigravity CLI"}, NO_HEADERS) is not None + assert check_mcp_client_allowed(HEADER_ONLY, None, {"x-mcp-client": "Antigravity CLI"}) is not None + + +def test_two_clients_may_share_an_alias_and_both_are_admitted() -> None: + settings: Final = { + "mcp_allowed_clients": [ + {"alias": "Coding CLI", "value": "cli-dev"}, + {"alias": "Coding CLI", "value": "cli-prod"}, + ], + "litellm_jwtauth": {"mcp_client_id_jwt_field": "azp"}, + } + loaded: Final = load_mcp_client_allowlist(settings) + assert check_mcp_client_allowed(loaded, {"azp": "cli-dev"}, NO_HEADERS) is None + assert check_mcp_client_allowed(loaded, {"azp": "cli-prod"}, NO_HEADERS) is None + assert check_mcp_client_allowed(loaded, {"azp": "Coding CLI"}, NO_HEADERS) is not None def test_unconfigured_allowlist_admits_callers_with_no_identity_at_all() -> None: @@ -96,7 +130,7 @@ def test_jwt_claim_identifies_the_client() -> None: def test_nested_jwt_claim_path_is_resolved_with_dot_notation() -> None: nested: Final = MCPClientAllowlist( - allowed_clients=frozenset({"antigravity-cli"}), jwt_field="resource_access.mcp.client", header=None + aliases_by_value=ANTIGRAVITY_ONLY, jwt_field="resource_access.mcp.client", header=None ) claims: Final = {"resource_access": {"mcp": {"client": "antigravity-cli"}}} assert check_mcp_client_allowed(nested, claims, NO_HEADERS) is None @@ -178,6 +212,6 @@ def test_allowlist_with_no_identity_source_rejects_everyone_and_says_what_to_con def test_empty_allowlist_rejects_an_identified_client() -> None: - empty: Final = MCPClientAllowlist(allowed_clients=frozenset(), jwt_field="azp", header="x-mcp-client") + empty: Final = MCPClientAllowlist(aliases_by_value={}, jwt_field="azp", header="x-mcp-client") assert check_mcp_client_allowed(empty, {"azp": "antigravity-cli"}, NO_HEADERS) is not None assert check_mcp_client_allowed(empty, None, {"x-mcp-client": "antigravity-cli"}) is not None 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 9663315cc8b..33e14736357 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 @@ -2046,7 +2046,7 @@ _INITIALIZE: Final = ( ) _TOOLS_LIST: Final = b'{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}' _ALLOWLIST_SETTINGS: Final[dict[str, object]] = { - "mcp_allowed_clients": ["antigravity-cli"], + "mcp_allowed_clients": [{"alias": "Antigravity CLI", "value": "antigravity-cli"}], "litellm_jwtauth": {"mcp_client_id_jwt_field": "azp"}, "mcp_client_id_header": "x-mcp-client", } diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py index fccec57bb0c..ac3ad9ed89e 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py @@ -4316,7 +4316,7 @@ class TestV1ResolvedOauth2Gate: _CLIENT_ALLOWLIST_SETTINGS: Final[dict[str, object]] = { - "mcp_allowed_clients": ["antigravity-cli"], + "mcp_allowed_clients": [{"alias": "Antigravity CLI", "value": "antigravity-cli"}], "litellm_jwtauth": {"mcp_client_id_jwt_field": "azp"}, "mcp_client_id_header": "x-mcp-client", } diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index 522a8276148..f43815b25ab 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -14272,10 +14272,14 @@ def test_settings_store_exposes_dashboard_saved_mcp_client_allowlist_to_the_mcp_ assert load_mcp_client_allowlist(settings) is None settings.apply_db_row( - "general_settings", {"mcp_allowed_clients": ["antigravity-cli"], "mcp_client_id_header": "X-MCP-Client"} + "general_settings", + { + "mcp_allowed_clients": [{"alias": "Antigravity CLI", "value": "antigravity-cli"}], + "mcp_client_id_header": "X-MCP-Client", + }, ) assert load_mcp_client_allowlist(settings) == MCPClientAllowlist( - allowed_clients=frozenset({"antigravity-cli"}), jwt_field="azp", header="x-mcp-client" + aliases_by_value={"antigravity-cli": "Antigravity CLI"}, jwt_field="azp", header="x-mcp-client" ) settings.apply_db_row("general_settings", {"mcp_client_id_header": "X-MCP-Client"}) 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 f78461e2253..92f6f7554d4 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 @@ -23,6 +23,17 @@ vi.mock("@/lib/toast", () => ({ const renderSettings = () => render(); +const ANTIGRAVITY = { alias: "Antigravity CLI", value: "antigravity-cli" }; +const CODEX = { alias: "Codex", value: "codex-mcp-client" }; + +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 } }); +}; + describe("MCPNetworkSettings", () => { beforeEach(() => { vi.clearAllMocks(); @@ -128,47 +139,122 @@ describe("MCPNetworkSettings", () => { expect(updateConfigFieldSetting).not.toHaveBeenCalled(); }); - it("renders the stored allowed client IDs once settings load", async () => { + it("labels the section Allowed Clients and renders each stored client as an alias and value row", async () => { vi.mocked(getGeneralSettingsCall).mockResolvedValue([ - { field_name: "mcp_allowed_clients", field_value: ["antigravity-cli", "codex-mcp-client"] }, + { field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY, CODEX] }, ]); renderSettings(); - expect(await screen.findByText("antigravity-cli")).toBeInTheDocument(); - expect(screen.getByText("codex-mcp-client")).toBeInTheDocument(); + 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"); }); - it("adds typed client IDs on Enter and saves them under mcp_allowed_clients", async () => { + it("ignores a stored allowlist in the old plain-string shape instead of rendering it", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_allowed_clients", field_value: ["antigravity-cli"] }, + ]); + renderSettings(); - const input = await screen.findByRole("textbox", { name: "Allowed client IDs" }); - await userEvent.type(input, "antigravity-cli, codex-mcp-client{Enter}"); + await screen.findByText("Allowed Clients"); + expect(screen.queryByRole("textbox", { name: "Client 1 value" })).not.toBeInTheDocument(); + expect(screen.queryByText(/every client is denied/)).not.toBeInTheDocument(); + }); - expect(screen.getByText("antigravity-cli")).toBeInTheDocument(); - expect(screen.getByText("codex-mcp-client")).toBeInTheDocument(); - expect(input).toHaveValue(""); + it("adds clients as alias and value pairs and saves them under mcp_allowed_clients", async () => { + renderSettings(); + await screen.findByText("Allowed Clients"); + await addClient(" Antigravity CLI ", " antigravity-cli "); + await addClient("Codex", "codex-mcp-client"); await userEvent.click(screen.getByRole("button", { name: /Save/ })); await waitFor(() => - expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ - "antigravity-cli", - "codex-mcp-client", - ]), + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ANTIGRAVITY, CODEX]), ); expect(deleteConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_allowed_clients"); }); - it("removes a client ID and clears the setting when the list becomes empty", async () => { + it("edits a stored client's value in place and saves the new value", async () => { vi.mocked(getGeneralSettingsCall).mockResolvedValue([ - { field_name: "mcp_allowed_clients", field_value: ["claude-code"] }, + { field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY] }, ]); renderSettings(); - await userEvent.click(await screen.findByRole("button", { name: "Remove claude-code" })); + fireEvent.change(await screen.findByRole("textbox", { name: "Client 1 value" }), { + target: { value: "0oa1b2c3d4e5f6g7h8i9" }, + }); + await userEvent.click(screen.getByRole("button", { name: /Save/ })); - expect(screen.queryByText("claude-code")).not.toBeInTheDocument(); + await waitFor(() => + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ + { alias: "Antigravity CLI", value: "0oa1b2c3d4e5f6g7h8i9" }, + ]), + ); + }); + + it("refuses to save a client that has an alias but no value, and reports why", async () => { + renderSettings(); + await screen.findByText("Allowed Clients"); + + await addClient("Antigravity CLI", ""); + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + 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(); + }); + + it("drops rows left completely blank instead of saving or failing on them", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY] }, + ]); + + renderSettings(); + await screen.findByText("Allowed Clients"); + await userEvent.click(screen.getByRole("button", { name: "Add client" })); + await userEvent.click(screen.getByRole("button", { name: /Save/ })); + + await waitFor(() => expect(toast.success).toHaveBeenCalledWith("MCP network settings saved")); + expect(updateConfigFieldSetting).not.toHaveBeenCalled(); + expect(deleteConfigFieldSetting).not.toHaveBeenCalled(); + }); + + it("removes the right client from the middle of the list", async () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { + field_name: "mcp_allowed_clients", + field_value: [ANTIGRAVITY, { alias: "Claude Code", value: "claude-code" }, CODEX], + }, + ]); + + renderSettings(); + await userEvent.click(await screen.findByRole("button", { name: "Remove client Claude Code" })); + 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 () => { + vi.mocked(getGeneralSettingsCall).mockResolvedValue([ + { field_name: "mcp_allowed_clients", field_value: [{ alias: "Claude Code", value: "claude-code" }] }, + ]); + + renderSettings(); + await userEvent.click(await screen.findByRole("button", { name: "Remove client Claude Code" })); + + expect(screen.queryByDisplayValue("claude-code")).not.toBeInTheDocument(); await userEvent.click(screen.getByRole("button", { name: /Save/ })); @@ -265,18 +351,16 @@ describe("MCPNetworkSettings", () => { 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"] }, + { field_name: "mcp_allowed_clients", field_value: [ANTIGRAVITY] }, ]); renderSettings(); - await userEvent.type(await screen.findByRole("textbox", { name: "Allowed client IDs" }), "codex-mcp-client{Enter}"); + await screen.findByText("Allowed Clients"); + await addClient("Codex", "codex-mcp-client"); await userEvent.click(screen.getByRole("button", { name: /Save/ })); await waitFor(() => - expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ - "antigravity-cli", - "codex-mcp-client", - ]), + expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [ANTIGRAVITY, CODEX]), ); expect(updateConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_internal_ip_ranges", expect.anything()); expect(deleteConfigFieldSetting).not.toHaveBeenCalled(); @@ -291,12 +375,10 @@ describe("MCPNetworkSettings", () => { renderSettings(); await userEvent.click(await screen.findByRole("button", { name: "Remove 10.0.0.0/8" })); - await userEvent.type(screen.getByRole("textbox", { name: "Allowed client IDs" }), "codex-mcp-client{Enter}"); + await addClient("Codex", "codex-mcp-client"); await userEvent.click(screen.getByRole("button", { name: /Save/ })); - await waitFor(() => - expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", ["codex-mcp-client"]), - ); + await waitFor(() => expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [CODEX])); await waitFor(() => expect(toast.fromError).toHaveBeenCalledWith(rangeFailure)); expect(toast.success).not.toHaveBeenCalled(); }); @@ -317,7 +399,7 @@ describe("MCPNetworkSettings", () => { renderSettings(); await userEvent.click(await screen.findByText("203.0.113.0/24")); - await userEvent.type(screen.getByRole("textbox", { name: "Allowed client IDs" }), "codex-mcp-client{Enter}"); + await addClient("Codex", "codex-mcp-client"); await userEvent.click(screen.getByRole("button", { name: /Save/ })); await waitFor(() => @@ -326,9 +408,7 @@ describe("MCPNetworkSettings", () => { expect(updateConfigFieldSetting).not.toHaveBeenCalledWith("tok", "mcp_allowed_clients", expect.anything()); finishRangeWrite?.(); - await waitFor(() => - expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", ["codex-mcp-client"]), - ); + await waitFor(() => expect(updateConfigFieldSetting).toHaveBeenCalledWith("tok", "mcp_allowed_clients", [CODEX])); await waitFor(() => expect(toast.success).toHaveBeenCalledWith("MCP network settings saved")); }); }); 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 61294e1eb8e..2ef3ee8707d 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 @@ -27,11 +27,48 @@ function ipToSlash24(ip: string): string { return `${parts[0]}.${parts[1]}.${parts[2]}.0/24`; } +export interface AllowedClient { + readonly alias: string; + readonly value: string; +} + +interface AllowedClientRow extends AllowedClient { + readonly key: string; +} + +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"; +}; + +const parseStoredClients = (fieldValue: unknown): AllowedClient[] | null => + Array.isArray(fieldValue) && fieldValue.every(isAllowedClient) + ? fieldValue.map(({ alias, value }) => ({ alias, value })) + : null; + +let nextRowKey = 0; +const newRow = (client: AllowedClient = { alias: "", value: "" }): 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]); +const sameClients = (a: AllowedClient[], b: AllowedClient[]) => + a.length === b.length && a.every((client, i) => client.alias === b[i].alias && client.value === b[i].value); + 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 headerUnchangedSinceLoad = (value: string, stored: string | null) => stored === null ? value === "" : value !== "" && value === stored; @@ -39,14 +76,13 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) const [loading, setLoading] = useState(true); const [saving, setSaving] = useState(false); const [privateRanges, setPrivateRanges] = useState([]); - const [allowedClients, setAllowedClients] = useState([]); + const [allowedClients, setAllowedClients] = useState([]); const [clientIdHeader, setClientIdHeader] = useState(""); const [storedRanges, setStoredRanges] = useState(null); - const [storedClients, setStoredClients] = useState(null); + const [storedClients, setStoredClients] = useState(null); const [storedClientIdHeader, setStoredClientIdHeader] = useState(null); const [currentIp, setCurrentIp] = useState(null); const [rangeDraft, setRangeDraft] = useState(""); - const [clientDraft, setClientDraft] = useState(""); useEffect(() => { loadSettings(); @@ -63,9 +99,12 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) setPrivateRanges(field.field_value); setStoredRanges(field.field_value); } - if (field.field_name === "mcp_allowed_clients" && Array.isArray(field.field_value)) { - setAllowedClients(field.field_value); - setStoredClients(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); + } } if (field.field_name === "mcp_client_id_header" && typeof field.field_value === "string") { setClientIdHeader(field.field_value); @@ -87,23 +126,30 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) } }; - const persistList = async ( - token: string, - fieldName: "mcp_internal_ip_ranges" | "mcp_allowed_clients", - { - value, - stored, - setStored, - }: { value: string[]; stored: string[] | null; setStored: (value: string[] | null) => void }, - ) => { - if (unchangedSinceLoad(value, stored)) return; - if (value.length > 0) { - await updateConfigFieldSetting(token, fieldName, value); - setStored(value); + const persistRanges = async (token: string) => { + if (unchangedSinceLoad(privateRanges, storedRanges)) return; + if (privateRanges.length > 0) { + await updateConfigFieldSetting(token, "mcp_internal_ip_ranges", privateRanges); + setStoredRanges(privateRanges); return; } - await deleteConfigFieldSetting(token, fieldName); - setStored(null); + await deleteConfigFieldSetting(token, "mcp_internal_ip_ranges"); + setStoredRanges(null); + }; + + 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"); + } + if (clientsUnchangedSinceLoad(clients, storedClients)) return; + if (clients.length > 0) { + await updateConfigFieldSetting(token, "mcp_allowed_clients", clients); + setStoredClients(clients); + return; + } + await deleteConfigFieldSetting(token, "mcp_allowed_clients"); + setStoredClients(null); }; const persistClientIdHeader = async (token: string) => { @@ -121,20 +167,8 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) const handleSave = async () => { if (!accessToken) return; setSaving(true); - const [rangeResult] = await Promise.allSettled([ - persistList(accessToken, "mcp_internal_ip_ranges", { - value: privateRanges, - stored: storedRanges, - setStored: setStoredRanges, - }), - ]); - const [clientResult] = await Promise.allSettled([ - persistList(accessToken, "mcp_allowed_clients", { - value: allowedClients, - stored: storedClients, - setStored: setStoredClients, - }), - ]); + const [rangeResult] = await Promise.allSettled([persistRanges(accessToken)]); + const [clientResult] = await Promise.allSettled([persistAllowedClients(accessToken)]); const [headerResult] = await Promise.allSettled([persistClientIdHeader(accessToken)]); setSaving(false); const failures = [rangeResult, clientResult, headerResult].filter( @@ -168,13 +202,10 @@ const MCPNetworkSettings: React.FC = ({ accessToken }) setRangeDraft(""); }; - const commitClientDraft = () => { - const added = splitDraft(clientDraft, allowedClients); - if (added.length > 0) { - setAllowedClients([...allowedClients, ...added]); - } - setClientDraft(""); - }; + const updateClient = (key: string, patch: Partial) => + setAllowedClients(allowedClients.map((row) => (row.key === key ? { ...row, ...patch } : row))); + + const removeClient = (key: string) => setAllowedClients(allowedClients.filter((row) => row.key !== key)); if (loading) { return ( @@ -270,7 +301,7 @@ const MCPNetworkSettings: React.FC = ({ accessToken })
-

Allowed Client IDs

+

Allowed Clients

{storedAllowlistDeniesEveryone && (

@@ -279,38 +310,52 @@ const MCPNetworkSettings: React.FC = ({ accessToken })

)} {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 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 */