mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-19 00:01:29 +00:00
fix(guardrails): normalize role casing in Lakera v2 masking scope, log skipped guardrails louder
Greptile finding: the masking scope helper compared roles case-sensitively while filter_messages_by_skip_flags (used to build what's actually sent to Lakera) normalizes casing, so an uppercase-cased "System"/"TOOL" role survived the scope filter but was excluded from the inspected list. The resulting length mismatch raised inside the strict positional zip, turning a maskable PII-only violation into an unhandled request failure. Lowercase the role comparison to match. Also, per veria-ai's finding that a skipped invalid guardrail now fails open: log it at error level with an explicit note that the proxy is starting without that guardrail, so it's not mistaken for routine info.
This commit is contained in:
parent
f1008f2aa7
commit
0716c55bfe
3 changed files with 36 additions and 5 deletions
|
|
@ -117,7 +117,11 @@ def _pre_masking_scope_indices(
|
|||
Preserved in original order, so it lines up positionally with the
|
||||
``messages_for_lakera`` list _build_lakera_inspection_messages/skip-filtering
|
||||
produces from the same input: both apply the identical "has text" and
|
||||
"not skipped by role" predicates over the same original sequence."""
|
||||
"not skipped by role" predicates over the same original sequence. Role
|
||||
comparison is lowercased to match filter_messages_by_skip_flags's own
|
||||
normalization (via its _message_role helper) -- an uppercase-cased
|
||||
"System"/"TOOL" role must be excluded by both or the two lists disagree
|
||||
on length and the caller's strict positional zip raises."""
|
||||
skip_system: Final = effective_skip_system_message_for_guardrail(guardrail)
|
||||
skip_tool: Final = effective_skip_tool_message_for_guardrail(guardrail)
|
||||
return tuple(
|
||||
|
|
@ -126,8 +130,8 @@ def _pre_masking_scope_indices(
|
|||
if isinstance(message, dict)
|
||||
and isinstance(message.get("content"), str)
|
||||
and message["content"]
|
||||
and not (skip_system and message.get("role") == "system")
|
||||
and not (skip_tool and message.get("role") == "tool")
|
||||
and not (skip_system and str(message.get("role") or "").lower() == "system")
|
||||
and not (skip_tool and str(message.get("role") or "").lower() == "tool")
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -34,8 +34,8 @@ def init_guardrails_v2(
|
|||
source="config",
|
||||
)
|
||||
except (ValueError, TypeError) as init_error:
|
||||
verbose_proxy_logger.warning(
|
||||
"Skipping guardrail '%s': invalid configuration: %s",
|
||||
verbose_proxy_logger.error(
|
||||
"Skipping guardrail '%s': invalid configuration, proxy is starting WITHOUT this guardrail: %s",
|
||||
guardrail.get("guardrail_name"),
|
||||
init_error,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -508,6 +508,33 @@ class TestPiiMaskingSafetyGuard:
|
|||
assert result["messages"][0] == SYSTEM_MSG
|
||||
assert "[MASKED" in result["messages"][1]["content"]
|
||||
|
||||
async def test_pii_only_violation_with_uppercase_skipped_role_masks_without_raising(self):
|
||||
"""
|
||||
Greptile finding on BerriAI/litellm#34940: filter_messages_by_skip_flags
|
||||
normalizes role casing (via _message_role's .lower()), but the scope-index
|
||||
helper compared roles case-sensitively. A "System"-cased role survived the
|
||||
scope-index filter while the shared filter correctly excluded it from what's
|
||||
sent to Lakera, so scope_indices and the masked results came back different
|
||||
lengths and the strict positional zip raised, turning a maskable PII-only
|
||||
violation into an unhandled request failure instead of a masked response."""
|
||||
guardrail = LakeraAIGuardrail(api_key="test_key", on_flagged="block", skip_system_message_in_guardrail=True)
|
||||
uppercase_system_msg = {"role": "System", "content": "be nice"}
|
||||
data = {
|
||||
"messages": [uppercase_system_msg.copy(), USER_MSG.copy()],
|
||||
"model": "gpt-3.5-turbo",
|
||||
"metadata": {},
|
||||
}
|
||||
with patch.object(guardrail, "call_v2_guard", new_callable=AsyncMock) as mock_call:
|
||||
mock_call.return_value = (PII_ONLY_LAKERA_RESPONSE, {})
|
||||
result = await guardrail.async_pre_call_hook(
|
||||
user_api_key_dict=UserAPIKeyAuth(api_key="test_key"),
|
||||
cache=MagicMock(),
|
||||
data=data,
|
||||
call_type="completion",
|
||||
)
|
||||
assert result["messages"][0] == uppercase_system_msg
|
||||
assert "[MASKED" in result["messages"][1]["content"]
|
||||
|
||||
async def test_pii_only_violation_with_empty_text_message_masks_and_leaves_it_untouched(self):
|
||||
"""build_inspection_messages drops empty-text messages before the skip filter
|
||||
ever sees them. The scope-index merge must leave that untouched empty message
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue