Encrypt callback_vars in key/team metadata in DB - #27141
Conversation
|
Michael Riad Zaky seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Greptile SummaryThis PR encrypts credential-bearing
Confidence Score: 3/5Not safe to merge without addressing the silent encryption failure: a missing LITELLM_SALT_KEY causes credentials to be stored as plaintext with no log, and /key/info then returns those secrets unmasked. The write path correctly encrypts when LITELLM_SALT_KEY is present, and the pre-call decryption path is properly wired. However, the bare except Exception in _encrypt_if_plaintext swallows any encryption failure silently, leaving credentials as plaintext in the DB and exposed via /key/info without any operator-visible signal. Additionally, neither /key/info, /v2/key/info, nor /team/info strip or redact callback_vars from the response, so a misconfigured environment would still surface plaintext secrets through those endpoints. litellm/proxy/common_utils/callback_utils.py (_encrypt_if_plaintext silent fallback) and litellm/proxy/management_endpoints/key_management_endpoints.py (info_key_fn / info_key_fn_v2 response handling)
|
| Filename | Overview |
|---|---|
| litellm/proxy/common_utils/callback_utils.py | Adds encrypt/decrypt utilities for callback_vars; the silent exception fallback in _encrypt_if_plaintext stores plaintext without any log warning when LITELLM_SALT_KEY is missing. |
| litellm/proxy/management_endpoints/key_management_endpoints.py | Wires encrypt_callback_vars into key creation/update and decrypt into key_health; /key/info and /v2/key/info still return callback_vars (as encrypted blobs or plaintext on failure) without stripping sensitive fields. |
| litellm/proxy/litellm_pre_call_utils.py | Correctly wraps key and team metadata reads with decrypt_callback_vars before use in the hot path; changes are minimal and safe. |
| litellm/proxy/management_endpoints/team_callback_endpoints.py | Adds encrypt_callback_vars before the DB write in add_team_callbacks and disable_team_logging; encryption is applied correctly on the write path. |
| litellm/proxy/management_endpoints/team_endpoints.py | Encrypts metadata before new_team and update_team DB writes; /team/info also returns metadata with encrypted blobs or plaintext without stripping, the same gap as /key/info. |
Comments Outside Diff (1)
-
litellm/proxy/management_endpoints/key_management_endpoints.py, line 3244-3249 (link)/v2/key/infoand/key/infoexposecallback_varswithout field-level redactionAfter this PR,
callback_varsvalues written by the encryption path carry a visiblelitellm_enc::prefix followed by the ciphertext and are returned verbatim by bothinfo_key_fn_v2andinfo_key_fn. In environments whereLITELLM_SALT_KEYis not set the fallback is silent plaintext (see companion comment), and in configured environments callers receive an opaque blob that leaks field names and the encryption scheme prefix. Stripping or redactingcallback_varsentirely before the response is returned is the more principled fix: it prevents both plaintext leakage when encryption is misconfigured and ciphertext exposure.
Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'origin/lit..." | Re-trigger Greptile
24d4d6a to
99a124e
Compare
|
@greptile-ai |
This is purposefully a breaking, backwards-incompatible change for security reasons. Users should not be relying on third-party plain-text keys being accessible from the DB / endpoints, and the codebase generally does not allow users to see complete keys after initial generation or input. |
|
@greptile-ai |
99a124e to
52cc3ef
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
e412d0d to
7887bfb
Compare
7887bfb to
f05da4f
Compare
…itellm_langfuseMetadataRedact # Conflicts: # litellm/proxy/litellm_pre_call_utils.py
|
@greptile review again. Pushing back on returning plaintext secrets inside of /key/info and /v2/key/info, this is bad for security |
| try: | ||
| return _CALLBACK_VAR_ENCRYPTED_PREFIX + encrypt_value_helper(value) | ||
| except Exception: | ||
| # No salt key / master key configured — leave the value as-is rather | ||
| # than crash the write. Dev environments without LITELLM_SALT_KEY hit | ||
| # this path; production always has a master key so encryption proceeds. | ||
| return value |
There was a problem hiding this comment.
Silent encryption failure leaves secrets in plaintext with no operator signal
When LITELLM_SALT_KEY is absent or encrypt_value_helper raises for any other reason (key derivation error, misconfigured secret store, etc.), the bare except Exception block silently returns the original plaintext value and writes it to the DB — with no log at any level. The PR's stated security goal (preventing plaintext credentials in the DB) is then not achieved, but neither the operator nor any monitoring system receives any indication. Critically, a /key/info or /v2/key/info call against such a row would then return the plaintext secret — the exact scenario the reviewer is pushing back on.
At minimum a verbose_proxy_logger.warning(...) should be emitted so operators know encryption was skipped.
| try: | |
| return _CALLBACK_VAR_ENCRYPTED_PREFIX + encrypt_value_helper(value) | |
| except Exception: | |
| # No salt key / master key configured — leave the value as-is rather | |
| # than crash the write. Dev environments without LITELLM_SALT_KEY hit | |
| # this path; production always has a master key so encryption proceeds. | |
| return value | |
| try: | |
| return _CALLBACK_VAR_ENCRYPTED_PREFIX + encrypt_value_helper(value) | |
| except Exception: | |
| # No salt key / master key configured — leave the value as-is rather | |
| # than crash the write. Dev environments without LITELLM_SALT_KEY hit | |
| # this path; production always has a master key so encryption proceeds. | |
| verbose_proxy_logger.warning( | |
| "callback_utils: failed to encrypt callback_var '%s' — " | |
| "storing as plaintext. Set LITELLM_SALT_KEY to enable encryption.", | |
| key, | |
| ) | |
| return value |
…itellm_langfuseMetadataRedact
3b2ce20
into
litellm_internal_staging
Co-authored-by: Michael Riad Zaky <michaelr@Michaels-MacBook-Air.local> Co-authored-by: Yuneng Jiang <yuneng@berri.ai>
Relevant issues
Several of the API keys stored in the DB are being kept in plain-text rather than encrypted. This allows anyone with auth access to the key to retrieve it through endpoints like key/info, when this should not be the case. For example, a team member with access to team keys can find team API keys for third-party services by hitting that endpoint. This PR encrypts them when stored in the DB and can decrypt them if needed with the existing salting pattern.
This change encrypts keys in the DB for the following:
Linear ticket
Resolves LIT-1958.
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Example with Langfuse:

Type
🐛 Bug Fix
✅ Test
Changes
callback_utils, litellm_pre_call_utils, key_management_endpoints, team_endpoints
Note
Medium Risk
Touches proxy key/team metadata persistence and dynamic logging callback resolution; incorrect encryption/decryption or key-rotation edge cases could break callbacks or make stored metadata unreadable.
Overview
Encrypts credential-bearing
callback_varsfields before they are stored in DB-backed key/team metadata, and transparently decrypts them when used at runtime.Adds
encrypt_callback_vars/decrypt_callback_varsutilities with a sentinel prefix and sensitive-key detection, and wires them into key creation/update flows and team callback/team metadata writes; pre-call logging settings now read decrypted values. Updates and adds unit tests to validate round-trip behavior, idempotency, non-mutation, plaintext passthrough, and that management endpoints persist encrypted values.Reviewed by Cursor Bugbot for commit f05da4f. Bugbot is set up for automated code reviews on this repo. Configure here.