From 32851f08e0767d4580cc22bafb9b70b702fa5549 Mon Sep 17 00:00:00 2001 From: lior-k Date: Tue, 28 Jul 2026 16:17:59 +0300 Subject: [PATCH] fix(guardrails): do not fail open on a persistent WonderFence client/config error With fail_open=True a non-empty but invalid or revoked api_key / app_id made the V2 SDK raise, the broad handler treated it as transient unavailability, and the request proceeded unscanned; a persistent auth misconfiguration therefore silently disabled scanning for every affected request until fixed (Greptile P1). The SDK raises Exception(":") on an HTTP error, so a client/config error surfaces as a 4xx. apply_guardrail now recognizes a 4xx other than 429 and does not fail open on it (HTTP 500 instead), mirroring how missing secrets are already handled and the SDK's own retry classification (4xx except 429 is not retried). 429 and 5xx remain transient and still fail open when enabled. Regression tests: a 401 does not fail open (500, SDK still invoked), and 503/429 still fail open under fail_open=True. --- .../alice_wonderfence/alice_wonderfence.py | 25 +++++++++++-- .../test_apply_guardrail_failmodes.py | 37 +++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/alice_wonderfence.py b/litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/alice_wonderfence.py index 38dcb91c077..8d6e328d449 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/alice_wonderfence.py +++ b/litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/alice_wonderfence.py @@ -62,6 +62,22 @@ if TYPE_CHECKING: logger = verbose_proxy_logger.getChild("alice_wonderfence") +def _is_wonderfence_client_error(exc: Exception) -> bool: + """True for a persistent WonderFence client/config error. + + The V2 SDK raises ``Exception(":")`` on an HTTP error, so an + invalid or revoked api_key / app_id surfaces as a 4xx. Such failures are + persistent, not a transient outage, so they must never fail open: doing so + would silently disable scanning for every affected request until the + credential is fixed. 429 and 5xx stay in the fail-open path, matching the + SDK's own retry classification (``utils.py``: 4xx except 429 is not retried). + """ + head = str(exc).split(":", 1)[0].strip() + if not head.isdigit(): + return False + return 400 <= int(head) < 500 and int(head) != 429 + + class WonderFenceGuardrail(CustomGuardrail): """Alice WonderFence guardrail handler using the V2 SDK client. @@ -282,7 +298,7 @@ class WonderFenceGuardrail(CustomGuardrail): }, ) from e except Exception as e: - if self.fail_open: + if self.fail_open and not _is_wonderfence_client_error(e): # Log only — do not add to the applied-guardrails header. The # header lists configured guardrail_names verbatim; consumers # rely on its membership to decide whether scanning ran. A @@ -298,9 +314,12 @@ class WonderFenceGuardrail(CustomGuardrail): exc_info=e, ) return inputs + # Either fail-open is off, or this is a persistent client/config + # error (4xx) that must never fail open — see + # _is_wonderfence_client_error. logger.error( - "Alice WonderFence unreachable; fail-open disabled, blocking " - "request. guardrail_name=%s input_type=%s error=%s", + "Alice WonderFence request failed and was not allowed through " + "(fail-open off or persistent client error). guardrail_name=%s input_type=%s error=%s", self.guardrail_name, input_type, str(e), diff --git a/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_apply_guardrail_failmodes.py b/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_apply_guardrail_failmodes.py index 8238e55f6c1..7863b029626 100644 --- a/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_apply_guardrail_failmodes.py +++ b/tests/test_litellm/proxy/guardrails/guardrail_hooks/alice_wonderfence/test_apply_guardrail_failmodes.py @@ -97,6 +97,43 @@ async def test_apply_guardrail_fail_closed_returns_500(guardrail_and_client, mak assert "Error in Alice WonderFence Guardrail" in exc.value.detail["error"] +@pytest.mark.asyncio +async def test_client_error_4xx_not_fail_open(make_guardrail, make_request_data): + """A persistent client/config error (the SDK raises Exception("<4xx>:...") + for an invalid/revoked api_key or app_id) must NOT fail open, even with + fail_open=True: it would silently disable scanning for every affected + request. It surfaces as HTTP 500 and the original inputs are not returned.""" + guardrail, client = make_guardrail(fail_open=True) + guardrail._client_cache["default-api-key"] = client + client.evaluate_prompt.side_effect = Exception('401:{"detail":"invalid api key"}') + + with pytest.raises(HTTPException) as exc: + await guardrail.apply_guardrail( + inputs={"texts": ["original"]}, + request_data=make_request_data(), + input_type="request", + ) + assert exc.value.status_code == 500 + client.evaluate_prompt.assert_awaited() + + +@pytest.mark.asyncio +async def test_transient_5xx_and_429_still_fail_open(make_guardrail, make_request_data): + """5xx and 429 are transient (matching the SDK's retry classification), so + fail_open still lets the request through unscanned.""" + for status in ("503", "429"): + guardrail, client = make_guardrail(fail_open=True) + guardrail._client_cache["default-api-key"] = client + client.evaluate_prompt.side_effect = Exception(f"{status}:upstream unavailable") + + out = await guardrail.apply_guardrail( + inputs={"texts": ["original"]}, + request_data=make_request_data(), + input_type="request", + ) + assert out["texts"] == ["original"], f"status {status} should fail open" + + @pytest.mark.asyncio async def test_malformed_override_does_not_fail_open(make_guardrail, make_request_data): """A non-string request-metadata app_id override must not slip through under