[Infra] Merge internal dev branch with main#24332
[Infra] Merge internal dev branch with main#24332yuneng-jiang wants to merge 67 commits intomainfrom
Conversation
- Add search_tools String[] column to LiteLLM_ObjectPermissionTable in all 3 Prisma schema files - Add Prisma migration for the new column - Add search_tools field to LiteLLM_ObjectPermissionBase and LiteLLM_ObjectPermissionTable Python types - Add ProxyErrorTypes for search tool access denied (key, team, org) - Add get_search_tool_access_error_type_for_object() classmethod to ProxyErrorTypes Co-authored-by: yuneng-jiang <yuneng-jiang@users.noreply.github.com>
- Add _can_object_call_search_tools() with least-privilege semantics (empty list = no access) - Add search_tool_access_check() that checks both key-level and team-level permissions - Add get_permitted_search_tool_names() helper for filtering search tool listings Co-authored-by: yuneng-jiang <yuneng-jiang@users.noreply.github.com>
- Add access check in search() endpoint after resolving search_tool_name - Add _get_allowed_search_tool_names() helper for computing allowed tools - Filter list_search_tools() results based on key/team permissions - Least privilege: empty search_tools list means no access Co-authored-by: yuneng-jiang <yuneng-jiang@users.noreply.github.com>
- Test _can_object_call_search_tools() with least-privilege semantics - Test search_tool_access_check() for key/team level permissions - Test get_permitted_search_tool_names() helper - Test ProxyErrorTypes for search tool access - Regression tests ensuring vector store semantics unchanged Co-authored-by: yuneng-jiang <yuneng-jiang@users.noreply.github.com>
Add tests for ResponseTimeIndicator, KeyValueInput, QueryParamInput, RoutePreview, OnboardingModal, EditUserModal, ModelFilters, AuditLogDrawer, PassThroughInfoView, and EmailSettings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lderPage Replace useEffect + useState fetch pattern for policy version management with React Query hooks (useQuery + useMutation), following established codebase conventions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Allow team admins and permissioned members to manage MCP servers scoped to their team, laying groundwork for full Permission Strings RBAC. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Without this, internal users get 401 when calling the endpoint. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
team_id is a request-level field for auth scoping, not a DB column. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ensures permission changes take effect immediately instead of waiting for cache TTL expiry. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- PUT /v1/mcp/server: 202 → 200 (synchronous update returns body)
- DELETE /v1/mcp/server: 202 → 204 (synchronous delete, no body)
- Also fix description typo ("deleting" → "updating") on PUT endpoint
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Wrap add/remove_mcp_server_to_team in DB transactions (race condition fix) - Consolidate role + extra_permissions into single DB write (atomicity) - Remove silent try/except on team linking (surface errors to caller) - Add email fallback to _find_member_in_team (email-only members) - Add None guard on payload.server_id in update endpoint - Add field_validator on Member.extra_permissions (resource:action format) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Handle ValueError on team-link failure as 400 (orphaned server fix) - Wrap delete's remove_from_team in try/except (prevent 500 after successful delete) - Revert DELETE status code to 202 (backwards-compatible) - Add extra_permissions to TeamMemberUpdateResponse Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Revert PUT /v1/mcp/server status code to 202 (backwards-compatible) - Strengthen Member.extra_permissions validator to check VALID_PERMISSIONS - Invalidate team cache after add/remove_mcp_server_to_team Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move extra_permissions validation before budget upsert to prevent partial DB writes on validation failure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…emantics - Wrap mutateAsync calls in try/catch to swallow re-thrown errors (notifications already handled by onError in mutation hooks) - Use isLoading instead of isPending for version loading state — isPending is true when query is disabled with no cache, isLoading is only true during active fetches (matches original behavior) - Add isLoading assertions to disabled-state tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
[Feature] MCP team management + per-member granular permissions
…rsions - Make PolicyVersionsResponse.versions optional (Policy[] | undefined) to match real API shape — select fallback handles normalization - Add policyName guard to useUpdatePolicyVersionStatus mutationFn to fail loudly instead of silently skipping cache invalidation - Add test for null policyName in updateStatus mutation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add PolicyVersionsData type for select output; specify TData generic so consumers get Policy[] (not Policy[] | undefined) for versions - Remove empty-string queryKey fallback — use policyName! since enabled:false prevents fetch when policyName is null - Add cache invalidation tests for both mutation hooks - Add explanatory comment for ?? [] fallback in component Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move callbacks outside try/catch so only mutation errors are caught, not errors from onVersionCreated/onVersionStatusUpdated callbacks - Replace policyName! non-null assertion with DISABLED_POLICY_KEY sentinel to avoid undefined in cache keys when query is disabled Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…sions_hook [Refactor] UI - Policies: Extract usePolicyVersions Hook
Add UI support for the MCP team management permissions introduced in PR #24266. - Add MemberPermissionsDrawer component (Ant Design Drawer) for managing per-member MCP permissions (mcp:read, mcp:create, mcp:update, mcp:delete) - Add permissions button to team member table actions column - Fetch available permissions from GET /team/available_permissions - Pass extra_permissions in team member update API calls - Allow all users to create MCP servers directly (backend enforces permissions) - Pass team_id when non-admin users create MCP servers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Type onUpdate as () => Promise<void> and await it before closing drawer - Replace accessToken! assertion with explicit null guard - Gate fetchAvailableTeamMemberPermissions behind canEditTeam check - Pass team_id for admins too when a team is selected in the filter Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Separate save failure from refresh failure: close drawer after successful save even if teamInfoCall refresh fails - Preserve unknown permissions not in availablePermissions when saving, preventing silent drops of permissions from newer backend versions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix unknown permissions duplication: seed selected state with only known permissions so existingUnknown and selected are disjoint on save - Disable Add MCP Server button for non-admins without a team selected, show tooltip explaining they need to select a team first Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix double error handling in getTeamPermissionsCall: return empty data on HTTP error instead of calling handleError + throwing, preventing duplicate error notifications Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix double error handling in getTeamPermissionsCall: return empty data on HTTP error instead of calling handleError + throwing, preventing duplicate error notifications Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…scoped MCP RBAC - Change all ObjectPermissionTable array field defaults from [] to None (None = all access / no restriction, [] = no access) - Update vector store access check: [] now denies access instead of allowing all - Add team-scoped MCP server management (create/update/delete) with granular permissions (mcp:create, mcp:update, mcp:delete) - Auto-assign created servers to team's ObjectPermissionTable - Auto-remove deleted servers from team's ObjectPermissionTable - Fix unreachable special MCP server name guard in add_mcp_server - Fix server_id validation ordering in edit_mcp_server - Fix description typo on PUT /server endpoint - Move inline imports to module level in common_utils.py (CLAUDE.md) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
[Feature] UI - MCP: Team member permissions drawer and CRUD access
- Migration: remove DEFAULT ARRAY[]::TEXT[] so existing rows get NULL (NULL = all access) instead of [] (no access) - Schema: remove @default([]) for search_tools in all 3 schema.prisma files - Use cached get_object_permission instead of raw DB queries in search_tool_access_check and _get_allowed_search_tool_names - Reject requests with missing search_tool_name (400) instead of silently bypassing access control - Fix vector store test: [] now correctly denies access (not allows) - Update search_tool_access_check tests to mock get_object_permission Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The early return on missing auth params blocked the entire page from rendering. The hooks already guard on accessToken being available, and DataTable shows its own loading state via isLoading prop. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Added team_id to the MCPServer model class, build_mcp_server_from_table, and _build_mcp_server_table so team_id flows from DB → registry → list endpoint responses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Removed low-value columns (Alias, Auth Type, Updated) to reduce horizontal overflow. Added explicit size hints to remaining columns. DataTable now applies column sizes to header and body cells. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add TeamDropdown as first field in create MCP server form - For internal users, hide form fields until team is selected - Fix useMCPServerHealth to append new servers to cache on recheck Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…th-routed pages The dashboard layout was using Sidebar2, which routed all entries to path-based URLs like /ui/keys — but only api-reference has been migrated. Switch back to the old leftnav so unmigrated pages navigate to the legacy root page (?page=X) and migrated pages use path routing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… header - Remove assertions for alias text (column was removed) - Disambiguate "Team" filter label from "Team (Owner)" column header Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Internal users can now see the create modal (with a team selection prompt). Updated the test from asserting the modal is hidden to asserting the team selection prompt is shown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests using MagicMock for MCPServerTable were missing the new team_id field, causing Pydantic validation errors. Updated auth failure test to expect 400 (team_id required) instead of 403 since non-admin users now must provide team_id before permission checks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
[Test] UI: Add vitest coverage for 10 untested components
…nite team dropdown Add search_tools permission support across the UI (key create/edit, team create, object permissions view) with breaking-change alerts for the new least-privilege default. Modernize the Search Tools page with AntD Tabs, a Test playground tab, and ProviderLogo integration. Migrate TeamDropdown to self-fetching infinite scroll pattern using useInfiniteTeams hook. Scope search tools visibility for internal users based on their team memberships. Backend: Add search_tools field to Prisma schema (all copies), Pydantic models (ObjectPermissionBase + ObjectPermissionTable), and allowed routes for virtual keys. Add permission filtering to /search_tools/list endpoint. Include object_permission in team list v2 queries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Add type casts for TypedDict .get() calls and rename loop variable to avoid type shadowing between config and DB search tool iterations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR wires Key changes:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/auth_checks.py | Adds _can_object_call_search_tools, search_tool_access_check, _normalize_search_tools_wildcard, and get_allowed_search_tool_names — well-structured with correct wildcard handling that addressed the previous-thread bugs. |
| litellm/proxy/management_endpoints/mcp_management_endpoints.py | Extends MCP create/update/delete to support team-scoped ownership and granular member permissions (mcp:create, mcp:update, mcp:delete). The admin-only guard on the discovery endpoint is silently removed — a backward-incompatible access-control change. |
| litellm/proxy/management_helpers/object_permission_utils.py | Adds transactional add_mcp_server_to_team and remove_mcp_server_from_team helpers with cache invalidation. CREATE path relies on DB default for search_tools implicitly rather than setting it explicitly — see inline comment. |
| ui/litellm-dashboard/src/components/organisms/create_key_button.tsx | Wires SearchToolSelector into key creation. New keys default to search_tools = [] (no access) — a backward-incompatible behavioral change communicated only via a UI alert with no server-side feature flag. |
| ui/litellm-dashboard/src/components/SearchTools/SearchTools.tsx | Modernized with AntD Tabs and a test playground. Client-side team-based filtering uses a hard-coded cap of 100 teams and duplicates server-side permission filtering already performed by the backend. |
| ui/litellm-dashboard/src/components/permissions/SearchToolPermissions.tsx | New component for displaying search tool permission badges. Contains a React hook bug: useEffect depends on searchTools.length instead of searchTools, causing stale display names when IDs change at constant length. |
| ui/litellm-dashboard/src/components/common_components/team_dropdown.tsx | Migrated from prop-driven to self-fetching infinite scroll (useInfiniteTeams). Removes the teams prop (now fetches internally), adds debounced server-side search, deduplication, and bottom-of-list page loading. Clean implementation. |
| litellm/proxy/management_endpoints/team_endpoints.py | Adds extra_permissions update in team_member_update with duplicate validation, adds GET /team/available_permissions endpoint, and includes object_permission in all team list queries. Validation is duplicated between endpoint and the Member model validator. |
| tests/test_litellm/proxy/auth/test_search_tool_access.py | New comprehensive mock-only test suite (509 lines) covering all branches of _can_object_call_search_tools, search_tool_access_check, wildcard normalization, and regression tests for unchanged vector store semantics. Well-structured. |
| litellm/proxy/search_endpoints/endpoints.py | Adds search_tool_access_check call in the search request path and get_allowed_search_tool_names filtering in the list endpoint. Clean integration; same filtering logic also applied independently in search_tool_management.py. |
Sequence Diagram
sequenceDiagram
participant UI as Dashboard UI
participant KE as Key/Team Create Endpoint
participant SE as /search_tools/list
participant SR as /search (search request)
participant AC as auth_checks.py
participant OPU as object_permission_utils.py
participant DB as Database
UI->>KE: Create Key (search_tools=[...])
KE->>DB: Upsert ObjectPermissionTable (search_tools=[...])
UI->>SE: GET /search_tools/list
SE->>AC: get_allowed_search_tool_names(user_api_key_dict)
AC->>DB: get_object_permission(key_perm_id)
AC->>DB: get_object_permission(team_perm_id)
AC-->>SE: allowed_names (intersection or None)
SE-->>UI: Filtered search tools list
UI->>SR: POST /search/{tool_name}
SR->>AC: search_tool_access_check(tool_name, token)
AC->>AC: _can_object_call_search_tools(key_perm)
AC->>AC: _can_object_call_search_tools(team_perm)
alt access denied
AC-->>SR: ProxyException (401)
SR-->>UI: 401 Unauthorized
else access granted
AC-->>SR: True
SR-->>UI: Search results
end
UI->>KE: Create MCP Server (team_id=...)
KE->>AC: check_member_permission(user, team, "mcp:create")
AC-->>KE: True/False
KE->>DB: Insert LiteLLM_MCPServerTable
KE->>OPU: add_mcp_server_to_team(prisma, team_id, server_id)
OPU->>DB: tx: upsert ObjectPermissionTable (mcp_servers=[...])
OPU->>DB: tx: update LiteLLM_TeamTable (object_permission_id)
OPU->>OPU: _invalidate_team_cache(team_id)
Comments Outside Diff (1)
-
litellm/proxy/management_endpoints/mcp_management_endpoints.py, line 2165-2172 (link)Backward-incompatible: discovery endpoint is now open to all authenticated users
The admin-only guard on
GET /mcp/server/discoverywas removed without a feature flag or documented migration path. Previously, any non-admin caller received a403; now they silently see the full server registry. This changes the security posture of the endpoint for all existing deployments the moment they upgrade, without any opt-in.Per the project rule on backward-incompatible changes, this should either be gated behind a config flag (
litellm.allow_non_admin_mcp_discovery = True) or documented as a breaking change in the release notes with explicit migration guidance.Rule Used: What: avoid backwards-incompatible changes without... (source)
Last reviewed commit: "[Fix] Search Tools: ..."
| if object_permissions.search_tools is None: | ||
| return True | ||
|
|
||
| # Empty list = no access (principle of least privilege) | ||
| # Non-empty list = only listed tools are accessible | ||
| if search_tool_name not in object_permissions.search_tools: | ||
| raise ProxyException( | ||
| message=f"User not allowed to access search tool '{search_tool_name}'. Allowed search tools: {object_permissions.search_tools}", | ||
| type=ProxyErrorTypes.get_search_tool_access_error_type_for_object( | ||
| object_type | ||
| ), | ||
| param="search_tool", | ||
| code=status.HTTP_401_UNAUTHORIZED, | ||
| ) | ||
|
|
||
| return True |
There was a problem hiding this comment.
Wildcard
"*" not handled — default permission blocks all search tools
The DB migration adds search_tools TEXT[] DEFAULT ARRAY['*']::TEXT[], meaning every existing LiteLLM_ObjectPermissionTable row will have search_tools = ["*"] after the migration runs. The intent (documented in the schema comments) is that ["*"] means "all access". However, this function never treats "*" specially, so:
"my-search-tool" not in ["*"] → True → ProxyException raised
All users whose keys/teams already have an ObjectPermissionTable record (e.g., because they use MCP servers) will silently lose all search-tool access after this migration is applied. This is a silent regression at migration time with no path to recovery other than manually clearing search_tools or patching the permission list.
The fix needs to short-circuit when "*" is in the allowed list:
if object_permissions.search_tools is None:
return True
# Wildcard: allow access to all tools
if "*" in object_permissions.search_tools:
return True
# Empty list or specific list without the tool → deny
if search_tool_name not in object_permissions.search_tools:
raise ProxyException(...)The same wildcard normalization must also be applied in _get_allowed_search_tool_names (see the companion comment on endpoints.py).
| if key_perm is not None: | ||
| key_allowed = key_perm.search_tools # None means no restriction | ||
|
|
||
| # Team-level permissions (via cached helper) | ||
| team_perm_id = getattr(user_api_key_dict, "team_object_permission_id", None) | ||
| if team_perm_id is not None: | ||
| team_perm = await get_object_permission( | ||
| object_permission_id=team_perm_id, | ||
| prisma_client=prisma_client, | ||
| user_api_key_cache=user_api_key_cache, | ||
| parent_otel_span=getattr(user_api_key_dict, "parent_otel_span", None), | ||
| proxy_logging_obj=proxy_logging_obj, | ||
| ) | ||
| if team_perm is not None: | ||
| team_allowed = team_perm.search_tools # None means no restriction | ||
|
|
||
| # Combine: both None → None (no restriction) | ||
| # One set → use that set | ||
| # Both set → intersection | ||
| if key_allowed is None and team_allowed is None: | ||
| return None | ||
| if key_allowed is None: | ||
| return team_allowed | ||
| if team_allowed is None: | ||
| return key_allowed | ||
| # Both are set - return the intersection | ||
| return list(set(key_allowed) & set(team_allowed)) |
There was a problem hiding this comment.
Wildcard
"*" not handled in list-filtering — returns empty results by default
Same root cause as the companion bug in auth_checks.py. When a permission record carries the default search_tools = ["*"], this function assigns key_allowed = ["*"] or team_allowed = ["*"]. That non-None list then flows into the filter:
if tool.get("search_tool_name") in ["*"] # always False for real tool namesSo the /search_tools/list endpoint returns an empty list for any caller whose key or team already has an ObjectPermissionTable with the default ["*"] value — a silent regression for every existing user with MCP server permissions.
The fix is to normalize ["*"] to None (meaning "no restriction") immediately after loading:
if key_perm is not None:
st = key_perm.search_tools
# ["*"] sentinel means unrestricted — treat as None
key_allowed = None if (st is None or "*" in st) else st
if team_perm_id is not None:
...
if team_perm is not None:
st = team_perm.search_tools
team_allowed = None if (st is None or "*" in st) else stWith this fix, ["*"] is treated consistently with None (both meaning "unrestricted"), the intersection logic produces None for all-access combinations, and the filter is only applied when a real, non-wildcard restriction is in place.
| ) | ||
|
|
||
| # Expect HTTPException to be raised | ||
| # Expect HTTPException — non-admin users must provide team_id | ||
| with pytest.raises(HTTPException) as exc_info: | ||
| await add_mcp_server( | ||
| payload=mcp_server_request, user_api_key_dict=user_auth | ||
| ) | ||
|
|
There was a problem hiding this comment.
Backward-incompatible status-code change in existing test
The original test asserted status_code == 403 with "permission" in the error body when a non-admin user attempted to create an MCP server. This PR changes it to 400 with "team_id is required".
This reflects a genuine API contract change: the add_mcp_server endpoint now returns 400 Bad Request instead of 403 Forbidden for non-admin users who do not supply a team_id. Any existing client that handles this error path by checking for 403 will break.
Per the project rule on avoiding backward-incompatible changes without user-controlled flags, this status-code flip needs either:
- a flag that preserves the
403response for deployments that have not opted in to the new team-scoped model, or - explicit documentation in release notes that the response code has changed for this scenario.
Additionally, the test now only exercises the "non-admin, no team_id" path (400). The "non-admin with team_id but without mcp:create permission" path (403) has no automated coverage.
Rule Used: What: avoid backwards-incompatible changes without... (source)
| updated_at=_convert_datetime_to_str(cast(Optional[Union[datetime, str]], db_tool.get("updated_at"))), | ||
| is_from_config=False, | ||
| ) | ||
| ) | ||
|
|
||
| # Filter based on caller's key/team permissions | ||
| from litellm.proxy.search_endpoints.endpoints import ( | ||
| _get_allowed_search_tool_names, | ||
| ) | ||
|
|
||
| allowed_names = await _get_allowed_search_tool_names(user_api_key_dict) | ||
| if allowed_names is not None: | ||
| search_tool_configs = [ | ||
| tool | ||
| for tool in search_tool_configs | ||
| if tool.get("search_tool_name") in allowed_names |
There was a problem hiding this comment.
Circular import between
search_tool_management.py and endpoints.py
search_tool_management.py dynamically imports _get_allowed_search_tool_names from endpoints.py (line 162–163), while endpoints.py already imports from the same proxy module. If endpoints.py ever imports anything from search_tool_management.py (e.g., for type hints or shared helpers), a circular import error will be triggered at runtime.
More importantly, a utility that computes allowed search tool names for a caller (_get_allowed_search_tool_names) should live in a shared module (e.g., litellm/proxy/auth/auth_checks.py alongside search_tool_access_check) rather than in a request-handler file. Moving it would eliminate the cross-router dependency entirely.
…ve auth helper The DB migration sets search_tools DEFAULT ARRAY['*'], but the auth check and listing filter treated "*" as a literal name, causing 401s and empty list responses for all callers with the default permission. - Add wildcard handling in _can_object_call_search_tools - Add _normalize_search_tools_wildcard to convert ["*"] → None (no restriction) - Move get_allowed_search_tool_names from endpoints.py to auth_checks.py to eliminate cross-router import between search_tool_management and endpoints Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| }; | ||
|
|
||
| loadSearchTools(); | ||
| }, [accessToken, searchTools.length]); |
There was a problem hiding this comment.
Stale
useEffect dependency — tool names don't refresh on same-length array change
The effect depends on searchTools.length rather than the searchTools array itself. If the set of tool IDs changes but the count stays the same (e.g. ["tool-a"] → ["tool-b"]), the effect never re-fires, so searchToolDetails stays stale and the wrong display names are shown.
| }, [accessToken, searchTools.length]); | |
| }, [accessToken, searchTools]); |
| const isAdmin = userRole ? isAdminRole(userRole) : false; | ||
| const { data: userTeamsResponse } = useQuery({ | ||
| queryKey: ["userTeamsForSearchTools", userID], | ||
| queryFn: () => { | ||
| if (!accessToken || !userID) throw new Error("Missing auth"); | ||
| return teamListCall(accessToken, 1, 100, { userID }) as Promise<TeamsResponse>; |
There was a problem hiding this comment.
Client-side team fetch is capped at 100 — silently drops allowed tools for members of many teams
The teamListCall hard-codes pageSize=100. A non-admin user who belongs to more than 100 teams will have the surplus teams silently excluded from scopedSearchTools, causing tools allowed by those teams to disappear from the UI even though the backend API would return them.
Additionally, the backend /search_tools/list endpoint already filters tools by the caller's key/team permissions, so this client-side re-filtering is largely redundant and introduces two divergent filtering paths that can produce conflicting results.
Consider either:
- Removing the client-side filter entirely and trusting the server response (which is already permission-scoped), or
- Using the
useInfiniteTeamshook here as well, to ensure all teams are loaded.
| delete formValues.allowed_agents_and_groups; | ||
| } | ||
|
|
||
| // Always send search_tools to ensure the permission record is created. | ||
| // Empty array = no access (least privilege for new keys). | ||
| if (!formValues.object_permission) { | ||
| formValues.object_permission = {}; |
There was a problem hiding this comment.
New-key default silently changed to "no search tool access" without API-level flag
This code always writes search_tools = [] into the key's object_permission when no search tools are selected — even if the user doesn't touch the Search Tool Settings section at all. For any existing user who creates new keys through the UI after this deployment, their keys will have zero search tool access by default, whereas before they had full access (["*"]).
The Alert widget informs users of the change at click-time, but the change still applies immediately without an opt-in flag on the server side. Consistent with the project rule on backward-incompatible changes, a server-side feature flag (e.g. litellm.search_tools_least_privilege_default = True) would allow existing deployments to preserve the previous default and opt into the new behaviour deliberately.
Rule Used: What: avoid backwards-incompatible changes without... (source)
|
|
||
| # Add server if not already present | ||
| if server_id not in existing_mcp_servers: | ||
| existing_mcp_servers.append(server_id) | ||
|
|
||
| # Upsert the ObjectPermissionTable |
There was a problem hiding this comment.
add_mcp_server_to_team upsert CREATE doesn't propagate existing search_tools value
When a team has no ObjectPermissionTable yet, the upsert CREATE sets only mcp_servers and object_permission_id:
"create": {
"object_permission_id": object_permission_id,
"mcp_servers": existing_mcp_servers,
},The database column has DEFAULT ARRAY['*']::TEXT[], so a brand-new row will get search_tools = ["*"] at the DB level. This is the correct default, but only because the DB default is relied on implicitly. A future schema change (e.g. changing the default to NULL or {}) would silently break the intended "all access" default for newly-created teams without any code change.
Consider explicitly including search_tools in the CREATE data to make the intent self-documenting and resilient to schema changes:
"create": {
"object_permission_id": object_permission_id,
"mcp_servers": existing_mcp_servers,
"search_tools": ["*"], # Explicit default: all tools accessible
},| identified_budget_id = tm.budget_id | ||
| break | ||
|
|
||
| ### Validate extra_permissions BEFORE any DB writes | ||
| if data.extra_permissions is not None: | ||
| from litellm.proxy.auth.permissions import VALID_PERMISSIONS | ||
|
|
||
| invalid_perms = set(data.extra_permissions) - VALID_PERMISSIONS | ||
| if invalid_perms: | ||
| raise HTTPException( | ||
| status_code=400, |
There was a problem hiding this comment.
extra_permissions validation is duplicated between the endpoint and the Member model validator
team_member_update validates extra_permissions against VALID_PERMISSIONS manually (lines ~2424–2434), and then the Member Pydantic model runs the same validation again inside validate_permission_format when the Member(...) object is constructed.
This is harmless but creates two maintenance points: the endpoint will raise HTTPException(400) and the model validator would raise ValidationError, and the two may diverge if one is updated without the other. Consider removing the endpoint-level check and relying solely on the Member validator (catching ValidationError and re-raising as HTTPException).
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
https://app.circleci.com/pipelines/github/BerriAI/litellm/70753
Problem
Search tools permissions were fully supported in the backend but completely missing from the UI. The Search Tools page used outdated Tremor components, and the TeamDropdown was limited to 10 teams due to v2 pagination.
Fix
useInfiniteTeams, replacing the prop-based approach across all callerssearch_toolsfield to Prisma schema and Pydantic models/search_tools/listendpoint/search_tools/listand/search_tools/ui/available_providersto allowed routes for virtual keysTesting
useInfiniteTeams,Alert, and team data alignmentType
🆕 New Feature
✅ Test