fix(team-callbacks): report API-registered callbacks from GET /team/{team_id}/callback - #35512
Conversation
…team_id}/callback
POST /team/{team_id}/callback writes metadata["logging"] while the GET read
metadata["callback_settings"], so every team configured through the API or the
Admin UI got back an empty list. c620d76 migrated the writer to the new key
and left this reader on the old one.
Resolve the read the same way request-time resolution does in
_get_dynamic_logging_metadata: a logging slot that is present wins outright and
callback_settings stays as the deprecated fallback, so the endpoint reports what
a request would really do rather than the union of both shapes. An empty logging
list therefore reports no callbacks, matching a request that fires none.
Decrypt callback_vars for the response and mask the credential keys. Ciphertext
would be unusable to the caller, and a value encrypted under a key that is no
longer classified as sensitive would otherwise come back as a raw blob.
Resolves LIT-5093
|
bugbot run |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 f02f4c1. Configure here.
| if not callbacks.callback_vars: | ||
| return | ||
| for key in tuple(callbacks.callback_vars): | ||
| if is_sensitive_callback_key(key): | ||
| callbacks.callback_vars[key] = _CALLBACK_VARS_REDACTED | ||
|
|
||
|
|
||
| def _resolve_team_callbacks(team_metadata: object) -> TeamCallbackMetadata: | ||
| """Report the callbacks that are actually in effect for a team. | ||
|
|
||
| A team's callback config can live in either of two metadata slots. | ||
| ``metadata["logging"]`` holds the ``AddTeamCallback`` entries written by | ||
| ``POST /team/{team_id}/callback`` and by the Admin UI, while | ||
| ``metadata["callback_settings"]`` holds the older ``TeamCallbackMetadata`` | ||
| shape. Request-time resolution in ``_get_dynamic_logging_metadata`` treats | ||
| the two as mutually exclusive: a populated ``logging`` slot wins outright | ||
| and ``callback_settings`` is consulted only as the deprecated fallback. | ||
| This reader applies the same precedence so it reports what a request would | ||
| really do. Merging the two instead would report a ``callback_settings`` | ||
| entry as active for a team whose requests never fire it. | ||
|
|
||
| Credential ``callback_vars`` are stored encrypted, so they are decrypted | ||
| before being masked by key; a value encrypted under a key that is no longer | ||
| classified as sensitive would otherwise come back as raw ciphertext. | ||
| """ | ||
| if not isinstance(team_metadata, dict): | ||
| return TeamCallbackMetadata() | ||
|
|
||
| decrypted = decrypt_callback_vars(team_metadata) | ||
| logging_entries = decrypted.get("logging") | ||
|
|
||
| if logging_entries is not None: | ||
| resolved = TeamCallbackMetadata() | ||
| for entry in logging_entries if isinstance(logging_entries, list) else (): | ||
| if not isinstance(entry, dict): | ||
| continue | ||
| callback = _get_validated_callback_metadata(item=entry, source="team-level read") | ||
| if callback is None: | ||
| continue | ||
| resolved = convert_key_logging_metadata_to_callback(data=callback, team_callback_settings_obj=resolved) | ||
| else: | ||
| callback_settings = decrypted.get("callback_settings") | ||
| resolved = ( | ||
| TeamCallbackMetadata(**callback_settings) if isinstance(callback_settings, dict) else TeamCallbackMetadata() | ||
| ) | ||
|
|
||
| _mask_sensitive_callback_vars(resolved) | ||
| return resolved |
There was a problem hiding this comment.
🟡 New callback resolution code builds results by repeated mutation instead of the required immutable style
The newly added resolution helper repeatedly overwrites its accumulator and mutates the credential mapping in place (resolved = convert_key_logging_metadata_to_callback(...) at litellm/proxy/management_endpoints/team_callback_endpoints.py:128), which the repository's coding guidelines explicitly disallow for new code.
Impact: The new code does not follow the project's mandated no-mutation/no-reassignment style, so it needs a rewrite before merge.
Rule source and affected code
CLAUDE.md requires for new/updated code: "No mutation; don't reassign variables, global or local. Instead of mutable lists and dicts, prefer tuples, frozen dataclasses..." and "build values in one shot with comprehensions or generators". _resolve_team_callbacks reassigns resolved inside the loop (litellm/proxy/management_endpoints/team_callback_endpoints.py:121-128), and _mask_sensitive_callback_vars (litellm/proxy/management_endpoints/team_callback_endpoints.py:89-93) mutates callbacks.callback_vars entries in place rather than constructing a new mapping.
Was this helpful? React with 👍 or 👎 to provide feedback.
Greptile SummaryThe PR updates the team callback read endpoint to resolve API-registered callback metadata using request-time precedence and safely mask credentials.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported undecryptable-value path now masks ciphertext that survives decryption, and the regression test exercises that behavior after a key change.
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/team_callback_endpoints.py | Resolves callbacks from the active metadata slot and masks sensitive or undecryptable values; the previously reported ciphertext-return issue is fixed. |
| tests/test_litellm/proxy/management_endpoints/test_team_callback_endpoints.py | Adds focused regression tests covering POST-to-GET behavior, metadata precedence, decryption failures, malformed entries, and legacy rows. |
| ui/litellm-dashboard/src/lib/http/schema.d.ts | Regenerates the endpoint documentation to describe callback resolution and credential masking. |
Reviews (2): Last reviewed commit: "fix(team-callbacks): mask callback vars ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
decrypt_callback_vars passes a value through untouched when it cannot be decrypted, which happens to existing rows after a salt-key rotation. Under a key that is not classified as sensitive that blob reached the caller as opaque ciphertext it could not use or tell apart from a real value, so mask anything still carrying the encrypted prefix. Raised by Greptile on the first commit.
|
@greptileai please review the current head 582587d The one concern from the previous review is addressed there: a value that fails to decrypt (existing rows after a salt-key rotation) is now masked rather than returned as ciphertext, since under a non-sensitive key it reached the caller as an opaque blob indistinguishable from a real value. Covered by test_get_team_callbacks_masks_values_that_fail_to_decrypt, which fails when the guard is removed. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
2 participants
TLDR
Problem this solves:
GET /team/{team_id}/callbackalways returned an empty listHow it solves it:
Relevant issues
Linear ticket
Resolves LIT-5093
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Root cause
POST /team/{team_id}/callbackwrites its entries tometadata["logging"], whileGET /team/{team_id}/callbackreadmetadata["callback_settings"]. Those are two different slots in the team row, so the GET reported nothing for any team configured through the API or the dashboard.The divergence came from
c620d76fe4(2025-09-17), which migrated the writer inadd_team_callbacksto the newerloggingkey and left the reader on the old one. The Admin UI kept working because it readsmetadata.loggingfrom/team/inforather than calling this endpoint.Approach
_resolve_team_callbacksmirrors the precedence that_get_dynamic_logging_metadataapplies at request time: aloggingslot that is present wins outright, andcallback_settingsis consulted only as the deprecated fallback. The two are resolved as mutually exclusive rather than merged, because merging would report acallback_settingsdestination as active for a team whose requests never send to it. An emptylogginglist therefore reports no callbacks, which is what such a team's requests actually do.callback_varsare decrypted for the response and credential keys are masked with the marker the audit-log redaction in this file already uses. The masking predicate isis_sensitive_callback_key, the same predicate that decides what gets encrypted on write, so read masking and write encryption cannot disagree. Non-secret configuration such as project names, bucket names, and hosts stays readable, which is what makes the response useful.Behavior changes
callback_varscredential values now come back as***REDACTED***. Previously they were returned as stored: ciphertext for rows written after callback encryption landed, and plaintext for rows predating it. No first-party consumer reads secrets back from this endpoint; the repo,ui/litellm-dashboard,litellm/proxy/client/, the e2e suites, and the docs were swept and only a POST-side helper exists.metadatais not a JSON object previously produced a 500 from anAttributeErrorand now returns the documented empty shape.loggingthat failAddTeamCallbackvalidation are skipped rather than failing the read, matching how request-time resolution treats them. Such entries do not fire callbacks either, so the response stays consistent with behavior.Known limitation, tracked separately
The masked value is a marker, not an absence, and the write paths have no rule for it. A caller that reads this endpoint and posts the payload back, for example a script that clones one team's logging config onto another, stores the literal
***REDACTED***as the credential. It is encrypted at rest like any real secret, so the row looks normal and that team's logging silently stops working. Reproduced on a live proxy: the source team keepslsv2-REAL-WORKING-KEYwhile the destination team ends up holding***REDACTED***.This is not a regression for any consumer that exists. Nothing in the repo reads this endpoint; the sweep covered
ui/litellm-dashboard/src,litellm/proxy/client/, the e2e suites, cookbooks and load tests, and the only caller anywhere is a POST-only e2e helper that readsstatuswithextra="ignore". The same clone flow did work before this PR, but only for pre-encryption rows and only because the endpoint handed back the credential in the clear, which is the exposure this PR closes; for teams configured through the API or the Admin UI the endpoint returned nothing at all, so there was no working flow to preserve.Tracked as LIT-5110 rather than fixed here, since the likely fix is to stop returning the credential keys at all rather than to add marker-restore logic to four separate write paths.
Scope
Limited to the GET. Two adjacent problems in the same area were found while reproducing this one and are tracked separately rather than bundled here:
POST /team/{team_id}/disable_loggingclears onlycallback_settings, so it is a no-op for teams configured through the API. Confirmed on the live rig: the endpoint returns success while the callback keeps delivering./team/info,/team/list,/v2/team/list, and/key/inforeturn team callbackcallback_varsunmasked, on a wider access gate than this endpoint.Screenshots / Proof of Fix
Live proxy with real Postgres and a real Bedrock model (
us.anthropic.claude-haiku-4-5-20251001-v1:0). No mocks.Before, at base
b1fd20f4cd. Register a callback, then read it back:The data was written correctly the whole time;
/team/infoon the same row shows where it went:After, at
f02f4c1813, same team row written by the unfixed build, same curl, no re-registration:Edge cases on the same live rig:
The response is only correct if it agrees with what a request would really do, so every team row on the rig was resolved through both this endpoint's resolver and the real
_get_dynamic_logging_metadata:Mutation matrix over the six new tests, each mutation applied to the fixed source and asserted to be a real edit before the suite ran:
End-to-end evidence
Captured after the fix, on the live rig, with the step order deliberately inverted from the verify run: a fresh team, read before any write, and the failure callback registered before the success one.
A
success_and_failureregistration lands in both lists, the non-secretlangfuse_hoststays readable, and every credential is masked.The response is only worth anything if it names the callbacks that really run, so the last check ties the report to an actual delivery. A team was given a langsmith callback pointed at a local HTTP collector, then a real Bedrock request was sent on a key scoped to that team:
The ticket noted the callback was visible in the Admin UI while the API returned nothing. Both now agree; the dashboard for the team above shows the same two integrations the API reports, Langfuse as Failure Only and LangSmith as Success and Failure:
No UI code is touched by this PR; the screenshot is there to show the dashboard and the API agree after the fix.
Type
🐛 Bug Fix
Changes
litellm/proxy/management_endpoints/team_callback_endpoints.pygains_resolve_team_callbacksand_mask_sensitive_callback_vars, andget_team_callbacksreads through the resolver. Six regression tests are added to the mapped test file, including a POST-then-GET round trip that feeds the GET exactly the payload the POST persisted so the two paths cannot drift apart again.schema.d.tscarries the regenerated docstring delta. A second commit masks callback vars that fail to decrypt, raised by Greptile against the first commit and covered by its own regression test.Final Attestation