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): deny every zero-resolution scoped request uniformly and persist agent MCP toolsets from the UI

Browse source 
Any scoped MCP request (/mcp/<name> path or x-mcp-servers header) that resolves to zero allowed servers now returns one generic 403, so unknown, unauthorized, and access-group names are indistinguishable and the error cannot be used to enumerate servers. The agent-attributed variant reruns the same scope resolver with the agent binding stripped and fires only when that rerun resolves, naming the vetoed server or access group and both fix paths

AgentObjectPermission now declares mcp_toolsets so PATCH /v1/agents keeps it, and the agent edit form round-trips toolsets alongside servers and access groups instead of dropping them on save. The two scope-resolver params only iterate their names, so they take Sequence[str] and the LIT001 budget ratchets down by the two annotations this widens
This commit is contained in:
mateo-berri 2026-09-01 18:37:23 -07:00
parent c4982ca407
commit 659eb0462e
10 changed files with 189 additions and 152 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

60
litellm/proxy/_experimental/mcp_server/server.py
View file

@ -1393,7 +1393,7 @@ if MCP_AVAILABLE:
########################################################
async def _get_allowed_mcp_servers_from_mcp_server_names(
mcp_servers: list[str] | None,
mcp_servers: Sequence[str] | None,
allowed_mcp_servers: list[MCPServer],
) -> list[MCPServer]:
"""
@ -1459,35 +1459,55 @@ if MCP_AVAILABLE:
client_ip: str | None = None,
) -> None:
"""A scoped request (``/mcp/<name>`` path or ``x-mcp-servers`` header) resolved to zero
allowed servers. When a requested name IS a registered server visible to this client IP,
the denial is a permission outcome and must be loud: a silent 200 with no tools reads as
a healthy server with no tools. Names matching no registered server stay fail-closed
empty so scoping cannot probe for server existence."""
known_targets: Final = tuple(
(name, server)
for name in requested_names
if (server := global_mcp_server_manager.get_mcp_server_by_name(name, client_ip=client_ip)) is not None
)
if not known_targets:
return
denied_name, denied_server = known_targets[0]
allowed servers, so the denial must be loud: a silent 200 with no tools reads as a healthy
server with no tools. Unknown, unauthorized, and access-group names all share one generic
error so scoping cannot probe which servers exist; the agent variant fires only when the
same request resolves once the agent binding is stripped, proving the binding caused the veto."""
agent_id: Final = user_api_key_auth.agent_id if user_api_key_auth else None
if user_api_key_auth is not None and agent_id:
allowed_without_agent: Final = await global_mcp_server_manager.get_allowed_mcp_servers(
user_api_key_auth.model_copy(update=types.MappingProxyType({"agent_id": None}))
resolved_without_agent: Final = await _get_allowed_mcp_servers(
user_api_key_auth=user_api_key_auth.model_copy(update=types.MappingProxyType({"agent_id": None})),
mcp_servers=requested_names,
client_ip=client_ip,
)
if denied_server.server_id in allowed_without_agent:
resolved_ids: Final = frozenset(server.server_id for server in resolved_without_agent)
def _registered_server_id(name: str) -> str | None:
server: Final = global_mcp_server_manager.get_mcp_server_by_name(name, client_ip=client_ip)
return server.server_id if server is not None else None
vetoed_server: Final = next(
(name for name in requested_names if _registered_server_id(name) in resolved_ids), None
)
if vetoed_server is not None:
agent_denial: Final[_McpDeniedDetail] = {
"error": (
f"MCP server '{denied_name}' is not available to this key: the key is bound to "
f"MCP server '{vetoed_server}' is not available to this key: the key is bound to "
f"agent '{agent_id}', whose MCP grants do not include this server. Add the server "
f"to the agent's object_permission.mcp_servers (edit the agent in the Admin UI or "
f"PATCH /v1/agents/{agent_id}), or use a key that is not bound to the agent."
)
}
raise HTTPException(status_code=403, detail=agent_denial)
key_denial: Final[_McpDeniedDetail] = {"error": f"The key is not allowed to access server {denied_name}"}
raise HTTPException(status_code=403, detail=key_denial)
vetoed_group: Final = (
next((name for name in requested_names if _registered_server_id(name) is None), None)
if resolved_ids
else None
)
if vetoed_group is not None:
group_denial: Final[_McpDeniedDetail] = {
"error": (
f"MCP access group '{vetoed_group}' is not available to this key: the key is bound to "
f"agent '{agent_id}', whose MCP grants do not include it. Add the group to the "
f"agent's object_permission.mcp_access_groups (edit the agent in the Admin UI or "
f"PATCH /v1/agents/{agent_id}), or use a key that is not bound to the agent."
)
}
raise HTTPException(status_code=403, detail=group_denial)
generic_denial: Final[_McpDeniedDetail] = {
"error": f"The key is not allowed to access the requested MCP servers: {', '.join(requested_names)}"
}
raise HTTPException(status_code=403, detail=generic_denial)
def _tool_name_matches(tool_name: str, filter_list: list[str], mcp_server: MCPServer) -> bool:
"""
@ -1581,7 +1601,7 @@ if MCP_AVAILABLE:
async def _get_allowed_mcp_servers(
user_api_key_auth: UserAPIKeyAuth | None,
mcp_servers: list[str] | None,
mcp_servers: Sequence[str] | None,
client_ip: str | None = None,
) -> list[MCPServer]:
"""Return allowed MCP servers for a request after applying filters.

14
litellm/proxy/_lazy_openapi_snapshot.json
View file

@ -2576,6 +2576,20 @@
],
"title": "Mcp Tool Permissions"
},
"mcp_toolsets": {
"anyOf": [
{
"items": {
"type": "string"
},
"type": "array"
},
{
"type": "null"
}
],
"title": "Mcp Toolsets"
},
"models": {
"anyOf": [
{

5
litellm/types/agents.py
View file

@ -1,9 +1,9 @@
from collections.abc import Mapping
from collections.abc import Mapping, Sequence
from datetime import datetime
from typing import TYPE_CHECKING, Any, Final, Literal
from pydantic import BaseModel, PrivateAttr, StrictInt
from typing_extensions import Required, TypedDict
from typing_extensions import ReadOnly, Required, TypedDict
from litellm.types.llms.base import LiteLLMPydanticObjectBase
@ -172,6 +172,7 @@ class AugmentedAgentCard(AgentCard):
class AgentObjectPermission(TypedDict, total=False):
mcp_servers: list[str] | None
mcp_access_groups: list[str] | None
mcp_toolsets: ReadOnly[Sequence[str] | None]
mcp_tool_permissions: dict[str, list[str]] | None
models: list[str] | None
agents: list[str] | None

216
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py
View file

@ -1329,157 +1329,155 @@ async def test_get_tools_from_mcp_servers_handles_all_servers_failing():
mock_logger.info.assert_any_call("Successfully fetched %s tools total from all MCP servers", 0)
def _denied_scope_manager(known_server_names_to_ids: dict[str, str], allowed_without_agent: list[str]) -> MagicMock:
"""A manager whose get_mcp_server_by_name knows the given names and whose
get_allowed_mcp_servers answers the agent-stripped permission rerun."""
def _denied_scope_manager(known_server_names_to_ids: dict[str, str]) -> MagicMock:
servers = {name: MagicMock(server_id=server_id) for name, server_id in known_server_names_to_ids.items()}
manager = MagicMock()
manager.get_mcp_server_by_name = lambda name, client_ip=None: servers.get(name)
manager.get_allowed_mcp_servers = AsyncMock(return_value=allowed_without_agent)
return manager
def _scope_resolver(resolved_without_agent: list[str]) -> AsyncMock:
async def resolve(user_api_key_auth, mcp_servers, client_ip=None):
if user_api_key_auth is not None and user_api_key_auth.agent_id:
return []
return [MagicMock(server_id=server_id) for server_id in resolved_without_agent]
return AsyncMock(side_effect=resolve)
async def _denied_scoped_list(
user_api_key_auth: UserAPIKeyAuth,
mcp_servers: list[str],
mock_manager: MagicMock,
resolver: AsyncMock,
) -> HTTPException:
from litellm.proxy._experimental.mcp_server.server import _get_tools_from_mcp_servers
with (
patch( # test-quality-ok: the permission resolver is a module-level function; the suite's only seam
"litellm.proxy._experimental.mcp_server.server._get_allowed_mcp_servers",
resolver,
),
patch( # test-quality-ok: the server registry is a module-level singleton; the suite's only seam
"litellm.proxy._experimental.mcp_server.server.global_mcp_server_manager",
mock_manager,
),
):
with pytest.raises(HTTPException) as exc_info:
await _get_tools_from_mcp_servers(
user_api_key_auth=user_api_key_auth,
mcp_auth_header=None,
mcp_servers=mcp_servers,
)
return exc_info.value
@pytest.mark.asyncio
async def test_scoped_list_denied_by_agent_binding_raises_403_naming_agent():
"""A scoped tools/list that resolves to zero servers because the key's bound agent lacks the
grant must raise a 403 naming the agent, never return a silent 200 with no tools."""
try:
from litellm.proxy._experimental.mcp_server.server import _get_tools_from_mcp_servers
except ImportError:
pytest.skip("MCP server not available")
"""The agent-binding veto must raise a 403 naming the agent, never a silent 200 with no tools."""
pytest.importorskip("litellm.proxy._experimental.mcp_server.server")
user_api_key_auth = UserAPIKeyAuth(api_key="test_key", user_id="test_user", agent_id="agent-123")
mock_manager = _denied_scope_manager({"github": "srv-github"}, allowed_without_agent=["srv-github"])
resolver = _scope_resolver(resolved_without_agent=["srv-github"])
with (
patch( # test-quality-ok: the permission resolver is a module-level function; the suite's only seam
"litellm.proxy._experimental.mcp_server.server._get_allowed_mcp_servers",
AsyncMock(return_value=[]),
),
patch( # test-quality-ok: the server registry is a module-level singleton; the suite's only seam
"litellm.proxy._experimental.mcp_server.server.global_mcp_server_manager",
mock_manager,
),
):
with pytest.raises(HTTPException) as exc_info:
await _get_tools_from_mcp_servers(
user_api_key_auth=user_api_key_auth,
mcp_auth_header=None,
mcp_servers=["github"],
)
denial = await _denied_scoped_list(
user_api_key_auth, ["github"], _denied_scope_manager({"github": "srv-github"}), resolver
)
assert exc_info.value.status_code == 403
message = exc_info.value.detail["error"]
assert "github" in message
assert denial.status_code == 403
message = denial.detail["error"]
assert "MCP server 'github'" in message
assert "agent 'agent-123'" in message
rerun_auth = mock_manager.get_allowed_mcp_servers.await_args.args[0]
assert "mcp_servers" in message
rerun_auth = resolver.await_args_list[1].kwargs["user_api_key_auth"]
assert rerun_auth.agent_id is None
assert rerun_auth.user_id == "test_user"
assert resolver.await_args_list[1].kwargs["mcp_servers"] == ["github"]
@pytest.mark.asyncio
async def test_scoped_list_denied_for_non_agent_key_raises_generic_403():
"""A scoped tools/list denied for a key with no agent binding raises the generic 403 and
never runs the agent-stripped permission rerun."""
try:
from litellm.proxy._experimental.mcp_server.server import _get_tools_from_mcp_servers
except ImportError:
pytest.skip("MCP server not available")
"""A denial for a key with no agent binding stays generic and skips the agent-stripped rerun."""
pytest.importorskip("litellm.proxy._experimental.mcp_server.server")
user_api_key_auth = UserAPIKeyAuth(api_key="test_key", user_id="test_user")
mock_manager = _denied_scope_manager({"github": "srv-github"}, allowed_without_agent=["srv-github"])
resolver = AsyncMock(return_value=[])
with (
patch( # test-quality-ok: the permission resolver is a module-level function; the suite's only seam
"litellm.proxy._experimental.mcp_server.server._get_allowed_mcp_servers",
AsyncMock(return_value=[]),
),
patch( # test-quality-ok: the server registry is a module-level singleton; the suite's only seam
"litellm.proxy._experimental.mcp_server.server.global_mcp_server_manager",
mock_manager,
),
):
with pytest.raises(HTTPException) as exc_info:
await _get_tools_from_mcp_servers(
user_api_key_auth=user_api_key_auth,
mcp_auth_header=None,
mcp_servers=["github"],
)
denial = await _denied_scoped_list(
user_api_key_auth, ["github"], _denied_scope_manager({"github": "srv-github"}), resolver
)
assert exc_info.value.status_code == 403
message = exc_info.value.detail["error"]
assert denial.status_code == 403
message = denial.detail["error"]
assert "github" in message
assert "agent" not in message
mock_manager.get_allowed_mcp_servers.assert_not_awaited()
resolver.assert_awaited_once()
@pytest.mark.asyncio
async def test_scoped_list_unknown_server_name_stays_silent_empty():
"""A scoped request naming no registered server stays fail-closed empty (200, no tools), so
scoping cannot probe for server existence."""
try:
from litellm.proxy._experimental.mcp_server.server import _get_tools_from_mcp_servers
except ImportError:
pytest.skip("MCP server not available")
async def test_scoped_list_unknown_name_raises_same_generic_403_as_unauthorized():
"""Unknown and registered-but-unauthorized names raise byte-identical generic 403s, so a
caller cannot probe which server names exist."""
pytest.importorskip("litellm.proxy._experimental.mcp_server.server")
user_api_key_auth = UserAPIKeyAuth(api_key="test_key", user_id="test_user", agent_id="agent-123")
mock_manager = _denied_scope_manager({}, allowed_without_agent=["srv-github"])
with (
patch( # test-quality-ok: the permission resolver is a module-level function; the suite's only seam
"litellm.proxy._experimental.mcp_server.server._get_allowed_mcp_servers",
AsyncMock(return_value=[]),
),
patch( # test-quality-ok: the server registry is a module-level singleton; the suite's only seam
"litellm.proxy._experimental.mcp_server.server.global_mcp_server_manager",
mock_manager,
),
):
result = await _get_tools_from_mcp_servers(
user_api_key_auth=user_api_key_auth,
mcp_auth_header=None,
mcp_servers=["doesnotexist"],
)
unknown = await _denied_scoped_list(
user_api_key_auth, ["github"], _denied_scope_manager({}), _scope_resolver(resolved_without_agent=[])
)
unauthorized = await _denied_scoped_list(
user_api_key_auth,
["github"],
_denied_scope_manager({"github": "srv-github"}),
_scope_resolver(resolved_without_agent=[]),
)
assert result.tools == []
assert result.outcomes == {}
mock_manager.get_allowed_mcp_servers.assert_not_awaited()
assert unknown.status_code == unauthorized.status_code == 403
assert unknown.detail["error"] == unauthorized.detail["error"]
assert "github" in unknown.detail["error"]
assert "agent" not in unknown.detail["error"]
@pytest.mark.asyncio
async def test_scoped_list_access_group_vetoed_by_agent_names_agent_and_group():
"""An access-group scope vetoed by the agent binding raises the 403 naming the agent and the
group instead of the silent empty list."""
pytest.importorskip("litellm.proxy._experimental.mcp_server.server")
user_api_key_auth = UserAPIKeyAuth(api_key="test_key", user_id="test_user", agent_id="agent-123")
denial = await _denied_scoped_list(
user_api_key_auth,
["prod-group"],
_denied_scope_manager({}),
_scope_resolver(resolved_without_agent=["srv-github"]),
)
assert denial.status_code == 403
message = denial.detail["error"]
assert "access group 'prod-group'" in message
assert "agent 'agent-123'" in message
assert "mcp_access_groups" in message
@pytest.mark.asyncio
async def test_scoped_list_agent_key_denied_by_key_grants_raises_generic_403():
"""When the agent-stripped rerun still denies the server, the denial is not the agent's doing,
so the 403 stays generic instead of blaming the agent binding."""
try:
from litellm.proxy._experimental.mcp_server.server import _get_tools_from_mcp_servers
except ImportError:
pytest.skip("MCP server not available")
"""When the agent-stripped rerun still resolves nothing, the 403 stays generic instead of
blaming the agent binding."""
pytest.importorskip("litellm.proxy._experimental.mcp_server.server")
user_api_key_auth = UserAPIKeyAuth(api_key="test_key", user_id="test_user", agent_id="agent-123")
mock_manager = _denied_scope_manager({"github": "srv-github"}, allowed_without_agent=[])
resolver = _scope_resolver(resolved_without_agent=[])
with (
patch( # test-quality-ok: the permission resolver is a module-level function; the suite's only seam
"litellm.proxy._experimental.mcp_server.server._get_allowed_mcp_servers",
AsyncMock(return_value=[]),
),
patch( # test-quality-ok: the server registry is a module-level singleton; the suite's only seam
"litellm.proxy._experimental.mcp_server.server.global_mcp_server_manager",
mock_manager,
),
):
with pytest.raises(HTTPException) as exc_info:
await _get_tools_from_mcp_servers(
user_api_key_auth=user_api_key_auth,
mcp_auth_header=None,
mcp_servers=["github"],
)
denial = await _denied_scoped_list(
user_api_key_auth, ["github"], _denied_scope_manager({"github": "srv-github"}), resolver
)
assert exc_info.value.status_code == 403
message = exc_info.value.detail["error"]
assert denial.status_code == 403
message = denial.detail["error"]
assert "github" in message
assert "agent" not in message
mock_manager.get_allowed_mcp_servers.assert_awaited_once()
assert resolver.await_count == 2
@pytest.mark.asyncio

2
type-discipline-budget.json
View file

@ -1,6 +1,6 @@
{
"LIT001": {
"limit": 22367
"limit": 22365
},
"LIT002": {
"limit": 26777

10
ui/litellm-dashboard/src/app/(dashboard)/agents/_components/agent_config.ts
View file

@ -313,25 +313,23 @@ export const buildAgentDataFromForm = (values: any, existingAgent?: any) => {
return agentData;
};
/**
* Parse MCP grants from an agent's object_permission into the shared MCP form fields
*/
export const parseMcpPermissionsForForm = (agent: any) => ({
allowed_mcp_servers_and_groups: {
servers: agent.object_permission?.mcp_servers ?? [],
accessGroups: agent.object_permission?.mcp_access_groups ?? [],
toolsets: agent.object_permission?.mcp_toolsets ?? [],
},
mcp_tool_permissions: agent.object_permission?.mcp_tool_permissions ?? {},
});
/**
* Build the object_permission payload from the shared MCP form fields.
* Always includes the MCP keys (empty when cleared) so removals persist;
* the proxy merges per key, leaving non-MCP grants untouched.
* Always includes every MCP key (empty when cleared) so removals persist;
* the proxy merges object_permission per key, leaving non-MCP grants untouched.
*/
export const buildMcpObjectPermission = (values: any) => ({
mcp_servers: values.allowed_mcp_servers_and_groups?.servers ?? [],
mcp_access_groups: values.allowed_mcp_servers_and_groups?.accessGroups ?? [],
mcp_toolsets: values.allowed_mcp_servers_and_groups?.toolsets ?? [],
mcp_tool_permissions: values.mcp_tool_permissions ?? {},
});

24
ui/litellm-dashboard/src/app/(dashboard)/agents/_components/agent_info.integration.test.tsx
View file

@ -141,7 +141,7 @@ describe("AgentInfoView update payload", () => {
rpm_limit: 222,
session_tpm_limit: 333,
session_rpm_limit: 444,
object_permission: { mcp_servers: [], mcp_access_groups: [], mcp_tool_permissions: {} },
object_permission: { mcp_servers: [], mcp_access_groups: [], mcp_toolsets: [], mcp_tool_permissions: {} },
});
});
@ -182,7 +182,7 @@ describe("AgentInfoView update payload", () => {
rpm_limit: 222,
session_tpm_limit: 333,
session_rpm_limit: 444,
object_permission: { mcp_servers: [], mcp_access_groups: [], mcp_tool_permissions: {} },
object_permission: { mcp_servers: [], mcp_access_groups: [], mcp_toolsets: [], mcp_tool_permissions: {} },
});
});
@ -260,18 +260,20 @@ describe("AgentInfoView update payload", () => {
api_base: "https://other.example.com",
model: "langgraph/asst_1",
},
object_permission: { mcp_servers: [], mcp_access_groups: [], mcp_tool_permissions: {} },
object_permission: { mcp_servers: [], mcp_access_groups: [], mcp_toolsets: [], mcp_tool_permissions: {} },
});
});
it("keeps the agent's existing MCP grants in the update payload", async () => {
const existingMcpGrants = {
mcp_servers: ["srv-1"],
mcp_access_groups: ["grp-a"],
mcp_toolsets: ["toolset-1"],
mcp_tool_permissions: { "srv-1": ["tool_x"] },
};
vi.mocked(networking.getAgentInfo).mockResolvedValue({
...A2A_AGENT,
object_permission: {
mcp_servers: ["srv-1"],
mcp_access_groups: ["grp-a"],
mcp_tool_permissions: { "srv-1": ["tool_x"] },
},
object_permission: existingMcpGrants,
} as never);
const user = setup();
renderView();
@ -279,11 +281,7 @@ describe("AgentInfoView update payload", () => {
await save(user);
expect(patchedPayload().object_permission).toEqual({
mcp_servers: ["srv-1"],
mcp_access_groups: ["grp-a"],
mcp_tool_permissions: { "srv-1": ["tool_x"] },
});
expect(patchedPayload().object_permission).toEqual(existingMcpGrants);
});
it("reloads the agent and leaves edit mode when the edit is cancelled", async () => {

3
ui/litellm-dashboard/src/app/(dashboard)/agents/_components/agent_info.test.tsx
View file

@ -74,7 +74,8 @@ describe("AgentInfoView settings", () => {
expect(token).toBe("sk-test");
expect(agentId).toBe("agent-1");
expect(payload.tpm_limit).toBe(42);
expect(payload.object_permission).toEqual({ mcp_servers: [], mcp_access_groups: [], mcp_tool_permissions: {} });
const clearedMcpGrants = { mcp_servers: [], mcp_access_groups: [], mcp_toolsets: [], mcp_tool_permissions: {} };
expect(payload.object_permission).toEqual(clearedMcpGrants);
});
it("shows MCP grants with server names on the overview tab", async () => {

5
ui/litellm-dashboard/src/app/(dashboard)/agents/_components/agent_info.tsx
View file

@ -359,6 +359,7 @@ const AgentInfoView: React.FC<AgentInfoViewProps> = ({ agentId, onClose, accessT
{agent.object_permission &&
(agent.object_permission.mcp_servers?.length ||
agent.object_permission.mcp_access_groups?.length ||
agent.object_permission.mcp_toolsets?.length ||
(agent.object_permission.mcp_tool_permissions &&
Object.keys(agent.object_permission.mcp_tool_permissions).length > 0)) && (
<div style={{ marginTop: 24 }}>
@ -379,6 +380,9 @@ const AgentInfoView: React.FC<AgentInfoViewProps> = ({ agentId, onClose, accessT
{agent.object_permission.mcp_access_groups.join(", ")}
</DetailItem>
)}
{agent.object_permission.mcp_toolsets && agent.object_permission.mcp_toolsets.length > 0 && (
<DetailItem label="MCP Toolsets">{agent.object_permission.mcp_toolsets.join(", ")}</DetailItem>
)}
{agent.object_permission.mcp_tool_permissions &&
Object.keys(agent.object_permission.mcp_tool_permissions).length > 0 && (
<DetailItem label="Tool permissions per server">
@ -501,6 +505,7 @@ const AgentInfoView: React.FC<AgentInfoViewProps> = ({ agentId, onClose, accessT
value={{
servers: (value as McpServerSelection | undefined)?.servers ?? [],
accessGroups: (value as McpServerSelection | undefined)?.accessGroups ?? [],
toolsets: (value as McpServerSelection | undefined)?.toolsets ?? [],
}}
accessToken={accessToken ?? ""}
placeholder="Select MCP servers or access groups (optional)"

2
ui/litellm-dashboard/src/lib/http/schema.d.ts generated vendored
View file

@ -22873,6 +22873,8 @@ export interface components {
mcp_tool_permissions?: {
[key: string]: string[];
} | null;
/** Mcp Toolsets */
mcp_toolsets?: string[] | null;
/** Models */
models?: string[] | null;
};