diff --git a/agent/shell_hooks.py b/agent/shell_hooks.py index 424d2d9e6d..ca7ac6d6b0 100644 --- a/agent/shell_hooks.py +++ b/agent/shell_hooks.py @@ -445,6 +445,15 @@ def _parse_pre_tool_call(data: Dict[str, Any]) -> Optional[Dict[str, Any]]: for verb, _, _, payload in _PRE_TOOL_DIALECTS: if data.get(verb) == "modify" and isinstance(data.get(payload), dict): return {"action": "modify", "args": data[payload]} + # Hermes-only escalation to the human-approval gate (#92553). Claude-Code's ``decision: + # approve`` means auto-ALLOW, so it is deliberately not mapped onto this. + if data.get("action") == "approve": + directive: Dict[str, Any] = {"action": "approve"} + for key in ("message", "rule_key"): + value = data.get(key) + if isinstance(value, str) and value.strip(): + directive[key] = value.strip() + return directive return None diff --git a/tests/agent/test_shell_hooks.py b/tests/agent/test_shell_hooks.py index ab376c5b8a..e2238b448a 100644 --- a/tests/agent/test_shell_hooks.py +++ b/tests/agent/test_shell_hooks.py @@ -49,7 +49,18 @@ class TestParseResponse: ) assert r == {"action": "block", "message": "nope"} - + @pytest.mark.parametrize("stdout, expected", [ + ('{"action": "approve", "message": " needs a human ", "rule_key": " terminal:rm "}', + {"action": "approve", "message": "needs a human", "rule_key": "terminal:rm"}), + ('{"action": "approve", "message": "", "rule_key": 7}', {"action": "approve"}), + # Claude-Code's ``decision: approve`` means auto-ALLOW, not "ask a human": never mapped. + ('{"decision": "approve", "reason": "ok"}', None), + ('{"action": "approve", "decision": "block", "reason": "no"}', {"action": "block", "message": "no"}), + ]) + def test_approve_is_parsed_like_the_plugin_directive(self, stdout, expected): + """The documented ``approve`` action used to parse to None, so the tool ran with no + approval prompt (#92553). It now yields the same shape Python plugins return.""" + assert shell_hooks._parse_response("pre_tool_call", stdout) == expected def test_empty_stdout_returns_none(self): assert shell_hooks._parse_response("pre_tool_call", "") is None @@ -201,6 +212,32 @@ class TestCallbackSubprocess: ) assert msg == "blocked-by-shell" + def test_approve_reaches_the_human_gate_through_plugin_manager(self, tmp_path, monkeypatch): + """End to end: a shell hook's approve directive escalates to request_tool_approval with its + message and rule_key, and the gate's denial blocks the tool (#92553).""" + from hermes_cli import plugins + + script = _write_script( + tmp_path, "approve.sh", + "#!/usr/bin/env bash\n" + 'printf \'{"action": "approve", "message": "risky", "rule_key": "terminal:rm"}\\n\'\n', + ) + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + monkeypatch.setenv("HERMES_ACCEPT_HOOKS", "1") + plugins._plugin_manager = plugins.PluginManager() + cfg = {"hooks": {"pre_tool_call": [{"matcher": "terminal", "command": str(script)}]}} + assert len(shell_hooks.register_from_config(cfg, accept_hooks=True)) == 1 + + seen = [] + + def _gate(tool_name, reason, **kwargs): + seen.append((tool_name, reason, kwargs.get("rule_key"))) + return {"approved": False, "message": "denied by human"} + + monkeypatch.setattr("tools.approval.request_tool_approval", _gate) + assert plugins.resolve_pre_tool_block("terminal", {"command": "rm"}) == "denied by human" + assert seen == [("terminal", "risky", "terminal:rm")] + def test_matcher_regex_filters_callback(self, tmp_path, monkeypatch): """A matcher set to 'terminal' must not fire for 'web_search'.""" calls = tmp_path / "calls.log" diff --git a/website/docs/user-guide/features/hooks.md b/website/docs/user-guide/features/hooks.md index 7303ef3f03..dba7b42013 100644 --- a/website/docs/user-guide/features/hooks.md +++ b/website/docs/user-guide/features/hooks.md @@ -1729,6 +1729,10 @@ profile's `HERMES_HOME`. `tool_name` and `tool_input` are `null` for non-tool ev {"action": "modify", "args": {"new_string": "fixed content"}} // Hermes-canonical {"decision": "modify", "tool_input": {"new_string": "fixed content"}} // Claude-Code style +// Escalate a pre_tool_call to the human-approval gate (Hermes-only; `message` and `rule_key` +// are optional). Claude-Code's `{"decision": "approve"}` means auto-allow and is NOT mapped here: +{"action": "approve", "message": "Why approval is required", "rule_key": "optional:scope"} + // Inject context for pre_llm_call: {"context": "Today is Friday, 2026-04-17"}