Incorrect test assertion — this test will fail

The test comment says "Vector stores: empty list = access to ALL (existing behavior)" and asserts result is True, but the actual implementation of _can_object_call_vector_stores (lines 3546-3555 of auth_checks.py) explicitly DENIES when vector_stores is an empty list:

# Empty list = no access to any vector stores
if len(object_permissions.vector_stores) == 0:
    raise ProxyException(...)

With perm.vector_stores = [], len([]) == 0 is True, so the function raises ProxyException. The assertion assert result is True will never be reached. The test should instead use pytest.raises(ProxyException) — matching the actual semantics that both vector stores and search tools share: None → allow, [] → DENY, non-empty list → check against list.

The comment and test are also inconsistent with the PR description, which claims "unlike vector stores where empty means all access." The existing code already denies on empty vector store lists.

Direct DB queries in critical request path — use get_object_permission with caching

_get_allowed_search_tool_names makes direct prisma_client.db.litellm_objectpermissiontable.find_unique calls (up to 2 per GET /v1/search/tools request) without going through the cached get_object_permission helper in auth_checks.py. This violates the project rule against direct DB queries in the critical path and adds unnecessary database load.

Additionally, the permission logic is duplicated from search_tool_access_check in auth_checks.py. For the list_search_tools endpoint, the same permissions are looked up via a completely separate code path. Consider reusing get_object_permission (from litellm.proxy.auth.auth_checks) and the get_permitted_search_tool_names helper that already exists in auth_checks.py to avoid the duplication.

Rule Used: What: In critical path of request, there should be... (source)

Backwards-incompatible default change for LiteLLM_ObjectPermissionTable fields

This PR changes the defaults of five pre-existing fields:

# Before
mcp_servers: Optional[List[str]] = []
mcp_access_groups: Optional[List[str]] = []
vector_stores: Optional[List[str]] = []
agents: Optional[List[str]] = []
agent_access_groups: Optional[List[str]] = []

# After
mcp_servers: Optional[List[str]] = None
mcp_access_groups: Optional[List[str]] = None
vector_stores: Optional[List[str]] = None
agents: Optional[List[str]] = None
agent_access_groups: Optional[List[str]] = None

Any code that constructs a LiteLLM_ObjectPermissionTable without these fields will now receive None instead of []. Callers that previously iterated these fields safely (e.g., for s in obj.mcp_servers:) without a None check will now raise TypeError: 'NoneType' object is not iterable. This is particularly risky for the mcp_servers and mcp_access_groups fields, which have access-control checks that behave differently for None vs [].

If this default change is intentional to unify the semantics, all downstream code should be audited for None-safety before merging.

Rule Used: What: avoid backwards-incompatible changes without... (source)

Implicit None return when object_type is unexpected

get_search_tool_access_error_type_for_object has no else/fallback branch. If object_type is anything other than "key", "team", or "org", it returns None implicitly. This None is then passed as type=None to ProxyException.__init__, which would likely cause a downstream TypeError or silently drop the error type.

Although the parameter is typed as Literal["key", "team", "org"], a defensive fallback matches the pattern of the sibling get_vector_store_access_error_type_for_object method and prevents silent failures:

Direct DB queries bypass caching on every search request

search_tool_access_check makes raw prisma_client.db.litellm_objectpermissiontable.find_unique calls (up to 2 per request) instead of going through the existing get_object_permission helper (line 2235), which is backed by DualCache and caches results under "object_permission_id:{id}". This means every /v1/search/* call unconditionally hits the database twice for permission lookups, bypassing the cache.

The get_object_permission helper should be used here instead — consistent with how agent_permission_handler.py and user_api_key_auth_mcp.py resolve object permissions. The helper requires a user_api_key_cache argument, which can be imported from litellm.proxy.proxy_server.

Note: the identical issue exists in the pre-existing vector_store_access_check function (lines 3503–3521), but that's outside this PR's scope.

Rule Used: What: Avoid creating new database requests or Rout... (source)

Migration default backfills [] — silently denies search tools for all existing records

The migration adds the column with DEFAULT ARRAY[]::TEXT[]. In PostgreSQL, ADD COLUMN ... DEFAULT backfills all existing rows with the supplied default, so every current LiteLLM_ObjectPermissionTable record will immediately have search_tools = [] after this migration runs.

The Python access check in _can_object_call_search_tools treats [] as "deny all" (principle of least privilege). This means any team or API key that already has an ObjectPermissionTable record (e.g. for MCP or vector-store access control) will be silently denied access to every search tool the moment they upgrade, even though they never configured search_tools at all. This contradicts the PR description's backward-compatibility promise: "None (field not set / no object_permission) → No restriction".

The safe fix is to default to NULL, so the column is absent from existing rows and the Python None-means-allow path is taken:

The matching Prisma schema lines in all three schema.prisma files should also drop the @default([]) annotation for this field so that new rows also get NULL unless explicitly set.

Access check skipped when search_tool_name is absent — unrestricted search possible

The access check at line 222 is guarded by if resolved_search_tool_name:, which means requests that do not specify a search_tool_name (or pass an empty string) bypass the permission check entirely and proceed to the underlying LLM routing. If the router can still route the request without a tool name (e.g. via a default model), the caller effectively gets unrestricted search access regardless of their object_permission.

Consider either (a) enforcing that search_tool_name is always required, or (b) performing a broader "has any search tool access" check when no specific tool is named.

Direct DB queries on every search request — violates no-raw-DB-calls-in-critical-path rule

search_tool_access_check makes up to two raw prisma_client.db.litellm_objectpermissiontable.find_unique calls per /v1/search/* request instead of going through the existing get_object_permission helper (backed by DualCache). This adds two uncached DB round-trips to every search request.

The project rule requires that all permission lookups in the critical path use the cached helper functions (get_team, get_user, get_key, get_object_permission) rather than raw Prisma calls. The identical raw-call pattern exists in the pre-existing vector_store_access_check (lines 3503–3521), but this PR introduces the same anti-pattern again.

Use get_object_permission (importable from litellm.proxy.auth.auth_checks) and pass user_api_key_cache from litellm.proxy.proxy_server as the cache argument, consistent with how agent_permission_handler.py and user_api_key_auth_mcp.py resolve object permissions.

This same issue exists in _get_allowed_search_tool_names in litellm/proxy/search_endpoints/endpoints.py (lines 36–54), which also makes two raw DB calls per GET /v1/search/tools request.

Rule Used: What: In critical path of request, there should be... (source)

Incorrect test assertion — _can_object_call_vector_stores with [] raises, not returns True

The test comment reads "Vector stores: empty list = access to ALL (existing behavior)" and asserts result is True, but the actual implementation of _can_object_call_vector_stores (lines 3546–3555 of auth_checks.py) explicitly raises when vector_stores is an empty list:

if len(object_permissions.vector_stores) == 0:
    raise ProxyException(...)

So this test will fail at runtime: the call raises ProxyException before the assert is reached. The test should use pytest.raises:

The PR description's claim that vector stores differ from search tools ("unlike vector stores where empty means all access") is also incorrect — both features deny when the list is empty.

check_member_permission, add_mcp_server_to_team, and remove_mcp_server_from_team are undefined — ImportError at startup

These three names are imported here but do not exist anywhere in the codebase:

• check_member_permission is imported from litellm.proxy.management_endpoints.common_utils (line 135), but a search of the entire repository finds no definition of this function.
• add_mcp_server_to_team (line 139) and remove_mcp_server_from_team (line 140) are imported from litellm.proxy.management_helpers.object_permission_utils, but only _get_team_allowed_mcp_servers exists in that file — the other two names are absent.

Because these imports are inside the if MCP_AVAILABLE: block that runs at module load time, any environment where MCP is available will raise an ImportError as soon as mcp_management_endpoints is imported, breaking all MCP endpoints (not just the new team-scoped paths). The three functions are called at lines 1237, 1301, 1542, 1578, and 1898.

These functions must be implemented and exported from their respective modules before this PR can be merged.

search_tools must be nullable (String[]?) in the Prisma schema

The field is defined as String[] (non-nullable), but the intent is for NULL to mean "no restriction". In Prisma, a non-nullable String[] field causes the generated client to type this as string[] (not string[] | null). When prisma-client-py reads a DB row where this column is NULL (all rows after the migration, since no DEFAULT was specified), the client may coerce NULL → [] to satisfy the non-nullable contract.

If that coercion happens, object_permissions.search_tools will be [] instead of None, causing _can_object_call_search_tools to deny access for every existing permission record — the exact opposite of the backward-compatibility promise.

Compare with mcp_tool_permissions Json? which uses ? to explicitly allow null. Change to:

The same fix is needed in litellm-proxy-extras/litellm_proxy_extras/schema.prisma and the root schema.prisma.

Rule Used: What: avoid backwards-incompatible changes without... (source)

get_permitted_search_tool_names is dead code

This helper is defined and exported but is never called from any production code path. Both search_tool_access_check (line 3587) and _get_allowed_search_tool_names in endpoints.py (line 48/61) access object_permissions.search_tools directly rather than going through this function. It is only exercised by the unit tests in test_search_tool_access.py.

Either wire the helper into _get_allowed_search_tool_names (replacing the direct .search_tools access) to justify its existence, or remove it to avoid confusion about the intended API surface.

Missing return type annotation

_can_object_call_search_tools has no -> bool: return type annotation, unlike the sibling _can_object_call_vector_stores which (implicitly) returns bool. Adding a type annotation keeps this consistent and makes the contract explicit for callers.

_{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!}

Missing database migration file

Three schema.prisma files (the root schema.prisma, litellm/proxy/schema.prisma, and litellm-proxy-extras/litellm_proxy_extras/schema.prisma) all add the search_tools String[]? column, but no corresponding SQL migration file is included in the PR.

Without a migration, existing production databases will not have this column. When the deployed code calls get_object_permission and the Prisma client tries to hydrate a LiteLLM_ObjectPermissionTable from a DB row missing the search_tools column, it will either error or silently return None depending on the Prisma-Python driver version — which could produce unexpected behavior.

Other columns in this project track explicit migrations (e.g. under litellm-proxy-extras/litellm_proxy_extras/migrations/). A migration needs to be added here, with DEFAULT NULL (not DEFAULT ARRAY[]::TEXT[]) so that existing rows are backward-compatible (NULL → no restriction, per the PR's own semantics). The same issue applies to the identical change in the root schema.prisma and litellm-proxy-extras/litellm_proxy_extras/schema.prisma.

Missing return type annotation on search_tool_access_check

search_tool_access_check is missing a return type annotation, unlike its sibling _can_object_call_search_tools which declares -> bool:. Adding -> bool: to the function signature keeps the contract explicit for callers and is consistent with the rest of the codebase.

_{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!}