mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-12 13:52:15 +00:00
feat(plugins): pre_tool_call approve action escalates to human gate
Extend the pre_tool_call plugin hook return contract with a new directive:
{"action": "approve", "message": "why this needs human confirmation"}
Previously a pre_tool_call hook could only veto a tool call (action: block)
or allow it silently. It could not escalate to the existing human-approval
flow. This unlocks user-defined runtime approval rules on ANY tool (HTTP
writes, file writes to sensitive paths, email sends), enforced at runtime —
resolving #51221 as a pure plugin, with no core approval.py rule schema.
Mechanism:
- get_pre_tool_call_directive() returns (action, message) for block|approve;
get_pre_tool_call_block_message() kept as a block-only back-compat shim.
- resolve_pre_tool_block() is the single dispatch-site chokepoint: fetches
the directive and, for approve, invokes the human gate; fail-closed to a
block on denial, timeout, or gate exception. ALL FOUR tool-dispatch sites
now call it: tool_executor (concurrent + sequential), agent_runtime_helpers,
and model_tools.handle_function_call.
- request_tool_approval() escalates via the SAME machinery as Tier-2
dangerous commands: session/permanent allowlist, prompt_dangerous_approval
(CLI) / submit_pending (gateway), [o]nce/[s]ession/[a]lways/[d]eny,
timeout fail-closed, approvals.cron_mode for cron contexts.
Architecture: extracted the shared decision core into _run_approval_gate(),
called by BOTH check_dangerous_command() and request_tool_approval() so the
fail-closed / cron / gateway / yolo / persist policy lives in ONE place and
cannot drift. Fixed a latent divergence — the plugin path now honors --yolo.
Approval grain: [a]lways is keyed on tool_name + a hash of the reason (an
explicit plugin rule_key overrides), so distinct reasons on the same tool
persist independently instead of one 'always' blanketing the whole tool.
Non-interactive: cron honors approvals.cron_mode (parity with commands); any
other non-interactive non-gateway context fails CLOSED for the plugin path
(the command path keeps its historical fail-open default, unchanged).
No new config schema, no new env vars, no new hook events.
This commit is contained in:
parent
1388cd1c0c
commit
f512d6f020
9 changed files with 666 additions and 103 deletions
|
|
@ -859,6 +859,115 @@ class TestPreToolCallBlocking:
|
|||
assert get_pre_tool_call_block_message("terminal", {}) == "first blocker"
|
||||
|
||||
|
||||
class TestPreToolCallDirective:
|
||||
"""Tests for the extended (block | approve) directive helper."""
|
||||
|
||||
def test_approve_directive_returned(self, monkeypatch):
|
||||
from hermes_cli.plugins import get_pre_tool_call_directive
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.plugins.invoke_hook",
|
||||
lambda hook_name, **kwargs: [
|
||||
{"action": "approve", "message": "needs human ok"}
|
||||
],
|
||||
)
|
||||
assert get_pre_tool_call_directive("write_file", {}) == (
|
||||
"approve", "needs human ok")
|
||||
|
||||
def test_approve_without_message_is_valid(self, monkeypatch):
|
||||
"""approve may omit a message (block may not)."""
|
||||
from hermes_cli.plugins import get_pre_tool_call_directive
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.plugins.invoke_hook",
|
||||
lambda hook_name, **kwargs: [{"action": "approve"}],
|
||||
)
|
||||
assert get_pre_tool_call_directive("write_file", {}) == ("approve", None)
|
||||
|
||||
def test_block_still_requires_message(self, monkeypatch):
|
||||
from hermes_cli.plugins import get_pre_tool_call_directive
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.plugins.invoke_hook",
|
||||
lambda hook_name, **kwargs: [{"action": "block"}],
|
||||
)
|
||||
assert get_pre_tool_call_directive("terminal", {}) == (None, None)
|
||||
|
||||
def test_first_directive_wins_across_actions(self, monkeypatch):
|
||||
from hermes_cli.plugins import get_pre_tool_call_directive
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.plugins.invoke_hook",
|
||||
lambda hook_name, **kwargs: [
|
||||
{"action": "approve", "message": "gate first"},
|
||||
{"action": "block", "message": "block second"},
|
||||
],
|
||||
)
|
||||
assert get_pre_tool_call_directive("terminal", {}) == (
|
||||
"approve", "gate first")
|
||||
|
||||
def test_shim_ignores_approve(self, monkeypatch):
|
||||
"""Back-compat shim only reports block, never approve."""
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.plugins.invoke_hook",
|
||||
lambda hook_name, **kwargs: [
|
||||
{"action": "approve", "message": "gate"}
|
||||
],
|
||||
)
|
||||
assert get_pre_tool_call_block_message("write_file", {}) is None
|
||||
|
||||
|
||||
class TestResolvePreToolBlock:
|
||||
"""Tests for the single dispatch-site chokepoint that resolves a
|
||||
directive (incl. the approve→gate escalation) to a block message."""
|
||||
|
||||
def test_block_returns_message(self, monkeypatch):
|
||||
from hermes_cli.plugins import resolve_pre_tool_block
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.plugins.invoke_hook",
|
||||
lambda hook_name, **kwargs: [{"action": "block", "message": "no"}],
|
||||
)
|
||||
assert resolve_pre_tool_block("terminal", {}) == "no"
|
||||
|
||||
def test_no_directive_returns_none(self, monkeypatch):
|
||||
from hermes_cli.plugins import resolve_pre_tool_block
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.plugins.invoke_hook", lambda hook_name, **kwargs: [])
|
||||
assert resolve_pre_tool_block("terminal", {}) is None
|
||||
|
||||
def test_approve_denied_blocks(self, monkeypatch):
|
||||
from hermes_cli.plugins import resolve_pre_tool_block
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.plugins.invoke_hook",
|
||||
lambda hook_name, **kwargs: [{"action": "approve", "message": "why"}],
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"tools.approval.request_tool_approval",
|
||||
lambda *a, **k: {"approved": False, "message": "user denied it"},
|
||||
)
|
||||
assert resolve_pre_tool_block("write_file", {}) == "user denied it"
|
||||
|
||||
def test_approve_granted_allows(self, monkeypatch):
|
||||
from hermes_cli.plugins import resolve_pre_tool_block
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.plugins.invoke_hook",
|
||||
lambda hook_name, **kwargs: [{"action": "approve", "message": "why"}],
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"tools.approval.request_tool_approval",
|
||||
lambda *a, **k: {"approved": True, "message": None},
|
||||
)
|
||||
assert resolve_pre_tool_block("write_file", {}) is None
|
||||
|
||||
def test_approve_gate_exception_fails_closed(self, monkeypatch):
|
||||
from hermes_cli.plugins import resolve_pre_tool_block
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.plugins.invoke_hook",
|
||||
lambda hook_name, **kwargs: [{"action": "approve", "message": "why"}],
|
||||
)
|
||||
def _boom(*a, **k):
|
||||
raise RuntimeError("gate crashed")
|
||||
monkeypatch.setattr("tools.approval.request_tool_approval", _boom)
|
||||
msg = resolve_pre_tool_block("terminal", {})
|
||||
assert msg is not None and "gate failed" in msg # fail-closed
|
||||
|
||||
|
||||
class TestGetPreVerifyContinueMessage:
|
||||
"""`pre_verify` directive aggregation — mirrors the pre_tool_call block path."""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue