fix(langfuse)!: source the emitted metadata blob from StandardLoggingPayload - #36744
Conversation
Greptile SummaryThe PR hardens Langfuse metadata emission by sourcing generation metadata from the normalized logging payload and retaining only explicit computed enrichments.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| litellm/integrations/langfuse/langfuse.py | Reworks Langfuse metadata assembly around normalized logging metadata and explicit enrichments without introducing a blocking failure. |
| tests/test_litellm/integrations/test_langfuse.py | Adds focused mocked regression coverage for the revised metadata and debug-emission paths. |
Reviews (4): Last reviewed commit: "fix(langfuse): source the emitted metada..." | Re-trigger Greptile
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| assembled: Final = { | ||
| **allowlisted_metadata, | ||
| **{ | ||
| key: value | ||
| for key, value in steering_metadata.items() | ||
| if isinstance(value, str) and key not in allowlisted_metadata | ||
| }, | ||
| **enrichments, | ||
| } |
There was a problem hiding this comment.
🔴 Custom labels sent with a request stop appearing in Langfuse traces
Custom values a caller attaches to a request are dropped from the emitted trace details ({**allowlisted_metadata, **enrichments} at litellm/integrations/langfuse/langfuse.py:765) instead of being copied back in, so anything the caller labelled the request with disappears from Langfuse.
Impact: Users who tag requests with their own fields lose those fields in Langfuse, breaking existing dashboards, filters and searches built on them.
The promised re-injection of caller string values is missing from the assembled blob
The PR description states "Caller strings are re-injected, so custom trace metadata survives", and the test docstring at tests/test_litellm/integrations/test_langfuse.py:636-641 reasons about a "string re-injection" that the deny-list must guard. No such re-injection exists in the code: generation_params["metadata"] is built purely from standard_logging_object["metadata"] merged with the litellm-computed enrichments (litellm/integrations/langfuse/langfuse.py:764-766). clean_metadata, which is the only thing derived from the raw request metadata, is now used exclusively for steering (session_id, trace_*, generation_name, prompt, version) and never contributes to the emitted blob.
Consequently _DENIED_STEERING_KEYS (litellm/integrations/langfuse/langfuse.py:51) has no effect on what is emitted at all, contradicting the stated rationale.
For SDK callers (litellm.completion(..., metadata={"my_key": "v"})) the loss is total: StandardLoggingPayloadSetup only populates requester_metadata from a nested metadata["metadata"] key (see the _potential_requester_metadata block in litellm/litellm_core_utils/litellm_logging.py), so flat caller keys have no allowlist entry and vanish. Proxy callers retain their values only under the nested requester_metadata key, which is a different shape than before. tests/test_litellm/integrations/test_langfuse.py:600-611 asserts this drop, so the tests lock in the behavior the description says was avoided.
Was this helpful? React with 👍 or 👎 to provide feedback.
| # StandardLoggingPayload is fail-open, so when it is absent the emitted | ||
| # blob is the caller strings and litellm enrichments only. Falling back | ||
| # to raw metadata here would put the UserAPIKeyAuth object on the wire |
There was a problem hiding this comment.
🟡 Explanatory comments added to the test file conflict with the repository's no-comments rule
Multi-line explanatory prose comments were added to the test fixture (# only real StandardLoggingMetadata fields: ... at tests/test_litellm/integrations/test_langfuse.py:385-387), which the repository guidelines explicitly disallow.
Impact: The change does not follow the project's documented style requirements and will need to be reworked before merge.
CLAUDE.md forbids comments outside of a narrow allowlist
CLAUDE.md states comments are only allowed when "absolutely necessary to explain some very complex business logic", when used as tool input (lint/type suppressions with a named rule), or for TODO/FIXME. The added comments are narrative explanations, not any of those categories. A second instance is at tests/test_litellm/integrations/test_langfuse.py:751-752 (# hidden_params comes off the payload, so it is omitted rather than emitted as an unserializable placeholder). Both should be removed or folded into the surrounding test docstrings.
Was this helpful? React with 👍 or 👎 to provide feedback.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
1a7f2a2 to
34df173
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
34df173 to
3c57c13
Compare
|
@greptileai review latest head |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3c57c13. Configure here.
ba01e71 to
bd0f074
Compare
|
@greptileai review latest head |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bd0f074. Configure here.
58298ad to
5afda54
Compare
…ayload Request metadata carries the whole UserAPIKeyAuth object, whose team_metadata holds the customer's own langfuse callback_vars. The only filter on the emitted blob was a four key deny list written as a circular reference crash guard, so those credentials reached the customer's own langfuse traces. The emitted blob is now the StandardLoggingPayload allowlist plus the litellm computed enrichments, and nothing is copied across from raw request metadata. That makes the credential exclusion structural rather than a filter someone has to keep correct. Steering keys keep reading raw metadata, matching literal_ai. Proxy callers are unaffected: their request metadata already rides under the allowlisted requester_metadata key, nesting intact. debug_langfuse dumped raw request metadata into the trace as a second copy of the same leak. It now emits caller scalars only. When StandardLoggingPayload is absent the trace is still emitted with the existing trace_id fallback, so failure traces survive.
5afda54 to
7189980
Compare
|
@greptileai please review the current head 7189980 |
|
bugbot run |
| debug_metadata: Final = { | ||
| key: value for key, value in metadata.items() if isinstance(value, (str, int, float, bool)) | ||
| } | ||
| trace_params["metadata"] = { | ||
| **(trace_params.get("metadata") or _NO_METADATA), | ||
| "metadata_passed_to_litellm": debug_metadata, | ||
| } |
There was a problem hiding this comment.
🟨 Debug trace dump still emits raw caller/proxy scalar metadata without user_api_key redaction
When debug_langfuse is enabled, the trace metadata dump is built directly from the raw request metadata, keeping every scalar value (debug_metadata at litellm/integrations/langfuse/langfuse.py:633-639). Raw proxy metadata contains scalar identity fields such as user_api_key (the hashed key), user_api_key_alias, team/org ids, and any scalar the caller injected. Unlike the emitted generation blob, this dump does not pass through redact_user_api_key_info, so the litellm_settings.redact_user_api_key_info flag has no effect on it.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7189980. Configure here.
423b791
into
litellm_internal_staging
yucheng-berri
changed the title
2 participants
TLDR
Problem this solves:
debug_langfuseships the same credentials a second timeHow it solves it:
User Flow
Before: a team that configured its own Langfuse destination finds its Langfuse secret key sitting in the traces it can read
user_api_key_auth.team_metadata.logging[0].callback_varsthey can readlangfuse_secret_keyandlangfuse_public_keyin fullAfter: the same trace carries the request's identity and cost, and no credentials
user_api_key_authis gone, along withuser_api_key_team_metadataanduser_api_key_metadata; key alias, team alias, spend and cost are still there, and anything they passed inmetadatastill shows up underrequester_metadataRelevant issues
Linear ticket
Resolves LIT-5492
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
Live proxy with real Postgres, a real Gemini call, and the Langfuse destination pointed at a local sink that records the exact outbound bytes. A marker credential is planted in the team row's
callback_vars, in the legacy plaintext shape that predates at-rest encryption.Setup, once:
BEFORE, at
fdd72b5b23:The captured body, verbatim:
AFTER, at
bd0f074a6a, same rig, same team row, four scenarios:Legs A, B and C on the wire:
Leg D is a pre-existing hole this PR does not close, disclosed rather than hidden.
update_trace_keysnames a key that the existing-trace branch pops out of raw metadata straight onto
trace_params, whichnever passes through the allowlisted blob. The generation metadata is clean even on that leg; the auth
object lands as a top-level field on the trace object. The channel is byte-identical before and after
this commit. It belongs to LIT-5484, which owns the steering redesign.
Real Langfuse cloud, same team, same request
Both traces were produced against Langfuse cloud with the team's own credentials, stored encrypted
at rest, so the destination resolves correctly and the value that leaks is ciphertext rather than a
usable secret.
Before, the generation metadata carries 52 keys. The team's own Langfuse credentials are readable at
user_api_key_auth.team_metadata.logging[0].callback_vars, which is the path the report describes.After, the same request emits 38 allowlisted fields.
user_api_key_authand thecallback_varsitcarried are gone.
user_api_key_auth_metadatareads as an empty object, which is the stripped copyrather than the leaking field, and the caller's own metadata still arrives under
requester_metadata.Emitted metadata delta, measured on the same live rig, base 52 keys to head 38:
dropped, credential carriers:
user_api_key_auth,user_api_key_team_metadata,user_api_key_metadatadropped, proxy and router internals with no allowlist entry:
agent_id,attempted_retries,caller_tags,deployment,deployment_model_name,global_max_parallel_requests,inherited_tags,litellm_api_version,litellm_parent_otel_span,litellm_received_at,max_retries,model_group,model_group_alias,model_group_size,model_info,queue_time_seconds,user_api_end_user_max_budget,user_api_key,user_api_key_end_user_model_max_budget,user_api_key_model_max_budget,user_api_key_object_permission_id,user_api_key_team_object_permission_idadded:
applied_guardrails,cold_storage_object_key,mcp_tool_call_metadata,prompt_management_metadata,requester_custom_headers,routing_decision,spend_logs_metadata,team_alias,team_id,usage_object,vector_store_request_metadatamodel_groupandmodel_idremain recoverable fromhidden_params, and request tags still reach Langfuse as first-class trace tags rather than as a metadata key.Note the Langfuse SDK reports the destination's own public key in its ingestion envelope under
metadata.public_key. That is the SDK authenticating, not our payload, and it is unchanged by this PR.Type
🐛 Bug Fix
Caveats (if any)
requester_metadata_log_langfuse_v1still ships raw metadata, unreachable on langfuse>=2update_trace_keyscan still name the auth object; owned by LIT-5484Final Attestation
Note
Medium Risk
Changes what metadata reaches external Langfuse projects (security fix with behavior shifts for custom top-level metadata); core proxy logging path is widely used but the change is localized to Langfuse v2 assembly.
Overview
Fixes a credential leak where team Langfuse keys and other proxy auth objects were copied from raw request metadata into Langfuse generation/trace payloads.
Langfuse v2 logging now builds the emitted metadata blob from
standard_logging_object["metadata"](the StandardLoggingPayload allowlist) plus explicit enrichments (response cost, filteredhidden_params,api_base, regions,cache_hit). Raw request metadata is no longer merged into that blob, so fields likeuser_api_key_authand team callback vars cannot ride along structurally.Generation metadata is assembled as allowlisted fields (with
redact_user_api_key_info) layered with enrichments; provider spans readhidden_paramsfrom enrichments instead of the old steering dict.debug_langfusestill attaches caller context to the trace, but only scalar keys from request metadata—never the full auth object. Steering controls (session_id,trace_name, deny-listed keys likeheaders/endpoint) still read raw metadata; sharedcache_hitNone→False normalization on kwargs is preserved.Adds regression tests (canary credentials, debug dump, spoofing, missing
standard_logging_objecton failures).Reviewed by Cursor Bugbot for commit 7189980. Bugbot is set up for automated code reviews on this repo. Configure here.