From 99f2487d3b41f643c34637ec6ad3730a740c4ae9 Mon Sep 17 00:00:00 2001 From: robin-fiddler Date: Wed, 13 May 2026 08:15:45 +0530 Subject: [PATCH] fix(guardrails): return HTTP 400 instead of 500 for blocked requests (#27617) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: reset org and tag budgets (#27326) * reset org budgets * reset tag budgets --------- Co-authored-by: Michael Riad Zaky * 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 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 Co-authored-by: Michael Riad Zaky Co-authored-by: ryan-crabbe-berri Co-authored-by: Krrish Dholakia --- litellm/exceptions.py | 4 ++ litellm/integrations/custom_guardrail.py | 11 +++- .../test_generic_guardrail_api.py | 1 + .../test_guardrail_exception_status_codes.py | 66 +++++++++++++++++++ 4 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 tests/test_litellm/test_guardrail_exception_status_codes.py diff --git a/litellm/exceptions.py b/litellm/exceptions.py index 8b005291556..17f5b43c273 100644 --- a/litellm/exceptions.py +++ b/litellm/exceptions.py @@ -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) diff --git a/litellm/integrations/custom_guardrail.py b/litellm/integrations/custom_guardrail.py index c937ad0a7bf..82a35f2eedd 100644 --- a/litellm/integrations/custom_guardrail.py +++ b/litellm/integrations/custom_guardrail.py @@ -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) diff --git a/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_generic_guardrail_api.py b/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_generic_guardrail_api.py index e01038cd35f..6ec793a1bb0 100644 --- a/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_generic_guardrail_api.py +++ b/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_generic_guardrail_api.py @@ -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( diff --git a/tests/test_litellm/test_guardrail_exception_status_codes.py b/tests/test_litellm/test_guardrail_exception_status_codes.py new file mode 100644 index 00000000000..c4df1295580 --- /dev/null +++ b/tests/test_litellm/test_guardrail_exception_status_codes.py @@ -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