diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index d57abef08f1c5..bc0fabb2ac4be 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -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. diff --git a/tests/tools/test_noninteractive_approval_mode.py b/tests/tools/test_noninteractive_approval_mode.py index 578d0a0faef5c..474722c83bbef 100644 --- a/tests/tools/test_noninteractive_approval_mode.py +++ b/tests/tools/test_noninteractive_approval_mode.py @@ -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 diff --git a/tools/approval.py b/tools/approval.py index c844af4d8b38e..c30a8ac1e48b9 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -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 --- diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index 47d63fbe4c8dc..7ba85122302f6 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -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. :::