diff --git a/litellm/proxy/guardrails/guardrail_endpoints.py b/litellm/proxy/guardrails/guardrail_endpoints.py index 2b3d10350d1..f41561a656f 100644 --- a/litellm/proxy/guardrails/guardrail_endpoints.py +++ b/litellm/proxy/guardrails/guardrail_endpoints.py @@ -2347,7 +2347,9 @@ def _metadata_fields(value: object) -> Mapping[str, object]: return {} -_CALLER_OWNED_KEY_FIELDS: Final[frozenset[str]] = frozenset({"user_api_key_end_user_id", "user_api_key_request_route"}) +_CALLER_OWNED_KEY_FIELDS: Final[frozenset[str]] = frozenset({"user_api_key_end_user_id"}) + +_WITHHELD_FIELDS: Final[frozenset[str]] = frozenset({"user_api_key_request_route"}) _RESERVED_IDENTITY_FIELDS: Final[frozenset[str]] = frozenset( {"requester_ip_address", "user_agent", "agent_id", "actor_agent_id"} @@ -2357,10 +2359,8 @@ _RESERVED_IDENTITY_FIELDS: Final[frozenset[str]] = frozenset( def _is_key_identity_field(key: str) -> bool: """Whether a metadata field attributes the authenticated caller rather than the request. - Pre-call processing owns the whole ``user_api_key_`` namespace, except the two fields that - describe the caller's own request: the end user it is made for, and the route it came from. - A guardrail needs both to decide, and on ``/apply_guardrail`` the proxy's own values say - nothing about the call the caller is asking about. + Pre-call processing owns the whole ``user_api_key_`` namespace, except the end user the call + is made for, which the caller owns here as it does on every LLM route. Attribution also reaches outside that namespace, so the fields pre-call processing fills in from the key and the connection are reserved by name as well. @@ -2377,12 +2377,19 @@ def _guardrail_request_metadata( The result is handed to the guardrail as ``request_data["metadata"]``, which providers write their own detection details into, so it cannot be a read-only mapping. + + The request route is withheld from both sides. The tool policy guardrail reads the request + itself when a route is present, instead of the tools replayed to it, so a route here turns + its checks off: the caller must not be able to set one, and this endpoint's own route + describes the evaluation rather than the call being evaluated. """ caller_fields: Final = tuple( (key, value) for key, value in _metadata_fields(caller).items() if not _is_key_identity_field(key) ) key_identity: Final = tuple( - (key, value) for key, value in _metadata_fields(proxy).items() if _is_key_identity_field(key) + (key, value) + for key, value in _metadata_fields(proxy).items() + if _is_key_identity_field(key) and key not in _WITHHELD_FIELDS ) return dict((*caller_fields, *key_identity)) diff --git a/tests/unit/proxy/guardrails/test_guardrail_endpoints.py b/tests/unit/proxy/guardrails/test_guardrail_endpoints.py index 5b40b26416f..8dde8c1b034 100644 --- a/tests/unit/proxy/guardrails/test_guardrail_endpoints.py +++ b/tests/unit/proxy/guardrails/test_guardrail_endpoints.py @@ -2504,16 +2504,19 @@ class TestGuardrailRequestMetadata: metadata: Final = _guardrail_request_metadata({"user_api_key": "spoof"}, {"user_api_key": "hashed"}) assert metadata == {"user_api_key": "hashed"} - def test_the_caller_owns_the_end_user_and_the_route(self) -> None: - """Both describe the call the caller is asking about, not the key it authenticated with.""" + def test_the_caller_owns_the_end_user(self) -> None: + """The end user describes the call the caller is asking about, as it does on every LLM route.""" + metadata: Final = _guardrail_request_metadata({"user_api_key_end_user_id": "customer-42"}, {}) + assert metadata == {"user_api_key_end_user_id": "customer-42"} + + def test_no_request_route_reaches_the_guardrail(self) -> None: + """A route makes the tool policy guardrail read the request instead of the replayed tools, + so neither the caller's nor the endpoint's own route may reach it.""" metadata: Final = _guardrail_request_metadata( - {"user_api_key_end_user_id": "customer-42", "user_api_key_request_route": "/v1/chat/completions"}, + {"user_api_key_request_route": "/v1/chat/completions", "topic": "tax"}, {"user_api_key_request_route": "/apply_guardrail"}, ) - assert metadata == { - "user_api_key_end_user_id": "customer-42", - "user_api_key_request_route": "/v1/chat/completions", - } + assert metadata == {"topic": "tax"} def test_identity_fields_outside_the_key_namespace_are_reserved_too(self) -> None: """The proxy sets who is calling and from where. A caller cannot claim another's."""