Skip to content

fix(security): hash Bearer-prefixed API keys in spend logs - #31799

Merged
yucheng-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_fix_plaintext_api_key_in_spend_logs
Jul 6, 2026
Merged

fix(security): hash Bearer-prefixed API keys in spend logs#31799
yucheng-berri merged 2 commits into
litellm_internal_stagingfrom
litellm_fix_plaintext_api_key_in_spend_logs

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Relevant issues

Resolves LIT-4121

Linear ticket

Resolves LIT-4121

Pre-Submission checklist

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Screenshots / Proof of Fix

To verify the fix, run the proxy and send a request with an invalid model that triggers a failure:

python litellm/proxy/proxy_cli.py --config litellm/proxy/dev_config.yaml --detailed_debug --reload --use_v2_migration_resolver 2>&1 | tee litellm.log

curl -s http://localhost:4000/v1/chat/completions \
  -H "Authorization: Bearer sk-your-test-key" \
  -H "Content-Type: application/json" \
  -d '{"model": "nonexistent-model", "messages": [{"role": "user", "content": "hi"}]}'

Then query LiteLLM_SpendLogs where status = 'failure'; the api_key column should now contain a 64-char SHA256 hex digest instead of Bearer sk-...

Type

Bug Fix

Changes

The safety-net hash in get_logging_payload only checked api_key.startswith("sk-"), so keys arriving as "Bearer sk-..." on the failure path bypassed hashing and were stored in plaintext in both the api_key column and the metadata.user_api_key JSON field of LiteLLM_SpendLogs

The fix adds _hash_api_key_for_spend_log(api_key) -> str which strips a case-insensitive Bearer prefix before checking for sk- and hashing via hash_token. The fallback path also returns the stripped token (without the Bearer prefix) for non-sk keys, so the prefix is never persisted. Applied in two places:

  • get_logging_payload (the api_key column)
  • _get_spend_logs_metadata (the user_api_key field inside the JSON metadata column)

Regression tests cover Bearer-prefixed, bare, case-insensitive, already-hashed, Bearer-prefixed non-sk keys, idempotency, and the full get_logging_payload integration path


Note

High Risk
Security fix for credential leakage in LiteLLM_SpendLogs; scope is limited to spend-tracking serialization with added regression tests.

Overview
Fixes LIT-4121: failed-request spend logs could persist plaintext Bearer sk-... because hashing only ran when the value started with sk-.

Adds _hash_api_key_for_spend_log, which strips a case-insensitive Bearer prefix, hashes sk- tokens via hash_token, and leaves already-hashed values unchanged. It is used when building the api_key column in get_logging_payload and metadata.user_api_key in _get_spend_logs_metadata.

Regression tests cover Bearer/bare/case variants, non-sk tokens, and the full failure get_logging_payload path.

Reviewed by Cursor Bugbot for commit 4e0117c. Bugbot is set up for automated code reviews on this repo. Configure here.

The safety-net hash in get_logging_payload only checked for keys
starting with 'sk-', missing keys that arrived as 'Bearer sk-...'.
This caused plaintext API keys to be stored in SpendLogs for failed
requests while successful requests correctly stored SHA256 hashes.

Adds _hash_api_key_for_spend_log that strips the Bearer prefix
before hashing, applied to both the api_key column and the
metadata.user_api_key field in spend log payloads.
@greptile-apps

greptile-apps Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a new helper function _hash_api_key_for_spend_log that strips a case-insensitive Bearer prefix before checking for the sk- prefix and hashing via hash_token, then applies it in two places where spend log payloads are assembled.

  • get_logging_payload: replaces an if api_key.startswith("sk-") guard with a call to the new helper, so Bearer-wrapped keys in the api_key column are now hashed before storage.
  • _get_spend_logs_metadata: adds an equivalent guard for clean_metadata["user_api_key"], ensuring the JSON metadata column is also sanitized on the same code path.
  • Tests: six unit tests for _hash_api_key_for_spend_log (Bearer/bare/case/already-hashed/non-sk/idempotency) and one integration test exercising the full get_logging_payload path with a Bearer-prefixed key on a failure response.

Confidence Score: 5/5

Safe to merge — the change is isolated to spend-log serialization, adds no new I/O, and the tests directly exercise both the helper and the full payload path.

The new helper correctly strips the Bearer prefix before hashing and is idempotent for already-hashed tokens. It is applied consistently in both the api_key column path and the metadata.user_api_key JSON path. No pre-existing behavior for non-Bearer, non-sk keys is altered. The tests are mock-only and cover all meaningful input variants.

No files require special attention.

Important Files Changed

Filename Overview
litellm/proxy/spend_tracking/spend_tracking_utils.py Adds _hash_api_key_for_spend_log that strips the Bearer prefix before hashing; applies it to both the api_key column and metadata.user_api_key JSON field. Logic is correct and idempotent for already-hashed tokens.
tests/test_litellm/proxy/spend_tracking/test_spend_tracking_utils.py Adds TestHashApiKeyForSpendLog (6 unit tests) and one integration test for get_logging_payload; covers Bearer/bare/case-insensitive/already-hashed/non-sk variants and the full failure path. No real network calls; mock-only.

Reviews (3): Last reviewed commit: "fix: strip Bearer prefix from non-sk key..." | Re-trigger Greptile

Comment thread litellm/proxy/spend_tracking/spend_tracking_utils.py Outdated
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

@greptileai review

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@yucheng-berri

Copy link
Copy Markdown
Contributor

bugbot run

@cursor cursor Bot left a comment

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.

✅ 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 4e0117c. Configure here.

@yucheng-berri

Copy link
Copy Markdown
Contributor

@greptileai

@yucheng-berri
yucheng-berri merged commit b487a80 into litellm_internal_staging Jul 6, 2026
127 checks passed
@yucheng-berri
yucheng-berri deleted the litellm_fix_plaintext_api_key_in_spend_logs branch July 6, 2026 20:30
mateo-berri added a commit to nitishagar/litellm that referenced this pull request Aug 21, 2026
The spend-log helper no longer treats a 64-hex shape as proof a value was already hashed, so this case has to say where the hash came from. Reconciles the test that came in with BerriAI#31799 against that change.
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.

2 participants