Address Greptile code review: response-only guard, fail-closed init, no matched_text in HTTP error, ReDoS protection

- Add input_type == "response" guard so tone detection only fires on LLM output
- Remove try/except in _init_tone_checker for fail-closed behavior on bad config
- Remove matched_text from HTTP 400 error detail (keep in internal detection only)
- Add _validate_user_pattern() with length limit and regex validation for ReDoS protection
- Add tests for input_type guard, invalid regex, and pattern length limit

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Krrish Dholakia 2026-03-11 20:42:15 -07:00
parent c60ea50baf
commit 9ebde9fb5c
3 changed files with 68 additions and 15 deletions

View file

@ -310,16 +310,10 @@ class ContentFilterGuardrail(CustomGuardrail):
)
def _init_tone_checker(self, tone_detection_config: Dict[str, Any]) -> None:
try:
self._tone_checker = ToneChecker(tone_detection_config)
verbose_proxy_logger.debug(
"ContentFilterGuardrail: tone checker enabled"
)
except Exception as e:
verbose_proxy_logger.warning(
"ContentFilterGuardrail: failed to init tone checker: %s",
e,
)
self._tone_checker = ToneChecker(tone_detection_config)
verbose_proxy_logger.debug(
"ContentFilterGuardrail: tone checker enabled"
)
def _apply_tone_detection_policy(
self,
@ -332,20 +326,19 @@ class ContentFilterGuardrail(CustomGuardrail):
detection: ToneDetection = {
"type": "tone",
"category": category,
# matched_text kept in internal detection for logging/tracing only
"matched_text": matched,
}
detections.append(detection)
verbose_proxy_logger.warning(
"ContentFilterGuardrail: tone violation (%s): '%s'",
"ContentFilterGuardrail: tone violation (%s)",
category,
matched,
)
raise HTTPException(
status_code=400,
detail={
"error": f"Tone violation detected: {category}",
"category": category,
"matched_text": matched,
},
)
@ -1869,8 +1862,8 @@ class ContentFilterGuardrail(CustomGuardrail):
self._apply_competitor_intent_policy(
intent_result, request_data, detections
)
# Tone detection (optional; raises on violation)
if self._tone_checker and text:
# Tone detection — only on LLM responses, not user input
if self._tone_checker and text and input_type == "response":
tone_result = self._tone_checker.run(text)
if tone_result is not None:
self._apply_tone_detection_policy(

View file

@ -11,6 +11,9 @@ from typing import Dict, List, Optional, Pattern, Tuple
from litellm._logging import verbose_proxy_logger
# Maximum length for user-supplied regex patterns to mitigate ReDoS
_MAX_PATTERN_LENGTH = 1024
# ---------------------------------------------------------------------------
# Built-in tone patterns — each is (raw_regex, category_label)
# ---------------------------------------------------------------------------
@ -71,6 +74,21 @@ def _compile_patterns(
_COMPILED_TONE_PATTERNS = _compile_patterns(_TONE_PATTERNS)
def _validate_user_pattern(pattern: str) -> None:
"""Validate a user-supplied regex pattern for safety.
Raises ValueError on invalid regex or patterns exceeding the length limit.
"""
if len(pattern) > _MAX_PATTERN_LENGTH:
raise ValueError(
f"Tone detection: pattern exceeds maximum length of {_MAX_PATTERN_LENGTH} characters"
)
try:
re.compile(pattern)
except re.error as e:
raise ValueError(f"Tone detection: invalid regex pattern: {e}") from e
class ToneChecker:
"""
CPU-only tone checker.
@ -83,12 +101,14 @@ class ToneChecker:
def __init__(self, config: Dict) -> None:
self._extra_blocked: List[Tuple[Pattern[str], str]] = []
for phrase in config.get("blocked_phrases") or []:
_validate_user_pattern(phrase)
self._extra_blocked.append(
(re.compile(phrase, re.IGNORECASE), "custom_blocked")
)
self._safe_patterns: List[Pattern[str]] = []
for phrase in config.get("safe_phrases") or []:
_validate_user_pattern(phrase)
self._safe_patterns.append(re.compile(phrase, re.IGNORECASE))
verbose_proxy_logger.debug(

View file

@ -423,3 +423,43 @@ class TestInit:
def test_tone_checker_disabled_when_no_config(self):
g = ContentFilterGuardrail(guardrail_name="test-no-tone")
assert g._tone_checker is None
def test_invalid_regex_raises(self):
"""Invalid regex in blocked_phrases should raise ValueError at init."""
with pytest.raises(ValueError, match="invalid regex pattern"):
_make_guardrail(blocked_phrases=[r"(unclosed"])
def test_pattern_too_long_raises(self):
"""Patterns exceeding the length limit should raise ValueError."""
with pytest.raises(ValueError, match="exceeds maximum length"):
_make_guardrail(blocked_phrases=["a" * 2000])
# ---------------------------------------------------------------------------
# INPUT TYPE GUARD — tone detection only fires on responses
# ---------------------------------------------------------------------------
class TestInputTypeGuard:
@pytest.mark.asyncio
async def test_request_input_type_skips_tone_detection(self):
"""Tone detection should NOT fire when input_type is 'request'."""
g = _make_guardrail()
# This text would be blocked on 'response' but should pass on 'request'
result = await g.apply_guardrail(
_inputs("That's not my problem."),
{},
"request",
)
assert result["texts"] == ["That's not my problem."]
@pytest.mark.asyncio
async def test_response_input_type_fires_tone_detection(self):
"""Tone detection should fire when input_type is 'response'."""
g = _make_guardrail()
with pytest.raises(HTTPException):
await g.apply_guardrail(
_inputs("That's not my problem."),
{},
"response",
)