fix(mcp): add auth check on toolset resolution in responses API; union mcp_servers in _merge_toolset_permissions

This commit is contained in:
Ishaan Jaffer 2026-03-23 10:54:57 -07:00
parent e9c4c71a1f
commit 97961237f1
2 changed files with 29 additions and 2 deletions

View file

@ -1497,8 +1497,14 @@ if MCP_AVAILABLE:
merged = list(set(existing_tools) | set(tool_names))
existing[server_id] = merged
# Build updated object_permission with merged tool permissions
updated_op = op.model_copy(update={"mcp_tool_permissions": existing})
# Build updated object_permission with merged tool permissions and server IDs.
# Union the toolset's server IDs into mcp_servers so downstream server-level
# filtering doesn't silently drop servers that the toolset references but that
# aren't already in the key's explicit mcp_servers list.
merged_servers = list(set(op.mcp_servers or []) | set(existing.keys()))
updated_op = op.model_copy(
update={"mcp_servers": merged_servers, "mcp_tool_permissions": existing}
)
return user_api_key_auth.model_copy(update={"object_permission": updated_op})
async def _list_mcp_tools(

View file

@ -177,6 +177,27 @@ class LiteLLM_Proxy_MCP_Handler:
)
)
if toolset is not None:
# Access control: only allow if the key explicitly grants this toolset.
if user_api_key_auth is not None:
is_admin = (
getattr(user_api_key_auth, "user_role", None)
== "proxy_admin"
)
if not is_admin:
op = user_api_key_auth.object_permission
granted = (
getattr(op, "mcp_toolsets", None)
if op
else None
)
if (
granted is not None
and toolset.toolset_id not in granted
):
verbose_logger.debug(
f"Key does not have access to toolset '{name}', skipping."
)
continue
resolved_toolset_ids.append(toolset.toolset_id)
# Don't add to resolved_mcp_servers — toolset scope
# restricts via object_permission, not server name filter.