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 */