From 25c8926c48874b6c0d3307be21d7510e7520d0d1 Mon Sep 17 00:00:00 2001 From: jesus Date: Fri, 4 Sep 2026 20:51:41 +0000 Subject: [PATCH 01/11] fix(mcp): keep oauth scopes in admin api credential redaction Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../mcp_management_endpoints.py | 34 +++++++-- .../test_mcp_management_endpoints.py | 75 ++++++++++++++++++- 2 files changed, 98 insertions(+), 11 deletions(-) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 5326cf3415f..23e74e8b9b6 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -22,7 +22,14 @@ import os from collections.abc import Iterable, Mapping, Sequence from dataclasses import dataclass from datetime import datetime, timedelta, timezone -from typing import TYPE_CHECKING, Annotated, Final, Literal, Protocol +from typing import ( + TYPE_CHECKING, + Annotated, + Final, + Literal, + Protocol, + cast, # noqa: TID251 # validated JSON values need explicit narrowing +) from fastapi import ( APIRouter, @@ -628,8 +635,8 @@ if MCP_AVAILABLE: def _preserved_admin_config_credentials( credentials: "MCPCredentials | str | None", - ) -> "dict[str, str] | None": - """Keep only the non-secret admin-config keys, which are stored unencrypted so they lift out + ) -> "dict[str, str | list[str]] | None": + """Keep non-secret admin-config keys and scopes, which are stored unencrypted so they lift out as plaintext; every secret and minted-token key is dropped. Total over every stored shape: a dict is read directly, a JSON-object string is parsed, and @@ -639,15 +646,26 @@ if MCP_AVAILABLE: parsed: object = credentials if isinstance(credentials, str): try: - parsed = json.loads(credentials) + parsed = cast(object, json.loads(credentials)) except (ValueError, TypeError): return None if not isinstance(parsed, dict): return None - preserved: Final = { - key: value - for key in MCP_ADMIN_CONFIG_CREDENTIAL_KEYS - if isinstance((value := parsed.get(key)), str) and value + parsed_credentials: Final = cast(Mapping[str, object], parsed) + scopes: Final[object] = parsed_credentials.get("scopes") + scopes_as_objects: Final[list[object]] = cast(list[object], scopes) if isinstance(scopes, list) else [] + preserved_scopes: Final[dict[str, list[str]]] = ( + {"scopes": cast(list[str], scopes_as_objects)} + if scopes_as_objects and all(isinstance(scope, str) and scope for scope in scopes_as_objects) + else {} + ) + preserved: Final[dict[str, str | list[str]]] = { + **{ + key: value + for key in MCP_ADMIN_CONFIG_CREDENTIAL_KEYS + if isinstance((value := parsed_credentials.get(key)), str) and value + }, + **preserved_scopes, } return preserved or None diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index afadd6f3d19..b273c1c95fa 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -6,7 +6,7 @@ import logging from contextlib import ExitStack from datetime import datetime, timedelta from types import SimpleNamespace -from typing import List, Optional +from typing import List, Optional, cast from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -29,7 +29,7 @@ from litellm.proxy._types import ( UpdateMCPServerRequest, UserAPIKeyAuth, ) -from litellm.types.mcp import MCPAuth +from litellm.types.mcp import MCPAuth, MCPCredentials from litellm.types.mcp_server.mcp_server_manager import MCPServer @@ -864,6 +864,75 @@ class TestListMCPServers: assert result.credentials == expected + @pytest.mark.parametrize( + "stored_credentials, expected", + [ + ( + { + "client_id": "cid", + "client_secret": "csecret", + "scopes": ["read", "write"], + "upstream_token_header": "esb-oauth", + }, + {"scopes": ["read", "write"], "upstream_token_header": "esb-oauth"}, + ), + ( + '{"client_id": "cid", "client_secret": "csecret", "scopes": ["read", "write"], ' + '"upstream_token_header": "esb-oauth"}', + {"scopes": ["read", "write"], "upstream_token_header": "esb-oauth"}, + ), + ( + {"client_id": "cid", "client_secret": "csecret", "scopes": ["read", ""]}, + None, + ), + ( + {"client_id": "cid", "client_secret": "csecret", "scopes": "read"}, + None, + ), + ], + ) + @pytest.mark.asyncio + async def test_fetch_single_mcp_server_preserves_valid_oauth_scopes( + self, stored_credentials: object, expected: object + ): + mock_server = generate_mock_mcp_server_db_record(server_id="server-scopes", alias="Scopes") + mock_server.credentials = cast(MCPCredentials, stored_credentials) + mock_health_result = generate_mock_mcp_server_db_record(server_id="server-scopes", alias="Scopes") + mock_health_result.status = "healthy" + mock_health_result.last_health_check = datetime.now() + mock_health_result.health_check_error = None + mock_user_auth = generate_mock_user_api_key_auth(user_role=LitellmUserRoles.PROXY_ADMIN) + + with ( + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints.get_prisma_client_or_throw", + return_value=MagicMock(), + ), + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints.get_mcp_server", + AsyncMock(return_value=mock_server), + ), + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints.global_mcp_server_manager.health_check_server", + AsyncMock(return_value=mock_health_result), + ), + patch( + "litellm.proxy.management_endpoints.mcp_management_endpoints._user_has_admin_view", + return_value=True, + ), + ): + from litellm.proxy.management_endpoints.mcp_management_endpoints import ( + fetch_mcp_server, + ) + + result = await fetch_mcp_server( + request=_make_mock_request(), + server_id="server-scopes", + user_api_key_dict=mock_user_auth, + ) + + assert result.credentials == expected + @pytest.mark.asyncio async def test_fetch_single_mcp_server_strips_upstream_resource_for_non_admin(self): """A non-full-admin viewer gets the whole blob nulled, including the non-secret admin config, @@ -2339,7 +2408,7 @@ class TestTemporaryMCPSessionEndpoints: "client_secret": "client-secret", "scopes": ["scope1"], } - assert response.credentials is None + assert response.credentials == {"scopes": ["scope1"]} @pytest.mark.asyncio async def test_add_session_mcp_server_rejects_non_admins(self): From 3a97dc4d4a2014af7c2ddb0f2cab5307d7b906ad Mon Sep 17 00:00:00 2001 From: jesus Date: Fri, 4 Sep 2026 21:04:08 +0000 Subject: [PATCH 02/11] fix(mcp): satisfy type-discipline lint Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../mcp_management_endpoints.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 23e74e8b9b6..02edfcb2e74 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -635,7 +635,7 @@ if MCP_AVAILABLE: def _preserved_admin_config_credentials( credentials: "MCPCredentials | str | None", - ) -> "dict[str, str | list[str]] | None": + ) -> "dict[str, str | list[str]] | None": # mutable-ok: API response payload """Keep non-secret admin-config keys and scopes, which are stored unencrypted so they lift out as plaintext; every secret and minted-token key is dropped. @@ -646,20 +646,24 @@ if MCP_AVAILABLE: parsed: object = credentials if isinstance(credentials, str): try: - parsed = cast(object, json.loads(credentials)) + parsed = cast(object, json.loads(credentials)) # cast-ok: JSON parse result is validated below except (ValueError, TypeError): return None if not isinstance(parsed, dict): return None - parsed_credentials: Final = cast(Mapping[str, object], parsed) + parsed_credentials: Final = cast(Mapping[str, object], parsed) # cast-ok: dict shape validated above scopes: Final[object] = parsed_credentials.get("scopes") - scopes_as_objects: Final[list[object]] = cast(list[object], scopes) if isinstance(scopes, list) else [] - preserved_scopes: Final[dict[str, list[str]]] = ( - {"scopes": cast(list[str], scopes_as_objects)} + scopes_as_objects: Final = ( + cast(Sequence[object], scopes) # cast-ok: list shape validated above + if isinstance(scopes, list) + else () + ) + preserved_scopes: Final = ( + {"scopes": cast(list[str], scopes_as_objects)} # cast-ok: every scope is validated below if scopes_as_objects and all(isinstance(scope, str) and scope for scope in scopes_as_objects) else {} ) - preserved: Final[dict[str, str | list[str]]] = { + preserved: Final = { # mutable-ok: API response payload **{ key: value for key in MCP_ADMIN_CONFIG_CREDENTIAL_KEYS From 9cff026baee626d60da352e3098feec8ce5ca3ea Mon Sep 17 00:00:00 2001 From: jesus Date: Fri, 4 Sep 2026 21:08:47 +0000 Subject: [PATCH 03/11] test(mcp): satisfy patch quality checks Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../management_endpoints/test_mcp_management_endpoints.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index b273c1c95fa..46b6b48c3aa 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -835,19 +835,19 @@ class TestListMCPServers: mock_user_auth = generate_mock_user_api_key_auth(user_role=LitellmUserRoles.PROXY_ADMIN) with ( - patch( + patch( # test-quality-ok: endpoint test must patch module globals "litellm.proxy.management_endpoints.mcp_management_endpoints.get_prisma_client_or_throw", return_value=MagicMock(), ), - patch( + patch( # test-quality-ok: endpoint test must patch module globals "litellm.proxy.management_endpoints.mcp_management_endpoints.get_mcp_server", AsyncMock(return_value=mock_server), ), - patch( + patch( # test-quality-ok: endpoint test must patch module globals "litellm.proxy.management_endpoints.mcp_management_endpoints.global_mcp_server_manager.health_check_server", AsyncMock(return_value=mock_health_result), ), - patch( + patch( # test-quality-ok: endpoint test must patch module globals "litellm.proxy.management_endpoints.mcp_management_endpoints._user_has_admin_view", return_value=True, ), From f59cd303c6ca1bcaf1b5647999f8c4440d52c670 Mon Sep 17 00:00:00 2001 From: Yucheng He Date: Fri, 18 Sep 2026 11:29:02 -0700 Subject: [PATCH 04/11] fix(mcp): preserve scopes through admin server edits --- .../mcp_server/mcp_server_manager.py | 1 + .../mcp_management_endpoints.py | 4 +- .../mcp_server/test_mcp_server_manager.py | 4 ++ .../test_mcp_management_endpoints.py | 41 ++++++++++++++----- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 469ea86ad4b..08151d8cab2 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -7088,6 +7088,7 @@ class MCPServerManager: spec_path=server.spec_path, transport=server.transport, auth_type=server.auth_type, + credentials={"scopes": server.scopes} if server.scopes else None, created_at=server.created_at, updated_at=server.updated_at, teams=[], diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 02edfcb2e74..c1388e8bb81 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -849,7 +849,9 @@ if MCP_AVAILABLE: if not credentials: return False as_dict: Final[dict[str, object]] = dict(credentials) - return any(value for key, value in as_dict.items() if key not in MCP_ADMIN_CONFIG_CREDENTIAL_KEYS) + return any( + value for key, value in as_dict.items() if key not in MCP_ADMIN_CONFIG_CREDENTIAL_KEYS and key != "scopes" + ) def _inherit_credentials_from_existing_server( payload: NewMCPServerRequest, diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py index d449ad06642..c0aa6c9cd32 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py @@ -10424,11 +10424,15 @@ def test_build_mcp_server_table_carries_oauth2_flow(): transport=MCPTransport.http, auth_type=MCPAuth.oauth2, oauth2_flow="client_credentials", + client_id="client-123", + client_secret="secret-xyz", + scopes=["scope:a", "scope:b"], ) table = manager._build_mcp_server_table(server) assert table.oauth2_flow == "client_credentials" + assert table.credentials == {"scopes": ["scope:a", "scope:b"]} def test_build_mcp_server_table_carries_null_oauth2_flow(): diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index 46b6b48c3aa..f3fc45480e1 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -1704,14 +1704,26 @@ class TestTemporaryMCPSessionEndpoints: return _inherit_credentials_from_existing_server(payload) - def test_admin_config_alone_does_not_suppress_credential_inheritance(self): - """The edit form round-trips upstream_resource, which is admin config rather than a credential. - Treating the blob as "credentials supplied" left the Authorize session with no declared app on - the exact path where this knob is configured.""" - updated = self._inherit_with({"upstream_resource": "api://audience"}) + @pytest.mark.parametrize( + "credentials", + [ + {"upstream_resource": "api://audience"}, + {"scopes": ["scope:a", "scope:b"]}, + {"scopes": ["scope:edited"], "upstream_resource": "api://audience"}, + {"scopes": ["scope:edited"], "upstream_token_header": "esb-oauth"}, + {"scopes": []}, + {"scopes": None}, + ], + ) + def test_admin_config_alone_does_not_suppress_credential_inheritance(self, credentials: MCPCredentials): + updated = self._inherit_with(credentials, scopes=["scope:stored"]) - assert updated.credentials["client_id"] == "client-123" - assert updated.credentials["client_secret"] == "secret-xyz" + assert updated.credentials == { + "client_id": "client-123", + "client_secret": "secret-xyz", + "scopes": ["scope:stored"], + **credentials, + } def test_upstream_token_header_is_inherited_like_other_admin_config(self): """It is admin config rather than a credential, so a session server derived from an existing @@ -1730,11 +1742,18 @@ class TestTemporaryMCPSessionEndpoints: assert updated.credentials["client_secret"] == "secret-xyz" assert updated.credentials["upstream_token_header"] == "esb-oauth" - def test_supplied_credential_still_wins_over_inheritance(self): - """A caller that supplies a real credential keeps it; inheritance must not overwrite it.""" - updated = self._inherit_with({"auth_value": "caller-token"}) + @pytest.mark.parametrize( + "credentials", + [ + {"auth_value": "caller-token"}, + {"client_id": "caller-client", "scopes": ["scope:edited"]}, + {"client_secret": "caller-secret", "scopes": ["scope:edited"]}, + ], + ) + def test_supplied_credential_still_wins_over_inheritance(self, credentials: MCPCredentials): + updated = self._inherit_with(credentials) - assert updated.credentials == {"auth_value": "caller-token"} + assert updated.credentials == credentials def test_inheritance_carries_upstream_resource_to_the_session_server(self): """Without this the temporary server omits the resource indicator and the Authorize leg it From 0b2dd9ba86370d3d5a18e3a053e47762fdbfccfb Mon Sep 17 00:00:00 2001 From: Yucheng He Date: Fri, 18 Sep 2026 12:45:50 -0700 Subject: [PATCH 05/11] fix(mcp): sanitize submissions for restricted admin keys --- .../mcp_management_endpoints.py | 4 +- .../test_mcp_management_endpoints.py | 48 ++++++++----------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index c1388e8bb81..df4d22fc1bf 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -1486,7 +1486,9 @@ if MCP_AVAILABLE: submissions: Final = await get_mcp_submissions(prisma_client) submissions.items = _redact_mcp_credentials_list(submissions.items) - if not _user_is_full_admin(user_api_key_dict): + if _is_restricted_virtual_key_request(user_api_key_dict): + submissions.items = _sanitize_mcp_server_list_for_virtual_key(submissions.items) + elif not _user_is_full_admin(user_api_key_dict): submissions.items = _sanitize_mcp_server_list_for_non_admin(submissions.items) return submissions diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index f3fc45480e1..448b3334b18 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -6,7 +6,7 @@ import logging from contextlib import ExitStack from datetime import datetime, timedelta from types import SimpleNamespace -from typing import List, Optional, cast +from typing import Final, List, Optional, cast from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -4625,18 +4625,17 @@ class TestMCPApprovalWorkflow: assert item.static_headers == {"Authorization": "Bearer sk-secret-header"} @pytest.mark.asyncio - async def test_get_submissions_full_admin_still_sees_secrets(self): - """The view-only redaction must not over-redact for a full PROXY_ADMIN, - who needs url/static_headers/env/env_vars to review the pending - submission. Only the explicit credentials field is cleared.""" + @pytest.mark.parametrize("allowed_routes", [[], ["llm_api_routes"], ["mcp_routes"]]) + async def test_get_submissions_respects_admin_key_route_restrictions(self, allowed_routes: list[str]) -> None: from litellm.proxy._types import MCPSubmissionsSummary - from litellm.proxy.management_endpoints.mcp_management_endpoints import ( - get_mcp_server_submissions, - ) - item = _leaky_list_server() - item.approval_status = "pending_review" - summary = MCPSubmissionsSummary(total=1, pending_review=1, active=0, rejected=0, items=[item]) + credentials: Final[MCPCredentials] = {"scopes": ["scope:review"], "client_secret": "secret-sentinel"} + item: Final = _leaky_list_server().model_copy( + update={"approval_status": "pending_review", "credentials": credentials} + ) + original: Final = item.model_dump() + summary: Final = MCPSubmissionsSummary(total=1, pending_review=1, active=0, rejected=0, items=[item]) + admin: Final = UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN, allowed_routes=allowed_routes) with ( patch( @@ -4648,25 +4647,18 @@ class TestMCPApprovalWorkflow: AsyncMock(return_value=summary), ), ): - result = await get_mcp_server_submissions( - user_api_key_dict=generate_mock_user_api_key_auth(user_role=LitellmUserRoles.PROXY_ADMIN), - ) + result: Final = await mgmt_endpoints.get_mcp_server_submissions(user_api_key_dict=admin) + assert (result.total, result.pending_review, result.active, result.rejected) == (1, 1, 0, 0) assert len(result.items) == 1 - raw = result.items[0] - assert raw.url == "https://leaky.example.com/mcp?api_key=sk-embedded-in-url" - assert raw.static_headers == {"Authorization": "Bearer sk-secret-header"} - assert raw.env == {"UPSTREAM_TOKEN": "sk-secret-env"} - assert raw.credentials is None - assert raw.env_vars is not None - assert len(raw.env_vars) == 1 - # ``model_construct`` in ``_leaky_list_server`` skips validation, so - # env_vars stays as raw dicts; mirror the fixture shape here. - entry = raw.env_vars[0] - name = entry["name"] if isinstance(entry, dict) else entry.name - value = entry["value"] if isinstance(entry, dict) else entry.value - assert name == "GLOBAL_KEY" - assert value == "super-secret" + returned: Final = result.items[0] + assert returned.server_id == item.server_id + assert returned.credentials == (None if allowed_routes else {"scopes": credentials["scopes"]}) + assert returned.url == (None if allowed_routes else item.url) + assert returned.static_headers == (None if allowed_routes else item.static_headers) + assert returned.env == ({} if allowed_routes else item.env) + assert returned.env_vars == (None if allowed_routes else item.env_vars) + assert item.model_dump() == original @pytest.mark.asyncio async def test_approve_non_pending_server_raises_400(self): From bbbd03089de367e256eb90dada93ba2617048d92 Mon Sep 17 00:00:00 2001 From: Yucheng He Date: Fri, 18 Sep 2026 12:59:16 -0700 Subject: [PATCH 06/11] fix(mcp): retain viewer submission sanitization precedence --- .../mcp_management_endpoints.py | 6 ++--- .../test_mcp_management_endpoints.py | 24 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index df4d22fc1bf..6ed131417d6 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -1486,10 +1486,10 @@ if MCP_AVAILABLE: submissions: Final = await get_mcp_submissions(prisma_client) submissions.items = _redact_mcp_credentials_list(submissions.items) - if _is_restricted_virtual_key_request(user_api_key_dict): - submissions.items = _sanitize_mcp_server_list_for_virtual_key(submissions.items) - elif not _user_is_full_admin(user_api_key_dict): + if not _user_is_full_admin(user_api_key_dict): submissions.items = _sanitize_mcp_server_list_for_non_admin(submissions.items) + elif _is_restricted_virtual_key_request(user_api_key_dict): + submissions.items = _sanitize_mcp_server_list_for_virtual_key(submissions.items) return submissions @router.put( diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index 448b3334b18..79b34ded13e 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -4583,20 +4583,17 @@ class TestMCPApprovalWorkflow: assert result.pending_review == 1 @pytest.mark.asyncio - async def test_get_submissions_sanitizes_for_view_only_admin(self): - """PROXY_ADMIN_VIEW_ONLY reviewing the submission queue must go through - the non-admin sanitizer that fetch/list endpoints use: url, - static_headers, env, env_vars, and credentials are all dropped. A - mutation swapping the gate back to the old partial-blank pattern (which - left url/static_headers/env and env-var names intact) would fail this.""" + @pytest.mark.parametrize("allowed_routes", [[], ["llm_api_routes"], ["mcp_routes"]]) + async def test_get_submissions_sanitizes_for_view_only_admin(self, allowed_routes: list[str]) -> None: from litellm.proxy._types import MCPSubmissionsSummary from litellm.proxy.management_endpoints.mcp_management_endpoints import ( get_mcp_server_submissions, ) - item = _leaky_list_server() - item.approval_status = "pending_review" - summary = MCPSubmissionsSummary(total=1, pending_review=1, active=0, rejected=0, items=[item]) + item: Final = _leaky_list_server().model_copy( + update={"approval_status": "pending_review", "spec_path": "https://example.com/spec?key=secret"} + ) + summary: Final = MCPSubmissionsSummary(total=1, pending_review=1, active=0, rejected=0, items=[item]) with ( patch( @@ -4608,12 +4605,15 @@ class TestMCPApprovalWorkflow: AsyncMock(return_value=summary), ), ): - result = await get_mcp_server_submissions( - user_api_key_dict=generate_mock_user_api_key_auth(user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY), + result: Final = await get_mcp_server_submissions( + user_api_key_dict=UserAPIKeyAuth( + user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY, allowed_routes=allowed_routes + ), ) assert len(result.items) == 1 - sanitized = result.items[0] + sanitized: Final = result.items[0] + assert sanitized.spec_path is None assert sanitized.url is None assert sanitized.static_headers is None assert sanitized.env == {} From ca287c1b1590194546a01f6eb50be7256cd54e39 Mon Sep 17 00:00:00 2001 From: Yucheng He Date: Fri, 18 Sep 2026 14:55:31 -0700 Subject: [PATCH 07/11] fix(mcp): preserve restricted admin submission fields --- .../mcp_management_endpoints.py | 2 - .../test_mcp_management_endpoints.py | 72 ++++++++++--------- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 6ed131417d6..c1388e8bb81 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -1488,8 +1488,6 @@ if MCP_AVAILABLE: submissions.items = _redact_mcp_credentials_list(submissions.items) if not _user_is_full_admin(user_api_key_dict): submissions.items = _sanitize_mcp_server_list_for_non_admin(submissions.items) - elif _is_restricted_virtual_key_request(user_api_key_dict): - submissions.items = _sanitize_mcp_server_list_for_virtual_key(submissions.items) return submissions @router.put( diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index 79b34ded13e..f3fc45480e1 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -6,7 +6,7 @@ import logging from contextlib import ExitStack from datetime import datetime, timedelta from types import SimpleNamespace -from typing import Final, List, Optional, cast +from typing import List, Optional, cast from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -4583,17 +4583,20 @@ class TestMCPApprovalWorkflow: assert result.pending_review == 1 @pytest.mark.asyncio - @pytest.mark.parametrize("allowed_routes", [[], ["llm_api_routes"], ["mcp_routes"]]) - async def test_get_submissions_sanitizes_for_view_only_admin(self, allowed_routes: list[str]) -> None: + async def test_get_submissions_sanitizes_for_view_only_admin(self): + """PROXY_ADMIN_VIEW_ONLY reviewing the submission queue must go through + the non-admin sanitizer that fetch/list endpoints use: url, + static_headers, env, env_vars, and credentials are all dropped. A + mutation swapping the gate back to the old partial-blank pattern (which + left url/static_headers/env and env-var names intact) would fail this.""" from litellm.proxy._types import MCPSubmissionsSummary from litellm.proxy.management_endpoints.mcp_management_endpoints import ( get_mcp_server_submissions, ) - item: Final = _leaky_list_server().model_copy( - update={"approval_status": "pending_review", "spec_path": "https://example.com/spec?key=secret"} - ) - summary: Final = MCPSubmissionsSummary(total=1, pending_review=1, active=0, rejected=0, items=[item]) + item = _leaky_list_server() + item.approval_status = "pending_review" + summary = MCPSubmissionsSummary(total=1, pending_review=1, active=0, rejected=0, items=[item]) with ( patch( @@ -4605,15 +4608,12 @@ class TestMCPApprovalWorkflow: AsyncMock(return_value=summary), ), ): - result: Final = await get_mcp_server_submissions( - user_api_key_dict=UserAPIKeyAuth( - user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY, allowed_routes=allowed_routes - ), + result = await get_mcp_server_submissions( + user_api_key_dict=generate_mock_user_api_key_auth(user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY), ) assert len(result.items) == 1 - sanitized: Final = result.items[0] - assert sanitized.spec_path is None + sanitized = result.items[0] assert sanitized.url is None assert sanitized.static_headers is None assert sanitized.env == {} @@ -4625,17 +4625,18 @@ class TestMCPApprovalWorkflow: assert item.static_headers == {"Authorization": "Bearer sk-secret-header"} @pytest.mark.asyncio - @pytest.mark.parametrize("allowed_routes", [[], ["llm_api_routes"], ["mcp_routes"]]) - async def test_get_submissions_respects_admin_key_route_restrictions(self, allowed_routes: list[str]) -> None: + async def test_get_submissions_full_admin_still_sees_secrets(self): + """The view-only redaction must not over-redact for a full PROXY_ADMIN, + who needs url/static_headers/env/env_vars to review the pending + submission. Only the explicit credentials field is cleared.""" from litellm.proxy._types import MCPSubmissionsSummary - - credentials: Final[MCPCredentials] = {"scopes": ["scope:review"], "client_secret": "secret-sentinel"} - item: Final = _leaky_list_server().model_copy( - update={"approval_status": "pending_review", "credentials": credentials} + from litellm.proxy.management_endpoints.mcp_management_endpoints import ( + get_mcp_server_submissions, ) - original: Final = item.model_dump() - summary: Final = MCPSubmissionsSummary(total=1, pending_review=1, active=0, rejected=0, items=[item]) - admin: Final = UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN, allowed_routes=allowed_routes) + + item = _leaky_list_server() + item.approval_status = "pending_review" + summary = MCPSubmissionsSummary(total=1, pending_review=1, active=0, rejected=0, items=[item]) with ( patch( @@ -4647,18 +4648,25 @@ class TestMCPApprovalWorkflow: AsyncMock(return_value=summary), ), ): - result: Final = await mgmt_endpoints.get_mcp_server_submissions(user_api_key_dict=admin) + result = await get_mcp_server_submissions( + user_api_key_dict=generate_mock_user_api_key_auth(user_role=LitellmUserRoles.PROXY_ADMIN), + ) - assert (result.total, result.pending_review, result.active, result.rejected) == (1, 1, 0, 0) assert len(result.items) == 1 - returned: Final = result.items[0] - assert returned.server_id == item.server_id - assert returned.credentials == (None if allowed_routes else {"scopes": credentials["scopes"]}) - assert returned.url == (None if allowed_routes else item.url) - assert returned.static_headers == (None if allowed_routes else item.static_headers) - assert returned.env == ({} if allowed_routes else item.env) - assert returned.env_vars == (None if allowed_routes else item.env_vars) - assert item.model_dump() == original + raw = result.items[0] + assert raw.url == "https://leaky.example.com/mcp?api_key=sk-embedded-in-url" + assert raw.static_headers == {"Authorization": "Bearer sk-secret-header"} + assert raw.env == {"UPSTREAM_TOKEN": "sk-secret-env"} + assert raw.credentials is None + assert raw.env_vars is not None + assert len(raw.env_vars) == 1 + # ``model_construct`` in ``_leaky_list_server`` skips validation, so + # env_vars stays as raw dicts; mirror the fixture shape here. + entry = raw.env_vars[0] + name = entry["name"] if isinstance(entry, dict) else entry.name + value = entry["value"] if isinstance(entry, dict) else entry.value + assert name == "GLOBAL_KEY" + assert value == "super-secret" @pytest.mark.asyncio async def test_approve_non_pending_server_raises_400(self): From 362d99e001be14c65b5a777b0cfcd01df721de0f Mon Sep 17 00:00:00 2001 From: Yucheng He Date: Fri, 18 Sep 2026 17:23:31 -0700 Subject: [PATCH 08/11] fix(mcp): keep discovered scopes out of saved settings --- .../mcp_server/mcp_server_manager.py | 25 ++- .../types/mcp_server/mcp_server_manager.py | 1 + .../mcp_server/test_mcp_server_manager.py | 203 ++++++++++++++++++ .../test_mcp_management_endpoints.py | 27 +-- 4 files changed, 240 insertions(+), 16 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 08151d8cab2..2ef6253ef50 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -2500,9 +2500,8 @@ class MCPServerManager: # Filter blank scopes (e.g. YAML ``scopes: [""]``) the same way the DB-build path does, so # an all-blank list normalizes to None rather than a ``("",)`` tuple that skips the # entra_obo fail-closed scope precondition and POSTs an empty scope to the IdP. - resolved_scopes = self._extract_scopes(server_config.get("scopes")) or ( - gated_oauth_metadata.scopes if gated_oauth_metadata else None - ) + configured_scopes = self._extract_scopes(server_config.get("scopes")) + resolved_scopes = configured_scopes or (gated_oauth_metadata.scopes if gated_oauth_metadata else None) resolved_authorization_url = manual_authorization_url or ( gated_oauth_metadata.authorization_url if gated_oauth_metadata else None ) @@ -2579,6 +2578,7 @@ class MCPServerManager: client_secret=server_config.get("client_secret", None), oauth2_flow=self._explicit_oauth2_flow(config_oauth2_flow), scopes=resolved_scopes, + configured_scopes=tuple(configured_scopes) if configured_scopes else None, issuer=effective_issuer, issuer_is_anchored=use_issuer_anchor, authorization_url=resolved_authorization_url, @@ -3041,6 +3041,18 @@ class MCPServerManager: if scopes_value is not None: scopes = self._extract_scopes(scopes_value) + stored_scopes: Final[object] = credentials_dict.get("scopes") if credentials_dict else None + scopes_as_objects: Final = ( + cast(Sequence[object], stored_scopes) # cast-ok: list shape validated below + if isinstance(stored_scopes, list) + else () + ) + configured_scopes: Final = ( + tuple(scope for scope in scopes_as_objects if isinstance(scope, str)) + if scopes_as_objects and all(isinstance(scope, str) and scope for scope in scopes_as_objects) + else None + ) + name_for_prefix: Final = mcp_server.alias or mcp_server.server_name or mcp_server.server_id mcp_info: Final[MCPInfo] = _mcp_info.copy() @@ -3115,6 +3127,7 @@ class MCPServerManager: client_secret=client_secret_value or getattr(mcp_server, "client_secret", None), oauth2_flow=self._explicit_oauth2_flow(getattr(mcp_server, "oauth2_flow", None)), scopes=resolved_scopes, + configured_scopes=configured_scopes, issuer=effective_issuer, issuer_is_anchored=use_issuer_anchor, authorization_url=manual_authorization_url or getattr(gated_oauth_metadata, "authorization_url", None), @@ -7088,7 +7101,11 @@ class MCPServerManager: spec_path=server.spec_path, transport=server.transport, auth_type=server.auth_type, - credentials={"scopes": server.scopes} if server.scopes else None, + credentials=( + {"scopes": list(server.configured_scopes)} # mutable-ok: MCPCredentials requires a JSON-array list + if server.configured_scopes + else None + ), created_at=server.created_at, updated_at=server.updated_at, teams=[], diff --git a/litellm/types/mcp_server/mcp_server_manager.py b/litellm/types/mcp_server/mcp_server_manager.py index 985d31af997..cb32299b143 100644 --- a/litellm/types/mcp_server/mcp_server_manager.py +++ b/litellm/types/mcp_server/mcp_server_manager.py @@ -99,6 +99,7 @@ class MCPServer(BaseModel): configured_authorization_url: str | None = None configured_token_url: str | None = None configured_registration_url: str | None = None + configured_scopes: tuple[str, ...] | None = None # How the gateway authenticates to the upstream token endpoint. When # "client_secret_basic" the credentials go in an HTTP Basic Authorization # header (omitted from the body); None defaults to "client_secret_post". diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py index c0aa6c9cd32..6b51cc342a5 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py @@ -61,6 +61,8 @@ from litellm.types.llms.custom_http import httpxSpecialProvider from litellm.types.mcp import MCPAuth, MCPAuthType from litellm.types.mcp_server.mcp_server_manager import MCPOAuthMetadata, MCPServer from litellm.caching.caching import DualCache +from litellm.caching.llm_caching_handler import LLMClientCache +from litellm.llms.custom_httpx.http_handler import AsyncHTTPHandler import litellm from litellm.integrations.custom_guardrail import CustomGuardrail from litellm.proxy.utils import ProxyLogging @@ -10427,6 +10429,7 @@ def test_build_mcp_server_table_carries_oauth2_flow(): client_id="client-123", client_secret="secret-xyz", scopes=["scope:a", "scope:b"], + configured_scopes=("scope:a", "scope:b"), ) table = manager._build_mcp_server_table(server) @@ -10456,6 +10459,206 @@ def test_build_mcp_server_table_carries_null_oauth2_flow(): assert table.oauth2_flow is None +async def _mock_oauth_discovery( + respx_mock: MockRouter, + monkeypatch: pytest.MonkeyPatch, + *, + server_url: str, + scopes: list[str], +) -> None: + resource_metadata_url: Final[str] = "https://up.example.com/.well-known/oauth-protected-resource" + authorization_server_url: Final[str] = "https://up.example.com" + authorization_metadata_url: Final[str] = f"{authorization_server_url}/.well-known/oauth-authorization-server" + respx_mock.get(server_url).respond( + status_code=401, + headers={"WWW-Authenticate": f'Bearer resource_metadata="{resource_metadata_url}"'}, + ) + respx_mock.get(resource_metadata_url).respond( + json={"authorization_servers": [authorization_server_url], "scopes_supported": scopes} + ) + respx_mock.get(authorization_metadata_url).respond( + json={ + "issuer": authorization_server_url, + "authorization_endpoint": f"{authorization_server_url}/authorize", + "token_endpoint": f"{authorization_server_url}/token", + } + ) + clients: Final[LLMClientCache] = LLMClientCache() + monkeypatch.setattr(litellm, "in_memory_llm_clients_cache", clients) + http_handler: Final[AsyncHTTPHandler] = AsyncHTTPHandler() + await http_handler.client.aclose() + http_handler.client = httpx.AsyncClient(transport=httpx.MockTransport(respx_mock.async_handler)) + http_handler._owns_client = True + cache_key: Final[str] = f"async_httpx_clienttimeout_{MCP_METADATA_TIMEOUT}{httpxSpecialProvider.MCP.value}" + clients.set_cache(cache_key, http_handler) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("discovery_on_startup", [True, False]) +async def test_management_view_serves_configured_scopes_not_discovered_ones_from_db( + discovery_on_startup: bool, + respx_mock: MockRouter, + monkeypatch: pytest.MonkeyPatch, +) -> None: + row: Final[LiteLLM_MCPServerTable] = LiteLLM_MCPServerTable( + server_id="discovered-scopes-db", + alias="discovered_scopes_db", + url="https://up.example.com/mcp", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + oauth2_flow="authorization_code", + created_at=datetime.now(), + updated_at=datetime.now(), + ) + await _mock_oauth_discovery(respx_mock, monkeypatch, server_url=row.url or "", scopes=["discovered.read"]) + env: Final[dict[str, str]] = {"LITELLM_MCP_OAUTH_DISCOVERY_ON_STARTUP": "1"} if discovery_on_startup else {} + with patch.dict(os.environ, env, clear=True): + manager: Final[MCPServerManager] = MCPServerManager() + built: Final[MCPServer] = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False) + manager.registry[built.server_id] = built + resolved: Final[MCPServer] = await manager.ensure_oauth_metadata_discovered(built) + + assert resolved.scopes == ["discovered.read"] + view: Final[LiteLLM_MCPServerTable] = manager._build_mcp_server_table(resolved) + assert view.credentials is None + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("stored_scopes", "runtime_scopes"), + [ + (None, ["openid"]), + ([], ["openid"]), + ([""], ["openid"]), + (["read", ""], ["read"]), + (["read", 7], ["read"]), + ("read", ["read"]), + ], +) +async def test_management_view_omits_invalid_or_absent_db_scopes( + stored_scopes: list[str | int] | str | None, + runtime_scopes: list[str], + respx_mock: MockRouter, + monkeypatch: pytest.MonkeyPatch, +) -> None: + row: Final[LiteLLM_MCPServerTable] = LiteLLM_MCPServerTable.model_construct( + server_id="empty-scopes-db", + alias="empty_scopes_db", + url="https://up.example.com/mcp", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + oauth2_flow="authorization_code", + credentials=json.dumps({"scopes": stored_scopes}), + created_at=datetime.now(), + updated_at=datetime.now(), + ) + await _mock_oauth_discovery(respx_mock, monkeypatch, server_url=row.url or "", scopes=["openid"]) + env: Final[dict[str, str]] = {"LITELLM_MCP_OAUTH_DISCOVERY_ON_STARTUP": "1"} + with patch.dict(os.environ, env, clear=True): + manager: Final[MCPServerManager] = MCPServerManager() + built: Final[MCPServer] = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False) + + assert built.scopes == runtime_scopes + view: Final[LiteLLM_MCPServerTable] = manager._build_mcp_server_table(built) + assert view.credentials is None + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("stored_scopes", "runtime_scopes"), + [ + (["calendar.read"], ["calendar.read"]), + ([" "], ["discovered.read"]), + (["read", " "], ["read"]), + (["read", "read"], ["read", "read"]), + ], +) +async def test_management_view_serves_explicitly_configured_scopes_from_db( + stored_scopes: list[str], + runtime_scopes: list[str], + respx_mock: MockRouter, + monkeypatch: pytest.MonkeyPatch, +) -> None: + row: Final[LiteLLM_MCPServerTable] = LiteLLM_MCPServerTable( + server_id="configured-scopes-db", + alias="configured_scopes_db", + url="https://up.example.com/mcp", + transport=MCPTransport.http, + auth_type=MCPAuth.oauth2, + oauth2_flow="authorization_code", + credentials={"scopes": stored_scopes}, + created_at=datetime.now(), + updated_at=datetime.now(), + ) + await _mock_oauth_discovery(respx_mock, monkeypatch, server_url=row.url or "", scopes=["discovered.read"]) + env: Final[dict[str, str]] = {"LITELLM_MCP_OAUTH_DISCOVERY_ON_STARTUP": "1"} + with patch.dict(os.environ, env, clear=True): + manager: Final[MCPServerManager] = MCPServerManager() + built: Final[MCPServer] = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False) + + assert built.scopes == runtime_scopes + view: Final[LiteLLM_MCPServerTable] = manager._build_mcp_server_table(built) + assert view.credentials == {"scopes": stored_scopes} + + +@pytest.mark.asyncio +@pytest.mark.parametrize("configured_scopes", [None, ["calendar.read"]]) +async def test_management_view_scopes_follow_yaml_config_not_discovery( + configured_scopes: list[str] | None, + respx_mock: MockRouter, + monkeypatch: pytest.MonkeyPatch, +) -> None: + config: Final[dict[str, dict[str, object]]] = { + "yamlscopes": { + "url": "https://up.example.com/mcp", + "transport": MCPTransport.http, + "auth_type": MCPAuth.oauth2, + "oauth2_flow": "authorization_code", + "client_id": "cid", + "client_secret": "csec", + **({"scopes": configured_scopes} if configured_scopes else {}), + } + } + await _mock_oauth_discovery(respx_mock, monkeypatch, server_url="https://up.example.com/mcp", scopes=["discovered.read"]) + env: Final[dict[str, str]] = {"LITELLM_MCP_OAUTH_DISCOVERY_ON_STARTUP": "1"} + with patch.dict(os.environ, env, clear=True): + manager: Final[MCPServerManager] = MCPServerManager() + await manager.load_servers_from_config(config) + + server: Final[MCPServer] = next(iter(manager.config_mcp_servers.values())) + expected_runtime: Final[list[str]] = configured_scopes or ["discovered.read"] + assert server.scopes == expected_runtime + view: Final[LiteLLM_MCPServerTable] = manager._build_mcp_server_table(server) + assert view.credentials == ({"scopes": configured_scopes} if configured_scopes else None) + + +@pytest.mark.asyncio +async def test_lazy_yaml_discovery_keeps_configured_scopes_out_of_the_management_view( + respx_mock: MockRouter, + monkeypatch: pytest.MonkeyPatch, +) -> None: + config: Final[dict[str, dict[str, object]]] = { + "lazyyamlscopes": { + "url": "https://up.example.com/mcp", + "transport": MCPTransport.http, + "auth_type": MCPAuth.oauth2, + "oauth2_flow": "authorization_code", + "client_id": "cid", + "client_secret": "csec", + } + } + await _mock_oauth_discovery(respx_mock, monkeypatch, server_url="https://up.example.com/mcp", scopes=["discovered.read"]) + with patch.dict(os.environ, {}, clear=True): + manager: Final[MCPServerManager] = MCPServerManager() + await manager.load_servers_from_config(config) + server: Final[MCPServer] = next(iter(manager.config_mcp_servers.values())) + resolved: Final[MCPServer] = await manager.ensure_oauth_metadata_discovered(server) + + assert resolved.scopes == ["discovered.read"] + view: Final[LiteLLM_MCPServerTable] = manager._build_mcp_server_table(resolved) + assert view.credentials is None + + @pytest.mark.asyncio async def test_resolve_toolset_tool_permissions_single_db_fetch_across_checks(): """The server-level and tool-level permission primitives each resolve the diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index f3fc45480e1..3d2487ec6ca 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -4582,13 +4582,9 @@ class TestMCPApprovalWorkflow: assert result.total == 1 assert result.pending_review == 1 + @pytest.mark.parametrize("allowed_routes", [None, [], ["llm_api_routes"], ["mcp_routes"]]) @pytest.mark.asyncio - async def test_get_submissions_sanitizes_for_view_only_admin(self): - """PROXY_ADMIN_VIEW_ONLY reviewing the submission queue must go through - the non-admin sanitizer that fetch/list endpoints use: url, - static_headers, env, env_vars, and credentials are all dropped. A - mutation swapping the gate back to the old partial-blank pattern (which - left url/static_headers/env and env-var names intact) would fail this.""" + async def test_get_submissions_sanitizes_for_view_only_admin(self, allowed_routes: list[str] | None): from litellm.proxy._types import MCPSubmissionsSummary from litellm.proxy.management_endpoints.mcp_management_endpoints import ( get_mcp_server_submissions, @@ -4596,6 +4592,7 @@ class TestMCPApprovalWorkflow: item = _leaky_list_server() item.approval_status = "pending_review" + item.spec_path = "https://example.com/spec.json?key=private" summary = MCPSubmissionsSummary(total=1, pending_review=1, active=0, rejected=0, items=[item]) with ( @@ -4609,11 +4606,15 @@ class TestMCPApprovalWorkflow: ), ): result = await get_mcp_server_submissions( - user_api_key_dict=generate_mock_user_api_key_auth(user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY), + user_api_key_dict=UserAPIKeyAuth( + user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY, allowed_routes=allowed_routes + ), ) + assert (result.total, result.pending_review, result.active, result.rejected) == (1, 1, 0, 0) assert len(result.items) == 1 sanitized = result.items[0] + assert sanitized.spec_path is None assert sanitized.url is None assert sanitized.static_headers is None assert sanitized.env == {} @@ -4624,11 +4625,9 @@ class TestMCPApprovalWorkflow: assert item.url == "https://leaky.example.com/mcp?api_key=sk-embedded-in-url" assert item.static_headers == {"Authorization": "Bearer sk-secret-header"} + @pytest.mark.parametrize("allowed_routes", [None, [], ["llm_api_routes"], ["mcp_routes"]]) @pytest.mark.asyncio - async def test_get_submissions_full_admin_still_sees_secrets(self): - """The view-only redaction must not over-redact for a full PROXY_ADMIN, - who needs url/static_headers/env/env_vars to review the pending - submission. Only the explicit credentials field is cleared.""" + async def test_get_submissions_full_admin_preserves_review_fields(self, allowed_routes: list[str] | None): from litellm.proxy._types import MCPSubmissionsSummary from litellm.proxy.management_endpoints.mcp_management_endpoints import ( get_mcp_server_submissions, @@ -4636,6 +4635,7 @@ class TestMCPApprovalWorkflow: item = _leaky_list_server() item.approval_status = "pending_review" + item.spec_path = "https://example.com/spec.json?key=private" summary = MCPSubmissionsSummary(total=1, pending_review=1, active=0, rejected=0, items=[item]) with ( @@ -4649,11 +4649,14 @@ class TestMCPApprovalWorkflow: ), ): result = await get_mcp_server_submissions( - user_api_key_dict=generate_mock_user_api_key_auth(user_role=LitellmUserRoles.PROXY_ADMIN), + user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN, allowed_routes=allowed_routes), ) + assert (result.total, result.pending_review, result.active, result.rejected) == (1, 1, 0, 0) assert len(result.items) == 1 raw = result.items[0] + assert raw.spec_path == item.spec_path + assert raw.approval_status == "pending_review" assert raw.url == "https://leaky.example.com/mcp?api_key=sk-embedded-in-url" assert raw.static_headers == {"Authorization": "Bearer sk-secret-header"} assert raw.env == {"UPSTREAM_TOKEN": "sk-secret-env"} From e768f25983e92bb995bc97a3cdef4140c8b51269 Mon Sep 17 00:00:00 2001 From: Yucheng He Date: Sat, 19 Sep 2026 14:24:05 -0700 Subject: [PATCH 09/11] test(mcp): cover lazy discovery and empty configured scopes --- .../mcp_server/test_mcp_server_manager.py | 46 +++++++++++++------ .../test_mcp_management_endpoints.py | 8 ++++ 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py index 6b51cc342a5..4e306023c2c 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py @@ -10564,6 +10564,7 @@ async def test_management_view_omits_invalid_or_absent_db_scopes( @pytest.mark.asyncio +@pytest.mark.parametrize("discovery_on_startup", [True, False]) @pytest.mark.parametrize( ("stored_scopes", "runtime_scopes"), [ @@ -10576,6 +10577,7 @@ async def test_management_view_omits_invalid_or_absent_db_scopes( async def test_management_view_serves_explicitly_configured_scopes_from_db( stored_scopes: list[str], runtime_scopes: list[str], + discovery_on_startup: bool, respx_mock: MockRouter, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -10591,20 +10593,34 @@ async def test_management_view_serves_explicitly_configured_scopes_from_db( updated_at=datetime.now(), ) await _mock_oauth_discovery(respx_mock, monkeypatch, server_url=row.url or "", scopes=["discovered.read"]) - env: Final[dict[str, str]] = {"LITELLM_MCP_OAUTH_DISCOVERY_ON_STARTUP": "1"} + env: Final[dict[str, str]] = {"LITELLM_MCP_OAUTH_DISCOVERY_ON_STARTUP": "1"} if discovery_on_startup else {} with patch.dict(os.environ, env, clear=True): manager: Final[MCPServerManager] = MCPServerManager() built: Final[MCPServer] = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False) + manager.registry[built.server_id] = built + resolved: Final[MCPServer] = await manager.ensure_oauth_metadata_discovered(built) - assert built.scopes == runtime_scopes - view: Final[LiteLLM_MCPServerTable] = manager._build_mcp_server_table(built) + assert resolved.scopes == runtime_scopes + view: Final[LiteLLM_MCPServerTable] = manager._build_mcp_server_table(resolved) assert view.credentials == {"scopes": stored_scopes} @pytest.mark.asyncio -@pytest.mark.parametrize("configured_scopes", [None, ["calendar.read"]]) +@pytest.mark.parametrize("discovery_on_startup", [True, False]) +@pytest.mark.parametrize( + ("configured_scopes", "expected_view_scopes"), + [ + (None, None), + (["calendar.read"], ["calendar.read"]), + ([" "], None), + ([""], None), + (["calendar.read", " "], ["calendar.read"]), + ], +) async def test_management_view_scopes_follow_yaml_config_not_discovery( configured_scopes: list[str] | None, + expected_view_scopes: list[str] | None, + discovery_on_startup: bool, respx_mock: MockRouter, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -10616,20 +10632,22 @@ async def test_management_view_scopes_follow_yaml_config_not_discovery( "oauth2_flow": "authorization_code", "client_id": "cid", "client_secret": "csec", - **({"scopes": configured_scopes} if configured_scopes else {}), + **({"scopes": configured_scopes} if configured_scopes is not None else {}), } } - await _mock_oauth_discovery(respx_mock, monkeypatch, server_url="https://up.example.com/mcp", scopes=["discovered.read"]) - env: Final[dict[str, str]] = {"LITELLM_MCP_OAUTH_DISCOVERY_ON_STARTUP": "1"} + await _mock_oauth_discovery( + respx_mock, monkeypatch, server_url="https://up.example.com/mcp", scopes=["discovered.read"] + ) + env: Final[dict[str, str]] = {"LITELLM_MCP_OAUTH_DISCOVERY_ON_STARTUP": "1"} if discovery_on_startup else {} with patch.dict(os.environ, env, clear=True): manager: Final[MCPServerManager] = MCPServerManager() await manager.load_servers_from_config(config) + server: Final[MCPServer] = next(iter(manager.config_mcp_servers.values())) + resolved: Final[MCPServer] = await manager.ensure_oauth_metadata_discovered(server) - server: Final[MCPServer] = next(iter(manager.config_mcp_servers.values())) - expected_runtime: Final[list[str]] = configured_scopes or ["discovered.read"] - assert server.scopes == expected_runtime - view: Final[LiteLLM_MCPServerTable] = manager._build_mcp_server_table(server) - assert view.credentials == ({"scopes": configured_scopes} if configured_scopes else None) + assert resolved.scopes == (expected_view_scopes or ["discovered.read"]) + view: Final[LiteLLM_MCPServerTable] = manager._build_mcp_server_table(resolved) + assert view.credentials == ({"scopes": expected_view_scopes} if expected_view_scopes else None) @pytest.mark.asyncio @@ -10647,7 +10665,9 @@ async def test_lazy_yaml_discovery_keeps_configured_scopes_out_of_the_management "client_secret": "csec", } } - await _mock_oauth_discovery(respx_mock, monkeypatch, server_url="https://up.example.com/mcp", scopes=["discovered.read"]) + await _mock_oauth_discovery( + respx_mock, monkeypatch, server_url="https://up.example.com/mcp", scopes=["discovered.read"] + ) with patch.dict(os.environ, {}, clear=True): manager: Final[MCPServerManager] = MCPServerManager() await manager.load_servers_from_config(config) diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index 3d2487ec6ca..80773f314d8 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -881,6 +881,14 @@ class TestListMCPServers: '"upstream_token_header": "esb-oauth"}', {"scopes": ["read", "write"], "upstream_token_header": "esb-oauth"}, ), + ( + {"client_id": "cid", "client_secret": "csecret", "scopes": []}, + None, + ), + ( + '{"client_id": "cid", "client_secret": "csecret", "scopes": []}', + None, + ), ( {"client_id": "cid", "client_secret": "csecret", "scopes": ["read", ""]}, None, From 1f1b61173d79ae86b2fdafd48111030bea7871f0 Mon Sep 17 00:00:00 2001 From: yucheng Date: Mon, 21 Sep 2026 18:10:41 +0000 Subject: [PATCH 10/11] fix(otel v2): map OCR page markdown onto the generation output Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- litellm/integrations/otel/model/payloads.py | 19 ++++++++- .../otel/test_otel_v2_sources_of_truth.py | 39 +++++++++++++++++++ .../otel/test_otel_v2_vendor_mappers.py | 22 +++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/litellm/integrations/otel/model/payloads.py b/litellm/integrations/otel/model/payloads.py index c23b3291365..3164e0977b7 100644 --- a/litellm/integrations/otel/model/payloads.py +++ b/litellm/integrations/otel/model/payloads.py @@ -427,7 +427,7 @@ class LLMCallSpanData: # plain ``.get`` — no repeated ``isinstance`` guards. raw_response: Final = payload.get("response") response: Final = cast(Mapping[str, object], raw_response if isinstance(raw_response, dict) else {}) - choices_out: Final = _dicts(response.get("choices")) or _responses_choices(response) + choices_out: Final = _dicts(response.get("choices")) or _responses_choices(response) or _ocr_choices(response) # ``finish_reasons`` is metadata, not content, so derive it from # ``choices_out`` before gating. The raw message/choice bodies are only # retained when content capture is enabled (see ``capture_span_content``); @@ -752,6 +752,23 @@ def _responses_choices(response: Mapping[str, object]) -> tuple[_Choice, ...]: return (choice,) +def _ocr_choices(response: Mapping[str, object]) -> tuple[_Choice, ...]: + """An ``OCRResponse`` ``pages`` list folded into one chat-shaped assistant choice.""" + markdowns: Final = tuple( + text for page in _dicts(response.get("pages")) if (text := as_str(page.get("markdown"))) is not None + ) + if not markdowns: + return () + message: Final[_AssistantMessage] = { + "role": "assistant", + "content": "\n\n".join(markdowns), + "refusal": None, + "tool_calls": None, + } + choice: Final[_Choice] = {"message": message, "finish_reason": None} + return (choice,) + + def _responses_parts_text(parts: tuple[Mapping[str, object], ...], part_type: str, field: str) -> str | None: texts: Final = tuple( text for part in parts if part.get("type") == part_type if (text := as_str(part.get(field))) is not None diff --git a/tests/test_litellm/integrations/otel/test_otel_v2_sources_of_truth.py b/tests/test_litellm/integrations/otel/test_otel_v2_sources_of_truth.py index 17de3cf1e8a..01f2a13d252 100644 --- a/tests/test_litellm/integrations/otel/test_otel_v2_sources_of_truth.py +++ b/tests/test_litellm/integrations/otel/test_otel_v2_sources_of_truth.py @@ -872,6 +872,45 @@ def test_chat_choices_win_over_a_responses_output_list(): assert data.finish_reasons == ("stop",) +def _ocr_payload(pages: list[object]): + return _sample_payload( + call_type="aocr", + custom_llm_provider="mistral", + model="mistral-ocr-latest", + messages=None, + response={"object": "ocr", "model": "mistral-ocr-latest", "pages": pages, "usage_info": {"pages_processed": 2}}, + ) + + +def test_ocr_pages_become_one_assistant_choice_joined_in_page_order(): + data = LLMCallSpanData.from_standard_logging_payload( + _ocr_payload([{"index": 0, "markdown": "# Invoice"}, {"index": 1, "markdown": "Total: 42"}]), + capture_content=True, + ) + + assert data.choices_out == ( + { + "message": {"role": "assistant", "content": "# Invoice\n\nTotal: 42", "refusal": None, "tool_calls": None}, + "finish_reason": None, + }, + ) + assert data.finish_reasons == () + + +def test_ocr_output_follows_the_content_capture_gate(): + data = LLMCallSpanData.from_standard_logging_payload(_ocr_payload([{"index": 0, "markdown": "# Invoice"}])) + + assert data.choices_out == () + + +def test_ocr_pages_without_markdown_stay_empty(): + data = LLMCallSpanData.from_standard_logging_payload( + _ocr_payload([{"index": 0, "images": []}, "not-a-page"]), capture_content=True + ) + + assert data.choices_out == () + + def test_request_identity_prefers_canonical_team_keys(): from litellm.integrations.otel.model.payloads import RequestIdentity diff --git a/tests/test_litellm/integrations/otel/test_otel_v2_vendor_mappers.py b/tests/test_litellm/integrations/otel/test_otel_v2_vendor_mappers.py index 4e375de0494..9fa198c4ec5 100644 --- a/tests/test_litellm/integrations/otel/test_otel_v2_vendor_mappers.py +++ b/tests/test_litellm/integrations/otel/test_otel_v2_vendor_mappers.py @@ -227,6 +227,28 @@ def test_langfuse_mapper_renders_a_responses_api_call_from_the_standard_logging_ assert attrs["langfuse.observation.type"] == "generation" +def test_langfuse_mapper_renders_an_ocr_call_with_the_page_markdown_as_output(): + payload = { + "call_type": "aocr", + "custom_llm_provider": "mistral", + "model": "mistral-ocr-latest", + "messages": None, + "response": { + "object": "ocr", + "model": "mistral-ocr-latest", + "pages": [{"index": 0, "markdown": "# Invoice"}, {"index": 1, "markdown": "Total: 42"}], + "usage_info": {"pages_processed": 2}, + }, + } + data = LLMCallSpanData.from_standard_logging_payload(payload, capture_content=True) + attrs = LangfuseMapper().map(data) + + assert json.loads(attrs["langfuse.observation.output"]) == [ + {"role": "assistant", "content": "# Invoice\n\nTotal: 42", "refusal": None, "tool_calls": None} + ] + assert attrs["langfuse.observation.type"] == "generation" + + # --------------------------------------------------------------------------- # # Weave # --------------------------------------------------------------------------- # From 5a63c932a4fa36dbd29899411dea89925c5ea0bd Mon Sep 17 00:00:00 2001 From: yucheng Date: Mon, 21 Sep 2026 19:42:45 +0000 Subject: [PATCH 11/11] fix(otel v2): drop the redundant _ocr_choices docstring Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- litellm/integrations/otel/model/payloads.py | 1 - 1 file changed, 1 deletion(-) diff --git a/litellm/integrations/otel/model/payloads.py b/litellm/integrations/otel/model/payloads.py index 3164e0977b7..d3ad7234d93 100644 --- a/litellm/integrations/otel/model/payloads.py +++ b/litellm/integrations/otel/model/payloads.py @@ -753,7 +753,6 @@ def _responses_choices(response: Mapping[str, object]) -> tuple[_Choice, ...]: def _ocr_choices(response: Mapping[str, object]) -> tuple[_Choice, ...]: - """An ``OCRResponse`` ``pages`` list folded into one chat-shaped assistant choice.""" markdowns: Final = tuple( text for page in _dicts(response.get("pages")) if (text := as_str(page.get("markdown"))) is not None )