Files
hermes-agent/tests/tools/test_computer_use_approval_isolation.py
teknium1 3e066dfedd fix(computer_use): approval goes through the shared gate; no callback now fails closed
computer_use kept its own approval decision: two module dicts
(_session_auto_approve / _always_allow) mirroring tools.approval's
session store and _persist_choice, a private verdict vocabulary
(approve_once/approve_session/always_approve) that hermes_cli mapped
back to once/session/always, and — the real problem — `if
_approval_callback is None: return None`. Only the interactive CLI ever
installed that callback, so every other host (gateway turns, cron,
api_server, tui_gateway, ACP) ran destructive desktop input with no
approval at all, ignoring cron_mode / unattended_mode / the permanent
allowlist, and "always" grants were invisible to `is_approved`,
`clear_session` and the messaging-platform approval buttons.

_request_approval now calls tools.approval._run_approval_gate with
pattern_key `cua:<action>:<background|foreground>` (the old scope shape,
so a background grant still never covers the visible foreground variant)
and fail_closed_when_no_human=True, the same posture as
request_tool_approval / the SSH-config write gate. The private dicts,
their release/atexit clearing, the verdict mapping in
hermes_cli/cli_modal_mixin.py and the extra callback install in cli.py
are deleted: the CLI's terminal_tool callback answers computer_use
prompts like any other tool. set_approval_callback stays as an optional
explicit-callback hook with the shared callback contract
(cb(command, description, **kw) -> once|session|always|deny|timeout);
no in-tree host uses it.

Behavior change:
- No approval callback and no gateway (cron, api_server/webhook,
  headless -q, plain library use): destructive actions are now REFUSED
  with a BLOCKED error and never reach the backend. Previously they
  silently ran. cron honors approvals.cron_mode, unattended platforms
  approvals.unattended_mode, -q approvals.single_query_mode.
- --yolo / gateway /yolo / approvals.mode: off still allow (unchanged).
- Gateway sessions (Telegram/Discord/Slack/...) now get a real pending
  approval with once/session/always buttons instead of default-allow.
- session/always grants live in tools.approval's store; "always" is one
  command_allowlist entry (`cua:click:background`) and is scoped to that
  action+mode — the old blanket "always_approve unlocks everything for
  the session" no longer exists.
- Denial wording is the shared gate's ("BLOCKED: User denied ...",
  "BLOCKED: Action timed out ..."); the error JSON keeps `action`.

Tests: tests/tools/test_computer_use_approval_isolation.py
::test_no_callback_refuses_unless_yolo (blocked + no backend call, then
yolo executes) and ::test_always_grant_lands_in_the_shared_store
(is_approved sees the cua:<action>:<mode> key; second call served from
the store). Sabotage: restoring the `callback is None -> allow`
short-circuit fails the first; swapping the shared gate for a private
grant set fails the second plus the three delivery-ladder scope tests.
tests/tools/conftest.py gains `grant_computer_use_approvals` for
dispatch tests that only care about routing.
2026-09-13 05:21:02 -07:00

131 lines
5.4 KiB
Python

"""computer_use approval is the shared ``tools.approval`` gate — no private grant store, no default-allow.
Two contracts:
* With nobody able to answer (no interactive CLI, no gateway, yolo off) a destructive action is REFUSED and
never reaches the backend; under yolo it runs. Historically the tool default-allowed whenever no CLI callback
was wired, which made every headless host (cron, api_server, tui_gateway, gateway turns) run desktop input
ungated.
* A grant answered through computer_use lives in ``tools.approval``'s store under computer_use's own scope key,
so ``is_approved`` sees it and ``clear_session`` retires it like any terminal pattern.
A leaked callback still poisons later tests (a raising one becomes deny, a blocking one hangs), so the autouse
reset in ``tests/conftest.py`` stays and the polluter/observer pair below keeps proving it.
"""
import json
import pytest
def _install_backend(cu_tool):
class _RecordingBackend:
def __init__(self):
self.calls = []
def start(self):
pass
def stop(self):
pass
def is_available(self):
return True
def click(self, **kw):
self.calls.append(("click", kw))
from tools.computer_use.backend import ActionResult
return ActionResult(ok=True, action="click")
def capture(self, mode="som", app=None):
from tools.computer_use.backend import CaptureResult
return CaptureResult(
mode=mode, width=1, height=1, png_b64=None, elements=[],
app="X", window_title="",
)
backend = _RecordingBackend()
cu_tool.reset_backend_for_tests()
cu_tool._backend = backend
return backend
@pytest.fixture
def _nobody_to_ask(monkeypatch):
"""No interactive CLI, no gateway, no per-thread terminal callback, yolo off."""
from tools import approval
for name in ("HERMES_INTERACTIVE", "HERMES_GATEWAY_SESSION", "HERMES_EXEC_ASK", "HERMES_YOLO_MODE"):
monkeypatch.delenv(name, raising=False)
monkeypatch.setattr(approval, "_YOLO_MODE_FROZEN", False)
monkeypatch.setattr("tools.terminal_tool._get_approval_callback", lambda: None)
yield
def test_no_callback_refuses_unless_yolo(_nobody_to_ask, monkeypatch):
"""Fail closed: with no human reachable the click is blocked and the backend sees nothing; yolo lets it run."""
from tools import approval
from tools.computer_use import tool as cu_tool
backend = _install_backend(cu_tool)
result = json.loads(cu_tool.handle_computer_use({"action": "click", "element": 3}))
assert result["error"].startswith("BLOCKED"), result
assert result["action"] == "click"
assert backend.calls == []
monkeypatch.setattr(approval, "_YOLO_MODE_FROZEN", True)
result = cu_tool.handle_computer_use({"action": "click", "element": 3})
assert [name for name, _ in backend.calls] == ["click"], result
def test_always_grant_lands_in_the_shared_store(monkeypatch):
"""One grant store: an "always" answered through computer_use is what ``tools.approval.is_approved`` reports
for the same session and ``cua:<action>:<mode>`` key, and the next call is served from that store."""
from tools import approval
from tools.approval_context import reset_current_session_key, set_current_session_key
from tools.computer_use import tool as cu_tool
monkeypatch.setenv("HERMES_INTERACTIVE", "1")
monkeypatch.setattr(approval, "_YOLO_MODE_FROZEN", False)
monkeypatch.setattr(approval, "save_permanent_allowlist", lambda patterns: None)
prompts = []
cu_tool.set_approval_callback(lambda command, description, **kw: prompts.append(command) or "always")
token = set_current_session_key("cua-grant-session")
try:
assert not approval.is_approved("cua-grant-session", "cua:click:background")
assert cu_tool._request_approval("click", {"element": 3}) is None
assert approval.is_approved("cua-grant-session", "cua:click:background")
assert cu_tool._request_approval("click", {"element": 3}) is None
assert len(prompts) == 1
finally:
cu_tool.set_approval_callback(None)
reset_current_session_key(token)
approval.clear_session("cua-grant-session")
with approval._lock:
approval._permanent_set().discard("cua:click:background")
def test_a_forgets_a_poisoned_approval_callback():
"""Simulates the polluter: installs a raising callback and deliberately does not reset it."""
from tools.computer_use import tool as cu_tool
def poisoned(command, description, **kw):
raise RuntimeError("dead UI")
cu_tool.set_approval_callback(poisoned)
# no reset — the autouse fixture must clean this up
def test_b_still_dispatches_after_the_polluter(monkeypatch):
"""Answers through the per-thread terminal callback only. The explicit computer_use callback takes precedence
in the shared gate, so if the polluter's raising one had leaked, this click would be denied."""
from tools.computer_use import tool as cu_tool
monkeypatch.setenv("HERMES_INTERACTIVE", "1")
monkeypatch.setattr("tools.terminal_tool._get_approval_callback", lambda: lambda command, description, **kw: "once")
backend = _install_backend(cu_tool)
result = cu_tool.handle_computer_use({"action": "click", "element": 3})
assert [name for name, _ in backend.calls] == ["click"], f"leaked approval callback poisoned this test: {result!r}"