From 2ccb4b94e548edb1612935884abf6b09a283ab1e Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Wed, 29 Apr 2026 14:56:47 -0700 Subject: [PATCH] fix(proxy/auth): gate guardrail modification check on key presence Use key-in-dict membership instead of truthy value lookup so explicitly supplied empty/falsy payloads still trigger the permission check. Adds parametrized regression coverage across all gated keys. --- litellm/proxy/auth/auth_checks.py | 2 +- .../proxy/auth/test_auth_checks.py | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index 840f64cfed..bf02d4afda 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -374,7 +374,7 @@ def _guardrail_modification_check( coerced = _coerce_to_dict(container) if coerced is None: return False - return any(coerced.get(key) for key in _GUARDRAIL_MODIFICATION_KEYS) + return any(key in coerced for key in _GUARDRAIL_MODIFICATION_KEYS) # Check both metadata keys — callers can populate either depending on the # endpoint. Cover the top-level too so root-level injection is rejected. diff --git a/tests/test_litellm/proxy/auth/test_auth_checks.py b/tests/test_litellm/proxy/auth/test_auth_checks.py index 676a32c202..d1b37c8e13 100644 --- a/tests/test_litellm/proxy/auth/test_auth_checks.py +++ b/tests/test_litellm/proxy/auth/test_auth_checks.py @@ -2078,6 +2078,33 @@ class TestGuardrailModificationCheck: ) assert exc.value.status_code == 403 + @pytest.mark.parametrize( + "key", + [ + "guardrails", + "disable_global_guardrails", + "disable_global_guardrail", + "opted_out_global_guardrails", + ], + ) + @pytest.mark.parametrize("empty_value", [{}, [], "", 0, False]) + def test_rejects_empty_value_modification(self, key, empty_value): + """Regression: an explicitly-supplied empty/falsy value still expresses + intent to modify and must trigger the permission check. Truthiness-based + gating let callers bypass the check by sending e.g. + ``metadata={"guardrails": {}}``, which downstream evaluation interpreted + as "disable all guardrails" while the auth layer treated it as no-op. + """ + from fastapi import HTTPException + + with patch( + "litellm.proxy.guardrails.guardrail_helpers.can_modify_guardrails", + return_value=False, + ): + with pytest.raises(HTTPException) as exc: + self._call({"metadata": {key: empty_value}}) + assert exc.value.status_code == 403 + def test_rejects_injection_via_litellm_metadata_key(self): """Caller can populate the OTHER metadata key; that must also 403.""" from fastapi import HTTPException