mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-23 00:41:40 +00:00
address review findings on #38143 (yucheng-berri, cursor, veria-ai, devin)
Rename fail_mode → unreachable_fallback (typed field) ───────────────────────────────────────────────────── The shim was reading a free-form ``fail_mode`` field; a typo silently defaulted the plugin to fail-open behavior. Switch to the typed ``LitellmParams.unreachable_fallback`` field so Pydantic validates the value at config load. The plugin's constructor kwarg stays as ``fail_mode`` — the initializer maps the typed field onto it. (yucheng-berri, devin-ai-integration) Fix timeout default (was silently discarded) ──────────────────────────────────────────── ``getattr(litellm_params, "timeout", 8.0)`` only applied the default when the attribute was missing; ``LitellmParams.timeout`` always exists and defaults to ``None``, so the intended 8-second budget was never used. Change to ``getattr(..., None) or 8.0`` so ``None`` (and ``0``) fall through to the default. (cursor[bot]) Move ImportError from module-load to __init__ ───────────────────────────────────────────── Raising ImportError at module load caused the guardrail-hook auto-discovery loop to silently drop the registration when ``conduct-litellm-guard`` was missing. Users saw configs load with no guardrail active and no error. Import lazily; raise the friendly ``pip install`` error at ``ConductGuardrail.__init__`` when actionable. (cursor[bot]) Advertise only supported event hooks ──────────────────────────────────── ``during_call`` mode was advertised in the guardrail config but the class never overrode ``async_moderation_hook`` — every request in that mode silently bypassed policy. Override ``get_supported_event_hooks`` to return only ``pre_call`` so LiteLLM validates configs against supported modes at load time. ``during_call`` / ``post_call`` support lands with plugin 0.3.x once the underlying response-gate is wired through ``guard_check_response``. (veria-ai) Text-completion + full-turn prompt scanning ─────────────────────────────────────────── Fixed in the standalone package: ``conduct-litellm-guard 0.2.2`` (BerriAI/litellm PR #38143 companion, shipping to PyPI shortly). Pinned in the docstring here as the minimum supported version. (veria-ai — text_completion bypass + 4KB truncation) Tests ───── * ``test_only_pre_call_event_hook_advertised`` — regression for ``during_call`` silent-bypass finding * ``test_initialize_prefers_typed_unreachable_fallback`` — regression for typo silent-fail-open finding * ``test_initialize_applies_timeout_default_when_field_is_none`` — regression for silently-discarded 8.0 default * ``test_missing_standalone_package_raises_at_construction`` — regression for silent-drop-on-import-failure finding (previous module-load raise replaced with lazy import + init-time raise)
This commit is contained in:
parent
e0f9484cb3
commit
58022daff7
3 changed files with 237 additions and 45 deletions
|
|
@ -8,22 +8,54 @@ if TYPE_CHECKING:
|
|||
from litellm.types.guardrails import Guardrail, LitellmParams
|
||||
|
||||
|
||||
# ─── Constants ────────────────────────────────────────────────────────
|
||||
# Kept as module-level names so the intent is obvious in review — no
|
||||
# `getattr(..., 8.0)` default that gets silently discarded because the
|
||||
# field always exists as None (cursor[bot] finding).
|
||||
_DEFAULT_TIMEOUT_SECONDS = 8.0
|
||||
_DEFAULT_UNREACHABLE_FALLBACK = "fail_closed"
|
||||
|
||||
|
||||
def initialize_guardrail(litellm_params: "LitellmParams", guardrail: "Guardrail"):
|
||||
"""Initialize the Conduct guardrail from LiteLLM's config block.
|
||||
|
||||
Maps LiteLLM's idiomatic ``api_base`` / ``api_key`` to Conduct's
|
||||
``api_url`` / ``agent_token`` constructor arguments. All other
|
||||
settings pass through unchanged.
|
||||
Maps LiteLLM's typed guardrail fields onto the Conduct constructor:
|
||||
|
||||
LiteLLM field → Conduct kwarg
|
||||
───────────────────────────────────────────────
|
||||
api_base → api_url
|
||||
api_key → agent_token
|
||||
workspace_id (extra) → workspace_id
|
||||
unreachable_fallback → fail_mode (fail_closed | fail_open)
|
||||
timeout → timeout
|
||||
|
||||
The typed ``unreachable_fallback`` field replaces the free-form
|
||||
``fail_mode`` this shim previously read. A typo on the old field
|
||||
silently defaulted the plugin to fail-open behavior; using the
|
||||
typed field forces Pydantic validation upstream (yucheng-berri,
|
||||
devin-ai-integration findings).
|
||||
"""
|
||||
import litellm
|
||||
|
||||
# ``getattr(..., default)`` only fires when the attribute is missing;
|
||||
# ``LitellmParams`` always defines ``timeout`` and defaults it to
|
||||
# ``None``, so the default was never applied. Use ``or`` so an
|
||||
# explicit ``None`` (or ``0``) also falls through to the intended
|
||||
# 8-second budget (cursor[bot] finding).
|
||||
timeout = getattr(litellm_params, "timeout", None) or _DEFAULT_TIMEOUT_SECONDS
|
||||
unreachable_fallback = (
|
||||
getattr(litellm_params, "unreachable_fallback", None)
|
||||
or _DEFAULT_UNREACHABLE_FALLBACK
|
||||
)
|
||||
|
||||
_conduct_callback: Final = ConductGuardrail(
|
||||
api_url=getattr(litellm_params, "api_base", None),
|
||||
agent_token=getattr(litellm_params, "api_key", None),
|
||||
workspace_id=getattr(litellm_params, "workspace_id", None),
|
||||
fail_mode=getattr(litellm_params, "fail_mode", "fail_closed"),
|
||||
tool_name=getattr(litellm_params, "tool_name", "llm_call"),
|
||||
timeout=getattr(litellm_params, "timeout", 8.0),
|
||||
# Conduct's constructor argument is still ``fail_mode`` — mapped
|
||||
# from the typed LiteLLM field above.
|
||||
fail_mode=unreachable_fallback,
|
||||
timeout=timeout,
|
||||
guardrail_name=guardrail.get("guardrail_name", ""),
|
||||
event_hook=litellm_params.mode,
|
||||
default_on=litellm_params.default_on,
|
||||
|
|
|
|||
|
|
@ -1,26 +1,93 @@
|
|||
"""Conduct Guard as a LiteLLM guardrail.
|
||||
|
||||
Thin re-export. The adapter, response-envelope parser, session-ID chain,
|
||||
and fail-mode logic all live in the `conduct-litellm-guard` PyPI package,
|
||||
which is where issues, versioning, and standalone-user support live.
|
||||
Thin adapter over the ``conduct-litellm-guard`` PyPI package. The
|
||||
adapter, response-envelope parser, session-ID chain, fail-mode logic,
|
||||
and the ``guard_check_prompt`` wire client all live in that package —
|
||||
this file only wires the base runtime into LiteLLM's ``CustomGuardrail``
|
||||
contract.
|
||||
|
||||
Install: `pip install conduct-litellm-guard`
|
||||
Install: ``pip install "conduct-litellm-guard>=0.2.2"``
|
||||
Source: https://github.com/sseshachala/conductai/tree/main/packages/conduct-litellm-guard
|
||||
Docs: https://conductai.ai/guard
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any, ClassVar
|
||||
|
||||
from litellm.integrations.custom_guardrail import CustomGuardrail
|
||||
from litellm.types.guardrails import GuardrailEventHooks
|
||||
|
||||
_IMPORT_ERROR_MESSAGE = (
|
||||
"conduct-litellm-guard is required for the Conduct guardrail. "
|
||||
'Install it with: pip install "conduct-litellm-guard>=0.2.2"'
|
||||
)
|
||||
|
||||
|
||||
# ── Base plugin import — deferred to init-time ─────────────────────────
|
||||
# Reason: the guardrail-hook auto-discovery loop treats a module-level
|
||||
# ``raise ImportError`` as "hook unavailable" and silently drops the
|
||||
# registration. A user who installed LiteLLM but forgot the
|
||||
# ``conduct-litellm-guard`` dependency would see their config load with
|
||||
# no guardrail active and no error message (cursor[bot] finding on
|
||||
# BerriAI/litellm#38143). Import here without raising; surface the
|
||||
# missing dep at ``__init__`` time when it is actionable.
|
||||
|
||||
try:
|
||||
from conduct_litellm_guard import ConductGuard as ConductGuardrail
|
||||
from conduct_litellm_guard import ConductGuard as _BaseConductGuard
|
||||
from conduct_litellm_guard.guardrail import (
|
||||
ConductGuardBlocked as ConductGuardrailBlocked,
|
||||
)
|
||||
from conduct_litellm_guard.guardrail import GuardDecision
|
||||
from conduct_litellm_guard.guardrail import GuardDecision # noqa: F401 — re-exported
|
||||
|
||||
_IMPORT_ERROR: ImportError | None = None
|
||||
except ImportError as _e:
|
||||
raise ImportError(
|
||||
"conduct-litellm-guard is required for the Conduct guardrail. Install with: pip install conduct-litellm-guard"
|
||||
) from _e
|
||||
_BaseConductGuard = None # type: ignore[assignment,misc]
|
||||
ConductGuardrailBlocked = None # type: ignore[assignment,misc]
|
||||
GuardDecision = None # type: ignore[assignment,misc]
|
||||
_IMPORT_ERROR = _e
|
||||
|
||||
|
||||
# When the base package isn't installed we still need a real class so
|
||||
# LiteLLM's registry lookup succeeds; the friendly error surfaces on
|
||||
# construction.
|
||||
_ParentClass = _BaseConductGuard if _BaseConductGuard is not None else CustomGuardrail
|
||||
|
||||
|
||||
class ConductGuardrail(_ParentClass): # type: ignore[valid-type,misc]
|
||||
"""LiteLLM adapter over ``conduct_litellm_guard.ConductGuard``.
|
||||
|
||||
Subclass exists so we can:
|
||||
- Advertise supported event hooks honestly to LiteLLM (see
|
||||
``get_supported_event_hooks``).
|
||||
- Raise a friendly error at construction time when the base package
|
||||
isn't installed (rather than at module import — see comment
|
||||
above).
|
||||
"""
|
||||
|
||||
# Advertised event hooks. The plugin currently runs at pre_call
|
||||
# (input rail) — a policy block short-circuits before the model
|
||||
# sees the prompt, which is the semantic LiteLLM users expect for
|
||||
# a "guardrail". ``during_call`` / ``post_call`` support lands with
|
||||
# 0.3.x once the underlying Conduct response gate is wired through
|
||||
# ``guard_check_response`` (tracked in the plugin repo). Advertising
|
||||
# only pre_call today prevents silent bypass of ``during_call``
|
||||
# configurations — see veria-ai finding on BerriAI/litellm#38143.
|
||||
SUPPORTED_EVENT_HOOKS: ClassVar[tuple[GuardrailEventHooks, ...]] = (
|
||||
GuardrailEventHooks.pre_call,
|
||||
)
|
||||
|
||||
def __init__(self, *args: Any, **kwargs: Any) -> None:
|
||||
if _IMPORT_ERROR is not None or _BaseConductGuard is None:
|
||||
raise ImportError(_IMPORT_ERROR_MESSAGE) from _IMPORT_ERROR
|
||||
super().__init__(*args, **kwargs)
|
||||
|
||||
@classmethod
|
||||
def get_supported_event_hooks(cls) -> list[GuardrailEventHooks]:
|
||||
"""LiteLLM calls this during config validation to reject
|
||||
unsupported ``mode:`` values (e.g. ``during_call`` while only
|
||||
pre_call is implemented)."""
|
||||
return list(cls.SUPPORTED_EVENT_HOOKS)
|
||||
|
||||
|
||||
__all__ = ["ConductGuardrail", "ConductGuardrailBlocked", "GuardDecision"]
|
||||
|
|
|
|||
|
|
@ -1,22 +1,28 @@
|
|||
"""Smoke tests for the Conduct guardrail integration.
|
||||
"""Tests for the Conduct guardrail integration.
|
||||
|
||||
The adapter itself is tested in the ``conduct-litellm-guard`` PyPI
|
||||
package. Here we only verify:
|
||||
* the LiteLLM-tree module imports cleanly when the standalone package
|
||||
is installed
|
||||
* the enum + registry entries are wired
|
||||
The adapter itself is tested in ``conduct-litellm-guard`` on PyPI —
|
||||
here we only verify the LiteLLM-tree wiring:
|
||||
* the module imports cleanly with and without the standalone package
|
||||
* the enum + registry entries are populated
|
||||
* ``initialize_guardrail`` reads the typed ``unreachable_fallback``
|
||||
field, applies the timeout default correctly, and registers the
|
||||
callback with LiteLLM's manager (yucheng-berri / cursor findings)
|
||||
* only supported event hooks are advertised (veria-ai finding)
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import sys
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
# The Conduct guardrail imports its runtime from `conduct-litellm-guard`
|
||||
# on PyPI. When the package is not installed in the CI environment,
|
||||
# skip — the wiring smoke tests only make sense against the real dep.
|
||||
# skip the wiring smoke tests. The missing-package test below runs
|
||||
# unconditionally because it needs a controlled ImportError.
|
||||
pytest.importorskip(
|
||||
"conduct_litellm_guard",
|
||||
reason="Install `conduct-litellm-guard` to test the Conduct guardrail integration.",
|
||||
|
|
@ -24,7 +30,6 @@ pytest.importorskip(
|
|||
|
||||
|
||||
def test_import_module() -> None:
|
||||
"""The wrapper module imports without side effects."""
|
||||
module = importlib.import_module("litellm.proxy.guardrails.guardrail_hooks.conduct")
|
||||
assert module.ConductGuardrail is not None
|
||||
|
||||
|
|
@ -52,16 +57,22 @@ def test_registries_populated() -> None:
|
|||
assert "conduct" in guardrail_initializer_registry
|
||||
|
||||
|
||||
def test_only_pre_call_event_hook_advertised() -> None:
|
||||
"""Regression for veria-ai finding on #38143 —
|
||||
``during_call`` mode was silently accepted but never evaluated
|
||||
because ``async_moderation_hook`` was not overridden. We only
|
||||
advertise pre_call today so LiteLLM validates configs against
|
||||
supported hooks and rejects unsupported modes."""
|
||||
from litellm.proxy.guardrails.guardrail_hooks.conduct import ConductGuardrail
|
||||
from litellm.types.guardrails import GuardrailEventHooks
|
||||
|
||||
hooks = ConductGuardrail.get_supported_event_hooks()
|
||||
assert hooks == [GuardrailEventHooks.pre_call]
|
||||
|
||||
|
||||
def test_initialize_guardrail_returns_wired_callback(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""``initialize_guardrail`` maps LiteLLM params to Conduct kwargs and
|
||||
registers the callback with ``logging_callback_manager``. This test
|
||||
exercises the full function body so coverage reports don't flag it
|
||||
as dead code."""
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
monkeypatch.setenv("CONDUCT_AGENT_TOKEN", "cond_agt_test_placeholder")
|
||||
|
||||
added_callbacks: list[object] = []
|
||||
|
|
@ -93,24 +104,106 @@ def test_initialize_guardrail_returns_wired_callback(
|
|||
assert added_callbacks == [callback]
|
||||
|
||||
|
||||
def test_missing_standalone_package_raises_helpful_error(
|
||||
def test_initialize_prefers_typed_unreachable_fallback(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""When ``conduct-litellm-guard`` is not installed, the import fails
|
||||
with a message pointing users at the ``pip install`` command."""
|
||||
# Ensure the module is re-imported without the standalone package.
|
||||
for name in list(sys.modules):
|
||||
if name.startswith(("conduct_litellm_guard", "litellm.proxy.guardrails.guardrail_hooks.conduct")):
|
||||
monkeypatch.delitem(sys.modules, name, raising=False)
|
||||
"""Regression for yucheng-berri / devin-ai-integration findings —
|
||||
the typed ``unreachable_fallback`` field replaces the free-form
|
||||
``fail_mode`` this shim previously read. Typos on the old field
|
||||
silently defaulted to fail-open behavior; the typed field forces
|
||||
Pydantic validation."""
|
||||
monkeypatch.setenv("CONDUCT_AGENT_TOKEN", "cond_agt_test_placeholder")
|
||||
import litellm
|
||||
from litellm.proxy.guardrails.guardrail_hooks.conduct import initialize_guardrail
|
||||
|
||||
real_import = __builtins__["__import__"] if isinstance(__builtins__, dict) else __builtins__.__import__
|
||||
captured: dict = {}
|
||||
|
||||
def _fake_import(name: str, *args: object, **kwargs: object) -> object:
|
||||
if name.startswith("conduct_litellm_guard"):
|
||||
raise ImportError("simulated missing package")
|
||||
return real_import(name, *args, **kwargs)
|
||||
class _FakeGuard:
|
||||
def __init__(self, **kwargs: object) -> None:
|
||||
captured.update(kwargs)
|
||||
|
||||
monkeypatch.setattr("builtins.__import__", _fake_import)
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.guardrails.guardrail_hooks.conduct.ConductGuardrail",
|
||||
_FakeGuard,
|
||||
raising=True,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
litellm, "logging_callback_manager", SimpleNamespace(add_litellm_callback=lambda cb: None)
|
||||
)
|
||||
|
||||
with pytest.raises(ImportError, match="pip install conduct-litellm-guard"):
|
||||
importlib.import_module("litellm.proxy.guardrails.guardrail_hooks.conduct.conduct")
|
||||
litellm_params = SimpleNamespace(
|
||||
api_base=None,
|
||||
api_key=None,
|
||||
unreachable_fallback="fail_open",
|
||||
mode="pre_call",
|
||||
default_on=True,
|
||||
)
|
||||
guardrail = MagicMock()
|
||||
guardrail.get.return_value = "conduct-guard"
|
||||
|
||||
initialize_guardrail(litellm_params, guardrail)
|
||||
# The Conduct constructor still accepts ``fail_mode`` — we map from
|
||||
# the typed LiteLLM field to it.
|
||||
assert captured["fail_mode"] == "fail_open"
|
||||
|
||||
|
||||
def test_initialize_applies_timeout_default_when_field_is_none(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Regression for cursor[bot] finding —
|
||||
``LitellmParams.timeout`` always exists as ``None``, so
|
||||
``getattr(litellm_params, "timeout", 8.0)`` was never applied. The
|
||||
default now uses ``or`` so ``None`` falls through to 8.0."""
|
||||
monkeypatch.setenv("CONDUCT_AGENT_TOKEN", "cond_agt_test_placeholder")
|
||||
import litellm
|
||||
from litellm.proxy.guardrails.guardrail_hooks.conduct import initialize_guardrail
|
||||
|
||||
captured: dict = {}
|
||||
|
||||
class _FakeGuard:
|
||||
def __init__(self, **kwargs: object) -> None:
|
||||
captured.update(kwargs)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.guardrails.guardrail_hooks.conduct.ConductGuardrail",
|
||||
_FakeGuard,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
litellm, "logging_callback_manager", SimpleNamespace(add_litellm_callback=lambda cb: None)
|
||||
)
|
||||
|
||||
litellm_params = SimpleNamespace(
|
||||
api_base=None,
|
||||
api_key=None,
|
||||
timeout=None, # the typical case — field exists but caller left it unset
|
||||
mode="pre_call",
|
||||
default_on=True,
|
||||
)
|
||||
guardrail = MagicMock()
|
||||
guardrail.get.return_value = "conduct-guard"
|
||||
|
||||
initialize_guardrail(litellm_params, guardrail)
|
||||
assert captured["timeout"] == 8.0
|
||||
|
||||
|
||||
def test_missing_standalone_package_raises_at_construction() -> None:
|
||||
"""Regression for cursor[bot] finding — the previous shim raised
|
||||
``ImportError`` at module load, which the guardrail-hook auto-loader
|
||||
treats as "hook unavailable" and silently drops. The subclass now
|
||||
imports lazily and raises at ``__init__`` time when actionable."""
|
||||
from litellm.proxy.guardrails.guardrail_hooks.conduct import conduct as _mod
|
||||
|
||||
original_base = _mod._BaseConductGuard
|
||||
original_error = _mod._IMPORT_ERROR
|
||||
try:
|
||||
_mod._BaseConductGuard = None # type: ignore[assignment]
|
||||
_mod._IMPORT_ERROR = ImportError("simulated missing package")
|
||||
|
||||
# Class is still importable — no module-load side effect.
|
||||
from litellm.proxy.guardrails.guardrail_hooks.conduct import ConductGuardrail
|
||||
|
||||
with pytest.raises(ImportError, match="pip install"):
|
||||
ConductGuardrail()
|
||||
finally:
|
||||
_mod._BaseConductGuard = original_base # type: ignore[assignment]
|
||||
_mod._IMPORT_ERROR = original_error
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue