From 93cf678beaffcd555b45700d0ffc3b86d6e1d9b7 Mon Sep 17 00:00:00 2001 From: Chisanan232 Date: Tue, 7 Jul 2026 11:56:47 +0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20(openai-agents):=20Fix=20=5Fis?= =?UTF-8?q?=5Fgovernance=5Ferror=20to=20maintain=20fail-closed=20posture?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The _is_governance_error() function unconditionally returned True, causing ALL exceptions during governance checks to be swallowed. This made the system fail-open instead of fail-closed, a security vulnerability. Now only AssemblyError subclasses (GovernanceError, PolicyError, etc.) are caught and result in fail-open behavior. Non-governance errors (RuntimeError, ValueError, network errors, etc.) propagate up, blocking the tool call. Closes AAASM-4252 Co-Authored-By: Claude Opus 4.5 --- .../adapters/openai_agents/patch.py | 10 ++++- .../unit/adapters/openai_agents/test_patch.py | 41 +++++++++++++++++-- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/agent_assembly/adapters/openai_agents/patch.py b/agent_assembly/adapters/openai_agents/patch.py index 3f3e5249..41c3f269 100644 --- a/agent_assembly/adapters/openai_agents/patch.py +++ b/agent_assembly/adapters/openai_agents/patch.py @@ -435,8 +435,14 @@ async def _record_async_tool_result( def _is_governance_error(error: Exception) -> bool: - del error - return True + """Check if error is a governance-specific error that should be handled. + + Non-governance errors (network, malformed response, etc.) should propagate + to maintain fail-closed security posture. + """ + from agent_assembly.exceptions import AssemblyError + + return isinstance(error, AssemblyError) def _apply_function_tool_patch(function_tool_cls: type[Any], callback_handler: Any) -> None: diff --git a/test/unit/adapters/openai_agents/test_patch.py b/test/unit/adapters/openai_agents/test_patch.py index bc7bb100..e85ef64a 100644 --- a/test/unit/adapters/openai_agents/test_patch.py +++ b/test/unit/adapters/openai_agents/test_patch.py @@ -280,9 +280,15 @@ async def record_result(self, **kwargs: object) -> None: @pytest.mark.asyncio -async def test_gateway_error_uses_fail_open_and_executes_original_tool( +async def test_non_governance_error_propagates_fail_closed( monkeypatch: pytest.MonkeyPatch, ) -> None: + """Non-governance errors (RuntimeError, ValueError, etc.) must propagate. + + This ensures fail-closed security posture: if an unexpected error occurs + during governance checks, the tool call is blocked rather than allowed + through. See AAASM-4252. + """ function_tool_cls = _install_fake_openai_agents_module(monkeypatch) class Interceptor: @@ -293,6 +299,34 @@ async def check_tool_start(self, **kwargs: object) -> dict[str, str]: patcher = openai_patch.OpenAIAgentsPatch(callback_handler=Interceptor()) assert patcher.apply() is True + tool = function_tool_cls(name="fail_closed_tool") + with pytest.raises(RuntimeError, match="gateway unavailable"): + await tool.on_invoke_tool(SimpleNamespace(agent_id="agent-fail-closed"), '{"x": 2}') + + +@pytest.mark.asyncio +async def test_governance_error_allows_fail_open( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """AssemblyError subclasses are handled gracefully (fail-open). + + Governance-specific errors (e.g., transient gateway issues represented as + GatewayError) are caught and the tool executes. This is intentional: if + the governance system itself has a known error, we fail-open rather than + blocking all tool calls. See AAASM-4252. + """ + from agent_assembly.exceptions import GatewayError + + function_tool_cls = _install_fake_openai_agents_module(monkeypatch) + + class Interceptor: + async def check_tool_start(self, **kwargs: object) -> dict[str, str]: + del kwargs + raise GatewayError("gateway temporarily unavailable") + + patcher = openai_patch.OpenAIAgentsPatch(callback_handler=Interceptor()) + assert patcher.apply() is True + tool = function_tool_cls(name="fail_open_tool") result = await tool.on_invoke_tool(SimpleNamespace(agent_id="agent-fail-open"), '{"x": 2}') @@ -300,9 +334,10 @@ async def check_tool_start(self, **kwargs: object) -> dict[str, str]: @pytest.mark.asyncio -async def test_non_governance_error_is_reraised( +async def test_value_error_propagates_fail_closed( monkeypatch: pytest.MonkeyPatch, ) -> None: + """ValueError (non-governance) must propagate for fail-closed posture.""" function_tool_cls = _install_fake_openai_agents_module(monkeypatch) class Interceptor: @@ -310,8 +345,6 @@ async def check_tool_start(self, **kwargs: object) -> dict[str, str]: del kwargs raise ValueError("unexpected") - monkeypatch.setattr(openai_patch, "_is_governance_error", lambda _error: False) - patcher = openai_patch.OpenAIAgentsPatch(callback_handler=Interceptor()) assert patcher.apply() is True