scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-10-09 03:18:44 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 109

fix: revert out-of-scope changes, fix remaining review feedback

Browse source 
- Revert MCP management endpoint changes: team-scoped CRUD depends on
  check_member_permission, add_mcp_server_to_team, remove_mcp_server_from_team
  which exist only in the base branch, not main. Importing them crashes
  all MCP endpoints on main.
- Revert ObjectPermissionTable default changes for pre-existing fields
  (mcp_servers, mcp_access_groups, vector_stores, agents, agent_access_groups)
  back to [] to avoid backwards-incompatible behavior change. Only
  search_tools (new field) uses None default.
- Restore vector store access check: [] = allow all (existing behavior)
- Restore vector store test assertion
- Revert common_utils.py inline import changes (infrastructure not in scope)
- Add ValueError fallback to get_search_tool_access_error_type_for_object
  for unrecognized object_type values

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
yuneng-jiang 2026-03-21 09:42:49 -07:00
parent d1a0b94919
commit 9019207111
5 changed files with 68 additions and 180 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
litellm/proxy/_types.py
View file

@ -1831,8 +1831,8 @@ class LiteLLM_ObjectPermissionTable(LiteLLMPydanticObjectBase):
"""Represents a LiteLLM_ObjectPermissionTable record"""
object_permission_id: str
mcp_servers: Optional[List[str]] = None
mcp_access_groups: Optional[List[str]] = None
mcp_servers: Optional[List[str]] = []
mcp_access_groups: Optional[List[str]] = []
mcp_tool_permissions: Optional[Dict[str, List[str]]] = None
"""
Mapping - server_id -> list of tools
@ -1843,10 +1843,10 @@ class LiteLLM_ObjectPermissionTable(LiteLLMPydanticObjectBase):
}
"""
vector_stores: Optional[List[str]] = None
search_tools: Optional[List[str]] = None
agents: Optional[List[str]] = None
agent_access_groups: Optional[List[str]] = None
vector_stores: Optional[List[str]] = []
search_tools: Optional[List[str]] = None # NULL = all access, [] = no access
agents: Optional[List[str]] = []
agent_access_groups: Optional[List[str]] = []
class LiteLLM_TeamTable(TeamBase):
@ -3599,6 +3599,9 @@ class ProxyErrorTypes(str, enum.Enum):
return cls.team_search_tool_access_denied
elif object_type == "org":
return cls.org_search_tool_access_denied
raise ValueError(
f"Unknown object_type '{object_type}' for search tool access error"
)
DB_CONNECTION_ERROR_TYPES = (

11
litellm/proxy/auth/auth_checks.py
View file

@ -3543,16 +3543,9 @@ def _can_object_call_vector_stores(
if object_permissions.vector_stores is None:
return True
# Empty list = no access to any vector stores
# If length is 0, then the object has access to all vector stores.
if len(object_permissions.vector_stores) == 0:
raise ProxyException(
message="User not allowed to access any vector stores. No vector stores are configured in object permissions.",
type=ProxyErrorTypes.get_vector_store_access_error_type_for_object(
object_type
),
param="vector_store",
code=status.HTTP_401_UNAUTHORIZED,
)
return True
for vector_store_id in vector_store_ids_to_run:
if vector_store_id not in object_permissions.vector_stores:

12
litellm/proxy/management_endpoints/common_utils.py
View file

@ -15,7 +15,6 @@ from litellm.proxy._types import (
UpdateProjectRequest,
UserAPIKeyAuth,
)
from litellm.proxy.auth.auth_checks import get_user_object
from litellm.proxy.utils import _premium_user_check
if TYPE_CHECKING:
@ -55,6 +54,7 @@ async def _is_user_org_admin_for_team(
if not team_obj.organization_id or not user_api_key_dict.user_id:
return False
from litellm.proxy.auth.auth_checks import get_user_object
from litellm.proxy.proxy_server import (
prisma_client,
proxy_logging_obj,
@ -124,11 +124,14 @@ async def _user_has_admin_privileges(
return False
# Get user object to check team and org admin status
from litellm.caching import DualCache as DualCacheImport
from litellm.proxy.auth.auth_checks import get_user_object
try:
user_obj = await get_user_object(
user_id=user_api_key_dict.user_id,
prisma_client=prisma_client,
user_api_key_cache=user_api_key_cache or DualCache(),
user_api_key_cache=user_api_key_cache or DualCacheImport(),
user_id_upsert=False,
proxy_logging_obj=proxy_logging_obj,
)
@ -270,8 +273,11 @@ async def admin_can_invite_user(
if prisma_client is None or user_api_key_dict.user_id is None:
return False
from litellm.caching import DualCache as DualCacheImport
from litellm.proxy.auth.auth_checks import get_user_object
try:
cache = user_api_key_cache or DualCache()
cache = user_api_key_cache or DualCacheImport()
admin_user_obj = await get_user_object(
user_id=user_api_key_dict.user_id,
prisma_client=prisma_client,

191
litellm/proxy/management_endpoints/mcp_management_endpoints.py
View file

@ -130,15 +130,7 @@ if MCP_AVAILABLE:
)
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
from litellm.proxy.common_utils.http_parsing_utils import _read_request_body
from litellm.proxy.management_endpoints.common_utils import (
_user_has_admin_view,
check_member_permission,
)
from litellm.proxy.management_helpers.object_permission_utils import (
_get_team_allowed_mcp_servers,
add_mcp_server_to_team,
remove_mcp_server_from_team,
)
from litellm.proxy.management_endpoints.common_utils import _user_has_admin_view
from litellm.proxy.management_helpers.utils import management_endpoint_wrapper
from litellm.types.mcp import MCPCredentials
from litellm.types.mcp_server.mcp_server_manager import MCPServer
@ -1214,50 +1206,15 @@ if MCP_AVAILABLE:
# Validate and normalize payload fields
validate_and_normalize_mcp_server_payload(payload)
# AuthZ - proxy admins, team admins, or members with mcp:create permission
from litellm.proxy.auth.auth_checks import get_team_object
from litellm.proxy.proxy_server import user_api_key_cache
team_obj = None
team_id = payload.team_id or user_api_key_dict.team_id
# AuthZ - restrict only proxy admins to create mcp servers
if LitellmUserRoles.PROXY_ADMIN != user_api_key_dict.user_role:
if not team_id:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={
"error": "team_id is required for non-proxy-admin users to create MCP servers."
},
)
team_obj = await get_team_object(
team_id=team_id,
prisma_client=prisma_client,
user_api_key_cache=user_api_key_cache,
)
if not check_member_permission(
user_api_key_dict, team_obj, "mcp:create"
):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail={
"error": "User does not have permission to create MCP servers for this team. "
"Requires team admin role or 'mcp:create' permission."
},
)
# Reject special reserved server names
if (
SpecialMCPServerName.all_team_servers == payload.server_id
or SpecialMCPServerName.all_proxy_servers == payload.server_id
):
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
status_code=status.HTTP_403_FORBIDDEN,
detail={
"error": f"MCP Server with id {payload.server_id} is special and cannot be used."
"error": "User does not have permission to create mcp servers. You can only create mcp servers if you are a PROXY_ADMIN."
},
)
if payload.server_id is not None:
elif payload.server_id is not None:
# fail if the mcp server with id already exists
mcp_server = await get_mcp_server(prisma_client, payload.server_id)
if mcp_server is not None:
@ -1267,6 +1224,16 @@ if MCP_AVAILABLE:
"error": f"MCP Server with id {payload.server_id} already exists. Cannot create another."
},
)
elif (
SpecialMCPServerName.all_team_servers == payload.server_id
or SpecialMCPServerName.all_proxy_servers == payload.server_id
):
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={
"error": f"MCP Server with id {payload.server_id} is special and cannot be used."
},
)
# TODO: audit log for create
@ -1294,20 +1261,6 @@ if MCP_AVAILABLE:
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail={"error": f"Error creating mcp server: {str(e)}"},
)
# Auto-assign server to team's ObjectPermissionTable if team-scoped
if team_id and new_mcp_server.server_id:
try:
await add_mcp_server_to_team(
prisma_client, team_id, new_mcp_server.server_id
)
except ValueError as e:
# Team not found — surface as 400 so caller knows
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={"error": str(e)},
)
return _redact_mcp_credentials(new_mcp_server)
@router.post(
@ -1492,7 +1445,7 @@ if MCP_AVAILABLE:
@router.delete(
"/server/{server_id}",
description="Allows deleting mcp servers in the db",
description="Allows deleting mcp serves in the db",
dependencies=[Depends(user_api_key_auth)],
response_class=JSONResponse,
status_code=status.HTTP_202_ACCEPTED,
@ -1520,44 +1473,16 @@ if MCP_AVAILABLE:
"Database not connected. Connect a database to your proxy - https://docs.litellm.ai/docs/simple_proxy#managing-auth---virtual-keys"
)
# AuthZ - proxy admins, team admins, or members with mcp:delete permission
from litellm.proxy.auth.auth_checks import get_team_object
from litellm.proxy.proxy_server import user_api_key_cache
team_id = user_api_key_dict.team_id
# Authz - restrict only admins to delete mcp servers
if LitellmUserRoles.PROXY_ADMIN != user_api_key_dict.user_role:
if not team_id:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={
"error": "team_id is required for non-proxy-admin users to delete MCP servers."
},
)
team_obj = await get_team_object(
team_id=team_id,
prisma_client=prisma_client,
user_api_key_cache=user_api_key_cache,
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail={
"error": "Call not allowed to delete MCP server. User is not a proxy admin. route={}".format(
"DELETE /v1/mcp/server"
)
},
)
if not check_member_permission(
user_api_key_dict, team_obj, "mcp:delete"
):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail={
"error": "User does not have permission to delete MCP servers for this team. "
"Requires team admin role or 'mcp:delete' permission."
},
)
# Verify server belongs to this team
team_servers = await _get_team_allowed_mcp_servers(team_obj)
if server_id not in team_servers:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail={
"error": f"MCP Server {server_id} does not belong to your team."
},
)
# try to delete the mcp server
mcp_server_record_deleted = await delete_mcp_server(prisma_client, server_id)
@ -1572,20 +1497,16 @@ if MCP_AVAILABLE:
# Ensure registry is up to date by reloading from database
await global_mcp_server_manager.reload_servers_from_database()
# Remove server from team's ObjectPermissionTable
if team_id:
try:
await remove_mcp_server_from_team(prisma_client, team_id, server_id)
except Exception as e:
verbose_proxy_logger.warning(
f"Failed to remove server {server_id} from team {team_id} permissions: {e}. "
"Server was deleted but team's ObjectPermissionTable may contain a stale entry."
)
# TODO: Enterprise: Finish audit log trail
if litellm.store_audit_logs:
pass
# TODO: Delete from virtual keys
# TODO: Delete from teams
# Update from global mcp store
return Response(status_code=status.HTTP_202_ACCEPTED)
@router.post(
@ -1846,7 +1767,7 @@ if MCP_AVAILABLE:
@router.put(
"/server",
description="Allows updating mcp servers in the db",
description="Allows deleting mcp serves in the db",
dependencies=[Depends(user_api_key_auth)],
response_model=LiteLLM_MCPServerTable,
status_code=status.HTTP_202_ACCEPTED,
@ -1877,50 +1798,16 @@ if MCP_AVAILABLE:
# Validate and normalize payload fields
validate_and_normalize_mcp_server_payload(payload)
# AuthZ - proxy admins, team admins, or members with mcp:update permission
from litellm.proxy.auth.auth_checks import get_team_object
from litellm.proxy.proxy_server import user_api_key_cache
# Authz - restrict only admins to delete mcp servers
if LitellmUserRoles.PROXY_ADMIN != user_api_key_dict.user_role:
team_id = user_api_key_dict.team_id
if not team_id:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={
"error": "team_id is required for non-proxy-admin users to update MCP servers."
},
)
team_obj = await get_team_object(
team_id=team_id,
prisma_client=prisma_client,
user_api_key_cache=user_api_key_cache,
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail={
"error": "Call not allowed to update MCP server. User is not a proxy admin. route={}".format(
"PUT /v1/mcp/server"
)
},
)
if not check_member_permission(
user_api_key_dict, team_obj, "mcp:update"
):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail={
"error": "User does not have permission to update MCP servers for this team. "
"Requires team admin role or 'mcp:update' permission."
},
)
# Verify server_id is provided and belongs to this team
if payload.server_id is None:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail={
"error": "server_id is required to update an MCP server."
},
)
team_servers = await _get_team_allowed_mcp_servers(team_obj)
if payload.server_id not in team_servers:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail={
"error": f"MCP Server {payload.server_id} does not belong to your team."
},
)
# try to update the mcp server
mcp_server_record_updated = await update_mcp_server(

19
tests/test_litellm/proxy/auth/test_search_tool_access.py
View file

@ -13,7 +13,6 @@ from typing import List, Optional
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from starlette import status
from litellm.proxy._types import (
LiteLLM_ObjectPermissionTable,
@ -376,18 +375,18 @@ class TestSearchToolErrorTypes:
class TestVectorStoreAccessNotBroken:
"""Vector store access follows same semantics: None = all access, [] = no access."""
"""Preserves existing vector store semantics: empty list = allow ALL."""
def test_should_deny_all_when_vector_stores_is_empty(self):
"""Vector stores: empty list = no access (matches search_tools semantics)."""
def test_should_allow_all_when_vector_stores_is_empty(self):
"""Vector stores: empty list = access to ALL (existing behavior)."""
perm = MagicMock()
perm.vector_stores = []
with pytest.raises(ProxyException):
_can_object_call_vector_stores(
object_type="key",
vector_store_ids_to_run=["store-1"],
object_permissions=perm,
)
result = _can_object_call_vector_stores(
object_type="key",
vector_store_ids_to_run=["store-1"],
object_permissions=perm,
)
assert result is True
def test_should_allow_when_vector_stores_is_none(self):
perm = MagicMock()