mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(hide-secrets): stop redacting benign identifiers (#39879)
* fix(hide-secrets): stop redacting benign identifiers and make redaction deterministic The OpenAI key detector matched `sk-` anywhere inside a word, so `<task-notification>` became `<ta[REDACTED]>`, and the Base64 entropy limit of 3.0 flagged ordinary quoted identifiers such as `"application/json"` and model ids. Redaction also iterated a hash-seeded set, so the same request produced different bytes on different workers and broke prompt caching. - require a standalone `sk-`/`sk_` token with a digit (still catches sk-proj-/sk-ant-) - raise Base64HighEntropyString limit from 3.0 to the detect-secrets default 4.5 - redact overlapping matches longest-first in a stable order Resolves LIT-7049 * fix(hide-secrets): treat separators as key boundaries and defer sk_live_ to the stripe detector The standalone-token boundary also rejected keys glued to a preceding `_`, `-` or percent-encoded delimiter (`openai_sk-…`, `key-sk-…`, `Bearer%20sk-…`), which the old pattern redacted, and `sk_live_…` was counted by both the OpenAI and the Stripe detector. * fix(hide-secrets): keep the openai key scan linear on repeated sk separators The digit requirement was a lookahead, so every `sk` inside a long `[a-zA-Z0-9_-]` run re-scanned the rest of that run looking for a digit. 100 KB of `-sk-` took over 5s in the worker's event loop and the proxy closed the connection without a response. The check now runs once per match in `analyze_string` instead. * chore(hide-secrets): remove redundant performance test comment * fix(hide-secrets): consume complete openai key tokens * chore(hide-secrets): remove redundant fixture comment * chore(hide-secrets): remove redundant test docstrings * fix(hide-secrets): redact whole stripe live keys * style(hide-secrets): wrap secret sorting key
This commit is contained in:
parent
e3b4a82ff9
commit
a0058ed157
3 changed files with 124 additions and 19 deletions
|
|
@ -433,9 +433,9 @@ _default_detect_secrets_config = {
|
|||
"name": "ZendeskSecretKeyDetector",
|
||||
"path": _custom_plugins_path + "/zendesk_secret_key.py",
|
||||
},
|
||||
{"name": "Base64HighEntropyString", "limit": 3.0},
|
||||
{"name": "Base64HighEntropyString", "limit": 4.5},
|
||||
{"name": "HexHighEntropyString", "limit": 3.0},
|
||||
]
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -466,16 +466,19 @@ class _ENTERPRISE_SecretDetection(CustomGuardrail):
|
|||
|
||||
os.remove(temp_file.name)
|
||||
|
||||
detected_secrets = []
|
||||
for file in secrets.files:
|
||||
for found_secret in secrets[file]:
|
||||
if found_secret.secret_value is None:
|
||||
continue
|
||||
detected_secrets.append(
|
||||
{"type": found_secret.type, "value": found_secret.secret_value}
|
||||
)
|
||||
|
||||
return detected_secrets
|
||||
return [
|
||||
{"type": found_secret.type, "value": found_secret.secret_value}
|
||||
for file in sorted(secrets.files)
|
||||
for found_secret in sorted(
|
||||
secrets[file],
|
||||
key=lambda secret: (
|
||||
-len(secret.secret_value or ""),
|
||||
secret.type,
|
||||
secret.secret_value or "",
|
||||
),
|
||||
)
|
||||
if found_secret.secret_value is not None
|
||||
]
|
||||
|
||||
def redact_text(self, text: str, source: str = "message") -> str:
|
||||
"""Replace every detected secret in ``text`` with ``[REDACTED]`` and
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ This plugin searches for OpenAI API Keys.
|
|||
"""
|
||||
|
||||
import re
|
||||
from collections.abc import Generator
|
||||
|
||||
from detect_secrets.plugins.base import RegexBasedDetector
|
||||
|
||||
|
|
@ -16,4 +17,16 @@ class OpenAIApiKeyDetector(RegexBasedDetector):
|
|||
|
||||
@property
|
||||
def denylist(self) -> list[re.Pattern]:
|
||||
return [re.compile(r"""(sk-[a-zA-Z0-9]{5,})""")]
|
||||
return [
|
||||
re.compile(
|
||||
r"((?:(?<![a-zA-Z0-9])|(?<=%[0-9A-Fa-f]{2}))"
|
||||
r"sk[-_]"
|
||||
r"[a-zA-Z0-9_-]{5,}"
|
||||
r"(?![a-zA-Z0-9_-]))"
|
||||
)
|
||||
]
|
||||
|
||||
def analyze_string(self, string: str) -> Generator[str, None, None]:
|
||||
# the digit check lives outside the regex: a lookahead re-scans the token
|
||||
# from every `sk` inside it, which is quadratic on `-sk-sk-sk-...` input
|
||||
yield from (match for match in super().analyze_string(string) if re.search(r"[0-9]", match))
|
||||
|
|
|
|||
|
|
@ -10,6 +10,8 @@ Covers the three defects from the ticket:
|
|||
handling live only on the native path).
|
||||
"""
|
||||
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
from litellm_enterprise.enterprise_callbacks.secret_detection import (
|
||||
|
|
@ -19,12 +21,16 @@ from litellm.caching.caching import DualCache
|
|||
from litellm.proxy._types import UserAPIKeyAuth
|
||||
|
||||
AWS_KEY = "AKIAIOSFODNN7EXAMPLE"
|
||||
OPENAI_KEY = "sk-test-abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGH"
|
||||
SHORT_OPENAI_KEY = "sk-12345"
|
||||
UNICODE_DIGIT_SUFFIX = "sk-notification٣"
|
||||
STRIPE_LIVE_KEY = f"sk_live_{'1234567890' * 3}"
|
||||
URL_ENCODED_KEY = "Bearer%20sk-Ab3dEf6Gh7Ij8Kl9Mn0Pq2Rs3Tu4Vw5X"
|
||||
AWS_KEYS = [f"AKIAIOSFODNN7EXAMPL{suffix}" for suffix in "FEDCBA"]
|
||||
|
||||
|
||||
def _guardrail() -> _ENTERPRISE_SecretDetection:
|
||||
return _ENTERPRISE_SecretDetection(
|
||||
guardrail_name="hide-secrets", event_hook="pre_call", default_on=True
|
||||
)
|
||||
return _ENTERPRISE_SecretDetection(guardrail_name="hide-secrets", event_hook="pre_call", default_on=True)
|
||||
|
||||
|
||||
def _recorded(request_data: dict) -> dict:
|
||||
|
|
@ -33,6 +39,91 @@ def _recorded(request_data: dict) -> dict:
|
|||
return entries[0]
|
||||
|
||||
|
||||
def test_scan_message_preserves_benign_identifiers_and_xml_tags():
|
||||
guardrail = _guardrail()
|
||||
content = "<task-notification> model: claude-sonnet-4-5-20250929 </task-notification>"
|
||||
|
||||
assert guardrail.scan_message_for_secrets(content) == []
|
||||
assert guardrail.redact_text(content) == content
|
||||
assert guardrail.redact_text("result = compute(x) </task-notification>") == (
|
||||
"result = compute(x) </task-notification>"
|
||||
)
|
||||
|
||||
|
||||
def test_scan_message_preserves_quoted_benign_identifiers():
|
||||
guardrail = _guardrail()
|
||||
content = '{"content-type": "application/json", "model": "claude-sonnet-4-5-20250929"}'
|
||||
|
||||
assert guardrail.scan_message_for_secrets(content) == []
|
||||
assert guardrail.redact_text(content) == content
|
||||
|
||||
|
||||
def test_scan_message_redacts_every_openai_key_occurrence():
|
||||
guardrail = _guardrail()
|
||||
content = f"first {OPENAI_KEY}, second {OPENAI_KEY}"
|
||||
|
||||
assert guardrail.redact_text(content) == "first [REDACTED], second [REDACTED]"
|
||||
|
||||
|
||||
def test_scan_message_redacts_short_numeric_openai_like_values():
|
||||
guardrail = _guardrail()
|
||||
|
||||
assert guardrail.redact_text(f"value {SHORT_OPENAI_KEY}") == "value [REDACTED]"
|
||||
|
||||
|
||||
def test_scan_message_requires_ascii_digits_for_openai_like_values():
|
||||
guardrail = _guardrail()
|
||||
|
||||
assert guardrail.scan_message_for_secrets(UNICODE_DIGIT_SUFFIX) == []
|
||||
assert guardrail.redact_text(UNICODE_DIGIT_SUFFIX) == UNICODE_DIGIT_SUFFIX
|
||||
|
||||
|
||||
def test_scan_message_redacts_openai_key_after_separator():
|
||||
guardrail = _guardrail()
|
||||
|
||||
assert guardrail.redact_text(f"openai_{OPENAI_KEY} key-{OPENAI_KEY}") == (
|
||||
"openai_[REDACTED] key-[REDACTED]"
|
||||
)
|
||||
assert guardrail.redact_text(URL_ENCODED_KEY) == "Bearer%20[REDACTED]"
|
||||
|
||||
|
||||
def test_scan_message_does_not_stop_openai_key_at_token_characters():
|
||||
guardrail = _guardrail()
|
||||
|
||||
assert guardrail.redact_text("key sk-proj-abcde12345/extra") == "key [REDACTED]/extra"
|
||||
|
||||
|
||||
def test_scan_message_stays_linear_on_repeated_sk_separators():
|
||||
guardrail = _guardrail()
|
||||
content = "-sk-" * 25_000
|
||||
|
||||
started = time.perf_counter()
|
||||
assert guardrail.scan_message_for_secrets(content) == []
|
||||
assert time.perf_counter() - started < 2.0
|
||||
|
||||
|
||||
def test_scan_message_redacts_whole_stripe_live_key():
|
||||
guardrail = _guardrail()
|
||||
|
||||
assert guardrail.redact_text(f"stripe {STRIPE_LIVE_KEY} end") == "stripe [REDACTED] end"
|
||||
|
||||
|
||||
def test_scan_message_returns_matches_in_stable_order():
|
||||
guardrail = _guardrail()
|
||||
detected = guardrail.scan_message_for_secrets(" ".join(AWS_KEYS))
|
||||
|
||||
assert [secret["value"] for secret in detected] == sorted(AWS_KEYS)
|
||||
|
||||
|
||||
def test_scan_message_replaces_longest_overlapping_match_first():
|
||||
guardrail = _guardrail()
|
||||
content = f'token = "{OPENAI_KEY}/extra"'
|
||||
|
||||
detected = guardrail.scan_message_for_secrets(content)
|
||||
assert [secret["value"] for secret in detected] == [f"{OPENAI_KEY}/extra", OPENAI_KEY]
|
||||
assert guardrail.redact_text(content) == 'token = "[REDACTED]"'
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_apply_guardrail_redacts_secrets():
|
||||
"""Playground path: the returned texts must carry [REDACTED], not the secret."""
|
||||
|
|
@ -199,9 +290,7 @@ async def test_apply_guardrail_without_texts_records_nothing():
|
|||
"messages": [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "image_url", "image_url": {"url": "https://x/y.png"}}
|
||||
],
|
||||
"content": [{"type": "image_url", "image_url": {"url": "https://x/y.png"}}],
|
||||
}
|
||||
],
|
||||
"metadata": {},
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue