mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-06 02:23:35 +00:00
fix(proxy): close three variant metadata/tag injection paths
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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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"'})
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user