mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-04 02:31:27 +00:00
fix(mcp): drop env vars entirely for virtual keys and reuse access lookup for per-user endpoints
This commit is contained in:
mateo-berri
parent
86cf341c3c
commit
4cbf8caeaf
2 changed files with 64 additions and 52 deletions
|
|
@ -577,7 +577,7 @@ if MCP_AVAILABLE:
|
|||
sanitized.allowed_tools = []
|
||||
sanitized.mcp_access_groups = []
|
||||
sanitized.teams = []
|
||||
_redact_global_env_var_values(sanitized)
|
||||
sanitized.env_vars = None
|
||||
|
||||
sanitized.authorization_url = None
|
||||
sanitized.token_url = None
|
||||
|
|
@ -2162,32 +2162,42 @@ if MCP_AVAILABLE:
|
|||
|
||||
# ── Per-user MCP env var endpoints ────────────────────────────────────────
|
||||
|
||||
async def _authorize_mcp_server_access(
|
||||
async def _authorize_and_fetch_mcp_server(
|
||||
prisma_client,
|
||||
user_api_key_dict: UserAPIKeyAuth,
|
||||
server_id: str,
|
||||
) -> None:
|
||||
"""Raise 403 if a non-admin caller cannot access this MCP server.
|
||||
) -> LiteLLM_MCPServerTable:
|
||||
"""Return the MCP server the caller may manage env vars for.
|
||||
|
||||
Mirrors the access gate in ``fetch_mcp_server`` so the per-user env-var
|
||||
endpoints can't be used to read or mutate state for servers outside the
|
||||
caller's allowed set.
|
||||
Admins look the server up directly. Non-admins reuse the access-scoped
|
||||
listing that already loads every server they can see, so we don't issue
|
||||
a second per-server query just to re-fetch a record the authorization
|
||||
check produced. A non-admin who can't see the server gets 403 (never
|
||||
404) so server ids can't be enumerated.
|
||||
"""
|
||||
if _user_has_admin_view(user_api_key_dict):
|
||||
return
|
||||
server = await get_mcp_server(prisma_client, server_id)
|
||||
if server is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail={"error": f"MCP Server {server_id} not found"},
|
||||
)
|
||||
return server
|
||||
accessible = await get_all_mcp_servers_for_user(
|
||||
prisma_client, user_api_key_dict
|
||||
)
|
||||
if not does_mcp_server_exist(accessible, server_id):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail={
|
||||
"error": (
|
||||
f"User does not have permission to access mcp server with id {server_id}. "
|
||||
"You can only manage env vars for mcp servers that you have access to."
|
||||
)
|
||||
},
|
||||
)
|
||||
for server in accessible:
|
||||
if server.server_id == server_id:
|
||||
return server
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail={
|
||||
"error": (
|
||||
f"User does not have permission to access mcp server with id {server_id}. "
|
||||
"You can only manage env vars for mcp servers that you have access to."
|
||||
)
|
||||
},
|
||||
)
|
||||
|
||||
def _compute_user_env_var_status(
|
||||
*,
|
||||
|
|
@ -2261,13 +2271,9 @@ if MCP_AVAILABLE:
|
|||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail={"error": "User ID not found in token"},
|
||||
)
|
||||
await _authorize_mcp_server_access(prisma_client, user_api_key_dict, server_id)
|
||||
server = await get_mcp_server(prisma_client, server_id)
|
||||
if server is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail={"error": f"MCP Server {server_id} not found"},
|
||||
)
|
||||
server = await _authorize_and_fetch_mcp_server(
|
||||
prisma_client, user_api_key_dict, server_id
|
||||
)
|
||||
stored = await get_user_env_vars(prisma_client, user_id, server_id)
|
||||
return _compute_user_env_var_status(server=server, stored_values=stored)
|
||||
|
||||
|
|
@ -2298,13 +2304,9 @@ if MCP_AVAILABLE:
|
|||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail={"error": "User ID not found in token"},
|
||||
)
|
||||
await _authorize_mcp_server_access(prisma_client, user_api_key_dict, server_id)
|
||||
server = await get_mcp_server(prisma_client, server_id)
|
||||
if server is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail={"error": f"MCP Server {server_id} not found"},
|
||||
)
|
||||
server = await _authorize_and_fetch_mcp_server(
|
||||
prisma_client, user_api_key_dict, server_id
|
||||
)
|
||||
# Only known per-user var names declared by the admin are accepted —
|
||||
# never persist arbitrary keys the user invents. Submitted values are
|
||||
# merged over the existing set so a user updating one credential does
|
||||
|
|
@ -2347,13 +2349,9 @@ if MCP_AVAILABLE:
|
|||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail={"error": "User ID not found in token"},
|
||||
)
|
||||
await _authorize_mcp_server_access(prisma_client, user_api_key_dict, server_id)
|
||||
server = await get_mcp_server(prisma_client, server_id)
|
||||
if server is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail={"error": f"MCP Server {server_id} not found"},
|
||||
)
|
||||
server = await _authorize_and_fetch_mcp_server(
|
||||
prisma_client, user_api_key_dict, server_id
|
||||
)
|
||||
await delete_user_env_vars(prisma_client, user_id, server_id)
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
invalidate_user_env_vars_cache,
|
||||
|
|
|
|||
|
|
@ -3580,19 +3580,9 @@ def test_sanitize_mcp_server_for_non_admin_clears_credential_fields():
|
|||
assert sanitized.alias == server.alias
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"sanitizer_name",
|
||||
["_sanitize_mcp_server_for_non_admin", "_sanitize_mcp_server_for_virtual_key"],
|
||||
)
|
||||
def test_sanitize_masks_global_env_var_secrets(sanitizer_name):
|
||||
"""Non-admin and virtual-key views must never expose the admin-supplied
|
||||
global env var secret, while per-user placeholders are left intact."""
|
||||
import litellm.proxy.management_endpoints.mcp_management_endpoints as mgmt
|
||||
|
||||
sanitizer = getattr(mgmt, sanitizer_name)
|
||||
|
||||
def _server_with_global_and_user_env_vars():
|
||||
base = generate_mock_mcp_server_db_record()
|
||||
server = LiteLLM_MCPServerTable(
|
||||
return LiteLLM_MCPServerTable(
|
||||
**{
|
||||
**base.model_dump(),
|
||||
"env_vars": [
|
||||
|
|
@ -3602,7 +3592,15 @@ def test_sanitize_masks_global_env_var_secrets(sanitizer_name):
|
|||
}
|
||||
)
|
||||
|
||||
sanitized = sanitizer(server)
|
||||
|
||||
def test_sanitize_non_admin_masks_global_env_var_secrets():
|
||||
"""The non-admin view blanks the admin-supplied global env var secret but
|
||||
keeps per-user placeholders so users still know which vars to fill in."""
|
||||
import litellm.proxy.management_endpoints.mcp_management_endpoints as mgmt
|
||||
|
||||
server = _server_with_global_and_user_env_vars()
|
||||
|
||||
sanitized = mgmt._sanitize_mcp_server_for_non_admin(server)
|
||||
|
||||
by_name = {ev.name: ev for ev in sanitized.env_vars}
|
||||
assert by_name["ADMIN_API_KEY"].value == ""
|
||||
|
|
@ -3613,6 +3611,22 @@ def test_sanitize_masks_global_env_var_secrets(sanitizer_name):
|
|||
assert original_by_name["ADMIN_API_KEY"].value == "super-secret"
|
||||
|
||||
|
||||
def test_sanitize_virtual_key_drops_all_env_vars():
|
||||
"""Virtual-key callers get a discovery-only view; env var entries (even the
|
||||
names, which are admin config metadata) must be dropped entirely, not just
|
||||
have their global values blanked."""
|
||||
import litellm.proxy.management_endpoints.mcp_management_endpoints as mgmt
|
||||
|
||||
server = _server_with_global_and_user_env_vars()
|
||||
|
||||
sanitized = mgmt._sanitize_mcp_server_for_virtual_key(server)
|
||||
|
||||
assert sanitized.env_vars is None
|
||||
|
||||
# The original object must not be mutated.
|
||||
assert server.env_vars[0].value == "super-secret"
|
||||
|
||||
|
||||
def _server_with_env_vars(server_id: str = "srv-env"):
|
||||
base = generate_mock_mcp_server_db_record(server_id=server_id)
|
||||
return LiteLLM_MCPServerTable(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue