fix(approval): apply noninteractive_mode in the consolidated guard

Review found the knob had no effect on real terminal calls: terminal_tool
routes through check_all_command_guards, whose non-interactive early
return approves non-cron commands regardless of the setting. Only the
direct check_dangerous_command entry point was covered.

Applies the policy in the consolidated guard before that return, mirroring
the cron branch beside it — dangerous-pattern detection first, then the
tirith content check, including the tirith_fail_open handling so an
operator who opted into fail-closed is not silently allowed here either.

Also adds the two things the review asked for alongside it:

- approvals.noninteractive_mode in DEFAULT_CONFIG, so it participates in
  the documented deep-merge contract rather than existing only as a
  cfg_get default. New tests exercise the real config path (writing a
  config.yaml under HERMES_HOME) instead of mocking load_config, and
  assert sibling keys survive the merge.
- The key documented in website/docs/user-guide/configuration.md, which
  the first pass missed; security.md and the configuration reference in
  the skills tree were already updated.

Verified against a live install: with noninteractive_mode: deny and no
human present, `git reset --hard HEAD~3` now blocks through
check_all_command_guards where it was previously approved. A profile on
the default is unchanged. 24 tests in the new file, 105 across the
approval suite plus the config-contract tests.
This commit is contained in:
Rowland O'Connor 2026-08-02 07:24:46 +01:00
parent 8b869b2367
commit ca076eef8a
4 changed files with 181 additions and 0 deletions

View File

@ -1967,6 +1967,13 @@ DEFAULT_CONFIG = {
# deny — block the command and let the agent find another way (default, safe)
# approve — auto-approve all dangerous commands in cron jobs
#
# noninteractive_mode — the same choice for a run that is headless but NOT
# cron: a scripted invocation, hermes-agent embedded as a library, a test
# harness, a dispatched kanban worker. That path has always auto-approved
# everything below the hardline floor with nobody present to answer.
# approve — keep that behaviour (default, so an upgrade changes nothing)
# deny — block instead; operators opt in
#
# timeout — seconds to wait for the user's approve/deny before failing
# closed (deny). Shared by the CLI prompt and gateway/messaging waits.
# Messaging approvals arrive as a push notification the user may not see
@ -1976,6 +1983,7 @@ DEFAULT_CONFIG = {
"mode": "smart",
"timeout": 300,
"cron_mode": "deny",
"noninteractive_mode": "approve",
# Operator-customizable policy text for smart approvals. When
# non-empty, this is appended to the smart-approval guardian's
# SYSTEM prompt (trusted channel) as additional rules — e.g.

View File

@ -101,3 +101,97 @@ class TestNoninteractiveGate:
with _config({"noninteractive_mode": "deny", "cron_mode": "approve"}):
result = check_dangerous_command("git push --force origin main", "local")
assert result["approved"] is True
# ---------------------------------------------------------------------------
# the consolidated guard — the path terminal execution actually takes
# ---------------------------------------------------------------------------
class TestConsolidatedGuard:
"""`terminal_tool` routes through `check_all_command_guards`, not
`check_dangerous_command`. A policy that only covers the latter has no
effect on real terminal calls."""
def test_deny_blocks_through_the_consolidated_guard(self, headless):
from tools.approval import check_all_command_guards
with _config({"noninteractive_mode": "deny"}):
result = check_all_command_guards("git reset --hard HEAD~3", "local")
assert result["approved"] is False
assert "noninteractive_mode" in result["message"]
def test_approve_preserves_existing_behaviour_through_the_guard(self, headless):
from tools.approval import check_all_command_guards
with _config({"noninteractive_mode": "approve"}):
result = check_all_command_guards("git reset --hard HEAD~3", "local")
assert result["approved"] is True
def test_default_preserves_existing_behaviour_through_the_guard(self, headless):
from tools.approval import check_all_command_guards
with _config({}):
result = check_all_command_guards("git reset --hard HEAD~3", "local")
assert result["approved"] is True
def test_harmless_command_unaffected_through_the_guard(self, headless):
from tools.approval import check_all_command_guards
with _config({"noninteractive_mode": "deny"}):
result = check_all_command_guards("git status", "local")
assert result["approved"] is True
def test_cron_still_governed_by_cron_mode_through_the_guard(self, headless, monkeypatch):
from tools.approval import check_all_command_guards
monkeypatch.setenv("HERMES_CRON_SESSION", "1")
with _config({"noninteractive_mode": "deny", "cron_mode": "approve"}):
result = check_all_command_guards("git reset --hard HEAD~3", "local")
assert result["approved"] is True
# ---------------------------------------------------------------------------
# the real DEFAULT_CONFIG / deep-merge contract, not a mocked load_config
# ---------------------------------------------------------------------------
def _write_hermes_config(monkeypatch, tmp_path, config):
import yaml
home = tmp_path / "hermes-home"
home.mkdir(exist_ok=True)
monkeypatch.setenv("HERMES_HOME", str(home))
(home / "config.yaml").write_text(yaml.safe_dump(config), encoding="utf-8")
@pytest.fixture(autouse=True)
def _clear_config_cache():
"""load_config memoises; each case must read its own file."""
import hermes_cli.config as cfg
for attr in ("_CONFIG_CACHE", "_config_cache"):
if hasattr(cfg, attr):
setattr(cfg, attr, None)
yield
class TestDefaultConfigContract:
def test_key_has_a_canonical_default(self):
from hermes_cli.config import DEFAULT_CONFIG
assert DEFAULT_CONFIG["approvals"]["noninteractive_mode"] == "approve"
def test_absent_from_user_config_deep_merges_to_approve(self, monkeypatch, tmp_path):
"""A user file that sets only `mode` must still resolve the new key."""
_write_hermes_config(monkeypatch, tmp_path, {"approvals": {"mode": "manual"}})
assert _get_noninteractive_approval_mode() == "approve"
def test_user_config_overrides_the_default(self, monkeypatch, tmp_path):
_write_hermes_config(
monkeypatch, tmp_path, {"approvals": {"noninteractive_mode": "deny"}}
)
assert _get_noninteractive_approval_mode() == "deny"
def test_sibling_keys_survive_the_merge(self, monkeypatch, tmp_path):
"""Setting the new key must not drop cron_mode's default."""
from hermes_cli.config import load_config
_write_hermes_config(
monkeypatch, tmp_path, {"approvals": {"noninteractive_mode": "deny"}}
)
approvals = load_config()["approvals"]
assert approvals["noninteractive_mode"] == "deny"
assert approvals["cron_mode"] == "deny"
assert approvals["timeout"] == 300

View File

@ -3521,6 +3521,66 @@ def check_all_command_guards(command: str, env_type: str,
),
}
# else: tirith_fail_open is True — allow as before
elif _get_noninteractive_approval_mode() == "deny":
# Non-cron and still nobody present: a scripted or headless run, a
# dispatched worker, hermes-agent embedded as a library. Same
# reasoning as the cron branch above, under its own key — and this
# is the branch that governs real terminal calls, since
# terminal_tool routes here rather than through
# check_dangerous_command.
is_dangerous, _pk, description = detect_dangerous_command(command)
if is_dangerous:
return {
"approved": False,
"message": (
f"BLOCKED: Command flagged as dangerous ({description}) "
"but no interactive user or gateway is present to "
"approve it (approvals.noninteractive_mode: deny). "
"Find an alternative approach, or run this where a "
"human can answer."
),
}
# Content-level threats that the pattern detector does not match
# (homograph URLs, pipe-to-interpreter, terminal injection) are
# caught the same way cron catches them.
try:
from tools.tirith_security import check_command_security
_ni_tirith = check_command_security(command)
if _ni_tirith.get("action") in ("block", "warn"):
return {
"approved": False,
"message": (
f"BLOCKED: {_format_tirith_description(_ni_tirith)} "
"but no interactive user or gateway is present to "
"approve it (approvals.noninteractive_mode: deny). "
"Find an alternative approach, or run this where a "
"human can answer."
),
}
except ImportError:
# Mirrors the cron branch: an operator who opted into
# tirith fail-closed must not be silently allowed here either.
_ni_fail_open = True
try:
from hermes_cli.config import load_config as _load_cfg
_sec = (_load_cfg() or {}).get("security", {}) or {}
if _sec.get("tirith_enabled", True):
_ni_fail_open = _sec.get("tirith_fail_open", True)
except Exception:
pass
if not _ni_fail_open:
return {
"approved": False,
"message": (
"BLOCKED: the Tirith security scanner could not be "
"imported and security.tirith_fail_open is false, "
"so this command cannot be silently allowed — and "
"no interactive user or gateway is present to "
"approve it. Find an alternative approach, install "
"tirith, or set approvals.noninteractive_mode: "
"approve in config.yaml."
),
}
return {"approved": True, "message": None}
# --- Phase 1: Gather findings from both checks ---

View File

@ -2135,6 +2135,25 @@ approvals:
Smart mode is particularly useful for reducing approval fatigue — it lets the agent work more autonomously on safe operations while still catching genuinely destructive commands.
### Runs with nobody present
Every mode above assumes someone can answer. Two keys cover the runs where nobody can:
```yaml
approvals:
cron_mode: deny # deny | approve — cron jobs
noninteractive_mode: approve # deny | approve — other headless runs
```
| Key | Default | Applies to |
|---|---|---|
| `cron_mode` | `deny` | Scheduled jobs (`HERMES_CRON_SESSION`). |
| `noninteractive_mode` | `approve` | Any other run with no interactive CLI and no gateway: a scripted invocation, hermes-agent embedded as a library, a test harness, a dispatched kanban worker. |
`noninteractive_mode` defaults to `approve` because that is the historical behaviour — everything below the hardline floor is auto-approved when nobody is present. Set it to `deny` to block instead. It is opt-in so that upgrading does not start blocking a running deployment.
Both are evaluated after the hardline floor and `approvals.deny`, which block regardless of either setting, and neither affects sessions where a user or gateway *is* reachable.
:::warning
Setting `approvals.mode: off` disables all safety checks for terminal commands. Only use this in trusted, sandboxed environments.
:::