mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
fix(security): prevent secret_fields from leaking into spend logs
secret_fields (containing raw HTTP headers including Authorization Bearer tokens) was being included in proxy_server_request['body'] because the body snapshot was a copy.copy(data) of the full request dict. This body gets serialized and persisted in the LiteLLM_SpendLogs table, exposing user credentials in the database. Root cause: data['secret_fields'] was set before the body snapshot at data['proxy_server_request']['body'] = copy.copy(data), so the full raw headers (including auth tokens) ended up in the snapshot. Fix (defense in depth): 1. Exclude 'secret_fields' when creating the body snapshot in litellm_pre_call_utils.py (primary fix) 2. Strip 'secret_fields' in _sanitize_request_body_for_spend_logs_payload as a secondary safeguard secret_fields remains available on the live data dict for legitimate downstream consumers (MCP, Responses API). Co-authored-by: Krrish Dholakia <krrish-berri-2@users.noreply.github.com>
This commit is contained in:
parent
555a8131fe
commit
5923c3209b
4 changed files with 146 additions and 2 deletions
|
|
@ -1469,7 +1469,12 @@ async def add_litellm_data_to_request( # noqa: PLR0915
|
|||
# been parsed and stripped. Consumers (standard_logging_payload, lago,
|
||||
# spend_tracking_utils, streaming_iterator) read `body` to audit the
|
||||
# request; taking the snapshot here ensures they see cleaned metadata.
|
||||
data["proxy_server_request"]["body"] = copy.copy(data)
|
||||
#
|
||||
# Exclude secret_fields (which contains raw_headers with Authorization
|
||||
# tokens) from the snapshot — they must never be persisted in spend logs
|
||||
# or any other audit trail.
|
||||
_body_snapshot = {k: v for k, v in data.items() if k != "secret_fields"}
|
||||
data["proxy_server_request"]["body"] = _body_snapshot
|
||||
|
||||
# Snapshot the (now-cleaned) requester-supplied metadata for downstream
|
||||
# consumers. Taking the deepcopy AFTER the strip prevents attacker-
|
||||
|
|
|
|||
|
|
@ -610,6 +610,9 @@ def _get_messages_for_spend_logs_payload(
|
|||
return "{}"
|
||||
|
||||
|
||||
_SENSITIVE_REQUEST_BODY_KEYS = frozenset({"secret_fields"})
|
||||
|
||||
|
||||
def _sanitize_request_body_for_spend_logs_payload(
|
||||
request_body: dict,
|
||||
visited: Optional[set] = None,
|
||||
|
|
@ -618,6 +621,9 @@ def _sanitize_request_body_for_spend_logs_payload(
|
|||
"""
|
||||
Recursively sanitize request body to prevent logging large base64 strings or other large values.
|
||||
Truncates strings longer than MAX_STRING_LENGTH_PROMPT_IN_DB characters and handles nested dictionaries.
|
||||
|
||||
Also strips keys listed in _SENSITIVE_REQUEST_BODY_KEYS (e.g. secret_fields
|
||||
which contains raw HTTP headers including Authorization tokens).
|
||||
"""
|
||||
from litellm.constants import (
|
||||
LITELLM_TRUNCATED_PAYLOAD_FIELD,
|
||||
|
|
@ -676,7 +682,11 @@ def _sanitize_request_body_for_spend_logs_payload(
|
|||
return value
|
||||
return value
|
||||
|
||||
return {k: _sanitize_value(v) for k, v in request_body.items()}
|
||||
return {
|
||||
k: _sanitize_value(v)
|
||||
for k, v in request_body.items()
|
||||
if k not in _SENSITIVE_REQUEST_BODY_KEYS
|
||||
}
|
||||
|
||||
|
||||
def _convert_to_json_serializable_dict(
|
||||
|
|
|
|||
|
|
@ -1523,3 +1523,67 @@ class TestIsMasterKey:
|
|||
master = "sk-master-key-123"
|
||||
hashed = hash_token(master)
|
||||
assert _is_master_key(api_key=hashed, _master_key=master) is False
|
||||
|
||||
|
||||
def test_sanitize_request_body_strips_secret_fields():
|
||||
"""
|
||||
secret_fields contains raw HTTP headers (including Authorization Bearer
|
||||
tokens). _sanitize_request_body_for_spend_logs_payload must strip it so
|
||||
that sensitive credentials are never persisted in the spend-logs DB.
|
||||
"""
|
||||
request_body = {
|
||||
"model": "gpt-4",
|
||||
"messages": [{"role": "user", "content": "hi"}],
|
||||
"secret_fields": {
|
||||
"raw_headers": {
|
||||
"authorization": "Bearer sk-GjX--WwRQmiX2cvASbKf5Q",
|
||||
"content-type": "application/json",
|
||||
"host": "litellm.example.com",
|
||||
}
|
||||
},
|
||||
}
|
||||
sanitized = _sanitize_request_body_for_spend_logs_payload(request_body)
|
||||
|
||||
assert (
|
||||
"secret_fields" not in sanitized
|
||||
), "secret_fields must be stripped from the sanitized request body"
|
||||
assert sanitized["model"] == "gpt-4"
|
||||
assert sanitized["messages"] == [{"role": "user", "content": "hi"}]
|
||||
|
||||
|
||||
@patch(
|
||||
"litellm.proxy.spend_tracking.spend_tracking_utils._should_store_prompts_and_responses_in_spend_logs"
|
||||
)
|
||||
def test_proxy_server_request_payload_excludes_secret_fields(mock_should_store):
|
||||
"""
|
||||
End-to-end test: when the proxy_server_request body contains
|
||||
secret_fields (as it did before the body-snapshot fix), the spend-log
|
||||
serialization must still strip them via the sanitizer fallback.
|
||||
"""
|
||||
mock_should_store.return_value = True
|
||||
|
||||
litellm_params = {
|
||||
"proxy_server_request": {
|
||||
"body": {
|
||||
"model": "gpt-4",
|
||||
"messages": [{"role": "user", "content": "hello"}],
|
||||
"secret_fields": {
|
||||
"raw_headers": {
|
||||
"authorization": "Bearer sk-super-secret-token",
|
||||
"host": "litellm.example.com",
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
result = _get_proxy_server_request_for_spend_logs_payload(
|
||||
metadata={}, litellm_params=litellm_params, kwargs={}
|
||||
)
|
||||
parsed = json.loads(result)
|
||||
|
||||
assert (
|
||||
"secret_fields" not in parsed
|
||||
), "secret_fields must never appear in the spend-log proxy_server_request column"
|
||||
assert parsed["model"] == "gpt-4"
|
||||
assert parsed["messages"] == [{"role": "user", "content": "hello"}]
|
||||
|
|
|
|||
|
|
@ -449,6 +449,71 @@ async def test_add_litellm_data_to_request_proxy_server_request_body_is_post_str
|
|||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_litellm_data_to_request_body_snapshot_excludes_secret_fields():
|
||||
"""Security: proxy_server_request['body'] must never contain secret_fields
|
||||
because that dict holds raw HTTP headers including Authorization Bearer
|
||||
tokens. The body snapshot is persisted in spend logs and other audit trails,
|
||||
so leaking secret_fields there exposes user credentials.
|
||||
|
||||
secret_fields must still be available on the live ``data`` dict for
|
||||
downstream consumers (MCP, Responses API) that legitimately need raw headers.
|
||||
"""
|
||||
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",
|
||||
"Authorization": "Bearer sk-super-secret-token",
|
||||
}
|
||||
request_mock.client = MagicMock()
|
||||
request_mock.client.host = "127.0.0.1"
|
||||
|
||||
data = {
|
||||
"model": "gpt-3.5-turbo",
|
||||
"messages": [{"role": "user", "content": "hello"}],
|
||||
}
|
||||
|
||||
user_api_key_dict = UserAPIKeyAuth(
|
||||
api_key="hashed-key",
|
||||
user_id="test-user",
|
||||
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",
|
||||
)
|
||||
|
||||
# secret_fields must exist on the live data dict
|
||||
assert (
|
||||
"secret_fields" in updated
|
||||
), "secret_fields must still be present on the live data dict"
|
||||
assert "raw_headers" in updated["secret_fields"]
|
||||
|
||||
# But the body snapshot must NOT contain secret_fields
|
||||
snapshot_body = updated["proxy_server_request"]["body"]
|
||||
assert "secret_fields" not in snapshot_body, (
|
||||
"secret_fields must be excluded from proxy_server_request['body'] "
|
||||
"to prevent Authorization tokens from leaking into spend logs"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_litellm_data_to_request_strips_string_encoded_admin_injection():
|
||||
"""Regression: metadata arriving as a JSON string (multipart/form-data or
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue