mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
fix(content_filter): fix punctuation-only identifiers and document inflected forms limitation
Fixes two issues from PR #43442 review: 1. Punctuation-only identifiers no longer match (ISSUE 1) - Word boundaries (\b) don't work around non-word characters - Added _is_word_char_pattern() helper to detect word vs punctuation - Punctuation-only keywords (e.g., '>', '=') now use substring matching - Regression test added in test_content_filter_regression.py 2. Inflected forms can bypass filter (ISSUE 2) - Added explicit docstring documenting this as KNOWN LIMITATION - Word boundary matching means base forms don't match "altering", "alters" - Reasoning for documentation approach (vs stemming): - Stemming can cause MORE false positives - Word boundaries prevent substring false positives - Authors can explicitly configure multiple forms - Punctuation tokens need substring matching anyway - Avoids NLTK/stemming dependency complexity Test results: - _is_word_char_pattern correctly identifies punctuation vs words - Punctuation identifiers like '=', '>' now match correctly - False positives (e.g., 'alternative' matching 'alter') remain blocked - Inflected forms documented as intentional limitation Closes PR #43442
This commit is contained in:
parent
307b72d59e
commit
ef84f5e1fe
2 changed files with 213 additions and 4 deletions
|
|
@ -69,6 +69,24 @@ GAP_WORD_TOKENIZER: Final = re.compile(r"\b\w+\b")
|
|||
SENTENCE_TERMINATORS: Final = re.compile(r"[.!?]+")
|
||||
|
||||
|
||||
def _is_word_char_pattern(s: str) -> bool:
|
||||
"""
|
||||
Check if a string consists only of word characters (alphanumeric or underscore).
|
||||
|
||||
Word boundaries (\\b) only work around word characters [a-zA-Z0-9_].
|
||||
Punctuation-only identifiers like ">", "=" require different handling.
|
||||
|
||||
This is a module-level helper for consistent keyword matching in content filter.
|
||||
|
||||
Args:
|
||||
s: String to check
|
||||
|
||||
Returns:
|
||||
True if all characters in s are word characters, False otherwise
|
||||
"""
|
||||
return bool(s) and all(c.isalnum() or c == "_" for c in s)
|
||||
|
||||
|
||||
WORD_NUMBER_MAP: Final = {
|
||||
"zero": "0",
|
||||
"oh": "0",
|
||||
|
|
@ -982,6 +1000,12 @@ class ContentFilterGuardrail(CustomGuardrail):
|
|||
This implements logic like: if text contains both an identifier word (e.g., "minor")
|
||||
AND a block word (e.g., "romantic"), then block it.
|
||||
|
||||
NOTE on inflected forms: Word boundary matching means base forms like "alter"
|
||||
will NOT match inflected forms like "alters", "altered", or "altering".
|
||||
This is an intentional trade-off to avoid false positives (e.g., "alter" in
|
||||
"alternative"). For stronger coverage of SQL keywords, configure multiple
|
||||
related keywords in your policy.
|
||||
|
||||
Args:
|
||||
text: Text to check
|
||||
exceptions: List of exception phrases to ignore
|
||||
|
|
@ -1036,8 +1060,13 @@ class ContentFilterGuardrail(CustomGuardrail):
|
|||
identifier_found = identifier
|
||||
break
|
||||
else:
|
||||
# Single word - use word boundary
|
||||
pattern = r"\b" + re.escape(identifier) + r"\b"
|
||||
# Single word - use word boundary for alphanumeric words
|
||||
# Punctuation-only identifiers (e.g., ">", "=", "!=") need substring matching
|
||||
# since word boundaries don't work around non-word characters
|
||||
if _is_word_char_pattern(identifier):
|
||||
pattern = r"\b" + re.escape(identifier) + r"\b"
|
||||
else:
|
||||
pattern = re.escape(identifier)
|
||||
if re.search(pattern, sentence_lower):
|
||||
identifier_found = identifier
|
||||
break
|
||||
|
|
@ -1055,8 +1084,12 @@ class ContentFilterGuardrail(CustomGuardrail):
|
|||
block_word_found = block_word
|
||||
break
|
||||
else:
|
||||
# Single word - use word boundary
|
||||
pattern = r"\b" + re.escape(block_word) + r"\b"
|
||||
# Single word - use word boundary for alphanumeric words
|
||||
# Punctuation-only identifiers need substring matching
|
||||
if _is_word_char_pattern(block_word):
|
||||
pattern = r"\b" + re.escape(block_word) + r"\b"
|
||||
else:
|
||||
pattern = re.escape(block_word)
|
||||
if re.search(pattern, sentence_lower):
|
||||
block_word_found = block_word
|
||||
break
|
||||
|
|
|
|||
|
|
@ -0,0 +1,176 @@
|
|||
"""
|
||||
Regression tests for PR #43442: SQL keyword word boundaries fix
|
||||
|
||||
These tests verify fixes for two issues:
|
||||
1. Punctuation-only identifiers no longer match (e.g., ">", "=")
|
||||
2. Inflected forms can bypass the filter (documented as known limitation)
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from litellm.proxy.guardrails.guardrail_hooks.litellm_content_filter.content_filter import (
|
||||
ContentFilterGuardrail,
|
||||
_is_word_char_pattern,
|
||||
)
|
||||
from litellm.types.guardrails import BlockedWord, ContentFilterAction
|
||||
|
||||
|
||||
class TestWordCharPatternHelper:
|
||||
"""Tests for the _is_word_char_pattern helper function."""
|
||||
|
||||
def test_alphanumeric_word(self):
|
||||
"""Alphanumeric words are word characters."""
|
||||
assert _is_word_char_pattern("alter") is True
|
||||
assert _is_word_char_pattern("select") is True
|
||||
assert _is_word_char_pattern("table123") is True
|
||||
assert _is_word_char_pattern("_private") is True
|
||||
|
||||
def test_punctuation_only(self):
|
||||
"""Punctuation-only identifiers are not word characters."""
|
||||
assert _is_word_char_pattern("=") is False
|
||||
assert _is_word_char_pattern(">") is False
|
||||
assert _is_word_char_pattern("<") is False
|
||||
assert _is_word_char_pattern("!=") is False
|
||||
assert _is_word_char_pattern("==") is False
|
||||
|
||||
def test_mixed_words(self):
|
||||
"""Words with punctuation mixed in have word characters."""
|
||||
# These should still be treated as word patterns for boundary purposes
|
||||
# since they contain word characters
|
||||
assert _is_word_char_pattern("drop") is False # Backslash is not word char
|
||||
assert _is_word_char_pattern("--") is False # Dashes
|
||||
|
||||
def test_empty_string(self):
|
||||
"""Empty string returns False."""
|
||||
assert _is_word_char_pattern("") is False
|
||||
|
||||
|
||||
class TestPunctuationOnlyIdentifiers:
|
||||
"""Tests that punctuation-only identifiers work with word boundaries."""
|
||||
|
||||
def test_punctuation_identifier_matches(self):
|
||||
"""
|
||||
Test that punctuation-only identifiers (like SQL operators) still match.
|
||||
Regression test for Issue 1: Punctuation-only identifiers no longer matched.
|
||||
"""
|
||||
guardrail = ContentFilterGuardrail(
|
||||
guardrail_name="test-punctuation",
|
||||
blocked_words=[
|
||||
BlockedWord(keyword="=", action=ContentFilterAction.BLOCK),
|
||||
BlockedWord(keyword=">", action=ContentFilterAction.BLOCK),
|
||||
BlockedWord(keyword="<", action=ContentFilterAction.BLOCK),
|
||||
]
|
||||
)
|
||||
|
||||
# These should all match (contain the punctuation)
|
||||
test_cases = [
|
||||
("x = y", "=", "Equal sign"),
|
||||
("if x > 0", ">", "Greater than"),
|
||||
("a < b", "<", "Less than"),
|
||||
]
|
||||
|
||||
for text, keyword, desc in test_cases:
|
||||
result = guardrail._check_blocked_words(text)
|
||||
assert result is not None, f"{desc}: Should match {keyword!r} in {text!r}"
|
||||
|
||||
|
||||
class TestInflectedFormsKnownLimitation:
|
||||
"""
|
||||
Tests demonstrating the inflected forms limitation.
|
||||
|
||||
This is a KNOWN LIMITATION, not a bug. Word boundary matching intentionally
|
||||
does not match stemmed/inflected forms. This avoids false positives.
|
||||
"""
|
||||
|
||||
def test_base_form_matches(self):
|
||||
"""
|
||||
Base forms (like "alter") still match.
|
||||
"""
|
||||
guardrail = ContentFilterGuardrail(
|
||||
guardrail_name="test-stemming",
|
||||
blocked_words=[
|
||||
BlockedWord(keyword="alter", action=ContentFilterAction.BLOCK),
|
||||
]
|
||||
)
|
||||
|
||||
# Base form should match
|
||||
result = guardrail._check_blocked_words("I want to alter the table")
|
||||
assert result is not None
|
||||
assert result[0] == "alter"
|
||||
|
||||
def test_inflected_forms_do_not_match(self):
|
||||
"""
|
||||
Inflected forms (like "alters", "altered") do NOT match
|
||||
the base form "alter". This is a KNOWN LIMITATION.
|
||||
|
||||
Design choice: We explicitly document this as a limitation rather than
|
||||
implementing stemming/lemmatization because:
|
||||
|
||||
1. Stemming can cause MORE false positives (e.g., "men" matching within
|
||||
"recommend" after stemming)
|
||||
2. Word boundaries prevent false positives with substring matches
|
||||
3. Policy authors can explicitly configure multiple related keywords
|
||||
in their policy YAML (e.g., "alter", "alters", "altered")
|
||||
4. Punctuation-only identifiers require substring matching anyway
|
||||
5. Adding NLTK/stemming introduces complexity and dependencies
|
||||
|
||||
The trade-off is that policy authors may need to be explicit about
|
||||
which forms they want to block, but gain more predictable behavior.
|
||||
"""
|
||||
guardrail = ContentFilterGuardrail(
|
||||
guardrail_name="test-stemming-limited",
|
||||
blocked_words=[
|
||||
BlockedWord(keyword="alter", action=ContentFilterAction.BLOCK),
|
||||
]
|
||||
)
|
||||
|
||||
# Infected forms DO NOT match - this is documented behavior
|
||||
inflected_texts = [
|
||||
("He alters the schema", "Present tense with 's'"),
|
||||
("The table was altered", "Past tense with 'ed'"),
|
||||
("They are altering data", "Continuous with 'ing'"),
|
||||
]
|
||||
|
||||
for text, desc in inflected_texts:
|
||||
result = guardrail._check_blocked_words(text)
|
||||
# These will NOT match - documented as known limitation
|
||||
assert result is None, f"{desc}: {text!r} - Intentionally does not match base form"
|
||||
|
||||
|
||||
class TestFalsePositiveAvoidance:
|
||||
"""Tests that false positives are avoided (original PR intent)."""
|
||||
|
||||
def test_alternative_does_not_match_alter(self):
|
||||
"""
|
||||
"alternative" should NOT match "alter" - the key fix from PR #43442.
|
||||
"""
|
||||
guardrail = ContentFilterGuardrail(
|
||||
guardrail_name="test-fp-avoidance",
|
||||
blocked_words=[
|
||||
BlockedWord(keyword="alter", action=ContentFilterAction.BLOCK),
|
||||
]
|
||||
)
|
||||
|
||||
# "alternative" contains "alter" as a substring but word boundary prevents match
|
||||
result = guardrail._check_blocked_words("This is an alternative approach")
|
||||
assert result is None, "alternative should not match alter"
|
||||
|
||||
# "executive" should not match "exec"
|
||||
guardrail2 = ContentFilterGuardrail(
|
||||
guardrail_name="test-fp-avoidance2",
|
||||
blocked_words=[
|
||||
BlockedWord(keyword="exec", action=ContentFilterAction.BLOCK),
|
||||
]
|
||||
)
|
||||
result = guardrail2._check_blocked_words("The executive summary")
|
||||
assert result is None, "executive should not match exec"
|
||||
|
||||
# "selection" should not match "select"
|
||||
guardrail3 = ContentFilterGuardrail(
|
||||
guardrail_name="test-fp-avoidance3",
|
||||
blocked_words=[
|
||||
BlockedWord(keyword="select", action=ContentFilterAction.BLOCK),
|
||||
]
|
||||
)
|
||||
result = guardrail3._check_blocked_words("The selection process")
|
||||
assert result is None, "selection should not match select"
|
||||
Loading…
Add table
Reference in a new issue