From fc8ad45ba3e5dc6a85074ba0e37cb2ead9e222ad Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Sat, 23 May 2026 19:11:41 -0700 Subject: [PATCH] feat(mcp): per-user env-vars for MCP servers (backend + UI wiring) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds end-to-end support for per-server env-var definitions and per-user values that interpolate `${VAR}` placeholders in `static_headers` at tool-call time. Schema: - New `env_vars` JSONB column on `LiteLLM_MCPServerTable` (list of `{name, scope: "instance"|"per_user", value}`); per-user values reuse `LiteLLM_MCPUserCredentials` with payload `type="vars"`. Backend: - `litellm/proxy/_experimental/mcp_server/env_vars.py` — parse / resolve / interpolate helpers + `MissingEnvVarsError`. - `MCPServerManager._resolve_env_vars_for_server` substitutes placeholders before merging extra headers. Missing required per-user vars surface as `CallToolResult(isError=True)` with a dashboard deep-link, so MCP clients (Claude Code, etc.) print the message to the user. - `GET`/`POST /v1/mcp/server/{server_id}/my-env-vars` for the fill modal and status pill. Only `per_user` definitions are returned; instance values stay admin-only. Stale names (after a rename or scope flip) are trimmed on both read and write paths. UI: - Fill modal + status pill now hit the real endpoints (mocks remain as fallback for demo mode). - `MCPServer` type gains `env_vars`; networking helpers `getMyMcpEnvVars` / `storeMyMcpEnvVars` added. Tests: - New `tests/test_litellm/proxy/_experimental/mcp_server/test_env_vars.py` covers parse / collect / resolve / interpolate / missing_required and user-var storage round-trip. - Manager tests extended to exercise placeholder interpolation and the missing-vars deep-link error path. Co-authored-by: Cursor --- .../migration.sql | 2 + .../litellm_proxy_extras/schema.prisma | 5 + litellm/proxy/_experimental/mcp_server/db.py | 4 + .../_experimental/mcp_server/env_vars.py | 314 ++++++++++++++++ .../mcp_server/mcp_server_manager.py | 113 +++++- litellm/proxy/_types.py | 40 ++ .../mcp_management_endpoints.py | 126 +++++++ litellm/proxy/schema.prisma | 5 + .../types/mcp_server/mcp_server_manager.py | 4 + schema.prisma | 5 + .../_experimental/mcp_server/test_env_vars.py | 350 ++++++++++++++++++ .../mcp_server/test_mcp_server.py | 34 ++ .../components/mcp_tools/MCPServerCard.tsx | 67 +++- .../mcp_tools/create_mcp_server.tsx | 21 +- .../mcp_tools/mcp_server_columns.tsx | 5 +- .../components/mcp_tools/mcp_server_edit.tsx | 34 +- .../src/components/mcp_tools/mcp_servers.tsx | 20 +- .../mcp_tools/mock/EnvVarsSection.tsx | 20 +- .../mcp_tools/mock/FillUserFieldsModal.tsx | 90 +++-- .../mcp_tools/mock/UserFieldsStatusCell.tsx | 121 +++--- .../mcp_tools/mock/mockMcpEnvVars.ts | 11 +- .../src/components/mcp_tools/types.tsx | 13 + .../src/components/networking.tsx | 77 ++++ 23 files changed, 1299 insertions(+), 182 deletions(-) create mode 100644 litellm-proxy-extras/litellm_proxy_extras/migrations/20260521143000_add_env_vars_to_mcp_servers/migration.sql create mode 100644 litellm/proxy/_experimental/mcp_server/env_vars.py create mode 100644 tests/test_litellm/proxy/_experimental/mcp_server/test_env_vars.py diff --git a/litellm-proxy-extras/litellm_proxy_extras/migrations/20260521143000_add_env_vars_to_mcp_servers/migration.sql b/litellm-proxy-extras/litellm_proxy_extras/migrations/20260521143000_add_env_vars_to_mcp_servers/migration.sql new file mode 100644 index 00000000000..926a45a8f43 --- /dev/null +++ b/litellm-proxy-extras/litellm_proxy_extras/migrations/20260521143000_add_env_vars_to_mcp_servers/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "LiteLLM_MCPServerTable" ADD COLUMN IF NOT EXISTS "env_vars" JSONB DEFAULT '[]'; diff --git a/litellm-proxy-extras/litellm_proxy_extras/schema.prisma b/litellm-proxy-extras/litellm_proxy_extras/schema.prisma index 78143fe0411..5cecbe93261 100644 --- a/litellm-proxy-extras/litellm_proxy_extras/schema.prisma +++ b/litellm-proxy-extras/litellm_proxy_extras/schema.prisma @@ -311,6 +311,11 @@ model LiteLLM_MCPServerTable { tool_name_to_description Json? @default("{}") extra_headers String[] @default([]) static_headers Json? @default("{}") + // Per-server env-var definitions: [{name, scope: "instance"|"per_user", value}]. + // `${NAME}` placeholders in static_headers / credentials are substituted at + // request time. instance values come from this column; per_user values from + // LiteLLM_MCPUserCredentials with payload type="vars". + env_vars Json? @default("[]") // Health check status status String? @default("unknown") last_health_check DateTime? diff --git a/litellm/proxy/_experimental/mcp_server/db.py b/litellm/proxy/_experimental/mcp_server/db.py index a6f0d145e9b..774b00ef4d6 100644 --- a/litellm/proxy/_experimental/mcp_server/db.py +++ b/litellm/proxy/_experimental/mcp_server/db.py @@ -61,6 +61,10 @@ def _prepare_mcp_server_data( if data.static_headers is not None: data_dict["static_headers"] = safe_dumps(data.static_headers) + # Handle env_vars serialization — list[{name, scope, value}], stored as JSON + if getattr(data, "env_vars", None) is not None: + data_dict["env_vars"] = safe_dumps(data.env_vars) + # Handle mcp_info serialization if data.mcp_info is not None: data_dict["mcp_info"] = safe_dumps(data.mcp_info) diff --git a/litellm/proxy/_experimental/mcp_server/env_vars.py b/litellm/proxy/_experimental/mcp_server/env_vars.py new file mode 100644 index 00000000000..18111da6eae --- /dev/null +++ b/litellm/proxy/_experimental/mcp_server/env_vars.py @@ -0,0 +1,314 @@ +"""Variable interpolation for MCP server `static_headers` and authentication. + +Admins define `${VAR_NAME}` placeholders in a server's `static_headers` (or +auth value) and a sibling `env_vars` list that declares each variable's scope: + + [{"name": "DB_HOSTNAME", "scope": "instance", "value": "db.corp.internal"}, + {"name": "CORP_PASSWORD", "scope": "per_user", "value": null}] + +At request time we resolve each `${NAME}` to: + 1. The calling user's stored value for that var (per_user scope), OR + 2. The instance value baked into the server config (instance scope). + +If a required per_user var has no stored value, we raise `MissingEnvVarsError`, +which the MCP handler converts into a `tools/call` error containing a deep +link back to the dashboard's fill-in-credentials modal. + +Per-user values live in `LiteLLM_MCPUserCredentials` next to BYOK / OAuth +credentials, distinguished by a `"type": "vars"` field in the JSON payload +(matching the existing convention from `CLAUDE.md`). +""" + +import json +import os +import re +from typing import Any, Dict, Iterable, List, Literal, Optional + +from pydantic import BaseModel + +from litellm._logging import verbose_proxy_logger +from litellm.proxy._experimental.mcp_server.db import ( + _decode_user_credential, +) +from litellm.proxy.common_utils.encrypt_decrypt_utils import encrypt_value_helper +from litellm.proxy.utils import PrismaClient + +EnvVarScope = Literal["instance", "per_user"] + +# Matches `${NAME}` where NAME is UPPER_SNAKE_CASE. Same rule as the UI's +# `EnvVarsSection` warningOnly validator, so anything the form accepts is +# resolved here and vice-versa. +_PLACEHOLDER_RE = re.compile(r"\$\{([A-Z_][A-Z0-9_]*)\}") + +# JSON-payload tag that distinguishes per-user env vars from BYOK / OAuth. +_VARS_PAYLOAD_TYPE = "vars" + + +class EnvVarDefinition(BaseModel): + """One row of the per-server env-var definition list.""" + + name: str + scope: EnvVarScope + # Only meaningful for `instance` scope. `per_user` entries store the + # admin-declared placeholder (no value); the value comes from each user. + value: Optional[str] = None + + +class MissingEnvVarsError(Exception): + """Raised when a server's static_headers reference per-user vars the + caller has not yet stored. The MCP handler catches this and returns a + `CallToolResult(isError=True)` so Claude Code prints the message. + """ + + def __init__( + self, + *, + server_alias: str, + server_name: Optional[str], + missing: List[str], + ): + self.server_alias = server_alias + self.server_name = server_name or server_alias + self.missing = list(missing) + super().__init__( + f"Missing per-user MCP vars for server={server_alias!r}: {missing}" + ) + + def deep_link(self) -> str: + """URL the user clicks to land on the MCP Servers page with the fill + modal auto-opened. The modal handler in `mcp_servers.tsx` only mounts + on `/tools/mcp-servers`, so the path matters. + + Resolution order: + 1. ``PROXY_UI_BASE_URL`` — full URL to the UI root (use this in + dev, e.g. ``http://localhost:3000``). + 2. ``PROXY_BASE_URL/ui`` — the proxy-served static export path. + 3. ``http://localhost:4000/ui`` — final fallback. + """ + ui_base = os.environ.get("PROXY_UI_BASE_URL") + if not ui_base: + proxy_base = os.environ.get( + "PROXY_BASE_URL", "http://localhost:4000" + ).rstrip("/") + ui_base = f"{proxy_base}/ui" + return ( + f"{ui_base.rstrip('/')}/tools/mcp-servers" + f"?fill_fields={self.server_alias}" + ) + + def to_user_message(self) -> str: + """Single-string error rendered into the terminal by the MCP client. + + The shape mirrors the prototype's `MockClaudeCodeModal` copy so the + real and mock experiences read the same. + """ + bullets = "\n".join(f" - {name}" for name in self.missing) + return ( + f'Cannot connect to MCP server "{self.server_name}".\n\n' + f"Your administrator configured this server to require per-user " + f"credentials, but you haven't set the following yet:\n" + f"{bullets}\n\n" + f"Set your credentials here:\n{self.deep_link()}" + ) + + +def parse_env_var_definitions(raw: Any) -> List[EnvVarDefinition]: + """Normalize whatever shape the DB / config gave us into typed objects. + + Accepts: + - None / empty → [] + - list of dicts (DB JSON) → parsed + - list of EnvVarDefinition (in-memory) → returned as-is + Silently drops malformed entries; they would be unresolvable anyway. + """ + if not raw: + return [] + if isinstance(raw, str): + try: + raw = json.loads(raw) + except (ValueError, TypeError): + return [] + if not isinstance(raw, list): + return [] + parsed: List[EnvVarDefinition] = [] + for entry in raw: + if isinstance(entry, EnvVarDefinition): + parsed.append(entry) + continue + if not isinstance(entry, dict): + continue + try: + parsed.append(EnvVarDefinition.model_validate(entry)) + except Exception as e: # noqa: BLE001 — config data, log and skip + verbose_proxy_logger.debug( + "env_vars: dropping malformed entry %r: %s", entry, e + ) + return parsed + + +def _placeholders_in(text: str) -> List[str]: + return _PLACEHOLDER_RE.findall(text or "") + + +def collect_placeholders(values: Iterable[Optional[str]]) -> List[str]: + """Return the union of `${NAME}` references found across `values`.""" + seen: List[str] = [] + for v in values: + if not v: + continue + for name in _placeholders_in(v): + if name not in seen: + seen.append(name) + return seen + + +def resolve_values( + defs: List[EnvVarDefinition], + per_user_values: Dict[str, str], + *, + referenced: Iterable[str], +) -> Dict[str, str]: + """Build the `{name: value}` map for substitution. + + Per-user wins over instance for the same name (defensive — admins should + not declare both, but if they do, the per-user value is the more specific). + Only resolves vars actually referenced; unreferenced defs are ignored so + a stray missing per-user value never blocks an unrelated request. + """ + by_name = {d.name: d for d in defs} + referenced_set = set(referenced) + resolved: Dict[str, str] = {} + for name in referenced_set: + if name in per_user_values and per_user_values[name]: + resolved[name] = per_user_values[name] + continue + d = by_name.get(name) + if d is None or d.scope != "instance": + continue + if d.value is None or d.value == "": + continue + resolved[name] = d.value + return resolved + + +def missing_required( + defs: List[EnvVarDefinition], + per_user_values: Dict[str, str], + *, + referenced: Iterable[str], +) -> List[str]: + """Names that are referenced AND defined-as-per_user AND not yet set. + + instance vars without a value are *not* reported as missing here — that + is admin misconfiguration, surfaced at server-edit time. Only per-user + gaps generate the dashboard deep-link error. + """ + by_name = {d.name: d for d in defs} + out: List[str] = [] + for name in referenced: + d = by_name.get(name) + if d is None or d.scope != "per_user": + continue + if not per_user_values.get(name): + out.append(name) + return out + + +def _interpolate_string(value: str, resolved: Dict[str, str]) -> str: + def _sub(match: "re.Match[str]") -> str: + name = match.group(1) + return resolved.get(name, match.group(0)) + + return _PLACEHOLDER_RE.sub(_sub, value) + + +def interpolate_headers( + headers: Optional[Dict[str, str]], + resolved: Dict[str, str], +) -> Optional[Dict[str, str]]: + """Return a new dict with `${NAME}` placeholders replaced in each value. + + Unresolved placeholders are left in place (caller already checked + `missing_required`); we don't want to silently strip them and ship a + broken header upstream. + """ + if not headers: + return headers + return {k: _interpolate_string(v or "", resolved) for k, v in headers.items()} + + +def interpolate_value(value: Optional[str], resolved: Dict[str, str]) -> Optional[str]: + """`interpolate_headers` for a scalar (auth token, etc.).""" + if value is None: + return None + return _interpolate_string(value, resolved) + + +# --------------------------------------------------------------------------- +# Storage helpers — per-user values in LiteLLM_MCPUserCredentials +# --------------------------------------------------------------------------- + + +def _encode_vars_payload(values: Dict[str, str]) -> str: + payload = {"type": _VARS_PAYLOAD_TYPE, "values": values} + return encrypt_value_helper(json.dumps(payload)) + + +def _decode_vars_payload(stored: str) -> Optional[Dict[str, str]]: + decoded = _decode_user_credential(stored) + if decoded is None: + return None + try: + parsed = json.loads(decoded) + except (ValueError, TypeError): + return None + if not isinstance(parsed, dict) or parsed.get("type") != _VARS_PAYLOAD_TYPE: + return None + values = parsed.get("values") + if not isinstance(values, dict): + return None + # Drop any non-string values defensively. + return {k: v for k, v in values.items() if isinstance(v, str)} + + +async def get_user_env_vars( + prisma_client: PrismaClient, + user_id: str, + server_id: str, +) -> Dict[str, str]: + """Return the per-user `{NAME: value}` map for (user, server), or {}.""" + row = await prisma_client.db.litellm_mcpusercredentials.find_unique( + where={"user_id_server_id": {"user_id": user_id, "server_id": server_id}} + ) + if row is None: + return {} + return _decode_vars_payload(row.credential_b64) or {} + + +async def store_user_env_vars( + prisma_client: PrismaClient, + user_id: str, + server_id: str, + values: Dict[str, str], +) -> None: + """Upsert the per-user values payload. Replaces any existing vars payload + in full (the UI form sends the complete map on save). + + Caveat — shares the (user_id, server_id) row with BYOK / OAuth payloads. + If a row already exists with a different `"type"`, this overwrites it. + In practice a server that uses per-user vars does not also need a per-user + BYOK key (the var *is* the credential), so the collision is acceptable for + the prototype. If we ever need both, split this into its own table. + """ + encoded = _encode_vars_payload(values) + await prisma_client.db.litellm_mcpusercredentials.upsert( + where={"user_id_server_id": {"user_id": user_id, "server_id": server_id}}, + data={ + "create": { + "user_id": user_id, + "server_id": server_id, + "credential_b64": encoded, + }, + "update": {"credential_b64": encoded}, + }, + ) diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index bbf40f6e9ef..fccc6c78cc8 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -26,6 +26,7 @@ from mcp.types import ( GetPromptResult, Prompt, ResourceTemplate, + TextContent, ) from mcp.types import Tool as MCPTool from pydantic import AnyUrl @@ -61,6 +62,15 @@ from litellm.proxy._experimental.mcp_server.utils import ( split_server_prefix_from_name, validate_mcp_server_name, ) +from litellm.proxy._experimental.mcp_server.env_vars import ( + MissingEnvVarsError, + collect_placeholders, + get_user_env_vars, + interpolate_headers, + missing_required, + parse_env_var_definitions, + resolve_values, +) from litellm.proxy._types import ( LiteLLM_MCPServerTable, MCPAuthType, @@ -668,6 +678,16 @@ class MCPServerManager: static_headers_dict = _deserialize_json_dict( getattr(mcp_server, "static_headers", None) ) + env_vars_raw = getattr(mcp_server, "env_vars", None) + env_vars_list: Optional[List[Dict[str, Any]]] = None + if env_vars_raw is not None: + if isinstance(env_vars_raw, str): + try: + env_vars_list = json.loads(env_vars_raw) + except (ValueError, TypeError): + env_vars_list = None + elif isinstance(env_vars_raw, list): + env_vars_list = env_vars_raw credentials_dict = _deserialize_json_dict( getattr(mcp_server, "credentials", None) ) @@ -767,6 +787,7 @@ class MCPServerManager: mcp_info=mcp_info, extra_headers=getattr(mcp_server, "extra_headers", None), static_headers=static_headers_dict, + env_vars=env_vars_list, client_id=client_id_value or getattr(mcp_server, "client_id", None), client_secret=client_secret_value or getattr(mcp_server, "client_secret", None), @@ -2662,6 +2683,71 @@ class MCPServerManager: ) ) + async def _resolve_env_vars_for_server( + self, + mcp_server: MCPServer, + user_api_key_auth: Optional[UserAPIKeyAuth], + ) -> Optional[Dict[str, str]]: + """Substitute `${VAR}` placeholders in this server's static_headers + using instance defaults + per-user values. + + Returns the interpolated `static_headers` dict (or None / the original + when the server has no placeholders to resolve). + + Note: only `static_headers` are interpolated today. Extending this to + `authentication_token` requires plumbing a resolved value through + `_create_mcp_client`; left as a follow-up since the demo uses + `Authorization: Bearer ${TOKEN}` inside `static_headers`. + + Raises: + MissingEnvVarsError: when a referenced per-user var has no stored + value for the caller. The MCP tool handler converts this to a + `CallToolResult(isError=True)` containing the dashboard + deep-link error message. + """ + defs = parse_env_var_definitions(getattr(mcp_server, "env_vars", None)) + scannable: List[Optional[str]] = [] + if mcp_server.static_headers: + scannable.extend(mcp_server.static_headers.values()) + + referenced = collect_placeholders(scannable) + if not referenced: + return mcp_server.static_headers + + user_id = user_api_key_auth.user_id if user_api_key_auth is not None else None + per_user_values: Dict[str, str] = {} + if user_id and defs: + # Only hit the DB if (a) we know who is calling and (b) the + # server actually has var definitions. + from litellm.proxy.proxy_server import prisma_client as _global_prisma + + if _global_prisma is not None: + try: + per_user_values = await get_user_env_vars( + _global_prisma, user_id, mcp_server.server_id + ) + except Exception as e: # noqa: BLE001 + verbose_logger.warning( + "MCP env-vars: failed to load per-user vars for " + "user=%s server=%s: %s", + user_id, + mcp_server.server_id, + e, + ) + + missing = missing_required(defs, per_user_values, referenced=referenced) + if missing: + raise MissingEnvVarsError( + server_alias=( + mcp_server.alias or mcp_server.server_name or mcp_server.name + ), + server_name=mcp_server.server_name or mcp_server.name, + missing=missing, + ) + + resolved = resolve_values(defs, per_user_values, referenced=referenced) + return interpolate_headers(mcp_server.static_headers, resolved) + async def _call_regular_mcp_tool( # noqa: PLR0915 self, mcp_server: MCPServer, @@ -2675,6 +2761,7 @@ class MCPServerManager: proxy_logging_obj: Optional[ProxyLogging], host_progress_callback: Optional[Callable] = None, hook_extra_headers: Optional[Dict[str, str]] = None, + user_api_key_auth: Optional[UserAPIKeyAuth] = None, ) -> CallToolResult: """ Call a regular MCP tool using the MCP client. @@ -2701,6 +2788,26 @@ class MCPServerManager: GuardrailRaisedException: If guardrails block the call HTTPException: If an HTTP error occurs """ + # Resolve `${VAR}` placeholders in static_headers before any merge + # work. If the caller is missing required per-user vars, surface a + # dashboard deep-link as an MCP tool error so Claude Code (or any + # client) prints it back to the user. + try: + resolved_static_headers = await self._resolve_env_vars_for_server( + mcp_server, user_api_key_auth + ) + except MissingEnvVarsError as e: + verbose_logger.info( + "MCP env-vars missing for server=%s user=%s: %s", + mcp_server.server_name or mcp_server.name, + user_api_key_auth.user_id if user_api_key_auth else None, + e.missing, + ) + return CallToolResult( + isError=True, + content=[TextContent(type="text", text=e.to_user_message())], + ) + # Get server-specific auth header if available (case-insensitive) # FIX: Added case-insensitive matching to handle auth header keys that may not match # the exact case of server alias/name (e.g., '1litellmagcgateway' vs '1LiteLLMAGCGateway') @@ -2754,10 +2861,10 @@ class MCPServerManager: continue extra_headers[header] = header_value - if mcp_server.static_headers: + if resolved_static_headers: if extra_headers is None: extra_headers = {} - extra_headers.update(mcp_server.static_headers) + extra_headers.update(resolved_static_headers) if hook_extra_headers: if extra_headers is None: @@ -3039,6 +3146,7 @@ class MCPServerManager: proxy_logging_obj=proxy_logging_obj, host_progress_callback=host_progress_callback, hook_extra_headers=hook_result.get("extra_headers"), + user_api_key_auth=user_api_key_auth, ) return await self._gather_openapi_tool_tasks(tasks, proxy_logging_obj) @@ -3649,6 +3757,7 @@ class MCPServerManager: extra_headers=server.extra_headers or [], mcp_info=server.mcp_info, static_headers=server.static_headers, + env_vars=server.env_vars, status=None, # No health check performed last_health_check=None, # No health check performed health_check_error=None, diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 9337aa7c8ea..54d4d2ac8db 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -1267,6 +1267,7 @@ class NewMCPServerRequest(LiteLLMPydanticObjectBase): tool_name_to_description: Optional[Dict[str, str]] = None extra_headers: Optional[List[str]] = None static_headers: Optional[Dict[str, str]] = None + env_vars: Optional[List[Dict[str, Any]]] = None instructions: Optional[str] = None # Stdio-specific fields command: Optional[str] = None @@ -1351,6 +1352,7 @@ class UpdateMCPServerRequest(LiteLLMPydanticObjectBase): tool_name_to_description: Optional[Dict[str, str]] = None extra_headers: Optional[List[str]] = None static_headers: Optional[Dict[str, str]] = None + env_vars: Optional[List[Dict[str, Any]]] = None instructions: Optional[str] = None # Stdio-specific fields command: Optional[str] = None @@ -1417,6 +1419,7 @@ class LiteLLM_MCPServerTable(LiteLLMPydanticObjectBase): extra_headers: List[str] = Field(default_factory=list) mcp_info: Optional[MCPInfo] = None static_headers: Optional[Dict[str, str]] = None + env_vars: Optional[List[Dict[str, Any]]] = None # Health check status status: Optional[Literal["healthy", "unhealthy", "unknown"]] = Field( default="unknown", @@ -1492,6 +1495,43 @@ class MCPUserCredentialListItem(LiteLLMPydanticObjectBase): credential_type: str # "oauth2" or "byok" has_credential: bool expires_at: Optional[str] = None # ISO-8601; None means non-expiring + + +class MCPEnvVarDefinitionPublic(LiteLLMPydanticObjectBase): + """Public-safe view of an env-var definition (no instance values). + + Returned to the calling user so the fill-in form knows which inputs to + render. Instance values are intentionally excluded — they may contain + secrets (e.g. a shared upstream token) that only the admin needs. + """ + + name: str + scope: Literal["instance", "per_user"] + + +class MCPUserEnvVarsStatus(LiteLLMPydanticObjectBase): + """Response for `GET /server/{server_id}/my-env-vars`. + + The UI uses this to render both the fill modal AND the "N missing" + status pill without a second request. + """ + + server_id: str + server_alias: Optional[str] = None + definitions: List[MCPEnvVarDefinitionPublic] = Field(default_factory=list) + values: Dict[str, str] = Field(default_factory=dict) # per-user values, plaintext + missing: List[str] = Field(default_factory=list) # per-user names with no value + + +class MCPUserEnvVarsRequest(LiteLLMPydanticObjectBase): + """Body for `POST /server/{server_id}/my-env-vars`. + + Full replace of the caller's vars for this server — the UI sends every + field every time it saves, matching the existing FillUserFieldsModal + behavior. + """ + + values: Dict[str, str] = Field(default_factory=dict) connected_at: Optional[str] = None # ISO-8601 diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index e9d9c243e7c..1acbd85cea4 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -134,11 +134,18 @@ if MCP_AVAILABLE: from litellm.proxy._experimental.mcp_server.ui_session_utils import ( build_effective_auth_contexts, ) + from litellm.proxy._experimental.mcp_server.env_vars import ( + get_user_env_vars, + missing_required, + parse_env_var_definitions, + store_user_env_vars, + ) from litellm.proxy._types import ( LiteLLM_MCPServerTable, LitellmUserRoles, MakeMCPServersPublicRequest, MCPApprovalStatus, + MCPEnvVarDefinitionPublic, MCPOAuthUserCredentialRequest, MCPOAuthUserCredentialStatus, MCPSubmissionsSummary, @@ -146,6 +153,8 @@ if MCP_AVAILABLE: MCPUserCredentialListItem, MCPUserCredentialRequest, MCPUserCredentialResponse, + MCPUserEnvVarsRequest, + MCPUserEnvVarsStatus, NewMCPServerRequest, RejectMCPServerRequest, SpecialMCPServerName, @@ -2055,6 +2064,123 @@ if MCP_AVAILABLE: connected_at=cred.get("connected_at"), ) + # ── Per-user env-vars endpoints ────────────────────────────────────────── + # Variables are admin-defined on the MCP server (per_user vs instance scope). + # Per-user values land here so the proxy can interpolate `${VAR}` placeholders + # in static_headers at request time. Missing values surface as an MCP tool + # error with a deep link back to the dashboard's fill modal. + + @router.get( + "/server/{server_id}/my-env-vars", + description="Return the calling user's env-var values + per-user definitions for this MCP server", + dependencies=[Depends(user_api_key_auth)], + response_model=MCPUserEnvVarsStatus, + ) + @management_endpoint_wrapper + async def get_my_mcp_env_vars( + server_id: str, + user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), + ): + """Used by the fill modal (`FillUserFieldsModal`) to render the form + with current values pre-filled, and by the status pill + (`UserFieldsStatusCell`) to render the "N missing" badge. + + Returns only `per_user` definitions — `instance` values may be + secrets the admin doesn't want exposed to the end user. + """ + prisma_client = get_prisma_client_or_throw( + "Database not connected. Connect a database to your proxy" + ) + user_id = user_api_key_dict.user_id or "" + if not user_id: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={"error": "User ID not found in token"}, + ) + mcp_server = await get_mcp_server(prisma_client, server_id) + if mcp_server is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail={"error": f"MCP Server {server_id} not found"}, + ) + defs = parse_env_var_definitions(getattr(mcp_server, "env_vars", None)) + per_user_defs = [d for d in defs if d.scope == "per_user"] + values = await get_user_env_vars(prisma_client, user_id, server_id) + # Trim to declared per_user names so stale entries (after a rename or + # scope flip) don't leak back to the form. + declared_names = {d.name for d in per_user_defs} + trimmed_values = {k: v for k, v in values.items() if k in declared_names} + missing = missing_required( + defs, trimmed_values, referenced=[d.name for d in per_user_defs] + ) + return MCPUserEnvVarsStatus( + server_id=server_id, + server_alias=getattr(mcp_server, "alias", None), + definitions=[ + MCPEnvVarDefinitionPublic(name=d.name, scope=d.scope) + for d in per_user_defs + ], + values=trimmed_values, + missing=missing, + ) + + @router.post( + "/server/{server_id}/my-env-vars", + description="Store / replace the calling user's env-var values for this MCP server", + dependencies=[Depends(user_api_key_auth)], + response_model=MCPUserEnvVarsStatus, + ) + @management_endpoint_wrapper + async def store_my_mcp_env_vars( + server_id: str, + payload: MCPUserEnvVarsRequest, + user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), + ): + """Full-replace upsert of the caller's values for this server. + + Silently drops any names not declared as `per_user` on the server — + the form should never have rendered an input for them, but we don't + want a stale UI payload to pollute the stored map either. + """ + prisma_client = get_prisma_client_or_throw( + "Database not connected. Connect a database to your proxy" + ) + user_id = user_api_key_dict.user_id or "" + if not user_id: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={"error": "User ID not found in token"}, + ) + mcp_server = await get_mcp_server(prisma_client, server_id) + if mcp_server is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail={"error": f"MCP Server {server_id} not found"}, + ) + defs = parse_env_var_definitions(getattr(mcp_server, "env_vars", None)) + per_user_names = {d.name for d in defs if d.scope == "per_user"} + # Keep only declared per-user names; trim blanks (treat as "not set"). + clean_values = { + k: v + for k, v in payload.values.items() + if k in per_user_names and isinstance(v, str) and v.strip() != "" + } + await store_user_env_vars(prisma_client, user_id, server_id, clean_values) + per_user_defs = [d for d in defs if d.scope == "per_user"] + missing = missing_required( + defs, clean_values, referenced=[d.name for d in per_user_defs] + ) + return MCPUserEnvVarsStatus( + server_id=server_id, + server_alias=getattr(mcp_server, "alias", None), + definitions=[ + MCPEnvVarDefinitionPublic(name=d.name, scope=d.scope) + for d in per_user_defs + ], + values=clean_values, + missing=missing, + ) + @router.get( "/user-credentials", description="List all OAuth2 MCP credentials stored for the calling user", diff --git a/litellm/proxy/schema.prisma b/litellm/proxy/schema.prisma index 78143fe0411..5cecbe93261 100644 --- a/litellm/proxy/schema.prisma +++ b/litellm/proxy/schema.prisma @@ -311,6 +311,11 @@ model LiteLLM_MCPServerTable { tool_name_to_description Json? @default("{}") extra_headers String[] @default([]) static_headers Json? @default("{}") + // Per-server env-var definitions: [{name, scope: "instance"|"per_user", value}]. + // `${NAME}` placeholders in static_headers / credentials are substituted at + // request time. instance values come from this column; per_user values from + // LiteLLM_MCPUserCredentials with payload type="vars". + env_vars Json? @default("[]") // Health check status status String? @default("unknown") last_health_check DateTime? diff --git a/litellm/types/mcp_server/mcp_server_manager.py b/litellm/types/mcp_server/mcp_server_manager.py index 776c7fa67a6..1dda623bbae 100644 --- a/litellm/types/mcp_server/mcp_server_manager.py +++ b/litellm/types/mcp_server/mcp_server_manager.py @@ -42,6 +42,10 @@ class MCPServer(BaseModel): static_headers: Optional[Dict[str, str]] = ( None # static headers to forward to the MCP server ) + # Per-server variable definitions: list of {name, scope, value}. + # `${NAME}` placeholders in static_headers / authentication_token get + # substituted at request time via env_vars.resolve_values(). + env_vars: Optional[List[Dict[str, Any]]] = None # OAuth-specific fields client_id: Optional[str] = None client_secret: Optional[str] = None diff --git a/schema.prisma b/schema.prisma index 78143fe0411..5cecbe93261 100644 --- a/schema.prisma +++ b/schema.prisma @@ -311,6 +311,11 @@ model LiteLLM_MCPServerTable { tool_name_to_description Json? @default("{}") extra_headers String[] @default([]) static_headers Json? @default("{}") + // Per-server env-var definitions: [{name, scope: "instance"|"per_user", value}]. + // `${NAME}` placeholders in static_headers / credentials are substituted at + // request time. instance values come from this column; per_user values from + // LiteLLM_MCPUserCredentials with payload type="vars". + env_vars Json? @default("[]") // Health check status status String? @default("unknown") last_health_check DateTime? diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_env_vars.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_env_vars.py new file mode 100644 index 00000000000..ad56d146caf --- /dev/null +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_env_vars.py @@ -0,0 +1,350 @@ +"""Unit tests for the per-server / per-user env-var resolver. + +Covers the pure-Python pieces of `env_vars.py` — interpolation, missing-var +detection, deep-link generation. DB-backed helpers (`store_user_env_vars`, +`get_user_env_vars`) are exercised in the integration suite. +""" + +import os +from typing import Dict + +import pytest + +from litellm.proxy._experimental.mcp_server.env_vars import ( + EnvVarDefinition, + MissingEnvVarsError, + collect_placeholders, + interpolate_headers, + interpolate_value, + missing_required, + parse_env_var_definitions, + resolve_values, +) + + +class TestParseEnvVarDefinitions: + def test_none_returns_empty(self): + assert parse_env_var_definitions(None) == [] + + def test_empty_list_returns_empty(self): + assert parse_env_var_definitions([]) == [] + + def test_list_of_dicts(self): + defs = parse_env_var_definitions( + [ + {"name": "TOKEN", "scope": "per_user"}, + {"name": "HOST", "scope": "instance", "value": "db.corp"}, + ] + ) + assert len(defs) == 2 + assert defs[0].name == "TOKEN" + assert defs[0].scope == "per_user" + assert defs[1].value == "db.corp" + + def test_json_string_parsed(self): + defs = parse_env_var_definitions('[{"name": "TOKEN", "scope": "per_user"}]') + assert len(defs) == 1 + assert defs[0].name == "TOKEN" + + def test_invalid_json_string_returns_empty(self): + assert parse_env_var_definitions("not json {{{") == [] + + def test_malformed_entries_dropped_silently(self): + defs = parse_env_var_definitions( + [ + {"name": "GOOD", "scope": "per_user"}, + "not a dict", + {"missing_scope": "BAD"}, + None, + ] + ) + # Only the GOOD entry should survive. + names = [d.name for d in defs] + assert names == ["GOOD"] + + def test_pre_typed_entries_pass_through(self): + original = EnvVarDefinition(name="TOKEN", scope="per_user") + defs = parse_env_var_definitions([original]) + assert defs == [original] + + +class TestCollectPlaceholders: + def test_no_placeholders(self): + assert collect_placeholders(["plain string", "no vars here"]) == [] + + def test_single_placeholder(self): + assert collect_placeholders(["Bearer ${TOKEN}"]) == ["TOKEN"] + + def test_multiple_placeholders_dedup(self): + result = collect_placeholders( + [ + "Bearer ${TOKEN}", + "${HOST}:${PORT}/${TOKEN}", # TOKEN dup + ] + ) + # Order preserved, no dupes + assert result == ["TOKEN", "HOST", "PORT"] + + def test_ignores_none_and_empty(self): + assert collect_placeholders([None, "", "${A}"]) == ["A"] + + def test_lowercase_placeholders_ignored(self): + # Regex only matches UPPER_SNAKE_CASE — matches the UI's validator. + assert collect_placeholders(["${lowercase}", "${MixedCase}"]) == [] + + +class TestMissingRequired: + def test_no_referenced_returns_empty(self): + defs = [EnvVarDefinition(name="A", scope="per_user")] + assert missing_required(defs, {}, referenced=[]) == [] + + def test_per_user_with_value_not_missing(self): + defs = [EnvVarDefinition(name="TOKEN", scope="per_user")] + assert missing_required(defs, {"TOKEN": "abc"}, referenced=["TOKEN"]) == [] + + def test_per_user_empty_value_is_missing(self): + defs = [EnvVarDefinition(name="TOKEN", scope="per_user")] + assert missing_required(defs, {"TOKEN": ""}, referenced=["TOKEN"]) == ["TOKEN"] + + def test_instance_missing_is_not_reported(self): + # Admin misconfiguration (an instance var with no value) is an + # admin-side problem, not a user-side one. The resolver simply leaves + # the placeholder in place; we don't surface it as a deep-link prompt. + defs = [EnvVarDefinition(name="HOST", scope="instance", value=None)] + assert missing_required(defs, {}, referenced=["HOST"]) == [] + + def test_unknown_referenced_name_ignored(self): + # If a placeholder references a var that isn't declared, it's not + # "missing per-user" — it's just unresolvable. Don't shadow the + # admin-misconfig path. + defs = [EnvVarDefinition(name="OTHER", scope="per_user")] + assert missing_required(defs, {}, referenced=["UNRELATED"]) == [] + + +class TestResolveValues: + def test_per_user_wins_over_instance(self): + defs = [ + EnvVarDefinition(name="X", scope="instance", value="admin-default"), + EnvVarDefinition(name="X", scope="per_user"), + ] + result = resolve_values(defs, {"X": "user-value"}, referenced=["X"]) + assert result == {"X": "user-value"} + + def test_instance_used_when_no_per_user(self): + defs = [EnvVarDefinition(name="HOST", scope="instance", value="db.corp")] + assert resolve_values(defs, {}, referenced=["HOST"]) == {"HOST": "db.corp"} + + def test_unreferenced_vars_skipped(self): + # Critical: a stray missing per-user value must NOT block a request + # that doesn't reference it. + defs = [ + EnvVarDefinition(name="USED", scope="instance", value="ok"), + EnvVarDefinition(name="UNUSED", scope="per_user"), + ] + assert resolve_values(defs, {}, referenced=["USED"]) == {"USED": "ok"} + + def test_instance_with_empty_value_skipped(self): + defs = [EnvVarDefinition(name="X", scope="instance", value="")] + assert resolve_values(defs, {}, referenced=["X"]) == {} + + +class TestInterpolateHeaders: + def test_substitutes_known_placeholders(self): + headers = {"Authorization": "Bearer ${TOKEN}", "X-Host": "${HOST}"} + resolved = {"TOKEN": "abc123", "HOST": "db.corp"} + assert interpolate_headers(headers, resolved) == { + "Authorization": "Bearer abc123", + "X-Host": "db.corp", + } + + def test_leaves_unresolved_placeholders_in_place(self): + # Important: don't silently strip — caller should have already + # detected missing vars and short-circuited with a useful error. + headers = {"X-Token": "${MISSING}"} + assert interpolate_headers(headers, {}) == {"X-Token": "${MISSING}"} + + def test_none_passes_through(self): + assert interpolate_headers(None, {"X": "y"}) is None + + def test_returns_new_dict_not_mutated(self): + original = {"X": "${A}"} + interpolate_headers(original, {"A": "1"}) + assert original == {"X": "${A}"} + + def test_multiple_placeholders_in_one_value(self): + headers = { + "X-Conn": "${PROTO}://${USER}:${PASS}@${HOST}", + } + resolved = { + "PROTO": "postgresql", + "USER": "alice", + "PASS": "s3cr3t", + "HOST": "db.corp:5432", + } + assert interpolate_headers(headers, resolved) == { + "X-Conn": "postgresql://alice:s3cr3t@db.corp:5432", + } + + +class TestInterpolateValue: + def test_substitutes(self): + assert interpolate_value("Bearer ${T}", {"T": "x"}) == "Bearer x" + + def test_none_passes_through(self): + assert interpolate_value(None, {"T": "x"}) is None + + def test_no_placeholder(self): + assert interpolate_value("Bearer hardcoded", {"T": "x"}) == "Bearer hardcoded" + + +class TestMissingEnvVarsError: + def test_deep_link_uses_proxy_base_url(self, monkeypatch): + monkeypatch.delenv("PROXY_UI_BASE_URL", raising=False) + monkeypatch.setenv("PROXY_BASE_URL", "https://proxy.example.com") + e = MissingEnvVarsError( + server_alias="github_corp", + server_name="GitHub (corp)", + missing=["TOKEN"], + ) + assert ( + e.deep_link() + == "https://proxy.example.com/ui/tools/mcp-servers?fill_fields=github_corp" + ) + + def test_deep_link_trims_trailing_slash(self, monkeypatch): + monkeypatch.delenv("PROXY_UI_BASE_URL", raising=False) + monkeypatch.setenv("PROXY_BASE_URL", "https://proxy.example.com/") + e = MissingEnvVarsError( + server_alias="alias", + server_name=None, + missing=["X"], + ) + assert ( + e.deep_link() + == "https://proxy.example.com/ui/tools/mcp-servers?fill_fields=alias" + ) + + def test_deep_link_falls_back_to_localhost(self, monkeypatch): + monkeypatch.delenv("PROXY_UI_BASE_URL", raising=False) + monkeypatch.delenv("PROXY_BASE_URL", raising=False) + e = MissingEnvVarsError( + server_alias="x", + server_name=None, + missing=["X"], + ) + assert ( + e.deep_link() == "http://localhost:4000/ui/tools/mcp-servers?fill_fields=x" + ) + + def test_deep_link_respects_proxy_ui_base_url_override(self, monkeypatch): + # Dev workflow: UI on :3000, proxy on :4000. The UI override gets + # used verbatim — no `/ui` prefix is appended. + monkeypatch.setenv("PROXY_UI_BASE_URL", "http://localhost:3000") + monkeypatch.setenv("PROXY_BASE_URL", "http://localhost:4000") + e = MissingEnvVarsError( + server_alias="dev_server", + server_name=None, + missing=["TOKEN"], + ) + assert ( + e.deep_link() + == "http://localhost:3000/tools/mcp-servers?fill_fields=dev_server" + ) + + def test_user_message_includes_server_name_and_link(self, monkeypatch): + monkeypatch.delenv("PROXY_UI_BASE_URL", raising=False) + monkeypatch.setenv("PROXY_BASE_URL", "https://proxy.test") + e = MissingEnvVarsError( + server_alias="x", + server_name="My Server", + missing=["CORP_PASSWORD", "API_TOKEN"], + ) + msg = e.to_user_message() + assert 'Cannot connect to MCP server "My Server"' in msg + assert "CORP_PASSWORD" in msg + assert "API_TOKEN" in msg + assert "https://proxy.test/ui/tools/mcp-servers?fill_fields=x" in msg + + def test_user_message_falls_back_to_alias_when_no_name(self): + e = MissingEnvVarsError( + server_alias="my-alias", + server_name=None, + missing=["X"], + ) + assert 'Cannot connect to MCP server "my-alias"' in e.to_user_message() + + +class TestEndToEndResolveFlow: + """The pattern the manager follows: parse defs → collect refs → check + missing → resolve → interpolate. Locked in here so refactors of the + individual helpers can't drift the contract. + """ + + def _resolve_or_raise( + self, + static_headers: Dict[str, str], + defs_list, + per_user_values: Dict[str, str], + server_alias: str = "demo", + ) -> Dict[str, str]: + defs = parse_env_var_definitions(defs_list) + referenced = collect_placeholders(static_headers.values()) + missing = missing_required(defs, per_user_values, referenced=referenced) + if missing: + raise MissingEnvVarsError( + server_alias=server_alias, + server_name=server_alias, + missing=missing, + ) + resolved = resolve_values(defs, per_user_values, referenced=referenced) + return interpolate_headers(static_headers, resolved) + + def test_happy_path_per_user_and_instance(self): + headers = self._resolve_or_raise( + static_headers={ + "Authorization": "Bearer ${CORP_TOKEN}", + "X-Tenant": "${TENANT_ID}", + }, + defs_list=[ + {"name": "CORP_TOKEN", "scope": "per_user"}, + {"name": "TENANT_ID", "scope": "instance", "value": "tenant-42"}, + ], + per_user_values={"CORP_TOKEN": "user-secret"}, + ) + assert headers == { + "Authorization": "Bearer user-secret", + "X-Tenant": "tenant-42", + } + + def test_missing_per_user_raises_with_deep_link(self): + with pytest.raises(MissingEnvVarsError) as excinfo: + self._resolve_or_raise( + static_headers={"Authorization": "Bearer ${CORP_TOKEN}"}, + defs_list=[{"name": "CORP_TOKEN", "scope": "per_user"}], + per_user_values={}, + server_alias="github_corp", + ) + e = excinfo.value + assert e.missing == ["CORP_TOKEN"] + # Deep link respects the alias, not a slugified server_name. + assert "fill_fields=github_corp" in e.deep_link() + + def test_no_placeholders_skips_lookup_entirely(self): + # Server has no `${X}` placeholders → resolver should pass through + # cleanly even when defs are absent / per-user values are empty. + headers = self._resolve_or_raise( + static_headers={"X-Plain": "literal"}, + defs_list=[], + per_user_values={}, + ) + assert headers == {"X-Plain": "literal"} + + def test_unused_missing_per_user_does_not_block(self): + # Server declares a per-user var that nothing references → no error, + # the request goes through. + headers = self._resolve_or_raise( + static_headers={"X-Plain": "literal"}, + defs_list=[{"name": "UNUSED", "scope": "per_user"}], + per_user_values={}, + ) + assert headers == {"X-Plain": "literal"} diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py index f2fd73f3f22..f296b03c316 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py @@ -2430,6 +2430,40 @@ class TestMCPServerManagerReload: ] assert "Skipping MCP server bad-openapi-server" in caplog.text + def test_build_mcp_server_table_preserves_env_vars(self): + """Registry → API list rows must expose env_vars so the edit form can + repopulate the Variables section. Regression test: previously the + ``_build_mcp_server_table`` projection dropped this field, so opening + an existing server in the dashboard showed an empty list even when + the DB row had env-var definitions. + """ + try: + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + MCPServerManager, + ) + except ImportError: + pytest.skip("MCP server not available") + + manager = MCPServerManager() + env_vars = [ + {"name": "DB_HOST", "scope": "instance", "value": "db.corp"}, + {"name": "CORP_TOKEN", "scope": "per_user", "value": None}, + ] + server = MCPServer( + server_id="server-with-vars", + name="server", + transport=MCPTransport.http, + env_vars=env_vars, + ) + + table = manager._build_mcp_server_table(server) + assert table.env_vars == env_vars + + empty = manager._build_mcp_server_table( + server.model_copy(update={"env_vars": None}) + ) + assert empty.env_vars is None + @pytest.mark.asyncio async def test_call_mcp_tool_logs_failure_via_post_call_failure_hook(): diff --git a/ui/litellm-dashboard/src/components/mcp_tools/MCPServerCard.tsx b/ui/litellm-dashboard/src/components/mcp_tools/MCPServerCard.tsx index 935b9bb349a..7057fd6314a 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/MCPServerCard.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/MCPServerCard.tsx @@ -10,17 +10,18 @@ import { } from "@ant-design/icons"; import type { MCPServer } from "./types"; import { getMaskedAndFullUrl } from "./utils"; -import { - getEnvVarDefinitions, - getMissingUserFields, - subscribeEnvVarsChanged, -} from "./mock/mockMcpEnvVars"; +import { getMyMcpEnvVars } from "../networking"; const { Text } = Typography; interface MCPServerCardProps { server: MCPServer; userID: string; + accessToken?: string | null; + // Bumped by the parent after the fill-fields modal saves so the card + // re-fetches the user's missing-vars status. Throwaway demo wiring — + // a real impl would do a single bulk endpoint at the page level. + envVarsRefreshKey?: number; isLoadingHealth?: boolean; isRechecking?: boolean; onClick: () => void; @@ -42,6 +43,8 @@ const stop = (e: MouseEvent | KeyboardEvent) => e.stopPropagation(); const MCPServerCard: FC = ({ server, userID, + accessToken, + envVarsRefreshKey, isLoadingHealth, isRechecking, onClick, @@ -50,14 +53,6 @@ const MCPServerCard: FC = ({ onOpenFillFields, onDelete, }) => { - // Re-render whenever the mock env var store changes, so the missing-fields - // badge updates as soon as the user fills in values in the modal. - const [, setEnvTick] = useState(0); - useEffect( - () => subscribeEnvVarsChanged(() => setEnvTick((t) => t + 1)), - [], - ); - const alias = server.alias || server.server_name || ""; const name = server.server_name || alias || server.server_id; // Logo is sourced exclusively from the admin-set `mcp_info.logo_url`. @@ -80,9 +75,49 @@ const MCPServerCard: FC = ({ (g): g is string => typeof g === "string", ); - const envDefs = alias ? getEnvVarDefinitions(alias) : []; - const perUserCount = envDefs.filter((d) => d.scope === "per_user").length; - const missingUserFields = alias ? getMissingUserFields(alias, userID) : []; + // `env_vars` comes straight from the list endpoint (no extra HTTP). The + // missing-for-this-user check requires a per-server fetch, which is N+1 + // for the demo but cheap enough on a list of <50 servers. + const perUserDefNames = (server.env_vars ?? []) + .filter((d) => d && d.scope === "per_user" && d.name) + .map((d) => d.name); + const perUserCount = perUserDefNames.length; + // Stable string key so the effect doesn't refire on every render just + // because `.map().filter()` returns a fresh array reference. + const perUserDefNamesKey = perUserDefNames.join(","); + const [missingUserFields, setMissingUserFields] = useState([]); + useEffect(() => { + // Force the linter to see envVarsRefreshKey as "used"; the parent + // bumps it after the fill modal saves to drive a refetch. + void envVarsRefreshKey; + if (perUserCount === 0 || !accessToken || !userID) { + setMissingUserFields([]); + return; + } + let cancelled = false; + getMyMcpEnvVars(accessToken, server.server_id) + .then((status) => { + if (cancelled) return; + setMissingUserFields(status.missing ?? []); + }) + .catch(() => { + if (cancelled) return; + // Fall back to "all per-user fields missing" so the red state still + // surfaces even if the per-row fetch fails (better to over-warn than + // silently swallow it for the demo). + setMissingUserFields(perUserDefNamesKey ? perUserDefNamesKey.split(",") : []); + }); + return () => { + cancelled = true; + }; + }, [ + accessToken, + userID, + server.server_id, + perUserCount, + perUserDefNamesKey, + envVarsRefreshKey, + ]); const needsAttention = perUserCount > 0 && missingUserFields.length > 0; const cardClass = needsAttention diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx index 8474fda3471..68cb9d17b88 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx @@ -13,11 +13,7 @@ import MCPPermissionManagement from "./MCPPermissionManagement"; import OpenAPIFormSection, { OpenAPIKeyTool } from "./OpenAPIFormSection"; import MCPLogoSelector from "./MCPLogoSelector"; import EnvVarsSection from "./mock/EnvVarsSection"; -import { - setEnvVarDefinitions, - notifyEnvVarsChanged, - EnvVarDefinition, -} from "./mock/mockMcpEnvVars"; +import { EnvVarDefinition } from "./mock/mockMcpEnvVars"; import { isAdminRole } from "@/utils/roles"; import { validateMCPServerUrl, @@ -306,25 +302,15 @@ const CreateMCPServer: React.FC = ({ ...restValues } = values; - // PROTOTYPE: persist the env-var definitions to localStorage keyed by - // the server alias. Replaced once the backend stores these properly. const cleanedEnvVars: EnvVarDefinition[] = Array.isArray(mockEnvVarsRaw) ? mockEnvVarsRaw .filter((row: any) => row && row.name && String(row.name).trim() !== "") .map((row: any) => ({ name: String(row.name).trim(), value: row.scope === "per_user" ? "" : (row.value ?? ""), - scope: row.scope === "per_user" ? "per_user" : "global", + scope: row.scope === "per_user" ? "per_user" : "instance", })) : []; - const aliasForEnvVars = - (restValues.alias && String(restValues.alias).trim()) || - (restValues.server_name && String(restValues.server_name).trim()) || - ""; - if (aliasForEnvVars && cleanedEnvVars.length > 0) { - setEnvVarDefinitions(aliasForEnvVars, cleanedEnvVars); - notifyEnvVarsChanged(); - } // Transform access groups into objects with name property const accessGroups = restValues.mcp_access_groups; @@ -428,6 +414,7 @@ const CreateMCPServer: React.FC = ({ available_on_public_internet: Boolean(availableOnPublicInternetRaw), delegate_auth_to_upstream: Boolean(delegateAuthToUpstreamRaw), static_headers: staticHeaders, + env_vars: cleanedEnvVars, ...(tokenValidation !== null && { token_validation: tokenValidation }), }; @@ -1037,7 +1024,7 @@ const CreateMCPServer: React.FC = ({ - {/* PROTOTYPE: Environment variables (global vs per-user) */} + {/* PROTOTYPE: Variables (instance vs per-user) */}
diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_columns.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_columns.tsx index c8567926abd..4b7e768bc91 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_columns.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_columns.tsx @@ -305,11 +305,10 @@ export const mcpServerColumns = ( id: "mock_user_fields", header: "My Credentials", cell: ({ row }) => { - const alias = row.original.alias || row.original.server_name || ""; + const envVars = (row.original as any).env_vars; return ( onOpenFillFields?.(row.original)} onOpenDemo={() => onOpenMockDemo?.(row.original)} /> diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx index c76f09dfd47..02b40b08675 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx @@ -10,12 +10,7 @@ import MCPToolConfiguration from "./mcp_tool_configuration"; import StdioConfiguration from "./StdioConfiguration"; import MCPLogoSelector from "./MCPLogoSelector"; import EnvVarsSection from "./mock/EnvVarsSection"; -import { - getEnvVarDefinitions, - setEnvVarDefinitions, - notifyEnvVarsChanged, - EnvVarDefinition, -} from "./mock/mockMcpEnvVars"; +import { EnvVarDefinition } from "./mock/mockMcpEnvVars"; import { validateMCPServerUrl, validateMCPServerName, @@ -207,13 +202,12 @@ const MCPServerEdit: React.FC = ({ return mcpServer.transport; }, [mcpServer]); - // PROTOTYPE: existing env-var definitions for this server, loaded from - // localStorage by alias and folded into the form's initialValues below so - // antd's Form.List picks them up at mount time. + // Existing env-var definitions for this server, taken straight from the + // server record (DB-backed) so the Form.List picks them up at mount time. const initialEnvVars = React.useMemo(() => { - const aliasKey = mcpServer.alias || mcpServer.server_name || ""; - return aliasKey ? getEnvVarDefinitions(aliasKey) : []; - }, [mcpServer.alias, mcpServer.server_name]); + const raw = (mcpServer as any).env_vars; + return Array.isArray(raw) ? raw : []; + }, [mcpServer]); const initialValues = React.useMemo( () => ({ @@ -421,26 +415,15 @@ const MCPServerEdit: React.FC = ({ ...restValues } = values; - // PROTOTYPE: persist updated env-var definitions to localStorage under - // the current alias. If the alias was renamed in this edit, we also - // copy the previously-saved defs to the new alias so the link survives. const cleanedEnvVars: EnvVarDefinition[] = Array.isArray(mockEnvVarsRaw) ? mockEnvVarsRaw .filter((row: any) => row && row.name && String(row.name).trim() !== "") .map((row: any) => ({ name: String(row.name).trim(), value: row.scope === "per_user" ? "" : (row.value ?? ""), - scope: row.scope === "per_user" ? "per_user" : "global", + scope: row.scope === "per_user" ? "per_user" : "instance", })) : []; - const newAlias = - (restValues.alias && String(restValues.alias).trim()) || - (restValues.server_name && String(restValues.server_name).trim()) || - ""; - if (newAlias) { - setEnvVarDefinitions(newAlias, cleanedEnvVars); - notifyEnvVarsChanged(); - } const accessGroups = (restValues.mcp_access_groups || []).map((g: any) => typeof g === "string" ? g : g.name || String(g), @@ -604,6 +587,7 @@ const MCPServerEdit: React.FC = ({ tool_name_to_description: Object.keys(toolNameToDescription).length > 0 ? toolNameToDescription : null, disallowed_tools: restValues.disallowed_tools || [], static_headers: staticHeaders, + env_vars: cleanedEnvVars, allow_all_keys: Boolean(allowAllKeysRaw ?? mcpServer.allow_all_keys), available_on_public_internet: Boolean(availableOnPublicInternetRaw ?? mcpServer.available_on_public_internet), // ``delegate_auth_to_upstream`` is only honored server-side for @@ -1127,7 +1111,7 @@ const MCPServerEdit: React.FC = ({ )} - {/* PROTOTYPE: Environment variables (global vs per-user) */} + {/* PROTOTYPE: Variables (instance vs per-user) */}
diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_servers.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_servers.tsx index 729986cdfc6..ac2ea6afabd 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_servers.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_servers.tsx @@ -118,6 +118,10 @@ const MCPServers: React.FC = ({ accessToken, userRole, userID }) const [mockDemoServer, setMockDemoServer] = useState(null); const [searchQuery, setSearchQuery] = useState(""); const [sortKey, setSortKey] = useState("created_desc"); + // Bumped after the per-user fill modal saves so each MCPServerCard + // re-fetches its missing-fields status. Demo wiring — a real impl + // would use a single bulk endpoint with React Query invalidation. + const [envVarsRefreshKey, setEnvVarsRefreshKey] = useState(0); const isInternalUser = userRole === "Internal User"; // PROTOTYPE: deep-link via ?fill_fields= (used by the mock Claude @@ -552,6 +556,8 @@ const MCPServers: React.FC = ({ accessToken, userRole, userID }) key={server.server_id} server={server} userID={userID || ""} + accessToken={accessToken} + envVarsRefreshKey={envVarsRefreshKey} isLoadingHealth={isLoadingHealth} isRechecking={recheckingServerIds?.has(server.server_id)} onClick={() => { @@ -613,16 +619,24 @@ const MCPServers: React.FC = ({ accessToken, userRole, userID }) /> )} - {/* PROTOTYPE: per-user env-var fill modal */} - {fillFieldsServer && ( + {/* Per-user env-var fill modal — DB-backed via /v1/mcp/server/{id}/my-env-vars */} + {fillFieldsServer && accessToken && ( setFillFieldsServer(null)} + onSaved={() => { + // Refresh table so the per-user count / row data stays current, + // and bump the env-vars refresh key so each card re-fetches its + // missing-fields status (the red border + Set button driver). + refetch(); + setEnvVarsRefreshKey((k) => k + 1); + }} /> )} diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mock/EnvVarsSection.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mock/EnvVarsSection.tsx index 5386f47be44..30410a17983 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mock/EnvVarsSection.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mock/EnvVarsSection.tsx @@ -20,14 +20,14 @@ const EnvVarsSection: React.FC = () => { Prototype - Environment Variables + Variables - Define variables you can interpolate in Static Headers using{" "} - {"${VAR_NAME}"}.
- Global: admin-defined value used for every user. + Define variables you can interpolate in Static Headers or + Authentication using {"${VAR_NAME}"}.
+ Instance: admin-defined value used for every user.
Per-user: each user supplies their own value (e.g. personal credentials). @@ -38,8 +38,8 @@ const EnvVarsSection: React.FC = () => { - Reference these in Static Headers as {"${VAR_NAME}"}. For - example:{" "} + Reference these in Static Headers or Authentication as{" "} + {"${VAR_NAME}"}. For example:{" "} {"${DB_PROTOCOL}://${CORP_USERNAME}:${CORP_PASSWORD}@${DB_HOSTNAME}"} @@ -89,12 +89,12 @@ const EnvVarsSection: React.FC = () => { {...restField} name={[name, "scope"]} className="mb-0" - initialValue="global" + initialValue="instance" style={{ width: 160 }} >