diff --git a/tests/integration/_support/mcp.py b/tests/integration/_support/mcp.py index d924ee6dad0..bdf60becbaa 100644 --- a/tests/integration/_support/mcp.py +++ b/tests/integration/_support/mcp.py @@ -9,7 +9,7 @@ import httpx from integration._support.asgi import asgi_server from integration._support.client import Gateway, Scenario from integration._support.database import read_rows -from mcp.server.fastmcp import FastMCP +from mcp.server.mcpserver import MCPServer from mcp.server.transport_security import TransportSecuritySettings from mcp_tests.mcp_e2e_upstream_server import add, multiply from starlette.requests import Request @@ -27,12 +27,7 @@ class McpPeer: @contextmanager def mcp_peer() -> Iterator[McpPeer]: - service: Final = FastMCP( - "integration-math", - stateless_http=True, - json_response=True, - transport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False), - ) + service: Final = MCPServer("integration-math") service.add_tool(add) service.add_tool(multiply) @@ -40,7 +35,11 @@ def mcp_peer() -> Iterator[McpPeer]: def fail() -> str: raise ValueError("synthetic tool failure") - app: Final = service.streamable_http_app() + app: Final = service.streamable_http_app( + stateless_http=True, + json_response=True, + transport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False), + ) observed: Final[queue.Queue[dict[str, object]]] = queue.Queue() async def capture(scope: Scope, receive: Receive, send: Send) -> None: @@ -94,9 +93,7 @@ def tool_names(gateway: Gateway, key: str, identity: str) -> dict[str, str]: } -def call_tool( - gateway: Gateway, key: str, identity: str, name: str, arguments: dict[str, object] -) -> httpx.Response: +def call_tool(gateway: Gateway, key: str, identity: str, name: str, arguments: dict[str, object]) -> httpx.Response: return gateway.client.post( "/mcp-rest/tools/call", headers={"x-litellm-api-key": key}, diff --git a/tests/integration/contracts.json b/tests/integration/contracts.json index 7472cde99d3..b370b577c9b 100644 --- a/tests/integration/contracts.json +++ b/tests/integration/contracts.json @@ -1320,6 +1320,15 @@ ], "tests/integration/observability/test_guardrail_effects.py::test_request_selected_mcp_guardrail_blocks_direct_and_virtual_calls": [ "other.mcp.guardrails.request_selection_blocks_resolved_tool_without_execution" + ], + "tests/integration/mcp/test_mcp_lifecycle.py::test_same_url_server_grants_scope_discovery_and_direct_or_virtual_execution": [ + "other.mcp.permissions.same_url_servers_enforce_discovery_and_execution" + ], + "tests/integration/mcp/test_oauth_configuration.py::test_same_url_oauth_credentials_and_revocation_are_isolated_by_user_and_server[revoke]": [ + "other.mcp.oauth.same_url_credentials_are_isolated_by_user_and_server" + ], + "tests/integration/mcp/test_oauth_configuration.py::test_same_url_oauth_credentials_and_revocation_are_isolated_by_user_and_server[expire]": [ + "other.mcp.oauth.same_url_credentials_are_isolated_by_user_and_server" ] }, "browser": { diff --git a/tests/integration/mcp/README.md b/tests/integration/mcp/README.md new file mode 100644 index 00000000000..870176e8196 --- /dev/null +++ b/tests/integration/mcp/README.md @@ -0,0 +1,28 @@ +# MCP security regression coverage + +[LIT-4506](https://linear.app/litellm-ai/issue/LIT-4506) tracks ten gateway guards and the later JWT/OAuth acceptance. This inventory distinguishes executable assertions from unresolved coverage. A listed test counts as verified only when its exact commit has an executed, passing result + +Run the controlled gateway cases through `python tests/integration/run.py extensions`. They use real HTTP, PostgreSQL, scoped non-master keys and an SDK upstream. The existing runner supplies test entitlement; these tests do not validate licenses or external-provider consent. Canonical nodes and contract IDs live in `../contracts.json` + +| Requested guard | Existing or added coverage | Remaining limitation and owner | +| --- | --- | --- | +| 1. Discovery scoped by org/team/user/key | `test_mcp_lifecycle.py` checks the exact key-granted catalog and health visibility in both management modes. [PR #38680](https://github.com/BerriAI/litellm/pull/38680) adds team/org/user toolset E2E assertions | Per-principal native MCP coverage is not established by REST results; reuse #38680 rather than duplicate it | +| 2. Users cannot attach unauthorized servers to their own keys | Existing live probes are recorded on LIT-4506; they are not durable endpoint regression tests | Own-key create/update escalation and its permission-validator boundary remain on existing management security tickets, including [LIT-4502](https://linear.app/litellm-ai/issue/LIT-4502). A generic route denial does not prove that validator ran | +| 3. UI/API permission parity | Existing dashboard tests cover admin operations | The same non-admin actor must be tested through browser and API; admin UI tests do not establish parity. Retained with [LIT-3644](https://linear.app/litellm-ai/issue/LIT-3644) | +| 4. Server ID determines identity | `test_mcp_lifecycle.py` grants one of two servers sharing a URL and denies explicit calls to the other, through direct and virtual REST execution | Duplicate aliases/names and unprefixed protocol routing remain with [LIT-4500](https://linear.app/litellm-ai/issue/LIT-4500) | +| 5. Same-URL servers do not share credentials | `test_oauth_configuration.py` crosses two gateway users with two server IDs and four distinct stored OAuth tokens. It checks actual upstream headers and successful results, then invalidates only one tuple | Controlled stored-token tests do not prove separate external-provider accounts or consent flows | +| 6. OAuth never falls back to anonymous | OAuth isolation variants remove a stored token or expire it without refresh, require separate list/call auth failures and no upstream requests, and preserve all other valid tuples. `test_mcp_lifecycle.py` also covers warm static-header removal and OBO without a caller JWT | External upstream revocation, refresh/reauthorization and aggregate challenges remain with [LIT-4501](https://linear.app/litellm-ai/issue/LIT-4501), [LIT-3433](https://linear.app/litellm-ai/issue/LIT-3433), [LIT-4422](https://linear.app/litellm-ai/issue/LIT-4422) and [LIT-4436](https://linear.app/litellm-ai/issue/LIT-4436) | +| 7. Stateful HTTP/session continuity | Legacy public-client tests exercise initialized sessions | No claim here proves upstream session state continuity; retained under [LIT-3143](https://linear.app/litellm-ai/issue/LIT-3143) | +| 8. Production guardrails/hooks run | `../observability/test_guardrail_effects.py` checks selected pre-call guards on direct and virtual execution, key/team/request selection, allowed results and zero denied executions. The two legacy test-owned dispatcher files are removed | This does not establish every post-call/output-scanning or concurrent hook contract | +| 9. Permissions enforced at discovery and execution | Exact key catalog plus forbidden direct/virtual calls in `test_mcp_lifecycle.py`; existing `../compatibility/test_persisted_toolsets.py` checks tool-level ceiling, denied sibling and allowed control | All principal/transport combinations are not established; link #38680's evidence for its additional principal cases | +| 10. Stateless/stateful matrix | These controlled peers use stateless HTTP upstreams | Stateful combinations depend on LIT-3143 and shared conformance runs. Modern-agent/legacy-upstream interaction remains deferred; legacy passes do not establish modern conformance | + +## Additional JWT/OAuth acceptance + +[LIT-3467 / PR #41909](https://github.com/BerriAI/litellm/pull/41909) owns one shared real login/consent, immediate list/call and cold-restart implementation, with aggregate SSO and explicitly configured per-server JWT variants. Reuse that implementation and its protected login secret; do not create another browser bootstrap here. Credit its exact-commit evidence separately from these controlled credential tests + +The two-user/two-server cases here create non-admin users and scoped API keys through management APIs. They store synthetic upstream OAuth credentials through the real credential endpoint and assert the actual bearer at the owned upstream. This deliberately isolates credential lookup, expiry and revocation from consent. No gateway API key may replace the expected upstream token + +Gateway JWT precedence, invalid/expired gateway JWTs, inactive-user denial, and their MCP-specific interaction with isolated credential lookup remain unverified by these API-key cases. General JWT unit/API tests are useful existing coverage but do not substitute for those MCP outcomes. Real-provider auth failures should extend LIT-3467's settled helpers; its explicit-header case must not be described as an uninterrupted Authorization-only OAuth flow + +[PR #41718 / LIT-7737](https://github.com/BerriAI/litellm/pull/41718) owns dependency and public-client compatibility checks. This suite consumes the merged SDK2 API and keeps the existing dependency constraints. Its result must be reported independently of an installation-matrix pass diff --git a/tests/integration/mcp/test_mcp_lifecycle.py b/tests/integration/mcp/test_mcp_lifecycle.py index fa0ae0ec643..0e29959bac7 100644 --- a/tests/integration/mcp/test_mcp_lifecycle.py +++ b/tests/integration/mcp/test_mcp_lifecycle.py @@ -221,3 +221,48 @@ def test_warm_credential_removal_rejects_without_upstream_traffic(gateway: Gatew control_names = tool_names(gateway, control_key, control_id) control = call_tool(gateway, control_key, control_id, control_names["multiply"], {"a": 3, "b": 5}) assert control.status_code == 200 and control.json()["content"][0]["text"] == "15", control.text + + +@pytest.mark.covers("other.mcp.permissions.same_url_servers_enforce_discovery_and_execution") +def test_same_url_server_grants_scope_discovery_and_direct_or_virtual_execution(gateway: Gateway) -> None: + with mcp_peer() as peer, gateway.scenario() as scenario: + allowed: Final = register_mcp(scenario, peer, "allowed" + uuid.uuid4().hex) + forbidden: Final = register_mcp(scenario, peer, "forbidden" + uuid.uuid4().hex) + caller: Final = scenario.key(object_permission={"mcp_servers": [allowed], "mcp_tool_search_enabled": True}) + control: Final = scenario.key(object_permission={"mcp_servers": [forbidden], "mcp_tool_search_enabled": True}) + allowed_names: Final = tool_names(gateway, caller, allowed) + forbidden_names: Final = tool_names(gateway, control, forbidden) + catalog: Final = gateway.request("GET", "/mcp-rest/tools/list", key=caller) + assert catalog.status_code == 200, catalog.text + assert {tool["mcp_info"]["server_id"] for tool in catalog.json()["tools"]} == {allowed} + assert {tool["name"] for tool in catalog.json()["tools"]} == set(allowed_names.values()) + for virtual in (False, True): + for server_id, names, key, expected in ( + (allowed, allowed_names, caller, 200), + (forbidden, forbidden_names, caller, 403), + (forbidden, forbidden_names, control, 200), + ): + peer.drain() + response: Final = gateway.request( + "POST", + "/mcp-rest/tools/call", + { + "server_id": server_id, + "name": "mcp_tool_call" if virtual else names["add"], + "arguments": ( + {"tool_name": names["add"], "arguments": {"a": 3, "b": 5}} if virtual else {"a": 3, "b": 5} + ), + }, + key=key, + ) + assert response.status_code == expected, response.text + calls: Final = tuple(item for item in peer.drain() if item["body"].get("method") == "tools/call") + if expected == 403: + assert "access" in response.text.lower(), response.text + assert calls == (), "a denied server must not execute through either route" + else: + assert response.json()["isError"] is False, response.text + assert response.json()["content"][0]["text"] == "8", response.text + assert len(calls) == 1 + assert calls[0]["body"]["params"]["name"] == "add" + assert calls[0]["body"]["params"]["arguments"] == {"a": 3, "b": 5} diff --git a/tests/integration/mcp/test_oauth_configuration.py b/tests/integration/mcp/test_oauth_configuration.py index 45d407f2423..fbef9e8fed9 100644 --- a/tests/integration/mcp/test_oauth_configuration.py +++ b/tests/integration/mcp/test_oauth_configuration.py @@ -2,14 +2,14 @@ import json import queue import uuid from urllib.parse import parse_qs, urlsplit -from typing import Final +from typing import Final, Literal from pathlib import Path import pytest from integration._support.client import Gateway, eventually from integration._support.database import read_rows -from integration._support.mcp import McpPeer, register_mcp +from integration._support.mcp import McpPeer, call_tool, mcp_peer, register_mcp, tool_names from integration._support.process import owned_proxy from integration._support.wire import Reply, Request, wire_server @@ -102,3 +102,86 @@ def test_partial_discovery_and_unrelated_edit_keep_actual_authorization_destinat "PUT", "/v1/mcp/server", {"server_id": identity, "server_name": alias + "renamed"} ) assert updated.status_code == 202, updated.text + + +@pytest.mark.covers("other.mcp.oauth.same_url_credentials_are_isolated_by_user_and_server") +@pytest.mark.parametrize("transition", ("revoke", "expire")) +def test_same_url_oauth_credentials_and_revocation_are_isolated_by_user_and_server( + gateway: Gateway, + transition: Literal["revoke", "expire"], +) -> None: + with mcp_peer() as peer, gateway.scenario() as scenario: + servers: Final = tuple( + register_mcp( + scenario, + peer, + "oauth" + uuid.uuid4().hex, + auth_type="oauth2", + oauth2_flow="authorization_code", + authorization_url=peer.url + "/authorize", + token_url=peer.url + "/token", + credentials={"client_id": "synthetic-oauth-client"}, + ) + for _ in range(2) + ) + users: Final = tuple(scenario.user(user_role="internal_user") for _ in range(2)) + keys: Final = tuple( + scenario.key(user_id=user, object_permission={"mcp_servers": list(servers)}) for user in users + ) + for user_index, key in enumerate(keys): + for server_index, server_id in enumerate(servers): + stored: Final = gateway.request( + "POST", + f"/v1/mcp/server/{server_id}/oauth-user-credential", + {"access_token": f"synthetic-user-{user_index}-server-{server_index}", "expires_in": 3600}, + key=key, + ) + assert stored.status_code == 200 and stored.json()["has_credential"] is True, stored.text + scenario.cleanups.callback( + gateway.request, + "DELETE", + f"/v1/mcp/server/{server_id}/oauth-user-credential", + key=key, + ) + names: Final = tuple(tool_names(gateway, keys[0], server) for server in servers) + for generation in range(2): + for user_index, key in enumerate(keys): + for server_index, server_id in enumerate(servers): + peer.drain() + discovery: Final = gateway.request( + "GET", + "/mcp-rest/tools/list", + key=key, + params={"server_id": server_id}, + ) + call: Final = call_tool(gateway, key, server_id, names[server_index]["add"], {"a": 3, "b": 5}) + observed: Final = peer.drain() + if generation == 1 and user_index == 0 and server_index == 0: + for rejected in (discovery, call): + assert rejected.status_code == 401, rejected.text + assert "uthorization required" in rejected.text, rejected.text + assert observed == (), "unusable credentials must not fall back to another user or server" + else: + assert discovery.status_code == 200, discovery.text + assert {tool["name"] for tool in discovery.json()["tools"]} == set(names[server_index].values()) + assert call.status_code == 200 and call.json()["isError"] is False, call.text + assert call.json()["content"][0]["text"] == "8", call.text + calls: Final = tuple(item for item in observed if item["body"].get("method") == "tools/call") + assert len(calls) == 1 + expected: Final = f"Bearer synthetic-user-{user_index}-server-{server_index}".encode() + assert calls[0]["headers"][b"authorization"] == expected + assert all(item["headers"].get(b"authorization") == expected for item in observed) + if generation == 0: + changed: Final = gateway.request( + "DELETE" if transition == "revoke" else "POST", + f"/v1/mcp/server/{servers[0]}/oauth-user-credential", + None + if transition == "revoke" + else { + "access_token": "synthetic-expired-user-0-server-0", + "expires_in": -60, + }, + key=keys[0], + ) + assert changed.status_code == 200, changed.text + assert changed.json()["has_credential"] is (transition == "expire"), changed.text diff --git a/tests/mcp_tests/mcp_e2e_upstream_server.py b/tests/mcp_tests/mcp_e2e_upstream_server.py index 28fb0846481..3361163badf 100644 --- a/tests/mcp_tests/mcp_e2e_upstream_server.py +++ b/tests/mcp_tests/mcp_e2e_upstream_server.py @@ -1,6 +1,6 @@ """Deterministic upstream MCP server for the mcp e2e suite. -A tiny FastMCP server exposing `add` and `multiply` over streamable-http so the +A tiny MCP server exposing `add` and `multiply` over streamable-http so the suite has a self-hosted, offline upstream to register and exercise. DNS-rebinding protection is turned off because the litellm container reaches this over the compose network by service name (`mcp-upstream:8090`), not localhost, and the @@ -9,15 +9,10 @@ stack is an isolated throwaway. Bind host/port come from MCP_HOST/MCP_PORT. import os -from mcp.server.fastmcp import FastMCP +from mcp.server.mcpserver import MCPServer from mcp.server.transport_security import TransportSecuritySettings -mcp: FastMCP = FastMCP( - "e2e-math", - host=os.getenv("MCP_HOST", "0.0.0.0"), - port=int(os.getenv("MCP_PORT", "8090")), - transport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False), -) +mcp: MCPServer = MCPServer("e2e-math") @mcp.tool() @@ -33,7 +28,12 @@ def multiply(a: int, b: int) -> int: def main() -> None: - mcp.run(transport="streamable-http") + mcp.run( + transport="streamable-http", + host=os.getenv("MCP_HOST", "0.0.0.0"), + port=int(os.getenv("MCP_PORT", "8090")), + transport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False), + ) if __name__ == "__main__":