From 45b4dca145291484b138e8bbbd0cc0691d57c65e Mon Sep 17 00:00:00 2001 From: Srivatsa Kamballa Date: Mon, 22 Jun 2026 07:02:31 -0500 Subject: [PATCH] fix(sensitive_data_masker): fully mask secrets at or below the reveal threshold (#30764) * fix(sensitive_data_masker): fully mask secrets at or below the reveal threshold _mask_value did partial reveal by showing the first visible_prefix and last visible_suffix characters, but for a value whose length was at or below visible_prefix + visible_suffix (8 by default) it returned the value verbatim. A value of exactly 8 chars fell through the length guard and computed masked_length == 0, reconstructing the original string with no mask characters; anything shorter hit the early return. Either way short credentials were emitted in plaintext. mask_dict routes real secrets through this path, so an 8-char-or-shorter redis password, api key, or token could be written to logs and the UI unmasked. The sibling helper mask_sensitive_keys already guards this case; _mask_value now does the same by fully masking any value at or below the threshold. * fix(sensitive_data_masker): add mask_short_values opt-out for truncation callers Fully masking short values is the right default for secret masking, but CooldownCache reuses the masker purely to truncate exception messages to the first 50 characters, and it relies on short messages being returned readable. Masking those blanked out short exception text and broke its tests. Add a mask_short_values flag (default True, secure) and have CooldownCache pass False so it keeps the truncation behavior, while every secret-masking caller still gets short values fully masked. * fix(mcp_debug): opt out of short-value masking to keep diagnostic token preview MCPDebug uses the masker to preview auth tokens in debug headers and documents that values of 10 chars or fewer are shown unchanged so token types stay distinguishable. Pass mask_short_values=False so that diagnostic behavior is preserved while secret maskers keep masking short values. * fix(mcp_debug): mask short auth values in debug headers instead of echoing them Earlier this masker opted out of short-value masking to keep a token preview, but that echoes short authorization and token values verbatim in debug response headers, which is the same leak this change is meant to close. Auth material should never be emitted in full, so mask short values here too; the first/last character preview still applies to longer tokens. Only CooldownCache keeps the opt-out, since it truncates exception text rather than masking secrets. * test(mcp_debug): assert masked short value preserves length --- .../sensitive_data_masker.py | 12 ++++-- litellm/router_utils/cooldown_cache.py | 1 + .../test_sensitive_data_masker.py | 43 +++++++++++++++++++ .../mcp_server/test_mcp_debug.py | 10 +++-- 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/litellm/litellm_core_utils/sensitive_data_masker.py b/litellm/litellm_core_utils/sensitive_data_masker.py index 4928dd08386..b14e12de7cd 100644 --- a/litellm/litellm_core_utils/sensitive_data_masker.py +++ b/litellm/litellm_core_utils/sensitive_data_masker.py @@ -12,6 +12,7 @@ class SensitiveDataMasker: visible_prefix: int = 4, visible_suffix: int = 4, mask_char: str = "*", + mask_short_values: bool = True, ): self.sensitive_patterns = sensitive_patterns or { "password", @@ -38,12 +39,17 @@ class SensitiveDataMasker: self.visible_prefix = visible_prefix self.visible_suffix = visible_suffix self.mask_char = mask_char + self.mask_short_values = mask_short_values def _mask_value(self, value: str) -> str: - if not value or len(str(value)) < (self.visible_prefix + self.visible_suffix): - return value - value_str = str(value) + if not value_str: + return value + if len(value_str) <= (self.visible_prefix + self.visible_suffix): + return ( + self.mask_char * len(value_str) if self.mask_short_values else value_str + ) + masked_length = len(value_str) - (self.visible_prefix + self.visible_suffix) # Handle the case where visible_suffix is 0 to avoid showing the entire string diff --git a/litellm/router_utils/cooldown_cache.py b/litellm/router_utils/cooldown_cache.py index b210ea44596..dcfa44381c1 100644 --- a/litellm/router_utils/cooldown_cache.py +++ b/litellm/router_utils/cooldown_cache.py @@ -38,6 +38,7 @@ class CooldownCache: visible_prefix=50, # Show first 50 characters visible_suffix=0, # Show last 0 characters mask_char="*", # Use * for masking + mask_short_values=False, # Truncate long messages only; keep short ones readable ) def _common_add_cooldown_logic( diff --git a/tests/test_litellm/litellm_core_utils/test_sensitive_data_masker.py b/tests/test_litellm/litellm_core_utils/test_sensitive_data_masker.py index 6808c4821c1..7239636fd48 100644 --- a/tests/test_litellm/litellm_core_utils/test_sensitive_data_masker.py +++ b/tests/test_litellm/litellm_core_utils/test_sensitive_data_masker.py @@ -126,6 +126,49 @@ def test_lists_with_sensitive_keys_are_masked(): assert masked["tags"] == ["prod", "test"] +def test_short_secrets_are_fully_masked(): + """ + Regression test: secrets at or below the reveal threshold (visible_prefix + + visible_suffix, 8 by default) were returned verbatim instead of masked. + An exactly-8-char value hit masked_length == 0 and round-tripped unchanged; + anything shorter hit the early return. Both leaked short credentials (e.g. an + 8-char redis password) in plaintext through mask_dict. + """ + masker = SensitiveDataMasker() + + # Boundary: exactly 8 chars previously returned verbatim. + assert masker._mask_value("abcd1234") == "********" + # Below threshold previously hit the early return and leaked verbatim. + assert masker._mask_value("sk-12") == "*****" + # Values above the threshold must still partially reveal, not over-mask. + assert masker._mask_value("abcd12345") == "abcd*2345" + + masked = masker.mask_dict({"redis_password": "pass1234", "api_key": "sk-7a"}) + assert masked["redis_password"] == "********" + assert masked["api_key"] == "*****" + + +def test_mask_short_values_false_keeps_short_values_readable(): + """ + mask_short_values=False opts out of full masking so short values are returned + as-is. This preserves the truncation use (e.g. CooldownCache shows the first 50 + chars of an exception and only masks longer tails), while longer values are still + partially masked. + """ + masker = SensitiveDataMasker( + visible_prefix=50, visible_suffix=0, mask_short_values=False + ) + + short = "Test exception for structure validation" + assert masker._mask_value(short) == short + + long_value = "x" * 60 + masked = masker._mask_value(long_value) + assert masked.startswith("x" * 50) + assert masked.endswith("*" * 10) + assert len(masked) == 60 + + def test_cost_per_token_fields_not_masked(): """ Regression test: cost fields like input_cost_per_token contain "token" in their name diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_debug.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_debug.py index 468bd946ae9..d299239f68e 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_debug.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_debug.py @@ -41,9 +41,13 @@ class TestMask: def test_empty_returns_none_label(self): assert MCPDebug._mask("") == "(none)" - def test_short_value_unchanged(self): - # visible_prefix=6 + visible_suffix=4 = 10, so <= 10 chars unchanged - assert MCPDebug._mask("sk-1234") == "sk-1234" + def test_short_value_masked(self): + # Short auth values must not be echoed verbatim in debug headers, even though + # visible_prefix + visible_suffix would otherwise reveal the whole value. + masked = MCPDebug._mask("sk-1234") + assert "sk-1234" not in masked + assert set(masked) == {"*"} + assert len(masked) == len("sk-1234") def test_long_value_masked(self): result = MCPDebug._mask("Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9")