diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 2609f5cb400..7ba3d03bdba 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -1552,11 +1552,14 @@ class MCPUserEnvVarsRequest(LiteLLMPydanticObjectBase): class MCPUserEnvVarSpec(LiteLLMPydanticObjectBase): - """Describes one per-user env var slot for the calling user.""" + """Describes one per-user env var slot for the calling user. + + Stored values are write-only: the status only reports whether a value + ``is_set`` and never echoes the decrypted secret back to the client. + """ name: str description: Optional[str] = None - value: Optional[str] = None # current value if the user has set one is_set: bool = False diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index d224dd262c9..6273678ac6e 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -2178,13 +2178,12 @@ if MCP_AVAILABLE: *, server: LiteLLM_MCPServerTable, stored_values: Dict[str, str], - include_values: bool = True, ) -> MCPUserEnvVarsStatus: """Build a status object for one server given the user's stored values. - ``include_values=False`` omits the stored credential values from the - response (the bulk status feed only needs ``is_set`` for the dashboard - badge, so there's no reason to echo secrets back across every server). + Stored credentials are write-only: the response reports only whether + each value ``is_set`` and never echoes the decrypted secret back, so a + leaked token can't be used to exfiltrate the raw upstream credential. """ _, user_specs = parse_admin_env_vars(getattr(server, "env_vars", None)) @@ -2214,7 +2213,6 @@ if MCP_AVAILABLE: MCPUserEnvVarSpec( name=name, description=spec.get("description"), - value=value if include_values else None, is_set=is_set, ) ) @@ -2354,7 +2352,7 @@ if MCP_AVAILABLE: for server in accessible: stored = stored_bulk.get(server.server_id, {}) status_obj = _compute_user_env_var_status( - server=server, stored_values=stored, include_values=False + server=server, stored_values=stored ) if status_obj.required: statuses.append(status_obj) diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index 4a2581e872a..873831341b6 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -3613,9 +3613,10 @@ class TestComputeUserEnvVarStatus: assert names == {"CORP_USERNAME", "CORP_PASSWORD"} by_name = {spec.name: spec for spec in status.required} assert by_name["CORP_USERNAME"].is_set is True - assert by_name["CORP_USERNAME"].value == "alice" assert by_name["CORP_USERNAME"].description == "Your username" assert by_name["CORP_PASSWORD"].is_set is False + # Stored credentials are write-only: the secret is never echoed back. + assert "alice" not in status.model_dump_json() assert status.missing_count == 1 assert status.server_id == "srv-1" assert status.server_name == "DB Server" @@ -3696,10 +3697,12 @@ class TestGetMCPUserEnvVars: assert result.server_id == "srv-1" assert result.missing_count == 1 assert {s.name for s in result.required} == {"CORP_USERNAME", "CORP_PASSWORD"} - # The single-server endpoint backs the fill-in modal, so it must echo the - # already-stored value back for pre-population. + # The single-server endpoint reports which credentials are set without + # ever echoing the decrypted secret back to the caller. by_name = {s.name: s for s in result.required} - assert by_name["CORP_USERNAME"].value == "alice" + assert by_name["CORP_USERNAME"].is_set is True + assert by_name["CORP_PASSWORD"].is_set is False + assert "alice" not in result.model_dump_json() @pytest.mark.asyncio async def test_missing_user_id_raises_400(self): @@ -3968,9 +3971,8 @@ class TestListMCPUserEnvVarStatus: ) by_name = {s.name: s for s in result[0].required} assert by_name["CORP_USERNAME"].is_set is True - assert by_name["CORP_USERNAME"].value is None assert by_name["CORP_PASSWORD"].is_set is False - assert by_name["CORP_PASSWORD"].value is None + assert "alice" not in result[0].model_dump_json() class TestMCPUserEnvVarsAccessControl: diff --git a/ui/litellm-dashboard/src/components/mcp_tools/UserEnvVarsModal.tsx b/ui/litellm-dashboard/src/components/mcp_tools/UserEnvVarsModal.tsx index 17d1f57c44b..f9cb3f9318b 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/UserEnvVarsModal.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/UserEnvVarsModal.tsx @@ -47,11 +47,7 @@ const UserEnvVarsModal: React.FC = ({ const fetched = await getMCPUserEnvVars(accessToken, server.server_id); if (cancelled) return; setStatus(fetched); - const initial: Record = {}; - for (const spec of fetched?.required ?? []) { - initial[spec.name] = spec.value ?? ""; - } - form.setFieldsValue(initial); + form.resetFields(); } catch (err) { if (!cancelled) { NotificationsManager.fromBackend( @@ -128,7 +124,8 @@ const UserEnvVarsModal: React.FC = ({ <> These values are private to you. Your admin configured this MCP - server to require these per-user credentials: + server to require these per-user credentials. Saved values are + never shown back; re-enter a value to update it.
= ({ key={spec.name} name={spec.name} label={ - - {spec.name} + + + {spec.name} + + {spec.is_set && Set} } extra={spec.description || undefined} rules={[{ required: true, message: `${spec.name} is required` }]} > diff --git a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx index ff564db901d..0e71f4e2cca 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/types.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/types.tsx @@ -261,7 +261,6 @@ export interface MCPEnvVar { export interface MCPUserEnvVarSpec { name: string; description?: string | null; - value?: string | null; is_set: boolean; }