diff --git a/agent/shell_hooks.py b/agent/shell_hooks.py index 687af5ec4ba..6639700b553 100644 --- a/agent/shell_hooks.py +++ b/agent/shell_hooks.py @@ -83,6 +83,7 @@ logger = logging.getLogger(__name__) DEFAULT_TIMEOUT_SECONDS = 60 MAX_TIMEOUT_SECONDS = 300 ALLOWLIST_FILENAME = "shell-hooks-allowlist.json" +_DEFAULT_BLOCK_MESSAGE = "Blocked by shell hook." # (event, matcher, command) triples that have been wired to the plugin # manager in the current process. Matcher is part of the key because @@ -515,10 +516,12 @@ def _parse_response(event: str, stdout: str) -> Optional[Dict[str, Any]]: if event == "pre_tool_call": if data.get("action") == "block": - message = data.get("message") or data.get("reason") or "Blocked by shell hook." + raw = data.get("message") or data.get("reason") + message = raw if isinstance(raw, str) and raw else _DEFAULT_BLOCK_MESSAGE return {"action": "block", "message": message} if data.get("decision") == "block": - message = data.get("reason") or data.get("message") or "Blocked by shell hook." + raw = data.get("reason") or data.get("message") + message = raw if isinstance(raw, str) and raw else _DEFAULT_BLOCK_MESSAGE return {"action": "block", "message": message} return None diff --git a/tests/agent/test_shell_hooks.py b/tests/agent/test_shell_hooks.py index 088c23eb466..743c9acb843 100644 --- a/tests/agent/test_shell_hooks.py +++ b/tests/agent/test_shell_hooks.py @@ -100,6 +100,30 @@ class TestParseResponse: ) assert r is None + def test_block_action_without_message_uses_default(self): + """Block is honored even when message/reason is absent.""" + r = shell_hooks._parse_response("pre_tool_call", '{"action": "block"}') + assert r == {"action": "block", "message": shell_hooks._DEFAULT_BLOCK_MESSAGE} + + def test_block_decision_without_reason_uses_default(self): + """Block is honored even when reason/message is absent.""" + r = shell_hooks._parse_response("pre_tool_call", '{"decision": "block"}') + assert r == {"action": "block", "message": shell_hooks._DEFAULT_BLOCK_MESSAGE} + + def test_block_action_empty_message_uses_default(self): + """Empty string message falls back to default, not empty string.""" + r = shell_hooks._parse_response( + "pre_tool_call", '{"action": "block", "message": ""}', + ) + assert r == {"action": "block", "message": shell_hooks._DEFAULT_BLOCK_MESSAGE} + + def test_block_action_non_string_message_uses_default(self): + """Non-string message (e.g. integer) falls back to default.""" + r = shell_hooks._parse_response( + "pre_tool_call", '{"action": "block", "message": 42}', + ) + assert r == {"action": "block", "message": shell_hooks._DEFAULT_BLOCK_MESSAGE} + # ── _serialize_payload ────────────────────────────────────────────────────