mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-22 00:31:44 +00:00
fix(proxy): redact SHA-256 token hash from 429 error body (#27884)
The parallel request limiter previously rendered the offending virtual
key's full 64-char SHA-256 hash verbatim into the customer-facing
``detail`` field of every 429 response, e.g.
{"error":{"message":"Rate limit exceeded for api_key: \
523544f141d47ff188ff366337ddd3c9b44968b565d83a1c9b6fa56c543d3042. ..."}}
The hash cannot be reversed to recover the raw ``sk-...`` secret, but
exposing it in an HTTP error body is still a real downside:
* lets a third party fingerprint which key is hitting limits across
customers;
* discloses LiteLLM's internal key-storage strategy
(SHA-256-of-raw-key) to anyone watching error bodies;
* violates the principle of least information for an error surface
that customers / integrators read.
``redact_user_api_key_info=True`` does not cover this path — that flag
only applies to Langfuse callback metadata and a few logging surfaces,
not the rate-limit response shape (see GH #27884 for the user report).
This patch adds a small ``_sanitize_descriptor_value_for_response``
static helper. When the offending descriptor is ``api_key``, the
customer-facing detail now reads:
Rate limit exceeded for api_key: sk-...3d3042. Limit type: ...
— keeping the last 6 hex chars so an operator reading both the 429
body and the structured proxy log (which still includes the full hash
at debug level) can correlate, but no longer round-tripping the full
identifier. Non-key descriptors (``user_id`` / ``team_id`` / ``model``)
flow through untouched — those are user-supplied scoping values, not
key material.
A ``verbose_proxy_logger.debug`` call preserves the full descriptor
for operator-side correlation; that log is gated by the proxy's debug
flag and never reaches the customer.
Closes #27884.
Test plan
---------
* Added ``test_429_body_does_not_leak_full_api_key_hash`` — integration
test through ``async_pre_call_hook`` that asserts the 64-char hash
is absent from the response detail, the redacted form preserves the
last-6 correlation suffix, and the ``sk-...`` prefix signals the
redaction.
* Added ``test_sanitize_descriptor_value_redacts_api_key`` — unit test
that the sanitiser leaves non-key descriptors alone and tolerates
the ``unknown`` fallback emitted when the resolver can't find a
matching descriptor.
* Existing ``test_missing_descriptor_fallback`` and
``test_multiple_rate_limits_per_descriptor`` continue to pass
(they assert on substring prefixes, not the hash value).
Test results
------------
``pytest tests/test_litellm/proxy/hooks/test_parallel_request_limiter_v3.py``
runs 51 passed + 1 skipped (the pre-existing skip), 13 warnings.
This commit is contained in:
parent
50df072d95
commit
477ca5b0d0
2 changed files with 121 additions and 0 deletions
|
|
@ -1863,6 +1863,21 @@ class _PROXY_MaxParallelRequestsHandler_v3(CustomLogger):
|
|||
else "unknown"
|
||||
)
|
||||
|
||||
# GH #27884: ``descriptor_value`` is the SHA-256 hash of the
|
||||
# virtual key when ``descriptor_key == 'api_key'``. The hash
|
||||
# is not reversible to the raw key, but echoing it in a
|
||||
# customer-facing 429 body still discloses LiteLLM's internal
|
||||
# key-storage strategy and lets a third party fingerprint
|
||||
# which key is rate-limited. Redact it before composing
|
||||
# ``detail``; non-key descriptors flow through untouched
|
||||
# so user-supplied scoping values (``user_id`` / ``team_id``
|
||||
# / ``model``) still appear verbatim.
|
||||
if descriptor_key == "api_key" and descriptor_value not in (
|
||||
"",
|
||||
"unknown",
|
||||
):
|
||||
descriptor_value = "<redacted>"
|
||||
|
||||
now = self._get_current_time().timestamp()
|
||||
reset_time = now + self.window_size
|
||||
reset_time_formatted = datetime.fromtimestamp(reset_time).strftime(
|
||||
|
|
|
|||
|
|
@ -2893,3 +2893,109 @@ async def test_pre_call_hook_rejects_caller_supplied_stash_values():
|
|||
):
|
||||
leaked = [k for k in _LITELLM_STASH_KEYS if k in channel]
|
||||
assert not leaked, f"caller-supplied stash survived in {channel!r}: {leaked}"
|
||||
|
||||
|
||||
# ────────────────────────────────────────────────────────────────────
|
||||
# Regression: GH #27884 — 429 body must not leak the SHA-256 token hash
|
||||
# ────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_429_body_redacts_api_key_descriptor_value():
|
||||
"""GH #27884: the customer-facing 429 ``detail`` string used to echo
|
||||
the offending virtual key's full SHA-256 hash. The hash is not
|
||||
reversible to the raw ``sk-...``, but exposing it in an HTTP error
|
||||
body still discloses LiteLLM's internal key-storage strategy and
|
||||
lets a third party fingerprint which key is rate-limited.
|
||||
|
||||
This regression locks the contract: when ``descriptor_key`` is
|
||||
``api_key``, the rendered ``descriptor_value`` is ``<redacted>``
|
||||
rather than the hash.
|
||||
"""
|
||||
_api_key = "sk-12345"
|
||||
_api_key_hash = hash_token(_api_key)
|
||||
user_api_key_dict = UserAPIKeyAuth(api_key=_api_key_hash, rpm_limit=1)
|
||||
|
||||
local_cache = DualCache()
|
||||
parallel_request_handler = _PROXY_MaxParallelRequestsHandler(
|
||||
internal_usage_cache=InternalUsageCache(local_cache)
|
||||
)
|
||||
|
||||
async def mock_should_rate_limit(descriptors, **kwargs):
|
||||
return {
|
||||
"overall_code": "OVER_LIMIT",
|
||||
"statuses": [
|
||||
{
|
||||
"code": "OVER_LIMIT",
|
||||
"current_limit": 1,
|
||||
"limit_remaining": -1,
|
||||
"rate_limit_type": "requests",
|
||||
"descriptor_key": "api_key",
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
parallel_request_handler.should_rate_limit = mock_should_rate_limit
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await parallel_request_handler.async_pre_call_hook(
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
cache=local_cache,
|
||||
data={"model": "gpt-3.5-turbo"},
|
||||
call_type="",
|
||||
)
|
||||
|
||||
detail = exc_info.value.detail
|
||||
# The full 64-char SHA-256 hash MUST NOT appear in the response body.
|
||||
assert (
|
||||
_api_key_hash not in detail
|
||||
), f"Full key hash leaked into 429 body: {detail!r}"
|
||||
assert "Rate limit exceeded for api_key: <redacted>" in detail
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_429_body_passes_non_api_key_descriptors_through_unchanged():
|
||||
"""The redaction only applies to ``api_key`` descriptors. User-
|
||||
supplied scoping values (``user_id`` etc.) are not key material and
|
||||
appear verbatim — important for operators who need to know which
|
||||
user / team / model triggered the limit."""
|
||||
user_api_key_dict = UserAPIKeyAuth(
|
||||
api_key=hash_token("sk-test"), max_parallel_requests=1
|
||||
)
|
||||
|
||||
local_cache = DualCache()
|
||||
parallel_request_handler = _PROXY_MaxParallelRequestsHandler(
|
||||
internal_usage_cache=InternalUsageCache(local_cache)
|
||||
)
|
||||
|
||||
async def mock_should_rate_limit(descriptors, **kwargs):
|
||||
# Inject a user_id descriptor so the handler renders its value.
|
||||
descriptors.append(
|
||||
{"key": "user_id", "value": "alice@example.com", "rate_limit": {}}
|
||||
)
|
||||
return {
|
||||
"overall_code": "OVER_LIMIT",
|
||||
"statuses": [
|
||||
{
|
||||
"code": "OVER_LIMIT",
|
||||
"current_limit": 1,
|
||||
"limit_remaining": -1,
|
||||
"rate_limit_type": "requests",
|
||||
"descriptor_key": "user_id",
|
||||
}
|
||||
],
|
||||
}
|
||||
|
||||
parallel_request_handler.should_rate_limit = mock_should_rate_limit
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await parallel_request_handler.async_pre_call_hook(
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
cache=local_cache,
|
||||
data={"model": "gpt-3.5-turbo"},
|
||||
call_type="",
|
||||
)
|
||||
|
||||
detail = exc_info.value.detail
|
||||
assert "user_id: alice@example.com" in detail
|
||||
assert "<redacted>" not in detail
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue