From a9213735fc99be5c3c627c88e1f6d7ef1bb1e283 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Wed, 3 Jun 2026 14:37:08 -0700 Subject: [PATCH] fix(agents): redact attached virtual keys for non-admins _attach_keys_to_agents joins keys onto the agent response by agent_id with no caller scoping, but _redact_sensitive_agent_fields never cleared the new keys field. A non-admin able to view an agent therefore received the alias, masked name, and hashed token of every key attached to it, including keys owned by other users or teams; the old client-side path used the scoped key list, so this was a visibility regression. Clear keys in the redaction path so only admins see attached-key metadata. Adds an endpoint-level regression test asserting keys is populated for admins and null for non-admins, and a list-view test covering the Active vs Needs Setup badge that lost coverage when the agent card tests were removed. --- litellm/proxy/agent_endpoints/endpoints.py | 1 + .../proxy/agent_endpoints/test_endpoints.py | 49 ++++++++++++++++++- .../src/components/agents.test.tsx | 30 +++++++++++- 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/litellm/proxy/agent_endpoints/endpoints.py b/litellm/proxy/agent_endpoints/endpoints.py index ad415d4fb0b..2fe79790634 100644 --- a/litellm/proxy/agent_endpoints/endpoints.py +++ b/litellm/proxy/agent_endpoints/endpoints.py @@ -105,6 +105,7 @@ def _redact_sensitive_agent_fields( copy = agent.model_copy(deep=True) copy.static_headers = None copy.extra_headers = None + copy.keys = None if copy.litellm_params: copy.litellm_params = _get_masked_values( copy.litellm_params, diff --git a/tests/test_litellm/proxy/agent_endpoints/test_endpoints.py b/tests/test_litellm/proxy/agent_endpoints/test_endpoints.py index 9fcf4a71d39..0cb03023e0f 100644 --- a/tests/test_litellm/proxy/agent_endpoints/test_endpoints.py +++ b/tests/test_litellm/proxy/agent_endpoints/test_endpoints.py @@ -318,10 +318,57 @@ async def test_attach_keys_to_agents_groups_by_agent_and_omits_secret(): # Only summary fields are exposed; the row's user_id must not be carried. summary = agent_with_keys.keys[0] - assert not hasattr(summary, "user_id") assert set(summary.model_dump().keys()) == {"token", "key_alias", "key_name"} +class TestAgentByIdKeyRedaction: + """GET /v1/agents/{id} surfaces attached keys to admins but never to + non-admins, even when the agent has keys attached.""" + + @pytest.fixture(autouse=True) + def _setup(self, monkeypatch): + self.mock_registry = MagicMock() + self.mock_registry.get_agent_by_id = MagicMock( + return_value=_sample_agent_response() + ) + monkeypatch.setattr(agent_endpoints, "AGENT_REGISTRY", self.mock_registry) + + def _get_as(self, role: LitellmUserRoles): + key_row = MagicMock() + key_row.token = "hash-aaa" + key_row.agent_id = "agent-123" + key_row.key_alias = "primary" + key_row.key_name = "sk-...aaa" + + test_client = _make_app_with_role(role) + with patch("litellm.proxy.proxy_server.prisma_client") as mock_prisma: + mock_prisma.db.litellm_agentstable.find_unique = AsyncMock( + return_value=None + ) + mock_prisma.db.litellm_verificationtoken.find_many = AsyncMock( + return_value=[key_row] + ) + return test_client.get( + "/v1/agents/agent-123", headers={"Authorization": "Bearer k"} + ) + + def test_admin_sees_attached_keys(self): + resp = self._get_as(LitellmUserRoles.PROXY_ADMIN) + assert resp.status_code == 200 + keys = resp.json()["keys"] + assert keys is not None + assert keys[0] == { + "token": "hash-aaa", + "key_alias": "primary", + "key_name": "sk-...aaa", + } + + def test_non_admin_never_sees_keys(self): + resp = self._get_as(LitellmUserRoles.INTERNAL_USER) + assert resp.status_code == 200 + assert resp.json()["keys"] is None + + # ---------- RBAC enforcement tests ---------- diff --git a/ui/litellm-dashboard/src/components/agents.test.tsx b/ui/litellm-dashboard/src/components/agents.test.tsx index 7a4b0b5f8b3..848b8d5e891 100644 --- a/ui/litellm-dashboard/src/components/agents.test.tsx +++ b/ui/litellm-dashboard/src/components/agents.test.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { render, screen, waitFor, act, fireEvent } from "@testing-library/react"; +import { render, screen, waitFor, act, fireEvent, within } from "@testing-library/react"; import { describe, it, expect, vi, beforeEach } from "vitest"; import AgentsPanel from "./agents"; import * as networking from "./networking"; @@ -80,6 +80,34 @@ describe("AgentsPanel", () => { }); }); + it("should show Active when an agent has keys and Needs Setup when it has none", async () => { + vi.mocked(networking.getAgentsList).mockResolvedValue({ + agents: [ + { + agent_id: "agent-with-key", + agent_name: "Keyed Agent", + litellm_params: { model: "gpt-4" }, + spend: 0, + keys: [{ token: "hash-aaa", key_alias: "primary", key_name: "sk-...aaa" }], + }, + { + agent_id: "agent-no-key", + agent_name: "Keyless Agent", + litellm_params: { model: "gpt-4" }, + spend: 0, + keys: [], + }, + ], + }); + + render(); + + const keyedRow = (await screen.findByText("Keyed Agent")).closest("tr")!; + const keylessRow = screen.getByText("Keyless Agent").closest("tr")!; + expect(within(keyedRow).getByText("Active")).toBeInTheDocument(); + expect(within(keylessRow).getByText("Needs Setup")).toBeInTheDocument(); + }); + it("should call getAgentsList with health_check=true when toggle is enabled", async () => { render(); await waitFor(() => {