Skip to content

fix(mcp): highlight MCP cards red when the logged-in user is missing per-user env vars #29856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mateo-berri merged 2 commits into from
Jun 6, 2026
+77 −30
Merged

fix(mcp): highlight MCP cards red when the logged-in user is missing per-user env vars #29856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
37b4aef
fix(mcp): flag missing per-user env vars on the card for every access…
mateo-berri Jun 6, 2026
52ad80b
test(mcp): drop dead get_all_mcp_servers_for_user patch from view_all…
mateo-berri Jun 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 29 additions & 27 deletions litellm/proxy/management_endpoints/mcp_management_endpoints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,32 @@ async def _get_team_scoped_mcp_server_list(

return _redact_mcp_credentials_list(servers)

async def _resolve_accessible_mcp_servers(
user_api_key_dict: UserAPIKeyAuth,
) -> List[LiteLLM_MCPServerTable]:
"""The server set the dashboard grid shows (GET /v1/mcp/server, no team
filter), returned unredacted. Callers that surface this to a client must
apply their own redaction; the per-user env-var status endpoint relies on
the raw env_vars and only ever returns is_set booleans, never secrets.

Sharing this resolution keeps the red "missing user fields" card status
aligned with the cards actually rendered: an admin in view_all mode sees
every server even when their key carries no per-server MCP grant.
"""
if (
_get_user_mcp_management_mode() == "view_all"
and not _is_restricted_virtual_key_request(user_api_key_dict)
):
return await global_mcp_server_manager.get_all_mcp_servers_unfiltered()

aggregated: Dict[str, LiteLLM_MCPServerTable] = {}
for auth_context in await build_effective_auth_contexts(user_api_key_dict):
for server in await global_mcp_server_manager.get_all_allowed_mcp_servers(
user_api_key_auth=auth_context
):
aggregated.setdefault(server.server_id, server)
return list(aggregated.values())

@router.get(
"/server",
description="Returns the mcp server list with associated teams",
Expand Down Expand Up @@ -950,30 +976,8 @@ async def fetch_all_mcp_servers(
sanitized_team_id
)
else:
user_mcp_management_mode = _get_user_mcp_management_mode()

if user_mcp_management_mode == "view_all" and not is_restricted_virtual_key:
servers = (
await global_mcp_server_manager.get_all_mcp_servers_unfiltered()
)
redacted_mcp_servers = _redact_mcp_credentials_list(servers)
else:
auth_contexts = await build_effective_auth_contexts(user_api_key_dict)

aggregated_servers: Dict[str, LiteLLM_MCPServerTable] = {}
for auth_context in auth_contexts:
servers = (
await global_mcp_server_manager.get_all_allowed_mcp_servers(
user_api_key_auth=auth_context
)
)
for server in servers:
if server.server_id not in aggregated_servers:
aggregated_servers[server.server_id] = server

redacted_mcp_servers = _redact_mcp_credentials_list(
aggregated_servers.values()
)
servers = await _resolve_accessible_mcp_servers(user_api_key_dict)
redacted_mcp_servers = _redact_mcp_credentials_list(servers)

# augment the mcp servers with public status
if litellm.public_mcp_servers is not None:
Expand Down Expand Up @@ -2391,9 +2395,7 @@ async def list_mcp_user_env_var_status(
user_id = user_api_key_dict.user_id or ""
if not user_id:
return []
accessible = await get_all_mcp_servers_for_user(
prisma_client, user_api_key_dict
)
accessible = await _resolve_accessible_mcp_servers(user_api_key_dict)

@veria-ai veria-ai Bot Jun 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low: Env-var metadata exposed outside the user's MCP grants

_resolve_accessible_mcp_servers() returns every server in view_all mode for any non-route-restricted caller, but this endpoint returns the per-user env var names/descriptions from those unredacted server objects. A normal authenticated user can call /v1/mcp/user-env-vars/status and enumerate credential-field metadata for servers their key/team cannot access through the per-server env-var endpoints, which still use the narrower authorization check. Keep this bulk status path scoped to the user's effective allowed servers for non-admins, or only use the view_all/unfiltered branch for admin-view callers.

@mateo-berri mateo-berri Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the careful look. I dug into this and I don't think it widens exposure for this PR.

The status endpoint and the card grid (GET /v1/mcp/server) now resolve their server set through the same _resolve_accessible_mcp_servers helper, so every server whose per-user var status is reported here is one the same caller already sees as a card on their dashboard. This endpoint cannot surface a server, or any field of one, that the grid would not already show that caller.

What it returns is also tightly limited. _compute_user_env_var_status only emits scope="user" variables that are actually blocking (referenced by a static header with no admin global fallback), and only as name + description + is_set; admin-configured scope="global" secret names and every stored value are filtered out before the response is built, so no admin credential and no stored secret is ever exposed here.

This is also the disclosure path the code already designates for non-admins. _sanitize_mcp_server_for_non_admin deliberately strips env_vars from the grid so the per-user subset comes from /user-env-vars/status instead (see the comment at litellm/proxy/management_endpoints/mcp_management_endpoints.py:548-552). Scoping this endpoint back to the narrow key grant would reintroduce the exact bug the PR fixes, where a card the user sees on the grid never lights up red for its missing per-user vars.

The only mode that returns the full inventory to a non-admin is view_all, an opt-in general setting (user_mcp_management_mode: view_all) whose defined behavior is to show every server to every dashboard user; the grid has resolved it that way for non-admins since before this PR. The default restricted mode keeps both surfaces scoped to the user's effective allowed servers

if not accessible:
return []
server_ids = [s.server_id for s in accessible]
Expand Down
51 changes: 48 additions & 3 deletions tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4146,7 +4146,7 @@ async def test_no_accessible_servers_returns_empty(self):
),
patch.object(
mgmt_endpoints,
"get_all_mcp_servers_for_user",
"_resolve_accessible_mcp_servers",
AsyncMock(return_value=[]),
),
):
Expand Down Expand Up @@ -4174,7 +4174,7 @@ async def test_only_servers_with_required_fields_are_returned(self):
),
patch.object(
mgmt_endpoints,
"get_all_mcp_servers_for_user",
"_resolve_accessible_mcp_servers",
AsyncMock(return_value=[server_with, server_without]),
),
patch.object(
Expand Down Expand Up @@ -4204,7 +4204,7 @@ async def test_bulk_status_omits_stored_credential_values(self):
),
patch.object(
mgmt_endpoints,
"get_all_mcp_servers_for_user",
"_resolve_accessible_mcp_servers",
AsyncMock(return_value=[server]),
),
patch.object(
Expand All @@ -4221,6 +4221,51 @@ async def test_bulk_status_omits_stored_credential_values(self):
assert by_name["CORP_PASSWORD"].is_set is False
assert "alice" not in result[0].model_dump_json()

@pytest.mark.asyncio
async def test_admin_view_all_flags_missing_fields_without_key_grants(self):
"""Regression: the red "user fields missing" card must light up for an
admin in view_all mode even when their key carries no per-server MCP
grant. The bulk status feed has to resolve the same server set the
dashboard grid renders; the old narrow key-scoped listing returned
nothing for such an admin, leaving every card un-highlighted."""
server = _make_env_var_server(
server_id="srv-with",
env_vars=_ENV_VARS_MIXED,
static_headers=_STATIC_HEADERS_MIXED,
)
with (
patch.object(
mgmt_endpoints, "get_prisma_client_or_throw", return_value=MagicMock()
),
patch.object(
mgmt_endpoints,
"_get_user_mcp_management_mode",
return_value="view_all",
),
patch.object(
mgmt_endpoints.global_mcp_server_manager,
"get_all_mcp_servers_unfiltered",
AsyncMock(return_value=[server]),
),
patch.object(
mgmt_endpoints,
"get_user_env_vars_bulk",
AsyncMock(return_value={}),
),
):
result = await mgmt_endpoints.list_mcp_user_env_var_status(
user_api_key_dict=generate_mock_user_api_key_auth(
user_id="admin",
user_role=LitellmUserRoles.PROXY_ADMIN,
)
)
assert [s.server_id for s in result] == ["srv-with"]
assert result[0].missing_count == 2
assert {f.name for f in result[0].required} == {
"CORP_USERNAME",
"CORP_PASSWORD",
}


class TestMCPUserEnvVarsAccessControl:
"""Per-server env-var endpoints must enforce the same access gate as
Expand Down
Loading