From 5e50f18b3041067b42af7b9a5a6778d55e93fdfb Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 9 Jul 2026 20:52:44 -0700 Subject: [PATCH] fix(agent): reject malformed tool call arguments (#61784) * fix(agent): reject malformed tool call arguments * test(agent): expect malformed tool arguments to fail closed --- agent/tool_executor.py | 70 ++++++++++--- .../test_malformed_tool_arguments.py | 98 +++++++++++++++++++ tests/run_agent/test_run_agent.py | 9 +- 3 files changed, 157 insertions(+), 20 deletions(-) create mode 100644 tests/run_agent/test_malformed_tool_arguments.py diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 9688f5d3dc6..822ec7b84cd 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -74,6 +74,25 @@ _MAX_TOOL_WORKERS = 8 _DEFAULT_CONCURRENT_TOOL_TIMEOUT_S = 420.0 +def _parse_tool_arguments(raw_arguments: Any) -> tuple[dict, Optional[str]]: + """Parse model-emitted arguments without repairing or coercing them.""" + try: + arguments = json.loads(raw_arguments) + except (json.JSONDecodeError, TypeError): + arguments = None + if isinstance(arguments, dict): + return arguments, None + return {}, json.dumps( + { + "error": "Invalid tool arguments", + "message": ( + "Tool arguments must be a valid JSON object; tool was not executed." + ), + }, + ensure_ascii=False, + ) + + def _resolve_concurrent_tool_timeout() -> float | None: raw = os.getenv("HERMES_CONCURRENT_TOOL_TIMEOUT_S", "").strip() if not raw: @@ -337,19 +356,29 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe for tool_call in tool_calls: function_name = tool_call.function.name - # Reset nudge counters + function_args, malformed_args_result = _parse_tool_arguments( + tool_call.function.arguments + ) + + if malformed_args_result is not None: + parsed_calls.append( + ( + tool_call, + function_name, + function_args, + [], + malformed_args_result, + False, + ) + ) + continue + + # Reset nudge counters only for a structurally valid invocation. if function_name == "memory": agent._turns_since_memory = 0 elif function_name == "skill_manage": agent._iters_since_skill = 0 - try: - function_args = json.loads(tool_call.function.arguments) - except json.JSONDecodeError: - function_args = {} - if not isinstance(function_args, dict): - function_args = {} - # ── Tool Search unwrap ──────────────────────────────────────── # When the model invokes the tool_call bridge, peel it open so # every downstream check (checkpointing, guardrails, plugin @@ -990,13 +1019,24 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe function_name = tool_call.function.name - try: - function_args = json.loads(tool_call.function.arguments) - except json.JSONDecodeError as e: - logger.warning(f"Unexpected JSON error after validation: {e}") - function_args = {} - if not isinstance(function_args, dict): - function_args = {} + function_args, malformed_args_result = _parse_tool_arguments( + tool_call.function.arguments + ) + if malformed_args_result is not None: + messages.append( + make_tool_result_message( + function_name, + malformed_args_result, + tool_call.id, + ) + ) + _flush_session_db_after_tool_progress( + agent, + messages, + stage=f"invalid tool arguments {function_name}", + ) + agent._apply_pending_steer_to_tool_results(messages, 1) + continue # Tool Search unwrap — see execute_tool_calls_concurrent for full # rationale, including the scope gate (the unwrap dispatches the diff --git a/tests/run_agent/test_malformed_tool_arguments.py b/tests/run_agent/test_malformed_tool_arguments.py new file mode 100644 index 00000000000..6736182d304 --- /dev/null +++ b/tests/run_agent/test_malformed_tool_arguments.py @@ -0,0 +1,98 @@ +"""Malformed model tool arguments are rejected at the dispatch boundary.""" + +import json +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +from run_agent import AIAgent + + +def _make_agent() -> AIAgent: + tool_defs = [ + { + "type": "function", + "function": { + "name": "web_search", + "description": "search", + "parameters": {"type": "object", "properties": {}}, + }, + } + ] + with ( + patch("run_agent.get_tool_definitions", return_value=tool_defs), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("hermes_cli.config.load_config", return_value={}), + patch("run_agent.OpenAI"), + ): + agent = AIAgent( + api_key="test-key-1234567890", + base_url="https://openrouter.ai/api/v1", + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + ) + agent.client = MagicMock() + agent.tool_delay = 0 + agent._flush_messages_to_session_db = MagicMock() + return agent + + +def _tool_call(call_id: str, arguments: str): + return SimpleNamespace( + id=call_id, + type="function", + function=SimpleNamespace(name="web_search", arguments=arguments), + ) + + +@pytest.mark.parametrize("dispatch_mode", ["sequential", "concurrent"]) +@pytest.mark.parametrize( + "bad_arguments", + [ + pytest.param("not-json", id="malformed-json"), + pytest.param('"scalar"', id="scalar"), + pytest.param("[]", id="list"), + pytest.param("", id="empty"), + pytest.param('{"query": "cut off', id="truncated"), + ], +) +def test_malformed_arguments_are_rejected_without_blocking_valid_sibling( + dispatch_mode: str, + bad_arguments: str, +): + agent = _make_agent() + assistant_message = SimpleNamespace( + content="", + tool_calls=[ + _tool_call("call-bad", bad_arguments), + _tool_call("call-good", '{"query": "valid"}'), + ], + ) + messages = [] + executed = [] + + def fake_dispatch(name, args, task_id, *positional, **kwargs): + call_id = kwargs.get("tool_call_id") or (positional[0] if positional else None) + executed.append((name, args, call_id)) + return json.dumps({"ok": args["query"]}) + + with ( + patch("run_agent.handle_function_call", side_effect=fake_dispatch), + patch.object(agent, "_invoke_tool", side_effect=fake_dispatch), + patch( + "agent.tool_executor.maybe_persist_tool_result", + side_effect=lambda **kwargs: kwargs["content"], + ), + ): + execute = getattr(agent, f"_execute_tool_calls_{dispatch_mode}") + execute(assistant_message, messages, "task-1") + + assert executed == [("web_search", {"query": "valid"}, "call-good")] + assert [message["tool_call_id"] for message in messages] == ["call-bad", "call-good"] + assert len([message for message in messages if message["tool_call_id"] == "call-bad"]) == 1 + + assert '"error": "Invalid tool arguments"' in messages[0]["content"] + assert "JSON object" in messages[0]["content"] + assert json.loads(messages[1]["content"]) == {"ok": "valid"} diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 171a7c11255..dda0b66c621 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -2333,7 +2333,7 @@ class TestExecuteToolCalls: or "interrupted" in messages[0]["content"].lower() ) - def test_invalid_json_args_defaults_empty(self, agent): + def test_invalid_json_args_are_rejected_without_dispatch(self, agent): tc = _mock_tool_call( name="web_search", arguments="not valid json", call_id="c1" ) @@ -2341,13 +2341,12 @@ class TestExecuteToolCalls: messages = [] with patch("run_agent.handle_function_call", return_value="ok") as mock_hfc: agent._execute_tool_calls(mock_msg, messages, "task-1") - # Invalid JSON args should fall back to empty dict - args, kwargs = mock_hfc.call_args - assert args[:3] == ("web_search", {}, "task-1") - assert set(kwargs.get("enabled_tools", [])) == agent.valid_tool_names + mock_hfc.assert_not_called() assert len(messages) == 1 assert messages[0]["role"] == "tool" assert messages[0]["tool_call_id"] == "c1" + assert "valid json object" in messages[0]["content"].lower() + assert "tool was not executed" in messages[0]["content"].lower() def test_result_truncation_over_100k(self, agent, tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))