refactor(hermes_cli): approvals_suggest/test, approval_mode/transport — json helper, _deny helper, packed layout

hermes_cli/approval_mode.py

@@ -37,24 +37,14 @@ def run_approval_mode_command(requested_mode: Optional[str]) -> ApprovalModeResu
     requested = (requested_mode or "").strip().lower()
 
     if not requested:
-        return ApprovalModeResult(
+        return ApprovalModeResult(True, current, False, f"Approval mode: {current} (persistent profile setting).")
-            True,
-            current,
-            False,
-            f"Approval mode: {current} (persistent profile setting).",
-        )
     if requested not in VALID_APPROVAL_MODES:
-        return ApprovalModeResult(
+        return ApprovalModeResult(False, current, False, "Usage: /approvals [manual|smart|off]")
-            False,
-            current,
-            False,
-            "Usage: /approvals [manual|smart|off]",
-        )
 
-    # set_config_value is the canonical managed-scope/write-safety chokepoint.
+    # set_config_value is the canonical managed-scope/write-safety chokepoint. It reports managed
-    # It reports managed policy through stderr + SystemExit, and the fail-closed
+    # policy through stderr + SystemExit, and the fail-closed write guard raises RuntimeError on an
-    # write guard raises RuntimeError on an unparseable config.yaml; capture both
+    # unparseable config.yaml; capture both for slash-command output instead of terminating the
-    # for slash-command output instead of terminating the interactive worker.
+    # interactive worker.
     from hermes_cli.config import set_config_value
 
     output = StringIO()
@@ -65,24 +55,14 @@ def run_approval_mode_command(requested_mode: Optional[str]) -> ApprovalModeResu
         detail = output.getvalue().strip() or "Approval mode is managed and cannot be changed."
         return ApprovalModeResult(False, current, False, detail)
     except Exception as exc:
-        return ApprovalModeResult(
+        return ApprovalModeResult(False, current, False, f"Failed to save approval mode: {exc}")
-            False,
-            current,
-            False,
-            f"Failed to save approval mode: {exc}",
-        )
 
     effective = _effective_mode()
     if effective != requested:
         return ApprovalModeResult(
-            False,
+            False, effective, False,
-            effective,
-            False,
             f"Approval mode remains {effective}; the requested value did not become effective.",
         )
     return ApprovalModeResult(
-        True,
+        True, effective, effective != current, f"Approval mode: {effective} (persistent profile setting).",
-        effective,
-        effective != current,
-        f"Approval mode: {effective} (persistent profile setting).",
     )

hermes_cli/approval_transport.py

@@ -25,9 +25,7 @@ _MAX_ACTIVE_TRANSPORT_WORKERS = 8
 _transport_worker_slots = threading.BoundedSemaphore(_MAX_ACTIVE_TRANSPORT_WORKERS)
 
 ApprovalChoice = Literal["once", "session", "always", "deny"]
-ApprovalPresentFn = Callable[
+ApprovalPresentFn = Callable[["ApprovalRequest"], "ApprovalDecision | Awaitable[ApprovalDecision]"]
-    ["ApprovalRequest"], "ApprovalDecision | Awaitable[ApprovalDecision]"
-]
 
 
 @dataclass(frozen=True)
@@ -81,9 +79,7 @@ class ApprovalRequest:
         )
         canonical = json.dumps({**fields, "session_key": session_key}, sort_keys=True, separators=(",", ":"))
         digest = hashlib.sha256(canonical.encode("utf-8")).hexdigest()
-        return cls(
+        return cls(**{**fields, "pattern_keys": pattern_keys, "allowed_choices": tuple(choices)}, digest=digest)
-            **{**fields, "pattern_keys": pattern_keys, "allowed_choices": tuple(choices)}, digest=digest
-        )
 
     def respond(self, choice: ApprovalChoice | str) -> ApprovalDecision:
         """Build the correlated response a transport should return."""
@@ -108,6 +104,10 @@ class RegisteredApprovalTransport:
     profile_home: str
 
 
+def _deny(failure: str) -> ApprovalTransportResult:
+    return ApprovalTransportResult("deny", failure)
+
+
 def invoke_approval_transport(
     present: ApprovalPresentFn,
     request: ApprovalRequest,
@@ -123,10 +123,9 @@ def invoke_approval_transport(
     loop. A callback must return before the host timeout; late results are discarded and cannot
     authorize another request.
     """
-
     if not _transport_worker_slots.acquire(blocking=False):
         logger.warning("Approval transport worker capacity exhausted")
-        return ApprovalTransportResult("deny", "busy")
+        return _deny("busy")
 
     results: queue.Queue[tuple[str, object, float]] = queue.Queue(maxsize=1)
     deadline = time.monotonic() + max(float(timeout_seconds), 0.0)
@@ -148,27 +147,23 @@ def invoke_approval_transport(
         finally:
             _transport_worker_slots.release()
 
-    worker = threading.Thread(
+    worker = threading.Thread(target=_run, name=f"approval-transport-{request.request_id[:8]}", daemon=True)
-        target=_run, name=f"approval-transport-{request.request_id[:8]}", daemon=True
-    )
     try:
         worker.start()
     except BaseException:
         _transport_worker_slots.release()
         logger.warning("Could not start approval transport worker")
-        return ApprovalTransportResult("deny", "error")
+        return _deny("error")
     while True:
         if is_interrupted is not None and is_interrupted():
             logger.info("Approval transport wait interrupted for %s", request.request_id)
-            return ApprovalTransportResult("deny", "interrupted")
+            return _deny("interrupted")
         remaining = deadline - time.monotonic()
         if remaining <= 0:
             logger.warning("Approval transport timed out for request %s", request.request_id)
-            return ApprovalTransportResult("deny", "timeout")
+            return _deny("timeout")
         try:
-            kind, value, completed_at = results.get(
+            kind, value, completed_at = results.get(timeout=min(max(float(poll_interval), 0.001), remaining))
-                timeout=min(max(float(poll_interval), 0.001), remaining)
-            )
             break
         except queue.Empty:
             if on_poll is not None:
@@ -178,9 +173,7 @@ def invoke_approval_transport(
                     logger.debug("Approval transport poll callback failed", exc_info=True)
 
     failure = _validate_decision(kind, value, completed_at, deadline, request)
-    if failure is not None:
+    return _deny(failure) if failure is not None else ApprovalTransportResult(value.choice)
-        return ApprovalTransportResult("deny", failure)
-    return ApprovalTransportResult(value.choice)
 
 
 def _validate_decision(kind, value, completed_at, deadline, request) -> str | None:

hermes_cli/approvals_suggest.py

@@ -2,12 +2,10 @@
 
 Hermes has no dedicated approval-decision ledger: ``always`` answers land in ``command_allowlist``
 (config.yaml) via :func:`tools.approval.save_permanent_allowlist`, while ``once``/``session``
-approvals are in-memory only.
+approvals are in-memory only. So this module mines *implied approvals*: a command that matches a
-
+dangerous-command class (the same :func:`tools.approval.detect_dangerous_command` classifier that
-So this module mines *implied approvals*: a command that matches a dangerous-command class (the same
+triggers the prompt) AND whose tool result is not a block/denial marker must have been approved by
-:func:`tools.approval.detect_dangerous_command` classifier that triggers the prompt) AND whose tool
+the user (once, session, always, smart-approve, or yolo) before it ran.
-result is not a block/denial marker must have been approved by the user (once, session, always,
-smart-approve, or yolo) before it ran.
 """
 
 from __future__ import annotations
@@ -25,12 +23,10 @@ from typing import Iterable, Iterator, Optional
 # Safety exclusions
 # ---------------------------------------------------------------------------
 
-# Dangerous-class descriptions matching ANY of these are never proposed,
+# Dangerous-class descriptions matching ANY of these are never proposed, regardless of approval
-# regardless of approval frequency.  Matched case-insensitively against the
+# frequency (matched case-insensitively against tools.approval's DANGEROUS_PATTERNS / execution-flag
-# pattern-key/description strings produced by tools.approval's
+# descriptions). Deliberately conservative: a benign class accidentally excluded costs the user
-# DANGEROUS_PATTERNS / execution-flag findings.  Deliberately conservative:
+# one manual config edit; a destructive class accidentally proposed costs them data.
-# a benign class accidentally excluded costs the user one manual config edit;
-# a destructive class accidentally proposed costs them data.
 _UNSAFE_CLASS_PATTERNS = [
     r"delete",              # recursive delete, find -delete, branch force delete, ...
     r"\brm\b",              # xargs with rm, find -exec rm
@@ -71,8 +67,8 @@ _UNSAFE_CLASS_PATTERNS = [
 ]
 _UNSAFE_CLASS_RE = re.compile("|".join(_UNSAFE_CLASS_PATTERNS), re.IGNORECASE)
 
-# Root binaries that must never anchor a proposed command glob, even if the
+# Root binaries that must never anchor a proposed command glob, even if the class survived the
-# class survived the description filter.  Prefix match for the mkfs family.
+# description filter. Prefix match for the mkfs family.
 _UNSAFE_ROOT_BINARIES = {
     "rm", "rmdir", "unlink", "shred", "dd", "fdisk", "parted", "wipefs",
     "sudo", "doas", "su", "chmod", "chown", "chgrp",
@@ -82,9 +78,8 @@ _UNSAFE_ROOT_BINARIES = {
 }
 _UNSAFE_ROOT_PREFIXES = ("mkfs",)
 
-# Substrings in a role='tool' result that mean the command did NOT execute
+# Substrings in a role='tool' result that mean the command did NOT execute with user consent
-# with user consent (blocked, denied, timed out, or still pending).  Kept in
+# (blocked, denied, timed out, or still pending). Kept in sync with tools/approval.py templates.
-# sync with the message templates in tools/approval.py.
 _BLOCK_MARKERS = (
     "BLOCKED (hardline)",
     "BLOCKED: User denied",
@@ -128,22 +123,23 @@ def default_db_path() -> Path:
 
 
 def _connect_readonly(db_path: Path) -> sqlite3.Connection:
-    uri = f"file:{db_path}?mode=ro"
+    return sqlite3.connect(f"file:{db_path}?mode=ro", uri=True)
-    return sqlite3.connect(uri, uri=True)
 
 
 def _fetch_rows(cur) -> Iterator[tuple]:
     """Stream cursor rows in 2000-row batches."""
-    while True:
+    while rows := cur.fetchmany(2000):
-        rows = cur.fetchmany(2000)
-        if not rows:
-            return
         yield from rows
 
 
-def _iter_terminal_calls(
+def _json_or_none(raw):
-    con: sqlite3.Connection, since_ts: float
+    try:
-) -> Iterator[tuple[str, str]]:
+        return json.loads(raw)
+    except (TypeError, ValueError):
+        return None
+
+
+def _iter_terminal_calls(con: sqlite3.Connection, since_ts: float) -> Iterator[tuple[str, str]]:
     """Yield ``(tool_call_id, command)`` for every terminal tool call."""
     cur = con.execute(
         "SELECT tool_calls FROM messages "
@@ -152,21 +148,13 @@ def _iter_terminal_calls(
         (since_ts,),
     )
     for (raw,) in _fetch_rows(cur):
-        try:
+        calls = _json_or_none(raw)
-            calls = json.loads(raw)
+        for call in calls if isinstance(calls, list) else ():
-        except (TypeError, ValueError):
+            fn = call.get("function") or {} if isinstance(call, dict) else {}
-            continue
-        if not isinstance(calls, list):
-            continue
-        for call in calls:
-            if not isinstance(call, dict):
-                continue
-            fn = call.get("function") or {}
             if fn.get("name") != "terminal":
                 continue
-            try:
+            args = _json_or_none(fn.get("arguments") or "{}")
-                args = json.loads(fn.get("arguments") or "{}")
+            if args is None:
-            except (TypeError, ValueError):
                 continue
             command = args.get("command")
             if isinstance(command, str) and command.strip():
@@ -188,12 +176,9 @@ def _blocked_tool_call_ids(con: sqlite3.Connection, since_ts: float) -> set:
     }
 
 
-def scan_approval_history(
+def scan_approval_history(db_path: Optional[Path] = None, days: int = 90) -> list[tuple[str, str]]:
-    db_path: Optional[Path] = None, days: int = 90
+    """``(command, dangerous_class_description)`` records for dangerous-classified terminal commands
-) -> list[tuple[str, str]]:
+    that actually executed (i.e. carried an implied user approval).
-    """Return ``(command, dangerous_class_description)`` records mined from the session DB —
-    dangerous-classified terminal commands that actually executed (i.e. carried an implied user
-    approval).
     """
     from tools.approval import detect_dangerous_command, detect_hardline_command
 
@@ -210,15 +195,13 @@ def scan_approval_history(
         for tool_call_id, command in _iter_terminal_calls(con, since_ts):
             if tool_call_id in blocked:
                 continue
-            is_hardline, _desc = detect_hardline_command(command)
+            # Hardline commands are unconditionally blocked at runtime; never mine them (defense in
-            if is_hardline:
+            # depth against stale DB rows).
-                # Hardline commands are unconditionally blocked at runtime;
+            if detect_hardline_command(command)[0]:
-                # never mine them (defense in depth against stale DB rows).
                 continue
             is_dangerous, _key, description = detect_dangerous_command(command)
-            if not is_dangerous:
+            if is_dangerous:
-                continue
+                records.append((command, description))
-            records.append((command, description))
     finally:
         con.close()
     return records
@@ -230,13 +213,9 @@ def scan_approval_history(
 
 def normalize_command(command: str) -> str:
     """Fold user/hermes home prefixes and collapse whitespace."""
-    from tools.approval import (
+    from tools.approval import _rewrite_resolved_hermes_home, _rewrite_resolved_user_home
-        _rewrite_resolved_hermes_home,
-        _rewrite_resolved_user_home,
-    )
 
-    folded = _rewrite_resolved_user_home(_rewrite_resolved_hermes_home(command))
+    return " ".join(_rewrite_resolved_user_home(_rewrite_resolved_hermes_home(command)).split())
-    return " ".join(folded.split())
 
 
 def is_unsafe_class(description: str) -> bool:
@@ -257,12 +236,8 @@ def derive_glob(normalized: str) -> Optional[str]:
     """
     from tools.approval import _has_allowlist_shell_operator
 
-    if _has_allowlist_shell_operator(normalized):
-        return None
     tokens = normalized.split()
-    if not tokens:
+    if _has_allowlist_shell_operator(normalized) or not tokens or _unsafe_root_binary(tokens[0]):
-        return None
-    if _unsafe_root_binary(tokens[0]):
         return None
     if len(tokens) == 1:
         return tokens[0]
@@ -292,19 +267,15 @@ def build_proposals(
             continue
         normalized = normalize_command(command)
         glob = derive_glob(normalized)
-        key = (glob, "glob") if glob is not None else (description, "class")
+        pattern, kind = (glob, "glob") if glob is not None else (description, "class")
-        pattern, kind = key
         if pattern in existing:
             continue
-        proposal = by_pattern.get(key)
+        proposal = by_pattern.setdefault((pattern, kind), Proposal(pattern=pattern, kind=kind))
-        if proposal is None:
-            proposal = by_pattern[key] = Proposal(pattern=pattern, kind=kind)
         proposal.count += 1
         proposal.classes.add(description)
         proposal.add_example(normalized)
 
-    ranked = [p for p in by_pattern.values() if p.count >= max(min_count, 1)]
+    ranked = sorted((p for p in by_pattern.values() if p.count >= max(min_count, 1)), key=lambda p: (-p.count, p.pattern))
-    ranked.sort(key=lambda p: (-p.count, p.pattern))
     return ranked[: max(limit, 1)]
 
 
@@ -336,12 +307,10 @@ def apply_proposals(proposals: list[Proposal], indices: list[int]) -> set:
     """Merge chosen proposal patterns into command_allowlist and persist."""
     import tools.approval as approval_module
 
-    merged = set(approval_module.load_permanent_allowlist())
+    merged = set(approval_module.load_permanent_allowlist()) | {proposals[idx].pattern for idx in indices}
-    for idx in indices:
-        merged.add(proposals[idx].pattern)
     approval_module.save_permanent_allowlist(merged)
-    # Keep the in-process allowlist consistent so a long-lived process sees
+    # Keep the in-process allowlist consistent so a long-lived process sees the new entries
-    # the new entries immediately (mirrors the interactive 'always' path).
+    # immediately (mirrors the interactive 'always' path).
     approval_module.load_permanent(merged)
     return merged
 
@@ -382,13 +351,13 @@ def suggest_command(args) -> int:
     import tools.approval as approval_module
 
     existing = set(approval_module.load_permanent_allowlist())
-    records = scan_approval_history(db_path, days=days)
     proposals = build_proposals(
-        records,
+        scan_approval_history(db_path, days=days),
         existing=existing,
         min_count=getattr(args, "min_count", 2),
         limit=getattr(args, "limit", 20),
     )
+    as_json = getattr(args, "json", False)
 
     apply_spec = getattr(args, "apply_indices", None)
     if apply_spec:
@@ -399,17 +368,16 @@ def suggest_command(args) -> int:
             return 1
         merged = apply_proposals(proposals, indices)
         applied = [proposals[i].pattern for i in indices]
-        if getattr(args, "json", False):
+        if as_json:
             print(json.dumps({"applied": applied, "allowlist_size": len(merged)}))
         else:
             print("Added to command_allowlist:")
             for pattern in applied:
                 print(f"  + {pattern}")
-            print(f"\ncommand_allowlist now has {len(merged)} entries "
+            print(f"\ncommand_allowlist now has {len(merged)} entries (~/.hermes/config.yaml).")
-                  "(~/.hermes/config.yaml).")
         return 0
 
-    if getattr(args, "json", False):
+    if as_json:
         payload = {
             "db": str(db_path),
             "days": days,

hermes_cli/approvals_test.py

@@ -7,7 +7,7 @@ order the runtime guard (``check_all_command_guards``) applies them:
 1. container-skip gate (isolated backends bypass all guards), 2. hardline blocklist (never
 bypassable, fires before yolo/off), 3. sudo-stdin guard (unconditional), 4. user ``approvals.deny``
 rules (fire before yolo/off), 5. yolo / ``approvals.mode: off`` bypass, 6. permanent
-``command_allowlist``, 7.
+``command_allowlist``, 7. dangerous-pattern detection (would prompt).
 """
 
 from __future__ import annotations
@@ -36,8 +36,8 @@ def evaluate_command(command: str, env_type: str = "local") -> dict:
     """
     import tools.approval as approval
 
-    # Sync config-persisted "always" patterns so the allowlist check below
+    # Sync config-persisted "always" patterns so the allowlist check below sees what the runtime
-    # sees what the runtime would see (load is read-only).
+    # would see (load is read-only).
     try:
         approval.load_permanent_allowlist()
     except Exception:
@@ -56,8 +56,7 @@ def evaluate_command(command: str, env_type: str = "local") -> dict:
             "normalized_variants": variants,
         }
 
-    # 1. Isolated container backends skip every guard (runtime parity:
+    # 1. Isolated container backends skip every guard (fires BEFORE the hardline floor at runtime).
-    #    this fires BEFORE the hardline floor in check_all_command_guards).
     if approval._should_skip_container_guards(env_type):
         return result(
             "allow",
@@ -77,10 +76,7 @@ def evaluate_command(command: str, env_type: str = "local") -> dict:
     # 3. Sudo stdin guard — unconditional, like the hardline floor.
     is_sudo_guess, sudo_desc = approval._check_sudo_stdin_guard(command)
     if is_sudo_guess:
-        return result(
+        return result("hardline-deny", rule=sudo_desc, detail="sudo stdin guard (unconditional block)")
-            "hardline-deny", rule=sudo_desc,
-            detail="sudo stdin guard (unconditional block)",
-        )
 
     # 4. User-defined approvals.deny rules — fire before yolo/off.
     deny_pattern = approval._match_user_deny_rule(command)
@@ -103,11 +99,7 @@ def evaluate_command(command: str, env_type: str = "local") -> dict:
 
     # 6. Permanent command_allowlist.
     if approval._command_matches_permanent_allowlist(command):
-        return result(
+        return result("allow", detail="matches command_allowlist in config.yaml (permanently approved)")
-            "allow",
-            detail="matches command_allowlist in config.yaml "
-                   "(permanently approved)",
-        )
 
     # 7. Dangerous-pattern detection → would prompt.
     is_dangerous, pattern_key, description = approval.detect_dangerous_command(command)
@@ -138,18 +130,14 @@ def _render_text(verdict: dict) -> None:
 def approvals_test_command(args) -> int:
     """Handle ``hermes approvals test <command...>``. Returns the exit code."""
     words = list(getattr(args, "command_words", None) or [])
-    # argparse REMAINDER keeps a leading "--" separator; it is not part of
+    # argparse REMAINDER keeps a leading "--" separator; it is not part of the command.
-    # the command being evaluated.
     if words and words[0] == "--":
         words = words[1:]
     if not words:
         print("usage: hermes approvals test [--env-type TYPE] [--json] -- <command...>")
         return EXIT_USAGE
-    command = " ".join(words)
-    env_type = getattr(args, "env_type", None) or "local"
-
-    verdict = evaluate_command(command, env_type=env_type)
 
+    verdict = evaluate_command(" ".join(words), env_type=getattr(args, "env_type", None) or "local")
     if getattr(args, "json", False):
         print(json.dumps(verdict, indent=2))
     else: