fix(security): prevent secret_fields from leaking into spend logs - #27143
Conversation
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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>
b773a17 to
5923c32
Compare
Greptile SummaryThis PR fixes a security bug where Confidence Score: 5/5Safe to merge — targeted, correct fix with solid defense-in-depth and full test coverage. No P0 or P1 issues found. Both fix layers (snapshot exclusion and sanitizer filter) are correctly implemented. The sanitizer applies the key filter recursively, so nested occurrences are also caught. Tests cover the sanitizer directly, the serialization pipeline end-to-end, and the No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/litellm_pre_call_utils.py | Primary fix: body snapshot now uses a dict comprehension that excludes secret_fields, preventing Authorization tokens from entering the audit/spend-log path. |
| litellm/proxy/spend_tracking/spend_tracking_utils.py | Secondary safeguard: adds _SENSITIVE_REQUEST_BODY_KEYS frozenset and filters it out in _sanitize_request_body_for_spend_logs_payload; the filter is applied recursively for nested dicts. |
| tests/test_litellm/proxy/spend_tracking/test_spend_tracking_utils.py | Two new unit tests: one verifying the sanitizer strips secret_fields, and one end-to-end test verifying the full serialization pipeline excludes it; both use mocks only (no network calls). |
| tests/test_litellm/proxy/test_litellm_pre_call_utils.py | New async test verifies add_litellm_data_to_request excludes secret_fields from the body snapshot while keeping it on the live data dict for downstream consumers. |
Reviews (1): Last reviewed commit: "fix(security): prevent secret_fields fro..." | Re-trigger Greptile
…s-in-spend-logs-a532 fix(security): prevent secret_fields from leaking into spend logs
Relevant issues
Reported in Slack: secret_fields (containing raw HTTP headers including Authorization Bearer tokens) were being logged in LiteLLM spend logs, exposing user credentials.
Pre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unitType
🐛 Bug Fix (Security)
Changes
Root Cause
data["secret_fields"]contains raw HTTP headers (includingAuthorization: Bearer ...tokens) via aSecretFields(raw_headers=...)dict. When the body snapshot is created at:...the entire
datadict — includingsecret_fields— is copied into the body snapshot. This body is later serialized to JSON and persisted in theLiteLLM_SpendLogs.proxy_server_requestdatabase column via_get_proxy_server_request_for_spend_logs_payload, which calls_sanitize_request_body_for_spend_logs_payloadbut that sanitizer only truncated long strings — it did not strip sensitive keys.Fix (defense in depth)
Primary fix (
litellm/proxy/litellm_pre_call_utils.py): Excludesecret_fieldswhen creating the body snapshot. The dict comprehension{k: v for k, v in data.items() if k != "secret_fields"}preventssecret_fieldsfrom entering the snapshot at all.secret_fieldsremains available on the livedatadict for legitimate downstream consumers (MCP, Responses API).Secondary safeguard (
litellm/proxy/spend_tracking/spend_tracking_utils.py): Added_SENSITIVE_REQUEST_BODY_KEYSfrozenset to_sanitize_request_body_for_spend_logs_payloadwhich stripssecret_fieldsfrom any request body before it's serialized to JSON for spend log storage. This catches any other code paths that might includesecret_fieldsin data that flows to spend logs.Tests added
test_add_litellm_data_to_request_body_snapshot_excludes_secret_fields— verifies the body snapshot inproxy_server_requestdoes not containsecret_fieldswhile the livedatadict still has ittest_sanitize_request_body_strips_secret_fields— verifies the sanitizer stripssecret_fieldsfrom request bodiestest_proxy_server_request_payload_excludes_secret_fields— end-to-end test that the spend log serialization pipeline stripssecret_fieldsScreenshots / Proof of Fix
All 118 pre_call_utils tests pass and all 46 spend_tracking_utils tests pass (including 3 new tests).
Slack Thread