diff --git a/agent_assembly/adapters/langchain/callback_handler.py b/agent_assembly/adapters/langchain/callback_handler.py index 76a5eee4..caf9a1fa 100644 --- a/agent_assembly/adapters/langchain/callback_handler.py +++ b/agent_assembly/adapters/langchain/callback_handler.py @@ -30,6 +30,16 @@ class _FallbackBaseCallbackHandler: class AssemblyCallbackHandler(_CallbackHandlerBase): # type: ignore[valid-type,misc] """Callback handler that delegates runtime events to governance interception.""" + # LangChain's CallbackManager LOGS-AND-SWALLOWS an exception raised inside a + # callback when ``raise_error`` is False (its inherited default), then runs the + # tool anyway. When a user wires this handler the idiomatic way + # (``callbacks=[handler]``), that would let a policy DENY be silently bypassed — + # ``on_tool_start`` raises ``ToolExecutionBlockedError`` but the denied tool still + # executes, with only a log line as trace (AAASM-4658). Setting this True makes + # LangChain propagate the block instead of swallowing it, so a DENY aborts the + # tool call as governance requires. + raise_error: bool = True + _UNKNOWN_DECISION_REASON = "Unrecognized governance decision; denied under enforce." def __init__(self, interceptor: Any) -> None: diff --git a/test/unit/adapters/langchain/test_callback_raise_error_enforcement.py b/test/unit/adapters/langchain/test_callback_raise_error_enforcement.py new file mode 100644 index 00000000..49d2e44e --- /dev/null +++ b/test/unit/adapters/langchain/test_callback_raise_error_enforcement.py @@ -0,0 +1,55 @@ +"""Regression test for AAASM-4658 — the ``raise_error`` swallow bypass. + +``AssemblyCallbackHandler.on_tool_start`` raises ``ToolExecutionBlockedError`` on a +policy DENY, but LangChain's ``CallbackManager`` only *propagates* an exception +raised inside a callback when the handler's ``raise_error`` flag is True; with the +inherited default of False it logs the exception and runs the tool anyway. A user +who wires the handler the idiomatic LangChain way (``callbacks=[handler]``) would +then have a denied tool execute regardless — governance silently bypassed. + +This exercises that real dispatch path (not a direct ``on_tool_start`` call) with +the actual LangChain ``CallbackManager``, so it fails when ``raise_error`` is False +and passes once it is True. It needs the real base class, so it is guarded by +``importorskip`` and runs only under the ``langchain-test`` dependency group: + + uv sync --group langchain-test + .venv/bin/python -m pytest \ + test/unit/adapters/langchain/test_callback_raise_error_enforcement.py +""" + +from __future__ import annotations + +import pytest + +pytest.importorskip("langchain_core") + +from langchain_core.tools import tool # noqa: E402 + +from agent_assembly.adapters.langchain import AssemblyCallbackHandler # noqa: E402 +from agent_assembly.exceptions import ToolExecutionBlockedError # noqa: E402 + + +class _DenyInterceptor: + """Interceptor that denies every tool call, as an enforce-mode policy would.""" + + def check_tool_start(self, **_kwargs: object) -> dict[str, str]: + return {"status": "deny", "reason": "execute_sql denied by policy"} + + +def test_denied_tool_wired_as_callback_is_blocked_not_executed() -> None: + executed: list[str] = [] + + @tool + def execute_sql(query: str) -> str: + """Run a SQL query (must never run when policy denies it).""" + executed.append(query) + return "rows" + + handler = AssemblyCallbackHandler(_DenyInterceptor()) + + # Attach the handler the idiomatic LangChain way rather than calling + # ``on_tool_start`` directly — this is the path AAASM-4658 reproduces. + with pytest.raises(ToolExecutionBlockedError): + execute_sql.invoke({"query": "SELECT 1"}, config={"callbacks": [handler]}) + + assert executed == [], "denied tool executed despite governance DENY" diff --git a/test/unit/adapters/langchain/test_getattr_contract_with_langchain.py b/test/unit/adapters/langchain/test_getattr_contract_with_langchain.py index 0cbd0f39..120a7586 100644 --- a/test/unit/adapters/langchain/test_getattr_contract_with_langchain.py +++ b/test/unit/adapters/langchain/test_getattr_contract_with_langchain.py @@ -87,9 +87,13 @@ class attribute lookup to the base class (or to the four methods "run_inline", ) -# The four contract methods AssemblyCallbackHandler defines itself; the remaining -# contract members must resolve to a ``langchain_core`` class. -_OVERRIDDEN_METHODS: frozenset[str] = frozenset({"on_tool_start", "on_tool_end", "on_llm_start", "on_llm_end"}) +# Contract members AssemblyCallbackHandler defines itself; the remaining members +# must resolve to a ``langchain_core`` class. The four tool/llm methods carry the +# governance dispatch; ``raise_error`` is overridden to True so LangChain propagates +# a DENY instead of swallowing it (AAASM-4658). +_OVERRIDDEN_MEMBERS: frozenset[str] = frozenset( + {"on_tool_start", "on_tool_end", "on_llm_start", "on_llm_end", "raise_error"} +) class _ExplodingInterceptor: @@ -144,7 +148,7 @@ def test_contract_members_resolve_statically_not_via_getattr() -> None: assert static_value is not None owner = _defining_class(name) - if name in _OVERRIDDEN_METHODS: + if name in _OVERRIDDEN_MEMBERS: assert owner is AssemblyCallbackHandler else: assert owner.__module__.startswith( @@ -156,7 +160,9 @@ def test_contract_members_resolve_statically_not_via_getattr() -> None: for name in _CONTRACT_EVENT_METHODS: assert callable(getattr(handler, name)) for name in _CONTRACT_FLAGS: - assert getattr(handler, name) is False + # ``raise_error`` is intentionally True (AAASM-4658); the rest are False. + expected = name == "raise_error" + assert getattr(handler, name) is expected assert spy.accessed == [], f"__getattr__ delegated contract members it must not: {spy.accessed}"