From b5f7b58a1ac9875f219b17d33ef1b3ef3f0f3362 Mon Sep 17 00:00:00 2001 From: yucheng Date: Fri, 11 Sep 2026 20:25:32 +0000 Subject: [PATCH] fix(guardrails): validate Conduct event hooks, forward tool_name, drop optional-package test skip Pass the plugin's supported hook list into CustomGuardrail so unsupported modes (during_call, post_call, logging_only) are rejected at config load instead of silently doing nothing. Forward the configured tool_name to the plugin, and replace the missing-package stub so the registry still discovers the guardrail while construction raises an install hint. The regression tests inject a recording guardrail class so they run without conduct-litellm-guard installed; the previous module-level skip left the adapter untested in CI. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../guardrail_hooks/conduct/__init__.py | 79 +++---- .../guardrail_hooks/conduct/conduct.py | 71 +----- .../guardrail_hooks/test_conduct.py | 141 ++++++++++++ .../guardrails/test_conduct_guardrail.py | 210 ------------------ 4 files changed, 175 insertions(+), 326 deletions(-) create mode 100644 tests/test_litellm/proxy/guardrails/guardrail_hooks/test_conduct.py delete mode 100644 tests/test_litellm/proxy/guardrails/test_conduct_guardrail.py diff --git a/litellm/proxy/guardrails/guardrail_hooks/conduct/__init__.py b/litellm/proxy/guardrails/guardrail_hooks/conduct/__init__.py index 09dbcfba76b..b115b848b58 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/conduct/__init__.py +++ b/litellm/proxy/guardrails/guardrail_hooks/conduct/__init__.py @@ -1,76 +1,49 @@ +from __future__ import annotations + +from collections.abc import Mapping +from types import MappingProxyType from typing import TYPE_CHECKING, Final from litellm.types.guardrails import SupportedGuardrailIntegrations -from .conduct import ConductGuardrail, raise_if_missing_package +from .conduct import ConductGuardrail if TYPE_CHECKING: + from litellm.integrations.custom_guardrail import CustomGuardrail 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" +DEFAULT_TIMEOUT_SECONDS: Final = 8.0 +_NO_EXTRAS: Final[Mapping[str, object]] = MappingProxyType({}) -def initialize_guardrail(litellm_params: "LitellmParams", guardrail: "Guardrail") -> ConductGuardrail: - """Initialize the Conduct guardrail from LiteLLM's config block. - - 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). - """ +def initialize_guardrail( + litellm_params: LitellmParams, + guardrail: Guardrail, + guardrail_cls: type[CustomGuardrail] = ConductGuardrail, +) -> CustomGuardrail: import litellm - # Surface the missing-package error at config load, not silently at - # module import (cursor[bot] finding — see conduct.py header comment). - raise_if_missing_package() - - # ``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), - # Conduct's constructor argument is still ``fail_mode`` — mapped - # from the typed LiteLLM field above. - fail_mode=unreachable_fallback, - timeout=timeout, + extras: Final = litellm_params.model_extra or _NO_EXTRAS + _callback: Final = guardrail_cls( + api_url=litellm_params.api_base, + agent_token=litellm_params.api_key, + workspace_id=extras.get("workspace_id"), + tool_name=extras.get("tool_name", "llm_call"), + fail_mode=litellm_params.unreachable_fallback, + timeout=DEFAULT_TIMEOUT_SECONDS if litellm_params.timeout is None else litellm_params.timeout, guardrail_name=guardrail.get("guardrail_name", ""), event_hook=litellm_params.mode, default_on=litellm_params.default_on, + supported_event_hooks=guardrail_cls.get_supported_event_hooks(), ) - litellm.logging_callback_manager.add_litellm_callback(_conduct_callback) - - return _conduct_callback + litellm.logging_callback_manager.add_litellm_callback(_callback) + return _callback -guardrail_initializer_registry: Final = { # mutable-ok: LiteLLM guardrail registry contract +guardrail_initializer_registry: Final = { # mutable-ok: module-level registry, built once and never mutated SupportedGuardrailIntegrations.CONDUCT.value: initialize_guardrail, } - -guardrail_class_registry: Final = { # mutable-ok: LiteLLM guardrail registry contract +guardrail_class_registry: Final = { # mutable-ok: module-level registry, built once and never mutated SupportedGuardrailIntegrations.CONDUCT.value: ConductGuardrail, } diff --git a/litellm/proxy/guardrails/guardrail_hooks/conduct/conduct.py b/litellm/proxy/guardrails/guardrail_hooks/conduct/conduct.py index a0cbbf4bd67..1be2c6e51a8 100644 --- a/litellm/proxy/guardrails/guardrail_hooks/conduct/conduct.py +++ b/litellm/proxy/guardrails/guardrail_hooks/conduct/conduct.py @@ -1,83 +1,28 @@ -"""Conduct Guard as a LiteLLM guardrail. - -Thin alias for the ``conduct-litellm-guard`` PyPI package. The -``ConductGuard`` class ships with ``SUPPORTED_EVENT_HOOKS`` + -``get_supported_event_hooks`` since plugin 0.2.4, so this file no -longer needs a subclass wrapper — keeps LiteLLM's type-discipline / -basedpyright / test-quality budget gates satisfied. - -When the standalone package isn't installed we still register a real -stub class so LiteLLM's guardrail registry can scan -``get_supported_event_hooks`` at load time without crashing (see -BerriAI/litellm#38143 CI regression: registry iteration expects every -registered class to expose the hooks classmethod). Instantiation of -the stub raises ``ImportError`` via ``raise_if_missing_package``. +"""Conduct Guard as a LiteLLM guardrail, backed by the ``conduct-litellm-guard`` PyPI package. Install: ``pip install "conduct-litellm-guard>=0.2.4"`` Source: https://github.com/sseshachala/conductai/tree/main/packages/conduct-litellm-guard -Docs: https://conductai.ai/guard """ from __future__ import annotations -from typing import ClassVar +from typing import Final from litellm.integrations.custom_guardrail import CustomGuardrail -from litellm.types.guardrails import GuardrailEventHooks -_import_error_message = ( +MISSING_PACKAGE_MESSAGE: Final = ( "conduct-litellm-guard is required for the Conduct guardrail. " 'Install it with: pip install "conduct-litellm-guard>=0.2.4"' ) - try: from conduct_litellm_guard import ConductGuard as ConductGuardrail - from conduct_litellm_guard.guardrail import ( - ConductGuardBlocked as ConductGuardrailBlocked, - ) - from conduct_litellm_guard.guardrail import GuardDecision - - _import_error: ImportError | None = None -except ImportError as _err: +except ImportError as import_error: + _import_error: Final = import_error class ConductGuardrail(CustomGuardrail): - """Stub used when ``conduct-litellm-guard`` isn't installed. - - Exposes the class-level surface LiteLLM's guardrail registry - scans at load time — ``SUPPORTED_EVENT_HOOKS`` + - ``get_supported_event_hooks`` — so the registry doesn't crash - when this hook is discovered without the runtime dependency. - ``initialize_guardrail`` calls ``raise_if_missing_package`` - before ever constructing this class, so users see a friendly - ``pip install`` error rather than the stub silently activating. - """ - - SUPPORTED_EVENT_HOOKS: ClassVar[tuple[GuardrailEventHooks, ...]] = (GuardrailEventHooks.pre_call,) - - @classmethod - def get_supported_event_hooks(cls) -> list[GuardrailEventHooks]: - return list(cls.SUPPORTED_EVENT_HOOKS) # mutable-ok: LiteLLM registry expects a fresh list - - ConductGuardrailBlocked = None - GuardDecision = None - _import_error = _err + def __init__(self, **kwargs: object) -> None: # kwargs-ok: mirrors the plugin constructor, only raises + raise ImportError(MISSING_PACKAGE_MESSAGE) from _import_error -def raise_if_missing_package() -> None: - """Called by ``initialize_guardrail`` before constructing the class. - - Surfaces the friendly ``pip install`` error at the actionable moment - (config load) rather than silently dropping the hook or letting the - stub run. - """ - if _import_error is not None: - raise ImportError(_import_error_message) from _import_error - - -__all__ = [ # mutable-ok: standard Python re-export list - "ConductGuardrail", - "ConductGuardrailBlocked", - "GuardDecision", - "raise_if_missing_package", -] +__all__ = ["MISSING_PACKAGE_MESSAGE", "ConductGuardrail"] # mutable-ok: standard Python re-export list diff --git a/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_conduct.py b/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_conduct.py new file mode 100644 index 00000000000..7303c3ff721 --- /dev/null +++ b/tests/test_litellm/proxy/guardrails/guardrail_hooks/test_conduct.py @@ -0,0 +1,141 @@ +from __future__ import annotations + +import importlib.util +from typing import Final + +import pytest + +import litellm +from litellm.integrations.custom_guardrail import CustomGuardrail +from litellm.proxy.guardrails.guardrail_hooks.conduct import ( + DEFAULT_TIMEOUT_SECONDS, + ConductGuardrail, + initialize_guardrail, +) +from litellm.proxy.guardrails.guardrail_registry import ( + guardrail_class_registry, + guardrail_initializer_registry, +) +from litellm.types.guardrails import Guardrail, GuardrailEventHooks, LitellmParams + +PACKAGE_INSTALLED: Final = importlib.util.find_spec("conduct_litellm_guard") is not None + + +class _RecordingGuardrail(CustomGuardrail): + """Stand-in with the ``conduct_litellm_guard.ConductGuard`` class contract.""" + + @classmethod + def get_supported_event_hooks(cls) -> list[GuardrailEventHooks]: + return [GuardrailEventHooks.pre_call] + + def __init__( + self, + *, + api_url: str | None = None, + agent_token: str | None = None, + workspace_id: str | None = None, + fail_mode: str = "fail_closed", + tool_name: str = "llm_call", + timeout: float = 8.0, + **kwargs: object, + ) -> None: + super().__init__(**kwargs) # pyright: ignore[reportArgumentType] # CustomGuardrail.__init__ is untyped + self.api_url = api_url + self.agent_token = agent_token + self.workspace_id = workspace_id + self.fail_mode = fail_mode + self.tool_name = tool_name + self.timeout = timeout + + +def _params(mode: str = "pre_call", **extras: object) -> LitellmParams: + return LitellmParams(guardrail="conduct", mode=mode, api_key="cond_agt_test", **extras) + + +def _guardrail(litellm_params: LitellmParams) -> Guardrail: + return Guardrail(guardrail_name="conduct-guard", litellm_params=litellm_params) + + +def _init(litellm_params: LitellmParams) -> _RecordingGuardrail: + callback: Final = initialize_guardrail( + litellm_params, _guardrail(litellm_params), guardrail_cls=_RecordingGuardrail + ) + assert isinstance(callback, _RecordingGuardrail) + return callback + + +@pytest.fixture(autouse=True) +def _isolate_callbacks(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(litellm, "callbacks", []) + + +def test_discovered_by_guardrail_registry() -> None: + assert guardrail_initializer_registry["conduct"] is initialize_guardrail + assert guardrail_class_registry["conduct"] is ConductGuardrail + + +def test_maps_typed_fields_and_extras_onto_plugin_kwargs() -> None: + callback: Final = _init( + _params( + api_base="https://guard.example.test", + unreachable_fallback="fail_open", + timeout="3", + workspace_id="ws_123", + tool_name="workflow", + default_on=True, + ) + ) + + assert callback.api_url == "https://guard.example.test" + assert callback.agent_token == "cond_agt_test" + assert callback.fail_mode == "fail_open" + assert callback.timeout == 3.0 + assert callback.workspace_id == "ws_123" + assert callback.tool_name == "workflow" + assert callback.guardrail_name == "conduct-guard" + assert callback.event_hook == "pre_call" + assert callback.default_on is True + assert litellm.callbacks == [callback] + + +def test_defaults_when_optional_config_is_omitted() -> None: + callback: Final = _init(_params()) + + assert callback.fail_mode == "fail_closed" + assert callback.timeout == DEFAULT_TIMEOUT_SECONDS + assert callback.workspace_id is None + assert callback.tool_name == "llm_call" + + +@pytest.mark.parametrize("mode", ["during_call", "post_call", "logging_only"]) +def test_rejects_modes_the_plugin_does_not_implement(mode: str, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("LITELLM_STRICT_GUARDRAIL_MODES", raising=False) + + with pytest.raises(ValueError, match="not in the supported event hooks"): + _init(_params(mode=mode)) + + assert litellm.callbacks == [] + + +@pytest.mark.skipif(PACKAGE_INSTALLED, reason="exercises the missing-package fallback") +def test_missing_package_fails_at_config_load_with_install_hint() -> None: + litellm_params: Final = _params() + + with pytest.raises(ImportError, match="pip install"): + initialize_guardrail(litellm_params, _guardrail(litellm_params)) + + assert litellm.callbacks == [] + + +@pytest.mark.skipif(not PACKAGE_INSTALLED, reason="needs conduct-litellm-guard") +def test_plugin_class_enforces_supported_modes() -> None: + assert ConductGuardrail.get_supported_event_hooks() == [GuardrailEventHooks.pre_call] + + accepted: Final = _params() + callback: Final = initialize_guardrail(accepted, _guardrail(accepted)) + assert callback.supported_event_hooks == [GuardrailEventHooks.pre_call] + assert litellm.callbacks == [callback] + + rejected: Final = _params(mode="during_call") + with pytest.raises(ValueError, match="not in the supported event hooks"): + initialize_guardrail(rejected, _guardrail(rejected)) diff --git a/tests/test_litellm/proxy/guardrails/test_conduct_guardrail.py b/tests/test_litellm/proxy/guardrails/test_conduct_guardrail.py deleted file mode 100644 index d0ce0ee45d7..00000000000 --- a/tests/test_litellm/proxy/guardrails/test_conduct_guardrail.py +++ /dev/null @@ -1,210 +0,0 @@ -"""Tests for the Conduct guardrail integration. - -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 -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. 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.", -) - - -def test_import_module() -> None: - module = importlib.import_module("litellm.proxy.guardrails.guardrail_hooks.conduct") - assert module.ConductGuardrail is not None - - -def test_class_is_custom_guardrail_subclass() -> None: - from litellm.integrations.custom_guardrail import CustomGuardrail - from litellm.proxy.guardrails.guardrail_hooks.conduct import ConductGuardrail - - assert issubclass(ConductGuardrail, CustomGuardrail) - - -def test_enum_value_registered() -> None: - from litellm.types.guardrails import SupportedGuardrailIntegrations - - assert SupportedGuardrailIntegrations.CONDUCT.value == "conduct" - - -def test_registries_populated() -> None: - from litellm.proxy.guardrails.guardrail_hooks.conduct import ( - guardrail_class_registry, - guardrail_initializer_registry, - ) - - assert "conduct" in guardrail_class_registry - 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. - Since plugin 0.2.4 the supported-hooks contract lives on - ``ConductGuard`` in the plugin package itself; the LiteLLM shim - is a pure alias, so we verify against the alias. LiteLLM's - registry calls ``.value`` on each entry, so the hooks must be - ``GuardrailEventHooks`` enum members, not bare strings.""" - 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: - monkeypatch.setenv("CONDUCT_AGENT_TOKEN", "cond_agt_test_placeholder") - - added_callbacks: list[object] = [] - fake_manager = SimpleNamespace( - add_litellm_callback=lambda cb: added_callbacks.append(cb), - ) - - import litellm - - monkeypatch.setattr(litellm, "logging_callback_manager", fake_manager) - - from litellm.proxy.guardrails.guardrail_hooks.conduct import ( - ConductGuardrail, - initialize_guardrail, - ) - - litellm_params = SimpleNamespace( - api_base=None, - api_key=None, - mode="pre_call", - default_on=True, - ) - guardrail = MagicMock() - guardrail.get.return_value = "conduct-guard" - - callback = initialize_guardrail(litellm_params, guardrail) - - assert isinstance(callback, ConductGuardrail) - assert added_callbacks == [callback] - - -def test_initialize_prefers_typed_unreachable_fallback( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """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 - - captured: dict = {} - - class _FakeGuard: - def __init__(self, **kwargs: object) -> None: - captured.update(kwargs) - - 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)) - - 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_initialize() -> 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 check is now - deferred to :func:`raise_if_missing_package` which - ``initialize_guardrail`` calls at config-load time when actionable.""" - from litellm.proxy.guardrails.guardrail_hooks.conduct import conduct as _mod - - original_error = _mod._import_error - try: - _mod._import_error = ImportError("simulated missing package") - with pytest.raises(ImportError, match="pip install"): - _mod.raise_if_missing_package() - finally: - _mod._import_error = original_error - - -def test_raise_if_missing_package_is_noop_when_present() -> None: - """The check should be silent when ``conduct-litellm-guard`` is - importable (the normal case for anyone who ``pip install``ed it).""" - from litellm.proxy.guardrails.guardrail_hooks.conduct.conduct import ( - raise_if_missing_package, - ) - - assert raise_if_missing_package() is None