mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-11 03:38:38 +00:00
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.
This commit is contained in:
parent
b8edaf62d9
commit
a9213735fc
3 changed files with 78 additions and 2 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 ----------
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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(<AgentsPanel accessToken="test-token" userRole="Admin" />);
|
||||
|
||||
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(<AgentsPanel accessToken="test-token" userRole="Admin" />);
|
||||
await waitFor(() => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue