mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
fix(guardrails): return HTTP 400 instead of 500 for blocked requests (#27617)
* fix: reset org and tag budgets (#27326) * reset org budgets * reset tag budgets --------- Co-authored-by: Michael Riad Zaky <michaelr@Mac.localdomain> * fix(ui): omit allowed_routes from key edit save when unchanged (#27553) * fix(ui): omit allowed_routes from key edit save when unchanged When a team admin opens Edit Settings on a key with key_type=AI APIs and saves without changing anything, the UI re-sends the existing allowed_routes value, which the backend's _check_allowed_routes_caller_permission gate rejects for non-proxy-admins (LIT-2681). Strip allowed_routes from the patch in handleSubmit when it deep-equals the original keyData.allowed_routes. The backend treats absence as "leave alone," so no-op saves now succeed for non-admins. Admins explicitly editing the field still send the new value. * fix(ui): order-insensitive allowed_routes diff + cover null-original case Address Greptile review: - Switch the "is allowed_routes unchanged" check to a Set-based comparison so a server-side reorder of the array doesn't register as a user edit and re-trigger LIT-2681. - Add two regression tests: (1) keyData.allowed_routes is null and the form is untouched — patch should strip the field; (2) server returned routes in a different order than the user originally entered — patch should still recognize the value as unchanged. * chore(ui): strip ticket refs and tighten comments in key edit fix - Remove internal-tracker references from in-code comments - Tighten the WHY comment in handleSubmit to two lines - Drop redundant test-block comments — test names already describe the case * fix(ui): annotate Set<string> generic in allowed_routes diff to fix tsc * fix(guardrails): return HTTP 400 instead of 500 for guardrail-blocked requests GuardrailRaisedException and BlockedPiiEntityError both lacked a status_code attribute. When these exceptions reached the proxy exception handler (getattr(e, 'status_code', 500)), the fallback defaulted to HTTP 500 — making intentional guardrail blocks indistinguishable from server errors and causing unnecessary client retries. Changes: - Add status_code=400 (keyword-only) to GuardrailRaisedException - Add status_code=400 (keyword-only) to BlockedPiiEntityError - Update _is_guardrail_intervention() to recognize both exceptions so downstream loggers record 'guardrail_intervened' instead of 'guardrail_failed_to_respond' - Add 6 unit tests for default/custom status codes and getattr pattern - Strengthen existing blocked-action test with status_code assertion Fixes #24348 --------- Co-authored-by: Michael-RZ-Berri <michael@berri.ai> Co-authored-by: Michael Riad Zaky <michaelr@Mac.localdomain> Co-authored-by: ryan-crabbe-berri <ryan@berri.ai> Co-authored-by: Krrish Dholakia <krrish+github@berri.ai>
This commit is contained in:
parent
e73bbb5e9a
commit
99f2487d3b
4 changed files with 80 additions and 2 deletions
|
|
@ -918,9 +918,11 @@ class GuardrailRaisedException(Exception):
|
|||
guardrail_name: Optional[str] = None,
|
||||
message: str = "",
|
||||
should_wrap_with_default_message: bool = True,
|
||||
status_code: int = 400,
|
||||
):
|
||||
default_message = f"Guardrail raised an exception, Guardrail: {guardrail_name}, Message: {message}"
|
||||
self.guardrail_name = guardrail_name
|
||||
self.status_code = status_code
|
||||
self.message = default_message if should_wrap_with_default_message else message
|
||||
super().__init__(self.message)
|
||||
|
||||
|
|
@ -930,12 +932,14 @@ class BlockedPiiEntityError(Exception):
|
|||
self,
|
||||
entity_type: str,
|
||||
guardrail_name: Optional[str] = None,
|
||||
status_code: int = 400,
|
||||
):
|
||||
"""
|
||||
Raised when a blocked entity is detected by a guardrail.
|
||||
"""
|
||||
self.entity_type = entity_type
|
||||
self.guardrail_name = guardrail_name
|
||||
self.status_code = status_code
|
||||
self.message = f"Blocked entity detected: {entity_type} by Guardrail: {guardrail_name}. This entity is not allowed to be used in this request."
|
||||
super().__init__(self.message)
|
||||
|
||||
|
|
|
|||
|
|
@ -43,7 +43,11 @@ if TYPE_CHECKING:
|
|||
dc = DualCache()
|
||||
|
||||
|
||||
from litellm.exceptions import ModifyResponseException as ModifyResponseException
|
||||
from litellm.exceptions import (
|
||||
BlockedPiiEntityError,
|
||||
GuardrailRaisedException,
|
||||
ModifyResponseException,
|
||||
)
|
||||
|
||||
|
||||
class CustomGuardrail(CustomLogger):
|
||||
|
|
@ -737,12 +741,15 @@ class CustomGuardrail(CustomLogger):
|
|||
(this was logged previously as an API failure - guardrail_failed_to_respond).
|
||||
|
||||
Guardrails signal intentional blocks by raising:
|
||||
- GuardrailRaisedException (generic guardrail API, tool permission)
|
||||
- BlockedPiiEntityError (Presidio PII detection)
|
||||
- HTTPException with status 400 (content policy violation)
|
||||
- ModifyResponseException (passthrough mode violation)
|
||||
"""
|
||||
|
||||
if isinstance(e, ModifyResponseException):
|
||||
return True
|
||||
if isinstance(e, (GuardrailRaisedException, BlockedPiiEntityError)):
|
||||
return True
|
||||
if (
|
||||
HTTPException is not None
|
||||
and isinstance(e, HTTPException)
|
||||
|
|
|
|||
|
|
@ -553,6 +553,7 @@ class TestGuardrailActions:
|
|||
# Verify the exception has the clean error message (no wrapper)
|
||||
assert str(exc_info.value) == "Content contains harmful instructions"
|
||||
assert exc_info.value.guardrail_name == "generic_guardrail_api"
|
||||
assert exc_info.value.status_code == 400
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_action_intervened_modifies_content(
|
||||
|
|
|
|||
66
tests/test_litellm/test_guardrail_exception_status_codes.py
Normal file
66
tests/test_litellm/test_guardrail_exception_status_codes.py
Normal file
|
|
@ -0,0 +1,66 @@
|
|||
"""
|
||||
Tests for guardrail exception status codes.
|
||||
|
||||
GuardrailRaisedException and BlockedPiiEntityError must carry
|
||||
``status_code = 400`` so the proxy exception handler
|
||||
(``getattr(e, "status_code", 500)``) returns HTTP 400 instead of 500
|
||||
for intentional guardrail blocks.
|
||||
"""
|
||||
|
||||
from litellm.exceptions import BlockedPiiEntityError, GuardrailRaisedException
|
||||
|
||||
|
||||
class TestGuardrailRaisedExceptionStatusCode:
|
||||
"""GuardrailRaisedException should default to status_code=400."""
|
||||
|
||||
def test_default_status_code(self):
|
||||
exc = GuardrailRaisedException(
|
||||
guardrail_name="test_guardrail",
|
||||
message="blocked",
|
||||
)
|
||||
assert exc.status_code == 400
|
||||
|
||||
def test_custom_status_code(self):
|
||||
exc = GuardrailRaisedException(
|
||||
guardrail_name="test_guardrail",
|
||||
message="rate limited",
|
||||
status_code=429,
|
||||
)
|
||||
assert exc.status_code == 429
|
||||
|
||||
def test_getattr_fallback_resolves_to_400(self):
|
||||
"""The proxy uses ``getattr(e, 'status_code', 500)`` — verify it
|
||||
resolves to 400, not the 500 default."""
|
||||
exc = GuardrailRaisedException(
|
||||
guardrail_name="test_guardrail",
|
||||
message="blocked",
|
||||
)
|
||||
assert getattr(exc, "status_code", 500) == 400
|
||||
|
||||
|
||||
class TestBlockedPiiEntityErrorStatusCode:
|
||||
"""BlockedPiiEntityError should default to status_code=400."""
|
||||
|
||||
def test_default_status_code(self):
|
||||
exc = BlockedPiiEntityError(
|
||||
entity_type="CREDIT_CARD",
|
||||
guardrail_name="presidio",
|
||||
)
|
||||
assert exc.status_code == 400
|
||||
|
||||
def test_custom_status_code(self):
|
||||
exc = BlockedPiiEntityError(
|
||||
entity_type="SSN",
|
||||
guardrail_name="presidio",
|
||||
status_code=403,
|
||||
)
|
||||
assert exc.status_code == 403
|
||||
|
||||
def test_getattr_fallback_resolves_to_400(self):
|
||||
"""The proxy uses ``getattr(e, 'status_code', 500)`` — verify it
|
||||
resolves to 400, not the 500 default."""
|
||||
exc = BlockedPiiEntityError(
|
||||
entity_type="PHONE_NUMBER",
|
||||
guardrail_name="presidio",
|
||||
)
|
||||
assert getattr(exc, "status_code", 500) == 400
|
||||
Loading…
Add table
Reference in a new issue