diff --git a/litellm/proxy/agent_endpoints/endpoints.py b/litellm/proxy/agent_endpoints/endpoints.py index b6c41a17503..10efd71b84c 100644 --- a/litellm/proxy/agent_endpoints/endpoints.py +++ b/litellm/proxy/agent_endpoints/endpoints.py @@ -12,6 +12,7 @@ import asyncio import os import uuid from collections.abc import Mapping, Sequence +from itertools import chain from types import MappingProxyType from typing import Annotated, Final, TypedDict, assert_never @@ -139,6 +140,68 @@ async def _attach_keys_to_agents(agents: Sequence[AgentResponse], prisma_client) agent.keys = matched_keys or None +_SECRET_PARAMS_MAX_DEPTH: Final = 10 + + +def _mask_secret_params(params: dict[str, object], _depth: int = 0) -> dict[str, object]: # mutable-ok: dict field + """Masks credential-like keys at every depth, not only under a credential-like parent; masks all past the cap.""" + if _depth >= _SECRET_PARAMS_MAX_DEPTH: + return {key: "*****" for key in params} # mutable-ok: litellm_params is a dict field + masked: Final = _get_masked_values(params, unmasked_length=4, number_of_asterisks=4) + return { # mutable-ok: litellm_params is a dict field + key: _mask_secret_params(value, _depth + 1) if isinstance(value, dict) else masked[key] + for key, value in params.items() + } + + +def _mask_agent_secrets(agent: AgentResponse) -> AgentResponse: + if not agent.litellm_params: + return agent + return agent.model_copy(update=MappingProxyType({"litellm_params": _mask_secret_params(agent.litellm_params)})) + + +def _restore_stored_value(existing: object, masked: object, incoming: object, depth: int) -> object: + if isinstance(existing, dict) and isinstance(incoming, dict): + return _restore_stored_secrets(existing, incoming, depth) + return existing if incoming == masked else incoming + + +def _restore_stored_secrets( + existing_params: object, + incoming_params: Mapping[str, object] | None, + _depth: int = 0, +) -> dict[str, object]: # mutable-ok: AgentConfig.litellm_params is a dict field + """A secret the client only ever saw masked (omitted, or echoed back masked) keeps its stored value.""" + incoming: Final = incoming_params or MappingProxyType({}) + if not isinstance(existing_params, dict) or _depth >= _SECRET_PARAMS_MAX_DEPTH: + return dict(incoming) # mutable-ok: AgentConfig.litellm_params is a dict field + masked_existing: Final = _mask_secret_params(existing_params, _depth) + restored: Final = ( + ( + key, + _restore_stored_value(existing_params[key], masked_existing[key], value, _depth + 1) + if key in existing_params + else value, + ) + for key, value in incoming.items() + ) + preserved: Final = ( + (key, value) for key, value in existing_params.items() if key not in incoming and masked_existing[key] != value + ) + return dict(chain(restored, preserved)) # mutable-ok: AgentConfig.litellm_params is a dict field + + +def _patch_with_stored_secrets(request: PatchAgentRequest, existing_params: object) -> PatchAgentRequest: + incoming: Final = request.get("litellm_params") + if incoming is None: + return request + patched: Final[PatchAgentRequest] = { + **request, + "litellm_params": _restore_stored_secrets(existing_params, incoming), + } + return patched + + def _redact_sensitive_agent_fields( agents: Sequence[AgentResponse], ) -> list[AgentResponse]: @@ -152,12 +215,6 @@ def _redact_sensitive_agent_fields( copy.static_headers = None copy.extra_headers = None copy.keys = None - if copy.litellm_params: - copy.litellm_params = _get_masked_values( - copy.litellm_params, - unmasked_length=4, - number_of_asterisks=4, - ) redacted.append(copy) return redacted @@ -345,13 +402,12 @@ async def get_agents( global_agent_registry.ids_for_agent(agent.agent_id).isdisjoint(litellm.public_agent_groups) ) - # Redact sensitive fields for non-admin users + masked_agents: Final = tuple(_mask_agent_secrets(agent) for agent in returned_agents) is_admin: Final = ( user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN or user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value ) - if not is_admin: - returned_agents = _redact_sensitive_agent_fields(returned_agents) + returned_agents = masked_agents if is_admin else _redact_sensitive_agent_fields(masked_agents) if health_check: agents_with_url: Final = [agent for agent in returned_agents if (agent.agent_card_params or {}).get("url")] @@ -505,7 +561,7 @@ async def create_agent( "Failed to register agent '%s' (ID: %s) in memory: %s", agent_name, agent_id, reg_error ) - return result + return _mask_agent_secrets(result) except HTTPException: raise @@ -578,15 +634,12 @@ async def get_agent_by_id( await _attach_keys_to_agents([agent], prisma_client) - # Redact sensitive fields for non-admin users - is_admin = ( + masked_agent: Final = _mask_agent_secrets(agent) + is_admin: Final = ( user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN or user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value ) - if not is_admin: - agent = _redact_sensitive_agent_fields([agent])[0] - - return agent + return masked_agent if is_admin else _redact_sensitive_agent_fields([masked_agent])[0] except HTTPException: raise except Exception as e: @@ -662,7 +715,12 @@ async def update_agent( # ``agent_card_params`` skip the merge so we don't synthesise an A2A # card for them. upstream_card: Final = request.get("agent_card_params") - agent_to_update: AgentConfig = request + agent_to_update: AgentConfig = { + **request, + "litellm_params": _restore_stored_secrets( + existing_agent.get("litellm_params"), request.get("litellm_params") + ), + } if upstream_card is not None: merged_card: Final = _build_merged_agent_card( upstream_card, @@ -670,7 +728,7 @@ async def update_agent( http_request=http_request, agent_name=request.get("agent_name"), ) - agent_to_update = {**request, "agent_card_params": merged_card} + agent_to_update = {**agent_to_update, "agent_card_params": merged_card} result: Final = await AGENT_REGISTRY.update_agent_in_db( agent_id=agent_id, @@ -688,7 +746,7 @@ async def update_agent( "Successfully updated agent '%s' (ID: %s) in memory", existing_agent.get("agent_name"), agent_id ) - return result + return _mask_agent_secrets(result) except HTTPException: raise except Exception as e: @@ -764,7 +822,7 @@ async def patch_agent( # ``agent_card_params`` — even an empty dict — still goes through the # merge so LiteLLM applies its security schemes and supported # interfaces instead of storing a bare card. - patch_payload: PatchAgentRequest = request + patch_payload: PatchAgentRequest = _patch_with_stored_secrets(request, existing_agent.get("litellm_params")) upstream_card: Final = request.get("agent_card_params") if upstream_card is not None: merged_card: Final = _build_merged_agent_card( @@ -773,7 +831,7 @@ async def patch_agent( http_request=http_request, agent_name=request.get("agent_name"), ) - patch_payload = {**request, "agent_card_params": merged_card} + patch_payload = {**patch_payload, "agent_card_params": merged_card} result: Final = await AGENT_REGISTRY.patch_agent_in_db( agent_id=agent_id, @@ -791,7 +849,7 @@ async def patch_agent( "Successfully updated agent '%s' (ID: %s) in memory", existing_agent.get("agent_name"), agent_id ) - return result + return _mask_agent_secrets(result) except HTTPException: raise except Exception as e: diff --git a/tests/code_coverage_tests/recursive_detector.py b/tests/code_coverage_tests/recursive_detector.py index 790956156b0..12fb4f560fa 100644 --- a/tests/code_coverage_tests/recursive_detector.py +++ b/tests/code_coverage_tests/recursive_detector.py @@ -64,6 +64,7 @@ IGNORE_FUNCTIONS = [ "_flatten_form_field", # bounded by the nesting depth of the already-parsed request body (a finite JSON tree, no cycles possible). "_flatten_form_data_field", # bounded by the nesting depth of the already-parsed request body (a finite JSON tree, no cycles possible). "_json_safe", # max depth set (_MAX_DEPTH) plus a seen-ids cycle guard for self-referential input. + "_mask_secret_params", # max depth set (_SECRET_PARAMS_MAX_DEPTH=10); fails closed by masking every value at the cap. ] diff --git a/tests/test_litellm/proxy/agent_endpoints/test_endpoints.py b/tests/test_litellm/proxy/agent_endpoints/test_endpoints.py index ea196bda529..58debc5a90b 100644 --- a/tests/test_litellm/proxy/agent_endpoints/test_endpoints.py +++ b/tests/test_litellm/proxy/agent_endpoints/test_endpoints.py @@ -544,7 +544,7 @@ class TestAgentRBACProxyAdminViewOnly: def test_should_still_redact_secrets_for_view_only_admin(self): """An unrestricted viewer sees the same agents as an admin but with keys - stripped and litellm_params masked.""" + stripped. litellm_params secrets are masked for both roles.""" self.allowed_agents_spy.return_value = UnrestrictedAgentAccess() viewer_resp = self._list_agents(self.viewer_client) admin_resp = self._list_agents(self.admin_client) @@ -557,10 +557,9 @@ class TestAgentRBACProxyAdminViewOnly: admin_by_id = {agent["agent_id"]: agent for agent in admin_resp.json()} assert admin_by_id["agent-1"]["keys"][0]["token"] == "hash-aaa" - assert ( - admin_by_id["agent-1"]["litellm_params"]["api_key"] - == "sk-super-secret-agent-key" - ) + assert admin_by_id["agent-1"]["litellm_params"]["api_key"] == "sk****ey" + assert "sk-super-secret-agent-key" not in admin_resp.text + assert self.agents[0].litellm_params["api_key"] == "sk-super-secret-agent-key" class TestAgentRBACProxyAdmin: @@ -634,6 +633,180 @@ class TestAgentRBACProxyAdmin: assert resp.status_code == 200 +SENTINEL_SECRET = "SENTINEL_SECRET_abcdef1234" +MASKED_SENTINEL = "SE****34" + + +def _agentcore_litellm_params(secret: str = SENTINEL_SECRET) -> dict: + return { + "model": "bedrock/agentcore/arn:aws:bedrock-agentcore:us-east-1:123456789012:runtime/demo", + "aws_region_name": "us-east-1", + "aws_access_key_id": "AKIAFAKEKEYID0001", + "aws_secret_access_key": secret, + } + + +def _databricks_oauth_params(secret: str = SENTINEL_SECRET) -> dict: + return { + "client_id": "client-id-0001", + "client_secret": secret, + "workspace_url": "https://dbc-abc123.example.com", + } + + +class TestAgentSecretsNeverReturned: + """Credential-like litellm_params are write-only: every /v1/agents response + masks them, for full proxy admins too, and an update that omits a secret or + echoes the masked form keeps the stored value.""" + + @pytest.fixture(autouse=True) + def _setup(self, monkeypatch): + self.admin_client = _make_app_with_role(LitellmUserRoles.PROXY_ADMIN) + self.mock_registry = MagicMock() + self.mock_registry.register_agent = MagicMock() + self.mock_registry.deregister_agent = MagicMock() + monkeypatch.setattr(agent_endpoints, "AGENT_REGISTRY", self.mock_registry) + self.stored_agent = AgentResponse( + agent_id="agent-123", + agent_name="agentcore", + agent_card_params=_sample_agent_card_params(), + litellm_params=_agentcore_litellm_params(), + ) + self.existing_row = { + "agent_id": "agent-123", + "agent_name": "agentcore", + "agent_card_params": _sample_agent_card_params(), + "litellm_params": _agentcore_litellm_params(), + } + mock_prisma = MagicMock() + mock_prisma.db.litellm_agentstable.find_unique = AsyncMock(side_effect=lambda **_: self.existing_row) + mock_prisma.db.litellm_verificationtoken.find_many = AsyncMock(return_value=[]) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma) + + def _request(self, method: str, path: str, **kwargs): + return self.admin_client.request(method, path, headers={"Authorization": "Bearer k"}, **kwargs) + + def test_create_response_masks_secret_but_stores_and_registers_it_raw(self): + self.mock_registry.get_agent_by_name = MagicMock(return_value=None) + self.mock_registry.add_agent_to_db = AsyncMock(return_value=self.stored_agent) + + resp = self._request( + "POST", + "/v1/agents", + json={**_sample_agent_config(), "litellm_params": _agentcore_litellm_params()}, + ) + + assert resp.status_code == 200 + assert resp.json()["litellm_params"]["aws_secret_access_key"] == MASKED_SENTINEL + assert resp.json()["litellm_params"]["aws_region_name"] == "us-east-1" + assert SENTINEL_SECRET not in resp.text + stored = self.mock_registry.add_agent_to_db.await_args.kwargs["agent"]["litellm_params"] + assert stored["aws_secret_access_key"] == SENTINEL_SECRET + registered = self.mock_registry.register_agent.call_args.kwargs["agent_config"] + assert registered.litellm_params["aws_secret_access_key"] == SENTINEL_SECRET + + def test_get_by_id_masks_secret_for_admin_without_mutating_registry_object(self): + self.mock_registry.get_agent_by_id = MagicMock(return_value=self.stored_agent) + self.existing_row = MagicMock(spend=0.0) + + resp = self._request("GET", "/v1/agents/agent-123") + + assert resp.status_code == 200 + assert resp.json()["litellm_params"]["aws_secret_access_key"] == MASKED_SENTINEL + assert SENTINEL_SECRET not in resp.text + assert self.stored_agent.litellm_params["aws_secret_access_key"] == SENTINEL_SECRET + + def test_put_echoing_masked_secret_keeps_stored_value_and_masks_response(self): + self.mock_registry.update_agent_in_db = AsyncMock(return_value=self.stored_agent) + echoed = {**_agentcore_litellm_params(MASKED_SENTINEL), "aws_region_name": "us-west-2"} + + resp = self._request( + "PUT", + "/v1/agents/agent-123", + json={**_sample_agent_config(), "agent_name": "agentcore", "litellm_params": echoed}, + ) + + assert resp.status_code == 200 + written = self.mock_registry.update_agent_in_db.await_args.kwargs["agent"]["litellm_params"] + assert written["aws_secret_access_key"] == SENTINEL_SECRET + assert written["aws_region_name"] == "us-west-2" + assert SENTINEL_SECRET not in resp.text + + def test_patch_omitting_secret_keeps_stored_value(self): + self.mock_registry.patch_agent_in_db = AsyncMock(return_value=self.stored_agent) + without_secret = {k: v for k, v in _agentcore_litellm_params().items() if k != "aws_secret_access_key"} + + resp = self._request( + "PATCH", + "/v1/agents/agent-123", + json={"litellm_params": {**without_secret, "aws_region_name": "eu-west-1"}}, + ) + + assert resp.status_code == 200 + written = self.mock_registry.patch_agent_in_db.await_args.kwargs["agent"]["litellm_params"] + assert written["aws_secret_access_key"] == SENTINEL_SECRET + assert written["aws_region_name"] == "eu-west-1" + assert SENTINEL_SECRET not in resp.text + + def test_patch_with_new_secret_rotates_it(self): + self.mock_registry.patch_agent_in_db = AsyncMock(return_value=self.stored_agent) + + self._request( + "PATCH", + "/v1/agents/agent-123", + json={"litellm_params": _agentcore_litellm_params("ROTATED_SECRET_zyxwvu9876")}, + ) + + written = self.mock_registry.patch_agent_in_db.await_args.kwargs["agent"]["litellm_params"] + assert written["aws_secret_access_key"] == "ROTATED_SECRET_zyxwvu9876" + + def test_patch_without_litellm_params_does_not_touch_them(self): + self.mock_registry.patch_agent_in_db = AsyncMock(return_value=self.stored_agent) + + self._request("PATCH", "/v1/agents/agent-123", json={"agent_name": "renamed"}) + + written = self.mock_registry.patch_agent_in_db.await_args.kwargs["agent"] + assert "litellm_params" not in written + assert written["agent_name"] == "renamed" + + def test_get_by_id_masks_secret_nested_under_non_secret_key(self): + self.stored_agent.litellm_params = {"databricks_oauth": _databricks_oauth_params()} + self.mock_registry.get_agent_by_id = MagicMock(return_value=self.stored_agent) + self.existing_row = MagicMock(spend=0.0) + + resp = self._request("GET", "/v1/agents/agent-123") + + assert resp.status_code == 200 + assert resp.json()["litellm_params"]["databricks_oauth"]["client_secret"] == MASKED_SENTINEL + assert resp.json()["litellm_params"]["databricks_oauth"]["workspace_url"] == "https://dbc-abc123.example.com" + assert SENTINEL_SECRET not in resp.text + + def test_patch_editing_nested_block_around_masked_secret_keeps_stored_value(self): + self.existing_row["litellm_params"] = {"databricks_oauth": _databricks_oauth_params()} + self.mock_registry.patch_agent_in_db = AsyncMock(return_value=self.stored_agent) + edited = {**_databricks_oauth_params(MASKED_SENTINEL), "workspace_url": "https://dbc-new.example.com"} + + self._request("PATCH", "/v1/agents/agent-123", json={"litellm_params": {"databricks_oauth": edited}}) + + written = self.mock_registry.patch_agent_in_db.await_args.kwargs["agent"]["litellm_params"] + assert written["databricks_oauth"]["client_secret"] == SENTINEL_SECRET + assert written["databricks_oauth"]["workspace_url"] == "https://dbc-new.example.com" + + def test_get_by_id_masks_everything_past_the_nesting_cap(self): + deep = {"workspace_url": "https://dbc-abc123.example.com", "client_secret": SENTINEL_SECRET} + for _ in range(12): + deep = {"level": deep} + self.stored_agent.litellm_params = deep + self.mock_registry.get_agent_by_id = MagicMock(return_value=self.stored_agent) + self.existing_row = MagicMock(spend=0.0) + + resp = self._request("GET", "/v1/agents/agent-123") + + assert resp.status_code == 200 + assert SENTINEL_SECRET not in resp.text + assert "https://dbc-abc123.example.com" not in resp.text + + class TestAgentProtocolVersionValidation: """Registration accepts spec-default semver protocolVersion values and still rejects genuinely unsupported versions.""" diff --git a/ui/litellm-dashboard/src/app/(dashboard)/agents/_components/dynamic_agent_form_fields.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/agents/_components/dynamic_agent_form_fields.test.tsx new file mode 100644 index 00000000000..2dacee69543 --- /dev/null +++ b/ui/litellm-dashboard/src/app/(dashboard)/agents/_components/dynamic_agent_form_fields.test.tsx @@ -0,0 +1,47 @@ +import { describe, expect, it } from "vitest"; +import type { AgentCreateInfo } from "@/components/networking"; +import { buildDynamicAgentData } from "./dynamic_agent_form_fields"; + +const agentcoreInfo: AgentCreateInfo = { + agent_type: "bedrock_agentcore", + agent_type_display_name: "Bedrock AgentCore", + model_template: "bedrock/agentcore/{agent_runtime_arn}", + credential_fields: [ + { key: "agent_runtime_arn", label: "Agent Runtime ARN" }, + { key: "aws_region_name", label: "AWS Region", include_in_litellm_params: true }, + { key: "aws_access_key_id", label: "AWS Access Key ID", include_in_litellm_params: true }, + { key: "aws_secret_access_key", label: "AWS Secret Access Key", include_in_litellm_params: true }, + ], +}; + +describe("buildDynamicAgentData", () => { + it("omits masked credential values the proxy returned so an edit keeps the stored secret", () => { + const prefilledFromMaskedResponse = { + agent_name: "agentcore", + agent_runtime_arn: "arn:aws:bedrock-agentcore:us-east-1:123456789012:runtime/demo", + aws_region_name: "us-west-2", + aws_access_key_id: "AK****01", + aws_secret_access_key: "SE****34", + }; + const payload = buildDynamicAgentData(prefilledFromMaskedResponse, agentcoreInfo); + + expect(payload.litellm_params).toEqual({ + model: "bedrock/agentcore/arn:aws:bedrock-agentcore:us-east-1:123456789012:runtime/demo", + agent_runtime_arn: "arn:aws:bedrock-agentcore:us-east-1:123456789012:runtime/demo", + aws_region_name: "us-west-2", + }); + }); + + it("sends a newly typed secret so the credential can be rotated", () => { + const payload = buildDynamicAgentData( + { + agent_name: "agentcore", + agent_runtime_arn: "arn:aws:bedrock-agentcore:us-east-1:123456789012:runtime/demo", + aws_secret_access_key: "ROTATED_SECRET_zyxwvu9876", + }, + agentcoreInfo, + ); + + expect(payload.litellm_params.aws_secret_access_key).toBe("ROTATED_SECRET_zyxwvu9876"); + }); +}); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/agents/_components/dynamic_agent_form_fields.tsx b/ui/litellm-dashboard/src/app/(dashboard)/agents/_components/dynamic_agent_form_fields.tsx index 04a8b0df9d9..c187c9e6682 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/agents/_components/dynamic_agent_form_fields.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/agents/_components/dynamic_agent_form_fields.tsx @@ -5,6 +5,7 @@ import { Textarea } from "@/components/ui/textarea"; import { FieldGroup } from "@/components/ui/field"; import { AgentCreateInfo, AgentCredentialFieldMetadata } from "@/components/networking"; import { PasswordInput } from "@/components/shared/PasswordInput"; +import { stripMaskedSecrets } from "@/utils/maskedSecretUtils"; import { AGENT_FORM_CONFIG } from "./agent_config"; import CostConfigFields, { COST_FIELD_NAMES } from "./cost_config_fields"; import { @@ -177,7 +178,7 @@ export const buildDynamicAgentData = (values: AgentFormValues, agentTypeInfo: Ag }, ], }, - litellm_params: litellmParams, + litellm_params: stripMaskedSecrets(litellmParams), }; if (values.tpm_limit != null) agentData.tpm_limit = values.tpm_limit;