Skip to content

Revert "fix: langfuse trace leak key on model params"#23868

Merged
yuneng-jiang merged 1 commit intolitellm_internal_dev_03_16_2026from
revert-22188-litellm_langfuse_key_leakage
Mar 17, 2026
Merged

Revert "fix: langfuse trace leak key on model params"#23868
yuneng-jiang merged 1 commit intolitellm_internal_dev_03_16_2026from
revert-22188-litellm_langfuse_key_leakage

Conversation

@yuneng-jiang
Copy link
Copy Markdown
Contributor

@yuneng-jiang yuneng-jiang merged commit b4c9c8a into litellm_internal_dev_03_16_2026 Mar 17, 2026
3 of 4 checks passed
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Building Building Preview, Comment Mar 17, 2026 3:58pm

Request Review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 17, 2026

Greptile Summary

This PR reverts #22188 ("fix: langfuse trace leak key on model params"), which had introduced whitelist-based sanitisation of optional_params before they were forwarded to Langfuse, and a broader scrubbing pass over sensitive metadata keys. The stated reason is a set of failing CI tests linked in the description.

Key changes and concerns:

  • langfuse.py – credential leak reintroduced (v1 & v2 paths): The whitelist filter (ModelParamHelper.get_standard_logging_model_parameters) has been removed. Now only secret_fields is popped before optional_params is forwarded to Langfuse as modelParameters. Fields such as api_key, api_base, authorization, and headers — all of which may carry live credentials — can now appear in Langfuse traces. The existing tests test_langfuse_model_parameters_no_secret_leakage and test_langfuse_v2_uses_standard_logging_model_parameters directly assert that these fields must NOT reach Langfuse; this revert breaks both tests.

  • litellm_logging.py – incomplete metadata scrubbing: scrub_sensitive_keys_in_metadata previously (a) removed the entire user_api_key_auth object (noted as "contains full auth object with nested credentials"), (b) scrubbed callback_settings from three metadata sub-dicts, and (c) scrubbed the logging key from all three of user_api_key_metadata, user_api_key_auth_metadata, and user_api_key_team_metadata. After this revert only the logging key in user_api_key_metadata is handled; user_api_key_auth, callback_settings, and the other two metadata scopes are all left intact and will reach downstream logging integrations.

  • Partial litellm_params["metadata"] write-back: The litellm_params["metadata"] = metadata reassignment is now inside a conditional block, so when user_api_key_metadata is absent the mutation is never written back — a logic regression relative to the original code.

Recommendation: Rather than a full revert, identify the specific tests that were failing and fix the filtering logic to accommodate them while keeping the security protections in place. The whitelist approach in ModelParamHelper could be extended, or the specific params causing test failures could be explicitly allow-listed, without abandoning all credential filtering.

Confidence Score: 1/5

  • Not safe to merge — this revert reintroduces credential leakage (API keys, auth headers, full auth objects) into Langfuse traces and other logging integrations.
  • Two independent P0 security regressions: (1) raw optional_params including api_key/authorization/headers forwarded to Langfuse modelParameters in both v1 and v2 logging paths, and (2) user_api_key_auth (full auth object with nested credentials) and callback_settings are no longer scrubbed from logging metadata. Existing unit tests explicitly cover these scenarios and would fail with these changes.
  • Both changed files require attention: litellm/integrations/langfuse/langfuse.py (credential leak in modelParameters) and litellm/litellm_core_utils/litellm_logging.py (incomplete metadata scrubbing).

Important Files Changed

Filename Overview
litellm/integrations/langfuse/langfuse.py Reverts whitelist-based model-parameter sanitisation; raw optional_params (including api_key, authorization, headers) are now forwarded to Langfuse in both v1 and v2 logging paths, reintroducing the credential-leak vulnerability that PR #22188 fixed.
litellm/litellm_core_utils/litellm_logging.py scrub_sensitive_keys_in_metadata now omits removal of user_api_key_auth, callback_settings, and logging scrubbing for user_api_key_auth_metadata / user_api_key_team_metadata, reintroducing credential exposure in all logging integrations.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[LiteLLM request completes] --> B[Logging handler collects kwargs]
    B --> C[optional_params extracted]
    C --> D{This PR: pop secret_fields only}
    D --> E[optional_params still contains:\napi_key, api_base,\nauthorization, headers]
    E --> F[Passed as modelParameters\nto Langfuse v1 & v2]
    F --> G[🚨 Credentials stored\nin Langfuse trace]

    B --> H[scrub_sensitive_keys_in_metadata]
    H --> I{user_api_key_metadata?}
    I -- yes --> J[Scrub 'logging' key only\nin user_api_key_metadata]
    I -- no --> K[No-op]
    J --> L[user_api_key_auth NOT removed\ncallback_settings NOT removed\nother metadata fields NOT scrubbed]
    L --> G

    style G fill:#f55,color:#fff
    style D fill:#ffa500,color:#000
    style L fill:#ffa500,color:#000
Loading

Last reviewed commit: 467706e

Comment on lines 513 to 515
model=model_name,
modelParameters=sanitized_model_params,
modelParameters=optional_params,
prompt=input,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Credential leak in Langfuse v1 model parameters

After this revert, the raw optional_params dict (with only secret_fields popped) is passed directly to Langfuse as modelParameters. Fields like api_key, api_base, authorization, and headers can still leak to Langfuse traces.

The existing test test_langfuse_model_parameters_no_secret_leakage explicitly verifies that these fields must NOT appear in the logged parameters:

# Sensitive params that must NOT leak
"api_key": "sk-secret-key-12345",
"api_base": "https://my-private-endpoint.com",
"authorization": "Bearer sk-another-secret",
"headers": {"X-Api-Key": "secret-header-value"},

Popping only secret_fields is insufficient — these other keys will still pass through to Langfuse. The whitelist approach from ModelParamHelper.get_standard_logging_model_parameters() was what prevented leakage of all non-standard keys.

Comment on lines 839 to 841
"model": model_name,
"model_parameters": sanitized_model_params,
"model_parameters": optional_params,
"input": input if not mask_input else "redacted-by-litellm",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Credential leak in Langfuse v2 model parameters

Same credential leak issue as the v1 path: optional_params is passed directly to Langfuse's generation() without whitelist filtering. Fields such as api_key, authorization, and headers present in optional_params will be recorded in Langfuse traces.

The removed logic previously fell back to ModelParamHelper.get_standard_logging_model_parameters() when no standard_logging_object was available, providing a last-resort sanitisation layer. That safety net no longer exists.

Comment on lines +5572 to +5586
## 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
):
if metadata_field in metadata and isinstance(metadata[metadata_field], dict):
for sensitive_key in _sensitive_keys:
metadata[metadata_field].pop(sensitive_key, None)

## remove user_api_key_auth entirely - contains full auth object with nested credentials
metadata.pop("user_api_key_auth", None)
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

litellm_params["metadata"] = metadata
metadata["user_api_key_metadata"] = cleaned_user_api_key_metadata
litellm_params["metadata"] = metadata
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Multiple sensitive metadata fields no longer scrubbed

The original scrub_sensitive_keys_in_metadata removed three categories of sensitive data that are no longer handled after this revert:

  1. user_api_key_auth entire key — the old code explicitly called metadata.pop("user_api_key_auth", None) with the comment "contains full auth object with nested credentials". That object (a UserAPIKeyAuth instance) may include team secrets, org tokens, and other nested credentials. It is no longer removed.

  2. callback_settings — removed from all three metadata fields (user_api_key_metadata, user_api_key_auth_metadata, user_api_key_team_metadata) previously. These can contain callback API keys (e.g., Langfuse secret keys). Only user_api_key_metadata is touched now, and only the logging key, not callback_settings.

  3. user_api_key_auth_metadata and user_api_key_team_metadata — the logging key inside these two fields is no longer scrubbed. This means per-team / per-key logging credentials could reach Langfuse.

Additionally, the litellm_params["metadata"] = metadata write-back is now inside the if "user_api_key_metadata" in metadata block. If the incoming metadata dict was resolved via the or {} fallback (i.e., it's a freshly allocated dict), any mutations will not be persisted back to litellm_params when user_api_key_metadata is absent.

@ishaan-berri ishaan-berri deleted the revert-22188-litellm_langfuse_key_leakage branch March 26, 2026 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant