diff --git a/tests/integration/contracts.json b/tests/integration/contracts.json index b370b577c9b..3f1ecab3489 100644 --- a/tests/integration/contracts.json +++ b/tests/integration/contracts.json @@ -1321,14 +1321,17 @@ "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" + ], + "tests/integration/mcp/test_mcp_lifecycle.py::test_same_url_server_grants_scope_discovery_and_direct_or_virtual_execution[anonymous]": [ + "other.mcp.permissions.same_url_servers_enforce_discovery_and_execution" + ], + "tests/integration/mcp/test_mcp_lifecycle.py::test_same_url_server_grants_scope_discovery_and_direct_or_virtual_execution[bearer]": [ + "other.mcp.permissions.same_url_servers_enforce_discovery_and_execution" ] }, "browser": { diff --git a/tests/integration/mcp/README.md b/tests/integration/mcp/README.md index 870176e8196..6260d128a2c 100644 --- a/tests/integration/mcp/README.md +++ b/tests/integration/mcp/README.md @@ -9,12 +9,12 @@ Run the controlled gateway cases through `python tests/integration/run.py extens | 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) | +| 4. Server ID determines identity | `test_mcp_lifecycle.py` grants one of two servers sharing a URL and denies calls to the other, using explicit server IDs for direct REST calls and server-qualified search results for virtual calls, with and without bearer credentials | Virtual calls identify the target by the searched tool name, not the REST `server_id` field. Bare names such as `add` are ambiguous across servers; 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 | +| 9. Permissions enforced at discovery and execution | Exact key catalog and virtual search results 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 diff --git a/tests/integration/mcp/test_mcp_lifecycle.py b/tests/integration/mcp/test_mcp_lifecycle.py index 0e29959bac7..b32cf97605f 100644 --- a/tests/integration/mcp/test_mcp_lifecycle.py +++ b/tests/integration/mcp/test_mcp_lifecycle.py @@ -1,3 +1,4 @@ +import json import uuid from contextlib import ExitStack from pathlib import Path @@ -56,7 +57,7 @@ def test_tool_error_remains_error_and_healthy_sibling_returns_value(gateway: Gat failure: Final = call_tool(gateway, key, identity, names["fail"], {}) assert failure.status_code == 200, failure.text assert failure.json()["isError"] is True - assert "synthetic tool failure" in failure.json()["content"][0]["text"] + assert failure.json()["content"][0]["text"] == "Error executing tool fail" healthy: Final = call_tool(gateway, key, identity, names["multiply"], {"a": 3, "b": 5}) assert healthy.status_code == 200, healthy.text assert healthy.json()["isError"] is False @@ -199,7 +200,11 @@ def test_warm_credential_removal_rejects_without_upstream_traffic(gateway: Gatew else call_tool(gateway, key, identity, names["add"], {"a": 3, "b": 5}) ) assert rejected.status_code == 500, rejected.text - assert "requires a usable upstream credential" in rejected.text, rejected.text + if operation == "list": + assert rejected.json()["detail"]["error"] == "internal", rejected.text + assert "Failed to list tools from server" in rejected.json()["detail"]["message"], rejected.text + else: + assert "requires a usable upstream credential" in rejected.text, rejected.text assert peer.drain() == (), "missing static credential escaped to upstream" changed = gateway.request( "PUT", @@ -223,46 +228,79 @@ def test_warm_credential_removal_rejects_without_upstream_traffic(gateway: Gatew assert control.status_code == 200 and control.json()["content"][0]["text"] == "15", control.text +@pytest.mark.parametrize("authenticated", (False, True), ids=("anonymous", "bearer")) @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: +def test_same_url_server_grants_scope_discovery_and_direct_or_virtual_execution( + gateway: Gateway, authenticated: bool +) -> 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()) + aliases: Final = tuple("scope" + uuid.uuid4().hex for _ in range(2)) + servers: Final = tuple( + register_mcp( + scenario, + peer, + alias, + auth_type="bearer_token" if authenticated else "none", + static_headers={ + "X-Integration-Server": alias, + **({"Authorization": f"Bearer synthetic-{alias}"} if authenticated else {}), + }, + ) + for alias in aliases + ) 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), - ): + keys: Final = tuple( + scenario.key(object_permission={"mcp_servers": [server], "mcp_tool_search_enabled": virtual}) + for server in servers + ) + for server, alias, key in zip(servers, aliases, keys): + catalog: Final = gateway.request("GET", "/mcp-rest/tools/list", key=key) + assert catalog.status_code == 200, catalog.text + if virtual: + assert {tool["name"] for tool in catalog.json()["tools"]} == { + "mcp_tool_search", + "mcp_tool_call", + "agent_search", + "skill_search", + }, catalog.text + search: Final = gateway.request( + "POST", + "/mcp-rest/tools/call", + {"name": "mcp_tool_search", "arguments": {"query": "add", "top_k": 10}}, + key=key, + ) + assert search.status_code == 200 and search.json()["isError"] is False, search.text + assert [tool["name"] for tool in json.loads(search.json()["content"][0]["text"])] == [ + f"{alias}-add" + ], search.text + else: + assert {tool["mcp_info"]["server_id"] for tool in catalog.json()["tools"]} == {server} + assert {tool["name"] for tool in catalog.json()["tools"]} == {"add", "multiply", "fail"} + for server_index, caller_index in ((0, 0), (1, 0), (1, 1)): peer.drain() response: Final = gateway.request( "POST", "/mcp-rest/tools/call", { - "server_id": server_id, - "name": "mcp_tool_call" if virtual else names["add"], + "name": "mcp_tool_call" if virtual else "add", + **({} if virtual else {"server_id": servers[server_index]}), "arguments": ( - {"tool_name": names["add"], "arguments": {"a": 3, "b": 5}} if virtual else {"a": 3, "b": 5} + {"tool_name": f"{aliases[server_index]}-add", "arguments": {"a": 3, "b": 5}} + if virtual + else {"a": 3, "b": 5} ), }, - key=key, + key=keys[caller_index], ) - 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} + observed: Final = peer.drain() + if server_index != caller_index: + assert response.status_code == 403 and "not allowed" in response.text, response.text + assert observed == (), "forbidden server reached the upstream" + continue + assert response.status_code == 200 and response.json()["isError"] is False, response.text + assert response.json()["content"][0]["text"] == "8", response.text + calls: Final = tuple(item for item in observed if item["body"].get("method") == "tools/call") + assert len(calls) == 1 + assert calls[0]["headers"][b"x-integration-server"] == aliases[server_index].encode() + expected_auth: Final = f"Bearer synthetic-{aliases[server_index]}".encode() if authenticated else None + assert all(item["headers"].get(b"authorization") == expected_auth for item in observed) diff --git a/tests/integration/mcp/test_oauth_configuration.py b/tests/integration/mcp/test_oauth_configuration.py index fbef9e8fed9..4c46c706054 100644 --- a/tests/integration/mcp/test_oauth_configuration.py +++ b/tests/integration/mcp/test_oauth_configuration.py @@ -159,7 +159,8 @@ def test_same_url_oauth_credentials_and_revocation_are_isolated_by_user_and_serv 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 rejected.json() == {"detail": "Unauthorized"}, rejected.text + assert "resource_metadata=" in rejected.headers["www-authenticate"] assert observed == (), "unusable credentials must not fall back to another user or server" else: assert discovery.status_code == 200, discovery.text