refactor(approval): compact leaf docstrings/comments by hand, keep every invariant (1375->1310)
This commit is contained in:
Teknium
+21
-36
@@ -2,8 +2,7 @@
|
||||
|
||||
Session identity and observability contextvars, the interactive/gateway/cron/
|
||||
unattended predicates, and the ``approvals.*`` config readers used by every
|
||||
gate in :mod:`tools.approval` (which re-exports all of them). No approval
|
||||
state or prompting lives here.
|
||||
gate in :mod:`tools.approval` (which re-exports all of them).
|
||||
"""
|
||||
|
||||
import contextvars
|
||||
@@ -19,23 +18,22 @@ def _ctx(name: str, default: "str | None" = "") -> contextvars.ContextVar:
|
||||
return contextvars.ContextVar(name, default=default)
|
||||
|
||||
|
||||
# Per-thread/per-task gateway session identity. Gateway runs agent turns
|
||||
# concurrently in executor threads, so a process-global env var is racy; the
|
||||
# env fallback stays for legacy single-threaded callers.
|
||||
# Per-thread/per-task gateway session identity: gateway runs agent turns
|
||||
# concurrently in executor threads, so a process-global env var is racy (the
|
||||
# env fallback stays for legacy single-threaded callers).
|
||||
_approval_session_key: contextvars.ContextVar[str] = _ctx("approval_session_key")
|
||||
_approval_turn_id: contextvars.ContextVar[str] = _ctx("approval_turn_id")
|
||||
_approval_tool_call_id: contextvars.ContextVar[str] = _ctx("approval_tool_call_id")
|
||||
# Hermes session id (observability identity, distinct from the gateway routing
|
||||
# session_key). Forwarded to approval hooks so observer plugins attach marks to
|
||||
# the REAL session scope — without it they fall back to a synthetic "default"
|
||||
# session_key), forwarded to approval hooks so observer plugins attach marks to
|
||||
# the REAL session scope — otherwise they fall back to a synthetic "default"
|
||||
# session whose scope never closes, so close-time exporters never ship them.
|
||||
_approval_session_id: contextvars.ContextVar[str] = _ctx("approval_session_id")
|
||||
# Interactive-CLI flag. Concurrent ACP sessions share a ThreadPoolExecutor, so
|
||||
# mutating os.environ["HERMES_INTERACTIVE"] races: one session's `finally`
|
||||
# restore can clobber another's set mid-run, dropping it onto the
|
||||
# non-interactive auto-approve path so a dangerous command runs without the
|
||||
# approval callback firing (GHSA-96vc-wcxf-jjff). None = unset → fall back to
|
||||
# the env var for legacy single-threaded CLI callers.
|
||||
# approval callback firing (GHSA-96vc-wcxf-jjff). None = unset → env fallback.
|
||||
_hermes_interactive_ctx: contextvars.ContextVar[str | None] = _ctx("hermes_interactive", None)
|
||||
|
||||
|
||||
@@ -75,8 +73,7 @@ def _fire_approval_hook(hook_name: str, **kwargs) -> None:
|
||||
kwargs.setdefault("session_id", _approval_session_id.get())
|
||||
invoke_hook(hook_name, **kwargs)
|
||||
except Exception as exc:
|
||||
# invoke_hook() swallows per-callback errors; reaching here means the
|
||||
# dispatch layer itself failed.
|
||||
# invoke_hook() swallows per-callback errors; this is the dispatch layer itself failing.
|
||||
logger.debug("Approval hook %s dispatch failed: %s", hook_name, exc)
|
||||
|
||||
|
||||
@@ -168,8 +165,7 @@ def _is_gateway_approval_context() -> bool:
|
||||
HERMES_SESSION_PLATFORM via contextvars. Cron is NEVER a gateway approval
|
||||
context even when it originated from a platform (cron binds the platform for
|
||||
delivery routing): falling through would submit a pending approval with no
|
||||
listener and block the job indefinitely. Unattended platforms are excluded
|
||||
for the same reason.
|
||||
listener and block the job indefinitely; unattended platforms likewise.
|
||||
"""
|
||||
from tools import approval as _a
|
||||
if _a._is_cron_approval_context() or _is_unattended_platform_approval_context():
|
||||
@@ -189,13 +185,11 @@ def _resolve_cli_approval_callback(approval_callback=None):
|
||||
|
||||
|
||||
def _should_fall_through_to_cli_approval(*, is_cli: bool, approval_callback, notify_cb) -> bool:
|
||||
"""Prefer the CLI Dangerous Command panel over a silent pending approval.
|
||||
|
||||
"""Prefer the CLI Dangerous Command panel over a silent pending approval:
|
||||
``HERMES_EXEC_ASK`` (or a platform marker) can leak into an interactive CLI
|
||||
process — historically via ``import gateway.run``. Without a gateway notify
|
||||
process (historically via ``import gateway.run``), and without a gateway notify
|
||||
listener the ask branch used to return ``pending_approval`` immediately and
|
||||
skip the panel the user can actually answer.
|
||||
"""
|
||||
skip the panel the user can actually answer."""
|
||||
return bool(is_cli and approval_callback is not None and notify_cb is None)
|
||||
|
||||
|
||||
@@ -203,12 +197,10 @@ _VALID_MODES = ("manual", "smart", "off")
|
||||
|
||||
|
||||
def _normalize_approval_mode(mode) -> str:
|
||||
"""Normalize approval mode values loaded from YAML/config.
|
||||
|
||||
YAML 1.1 parses a bare ``off`` as False, so ``mode: off`` arrives as a bool;
|
||||
treat it as the intended string mode. Unknown strings (e.g. 'auto') warn and
|
||||
fall back to 'manual' instead of silently failing every mode check.
|
||||
"""
|
||||
"""Normalize approval mode values loaded from YAML/config. YAML 1.1 parses a
|
||||
bare ``off`` as False, so ``mode: off`` arrives as a bool; treat it as the
|
||||
intended string mode. Unknown strings (e.g. 'auto') warn and fall back to
|
||||
'manual' instead of silently failing every mode check."""
|
||||
if isinstance(mode, bool):
|
||||
return "off" if mode is False else "manual"
|
||||
if isinstance(mode, str):
|
||||
@@ -222,11 +214,8 @@ def _normalize_approval_mode(mode) -> str:
|
||||
|
||||
|
||||
def _get_approval_config() -> dict:
|
||||
"""Read the approvals config block.
|
||||
|
||||
Returns the LIVE config-cache sub-dict (load_config_readonly contract) —
|
||||
callers must not mutate it or any nested structure.
|
||||
"""
|
||||
"""Read the approvals config block: the LIVE config-cache sub-dict
|
||||
(load_config_readonly contract) — callers must not mutate it or any nested structure."""
|
||||
try:
|
||||
from hermes_cli.config import load_config_readonly
|
||||
return load_config_readonly().get("approvals", {}) or {}
|
||||
@@ -251,12 +240,10 @@ def _get_approval_mode() -> str:
|
||||
def _get_approval_timeout() -> int:
|
||||
"""Read ``approvals.timeout`` (default 300s: gateway push notifications may
|
||||
not be seen for minutes; 60s failed closed before Telegram taps landed).
|
||||
|
||||
Clamped to ``agent.deadline.MAX_SAFE_TIMEOUT_S`` (~1 year): a larger value
|
||||
overflows ``time_t`` inside ``Thread.join`` / ``Lock.acquire`` on macOS and
|
||||
crashed every parallel tool batch. Clamping at the single config-read site
|
||||
keeps every consumer platform-safe at once.
|
||||
"""
|
||||
crashed every parallel tool batch; clamping at the single config-read site
|
||||
keeps every consumer platform-safe at once."""
|
||||
from tools import approval as _a
|
||||
try:
|
||||
raw = int(_a._get_approval_config().get("timeout", 300))
|
||||
@@ -304,10 +291,8 @@ def _get_unattended_approval_mode() -> str:
|
||||
|
||||
def _tirith_fail_open() -> bool:
|
||||
"""``security.tirith_fail_open`` (default True; True when config is unreadable).
|
||||
|
||||
False means the operator opted into fail-closed: an un-importable scanner
|
||||
must not silently grant access.
|
||||
"""
|
||||
must not silently grant access."""
|
||||
try:
|
||||
from hermes_cli.config import load_config_readonly as _load_cfg
|
||||
_sec = (_load_cfg() or {}).get("security", {}) or {}
|
||||
|
||||
+27
-37
@@ -18,14 +18,12 @@ logger = logging.getLogger("tools.approval")
|
||||
|
||||
|
||||
def _match_user_deny_rule(command: str) -> str | None:
|
||||
"""Return the matching ``approvals.deny`` glob, or None.
|
||||
|
||||
User-defined fnmatch globs that block unconditionally — like the hardline
|
||||
floor, a match fires BEFORE the yolo / mode=off bypass ("never let the
|
||||
agent run this, even under yolo"). Case-insensitive, run over the same
|
||||
normalized/deobfuscated variants the dangerous-pattern detector uses so
|
||||
quoting tricks (``r\\m``, ``git st""atus``) can't sidestep a rule.
|
||||
"""
|
||||
"""Return the matching ``approvals.deny`` glob, or None. User-defined fnmatch
|
||||
globs that block unconditionally — like the hardline floor, a match fires
|
||||
BEFORE the yolo / mode=off bypass ("never let the agent run this, even under
|
||||
yolo"). Case-insensitive, run over the same normalized/deobfuscated variants
|
||||
the dangerous-pattern detector uses so quoting tricks (``r\\m``,
|
||||
``git st""atus``) can't sidestep a rule."""
|
||||
from tools import approval as _a
|
||||
try:
|
||||
deny_patterns = _a._get_approval_config().get("deny") or []
|
||||
@@ -58,15 +56,13 @@ def _user_deny_block_result(pattern: str) -> dict:
|
||||
|
||||
|
||||
def _save_blocked_payload(command: str) -> str | None:
|
||||
"""Persist a parser-limit-blocked command as a runnable script.
|
||||
|
||||
The parser-limit block fires on payload SIZE/shape, not the operation —
|
||||
usually a legitimate script the model inlined. Saving it makes recovery one
|
||||
turn (`bash <file>`) instead of two, and is strictly safer than the
|
||||
hint-only path: the file goes through the normal execution pipeline
|
||||
(including the referenced-script content guard) and nothing runs here.
|
||||
Returns the path, or None on any failure (hint falls back to write_file).
|
||||
"""
|
||||
"""Persist a parser-limit-blocked command as a runnable script. That block
|
||||
fires on payload SIZE/shape, not the operation — usually a legitimate script
|
||||
the model inlined. Saving it makes recovery one turn (`bash <file>`) instead
|
||||
of two, and is strictly safer than the hint-only path: the file goes through
|
||||
the normal execution pipeline (including the referenced-script content guard)
|
||||
and nothing runs here. Returns the path, or None on any failure (hint falls
|
||||
back to write_file)."""
|
||||
try:
|
||||
from hermes_constants import get_hermes_home
|
||||
script_dir = get_hermes_home() / "cache" / "blocked-scripts"
|
||||
@@ -113,8 +109,8 @@ def _hardline_block_result(description: str, command: str = "") -> dict:
|
||||
"agent."
|
||||
)
|
||||
# The parser-limit block is almost always a giant inline payload, not a
|
||||
# forbidden operation, and is typically followed by blind rephrase
|
||||
# retries — point at the saved script (or the write_file recipe).
|
||||
# forbidden operation, and is typically followed by blind rephrase retries —
|
||||
# point at the saved script (or the write_file recipe).
|
||||
if description in (_PARSER_LIMIT_DESCRIPTION, _MALFORMED_EXEC_DESCRIPTION):
|
||||
saved = _a._save_blocked_payload(command) if command else None
|
||||
if saved:
|
||||
@@ -146,28 +142,24 @@ def _sudo_stdin_block_result(description: str) -> dict:
|
||||
}
|
||||
|
||||
|
||||
# Shell control characters that make a command compound when they appear
|
||||
# OUTSIDE quotes. Inside quotes they are literal to the outer shell — but they
|
||||
# become executable again if an option like `-c`/`-e`/`--eval` (or a git
|
||||
# `-c alias.x=!...`) hands the quoted argument to another interpreter, so quoted
|
||||
# control chars only disqualify a command when such an option is present.
|
||||
# Shell control characters that make a command compound when they appear OUTSIDE
|
||||
# quotes. Inside quotes they are literal to the outer shell — but they become
|
||||
# executable again if an option like `-c`/`-e`/`--eval` (or a git `-c alias.x=!...`)
|
||||
# hands the quoted argument to another interpreter, so quoted control chars only
|
||||
# disqualify a command when such an option is present.
|
||||
_SHELL_CONTROL_CHARS = frozenset("\n\r;&|<>`$()")
|
||||
|
||||
_REINTERPRETED_ARGUMENT_RE = re.compile(
|
||||
r"(?:^|[ \t])(?:-[^-\s]*[ce]|--(?:command|eval))(?:[= \t]|$)"
|
||||
)
|
||||
_REINTERPRETED_ARGUMENT_RE = re.compile(r"(?:^|[ \t])(?:-[^-\s]*[ce]|--(?:command|eval))(?:[= \t]|$)")
|
||||
|
||||
|
||||
def _has_allowlist_shell_operator(command: str) -> bool:
|
||||
"""Return True when a command is too compound for the allowlist shortcut.
|
||||
|
||||
Quote-aware: metacharacters inside quotes or behind a backslash are literal
|
||||
arguments (``cargo bench -- '^a(b|c)$'``), not shell syntax. Still
|
||||
disqualifying: ``$`` or backtick inside DOUBLE quotes (expansion stays
|
||||
active), and any quoted/escaped control character when the command also
|
||||
carries a ``-c``/``-e``/``--command``/``--eval``-style option that would
|
||||
hand the quoted text to another interpreter.
|
||||
"""
|
||||
hand the quoted text to another interpreter."""
|
||||
command = command or ""
|
||||
quote = None # None | "'" | '"'
|
||||
has_reinterpretable = False
|
||||
@@ -191,8 +183,8 @@ def _has_allowlist_shell_operator(command: str) -> bool:
|
||||
elif ch in ("'", '"'):
|
||||
quote = ch
|
||||
elif ch == "$":
|
||||
# Unquoted $ is only compound when it opens a substitution
|
||||
# ("$HOME" stays simple, matching the historical `\$\(` behavior).
|
||||
# Unquoted $ is only compound when it opens a substitution ("$HOME"
|
||||
# stays simple, matching the historical `\$\(` behavior).
|
||||
if i + 1 < n and command[i + 1] == "(":
|
||||
return True
|
||||
elif ch in _SHELL_CONTROL_CHARS and ch not in "()":
|
||||
@@ -205,12 +197,10 @@ def _has_allowlist_shell_operator(command: str) -> bool:
|
||||
|
||||
|
||||
def _command_matches_permanent_allowlist(command: str) -> bool:
|
||||
"""True when command_allowlist holds this exact command text or a matching glob.
|
||||
|
||||
Permanent approvals historically store dangerous-pattern keys such as
|
||||
"""True when command_allowlist holds this exact command text or a matching
|
||||
glob. Permanent approvals historically store dangerous-pattern keys such as
|
||||
``recursive delete``; manual entries are command text, possibly with
|
||||
shell-style wildcards like ``podman *``.
|
||||
"""
|
||||
shell-style wildcards like ``podman *``."""
|
||||
from tools import approval as _a
|
||||
command = (command or "").strip()
|
||||
if not command or _a._has_allowlist_shell_operator(command):
|
||||
|
||||
@@ -5,10 +5,9 @@ pending approval, the gateway notifies the user, and the thread blocks until
|
||||
``/approve`` / ``/deny`` resolves it or the approval timeout elapses. Multiple
|
||||
threads (parallel subagents, execute_code RPC handlers) can block concurrently
|
||||
— each gets its own ``threading.Event``; ``/approve`` resolves the oldest,
|
||||
``/approve all`` every pending entry.
|
||||
|
||||
Queue state (``_gateway_queues``, ``_lock``) is owned by ``tools.approval`` and
|
||||
reached through that module at call time so tests patching it keep working.
|
||||
``/approve all`` every pending entry. Queue state (``_gateway_queues``,
|
||||
``_lock``) is owned by ``tools.approval`` and reached through that module at
|
||||
call time so tests patching it keep working.
|
||||
"""
|
||||
|
||||
import logging
|
||||
@@ -31,27 +30,23 @@ class _ApprovalEntry:
|
||||
self.data.setdefault("request_id", uuid.uuid4().hex)
|
||||
self.acknowledged = False
|
||||
self.result: str | None = None # "once"|"session"|"always"|"deny"
|
||||
# Free-text reason from ``/deny <reason>`` so the agent can adapt
|
||||
# instead of only hearing "denied".
|
||||
# Free-text reason from ``/deny <reason>`` so the agent can adapt, not just hear "denied".
|
||||
self.reason: str | None = None
|
||||
|
||||
|
||||
def _poll_event(event: threading.Event, session_key: str, *, interrupt_log: str) -> str:
|
||||
"""Wait on *event* until it fires, the turn is interrupted, or approvals.timeout elapses.
|
||||
|
||||
Returns ``"set"`` | ``"interrupted"`` | ``"timeout"``. Polls in ~1s slices
|
||||
so activity heartbeats reach the agent's inactivity tracker every ~10s —
|
||||
"""Wait on *event* until it fires, the turn is interrupted, or approvals.timeout
|
||||
elapses; returns ``"set"`` | ``"interrupted"`` | ``"timeout"``. Polls in ~1s
|
||||
slices so activity heartbeats reach the agent's inactivity tracker every ~10s —
|
||||
otherwise the gateway watchdog kills the agent while the user is still
|
||||
responding (mirrors ``_wait_for_process()`` cadence). The loop is recorded
|
||||
as human-wait time so the concurrent batch deadline excludes it.
|
||||
responding (mirrors ``_wait_for_process()`` cadence). The loop is recorded as
|
||||
human-wait time so the concurrent batch deadline excludes it.
|
||||
|
||||
``is_interrupted()`` deliberately does NOT distinguish a deliberate /stop
|
||||
from a gateway inactivity timeout — both resolve as 'deny' (not
|
||||
outcome='timeout'). The per-thread interrupt flag carries no stable
|
||||
machine-checkable cause, so a fail-closed deny preserves the historical
|
||||
semantics; changing this needs a dedicated interrupt-cause channel, not
|
||||
string matching.
|
||||
"""
|
||||
``is_interrupted()`` deliberately does NOT distinguish a deliberate /stop from
|
||||
a gateway inactivity timeout — both resolve as 'deny' (not outcome='timeout').
|
||||
The per-thread interrupt flag carries no stable machine-checkable cause, so a
|
||||
fail-closed deny preserves the historical semantics; changing this needs a
|
||||
dedicated interrupt-cause channel, not string matching."""
|
||||
from tools.approval import _get_approval_timeout, human_wait_window
|
||||
|
||||
timeout = _get_approval_timeout()
|
||||
@@ -87,15 +82,13 @@ def _finish(payload: dict, resolved: bool, choice: str | None, reason, **extra)
|
||||
|
||||
def _await_coalesced_leader(session_key: str, leader, payload: dict):
|
||||
"""Wait on an already-pending identical approval instead of re-prompting.
|
||||
|
||||
Adopts the leader's decision: ``session``/``always`` → approval (same dict
|
||||
shape as a direct resolution; persistence stays the caller's and is
|
||||
idempotent across leader and followers); ``deny`` → denial carrying the
|
||||
leader's reason; leader timeout / our own deadline → unresolved. ``once``
|
||||
returns ``None``: single-use consent covers only the leader's execution,
|
||||
so the caller must issue a fresh prompt. Hooks fire with ``coalesced=True``
|
||||
so observers see the follower's lifecycle without a duplicate prompt.
|
||||
"""
|
||||
so observers see the follower's lifecycle without a duplicate prompt."""
|
||||
from tools.approval import _fire_approval_hook
|
||||
_fire_approval_hook("pre_approval_request", **payload, coalesced=True)
|
||||
state = _poll_event(leader.event, session_key,
|
||||
@@ -117,9 +110,8 @@ def _await_coalesced_leader(session_key: str, leader, payload: dict):
|
||||
|
||||
def _await_gateway_decision(session_key: str, notify_cb, approval_data: dict,
|
||||
*, surface: str = "gateway") -> dict:
|
||||
"""Enqueue *approval_data*, notify the user, and block until resolved or timed out.
|
||||
|
||||
Shared by the terminal command guard, the execute_code guard, the plugin
|
||||
"""Enqueue *approval_data*, notify the user, and block until resolved or timed
|
||||
out. Shared by the terminal command guard, the execute_code guard, the plugin
|
||||
escalation gate, and MCP elicitation. Returns ``{"resolved", "choice",
|
||||
"reason"}`` or ``{"resolved": False, "choice": None, "notify_failed": True}``
|
||||
when the notify callback raised. Persisting the choice and building the
|
||||
@@ -129,8 +121,7 @@ def _await_gateway_decision(session_key: str, notify_cb, approval_data: dict,
|
||||
coalesced: parallel tool calls would otherwise fire N identical prompts
|
||||
the user must /approve N times while the agent sits wedged. Followers adopt
|
||||
the leader's ``session``/``always``/``deny``/timeout; a ``once`` covers only
|
||||
the leader, so the follower falls through to a fresh prompt.
|
||||
"""
|
||||
the leader, so the follower falls through to a fresh prompt."""
|
||||
from tools import approval as _approval
|
||||
|
||||
primary_key = approval_data.get("pattern_key", "")
|
||||
|
||||
@@ -6,11 +6,10 @@ deadline in agent/tool_executor.py excludes this time so a slow human answer
|
||||
never times a batch out — but ONLY this time. Measuring at the source (rather
|
||||
than residency in the authorization gate, which is arbitrary code) is what keeps
|
||||
a wedged pre_tool_call plugin or a dead approval client from growing the
|
||||
exclusion 1:1 with wall clock and defeating the deadline entirely.
|
||||
|
||||
Keyed by session so one gateway session's pending approval cannot extend a
|
||||
different session's batch deadline. State is process-global like the rest of
|
||||
the approval state; entries are bounded by _HUMAN_WAIT_MAX_SESSIONS.
|
||||
exclusion 1:1 with wall clock and defeating the deadline entirely. Keyed by
|
||||
session so one gateway session's pending approval cannot extend a different
|
||||
session's batch deadline; state is process-global like the rest of the approval
|
||||
state, bounded by _HUMAN_WAIT_MAX_SESSIONS.
|
||||
"""
|
||||
|
||||
import contextlib
|
||||
@@ -30,26 +29,23 @@ class _HumanWaitState:
|
||||
_human_wait_lock = threading.Lock()
|
||||
_human_wait_states: dict[str, _HumanWaitState] = {}
|
||||
_HUMAN_WAIT_MAX_SESSIONS = 256
|
||||
# Margin added on top of approvals.timeout when clamping a window's
|
||||
# contribution (read-side AND close-side) and when bounding the authorization
|
||||
# gate's serialization-lock acquire in agent/tool_executor.py. One constant so
|
||||
# the clamps can't drift apart.
|
||||
# Margin added on top of approvals.timeout when clamping a window's contribution
|
||||
# (read-side AND close-side) and when bounding the authorization gate's
|
||||
# serialization-lock acquire in agent/tool_executor.py. One constant so the
|
||||
# clamps can't drift apart.
|
||||
HUMAN_WAIT_MARGIN_S = 60.0
|
||||
|
||||
|
||||
def human_wait_ceiling() -> float:
|
||||
"""Max seconds a single window may contribute: approvals.timeout + margin.
|
||||
|
||||
Every legitimate human wait self-terminates at ``approvals.timeout`` (the
|
||||
CLI prompt join and the gateway poll loop both enforce it), so a window
|
||||
that overstays this ceiling is itself wedged and must not keep extending
|
||||
a batch deadline. Also the bound on the authorization gate's
|
||||
serialization-lock acquire in agent/tool_executor.py, so the two cannot
|
||||
drift. Never call while holding ``_human_wait_lock`` — it reads the
|
||||
config cache. ``_get_approval_timeout`` caps at
|
||||
``agent.deadline.MAX_SAFE_TIMEOUT_S`` so the value is always safe for
|
||||
``Lock.acquire(timeout=...)`` / ``Thread.join(timeout=...)``.
|
||||
"""
|
||||
Every legitimate human wait self-terminates at ``approvals.timeout`` (the CLI
|
||||
prompt join and the gateway poll loop both enforce it), so a window that
|
||||
overstays this ceiling is itself wedged and must not keep extending a batch
|
||||
deadline. Also the bound on the authorization gate's serialization-lock
|
||||
acquire in agent/tool_executor.py, so the two cannot drift. Never call while
|
||||
holding ``_human_wait_lock`` — it reads the config cache.
|
||||
``_get_approval_timeout`` caps at ``agent.deadline.MAX_SAFE_TIMEOUT_S`` so the
|
||||
value is always safe for ``Lock.acquire(timeout=...)`` / ``Thread.join(timeout=...)``."""
|
||||
from tools.approval import _get_approval_timeout
|
||||
return float(_get_approval_timeout()) + HUMAN_WAIT_MARGIN_S
|
||||
|
||||
@@ -62,14 +58,12 @@ def _clamped_window_seconds(started: float, now: float, ceiling: float) -> float
|
||||
|
||||
|
||||
def _human_wait_state(session_key: str) -> _HumanWaitState:
|
||||
"""Return (creating if needed) the wait state for *session_key*.
|
||||
|
||||
Caller must hold ``_human_wait_lock``. Evicts idle entries (no pending
|
||||
waiter) insertion-order-first until the table is under the cap so an army
|
||||
of short-lived session keys cannot grow it without bound. Entries with an
|
||||
open window are never evicted (that would corrupt live accounting), so
|
||||
the cap is best-effort under 256+ concurrently-pending sessions.
|
||||
"""
|
||||
"""Return (creating if needed) the wait state for *session_key*. Caller must
|
||||
hold ``_human_wait_lock``. Evicts idle entries (no pending waiter)
|
||||
insertion-order-first until the table is under the cap so an army of
|
||||
short-lived session keys cannot grow it without bound. Entries with an open
|
||||
window are never evicted (that would corrupt live accounting), so the cap is
|
||||
best-effort under 256+ concurrently-pending sessions."""
|
||||
state = _human_wait_states.get(session_key)
|
||||
if state is None:
|
||||
for key in list(_human_wait_states):
|
||||
@@ -90,16 +84,13 @@ def _resolve_key(session_key: str | None) -> str:
|
||||
|
||||
@contextlib.contextmanager
|
||||
def human_wait_window(session_key: str | None = None):
|
||||
"""Mark the enclosed block as time spent blocked on a human prompt.
|
||||
|
||||
Wrap ONLY code that is genuinely parked waiting for a user's answer (the
|
||||
CLI approval prompt, the gateway approval poll loop). The concurrent tool
|
||||
batch deadline excludes this time; wrapping anything else re-creates the
|
||||
hang where arbitrary wedged code pushes the deadline out forever.
|
||||
|
||||
Overlapping windows for the same session coalesce (pending counter), so
|
||||
two serialized approval prompts don't double-count the same wall clock.
|
||||
"""
|
||||
"""Mark the enclosed block as time spent blocked on a human prompt. Wrap ONLY
|
||||
code that is genuinely parked waiting for a user's answer (the CLI approval
|
||||
prompt, the gateway approval poll loop). The concurrent tool batch deadline
|
||||
excludes this time; wrapping anything else re-creates the hang where
|
||||
arbitrary wedged code pushes the deadline out forever. Overlapping windows
|
||||
for the same session coalesce (pending counter), so two serialized approval
|
||||
prompts don't double-count the same wall clock."""
|
||||
key = _resolve_key(session_key)
|
||||
now = time.monotonic()
|
||||
with _human_wait_lock:
|
||||
@@ -111,8 +102,8 @@ def human_wait_window(session_key: str | None = None):
|
||||
yield
|
||||
finally:
|
||||
now = time.monotonic()
|
||||
# Clamp the accrual too: a window that overstayed the ceiling was
|
||||
# wedged — record at most the ceiling, not the whole overstay.
|
||||
# Clamp the accrual too: a window that overstayed the ceiling was wedged —
|
||||
# record at most the ceiling, not the whole overstay.
|
||||
ceiling = human_wait_ceiling()
|
||||
with _human_wait_lock:
|
||||
state = _human_wait_states.get(key)
|
||||
@@ -120,27 +111,23 @@ def human_wait_window(session_key: str | None = None):
|
||||
state.pending -= 1
|
||||
if state.pending == 0:
|
||||
if state.window_started is not None:
|
||||
state.completed_seconds += _clamped_window_seconds(
|
||||
state.window_started, now, ceiling
|
||||
)
|
||||
state.completed_seconds += _clamped_window_seconds(state.window_started, now, ceiling)
|
||||
state.window_started = None
|
||||
|
||||
|
||||
def human_wait_seconds(session_key: str | None = None) -> float:
|
||||
"""Return total human-wait seconds recorded for the session.
|
||||
|
||||
Completed windows plus the currently open one (if any). Monotonically
|
||||
non-decreasing for the life of the process — except when an idle session's
|
||||
entry is evicted under cap pressure, which can only shrink a consumer's
|
||||
baseline delta to zero (the safe direction: the deadline fires sooner).
|
||||
Deadline consumers snapshot a baseline at batch start and use the delta.
|
||||
Each window's contribution is clamped to :func:`human_wait_ceiling`
|
||||
(belt-and-braces against the wedged-window hang).
|
||||
"""
|
||||
"""Return total human-wait seconds recorded for the session: completed windows
|
||||
plus the currently open one (if any). Monotonically non-decreasing for the
|
||||
life of the process — except when an idle session's entry is evicted under
|
||||
cap pressure, which can only shrink a consumer's baseline delta to zero (the
|
||||
safe direction: the deadline fires sooner). Deadline consumers snapshot a
|
||||
baseline at batch start and use the delta. Each window's contribution is
|
||||
clamped to :func:`human_wait_ceiling` (belt-and-braces against the
|
||||
wedged-window hang)."""
|
||||
key = _resolve_key(session_key)
|
||||
now = time.monotonic()
|
||||
# Resolve the clamp outside the lock: it reads the config cache, which
|
||||
# must never nest under _human_wait_lock.
|
||||
# Resolve the clamp outside the lock: it reads the config cache, which must
|
||||
# never nest under _human_wait_lock.
|
||||
ceiling = human_wait_ceiling()
|
||||
with _human_wait_lock:
|
||||
state = _human_wait_states.get(key)
|
||||
|
||||
+43
-55
@@ -22,29 +22,26 @@ def prompt_dangerous_approval(command: str, description: str, timeout_seconds: i
|
||||
*, allow_session: bool = True, smart_denied: bool = False) -> str:
|
||||
"""Prompt the user to approve a dangerous command (CLI only).
|
||||
|
||||
Args:
|
||||
allow_permanent: When False, hide [a]lways (tirith warnings present:
|
||||
broad permanent allowlisting is wrong for content-level findings).
|
||||
allow_session: When False, hide [s]ession too — the caller grants one
|
||||
operation and re-asks next time (the protected agent-instruction
|
||||
gate in ``tools/file_tools.py``). Offering a scope the caller
|
||||
discards makes every later write re-prompt and reads as broken.
|
||||
smart_denied: Owner override of a Smart DENY: offer only once/deny.
|
||||
approval_callback: CLI prompt_toolkit callback,
|
||||
``(command, description, *, allow_permanent=True,
|
||||
allow_session=True, smart_denied=False) -> str``. Legacy
|
||||
signatures keep working while both keywords hold their defaults.
|
||||
allow_permanent=False hides [a]lways (tirith warnings present: broad permanent
|
||||
allowlisting is wrong for content-level findings). allow_session=False hides
|
||||
[s]ession too — the caller grants one operation and re-asks next time (the
|
||||
protected agent-instruction gate in ``tools/file_tools.py``); offering a scope
|
||||
the caller discards makes every later write re-prompt and reads as broken.
|
||||
smart_denied: owner override of a Smart DENY, offer only once/deny.
|
||||
approval_callback: CLI prompt_toolkit callback ``(command, description, *,
|
||||
allow_permanent=True, allow_session=True, smart_denied=False) -> str``; legacy
|
||||
signatures keep working while both keywords hold their defaults.
|
||||
|
||||
Returns: 'once', 'session', 'always', 'deny', or 'timeout'. 'timeout'
|
||||
means no user response — still blocked (fail-closed), but callers
|
||||
report "no response" rather than an explicit denial.
|
||||
Returns 'once', 'session', 'always', 'deny', or 'timeout'. 'timeout' means no
|
||||
user response — still blocked (fail-closed), but callers report "no response"
|
||||
rather than an explicit denial.
|
||||
"""
|
||||
from tools import approval as _a
|
||||
if timeout_seconds is None:
|
||||
timeout_seconds = _a._get_approval_timeout()
|
||||
# Everything below is a human prompt (callback panel or input() fallback,
|
||||
# both bounded by the approval deadline): record it as human-wait time so
|
||||
# the concurrent batch deadline excludes it.
|
||||
# Everything below is a human prompt (callback panel or input() fallback, both
|
||||
# bounded by the approval deadline): record it as human-wait time so the
|
||||
# concurrent batch deadline excludes it.
|
||||
with human_wait_window():
|
||||
return _prompt_dangerous_approval_inner(
|
||||
command, description, timeout_seconds, allow_permanent,
|
||||
@@ -85,9 +82,9 @@ def _read_choice(prompt: str, timeout_seconds: int) -> str | None:
|
||||
def _prompt_dangerous_approval_inner(command: str, description: str, timeout_seconds: int,
|
||||
allow_permanent: bool = True, approval_callback=None,
|
||||
*, allow_session: bool = True, smart_denied: bool = False) -> str:
|
||||
# Redact before any user-visible rendering; the original `command` is
|
||||
# still what executes after approval. Same redactor as memory/log
|
||||
# sanitization so tokens mask consistently across surfaces.
|
||||
# Redact before any user-visible rendering; the original `command` still
|
||||
# executes after approval. Same redactor as memory/log sanitization so
|
||||
# tokens mask consistently across surfaces.
|
||||
from agent.redact import redact_sensitive_text
|
||||
display_command = redact_sensitive_text(command)
|
||||
display_description = redact_sensitive_text(description)
|
||||
@@ -106,12 +103,11 @@ def _prompt_dangerous_approval_inner(command: str, description: str, timeout_sec
|
||||
logger.error("Approval callback failed: %s", e, exc_info=True)
|
||||
return "deny"
|
||||
|
||||
# Fail-closed guard: when prompt_toolkit owns the terminal and no callback
|
||||
# is registered on this thread, the input() fallback would spawn a daemon
|
||||
# thread whose read never sees Enter (keystrokes go to prompt_toolkit) —
|
||||
# an invisible deadlock. Deny loudly instead; threads needing interactive
|
||||
# approval must install a callback via
|
||||
# tools.terminal_tool.set_approval_callback() first.
|
||||
# Fail-closed guard: when prompt_toolkit owns the terminal and no callback is
|
||||
# registered on this thread, the input() fallback would spawn a daemon thread
|
||||
# whose read never sees Enter (keystrokes go to prompt_toolkit) — an invisible
|
||||
# deadlock. Deny loudly instead; threads needing interactive approval must
|
||||
# install a callback via tools.terminal_tool.set_approval_callback() first.
|
||||
try:
|
||||
from prompt_toolkit.application.current import get_app_or_none
|
||||
if get_app_or_none() is not None:
|
||||
@@ -163,9 +159,9 @@ def _prompt_dangerous_approval_inner(command: str, description: str, timeout_sec
|
||||
def get_plugin_manager():
|
||||
"""Lazy plugin-manager seam used by tests and early tool-only imports."""
|
||||
from hermes_cli.plugins import discover_plugins, get_plugin_manager as _get_manager
|
||||
# Approval can be imported before model_tools (which triggers discovery);
|
||||
# make an explicitly selected transport available on the first approval
|
||||
# instead of treating the undiscovered registry as unavailable.
|
||||
# Approval can be imported before model_tools (which triggers discovery); make
|
||||
# an explicitly selected transport available on the first approval instead of
|
||||
# treating the undiscovered registry as unavailable.
|
||||
discover_plugins()
|
||||
return _get_manager()
|
||||
|
||||
@@ -178,13 +174,11 @@ def _attempt(name: str, choice, failure, fallback) -> dict:
|
||||
def _present_with_selected_transport(*, command: str, description: str, pattern_key: str,
|
||||
pattern_keys: list[str], session_key: str, surface: str,
|
||||
allow_session: bool, allow_permanent: bool) -> dict:
|
||||
"""Present through an explicitly selected plugin transport, if any.
|
||||
|
||||
A selected transport replaces every built-in prompt surface; detection,
|
||||
allowed scopes, persistence, timeout, and final authorization stay
|
||||
host-owned. A failed transport reaches a built-in surface only under the
|
||||
explicit ``transport_fallback: builtin`` opt-in.
|
||||
"""
|
||||
"""Present through an explicitly selected plugin transport, if any. A selected
|
||||
transport replaces every built-in prompt surface; detection, allowed scopes,
|
||||
persistence, timeout, and final authorization stay host-owned. A failed
|
||||
transport reaches a built-in surface only under the explicit
|
||||
``transport_fallback: builtin`` opt-in."""
|
||||
from tools import approval as _a
|
||||
name, fallback = _a._get_approval_transport_config()
|
||||
if name == "builtin":
|
||||
@@ -213,9 +207,8 @@ def _present_with_selected_transport(*, command: str, description: str, pattern_
|
||||
timeout_seconds=timeout_seconds,
|
||||
)
|
||||
except Exception:
|
||||
# Never fall back to raw text if redaction or request construction
|
||||
# fails: fail closed without calling the plugin or leaking the
|
||||
# unredacted payload to logs/hooks.
|
||||
# Never fall back to raw text if redaction or request construction fails:
|
||||
# fail closed without calling the plugin or leaking the unredacted payload.
|
||||
logger.warning("Could not build redacted plugin approval request")
|
||||
return _attempt(name, "deny", "error", None)
|
||||
hook_kwargs = dict(
|
||||
@@ -246,12 +239,10 @@ def _present_with_selected_transport(*, command: str, description: str, pattern_
|
||||
|
||||
|
||||
def _transport_choice(attempt: dict, *, pattern_key: str, description: str):
|
||||
"""Interpret a ``_present_with_selected_transport`` attempt.
|
||||
|
||||
Returns ``(choice, denied_result)``: both None when the built-in surfaces
|
||||
should run (no transport selected, or a failure with the explicit builtin
|
||||
fallback); a denied result for any other failure; else the user's choice.
|
||||
"""
|
||||
"""Interpret a ``_present_with_selected_transport`` attempt into
|
||||
``(choice, denied_result)``: both None when the built-in surfaces should run
|
||||
(no transport selected, or a failure with the explicit builtin fallback); a
|
||||
denied result for any other failure; else the user's choice."""
|
||||
if not attempt.get("selected"):
|
||||
return None, None
|
||||
failure = attempt.get("failure")
|
||||
@@ -281,14 +272,12 @@ def _consent(choice, unresolved: str) -> str:
|
||||
def request_elicitation_consent(message: str, description: str, *,
|
||||
timeout_seconds: int | None = None,
|
||||
surface: str = "mcp-elicitation") -> str:
|
||||
"""Route an MCP elicitation request to the surface owning the active session.
|
||||
|
||||
Gateway sessions go through ``_await_gateway_decision``; CLI/TUI through
|
||||
``prompt_dangerous_approval``. Always fails closed: a missing notify_cb in
|
||||
a gateway session, timeouts, and exceptions map to ``"decline"`` so a server
|
||||
"""Route an MCP elicitation request to the surface owning the active session:
|
||||
gateway sessions through ``_await_gateway_decision``, CLI/TUI through
|
||||
``prompt_dangerous_approval``. Always fails closed: a missing notify_cb in a
|
||||
gateway session, timeouts, and exceptions map to ``"decline"`` so a server
|
||||
treats them as "user did not approve" rather than retrying or hanging.
|
||||
Returns ``"accept" | "decline" | "cancel"``.
|
||||
"""
|
||||
Returns ``"accept" | "decline" | "cancel"``."""
|
||||
from tools import approval as _a
|
||||
try:
|
||||
session_key = _a.get_current_session_key()
|
||||
@@ -315,8 +304,7 @@ def request_elicitation_consent(message: str, description: str, *,
|
||||
return "cancel"
|
||||
return _consent(decision.get("choice"), "decline")
|
||||
|
||||
# allow_permanent=False: elicitation is a per-call confirmation — there
|
||||
# is no pattern to remember.
|
||||
# allow_permanent=False: elicitation is a per-call confirmation — no pattern to remember.
|
||||
try:
|
||||
choice = _a.prompt_dangerous_approval(
|
||||
message, description, timeout_seconds=timeout_seconds, allow_permanent=False,
|
||||
|
||||
+10
-16
@@ -55,12 +55,9 @@ def _strip_line_comment(line: str) -> str:
|
||||
|
||||
|
||||
def _strip_shell_comments(command: str) -> str:
|
||||
"""Strip unquoted ``# ...`` comments before LLM assessment.
|
||||
|
||||
Not a POSIX parser — quoted ``#`` and heredoc bodies are preserved by a
|
||||
simple state machine. The goal is removing the low-hanging injection
|
||||
surface, not full shell parsing.
|
||||
"""
|
||||
"""Strip unquoted ``# ...`` comments before LLM assessment. Not a POSIX parser
|
||||
— quoted ``#`` and heredoc bodies are preserved by a simple state machine; the
|
||||
goal is removing the low-hanging injection surface, not full shell parsing."""
|
||||
cleaned: list[str] = []
|
||||
for line in command.split("\n"):
|
||||
stripped = _strip_line_comment(line)
|
||||
@@ -82,16 +79,15 @@ def _smart_approve(command: str, description: str) -> str:
|
||||
try:
|
||||
from agent.auxiliary_client import _get_task_timeout, call_llm
|
||||
|
||||
# Pass the timeout explicitly AND log call + duration: this synchronous
|
||||
# call gates EVERY flagged command, and a stalled provider once froze
|
||||
# turns for tens of minutes with zero log output.
|
||||
# Pass the timeout explicitly AND log call + duration: this synchronous call
|
||||
# gates EVERY flagged command, and a stalled provider once froze turns for
|
||||
# tens of minutes with zero log output.
|
||||
smart_timeout = _get_task_timeout("approval")
|
||||
logger.debug("Smart approvals: assessing risk for command (timeout=%ss)", smart_timeout)
|
||||
system_prompt = _SYSTEM_PROMPT
|
||||
# Operator policy goes in the SYSTEM prompt only — the trusted channel.
|
||||
# Never next to the <command> block: that would dilute the trust
|
||||
# boundary and teach the guard to accept policy-looking text adjacent
|
||||
# to (untrusted) commands.
|
||||
# Operator policy goes in the SYSTEM prompt only — the trusted channel. Never
|
||||
# next to the <command> block: that would dilute the trust boundary and teach
|
||||
# the guard to accept policy-looking text adjacent to (untrusted) commands.
|
||||
operator_policy = _get_smart_policy()
|
||||
if operator_policy:
|
||||
system_prompt += (
|
||||
@@ -127,10 +123,8 @@ def _smart_approve(command: str, description: str) -> str:
|
||||
def _smart_verdict(command: str, description: str, pattern_key: str,
|
||||
pattern_keys: list[str], session_key: str) -> str:
|
||||
"""Run the guardian LLM with observer hooks; 'approve' | 'deny' | 'escalate'.
|
||||
|
||||
Redaction is observer-payload preparation, not approval policy: if it fails,
|
||||
skip observability rather than leak raw data or block the LLM decision.
|
||||
"""
|
||||
skip observability rather than leak raw data or block the LLM decision."""
|
||||
from tools import approval as _a
|
||||
try:
|
||||
from agent.redact import redact_sensitive_text
|
||||
|
||||
Reference in New Issue
Block a user