fix(proxy): sanitize per-key callback config out of logged metadata - #32583
Conversation
Greptile SummaryThis PR sanitizes per-key and per-team callback configuration (the
Confidence Score: 5/5This PR is safe to merge — it fixes a data-leakage issue in logging paths without touching request routing or auth logic, and all changes are covered by targeted unit tests. All three call sites that stamp raw key/team metadata into request metadata are correctly updated. The old partial scrub is removed only after the upstream fix covers more fields. The LangSmith inputs/extra divergence is closed by a shared helper. Tests verify non-mutation of the live auth object and preservation of fields consumed by downstream features (rate limiter, guardrails). Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/common_utils/callback_utils.py | Adds strip_callback_config which returns a new dict without logging/callback_settings slots, leaving priority, guardrails, and other application fields intact. |
| litellm/integrations/langsmith.py | Extracts _redact_metadata helper shared by _build_extra_metadata and the new inputs construction; inputs now carries a redacted copy of payload.metadata instead of the raw payload. |
| litellm/proxy/litellm_pre_call_utils.py | Routes user_api_key_auth_metadata, user_api_key_metadata, and user_api_key_team_metadata through strip_callback_config at the two main call sites that build request metadata. |
| litellm/proxy/proxy_server.py | Applies strip_callback_config to user_api_key_metadata in async_queue_request, the third call site that stamps raw key metadata into request metadata. |
| litellm/litellm_core_utils/litellm_logging.py | Removes the legacy scrub_sensitive_keys_in_metadata block that only scrubbed logging under user_api_key_metadata; now fully superseded by the upstream strip_callback_config calls. |
| tests/test_litellm/integrations/test_langsmith_init.py | Adds two new tests covering the redact-enabled/disabled paths for inputs.metadata, including mutation-safety of the shared standard_logging_object. No real network calls. |
| tests/test_litellm/proxy/common_utils/test_callback_utils.py | Adds unit tests for strip_callback_config: verifies credential-bearing slots are dropped, other fields survive, original dict is not mutated, and non-dict values pass through unchanged. |
| tests/test_litellm/proxy/test_litellm_pre_call_utils.py | Adds a regression test for get_sanitized_user_information_from_key confirming callback config is stripped while priority survives and the source UserAPIKeyAuth is not mutated. |
Reviews (2): Last reviewed commit: "fix(proxy): sanitize per-key callback co..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
038b791 to
5e34e04
Compare
|
@grepile |
QA of this PRIndependent QA against a live proxy on a local Postgres, hitting the real Anthropic API with Setup for every run: a team whose metadata carries a team-level LangSmith callback, a key on that team whose metadata carries a key-level LangSmith callback plus TEAM=$(curl -s http://localhost:4001/team/new \
-H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' \
-d '{"team_alias":"qa-team","metadata":{"logging":[{"callback_name":"langsmith","callback_type":"success_and_failure","callback_vars":{"langsmith_api_key":"lsv2_sk_TEAMLEVEL_SECRET","langsmith_project":"team-proj","langsmith_base_url":"http://127.0.0.1:4002"}}]}}' | jq -r .team_id)
KEY=$(curl -s http://localhost:4001/key/generate \
-H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' \
-d "{\"models\":[\"claude-sonnet-5\"],\"team_id\":\"$TEAM\",\"metadata\":{\"logging\":[{\"callback_name\":\"langsmith\",\"callback_type\":\"success_and_failure\",\"callback_vars\":{\"langsmith_api_key\":\"lsv2_sk_KEYLEVEL_SECRET\",\"langsmith_project\":\"key-proj\",\"langsmith_base_url\":\"http://127.0.0.1:4002\"}}],\"langsmith_provisioning\":{\"api_key_id\":\"prov-uuid-1\",\"api_key_short\":\"lsv2_sk_a365...33b2\"},\"priority\":\"high\"}}" | jq -r .key)
PLAINKEY=$(curl -s http://localhost:4001/key/generate \
-H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' \
-d "{\"models\":[\"claude-sonnet-5\"],\"team_id\":\"$TEAM\"}" | jq -r .key)
curl -s http://localhost:4001/v1/chat/completions \
-H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' \
-d '{"model":"claude-sonnet-5","messages":[{"role":"user","content":"say hi in one word"}],"max_tokens":16}' \
| jq -c '{model, content: .choices[0].message.content, prompt_tokens: .usage.prompt_tokens}'{"model":"claude-sonnet-5","content":"Hi!","prompt_tokens":12}The chat responses are HTTP 200 and identical on both builds, and both keys keep logging (the run for jq -r '.body.post[] | "project=\(.session_name) inputs_user_api_key_fields=\([.inputs.metadata|keys[]|select(startswith("user_api_key"))]|length) extra_user_api_key_fields=\([.extra|keys[]|select(startswith("user_api_key"))]|length) callback_config_logged=\((.inputs.metadata.user_api_key_auth_metadata // {})|has("logging"))"' sink-capture.jsonl
So both halves of the fix reproduce. With the flag on, jq -c '.body.post[0].inputs.metadata.user_api_key_auth_metadata' sink-capture.jsonlbefore, flag off {"logging":[{"callback_name":"langsmith","callback_type":"success_and_failure","callback_vars":{"langsmith_api_key":"litellm_enc::bb11vqvWPXqRsZmL2_VCJ...","langsmith_project":"key-proj","langsmith_base_url":"http://127.0.0.1:4002"}}],"priority":"high","langsmith_provisioning":{"api_key_id":"prov-uuid-1","api_key_short":"lsv2_sk_a365...33b2"}}after, flag off {"priority":"high","langsmith_provisioning":{"api_key_id":"prov-uuid-1","api_key_short":"lsv2_sk_a365...33b2"}}A full recursive scan of the after-run captures finds no
K=$(curl -s http://localhost:4001/key/generate -H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' \
-d '{"models":["claude-sonnet-5"],"metadata":{"model_rpm_limit":{"claude-sonnet-5":1},"priority":"high"}}' | jq -r .key)
for i in 1 2 3; do curl -s -o /tmp/r$i.json -w "req$i http=%{http_code}\n" http://localhost:4001/v1/chat/completions \
-H "Authorization: Bearer $K" -H 'Content-Type: application/json' \
-d '{"model":"claude-sonnet-5","messages":[{"role":"user","content":"hi"}],"max_tokens":8}'; doneOn the test side, the three touched files pass on this branch (238 tests), and the two new behavioral tests fail when only CI is green apart from One thing outside the scope of this PR, worth a follow-up rather than a change here: QA verdict: passes |
38ea85b
into
litellm_internal_staging
TLDR
Problem this solves:
redact_user_api_key_infonever applied to LangSmith runinputsHow it solves it:
inputsandextraRelevant issues
Linear ticket
Resolves LIT-4306
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Verified against a live proxy making real Anthropic
claude-sonnet-5calls, withlitellm_settings.redact_user_api_key_info: trueand a per-key LangSmith callback whoselangsmith_base_urlpoints at a local sink that records the exact/runs/batchbody. Before =b9b27c2beb(the parent commit), after =5e34e0460b(this PR). The chat completion response is identical either way; the observable difference is what LiteLLM POSTs to LangSmith. Values below are placeholdersSetup, identical for both runs. Create a team and a key whose metadata carries per-key LangSmith callback config, matching the reported shape
Chat with that key. HTTP 200 in both runs, response unchanged by this PR
{"id":"chatcmpl-...","object":"chat.completion","model":"claude-sonnet-5","choices":[{"index":0,"message":{"role":"assistant","content":"Hi!"}}],"usage":{...}}Before, on
b9b27c2beb. Everyuser_api_key_*field ships underinputs.metadataeven though the flag is on, andextrais already clean, so the two disagreejq -c '.body.post[0].inputs.metadata.user_api_key_auth_metadata' sink-capture.jsonl{"logging":[{"callback_name":"langsmith","callback_type":"success_and_failure","callback_vars":{"langsmith_api_key":"litellm_enc::pt71BYmrVlJk...","langsmith_project":"key-proj","langsmith_base_url":"http://127.0.0.1:4002"}}],"priority":"high","langsmith_provisioning":{"api_key_id":"prov-uuid-1","api_key_short":"lsv2_sk_a365...33b2"}}After, on
5e34e0460b.inputsnow agrees withextra, and the key's callback config is gone from what is loggedjq -c '.body.post[0].inputs.metadata | {team_id, team_alias, model: .model, requester_metadata}' sink-capture.jsonl{"team_id":"d842a11f-b749-4722-9227-0d8b1a1c5536","team_alias":"repro-team","requester_metadata":{}}The second half is independent of the flag. Re-running with
redact_user_api_key_info: false, the identity fields stay (that is what the flag governs) but the callback config no longer reaches the logged payload, andpriority, which the dynamic rate limiter reads back off this exact field, is preservedjq -c '.body.post[0].inputs.metadata.user_api_key_auth_metadata' sink-capture.jsonl{"priority":"high","langsmith_provisioning":{"api_key_id":"prov-uuid-1","api_key_short":"lsv2_sk_a365...33b2"}}A key with no per-key logging config, and the team-level callback, both still log and still return 200 on the after run
Type
🐛 Bug Fix
Changes
LiteLLMProxyRequestSetup.get_sanitized_user_information_from_keycopiedUserAPIKeyAuth.metadataverbatim intouser_api_key_auth_metadata, so the key's callback configuration, including the values insidecallback_vars, reached theStandardLoggingPayloadthat every integration receives.add_litellm_data_to_requestandasync_queue_requeststamped the same raw metadata intouser_api_key_metadataanduser_api_key_team_metadata. The existing scrub inscrub_sensitive_keys_in_metadataonly matched the literal keyloggingunder one of the two field names and never coveredcallback_settings, so it missed the field that actually reaches the payloadThis adds
strip_callback_confignext to the callback_vars traversal that already lives incommon_utils/callback_utils.py, and routes all three sites through it. It drops theloggingandcallback_settingsslots and leaves everything else untouched, sopriorityfor the dynamic rate limiter,guardrailsfor the guardrail hooks, and the Arize/Phoenix project overrides all keep working. Those two slots are resolved fromUserAPIKeyAuthduring pre-call setup and are never read back off the logged copies, so nothing downstream loses input.litellm/proxy/utils.pybuilds its payload through the same helper on the proxy-error path and inherits this. With the source sanitized the old scrub is dead, so it is removedSeparately, LangSmith set the run's
inputsto the rawStandardLoggingPayloadwhile redacting onlyextra.redact_user_api_key_infotherefore left the wholeuser_api_key_*family ininputs.metadata, and the nestedrequester_metadatahandling thatextrahad was not applied there either. Both now go through one_redact_metadatahelper, which is what keeps them from drifting apart againTests cover both halves and fail on the parent commit:
strip_callback_configdrops the two slots without mutating the caller's dict,get_sanitized_user_information_from_keykeepsprioritywhile dropping the callback config, and_prepare_log_datawith the flag on leaves nouser_api_key_*field in eitherinputs.metadataor its nestedrequester_metadatawhile leaving the sharedstandard_logging_objectunmutatedFinal Attestation