From 76aa97f77b0781b77e4ea248e7245b0bdeefa587 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Thu, 16 Apr 2026 23:35:26 +0000 Subject: [PATCH] fix(proxy): close three variant metadata/tag injection paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close three variant bypasses adjacent to VERIA-28 found during post-fix variant audit: 1. _guardrail_modification_check had the same isinstance(dict) bypass Veria-AI just flagged on the pre-call strip. A caller sending `{"metadata": "{…}"}` as a JSON-encoded string (multipart/form-data or extra_body) skipped the guard, got parsed to dict downstream, and reached guardrail logic with bypass flags intact. Coerce strings via safe_json_loads before evaluating. 2. The allow_client_tags strip only covered body metadata.tags and litellm_metadata.tags — caller-supplied tags arriving via the x-litellm-tags header or root-level data["tags"] bypassed it. Gate add_request_tag_to_metadata's result on the same flag. 3. requester_metadata was deepcopied BEFORE the strip, so attacker injections (user_api_key_metadata shadows, disallowed tags, _pipeline_managed_guardrails) persisted in the snapshot. The PANW guardrail (and any future consumer) trusting requester_metadata would see forged values. Move the deepcopy to after the strip. Regression tests added for each. --- litellm/proxy/auth/auth_checks.py | 22 ++- litellm/proxy/litellm_pre_call_utils.py | 30 +++- .../proxy/auth/test_auth_checks.py | 37 +++++ .../proxy/test_litellm_pre_call_utils.py | 131 ++++++++++++++++++ 4 files changed, 213 insertions(+), 7 deletions(-) diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index 1f4b5311ba..846d61384b 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -350,12 +350,30 @@ def _guardrail_modification_check( failing loudly at the auth layer so operators see an explicit 403 instead of a confusing silent-ignore. """ + from litellm.litellm_core_utils.safe_json_loads import safe_json_loads from litellm.proxy.guardrails.guardrail_helpers import can_modify_guardrails + def _coerce_to_dict(container: Any) -> Optional[dict]: + """Accept dict or JSON-string (from multipart/form-data or extra_body). + + Without this, an attacker can smuggle guardrail keys past the check by + sending ``{"metadata": "{\\"disable_global_guardrails\\": true}"}`` — + ``isinstance(dict)`` on the string returns False, the check returns + no-modification, and ``add_litellm_data_to_request`` parses the string + to a dict downstream. + """ + if isinstance(container, dict): + return container + if isinstance(container, str): + parsed = safe_json_loads(container) + return parsed if isinstance(parsed, dict) else None + return None + def _user_requested_modification(container: Any) -> bool: - if not isinstance(container, dict): + coerced = _coerce_to_dict(container) + if coerced is None: return False - return any(container.get(key) for key in _GUARDRAIL_MODIFICATION_KEYS) + return any(coerced.get(key) for key in _GUARDRAIL_MODIFICATION_KEYS) # Check both metadata keys — callers can populate either depending on the # endpoint. Cover the top-level too so root-level injection is rejected. diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 33affa5351..51f82b7c00 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -1069,9 +1069,10 @@ async def add_litellm_data_to_request( # noqa: PLR0915 verbose_proxy_logger.warning( f"Failed to parse 'metadata' as JSON dict. Received value: {data['metadata']}" ) - data[_metadata_variable_name]["requester_metadata"] = copy.deepcopy( - data["metadata"] - ) + # requester_metadata is snapshotted AFTER the strip below so + # downstream consumers (e.g. PANW guardrail reading user_ip / + # profile_id) don't see attacker-injected admin slots preserved in + # the deepcopy. # Parse litellm_metadata if it's a string (e.g., from multipart/form-data or extra_body) if "litellm_metadata" in data and data["litellm_metadata"] is not None: @@ -1131,6 +1132,16 @@ async def add_litellm_data_to_request( # noqa: PLR0915 ", ".join(_stripped_from), ) + # Snapshot the (now-cleaned) requester-supplied metadata for downstream + # consumers. Taking the deepcopy AFTER the strip prevents attacker- + # injected admin slots (user_api_key_metadata, tags without opt-in, + # _pipeline_managed_guardrails) from surviving in requester_metadata + # where guardrails and audit paths may read from it. + if "metadata" in data and isinstance(data["metadata"], dict): + data[_metadata_variable_name]["requester_metadata"] = copy.deepcopy( + data["metadata"] + ) + # Now merge litellm_metadata into the metadata variable (preserving existing # values) — runs AFTER the strip so attacker injections in litellm_metadata # cannot cross-contaminate the admin-authoritative metadata dict. @@ -1300,15 +1311,24 @@ async def add_litellm_data_to_request( # noqa: PLR0915 user_agent = request.headers["user-agent"] data[_metadata_variable_name]["user_agent"] = user_agent - # Check if using tag based routing + # Check if using tag based routing. The helper reads caller-controlled + # sources (x-litellm-tags header, data["tags"] root-level), so its result + # is still gated by the same allow_client_tags flag that gated the + # body-metadata tag strip above. Otherwise the strip is trivially + # bypassed by sending tags via header or at the root of the body. tags = LiteLLMProxyRequestSetup.add_request_tag_to_metadata( llm_router=llm_router, headers=_headers, data=data, ) - if tags is not None: + if tags is not None and _admin_allow_client_tags: data[_metadata_variable_name]["tags"] = tags + elif tags is not None: + verbose_proxy_logger.warning( + "Ignored caller-supplied tags from header/root body: this " + "key/team does not have `allow_client_tags: true` in its metadata." + ) # Team Callbacks controls callback_settings_obj = _get_dynamic_logging_metadata( diff --git a/tests/test_litellm/proxy/auth/test_auth_checks.py b/tests/test_litellm/proxy/auth/test_auth_checks.py index a3d24fc8bf..0391e22408 100644 --- a/tests/test_litellm/proxy/auth/test_auth_checks.py +++ b/tests/test_litellm/proxy/auth/test_auth_checks.py @@ -1897,3 +1897,40 @@ class TestGuardrailModificationCheck: ): # no-op, should not raise self._call({"metadata": {"disable_global_guardrails": True}}) + + def test_rejects_string_encoded_metadata_bypass(self): + """Regression: attacker sends metadata as JSON string to bypass the + isinstance(dict) guard. The check must coerce the string to dict + and evaluate guardrail modification keys inside it.""" + import json as _json + + from fastapi import HTTPException + + attacker_payload = {"disable_global_guardrails": True} + with patch( + "litellm.proxy.guardrails.guardrail_helpers.can_modify_guardrails", + return_value=False, + ): + with pytest.raises(HTTPException) as exc: + self._call({"metadata": _json.dumps(attacker_payload)}) + assert exc.value.status_code == 403 + + def test_rejects_string_encoded_litellm_metadata_bypass(self): + """Same bypass via the litellm_metadata key.""" + import json as _json + + from fastapi import HTTPException + + attacker_payload = {"guardrails": ["evaded"]} + with patch( + "litellm.proxy.guardrails.guardrail_helpers.can_modify_guardrails", + return_value=False, + ): + with pytest.raises(HTTPException) as exc: + self._call({"litellm_metadata": _json.dumps(attacker_payload)}) + assert exc.value.status_code == 403 + + def test_noop_when_string_is_not_json_object(self): + """Unparseable strings should not trigger a 403 — they have no keys.""" + self._call({"metadata": "not-json"}) + self._call({"metadata": '"just a string"'}) diff --git a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py index 351857b9b1..5f5ae0c64d 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -344,6 +344,137 @@ async def test_add_litellm_data_to_request_strips_string_encoded_admin_injection assert "_pipeline_managed_guardrails" not in other +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_ignores_x_litellm_tags_header_without_permission(): + """Regression: the `x-litellm-tags` header bypassed the body-metadata + tag strip. Header tags must also be gated by `allow_client_tags`.""" + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + request_mock = MagicMock(spec=Request) + request_mock.url.path = "/v1/chat/completions" + request_mock.url = MagicMock() + request_mock.url.__str__.return_value = "http://localhost/v1/chat/completions" + request_mock.method = "POST" + request_mock.query_params = {} + request_mock.headers = { + "Content-Type": "application/json", + "x-litellm-tags": "restricted-tier,victim-team", + } + request_mock.client = MagicMock() + request_mock.client.host = "127.0.0.1" + + data = {"model": "gpt-3.5-turbo"} + + user_api_key_dict = UserAPIKeyAuth( + api_key="hashed-key", + metadata={}, + team_metadata={}, + spend=0.0, + max_budget=100.0, + model_max_budget={}, + team_spend=0.0, + team_max_budget=200.0, + ) + + updated = await add_litellm_data_to_request( + data=data, + request=request_mock, + user_api_key_dict=user_api_key_dict, + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + assert "tags" not in (updated.get("metadata") or {}) + + +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_ignores_root_level_tags_without_permission(): + """Regression: root-level `data["tags"]` bypassed the body-metadata + tag strip. Root-level tags must also be gated by `allow_client_tags`.""" + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + request_mock = MagicMock(spec=Request) + request_mock.url.path = "/v1/chat/completions" + request_mock.url = MagicMock() + request_mock.url.__str__.return_value = "http://localhost/v1/chat/completions" + request_mock.method = "POST" + request_mock.query_params = {} + request_mock.headers = {"Content-Type": "application/json"} + request_mock.client = MagicMock() + request_mock.client.host = "127.0.0.1" + + data = { + "model": "gpt-3.5-turbo", + "tags": ["restricted-tier", "victim-team"], + } + + user_api_key_dict = UserAPIKeyAuth( + api_key="hashed-key", + metadata={}, + team_metadata={}, + spend=0.0, + max_budget=100.0, + model_max_budget={}, + team_spend=0.0, + team_max_budget=200.0, + ) + + updated = await add_litellm_data_to_request( + data=data, + request=request_mock, + user_api_key_dict=user_api_key_dict, + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + assert "tags" not in (updated.get("metadata") or {}) + + +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_honors_header_tags_when_opted_in(): + """When allow_client_tags=True, header-supplied tags flow through.""" + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + request_mock = MagicMock(spec=Request) + request_mock.url.path = "/v1/chat/completions" + request_mock.url = MagicMock() + request_mock.url.__str__.return_value = "http://localhost/v1/chat/completions" + request_mock.method = "POST" + request_mock.query_params = {} + request_mock.headers = { + "Content-Type": "application/json", + "x-litellm-tags": "production,ab-test", + } + request_mock.client = MagicMock() + request_mock.client.host = "127.0.0.1" + + data = {"model": "gpt-3.5-turbo"} + + user_api_key_dict = UserAPIKeyAuth( + api_key="hashed-key", + metadata={"allow_client_tags": True}, + team_metadata={}, + spend=0.0, + max_budget=100.0, + model_max_budget={}, + team_spend=0.0, + team_max_budget=200.0, + ) + + updated = await add_litellm_data_to_request( + data=data, + request=request_mock, + user_api_key_dict=user_api_key_dict, + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + assert updated["metadata"].get("tags") == ["production", "ab-test"] + + @pytest.mark.asyncio async def test_add_litellm_data_to_request_strips_user_tags_without_permission(): """Caller-supplied metadata.tags must be stripped when the key/team