mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
fix(mcp): keep discovered scopes out of saved settings
This commit is contained in:
parent
ca287c1b15
commit
362d99e001
4 changed files with 240 additions and 16 deletions
|
|
@ -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=[],
|
||||
|
|
|
|||
|
|
@ -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".
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue