mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(guardrails): treat unscored judge verdicts as indeterminate
When deriving overall_score from per-criterion verdicts, a verdict that
could not be scored (non-dict, or a missing/unparsable/non-finite score)
was silently skipped and the remaining verdicts averaged. An attacker who
can influence the judge output could omit the score on a failing criterion
(e.g. {"passed": false} with no score) so only the passing verdicts are
averaged, inflating the result into a pass and slipping past a
fail_closed=True, on_failure="block" guardrail.
Treat any such verdict as indeterminate: _derive_overall_score now returns
None so the caller fails closed, instead of dropping the verdict. Weights
stay optional — a missing/non-numeric weight only disables the weighted
path (the score still counts via a simple mean), so it is not a fail-open
vector.
Updates the two tests that asserted the old skip behaviour and adds unit +
end-to-end coverage for the unscored-failing-verdict bypass.
Refs #30731
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
022539de68
commit
26346e50ac
2 changed files with 72 additions and 12 deletions
|
|
@ -93,6 +93,16 @@ def _derive_overall_score(judge_result: Dict[str, Any]) -> Optional[float]:
|
|||
judge which returns only verdicts cannot silently pass. All scores are
|
||||
validated as finite and clamped to ``[0, 100]`` before use, so injected
|
||||
``NaN``/``Infinity`` or out-of-range values cannot inflate the result.
|
||||
|
||||
When deriving from verdicts, *every* verdict must yield a usable finite
|
||||
score. A verdict that cannot be scored -- not a dict, or a missing,
|
||||
unparsable, or non-finite ``score`` -- makes the whole evaluation
|
||||
indeterminate and returns ``None``. Silently dropping such a verdict would
|
||||
let an attacker who can influence the judge output omit the score on a
|
||||
failing criterion and inflate the average into a pass. Weights stay optional:
|
||||
a missing/zero/non-numeric weight only disables the weighted path (the score
|
||||
still counts via a simple mean), so it is not a fail-open vector.
|
||||
|
||||
Returns ``None`` when no usable score can be determined (including a non-dict
|
||||
judge result), leaving the fail-open/closed decision to the caller.
|
||||
"""
|
||||
|
|
@ -118,17 +128,21 @@ def _derive_overall_score(judge_result: Dict[str, Any]) -> Optional[float]:
|
|||
scores: List[float] = []
|
||||
all_weighted = True
|
||||
for verdict in verdicts:
|
||||
# Every verdict must contribute a usable finite score. A verdict we
|
||||
# cannot score is treated as indeterminate (return None) rather than
|
||||
# skipped: silently dropping it would let an attacker omit the score on a
|
||||
# failing criterion and inflate the remaining scores into a pass.
|
||||
if not isinstance(verdict, dict):
|
||||
continue
|
||||
return None
|
||||
raw_score = verdict.get("score")
|
||||
if raw_score is None:
|
||||
continue
|
||||
return None
|
||||
try:
|
||||
score = float(raw_score)
|
||||
except (TypeError, ValueError):
|
||||
continue
|
||||
return None
|
||||
if not math.isfinite(score):
|
||||
continue
|
||||
return None
|
||||
score = max(0.0, min(100.0, score)) # clamp per verdict before averaging
|
||||
scores.append(score)
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -275,14 +275,12 @@ def test_derive_overall_score_none_when_no_data():
|
|||
assert _derive_overall_score({"verdicts": [{"reasoning": "no score"}]}) is None
|
||||
|
||||
|
||||
def test_derive_overall_score_skips_non_dict_verdict_and_bad_weight():
|
||||
# Non-dict entries are skipped; verdicts whose weight is non-numeric fall
|
||||
# back to a simple mean of the parsed scores instead of crashing.
|
||||
def test_derive_overall_score_bad_weight_falls_back_to_simple_mean():
|
||||
# A non-numeric weight is not a fail-open vector: the score still counts, the
|
||||
# weighted path is just disabled in favour of a simple mean of all scores.
|
||||
result = _derive_overall_score(
|
||||
{
|
||||
"verdicts": [
|
||||
"not-a-dict", # skipped
|
||||
{"score": "abc", "weight": 50}, # non-numeric score -> skipped
|
||||
{"score": 40, "weight": "heavy"}, # bad weight -> ignored weight
|
||||
{"score": 60, "weight": "x"}, # bad weight -> ignored weight
|
||||
]
|
||||
|
|
@ -324,12 +322,14 @@ def test_derive_overall_score_non_finite_top_level_falls_back_to_verdicts():
|
|||
assert result == pytest.approx(10.0)
|
||||
|
||||
|
||||
def test_derive_overall_score_skips_non_finite_verdict_score():
|
||||
# A NaN verdict score is skipped, not averaged in as 100.
|
||||
def test_derive_overall_score_indeterminate_on_non_finite_verdict_score():
|
||||
# A NaN verdict score makes the evaluation indeterminate: it must NOT be
|
||||
# silently skipped and the remaining verdicts averaged, because the dropped
|
||||
# verdict could be the failing one. Indeterminate -> None -> caller decides.
|
||||
result = _derive_overall_score(
|
||||
{"verdicts": [{"score": "nan", "weight": 60}, {"score": 0, "weight": 40}]}
|
||||
)
|
||||
assert result == pytest.approx(0.0)
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_derive_overall_score_clamps_inflated_verdict_score():
|
||||
|
|
@ -345,6 +345,30 @@ def test_derive_overall_score_none_for_non_dict():
|
|||
assert _derive_overall_score(123) is None
|
||||
|
||||
|
||||
def test_derive_overall_score_indeterminate_on_unscored_failing_verdict():
|
||||
# The judge marks a criterion as failed but omits its numeric score. The
|
||||
# failing verdict must NOT be dropped while the passing one is averaged into
|
||||
# a 100 -> the whole evaluation is indeterminate (None).
|
||||
result = _derive_overall_score(
|
||||
{
|
||||
"verdicts": [
|
||||
{"criterion_name": "Accuracy", "score": 100, "weight": 60},
|
||||
{"criterion_name": "Safety", "passed": False, "weight": 40}, # no score
|
||||
]
|
||||
}
|
||||
)
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_derive_overall_score_indeterminate_on_malformed_verdict():
|
||||
# A non-dict entry or an unparsable score among otherwise-valid verdicts also
|
||||
# makes the derivation indeterminate rather than averaging the rest.
|
||||
assert _derive_overall_score({"verdicts": [{"score": 90}, "junk"]}) is None
|
||||
assert (
|
||||
_derive_overall_score({"verdicts": [{"score": 90}, {"score": "abc"}]}) is None
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion")
|
||||
async def test_non_dict_judge_output_fail_closed_blocks(mock_completion):
|
||||
|
|
@ -382,6 +406,28 @@ async def test_inflated_verdict_scores_still_block(mock_completion):
|
|||
assert exc_info.value.status_code == 422
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion")
|
||||
async def test_unscored_failing_verdict_fail_closed_blocks(mock_completion):
|
||||
# End-to-end: judge fails a criterion but omits its score. The passing
|
||||
# verdict alone must not slip through -> indeterminate -> fail_closed blocks.
|
||||
payload = {
|
||||
"verdicts": [
|
||||
{"criterion_name": "Accuracy", "score": 100, "weight": 60},
|
||||
{"criterion_name": "Safety", "passed": False, "weight": 40}, # no score
|
||||
]
|
||||
}
|
||||
mock_completion.return_value = MagicMock(
|
||||
choices=[MagicMock(message=MagicMock(content=json.dumps(payload)))]
|
||||
)
|
||||
guardrail = _make_guardrail(fail_closed=True, on_failure="block")
|
||||
inputs = {"texts": ["partially unsafe response"]}
|
||||
request_data: dict = {"messages": [], "metadata": {}}
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await guardrail.apply_guardrail(inputs, request_data, "response")
|
||||
assert exc_info.value.status_code == 422
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("litellm.proxy.guardrails.guardrail_hooks.llm_as_a_judge.litellm.acompletion")
|
||||
async def test_missing_weight_on_failing_verdict_still_blocks(mock_completion):
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue