diff --git a/litellm/proxy/guardrails/guardrail_hooks/llm_as_a_judge/__init__.py b/litellm/proxy/guardrails/guardrail_hooks/llm_as_a_judge/__init__.py index b03ae70225e..a536846860e 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/llm_as_a_judge/__init__.py +++ b/litellm/proxy/guardrails/guardrail_hooks/llm_as_a_judge/__init__.py @@ -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: diff --git a/tests/test_litellm/proxy/guardrails/test_llm_as_a_judge.py b/tests/test_litellm/proxy/guardrails/test_llm_as_a_judge.py index 26b8ecddc44..49afad85e43 100644 --- a/tests/test_litellm/proxy/guardrails/test_llm_as_a_judge.py +++ b/tests/test_litellm/proxy/guardrails/test_llm_as_a_judge.py @@ -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):