Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions model_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -1157,6 +1157,44 @@ def _emit_post_tool_call_hook(
logger.debug("post_tool_call hook error: %s", _hook_err)


def _validate_tool_args_against_schema(name: str, args: Any) -> Optional[str]:
"""Reject a tool call missing schema-``required`` arguments (fail-open).

Only *key absence* of ``parameters.required`` fields counts as invalid —
the same contract as ``tools.tool_search.validate_deferred_call_args``.
No type/null checking: ``coerce_tool_args`` already repairs types
downstream. Returns a JSON error string when a required arg is missing,
``None`` when the call should dispatch. Never blocks a legitimate call on
a validator bug.
"""
if not isinstance(args, dict):
return None
try:
from tools.registry import registry as _registry

schema = _registry.get_schema(name)
if not isinstance(schema, dict):
return None
params = schema.get("parameters")
if not isinstance(params, dict):
return None
required = params.get("required")
if not isinstance(required, list) or not required:
return None
missing = [r for r in required if isinstance(r, str) and r not in args]
if not missing:
return None
return tool_error(
f"tool_call to '{name}' is missing required argument(s): "
f"{', '.join(missing)}. The tool was NOT invoked.",
effect_disposition="not_started",
retryable=True,
)
except Exception: # pragma: no cover — never block dispatch on validator bugs
logger.debug("schema validation for %s failed (fail-open)", name, exc_info=True)
return None


def handle_function_call(
function_name: str,
function_args: Dict[str, Any],
Expand Down Expand Up @@ -1338,6 +1376,13 @@ def _return_bridge_result(result: Any) -> Any:
if function_name in _AGENT_LOOP_TOOLS:
return tool_error(f"{function_name} must be handled by the agent loop")

# Validate the (coerced + middleware-transformed) args against the
# tool's schema before any authorization or side effect (TL-01). A
# tool with required fields used to execute with {}.
_schema_err = _validate_tool_args_against_schema(function_name, function_args)
if _schema_err is not None:
return _schema_err

# Check plugin hooks for a block/approve directive (unless caller
# already checked — e.g. run_agent._invoke_tool passes skip=True to
# avoid double-firing the hook).
Expand Down
68 changes: 60 additions & 8 deletions tests/test_model_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,58 @@ def test_agent_loop_tool_returns_error(self):
assert "error" in result
assert "agent loop" in result["error"].lower()

def test_missing_required_arg_is_rejected_before_dispatch(self, monkeypatch):
"""A tool call missing a schema-required argument must not execute."""
from tools.registry import registry as _reg

_reg.register(
name="_tl01_required_tool",
toolset="test",
schema={
"name": "_tl01_required_tool",
"description": "d",
"parameters": {
"type": "object",
"properties": {"x": {"type": "string"}},
"required": ["x"],
},
},
handler=lambda **kw: "RAN",
)
dispatched = []
monkeypatch.setattr(
_reg, "dispatch", lambda name, args, **kw: dispatched.append(name) or "RAN"
)
try:
out = json.loads(
handle_function_call(
"_tl01_required_tool",
{},
task_id="t",
session_id="s",
tool_call_id="c",
skip_pre_tool_call_hook=True,
)
)
assert dispatched == [] # never dispatched
assert "NOT invoked" in out["error"]
assert "x" in out["error"]
assert out["effect_disposition"] == "not_started"

# A valid call still dispatches.
ok = handle_function_call(
"_tl01_required_tool",
{"x": "hi"},
task_id="t",
session_id="s",
tool_call_id="c",
skip_pre_tool_call_hook=True,
)
assert ok == "RAN"
assert dispatched == ["_tl01_required_tool"]
finally:
_reg.deregister("_tl01_required_tool")

def test_unknown_tool_returns_error(self):
result = json.loads(handle_function_call("totally_fake_tool_xyz", {}))
assert "error" in result
Expand All @@ -43,7 +95,7 @@ def test_post_tool_call_receives_non_negative_integer_duration_ms(self):
patch("hermes_cli.plugins.has_hook", return_value=True),
patch("hermes_cli.plugins.invoke_hook") as mock_invoke_hook,
):
handle_function_call("web_search", {"q": "test"}, task_id="t1")
handle_function_call("web_search", {"query": "test"}, task_id="t1")

kwargs_by_hook = {
c.args[0]: c.kwargs for c in mock_invoke_hook.call_args_list
Expand Down Expand Up @@ -90,7 +142,7 @@ def test_no_listener_skips_post_and_transform_emit(self):
patch("hermes_cli.plugins.has_hook", return_value=False),
patch("hermes_cli.plugins.invoke_hook") as mock_invoke_hook,
):
result = handle_function_call("web_search", {"q": "test"}, task_id="t1")
result = handle_function_call("web_search", {"query": "test"}, task_id="t1")

assert result == '{"ok":true}'
fired = {c.args[0] for c in mock_invoke_hook.call_args_list}
Expand Down Expand Up @@ -135,16 +187,16 @@ def fake_dispatch(tool_name, args, **kwargs):
result = json.loads(
handle_function_call(
"web_search",
{"q": "test"},
{"query": "test"},
task_id="task-1",
tool_call_id="tool-1",
session_id="session-1",
)
)

assert seen["execution_args"] == {"q": "test", "rewritten": True}
assert seen["dispatch"][1] == {"q": "test", "rewritten": True, "wrapped": True}
assert result["args"] == {"q": "test", "rewritten": True, "wrapped": True}
assert seen["execution_args"] == {"query": "test", "rewritten": True}
assert seen["dispatch"][1] == {"query": "test", "rewritten": True, "wrapped": True}
assert result["args"] == {"query": "test", "rewritten": True, "wrapped": True}
expected_trace = [{"source": "test-middleware", "reason": "rewrite"}]
pre_call = next(call for call in hook_calls if call[0] == "pre_tool_call")
post_call = next(call for call in hook_calls if call[0] == "post_tool_call")
Expand All @@ -170,7 +222,7 @@ def test_registry_exception_emits_terminal_tool_hook(self, monkeypatch):
result = json.loads(
handle_function_call(
"web_search",
{"q": "test"},
{"query": "test"},
task_id="task-1",
session_id="session-1",
tool_call_id="tool-1",
Expand Down Expand Up @@ -288,7 +340,7 @@ def fake_invoke_hook(hook_name, **kwargs):
monkeypatch.setattr("tools.file_tools.notify_other_tool_call",
lambda task_id: notifications.append(task_id))

result = json.loads(handle_function_call("web_search", {"q": "test"}, task_id="t1"))
result = json.loads(handle_function_call("web_search", {"query": "test"}, task_id="t1"))
assert result == {"error": "Blocked"}
assert notifications == []

Expand Down
8 changes: 7 additions & 1 deletion tests/test_sanitize_tool_error.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,14 @@ def boom(_args, **_kwargs):
target = all_tools[0]
original = _registry._tools[target].handler
_registry._tools[target].handler = boom
# Satisfy the schema's required fields (if any) so the call reaches the
# handler instead of being rejected pre-dispatch by schema validation.
schema = _registry.get_schema(target)
params = (schema or {}).get("parameters") or {}
required = params.get("required") or []
args = {r: "" for r in required}
try:
result_str = handle_function_call(target, {})
result_str = handle_function_call(target, args)
finally:
_registry._tools[target].handler = original

Expand Down
Loading