mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-21 00:21:49 +00:00
test(mcp): align live regressions with discovery and error contracts
This commit is contained in:
Joshua Valluru
parent
434659e72c
commit
a41b60cf77
4 changed files with 81 additions and 39 deletions
|
|
@ -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": {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue