scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-11 22:51:28 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 123

fix(mcp): bound pinned-config discovery to the corroborated authorization server, scopes included

Browse source 
Provenance is a property of the whole discovered metadata document, not per
field. Waving scopes through while gating endpoints left a second inflation
vector: a compromised upstream advertises broad scopes via the resource
metadata (RFC 9728 / WWW-Authenticate), the gateway requests them from the
trusted authorization server, and the resulting token flows back to the
upstream. Both that and the token-endpoint mix-up are now one rule: when
authorization_url is admin-pinned, discovered token_url/registration_url are
kept only if the document corroborates the pin, and scopes come from the
authorization server's own scopes_supported (a new authorization_server_scopes
field, trusted tier) rather than the resource-advertised scopes. A document
that does not corroborate backfills nothing. Blank (empty-string)
authorization_url is treated as unpinned so the merge and the gate agree.
Carry-forward, the other non-manual source, drops the same three across an
authorization_url change.
This commit is contained in:
Tin Chi Lo 2026-07-15 10:57:40 -07:00
parent 447d50fa40
commit 8650f6c7d3
3 changed files with 147 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
View file

@ -219,7 +219,7 @@ def _endpoints_corroborate_authorization_url(
value (``trusted_authorization_url is None``) there is nothing to protect: the authorize endpoint
comes from the same source as the token endpoint, so they corroborate each other by construction.
"""
if trusted_authorization_url is None:
if not (trusted_authorization_url and trusted_authorization_url.strip()):
return True
return bool(source_authorization_url) and _normalized_authorize_endpoint(
source_authorization_url
@ -240,43 +240,53 @@ def _carry_forward_resolved_oauth_endpoints(new_server: MCPServer, previous_serv
servers that never had one configured.
Carry-forward is a non-manual endpoint source, so the same trust rule as discovery applies: the
previous ``token_url``/``registration_url`` are carried only when the previous
previous ``token_url``/``registration_url``/``scopes`` are carried only when the previous
``authorization_url`` corroborates the authorize endpoint this build will use, i.e. when the
incoming build has no pinned authorize endpoint (``None`` -> we adopt the previous one too, a
consistent group) or pins the same one. An admin re-pointing ``authorization_url`` to a different
server must not keep serving the old token endpoint.
server must not keep serving the old server's token endpoint or granted scopes.
"""
if previous_server is None:
return
if previous_server.url != new_server.url or previous_server.auth_type != new_server.auth_type:
return
may_carry_endpoints = _endpoints_corroborate_authorization_url(
may_carry = _endpoints_corroborate_authorization_url(
previous_server.authorization_url, new_server.authorization_url
)
if new_server.authorization_url is None and previous_server.authorization_url:
new_server.authorization_url = previous_server.authorization_url
if may_carry_endpoints and new_server.token_url is None and previous_server.token_url:
if may_carry and new_server.token_url is None and previous_server.token_url:
new_server.token_url = previous_server.token_url
if may_carry_endpoints and new_server.registration_url is None and previous_server.registration_url:
if may_carry and new_server.registration_url is None and previous_server.registration_url:
new_server.registration_url = previous_server.registration_url
if not new_server.scopes and previous_server.scopes:
if may_carry and not new_server.scopes and previous_server.scopes:
new_server.scopes = previous_server.scopes
def _gate_discovered_endpoints_against_manual_authorization_url(
def _restrict_discovery_to_corroborated_authorization_server(
metadata: MCPOAuthMetadata | None,
manual_authorization_url: str | None,
server_identifier: str,
is_dcr_bridge: bool,
) -> MCPOAuthMetadata | None:
"""Apply :func:`_endpoints_corroborate_authorization_url` to freshly discovered metadata, the
other non-manual endpoint source. Drops the discovered ``token_url``/``registration_url`` (never
the scopes, which carry no credentials) when they cannot be vouched for by the pinned authorize
endpoint, logging why so an intentional mismatch can be resolved by setting Token URL by hand.
"""Bound what freshly discovered metadata may backfill into a manually pinned config.
Discovery is rooted at the MCP resource, so provenance is a property of the whole metadata
document, not per field: a compromised upstream can advertise both an attacker ``token_endpoint``
(the RFC 9700 mix-up) and inflated ``scopes`` (tricking the user into granting a broader token
that then flows to the upstream). Both are closed by one rule. When ``authorization_url`` is
admin-pinned, the discovered ``token_url`` and ``registration_url`` are kept only if the document
corroborates the pin (its ``authorization_endpoint`` matches), and scopes are taken from the
authorization server's own ``scopes_supported`` (``authorization_server_scopes``, trusted tier)
rather than the resource-advertised ``scopes`` a compromised upstream controls. A document that
does not corroborate backfills nothing. With no pin there is no trust anchor to protect and the
authorize endpoint comes from the same chain as everything else, so discovery is returned as-is.
"""
if metadata is None or (not metadata.token_url and not metadata.registration_url):
if metadata is None or not (manual_authorization_url and manual_authorization_url.strip()):
return metadata
if _endpoints_corroborate_authorization_url(metadata.authorization_url, manual_authorization_url):
return metadata.model_copy(update={"scopes": metadata.authorization_server_scopes})
if not metadata.token_url and not metadata.registration_url and not metadata.scopes:
return metadata
bridge_note = (
" The discovered registration_url is rejected with it, so this dcr_bridge server stays on the"
@ -286,15 +296,15 @@ def _gate_discovered_endpoints_against_manual_authorization_url(
)
verbose_logger.warning(
"MCP OAuth discovery for server %s advertised authorization_endpoint %s, which does not match the "
"manually configured authorization_url %s; rejecting the discovered token_url/registration_url so "
"authorization codes and client credentials only go to endpoints vouched for by the configured "
"authorization server. Configure Token URL manually if the mismatch is intentional.%s",
"manually configured authorization_url %s; rejecting the discovered token_url/registration_url/scopes "
"so authorization codes, client credentials, and granted scopes only follow the configured "
"authorization server. Configure Token URL and Scopes manually if the mismatch is intentional.%s",
server_identifier,
_normalized_authorize_endpoint(metadata.authorization_url) if metadata.authorization_url else "<absent>",
_normalized_authorize_endpoint(manual_authorization_url) if manual_authorization_url else "<absent>",
_normalized_authorize_endpoint(manual_authorization_url),
bridge_note,
)
return metadata.model_copy(update={"token_url": None, "registration_url": None})
return metadata.model_copy(update={"token_url": None, "registration_url": None, "scopes": None})
def invalidate_user_env_vars_cache(user_id: str, server_id: str) -> None:
@ -1126,7 +1136,7 @@ class MCPServerManager:
mcp_oauth_metadata = None
gated_oauth_metadata = (
_gate_discovered_endpoints_against_manual_authorization_url(
_restrict_discovery_to_corroborated_authorization_server(
mcp_oauth_metadata,
server_config.get("authorization_url"),
server_name or server_id,
@ -1568,7 +1578,7 @@ class MCPServerManager:
server_url,
)
gated_oauth_metadata = (
_gate_discovered_endpoints_against_manual_authorization_url(
_restrict_discovery_to_corroborated_authorization_server(
mcp_oauth_metadata,
mcp_server.authorization_url,
mcp_server.server_id,
@ -3365,6 +3375,7 @@ class MCPServerManager:
authorization_url=data.get("authorization_endpoint"),
token_url=data.get("token_endpoint"),
registration_url=data.get("registration_endpoint"),
authorization_server_scopes=scopes,
)
if any(

9
litellm/types/mcp_server/mcp_server_manager.py
View file

@ -17,9 +17,18 @@ MCPInfo = Dict[str, Any]
class MCPOAuthMetadata(BaseModel):
scopes: Optional[List[str]] = None
"""Effective scopes, resource-preferred: the RFC 9728 protected-resource advertisement or the
WWW-Authenticate challenge when the resource supplied one, else the authorization server's
``scopes_supported``. A compromised resource server can influence this, so it must not expand a
manually pinned ``authorization_url`` (see ``authorization_server_scopes``)."""
authorization_url: Optional[str] = None
token_url: Optional[str] = None
registration_url: Optional[str] = None
authorization_server_scopes: Optional[List[str]] = None
"""The ``scopes_supported`` enumerated by the authorization-server metadata document itself
(RFC 8414), independent of anything the resource server advertised. This is the only scope
source trusted to backfill a manually pinned ``authorization_url``, because it shares provenance
with the ``authorization_endpoint`` used to corroborate that pin."""
from_origin_fallback: bool = False
"""True when the metadata came from guessing the resource origin as its authorization
server rather than from an RFC 9728/8414-advertised document. Guessed endpoints are

137
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py
View file

@ -357,17 +357,18 @@ class TestMCPServerManager:
assert server.needs_user_oauth_token is True
@pytest.mark.asyncio
async def test_load_servers_from_config_rejects_discovered_token_url_on_authorization_endpoint_mismatch(self):
async def test_load_servers_from_config_rejects_uncorroborated_discovery_including_scopes(self):
"""The config loader always runs discovery and or-merges per field, so a yaml server with a
manual authorization_url has the same config-time mix-up exposure as a DB row: a discovered
token_url from a document advertising a different authorize endpoint must not be combined
with the pinned one. Scopes still backfill."""
manual authorization_url has the same config-time mix-up exposure as a DB row: a document
advertising a different authorize endpoint backfills nothing, neither its token_url nor its
scopes."""
manager = MCPServerManager()
metadata = MCPOAuthMetadata(
authorization_url="https://attacker.example.com/authorize",
token_url="https://attacker.example.com/token",
scopes=["read"],
scopes=["read", "admin"],
authorization_server_scopes=["read", "admin"],
)
config = self._oauth2_config(
oauth2_flow="authorization_code",
@ -380,19 +381,20 @@ class TestMCPServerManager:
server = next(iter(manager.config_mcp_servers.values()))
assert server.authorization_url == "https://idp.example.com/authorize"
assert server.token_url is None
assert server.scopes == ["read"]
assert server.scopes is None
@pytest.mark.asyncio
async def test_load_servers_from_config_fills_token_url_when_metadata_corroborates_manual_authorization_url(self):
"""Corroborated metadata keeps the self-heal on the config path: when the discovered
document advertises the same authorize endpoint the admin pinned, its token_url fills the
blank field."""
blank field and scopes come from the authorization server's own scopes_supported."""
manager = MCPServerManager()
metadata = MCPOAuthMetadata(
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
scopes=["read"],
scopes=["read", "admin"],
authorization_server_scopes=["read"],
)
config = self._oauth2_config(
oauth2_flow="authorization_code",
@ -404,6 +406,34 @@ class TestMCPServerManager:
server = next(iter(manager.config_mcp_servers.values()))
assert server.token_url == "https://idp.example.com/token"
assert server.scopes == ["read"]
@pytest.mark.asyncio
async def test_load_servers_from_config_blank_authorization_url_is_not_a_pin(self):
"""A blank (empty-string) authorization_url is not a trust anchor, so discovery backfills
the whole set — authorize endpoint, token_url, and its resource-preferred scopes — from the
same chain, exactly as if the field had been omitted. The corroboration gate must treat
empty-string as unpinned so it does not strand the token_url the merge still fills."""
manager = MCPServerManager()
metadata = MCPOAuthMetadata(
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
scopes=["read"],
authorization_server_scopes=["read"],
)
config = self._oauth2_config(
oauth2_flow="authorization_code",
authorization_url="",
token_url=None,
)
with patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)):
await manager.load_servers_from_config(config)
server = next(iter(manager.config_mcp_servers.values()))
assert server.authorization_url == "https://idp.example.com/authorize"
assert server.token_url == "https://idp.example.com/token"
assert server.scopes == ["read"]
@pytest.mark.asyncio
async def test_load_servers_from_config_non_oauth2_needs_no_flow(self):
@ -1102,12 +1132,12 @@ class TestMCPServerManager:
assert built.token_url == "https://idp.example.com/token"
@pytest.mark.asyncio
async def test_build_from_table_discovers_scopes_when_authorization_url_is_manual(self):
"""An admin-typed authorization_url must not switch off discovery for the fields left
blank: without the scopes_supported backfill the authorize redirect goes out scope-less
and IdPs like Google hard-fail it with 400 "Missing required parameter: scope". Scope
backfill works even when the advertised authorization_endpoint differs from the manual
value, because scopes only steer the redirect to the trusted authorize endpoint."""
async def test_build_from_table_backfills_scopes_from_authorization_server_not_resource(self):
"""When authorization_url is admin-pinned, scopes backfill from the authorization server's
own scopes_supported (trusted tier), never from the resource-advertised scopes a compromised
upstream controls. Here the corroborating document carries an inflated resource `scopes`
(`admin`) alongside the real authorization_server_scopes; only the latter may be requested,
otherwise a hostile resource could trick the user into granting a broader token."""
manager = MCPServerManager()
row = LiteLLM_MCPServerTable(
server_id="manual-auth-url-1",
@ -1116,24 +1146,24 @@ class TestMCPServerManager:
url="https://up.example.com/mcp",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
authorization_url="https://idp.example.com/manual-authorize",
authorization_url="https://idp.example.com/authorize",
created_at=datetime.now(),
updated_at=datetime.now(),
)
metadata = MCPOAuthMetadata(
authorization_url="https://idp.example.com/discovered-authorize",
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
registration_url=None,
scopes=["calendar.read", "calendar.write"],
scopes=["read", "admin"],
authorization_server_scopes=["read", "write"],
)
with patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)) as mock_discovery:
built = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False)
mock_discovery.assert_awaited_once()
assert built.authorization_url == "https://idp.example.com/manual-authorize"
assert built.token_url is None
assert built.scopes == ["calendar.read", "calendar.write"]
assert built.authorization_url == "https://idp.example.com/authorize"
assert built.token_url == "https://idp.example.com/token"
assert built.scopes == ["read", "write"]
@pytest.mark.asyncio
async def test_build_from_table_fills_endpoints_when_metadata_corroborates_manual_authorization_url(self):
@ -1159,6 +1189,7 @@ class TestMCPServerManager:
token_url="https://idp.example.com/token",
registration_url="https://idp.example.com/register",
scopes=["read"],
authorization_server_scopes=["read"],
)
with patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)):
built = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False)
@ -1173,15 +1204,15 @@ class TestMCPServerManager:
"advertised_authorization_url",
["https://attacker.example.com/authorize", None],
)
async def test_build_from_table_rejects_discovered_token_url_on_authorization_endpoint_mismatch(
async def test_build_from_table_rejects_uncorroborated_discovery_including_scopes(
self, advertised_authorization_url
):
"""Resource-rooted discovery lets a compromised upstream advertise its own authorization
server. With a manual authorization_url pinned, accepting that document's token_url would
send the authorization code, stored client secret, and PKCE verifier to the attacker's
token endpoint (config-time RFC 9700 mix-up), and the persist hook would make the hostile
endpoint durable. Both the in-memory merge and the persisted metadata must drop the
uncorroborated token_url and registration_url."""
server. With a manual authorization_url pinned, a document that does not corroborate it
backfills nothing: accepting its token_url would send the code, client secret, and PKCE
verifier to the attacker (config-time RFC 9700 mix-up), and accepting its scopes would let
the upstream inflate the granted token. Both the in-memory merge and the persisted metadata
must drop the uncorroborated token_url, registration_url, and scopes."""
manager = MCPServerManager()
row = LiteLLM_MCPServerTable(
server_id="manual-auth-url-3",
@ -1199,7 +1230,8 @@ class TestMCPServerManager:
authorization_url=advertised_authorization_url,
token_url="https://attacker.example.com/token",
registration_url="https://attacker.example.com/register",
scopes=["read"],
scopes=["read", "admin"],
authorization_server_scopes=["read", "admin"],
)
with (
patch.object(manager, "_descovery_metadata", new=AsyncMock(return_value=metadata)),
@ -1210,11 +1242,11 @@ class TestMCPServerManager:
assert built.authorization_url == "https://idp.example.com/authorize"
assert built.token_url is None
assert built.registration_url is None
assert built.scopes == ["read"]
assert built.scopes is None
persisted_metadata = mock_persist.await_args.kwargs["metadata"]
assert persisted_metadata.token_url is None
assert persisted_metadata.registration_url is None
assert persisted_metadata.scopes == ["read"]
assert persisted_metadata.scopes is None
@pytest.mark.asyncio
async def test_build_from_table_skips_discovery_when_all_upstream_oauth_fields_present(self):
@ -2317,6 +2349,48 @@ class TestMCPServerManager:
assert result.scopes == ["api://some-scope/.default"]
assert result.from_origin_fallback is False
@pytest.mark.asyncio
async def test_descovery_metadata_preserves_authorization_server_scopes_under_resource_override(self):
"""The effective `scopes` field is resource-preferred (RFC 9728 / WWW-Authenticate), but the
authorization server's own scopes_supported must survive on `authorization_server_scopes` so
the pinned-config backfill can request the trusted-tier scopes instead of resource-advertised
ones. This is the provenance split the scope-inflation defense depends on."""
manager = MCPServerManager()
mock_response = MagicMock()
mock_response.raise_for_status = MagicMock()
mock_client = MagicMock()
mock_client.get = AsyncMock(return_value=mock_response)
authorization_server_metadata = MCPOAuthMetadata(
scopes=["as.read", "as.write"],
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
authorization_server_scopes=["as.read", "as.write"],
)
with (
patch(
"litellm.proxy._experimental.mcp_server.mcp_server_manager.get_async_httpx_client",
return_value=mock_client,
),
patch.object(
manager,
"_attempt_well_known_discovery",
AsyncMock(return_value=(["https://idp.example.com"], ["resource.only"])),
),
patch.object(
manager,
"_fetch_authorization_server_metadata",
AsyncMock(return_value=authorization_server_metadata),
),
):
result = await manager._descovery_metadata("https://up.example.com/mcp")
assert result is not None
assert result.scopes == ["resource.only"]
assert result.authorization_server_scopes == ["as.read", "as.write"]
@pytest.mark.asyncio
async def test_fetch_single_authorization_server_metadata_supports_azure_issuer_path(
self,
@ -2357,6 +2431,9 @@ class TestMCPServerManager:
assert result.authorization_url == "https://login.microsoftonline.com/test-tenant-id/oauth2/v2.0/authorize"
assert result.token_url == "https://login.microsoftonline.com/test-tenant-id/oauth2/v2.0/token"
assert result.scopes == ["api://some-scope/.default"]
# The authorization server's own scopes_supported is retained under a dedicated field so a
# later resource-scope override cannot erase the trusted-tier value used to backfill a pin.
assert result.authorization_server_scopes == ["api://some-scope/.default"]
@pytest.mark.asyncio
async def test_fetch_single_authorization_server_metadata_derives_azure_metadata(