[AAASM-4658] 🐛 (adapters): Set raise_error so LangChain enforces policy deny#262
Conversation
LangChain's CallbackManager logs-and-swallows an exception raised in a callback when raise_error is False (its inherited default), then runs the tool anyway. So AssemblyCallbackHandler wired the idiomatic way (callbacks=[handler]) let a policy DENY be silently bypassed: on_tool_start raised ToolExecutionBlockedError but the denied tool still executed, with only a log line as trace. Setting raise_error=True makes LangChain propagate the block so a DENY aborts the tool call. Refs AAASM-4658 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_019NafDDcehs2ez4Ax9kuWfi
Attaches AssemblyCallbackHandler the idiomatic LangChain way (callbacks=[handler]) on a real @tool and asserts a denied tool raises ToolExecutionBlockedError and never runs its body — the AAASM-4658 path that a direct on_tool_start call does not exercise. Guarded by importorskip so it runs under the langchain-test dependency group. Refs AAASM-4658 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_019NafDDcehs2ez4Ax9kuWfi
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
🤖 Claude Code — PR reviewCI state: GREEN — 19/19 checks pass, including the full CI unit+integration runs, the Scope coverage: Fully covers AAASM-4658's Side-effect assessment: Safe for existing callers.
FE: No FE-app change; adapter library code only. Playwright validation N/A. Verdict: READY to merge (pending required review approval). Correctly closes the HIGH-severity governance-bypass on the idiomatic LangChain callback path. |



Description
AssemblyCallbackHandler.on_tool_startraisesToolExecutionBlockedErroron a policy DENY, but LangChain'sCallbackManageronly propagates an exception raised inside a callback when the handler'sraise_errorflag is True. The handler inherited LangChain's default ofraise_error = False, so LangChain logged the exception and ran the tool anyway.The consequence: a user who wires the handler the idiomatic LangChain way (
callbacks=[handler]) had a policy DENY silently bypassed — the denied tool (execute_sql,run_shell_command, …) still executed and returned a result, with only a single log line as trace. This defeats the product's core promise ("every tool call … allowed, denied, or recorded") on the LangChain callback path. The shipped example avoided the bug only by callingon_tool_startdirectly, which a real user wrapping anAgentExecutorcannot do.Fix: set the class attribute
raise_error = TrueonAssemblyCallbackHandler(one line + rationale comment) so LangChain propagates the block instead of swallowing it. Minimal surface; distinct from the AAASM-4014/4034__getattr__delegation fix.Type of Change
Breaking Changes
Governance now enforces a DENY on the idiomatic callback path where it previously (incorrectly) did not — a security correction, not an API change.
Related Issues
Testing
New regression test
test/unit/adapters/langchain/test_callback_raise_error_enforcement.pyattaches the handler the idiomatic way (callbacks=[handler]) on a real@tooland asserts a denied tool raisesToolExecutionBlockedErrorand never runs its body — the exact AAASM-4658 path that a directon_tool_startcall does not exercise. It is guarded byimportorskipand runs under thelangchain-testdependency group.Verified the reproduction: with
raise_error=False(pre-fix) the denied tool executes and returns a result; with the fix it is blocked. The existingtest_getattr_contract_with_langchain.pyinvariant (all contract flags resolve statically;raise_errornow intentionally overridden to True) was reconciled in the fix commit to keep every commit bisectable.Local runs (with
uv sync --group langchain-test):pytest test/unit/adapters/langchain/-> 84 passedpytest test/unit-> 728 passed, 4 skippedpre-commit run(isort / autoflake / black / mypy) -> all passedChecklist
🤖 Generated with Claude Code
https://claude.ai/code/session_019NafDDcehs2ez4Ax9kuWfi