From 477ca5b0d0b3f0a8bea5df58716d7879901e5291 Mon Sep 17 00:00:00 2001 From: Aldrich_CC <109075336+Chen17-sq@users.noreply.github.com> Date: Sat, 16 May 2026 03:47:01 +0800 Subject: [PATCH] fix(proxy): redact SHA-256 token hash from 429 error body (#27884) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../hooks/parallel_request_limiter_v3.py | 15 +++ .../hooks/test_parallel_request_limiter_v3.py | 106 ++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/litellm/proxy/hooks/parallel_request_limiter_v3.py b/litellm/proxy/hooks/parallel_request_limiter_v3.py index 283a3d8d10b..f6d5ddb475c 100644 --- a/litellm/proxy/hooks/parallel_request_limiter_v3.py +++ b/litellm/proxy/hooks/parallel_request_limiter_v3.py @@ -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 = "" + now = self._get_current_time().timestamp() reset_time = now + self.window_size reset_time_formatted = datetime.fromtimestamp(reset_time).strftime( diff --git a/tests/test_litellm/proxy/hooks/test_parallel_request_limiter_v3.py b/tests/test_litellm/proxy/hooks/test_parallel_request_limiter_v3.py index 3e2eb4b02c2..05ff906dd97 100644 --- a/tests/test_litellm/proxy/hooks/test_parallel_request_limiter_v3.py +++ b/tests/test_litellm/proxy/hooks/test_parallel_request_limiter_v3.py @@ -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 ```` + 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: " 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 "" not in detail