scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-12 23:01:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 87

fix(mcp): separate issuer identity from anchoring so carry-forward keeps endpoints

Browse source 
Making the in-memory issuer reflect a trust-on-first-use discovered value fixed
the registry/row token-identity drift, but it overloaded a single field: the
carry-forward gate keyed on issuer truthiness as a proxy for "endpoints are
anchored to a pinned issuer, fail-closed". A discovered issuer is truthy yet not
anchored, so a resource-rooted server that had learned its issuer would drop its
last-known-good endpoints on a transient discovery blip instead of carrying them
forward.

Anchoring is now a first-class property rather than a proxy. MCPServer carries
issuer_is_anchored, set at both build paths from the single _uses_issuer_anchor
definition (a pinned issuer on a discovery auth type). issuer stays the identity
value used by the token-identity tuple and the serializers; issuer_is_anchored is
the provenance value the carry-forward gate reads to decide fail-closed. The two
properties can no longer be conflated, so a discovered issuer keeps its
resource-rooted endpoints carrying forward while a pinned issuer still fails
closed.

Regression tests pin both directions: a discovered-but-not-anchored server
restores its endpoints on a discovery blip, an anchored server does not, and the
build sets issuer_is_anchored true only when the issuer is pinned
This commit is contained in:
Tin Chi Lo 2026-07-15 23:11:37 -07:00
parent ad73f3a7a2
commit afd7917b8b
3 changed files with 74 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -201,6 +201,18 @@ def _blank_to_none(value: str | None) -> str | None:
return value.strip() or None
def _uses_issuer_anchor(manual_issuer: str | None, is_discovery_auth_type: bool) -> bool:
"""Whether the endpoints are authoritatively anchored to an admin-pinned issuer (RFC 8414 §3.3).
This is the trust/provenance property, distinct from whether the ``issuer`` field is merely
populated: a trust-on-first-use discovered issuer sets ``issuer`` for token identity but is NOT
anchored, so its endpoints stay resource-rooted. Anchoring holds only when the issuer was pinned
(present on the row/config) on a discovery auth type. Every consumer of "is this anchored" reads
this one definition, so the answer cannot diverge across build paths.
"""
return _blank_to_none(manual_issuer) is not None and is_discovery_auth_type
def _endpoints_yield_to_issuer(
issuer: str | None,
is_discovery_auth_type: bool,
@ -292,16 +304,20 @@ def _carry_forward_resolved_oauth_endpoints(new_server: MCPServer, previous_serv
consistent group) or pins the same one. An admin re-pointing ``authorization_url`` to a different
server must not keep serving the old server's token endpoint or granted scopes.
When an ``issuer`` is configured the endpoints must come solely from the §3.3-validated issuer
document, so carry-forward is skipped entirely for its endpoints: a failed issuer fetch leaves
them ``None`` and must stay ``None`` (fail-closed), never resurrected from the previous registry
entry. Scopes stay resource-driven and can still carry.
When the server is issuer-anchored (``issuer_is_anchored`` -- a pinned issuer on a discovery auth
type), the endpoints come solely from the §3.3-validated issuer document, so carry-forward is
skipped entirely for its endpoints: a failed issuer fetch leaves them ``None`` and must stay
``None`` (fail-closed), never resurrected from the previous registry entry. A merely discovered
(trust-on-first-use) issuer is NOT anchored -- ``issuer`` is set for token identity but the
endpoints are resource-rooted, so they still carry forward as last-known-good, gated by the
corroboration check below like any other resource-rooted server. Scopes stay resource-driven and
can carry either way.
"""
if previous_server is None:
return
if previous_server.url != new_server.url or previous_server.auth_type != new_server.auth_type:
return
if _blank_to_none(new_server.issuer):
if new_server.issuer_is_anchored:
# Endpoints come solely from the §3.3-validated issuer document; a failed fetch stays
# fail-closed and must not be resurrected from the previous entry. Only the resource-driven
# scopes carry as last-known-good.
@ -1185,7 +1201,7 @@ class MCPServerManager:
manual_token_url = _blank_to_none(server_config.get("token_url"))
manual_registration_url = _blank_to_none(server_config.get("registration_url"))
is_discovery_auth_type = auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES
use_issuer_anchor = manual_issuer is not None and is_discovery_auth_type
use_issuer_anchor = _uses_issuer_anchor(manual_issuer, is_discovery_auth_type)
manual_authorization_url, manual_token_url, manual_registration_url = _endpoints_yield_to_issuer(
manual_issuer,
is_discovery_auth_type,
@ -1291,6 +1307,7 @@ class MCPServerManager:
oauth2_flow=self._explicit_oauth2_flow(config_oauth2_flow),
scopes=resolved_scopes,
issuer=effective_issuer,
issuer_is_anchored=use_issuer_anchor,
authorization_url=resolved_authorization_url,
token_url=resolved_token_url,
registration_url=resolved_registration_url,
@ -1685,7 +1702,7 @@ class MCPServerManager:
manual_token_url = _blank_to_none(mcp_server.token_url)
manual_registration_url = _blank_to_none(mcp_server.registration_url)
is_discovery_auth_type = auth_type in _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES
use_issuer_anchor = manual_issuer is not None and is_discovery_auth_type
use_issuer_anchor = _uses_issuer_anchor(manual_issuer, is_discovery_auth_type)
manual_authorization_url, manual_token_url, manual_registration_url = _endpoints_yield_to_issuer(
manual_issuer, is_discovery_auth_type, manual_authorization_url, manual_token_url, manual_registration_url
)
@ -1732,6 +1749,7 @@ class MCPServerManager:
oauth2_flow=self._explicit_oauth2_flow(getattr(mcp_server, "oauth2_flow", None)),
scopes=resolved_scopes,
issuer=effective_issuer,
issuer_is_anchored=use_issuer_anchor,
authorization_url=manual_authorization_url or getattr(gated_oauth_metadata, "authorization_url", None),
token_url=manual_token_url or getattr(gated_oauth_metadata, "token_url", None),
registration_url=manual_registration_url or getattr(gated_oauth_metadata, "registration_url", None),

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

@ -66,6 +66,7 @@ class MCPServer(BaseModel):
client_id: Optional[str] = None
client_secret: Optional[str] = None
issuer: Optional[str] = None
issuer_is_anchored: bool = False
scopes: Optional[List[str]] = None
authorization_url: Optional[str] = None
token_url: Optional[str] = None

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

@ -1196,6 +1196,7 @@ class TestMCPServerManager:
built = await manager.build_mcp_server_from_table(row, credentials_are_encrypted=False)
assert built.issuer == "https://idp.example.com"
assert built.issuer_is_anchored is False
assert built.authorization_url == "https://idp.example.com/authorize"
@pytest.mark.asyncio
@ -1326,6 +1327,7 @@ class TestMCPServerManager:
anchored.assert_awaited_once_with("https://idp.example.com", "https://up.example.com/mcp")
resource_rooted.assert_not_awaited()
assert built.issuer == "https://idp.example.com"
assert built.issuer_is_anchored is True
assert built.authorization_url == "https://idp.example.com/authorize"
assert built.token_url == "https://idp.example.com/token"
assert built.registration_url == "https://idp.example.com/register"
@ -5869,11 +5871,12 @@ class TestMCPServerTimestamps:
assert same_authorize.registration_url == "https://idp.example.com/register"
def test_carry_forward_does_not_restore_endpoints_for_issuer_anchored_server(self):
"""When an issuer is configured the endpoints come solely from the §3.3-validated issuer
"""When the server is issuer-anchored the endpoints come solely from the §3.3-validated issuer
document, so a failed issuer fetch (token_url None) must stay fail-closed. Carry-forward must
NOT resurrect the previous registry entry's token endpoint, or the very attacker-controlled
endpoint the issuer anchor distrusts would keep being served across rebuilds. Resource-driven
scopes still carry as last-known-good."""
scopes still carry as last-known-good. Anchoring is keyed on the explicit issuer_is_anchored
flag, not on issuer truthiness, so a discovered issuer does not trip this fail-closed branch."""
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
_carry_forward_resolved_oauth_endpoints,
)
@ -5885,6 +5888,7 @@ class TestMCPServerTimestamps:
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
issuer="https://idp.example.com",
issuer_is_anchored=True,
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
registration_url="https://idp.example.com/register",
@ -5897,6 +5901,7 @@ class TestMCPServerTimestamps:
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
issuer="https://idp.example.com",
issuer_is_anchored=True,
)
_carry_forward_resolved_oauth_endpoints(new_server=failed_rebuild, previous_server=previous)
@ -5906,6 +5911,47 @@ class TestMCPServerTimestamps:
assert failed_rebuild.registration_url is None
assert failed_rebuild.scopes == ["read"]
def test_carry_forward_restores_endpoints_for_discovered_issuer_not_anchored(self):
"""A server that merely DISCOVERED its issuer trust-on-first-use is not anchored: issuer is set
for token identity but the endpoints are resource-rooted, so on a transient discovery blip they
must still carry forward as last-known-good, the same as any resource-rooted server. This is the
regression the explicit issuer_is_anchored flag prevents: keying fail-closed on issuer truthiness
alone would drop the working endpoints the moment the server learned its issuer."""
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
_carry_forward_resolved_oauth_endpoints,
)
previous = MCPServer(
server_id="s1",
name="s1",
url="https://up.example.com/mcp",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
issuer="https://idp.example.com",
issuer_is_anchored=False,
authorization_url="https://idp.example.com/authorize",
token_url="https://idp.example.com/token",
registration_url="https://idp.example.com/register",
scopes=["read"],
)
blipped_rebuild = MCPServer(
server_id="s1",
name="s1",
url="https://up.example.com/mcp",
transport=MCPTransport.http,
auth_type=MCPAuth.oauth2,
issuer="https://idp.example.com",
issuer_is_anchored=False,
authorization_url=None,
)
_carry_forward_resolved_oauth_endpoints(new_server=blipped_rebuild, previous_server=previous)
assert blipped_rebuild.authorization_url == "https://idp.example.com/authorize"
assert blipped_rebuild.token_url == "https://idp.example.com/token"
assert blipped_rebuild.registration_url == "https://idp.example.com/register"
assert blipped_rebuild.scopes == ["read"]
def test_normalized_authorize_endpoint_treats_default_port_and_slash_as_identity(self):
"""The corroboration check must not fail on formatting-only differences an IdP legitimately
emits: default port, trailing slash, host case, and query string are not identity, but a