mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
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("<status>:<body>") 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.
This commit is contained in:
parent
1abc8e0720
commit
32851f08e0
2 changed files with 59 additions and 3 deletions
|
|
@ -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("<status>:<body>")`` 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),
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue