mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
fix(proxy): redact credential headers from request logging copies (#35678)
* fix(proxy): redact credential headers from request logging copies clean_headers preserves an Anthropic subscription OAuth token, and other client-supplied provider credentials, so they can be forwarded upstream. The same dict was also stored as proxy_server_request["headers"] and metadata["headers"], so those credentials reached every logging callback and the SpendLogs proxy_server_request column that the Admin UI logs page renders. Build the observability facing copies through redact_credential_headers, and drop the transport-only keys (provider_specific_header, headers, api_key) from the request body snapshot since they have to keep the real values. * fix(proxy): use the redacted header copy in the request debug log The stdout secret filter matches Bearer and sk- shaped values, so an MCP auth token printed by the request-header debug line survived it in cleartext. * fix(proxy): resolve the configured MCP auth header name through the secret manager get_secret_str also consults a configured secret manager, so a deployment that stores the header name there now gets that header masked too. Drops the added comments in favour of a named constant. * perf(proxy): resolve the MCP auth header name once per process get_secret_str issues a blocking secret-manager SDK call when one is configured, and configured_credential_header_names runs on every proxied request. * fix(proxy): read the MCP auth header name live, cache only the secret manager The config reloader rewrites os.environ on an interval and after /config/update, and MCPRequestHandler resolves the same setting per request, so caching the env lookup left a renamed header logged in the clear until the process restarted. Only the blocking secret-manager call stays cached. * refactor(proxy): narrow header redaction to the reported credential set Drops the MCP header-name resolution, its per-request config and secret-manager lookups, and the x-mcp- prefix rule. Those cover a separate credential family than the one this ticket reports and carried their own config-reload staleness surface; they belong in their own change. --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
db1b4d54bd
commit
c98d595359
2 changed files with 185 additions and 6 deletions
|
|
@ -4,6 +4,7 @@ import json
|
|||
import re
|
||||
import time
|
||||
from collections import OrderedDict
|
||||
from collections.abc import Mapping
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
from fastapi import HTTPException, Request
|
||||
|
|
@ -48,6 +49,12 @@ from litellm.proxy.common_utils.http_parsing_utils import _safe_get_request_head
|
|||
# Cache special headers as a frozenset for O(1) lookup performance
|
||||
_SPECIAL_HEADERS_CACHE = frozenset(v.value.lower() for v in SpecialHeaders._member_map_.values())
|
||||
|
||||
_REDACTED_HEADER_VALUE = "***REDACTED***"
|
||||
_CREDENTIAL_HEADER_NAMES = SpecialHeaders.litellm_credential_header_names() | frozenset(
|
||||
{"cookie", "proxy-authorization"}
|
||||
)
|
||||
_TRANSPORT_ONLY_CREDENTIAL_KEYS = frozenset({"provider_specific_header", "headers", "api_key"})
|
||||
|
||||
# Matches any header of the form x-<something>-session-id (case-insensitive).
|
||||
# Excludes the two explicit litellm headers which are handled with higher priority.
|
||||
_GENERIC_SESSION_ID_HEADER_RE = re.compile(r"^x-.+-session-id$", re.IGNORECASE)
|
||||
|
|
@ -747,6 +754,30 @@ def clean_headers(
|
|||
return clean_headers
|
||||
|
||||
|
||||
def _is_credential_header(header: str) -> bool:
|
||||
"""Whether `header` carries a caller credential rather than request context."""
|
||||
return header.lower() in _CREDENTIAL_HEADER_NAMES
|
||||
|
||||
|
||||
def redact_credential_headers(headers: Mapping[str, str]) -> Mapping[str, str]:
|
||||
"""Return a copy of `headers` with credential-bearing values masked.
|
||||
|
||||
`clean_headers` deliberately preserves some credential headers so they can be
|
||||
forwarded to the upstream provider; an Anthropic subscription OAuth token in
|
||||
`Authorization`, or a client-supplied provider key in `x-api-key`. Those values
|
||||
must never reach a logging callback or a spend log, so every observability-facing
|
||||
copy of the header dict is built through this helper while the copy that is
|
||||
forwarded upstream keeps the real values.
|
||||
|
||||
The returned object is a plain dict; guardrail hooks stamp their own headers onto
|
||||
the stored copy and the logging callbacks JSON-serialize it.
|
||||
"""
|
||||
return {
|
||||
header: (_REDACTED_HEADER_VALUE if _is_credential_header(header) else value)
|
||||
for header, value in headers.items()
|
||||
}
|
||||
|
||||
|
||||
class LiteLLMProxyRequestSetup:
|
||||
@staticmethod
|
||||
def _get_timeout_from_request(headers: dict) -> float | None:
|
||||
|
|
@ -1443,7 +1474,8 @@ async def add_litellm_data_to_request(
|
|||
_headers,
|
||||
allow_client_message_redaction_opt_out=_allow_client_message_redaction_opt_out,
|
||||
)
|
||||
verbose_proxy_logger.debug(f"Request Headers: {_headers}")
|
||||
_logging_safe_headers = redact_credential_headers(_headers)
|
||||
verbose_proxy_logger.debug(f"Request Headers: {_logging_safe_headers}")
|
||||
verbose_proxy_logger.debug(f"Raw Headers: {_raw_headers}")
|
||||
|
||||
if forward_llm_auth and "x-api-key" in _headers:
|
||||
|
|
@ -1464,7 +1496,7 @@ async def add_litellm_data_to_request(
|
|||
data["proxy_server_request"] = {
|
||||
"url": str(request.url),
|
||||
"method": request.method,
|
||||
"headers": _headers,
|
||||
"headers": _logging_safe_headers,
|
||||
"body": None, # filled in post-strip; see below
|
||||
"arrival_time": arrival_time, # Track when request arrived at proxy
|
||||
}
|
||||
|
|
@ -1490,7 +1522,7 @@ async def add_litellm_data_to_request(
|
|||
|
||||
# Expose request headers under the metadata field for guardrails (fixes #17477)
|
||||
if _metadata_variable_name in data and isinstance(data[_metadata_variable_name], dict):
|
||||
data[_metadata_variable_name]["headers"] = _headers
|
||||
data[_metadata_variable_name]["headers"] = _logging_safe_headers
|
||||
|
||||
# check for forwardable headers
|
||||
data = LiteLLMProxyRequestSetup.add_headers_to_llm_call_by_model_group(
|
||||
|
|
@ -1619,7 +1651,7 @@ async def add_litellm_data_to_request(
|
|||
# self-reference — body.proxy_server_request.body would be the same
|
||||
# dict as body, producing an infinite traversal loop for any consumer
|
||||
# that walks the structure.
|
||||
_body_snapshot_exclude = {"secret_fields", "proxy_server_request"}
|
||||
_body_snapshot_exclude = frozenset({"secret_fields", "proxy_server_request"}) | _TRANSPORT_ONLY_CREDENTIAL_KEYS
|
||||
_body_snapshot = {k: v for k, v in data.items() if k not in _body_snapshot_exclude}
|
||||
data["proxy_server_request"]["body"] = _body_snapshot
|
||||
|
||||
|
|
@ -1726,7 +1758,7 @@ async def add_litellm_data_to_request(
|
|||
data[_metadata_variable_name]["user_api_key_team_object_permission_id"] = getattr(
|
||||
user_api_key_dict, "team_object_permission_id", None
|
||||
)
|
||||
data[_metadata_variable_name]["headers"] = _headers
|
||||
data[_metadata_variable_name]["headers"] = _logging_safe_headers
|
||||
data[_metadata_variable_name]["endpoint"] = str(request.url)
|
||||
# Carry the proxy-receive instant via metadata (like `endpoint`) so the
|
||||
# OTel layer can compute pre-request latency, including on the failure
|
||||
|
|
|
|||
|
|
@ -5559,6 +5559,153 @@ def test_warn_stale_team_alias_once_evicts_oldest_key_beyond_cap(monkeypatch):
|
|||
assert list(pre_call_utils._STALE_TEAM_ALIAS_WARNING_KEYS) == ["key-2", "key-3"]
|
||||
|
||||
|
||||
_OAUTH_TOKEN = "Bearer sk-ant-oat01-regression-token-lit5108"
|
||||
|
||||
|
||||
def _all_header_dicts(data: dict, metadata_variable_name: str) -> list[dict]:
|
||||
metadata = data.get(metadata_variable_name) or {}
|
||||
proxy_server_request = data["proxy_server_request"]
|
||||
body = proxy_server_request["body"]
|
||||
return [
|
||||
metadata.get("headers") or {},
|
||||
(metadata.get("requester_metadata") or {}).get("headers") or {},
|
||||
proxy_server_request["headers"],
|
||||
(body.get(metadata_variable_name) or {}).get("headers") or {},
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"path, metadata_variable_name",
|
||||
[
|
||||
("/v1/messages", "litellm_metadata"),
|
||||
("/v1/chat/completions", "metadata"),
|
||||
],
|
||||
)
|
||||
async def test_add_litellm_data_to_request_redacts_oauth_header_from_logging_copies(path, metadata_variable_name):
|
||||
"""The Anthropic subscription token is forwarded upstream but never handed to logging."""
|
||||
request_mock = _make_request_mock(
|
||||
path,
|
||||
{
|
||||
"Content-Type": "application/json",
|
||||
"anthropic-version": "2023-06-01",
|
||||
"Authorization": _OAUTH_TOKEN,
|
||||
"x-litellm-api-key": "Bearer sk-virtual-key",
|
||||
},
|
||||
)
|
||||
|
||||
updated = await add_litellm_data_to_request(
|
||||
data={"model": "anthropic-claude", "messages": [{"role": "user", "content": "hello"}]},
|
||||
request=request_mock,
|
||||
user_api_key_dict=UserAPIKeyAuth(api_key="hashed-key"),
|
||||
proxy_config=MagicMock(),
|
||||
general_settings={"forward_client_headers_to_llm_api": True},
|
||||
version="test-version",
|
||||
)
|
||||
|
||||
for header_dict in _all_header_dicts(updated, metadata_variable_name):
|
||||
assert header_dict.get("Authorization") != _OAUTH_TOKEN
|
||||
assert "sk-ant-oat01" not in json.dumps(header_dict)
|
||||
|
||||
assert "sk-ant-oat01" not in json.dumps(updated["proxy_server_request"], default=repr)
|
||||
|
||||
assert updated["proxy_server_request"]["headers"] is updated[metadata_variable_name]["headers"]
|
||||
|
||||
assert updated["provider_specific_header"]["extra_headers"]["Authorization"] == _OAUTH_TOKEN
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_litellm_data_to_request_keeps_every_forwarded_credential_out_of_logging_copies():
|
||||
"""Credentials kept for transport must not survive anywhere under proxy_server_request."""
|
||||
secrets = {
|
||||
"x-api-key": "sk-byok-provider-key-lit5108",
|
||||
"cookie": "litellm_jwt=session-token-lit5108",
|
||||
"proxy-authorization": "Bearer proxy-token-lit5108",
|
||||
}
|
||||
request_mock = _make_request_mock(
|
||||
"/v1/chat/completions",
|
||||
{
|
||||
"Content-Type": "application/json",
|
||||
"x-litellm-api-key": "Bearer sk-virtual-key",
|
||||
**secrets,
|
||||
},
|
||||
)
|
||||
|
||||
updated = await add_litellm_data_to_request(
|
||||
data={"model": "gpt-4o", "messages": [{"role": "user", "content": "hello"}]},
|
||||
request=request_mock,
|
||||
user_api_key_dict=UserAPIKeyAuth(api_key="hashed-key"),
|
||||
proxy_config=MagicMock(),
|
||||
general_settings={
|
||||
"forward_llm_provider_auth_headers": True,
|
||||
"forward_client_headers_to_llm_api": True,
|
||||
},
|
||||
version="test-version",
|
||||
)
|
||||
|
||||
assert updated["api_key"] == secrets["x-api-key"]
|
||||
assert updated["headers"]["x-api-key"] == secrets["x-api-key"]
|
||||
|
||||
logged = json.dumps(updated["proxy_server_request"], default=repr)
|
||||
for value in secrets.values():
|
||||
assert value not in logged
|
||||
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"header, expected_redacted",
|
||||
[
|
||||
("Authorization", True),
|
||||
("X-Api-Key", True),
|
||||
("x-goog-api-key", True),
|
||||
("Ocp-Apim-Subscription-Key", True),
|
||||
("API-Key", True),
|
||||
("Cookie", True),
|
||||
("Proxy-Authorization", True),
|
||||
("anthropic-version", False),
|
||||
("user-agent", False),
|
||||
],
|
||||
)
|
||||
def test_redact_credential_headers_classifies_each_header(header, expected_redacted):
|
||||
from litellm.proxy.litellm_pre_call_utils import redact_credential_headers
|
||||
|
||||
headers = {header: "secret-value"}
|
||||
|
||||
redacted = redact_credential_headers(headers)
|
||||
|
||||
assert redacted[header] == ("***REDACTED***" if expected_redacted else "secret-value")
|
||||
assert headers[header] == "secret-value"
|
||||
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_litellm_data_to_request_debug_log_does_not_print_credentials():
|
||||
"""The request-header debug line carries values the stdout secret filter does not match."""
|
||||
import litellm.proxy.litellm_pre_call_utils as pre_call_utils
|
||||
|
||||
request_mock = _make_request_mock(
|
||||
"/v1/chat/completions",
|
||||
{
|
||||
"Content-Type": "application/json",
|
||||
"Ocp-Apim-Subscription-Key": "apim-plaintext-token-lit5108",
|
||||
"x-litellm-api-key": "Bearer sk-virtual-key",
|
||||
},
|
||||
)
|
||||
|
||||
with patch.object(pre_call_utils.verbose_proxy_logger, "debug") as mock_debug:
|
||||
await add_litellm_data_to_request(
|
||||
data={"model": "gpt-4o", "messages": [{"role": "user", "content": "hello"}]},
|
||||
request=request_mock,
|
||||
user_api_key_dict=UserAPIKeyAuth(api_key="hashed-key"),
|
||||
proxy_config=MagicMock(),
|
||||
general_settings={"forward_llm_provider_auth_headers": True},
|
||||
version="test-version",
|
||||
)
|
||||
|
||||
logged = " ".join(str(call) for call in mock_debug.call_args_list)
|
||||
assert "apim-plaintext-token-lit5108" not in logged
|
||||
|
||||
|
||||
def _callback_credential_request_mock() -> MagicMock:
|
||||
request_mock = MagicMock(spec=Request)
|
||||
request_mock.url = MagicMock()
|
||||
|
|
@ -5722,4 +5869,4 @@ async def test_key_level_callback_vars_survive_the_strip():
|
|||
)
|
||||
|
||||
assert updated[TRUSTED_CALLBACK_VARS_FIELD] == {"dd_api_key": "key-dd-key", "dd_site": "us5.datadoghq.com"}
|
||||
assert updated["dd_site"] == "us5.datadoghq.com"
|
||||
assert updated["dd_site"] == "us5.datadoghq.com"
|
||||
Loading…
Add table
Reference in a new issue