mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-05 02:23:42 +00:00
fix: include mcp_tool_permissions server ids in allowed mcp servers (#22311)
when a key/team/end-user has mcp_tool_permissions for a server but that server is not in mcp_servers, the server was excluded from the allowed list — making the tool permissions useless. now we union the keys from mcp_tool_permissions into the allowed server set alongside direct servers and access group servers. fixes #21954
This commit is contained in:
committed by
Sameer Kankute
parent
fca08e8acc
commit
64077553ec
@@ -649,8 +649,13 @@ class MCPRequestHandler:
|
||||
)
|
||||
)
|
||||
|
||||
# Combine both lists
|
||||
all_servers = direct_mcp_servers + access_group_servers
|
||||
# servers referenced in tool permissions should also be accessible
|
||||
tool_perm_servers = list(
|
||||
(key_object_permission.mcp_tool_permissions or {}).keys()
|
||||
)
|
||||
|
||||
# Combine all lists
|
||||
all_servers = direct_mcp_servers + access_group_servers + tool_perm_servers
|
||||
return list(set(all_servers))
|
||||
except Exception as e:
|
||||
verbose_logger.warning(
|
||||
@@ -686,8 +691,13 @@ class MCPRequestHandler:
|
||||
)
|
||||
)
|
||||
|
||||
# Combine both lists
|
||||
all_servers = direct_mcp_servers + access_group_servers
|
||||
# servers referenced in tool permissions should also be accessible
|
||||
tool_perm_servers = list(
|
||||
(object_permissions.mcp_tool_permissions or {}).keys()
|
||||
)
|
||||
|
||||
# Combine all lists
|
||||
all_servers = direct_mcp_servers + access_group_servers + tool_perm_servers
|
||||
return list(set(all_servers))
|
||||
except Exception as e:
|
||||
verbose_logger.warning(
|
||||
@@ -737,8 +747,6 @@ class MCPRequestHandler:
|
||||
# Get direct MCP servers
|
||||
direct_mcp_servers = end_user_obj.object_permission.mcp_servers or []
|
||||
|
||||
|
||||
|
||||
# Get MCP servers from access groups
|
||||
access_group_servers = (
|
||||
await MCPRequestHandler._get_mcp_servers_from_access_groups(
|
||||
@@ -746,8 +754,13 @@ class MCPRequestHandler:
|
||||
)
|
||||
)
|
||||
|
||||
# Combine both lists
|
||||
all_servers = direct_mcp_servers + access_group_servers
|
||||
# servers referenced in tool permissions should also be accessible
|
||||
tool_perm_servers = list(
|
||||
(end_user_obj.object_permission.mcp_tool_permissions or {}).keys()
|
||||
)
|
||||
|
||||
# Combine all lists
|
||||
all_servers = direct_mcp_servers + access_group_servers + tool_perm_servers
|
||||
return list(set(all_servers))
|
||||
except Exception as e:
|
||||
verbose_logger.warning(
|
||||
|
||||
@@ -1738,3 +1738,34 @@ class TestAgentMCPPermissions:
|
||||
user_api_key_auth=user_api_key_auth,
|
||||
)
|
||||
assert sorted(result) == ["tool_a", "tool_b"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_tool_permission_servers_included_in_allowed_servers():
|
||||
"""
|
||||
Servers listed only in mcp_tool_permissions (not in mcp_servers)
|
||||
should still be accessible.
|
||||
|
||||
Regression test for https://github.com/BerriAI/litellm/issues/21954
|
||||
"""
|
||||
perm = MagicMock()
|
||||
perm.mcp_servers = []
|
||||
perm.mcp_access_groups = []
|
||||
perm.mcp_tool_permissions = {"server_id_123": ["tool_a", "tool_b"]}
|
||||
|
||||
user_api_key_auth = UserAPIKeyAuth(
|
||||
api_key="test-key",
|
||||
user_id="test-user",
|
||||
)
|
||||
|
||||
with patch.object(
|
||||
MCPRequestHandler, "_get_key_object_permission", return_value=perm
|
||||
), patch.object(
|
||||
MCPRequestHandler, "_get_mcp_servers_from_access_groups",
|
||||
new_callable=AsyncMock,
|
||||
return_value=[],
|
||||
):
|
||||
result = await MCPRequestHandler._get_allowed_mcp_servers_for_key(
|
||||
user_api_key_auth=user_api_key_auth,
|
||||
)
|
||||
assert "server_id_123" in result
|
||||
|
||||
Reference in New Issue
Block a user