mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
fix(proxy): sanitize per-key callback config out of logged metadata
get_sanitized_user_information_from_key copied UserAPIKeyAuth.metadata verbatim into user_api_key_auth_metadata, so the key's callback configuration - including the integration credentials inside callback_vars - reached the StandardLoggingPayload every integration receives. The two other sites that stamp key/team metadata into request metadata did the same. Sanitize at those sources with strip_callback_config, which drops the `logging` and `callback_settings` slots and leaves everything else (notably `priority`, read back by the dynamic rate limiter) untouched. Those slots are resolved from UserAPIKeyAuth during pre-call setup and never read off the logged copies, so nothing downstream loses input. This makes the scrub in scrub_sensitive_keys_in_metadata dead - it only matched the string "logging" under one of the two field names and never covered callback_settings - so it is removed. Separately, LangSmith set the run's `inputs` to the raw StandardLoggingPayload while redacting only `extra`, so redact_user_api_key_info left every user_api_key_* field in inputs.metadata. Both now go through one _redact_metadata helper, which also covers the nested requester_metadata copy.
This commit is contained in:
parent
b9b27c2beb
commit
5e34e0460b
8 changed files with 191 additions and 24 deletions
|
|
@ -133,6 +133,15 @@ class LangsmithLogger(CustomBatchLogger):
|
|||
"dotted_order": metadata.get("dotted_order", None),
|
||||
}
|
||||
|
||||
def _redact_metadata(self, metadata: dict) -> dict:
|
||||
# helper is shallow; also scrub nested requester_metadata since
|
||||
# LangSmith forwards the whole dict into the run
|
||||
redacted = redact_user_api_key_info(metadata=dict(metadata))
|
||||
nested = redacted.get("requester_metadata")
|
||||
if isinstance(nested, dict):
|
||||
redacted["requester_metadata"] = redact_user_api_key_info(metadata=nested)
|
||||
return redacted
|
||||
|
||||
def _build_extra_metadata(self, metadata: Dict):
|
||||
extra_metadata = dict(metadata)
|
||||
requester_metadata = extra_metadata.get("requester_metadata")
|
||||
|
|
@ -141,13 +150,7 @@ class LangsmithLogger(CustomBatchLogger):
|
|||
if key in requester_metadata and key not in extra_metadata:
|
||||
extra_metadata[key] = requester_metadata[key]
|
||||
|
||||
# helper is shallow; also scrub nested requester_metadata since
|
||||
# LangSmith forwards the whole dict into `extra`
|
||||
extra_metadata = redact_user_api_key_info(metadata=extra_metadata)
|
||||
nested = extra_metadata.get("requester_metadata")
|
||||
if isinstance(nested, dict):
|
||||
extra_metadata["requester_metadata"] = redact_user_api_key_info(metadata=nested)
|
||||
return extra_metadata
|
||||
return self._redact_metadata(extra_metadata)
|
||||
|
||||
def _build_outputs_with_usage(self, payload: StandardLoggingPayload) -> Dict[str, Any]:
|
||||
response = payload["response"]
|
||||
|
|
@ -200,12 +203,13 @@ class LangsmithLogger(CustomBatchLogger):
|
|||
|
||||
metadata = payload["metadata"]
|
||||
extra_metadata = self._build_extra_metadata(dict(metadata))
|
||||
inputs = {**payload, "metadata": self._redact_metadata(dict(metadata))}
|
||||
outputs = self._build_outputs_with_usage(payload)
|
||||
|
||||
data = {
|
||||
"name": fields["run_name"],
|
||||
"run_type": "llm",
|
||||
"inputs": payload,
|
||||
"inputs": inputs,
|
||||
"outputs": outputs,
|
||||
"session_name": fields["project_name"],
|
||||
"start_time": payload["startTime"],
|
||||
|
|
|
|||
|
|
@ -5547,18 +5547,6 @@ def scrub_sensitive_keys_in_metadata(litellm_params: Optional[dict]):
|
|||
litellm_params["_langfuse_masking_function"] = masking_fn
|
||||
litellm_params["metadata"] = metadata
|
||||
|
||||
## check user_api_key_metadata for sensitive logging keys
|
||||
cleaned_user_api_key_metadata = {}
|
||||
if "user_api_key_metadata" in metadata and isinstance(metadata["user_api_key_metadata"], dict):
|
||||
for k, v in metadata["user_api_key_metadata"].items():
|
||||
if k == "logging": # prevent logging user logging keys
|
||||
cleaned_user_api_key_metadata[k] = "scrubbed_by_litellm_for_sensitive_keys"
|
||||
else:
|
||||
cleaned_user_api_key_metadata[k] = v
|
||||
|
||||
metadata["user_api_key_metadata"] = cleaned_user_api_key_metadata
|
||||
litellm_params["metadata"] = metadata
|
||||
|
||||
return litellm_params
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -31,6 +31,10 @@ _EXTRA_SENSITIVE_CALLBACK_KEYS = {"gcs_path_service_account"}
|
|||
# already-encrypted input cheaply (no decrypt-attempt round trip) and
|
||||
# avoid double-encrypting if `LITELLM_SALT_KEY` is rotated between writes.
|
||||
_CALLBACK_VAR_ENCRYPTED_PREFIX = "litellm_enc::"
|
||||
# Metadata slots that hold operator-configured callback setup (and therefore
|
||||
# integration credentials). Resolved from UserAPIKeyAuth during pre-call setup,
|
||||
# never read back off the copies stamped into request metadata.
|
||||
_CALLBACK_CONFIG_SLOTS = frozenset({"logging", "callback_settings"})
|
||||
|
||||
blue_color_code = "\033[94m"
|
||||
reset_color_code = "\033[0m"
|
||||
|
|
@ -547,6 +551,13 @@ def normalize_callback_names(callbacks: Iterable[Any]) -> List[Any]:
|
|||
return [c.lower() if isinstance(c, str) else c for c in callbacks]
|
||||
|
||||
|
||||
def strip_callback_config(metadata: dict[str, Any] | None) -> dict[str, Any] | None:
|
||||
"""Return key/team metadata without the slots that carry callback credentials."""
|
||||
if not isinstance(metadata, dict):
|
||||
return metadata
|
||||
return {k: v for k, v in metadata.items() if k not in _CALLBACK_CONFIG_SLOTS}
|
||||
|
||||
|
||||
def encrypt_callback_vars(metadata: Any) -> Any:
|
||||
"""Return a deep copy of metadata with callback_vars values encrypted at rest.
|
||||
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ from litellm.proxy._types import (
|
|||
from litellm.proxy.common_utils.callback_utils import (
|
||||
decrypt_callback_vars,
|
||||
get_metadata_variable_name_from_kwargs,
|
||||
strip_callback_config,
|
||||
)
|
||||
from litellm.proxy.common_utils.http_parsing_utils import _safe_get_request_headers
|
||||
|
||||
|
|
@ -1032,7 +1033,7 @@ class LiteLLMProxyRequestSetup:
|
|||
user_api_key_budget_reset_at=(
|
||||
user_api_key_dict.budget_reset_at.isoformat() if user_api_key_dict.budget_reset_at else None
|
||||
),
|
||||
user_api_key_auth_metadata=user_api_key_dict.metadata,
|
||||
user_api_key_auth_metadata=strip_callback_config(user_api_key_dict.metadata),
|
||||
)
|
||||
return user_api_key_logged_metadata
|
||||
|
||||
|
|
@ -1670,8 +1671,8 @@ async def add_litellm_data_to_request(
|
|||
data[_metadata_variable_name]["user_api_key_user_spend"] = user_api_key_dict.user_spend
|
||||
data[_metadata_variable_name]["user_api_key_user_max_budget"] = user_api_key_dict.user_max_budget
|
||||
|
||||
data[_metadata_variable_name]["user_api_key_metadata"] = user_api_key_dict.metadata
|
||||
data[_metadata_variable_name]["user_api_key_team_metadata"] = user_api_key_dict.team_metadata
|
||||
data[_metadata_variable_name]["user_api_key_metadata"] = strip_callback_config(user_api_key_dict.metadata)
|
||||
data[_metadata_variable_name]["user_api_key_team_metadata"] = strip_callback_config(user_api_key_dict.team_metadata)
|
||||
data[_metadata_variable_name]["user_api_key_object_permission_id"] = getattr(
|
||||
user_api_key_dict, "object_permission_id", None
|
||||
)
|
||||
|
|
|
|||
|
|
@ -109,6 +109,7 @@ from litellm.proxy.common_utils.callback_utils import (
|
|||
is_sensitive_callback_key,
|
||||
normalize_callback_names,
|
||||
process_callback,
|
||||
strip_callback_config,
|
||||
)
|
||||
from litellm.proxy.common_utils.realtime_utils import _realtime_request_body
|
||||
from litellm.router_utils.add_retry_fallback_headers import (
|
||||
|
|
@ -13375,7 +13376,7 @@ async def async_queue_request(
|
|||
# extra_body); see above for the same guard upstream.
|
||||
data["metadata"] = {}
|
||||
data["metadata"]["user_api_key"] = user_api_key_dict.api_key
|
||||
data["metadata"]["user_api_key_metadata"] = user_api_key_dict.metadata
|
||||
data["metadata"]["user_api_key_metadata"] = strip_callback_config(user_api_key_dict.metadata)
|
||||
_headers = _safe_get_request_headers(request).copy()
|
||||
_headers.pop("authorization", None) # do not store the original `sk-..` api key in the db
|
||||
data["metadata"]["headers"] = _headers
|
||||
|
|
|
|||
|
|
@ -347,3 +347,90 @@ class TestLangsmithRedactUserApiKeyInfo:
|
|||
assert "user_api_key_user_id" not in nested
|
||||
assert nested["session_id"] == "sess-1"
|
||||
assert extra["session_id"] == "sess-1"
|
||||
|
||||
def test_redact_enabled_strips_user_api_key_info_from_inputs(self, reset_redact_flag):
|
||||
"""
|
||||
Regression (LIT-4306): `inputs` is the whole StandardLoggingPayload, so
|
||||
`redact_user_api_key_info` has to cover `inputs.metadata` the same way it
|
||||
covers `extra` - including the nested `requester_metadata` copy. Before
|
||||
the fix `extra` was redacted and `inputs` shipped every user_api_key_*
|
||||
field verbatim.
|
||||
"""
|
||||
litellm.redact_user_api_key_info = True
|
||||
logger = self._logger()
|
||||
metadata = self._metadata_with_user_api_key_fields()
|
||||
metadata["user_api_key_auth_metadata"] = {"priority": "high"}
|
||||
payload = {
|
||||
"id": "run-1",
|
||||
"response": {"choices": []},
|
||||
"metadata": metadata,
|
||||
"startTime": 1.0,
|
||||
"endTime": 2.0,
|
||||
"request_tags": [],
|
||||
"error_str": None,
|
||||
"status": "success",
|
||||
"response_cost": 0.0,
|
||||
"prompt_tokens": 1,
|
||||
"completion_tokens": 1,
|
||||
"total_tokens": 2,
|
||||
}
|
||||
credentials = {
|
||||
"LANGSMITH_API_KEY": "test-key",
|
||||
"LANGSMITH_PROJECT": "test-project",
|
||||
"LANGSMITH_BASE_URL": "https://api.smith.langchain.com",
|
||||
}
|
||||
|
||||
data = logger._prepare_log_data(
|
||||
kwargs={"litellm_params": {"metadata": metadata}, "standard_logging_object": payload},
|
||||
response_obj=None,
|
||||
start_time=1.0,
|
||||
end_time=2.0,
|
||||
credentials=credentials,
|
||||
)
|
||||
|
||||
inputs_metadata = data["inputs"]["metadata"]
|
||||
assert [k for k in inputs_metadata if k.startswith("user_api_key")] == []
|
||||
assert [k for k in inputs_metadata["requester_metadata"] if k.startswith("user_api_key")] == []
|
||||
# inputs and extra must agree - they go through the same redaction now
|
||||
assert [k for k in data["extra"] if k.startswith("user_api_key")] == []
|
||||
# non-identity payload is untouched
|
||||
assert inputs_metadata["model"] == "gpt-4"
|
||||
assert inputs_metadata["requester_metadata"]["session_id"] == "sess-1"
|
||||
assert data["inputs"]["total_tokens"] == 2
|
||||
# the shared standard_logging_object other loggers read is not mutated
|
||||
assert "user_api_key_hash" in payload["metadata"]
|
||||
assert "user_api_key_user_id" in payload["metadata"]["requester_metadata"]
|
||||
|
||||
def test_redact_disabled_keeps_user_api_key_info_in_inputs(self, reset_redact_flag):
|
||||
"""Flag off: the identity fields stay. The flag governs them, not this fix."""
|
||||
litellm.redact_user_api_key_info = False
|
||||
logger = self._logger()
|
||||
metadata = self._metadata_with_user_api_key_fields()
|
||||
payload = {
|
||||
"id": "run-1",
|
||||
"response": {"choices": []},
|
||||
"metadata": metadata,
|
||||
"startTime": 1.0,
|
||||
"endTime": 2.0,
|
||||
"request_tags": [],
|
||||
"error_str": None,
|
||||
"status": "success",
|
||||
"response_cost": 0.0,
|
||||
"prompt_tokens": 1,
|
||||
"completion_tokens": 1,
|
||||
"total_tokens": 2,
|
||||
}
|
||||
|
||||
data = logger._prepare_log_data(
|
||||
kwargs={"litellm_params": {"metadata": metadata}, "standard_logging_object": payload},
|
||||
response_obj=None,
|
||||
start_time=1.0,
|
||||
end_time=2.0,
|
||||
credentials={
|
||||
"LANGSMITH_API_KEY": "test-key",
|
||||
"LANGSMITH_PROJECT": "test-project",
|
||||
"LANGSMITH_BASE_URL": "https://api.smith.langchain.com",
|
||||
},
|
||||
)
|
||||
|
||||
assert data["inputs"]["metadata"]["user_api_key_hash"] == "abc123"
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ from litellm.proxy.common_utils.callback_utils import (
|
|||
get_remaining_tokens_and_requests_from_request_data,
|
||||
normalize_callback_names,
|
||||
sanitize_openai_provider_metadata,
|
||||
strip_callback_config,
|
||||
)
|
||||
import litellm
|
||||
|
||||
|
|
@ -452,3 +453,41 @@ def test_initialize_callbacks_on_proxy_non_dict_callback_specific_params_root(
|
|||
)
|
||||
finally:
|
||||
litellm.callbacks = original_callbacks
|
||||
|
||||
|
||||
def test_strip_callback_config_drops_credential_bearing_slots():
|
||||
"""
|
||||
`logging` and `callback_settings` hold operator-configured integration
|
||||
credentials. Both must be dropped from the key/team metadata the proxy
|
||||
stamps into request metadata, while every other field survives untouched
|
||||
(`priority` is read back by the dynamic rate limiter, `guardrails` by the
|
||||
guardrail hooks).
|
||||
"""
|
||||
metadata = {
|
||||
"logging": [
|
||||
{
|
||||
"callback_name": "langsmith",
|
||||
"callback_vars": {"langsmith_api_key": "litellm_enc::ciphertext"},
|
||||
}
|
||||
],
|
||||
"callback_settings": {"callback_vars": {"langfuse_secret_key": "litellm_enc::other"}},
|
||||
"priority": "high",
|
||||
"guardrails": ["presidio"],
|
||||
"langsmith_provisioning": {"api_key_id": "prov-1"},
|
||||
}
|
||||
|
||||
stripped = strip_callback_config(metadata)
|
||||
|
||||
assert "logging" not in stripped
|
||||
assert "callback_settings" not in stripped
|
||||
assert stripped["priority"] == "high"
|
||||
assert stripped["guardrails"] == ["presidio"]
|
||||
assert stripped["langsmith_provisioning"] == {"api_key_id": "prov-1"}
|
||||
# the caller's dict (UserAPIKeyAuth.metadata) is shared state - never mutate it
|
||||
assert "logging" in metadata
|
||||
assert "callback_settings" in metadata
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", [None, "not-a-dict", 42])
|
||||
def test_strip_callback_config_passes_through_non_dicts(value):
|
||||
assert strip_callback_config(value) is value
|
||||
|
|
|
|||
|
|
@ -5421,3 +5421,39 @@ async def test_overwrite_user_with_key_hash_rejects_alias_without_marker(monkeyp
|
|||
)
|
||||
|
||||
assert updated_data["user"] == "caller-chosen-id"
|
||||
|
||||
|
||||
def test_get_sanitized_user_information_from_key_drops_callback_config():
|
||||
"""
|
||||
Regression (LIT-4306): `user_api_key_auth_metadata` lands in the
|
||||
StandardLoggingPayload every integration receives, so the per-key callback
|
||||
config (and the integration credentials inside it) must not ride along.
|
||||
Everything else - notably `priority`, which the dynamic rate limiter reads
|
||||
back off this exact field - has to survive.
|
||||
"""
|
||||
user_api_key_dict = UserAPIKeyAuth(
|
||||
api_key="test-key-hash",
|
||||
metadata={
|
||||
"logging": [
|
||||
{
|
||||
"callback_name": "langsmith",
|
||||
"callback_vars": {"langsmith_api_key": "litellm_enc::ciphertext"},
|
||||
}
|
||||
],
|
||||
"callback_settings": {"callback_vars": {"langfuse_secret_key": "litellm_enc::other"}},
|
||||
"priority": "high",
|
||||
},
|
||||
)
|
||||
|
||||
result = LiteLLMProxyRequestSetup.get_sanitized_user_information_from_key(
|
||||
user_api_key_dict=user_api_key_dict
|
||||
)
|
||||
|
||||
auth_metadata = result["user_api_key_auth_metadata"]
|
||||
assert "logging" not in auth_metadata
|
||||
assert "callback_settings" not in auth_metadata
|
||||
assert "litellm_enc::" not in json.dumps(auth_metadata)
|
||||
assert auth_metadata["priority"] == "high"
|
||||
# UserAPIKeyAuth is the live auth object; the per-key callbacks are resolved
|
||||
# from it during pre-call, so it must not be mutated by building the log view
|
||||
assert "logging" in (user_api_key_dict.metadata or {})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue