# NousResearch/hermes-agent · Approval system bugs and fixes across multiple surfaces ## Open now - 6 issues, 5 PRs open on GitHub. - #21563: MCP approval tools are no-ops: bridge subprocess has no IPC channel to gateway approval state - #22001: MCP \`permissions_list_open\` and \`permissions_respond\` are non-functional — \`_pending_approvals\` is never populated - #25808: Feature: Remote human-input bridge for in-flight CLI/TUI runs via Gateway platforms - #59260: \[Bug\]: Plugin tool approval prompts fail silently over gateway bridge with 300s timeout - #63183: \[Security\] Kanban workers inherit gateway/cron approval context but not the approval callback - #65291: Bug: Sub-agent approval callback (_subagent_auto_deny) silently bypassed in gateway mode — misleading safety comment - PR #22300: fix(mcp): drop broken approval tools from MCP surface - PR #56971: fix(mcp): wire bridge approval tools to the gateway via a cross-process handshake - PR #63189: fix(kanban): drop approval context env vars from spawned workers - PR #63342: fix(mcp): unblock bridge approval responses - PR #63421: fix(kanban): broker detached worker approvals across processes These issues and PRs concern a cluster of related problems with tool-call approval flows failing, being silently bypassed, or not rendering correctly across gateway, desktop, MCP, kanban workers, and execute_code contexts. Both bug reports and attempted fixes are represented, spanning UI, IPC, security enforcement, and cross-process callback wiring. ## TL;DR - 23 member(s): 6 open issue(s), 6 closed issue(s), 5 open PR(s), 1 merged PR(s), 5 PR(s) closed without merging. - A fix was merged to main in PR #34497. Verify it addresses your case. - **Recommended:** read PR #34497 (and any competing PRs below) before filing. Confirm the merged change covers your symptom. ## Maintainer resolution verdict - Internal-tracker verdict over this cluster's issue members: `partially_resolved` · deciding PR(s): `pr-34497`. This is distinct from the fix status above. Weigh it yourself. ## How this cluster was grouped - Link evidence (`STRUCTURAL_PARTIAL`): some members are connected by structural evidence (shared changed files, or a closing/superseded reference); the rest are joined by an automated relevance judgement over their text. - Members outside that structural core may belong to a different underlying problem. - Pairwise link density `0.06`: the share of member pairs that carry a link, counting a structural link in full and a relevance-only link at 0.36. `1.00` means every pair is linked. ## Open related issues - #21563: MCP approval tools are no-ops: bridge subprocess has no IPC channel to gateway approval state - #22001: MCP \`permissions_list_open\` and \`permissions_respond\` are non-functional — \`_pending_approvals\` is never populated - #25808: Feature: Remote human-input bridge for in-flight CLI/TUI runs via Gateway platforms - #59260: \[Bug\]: Plugin tool approval prompts fail silently over gateway bridge with 300s timeout - #63183: \[Security\] Kanban workers inherit gateway/cron approval context but not the approval callback - #65291: Bug: Sub-agent approval callback (_subagent_auto_deny) silently bypassed in gateway mode — misleading safety comment ## Closed / duplicate issues - #21558: MCP approvals subsystem (permissions_list_open / permissions_respond) lacks gateway IPC and is non-functional - #30882: \[Bug\]: \[Security\] approvals.mode=manual silently bypassed in gateway mode — dangerous commands execute without approval, including via execute_code sandbox evasion - #46526: \[Bug\]: In Hermes Desktop, approval-gated tool calls time out without a visible approval prompt - #59201: \[Bug\]: Approval plugin doesn't show approve controls on desktop and gateway (works on CLI) - #59413: Bug: request_tool_approval doesn't trigger gateway approval UI (Discord/Telegram/Slack) - #60260: \[Bug\]: Desktop approval bar UI not rendered (val.request event arrives but DOM stays hidden) ## Prior & competing PRs - **#34497: fix(security): restore approval/sudo context in execute_code + guard entry points (salvage #34131)**. merged to main · fix tier `verified` - maintainer direction: category `merged`; implication `superseded` - files: `agent/tool_executor.py`, `gateway/run.py`, `tests/run_agent/test_tool_executor_contextvar_propagation.py`, `tests/tools/test_code_execution_windows_env.py`, `tests/tools/test_execute_code_approval_cluster.py`, `tools/approval.py`, `tools/code_execution_tool.py`, `tools/thread_context.py` - fix hunks: - `agent/tool_executor.py` ```diff @@ -13,7 +13,6 @@ from __future__ import annotations import concurrent.futures -import contextvars import json import logging import os ``` ```diff @@ -38,12 +37,9 @@ make_tool_result_message, ) from tools.terminal_tool import ( - _get_approval_callback, - _get_sudo_password_callback, - set_approval_callback as _set_approval_callback, - set_sudo_password_callback as _set_sudo_password_callback, get_active_env, ) +from tools.thread_context import propagate_context_to_thread from tools.tool_result_storage import ( maybe_persist_tool_result, enforce_turn_budget, ``` ```diff @@ -186,14 +182,6 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe agent._current_tool = tool_names_str agent._touch_activity(f"executing {num_tools} tools concurrently: {tool_names_str}") - # Capture CLI callbacks from the agent thread so worker threads can - # register them locally. Without this, _get_approval_callback() in - # terminal_tool returns None in ThreadPoolExecutor workers, causing - # the dangerous-command prompt to fall back to input() — which - # deadlocks against prompt_toolkit's raw terminal mode (#13617). - _parent_approval_cb = _get_approval_callback() - _parent_sudo_cb = _get_sudo_password_callback() - def _run_tool(index, tool_call, function_name, function_args): """Worker function executed in a thread.""" # Register this worker tid so the agent can fan out an interrupt ``` ```diff @@ -220,18 +208,9 @@ def _run_tool(index, tool_call, function_name, function_args): set_activity_callback(agent._touch_activity) except Exception: pass - # Propagate approval/sudo callbacks to this worker thread. - # Mirrors cli.py run_agent() pattern (GHSA-qg5c-hvr5-hjgr). - if _parent_approval_cb is not None: - try: - _set_approval_callback(_parent_approval_cb) - except Exception: - pass - if _parent_sudo_cb is not None: - try: - _set_sudo_password_callback(_parent_sudo_cb) - except Exception: - pass + # Approval/sudo callbacks (thread-local) and the agent turn's + # ContextVars are propagated by propagate_context_to_thread() at the + # submit site below (GHSA-qg5c-hvr5-hjgr, #13617). start = time.time() try: result = agent._invoke_tool( ``` ```diff @@ -261,13 +240,6 @@ def _run_tool(index, tool_call, function_name, function_args): _ra()._set_interrupt(False, _worker_tid) except Exception: pass - # Clear thread-local callbacks so a recycled worker thread - # doesn't hold stale references to a disposed CLI instance. - try: - _set_approval_callback(None) - _set_sudo_password_callback(None) - except Exception: - pass # Start spinner for CLI mode (skip when TUI handles tool progress) spinner = None ``` ```diff @@ -287,9 +259,12 @@ def _run_tool(index, tool_call, function_name, function_args): max_workers = min(len(runnable_calls), _MAX_TOOL_WORKERS) with concurrent.futures.ThreadPoolExecutor(max_workers=max_workers) as executor: for i, tc, name, args in runnable_calls: - # Propagate ContextVars (e.g. _approval_session_key); mirrors asyncio.to_thread. - ctx = contextvars.copy_context() - f = executor.submit(ctx.run, _run_tool, i, tc, name, args) + # Propagate the agent turn's ContextVars (e.g. + # _approval_session_key) AND thread-local approval/sudo + # callbacks into the worker thread; clears callbacks on exit. + f = executor.submit( + propagate_context_to_thread(_run_tool), i, tc, name, args + ) futures.append(f) # Wait for all to complete with periodic heartbeats so the ``` - `gateway/run.py` ```diff @@ -1807,7 +1807,34 @@ def __init__(self, config: Optional[GatewayConfig] = None): ensure_installed(log_failures=False) except Exception: pass # Non-fatal — fail-open at scan time if unavailable - + + # Startup heads-up (#30882): a gateway in manual approval mode with no + # automated risk assessor (tirith disabled AND no auxiliary.approval + # model) can only gate dangerous commands / execute_code scripts via + # live in-chat approval. With approval routing fixed, those actions now + # fail closed (block) rather than silently auto-running — surface that + # so operators knowingly enable tirith or configure auxiliary.approval + # for unattended gateways. + try: + from hermes_cli.config import load_config as _load_full_config + _appr_cfg = _load_full_config() + _appr_mode = str( + cfg_get(_appr_cfg, "approvals", "mode", default="manual") or "manual" + ).strip().lower() + _tirith_on = bool(cfg_get(_appr_cfg, "security", "tirith_enabled", default=True)) + _aux_approval = cfg_get(_appr_cfg, "auxiliary", "approval", default=None) + if _appr_mode == "manual" and not _tirith_on and not _aux_approval: + logger.warning( + "Gateway approvals.mode=manual with no automated risk " + "assessor (security.tirith_enabled is false and " + "auxiliary.approval is unset): dangerous commands and " + "execute_code scripts will BLOCK until a human approves " + "them in chat. Enable security.tirith_enabled or configure " + "auxiliary.approval for unattended operation." + ) + except Exception: + logger.debug("approvals.mode startup check skipped", exc_info=True) + # Initialize session database for session_search tool support self._session_db = None try: ``` - `tests/run_agent/test_tool_executor_contextvar_propagation.py` ```diff @@ -197,6 +197,19 @@ def test_run_agent_concurrent_executor_wraps_submit_with_copy_context(): and call.args[1].id == "_run_tool" ): tool_submits.append(("fixed", call)) + # Fixed (shared helper): executor.submit( + # propagate_context_to_thread(_run_tool), ...) — the helper in + # tools/thread_context.py does copy_context().run(...) internally and + # additionally propagates the thread-local approval/sudo callbacks. + elif ( + isinstance(first, ast.Call) + and isinstance(first.func, ast.Name) + and first.func.id == "propagate_context_to_thread" + and first.args + and isinstance(first.args[0], ast.Name) + and first.args[0].id == "_run_tool" + ): + tool_submits.append(("fixed", call)) assert tool_submits, ( "Could not locate `executor.submit(... _run_tool ...)` in " ``` - `tests/tools/test_code_execution_windows_env.py` ```diff @@ -253,20 +253,24 @@ def test_child_can_create_socket_with_scrubbed_env(self): # --------------------------------------------------------------------------- def _legacy_posix_scrubber(source_env, is_passthrough): - """Verbatim copy of the pre-Windows-fix inline scrubbing logic. - - This is the oracle used by TestPosixEquivalence to prove the refactor - did not change POSIX behavior. DO NOT edit this to "match" a future - production change — if _scrub_child_env's POSIX behavior legitimately - needs to evolve, delete this function and adjust the equivalence test - on purpose, so the churn is visible in review. + """Independent oracle for TestPosixEquivalence — a from-scratch reimpl of + _scrub_child_env's POSIX behavior, used to prove the production helper does + what we think it does. + + Deliberately updated for #27303 (the broad ``HERMES_`` prefix was dropped + in favor of an explicit operational allowlist, and DSN/WEBHOOK were added + to the secret substrings). The original docstring said: if POSIX behavior + legitimately needs to evolve, adjust this oracle on purpose so the churn is + visible in review — that is what this change is. """ _SAFE_ENV_PREFIXES = ("PATH", "HOME", "USER", "LANG", "LC_", "TERM", "TMPDIR", "TMP", "TEMP", "SHELL", "LOGNAME", - "XDG_", "PYTHONPATH", "VIRTUAL_ENV", "CONDA", - "HERMES_") + "XDG_", "PYTHONPATH", "VIRTUAL_ENV", "CONDA") _SECRET_SUBSTRINGS = ("KEY", "TOKEN", "SECRET", "PASSWORD", "CREDENTIAL", - "PASSWD", "AUTH") + "PASSWD", "AUTH", "DSN", "WEBHOOK") + _HERMES_CHILD_ALLOWED = frozenset({ + "HERMES_HOME", "HERMES_PROFILE", "HERMES_CONFIG", "HERMES_ENV", + }) out = {} for k, v in source_env.items(): if is_passthrough(k): ``` ```diff @@ -276,6 +280,9 @@ def _legacy_posix_scrubber(source_env, is_passthrough): continue if any(k.startswith(p) for p in _SAFE_ENV_PREFIXES): out[k] = v + continue + if k in _HERMES_CHILD_ALLOWED: + out[k] = v return out ``` ```diff @@ -308,13 +315,20 @@ class TestPosixEquivalence: "PYTHONPATH": "/opt/lib", "VIRTUAL_ENV": "/home/alice/.venv", "CONDA_PREFIX": "/opt/conda", - "HERMES_HOME": "/home/alice/.hermes", - "HERMES_INTERACTIVE": "1", + # HERMES_* handling (#27303): only the operational allowlist passes; + # every other HERMES_* is dropped (the broad prefix was removed). + "HERMES_HOME": "/home/alice/.hermes", # allowlisted → kept + "HERMES_PROFILE": "default", # allowlisted → kept + "HERMES_INTERACTIVE": "1", # not allowlisted → dropped + "HERMES_BASE_URL": "https://api.internal", # not allowlisted → dropped + "HERMES_KANBAN_DB": "postgres://[redacted-credentials]@h/db", # not allowlisted → dropped # Secret-substring blocks "OPENAI_API_KEY": "sk-xxx", "GITHUB_TOKEN": "ghp_xxx", "AWS_SECRET_ACCESS_KEY": "yyy", "MY_PASSWORD": "hunter2", + "SENTRY_DSN": "https://[redacted:email]/1", # DSN substring → blocked + "SLACK_WEBHOOK": "https://hooks.slack/x", # WEBHOOK substring → blocked # Uncategorized — must be dropped "RANDOM_UNKNOWN": "drop-me", "DISPLAY": ":0", ``` - `tests/tools/test_execute_code_approval_cluster.py` ```diff @@ -0,0 +1,349 @@ +"""Regression tests for the execute_code approval-bypass cluster. + +Covers the canonical fix for issues #4146, #27303, #30882, #33057: + + 1. tools.thread_context.propagate_context_to_thread — propagates the agent + turn's ContextVars AND thread-local approval/sudo callbacks into worker + threads, and clears the callbacks on teardown. + 2. Both execute_code RPC threads are wrapped with that helper (source guard). + 3. tools.approval.check_execute_code_guard — the entry-point guard decision + matrix (isolated backends, yolo/off, cron-deny, headless-local, + gateway approve/deny/timeout/missing-notify, smart mode). + 4. tools.code_execution_tool._scrub_child_env — broad HERMES_ prefix dropped, + operational allowlist kept, DSN/WEBHOOK blocked, passthrough precedence. +""" + +from __future__ import annotations + +import concurrent.futures +import contextvars +import threading + +import pytest + +from tools import approval as A +from tools.thread_context import propagate_context_to_thread + + +# --------------------------------------------------------------------------- +# 1. Context + callback propagation helper +# --------------------------------------------------------------------------- + +def test_helper_propagates_contextvar_and_approval_callback(): + from tools import terminal_tool as TT + + probe: contextvars.ContextVar[str] = contextvars.ContextVar( + "cluster_probe", default="unset" + ) + probe.set("parent-value") + sentinel = object() + TT.set_approval_callback(sentinel) + try: + seen: dict = {} + + def worker(): + seen["probe"] = probe.get() + seen["cb"] = TT._get_approval_callback() + + t = threading.Thread(target=propagate_context_to_thread(worker)) + t.start() + t.join(timeout=5) + + assert seen["probe"] == "parent-value" # ContextVar propagated + assert seen["cb"] is sentinel # thread-local callback propagated + finally: + TT.set_approval_callback(None) + + +def test_helper_clears_callbacks_on_teardown(): + """A recycled worker thread must not retain the propagated callback after + the wrapped target finishes (mirrors the GHSA-qg5c-hvr5-hjgr teardown).""" + from tools import terminal_tool as TT + + sentinel = object() + TT.set_approval_callback(sentinel) + try: + seen: dict = {} + + def first(): + seen["during"] = TT._get_approval_callback() + + def second(): # NOT wrapped — runs on the same recycled worker thread + seen["after"] = TT._get_approval_callback() + + with concurrent.futures.ThreadPoolExecutor(max_workers=1) as ex: + ex.submit(propagate_context_to_thread(first)).result(timeout=5) + ex.submit(second).result(timeout=5) + + assert seen["during"] is sentinel # installed for the wrapped target + assert seen["after"] is None # cleared on teardown + finally: + TT.set_approval_callback(None) + + +def test_both_rpc_threads_use_propagation_helper(): + """Source guard: both execute_code RPC threads must wrap their target with + propagate_context_to_thread, or the gateway approval bypass (#33057) + silently returns.""" + import inspect + import tools.code_execution_tool as cet + + src = inspect.getsource(cet) + assert "propagate_context_to_thread(_rpc_server_loop)" in src, ( + "local UDS RPC server thread is not wrapped with " + "propagate_context_to_thread — gateway approval routing will be lost." + ) + assert "propagate_context_to_thread(_rpc_poll_loop)" in src, ( + "remote file-RPC poll thread is not wrapped with " + "propagate_context_to_thread — gateway approval routing will be lost." + ) + + +# --------------------------------------------------------------------------- +# 3. check_execute_code_guard decision matrix +# --------------------------------------------------------------------------- + [redacted:email] +def gw_session(monkeypatch): + """A clean gateway session: HERMES_GATEWAY_SESSION set, a bound session + key, and isolated gateway queues/callbacks. Yields the session_key.""" + monkeypatch.setenv("HERMES_GATEWAY_SESSION", "1") + monkeypatch.delenv("HERMES_INTERACTIVE", raising=False) + monkeypatch.delenv("HERMES_CRON_SESSION", raising=False) + monkeypatch.delenv("HERMES_EXEC_ASK", raising=False) + # Force manual mode regardless of host config. + monkeypatch.setattr(A, "_get_approval_mode", lambda: "manual") + + session_key = "cluster-test-session" + token = [redacted:secret-assignment] + with A._lock: + A._gateway_queues.pop(session_key, None) + A._gateway_notify_cbs.pop(session_key, None) + try: + yield session_key + finally: + A.reset_current_session_key(token) + with A._lock: + A._gateway_queues.pop(session_key, None) + A._gateway_notify_cbs.pop(session_key, None) + + +def _register_resolver(session_key: str, result): + """Register a gateway notify callback that immediately resolves the most + recent queued approval entry with *result* (simulating a user response).""" + def cb(_approval_data): + with A._lock: + entries = A._gateway_queues.get(session_key, []) + if entries: + entry = entries[-1] + entry.result = result + entry.event.set() + with A._lock: + A._gateway_notify_cbs[session_key] = cb + + +def test_guard_isolated_backend_approved(): + # Container backends already sandbox the child — no-op approve. + assert A.check_execute_code_guard("import os", "docker")["approved"] is True + + +def test_guard_headless_local_approved(monkeypatch): + # Documented #30882 limitation: no approval surface → preserve auto-run. + monkeypatch.delenv("HERMES_GATEWAY_SESSION", raising=False) + monkeypatch.delenv("HERMES_INTERACTIVE", raising=False) + monkeypatch.delenv("HERMES_CRON_SESSION", raising=False) + monkeypatch.delenv("HERMES_EXEC_ASK", raising=False) + monkeypatch.setattr(A, "_get_approval_mode", lambda: "manual") + assert A.check_execute_code_guard("import os", "local")["approved"] is True + + +def test_guard_cron_deny_blocks(monkeypatch): + monkeypatch.setenv("HERMES_CRON_SESSION", "1") + monkeypatch.delenv("HERMES_GATEWAY_SESSION", raising=False) + monkeypatch.setattr(A, "_get_approval_mode", lambda: "manual") + monkeypatch.setattr(A, "_get_cron_approval_mode", lambda: "deny") + res = A.check_execute_code_guard("import os", "local") + assert res["approved"] is False + assert res["outcome"] == "blocked" + + +def test_guard_gateway_user_approves_is_one_shot(gw_session): + _register_resolver(gw_session, "once") + res = A.check_execute_code_guard("import os; print(1)", "local") + assert res["approved"] is True + assert res.get("user_approved") is True + # One-shot: approval must NOT persist to future scripts. + assert A.is_approved(gw_session, "execute_code") is False + + +def test_guard_gateway_user_denies_blocks(gw_session): + _register_resolver(gw_session, "deny") + res = A.check_execute_code_guard("import os", "local") + assert res["approved"] is False + assert res["outcome"] == "denied" + assert res["user_consent"] is False + + +def test_guard_gateway_timeout_blocks(gw_session, monkeypatch): + # Register a callback that never resolves; force an immediate timeout. + with A._lock: + A._gateway_notify_cbs[gw_session] = lambda _d: None + monkeypatch.setattr(A, "_get_approval_config", lambda: {"gateway_timeout": 0}) + res = A.check_execute_code_guard("import os", "local") + assert res["approved"] is False + assert res["outcome"] == "timeout" + + +def test_guard_gateway_missing_notify_is_pending(gw_session): + # No notify callback registered → backward-compat pending approval. + res = A.check_execute_code_guard("import os", "local") + assert res["approved"] is False + assert res["status"] == "pending_approval" + + +def test_guard_smart_mode(gw_session, monkeypatch): + monkeypatch.setattr(A, "_get_approval_mode", lambda: "smart") + + monkeypatch.setattr(A, "_smart_approve", lambda c, d: "approve") + res = A.check_execute_code_guard("import os", "local") + assert res["approved"] is True and res.get("smart_approved") is True + + monkeypatch.setattr(A, "_smart_approve", lambda c, d: "deny") + res = A.check_execute_code_guard("import os", "local") + assert res["approved"] is False and res.get("smart_denied") is True + + # escalate → falls through to manual gateway approval + monkeypatch.setattr(A, "_smart_approve", lambda c, d: "escalate") + _register_resolver(gw_session, "once") + res = A.check_execute_code_guard("import os", "local") + assert res["approved"] is True + + +def test_guard_session_yolo_bypasses(gw_session): + A.enable_session_yolo(gw_session) + try: + # Even with a denier registered, yolo short-circuits before the prompt. + _register_resolver(gw_session, "deny") + assert A.check_execute_code_guard("import os", "local")["approved"] is True + finally: + A.disable_session_yolo(gw_session) + + +# --------------------------------------------------------------------------- +# 4. Env scrubbing (#27303) +# --------------------------------------------------------------------------- + +def test_env_scrub_hermes_allowlist_and_secret_blocks(): + from tools.code_execution_tool import _scrub_child_env + + env = { + # operational allowlist → kept + "HERMES_HOME": "/h", "HERMES_PROFILE": "p", + "HERMES_CONFIG": "/c.yaml", "HERMES_ENV": "/e", + # other HERMES_* → dropped (broad prefix removed) + "HERMES_BASE_URL": "https://x", "HERMES_INTERACTIVE": "1", + "HERMES_KANBAN_DB": "postgres://[redacted-credentials]@h/db", + # secret substrings (incl. new DSN/WEBHOOK) → dropped + "SENTRY_DSN": "https://[redacted:email]/1", "SLACK_WEBHOOK": "https://h/x", + "OPENAI_API_KEY": "sk", "GITHUB_TOKEN": "ghp", + # safe prefix → kept; uncategorized → dropped + "PATH": "/usr/bin", "RANDOM_X": "y", + } + out = _scrub_child_env(env, is_passthrough=lambda _: False, is_windows=False) + + for kept in ("HERMES_HOME", "HERMES_PROFILE", "HERMES_CONFIG", "HERMES_ENV", "PATH"): + assert kept in out, f"{kept} should be kept" + for dropped in ( + "HERMES_BASE_URL", "HERMES_INTERACTIVE", "HERMES_KANBAN_DB", + "SENTRY_DSN", "SLACK_WEBHOOK", "OPENAI_API_KEY", "GITHUB_TOKEN", + "RANDOM_X", + ): + assert dropped not in out, f"{dropped} should be dropped" + + +def test_env_scrub_passthrough_overrides_secret_block(): + """A skill/config-declared passthrough var is an explicit user opt-in and + passes even if it matches a secret substring (precedence is intentional).""" + from tools.code_execution_tool import _scrub_child_env + + env = {"MY_SERVICE_DSN": "value"} + out = _scrub_child_env(env, is_passthrough=lambda k: k == "MY_SERVICE_DSN", + is_windows=False) + assert out.get("MY_SERVICE_DSN") == "value" + + +# --------------------------------------------------------------------------- +# 5. File-tool sensitive-path refusal (security B1) +# --------------------------------------------------------------------------- + +def test_execute_code_entry_blocks_before_spawn_when_guard_denies(monkeypatch, tmp_path): + """Behavioral wiring test: execute_code() consults the entry guard and, on + denial, returns the block message WITHOUT spawning the child — proven by a + marker file the script would create that never appears.""" + import json + + import tools.code_execution_tool as cet + from tools import terminal_tool as TT + + marker = tmp_path / "child-ran.marker" + monkeypatch.setenv("HERMES_CRON_SESSION", "1") + monkeypatch.delenv("HERMES_GATEWAY_SESSION", raising=False) + monkeypatch.delenv("HERMES_INTERACTIVE", raising=False) + monkeypatch.setattr(A, "_get_approval_mode", lambda: "manual") + monkeypatch.setattr(A, "_get_cron_approval_mode", lambda: "deny") + monkeypatch.setattr(TT, "_get_env_config", lambda: {"env_type": "local"}) + + result = json.loads( + cet.execute_code(f"open({str(marker)!r}, 'w').close()", task_id="cluster-t") + ) + assert result["status"] == "error" + assert "BLOCKED" in result["error"] + assert not marker.exists() # guard denied before the child was spawned + + +# --------------------------------------------------------------------------- +# 6. Env-scrub diagnosability mitigation (#27303 follow-up) +# --------------------------------------------------------------------------- + +def test_env_scrub_logs_dropped_hermes_vars(caplog): + """Dropping a non-allowlisted, non-secret HERMES_* var must be diagnosable: + the scrub emits a one-shot debug log naming the dropped vars and pointing at + the env_passthrough opt-in, so the silent behavior change (#27303) doesn't + leave users guessing why a sandbox script sees an unset HERMES_* var.""" + import logging + + from tools.code_execution_tool import _scrub_child_env + + env = { + "HERMES_HOME": "/h", # allowlisted → kept, not logged + "HERMES_BASE_URL": "https://x", # dropped → logged + "HERMES_KANBAN_DB": "postgres://[redacted-credentials]@h/db", # dropped → logged + "HERMES_API_KEY": "sk", # secret → dropped silently (not logged) + "PATH": "/usr/bin", # safe prefix → kept + } + with caplog.at_level(logging.DEBUG, logger="tools.code_execution_tool"): + out = _scrub_child_env(env, is_passthrough=lambda _: False, is_windows=False) + + assert "HERMES_HOME" in out and "PATH" in out + assert "HERMES_BASE_URL" not in out and "HERMES_KANBAN_DB" not in out + + msgs = "\n".join(r.getMessage() for r in caplog.records) + assert "HERMES_BASE_URL" in msgs and "HERMES_KANBAN_DB" in msgs + assert "env_passthrough" in msgs + # Secret vars are dropped but must NOT be named in the diagnostic log. + assert "HERMES_API_KEY" not in msgs + + +def test_env_scrub_no_log_when_nothing_dropped(caplog): + """No diagnostic noise when there are no dropped HERMES_* vars.""" + import logging + + from tools.code_execution_tool import _scrub_child_env + + with caplog.at_level(logging.DEBUG, logger="tools.code_execution_tool"): + _scrub_child_env( + {"HERMES_HOME": "/h", "PATH": "/usr/bin"}, + is_passthrough=lambda _: False, + is_windows=False, + ) + assert "dropped" not in "\n".join(r.getMessage() for r in caplog.records) ``` - `tools/approval.py` ```diff @@ -1057,6 +1057,107 @@ def _format_tirith_description(tirith_result: dict) -> str: return "Security scan — " + "; ".join(parts) +def _await_gateway_decision(session_key: str, notify_cb, approval_data: dict, + *, surface: str = "gateway") -> dict: + """Enqueue *approval_data*, notify the user, and block the calling agent + thread until the request is resolved or the gateway approval timeout + elapses — firing pre/post approval hooks and cleaning up the queue entry. + + Shared by the terminal command guard (``check_all_command_guards``) and + the execute_code guard (``check_execute_code_guard``) so the fiddly + heartbeat-polling wait loop lives in one place. + + Returns ``{"resolved": bool, "choice": str|None}`` on completion, or + ``{"resolved": False, "choice": None, "notify_failed": True}`` if the + notify callback raised. Persistence of an approved choice and building + the final tool-facing result dict remain the caller's responsibility. + """ + command = approval_data.get("command", "") + description = approval_data.get("description", "") + primary_key = approval_data.get("pattern_key", "") + all_keys = approval_data.get("pattern_keys", [primary_key]) + + entry = _ApprovalEntry(approval_data) + with _lock: + _gateway_queues.setdefault(session_key, []).append(entry) + + def _drop_entry() -> None: + with _lock: + queue = _gateway_queues.get(session_key, []) + if entry in queue: + queue.remove(entry) + if not queue: + _gateway_queues.pop(session_key, None) + + # Notify plugins that an approval is being requested. Fires before the + # gateway notify callback so observers get the event in real time. + _fire_approval_hook( + "pre_approval_request", + command=command, + description=description, + pattern_key=primary_key, + pattern_keys=list(all_keys), + session_key=session_key, + surface=surface, + ) + + # Notify the user (bridges sync agent thread → async gateway) + try: + notify_cb(approval_data) + except Exception as exc: + logger.warning("Gateway approval notify failed: %s", exc) + _drop_entry() + return {"resolved": False, "choice": None, "notify_failed": True} + + # Block until the user responds or timeout (default 5 min). Poll in short + # slices so we can fire activity heartbeats every ~10s to the agent's + # inactivity tracker — otherwise the gateway watchdog kills the agent + # while the user is still responding. Mirrors _wait_for_process() cadence. + timeout = _get_approval_config().get("gateway_timeout", 300) + try: + timeout = int(timeout) + except (ValueError, TypeError): + timeout = 300 + + try: + from tools.environments.base import touch_activity_if_due + except Exception: # pragma: no cover + touch_activity_if_due = None + + _now = time.monotonic() + _deadline = _now + max(timeout, 0) + _activity_state = {"last_touch": _now, "start": _now} + resolved = False + while True: + _remaining = _deadline - time.monotonic() + if _remaining <= 0: + break + if entry.event.wait(timeout=min(1.0, _remaining)): + resolved = True + break + if touch_activity_if_due is not None: + touch_activity_if_due(_activity_state, "waiting for user approval") + + _drop_entry() + + choice = entry.result + # Normalize outcome for the post hook. Unresolved (timeout) and None both + # mean the user never responded; report that explicitly so plugins can + # distinguish timeout from explicit deny. + _outcome = "timeout" if not resolved else (choice if choice else "timeout") + _fire_approval_hook( + "post_approval_response", + command=command, + description=description, + pattern_key=primary_key, + pattern_keys=list(all_keys), + session_key=session_key, + surface=surface, + choice=_outcome, + ) + return {"resolved": resolved, "choice": choice} + + def check_all_command_guards(command: str, env_type: str, approval_callback=None) -> dict: """Run all pre-exec security checks and return a single approval decision. ``` ```diff @@ -1207,113 +1308,27 @@ def check_all_command_guards(command: str, env_type: str, if notify_cb is not None: # --- Blocking gateway approval (queue-based) --- - # Each call gets its own _ApprovalEntry so parallel subagents - # and execute_code threads can block concurrently. + # Block the agent thread until the user responds; the notify + + # heartbeat wait loop is shared with check_execute_code_guard via + # _await_gateway_decision(). approval_data = { "command": command, "pattern_key": primary_key, "pattern_keys": all_keys, "description": combined_desc, } - entry = _ApprovalEntry(approval_data) - with _lock: - _gateway_queues.setdefault(session_key, []).append(entry) - - # Notify plugins that an approval is being requested. Fires before - # the gateway notify callback so observers (e.g. macOS notifier - # plugins, audit logs, Slack alerts) get the event in real time. - _fire_approval_hook( - "pre_approval_request", - command=command, - description=combined_desc, - pattern_key=primary_key, - pattern_keys=list(all_keys), - session_key=session_key, - surface="gateway", + decision = _await_gateway_decision( + session_key, notify_cb, approval_data, surface="gateway" ) - - # Notify the user (bridges sync agent thread → async gateway) - try: - notify_cb(approval_data) - except Exception as exc: - logger.warning("Gateway approval notify failed: %s", exc) - with _lock: - queue = _gateway_queues.get(session_key, []) - if entry in queue: - queue.remove(entry) - if not queue: - _gateway_queues.pop(session_key, None) + if decision.get("notify_failed"): return { "approved": False, "message": "BLOCKED: Failed to send approval request to user. Do NOT retry.", "pattern_key": primary_key, "description": combined_desc, } - - # Block until the user responds or timeout (default 5 min). - # Poll in short slices so we can fire activity heartbeats every - # ~10s to the agent's inactivity tracker. Without this, the - # blocking event.wait() never touches activity, and the - # gateway's inactivity watchdog (agent.gateway_timeout, default - # 1800s) kills the agent while the user is still responding to - # the approval prompt. Mirrors the _wait_for_process() cadence - # in tools/environments/base.py. - timeout = _get_approval_config().get("gateway_timeout", 300) - try: - timeout = int(timeout) - except (ValueError, TypeError): - timeout = 300 - - try: - from tools.environments.base import touch_activity_if_due - except Exception: # pragma: no cover - touch_activity_if_due = None - - _now = time.monotonic() - _deadline = _now + max(timeout, 0) - _activity_state = {"last_touch": _now, "start": _now} - resolved = False - while True: - _remaining = _deadline - time.monotonic() - if _remaining <= 0: - break - # 1s poll slice — the event is set immediately when the - # user responds, so slice length only controls heartbeat - # cadence, not user-visible responsiveness. - if entry.event.wait(timeout=min(1.0, _remaining)): - resolved = True - break - if touch_activity_if_due is not None: - touch_activity_if_due( - _activity_state, "waiting for user approval" - ) - - # Clean up this entry from the queue - with _lock: - queue = _gateway_queues.get(session_key, []) - if entry in queue: - queue.remove(entry) - if not queue: - _gateway_queues.pop(session_key, None) - - choice = entry.result - # Normalize outcome for the post hook. Unresolved (timeout) and - # None both mean the user never responded; report that explicitly - # so plugins can distinguish timeout from explicit deny. - _outcome = ( - "timeout" if not resolved - else (choice if choice else "timeout") - ) - _fire_approval_hook( - "post_approval_response", - command=command, - description=combined_desc, - pattern_key=primary_key, - pattern_keys=list(all_keys), - session_key=session_key, - surface="gateway", - choice=_outcome, - ) + resolved = decision["resolved"] + choice = decision["choice"] if not resolved or choice is None or choice == "deny": # Consent contract: silence is NOT consent, and an explicit ``` ```diff @@ -1437,5 +1452,173 @@ def check_all_command_guards(command: str, env_type: str, "user_approved": True, "description": combined_desc} +def check_execute_code_guard(code: str, env_type: str) -> dict: + """Approve an execute_code script before its child process is spawned. + + execute_code runs arbitrary local Python — the script can call + ``subprocess``, ``os.system``, ``ctypes``, or other process/file APIs + directly, none of which pass through ``terminal()`` / + ``DANGEROUS_PATTERNS``. In gateway/ask contexts we fail closed by approving + the script as a whole before it runs (#30882). Returns the same dict + contract as ``check_all_command_guards``. + + Scope (documented limitation, #30882): in a purely local non-interactive + non-gateway session (no TTY, not gateway, not cron-deny) this returns + approved — matching the existing terminal auto-approve contract. The + hardline floor still blocks catastrophic ``terminal()`` commands the script + issues; running arbitrary code headlessly without any approval surface is + trusted-by-config (set a gateway/ask surface or ``approvals.cron_mode`` to + require approval). + """ + pattern_key = "execute_code" + description = ( + "execute_code script execution. The script can spawn subprocesses or " + "mutate files without passing through terminal command approval; " + "approval is one-shot for this run." + ) + + # Isolated backends already sandbox the child — matches the container skip + # in check_all_command_guards / check_dangerous_command. + if env_type in {"docker", "singularity", "modal", "daytona", "vercel_sandbox"}: + return {"approved": True, "message": None} + + # --yolo or approvals.mode=off: bypass (session- or process-scoped). + approval_mode = _get_approval_mode() + if _YOLO_MODE_FROZEN or is_current_session_yolo_enabled() or approval_mode == "off": + return {"approved": True, "message": None} + + is_gateway = _is_gateway_approval_context() + is_ask = env_var_enabled("HERMES_EXEC_ASK") + + # Cron: no user is present to approve arbitrary code. + if env_var_enabled("HERMES_CRON_SESSION"): + if _get_cron_approval_mode() == "deny": + return { + "approved": False, + "message": ( + "BLOCKED: execute_code runs arbitrary local Python " + "(including subprocess calls that bypass shell-string " + "approval checks). Cron jobs run without a user present " + "to approve it. Use normal tools instead, or set " + "approvals.cron_mode: approve only if this cron profile " + "is intentionally trusted." + ), + "pattern_key": pattern_key, + "description": description, + "outcome": "blocked", + "user_consent": False, + } + return {"approved": True, "message": None} + + # Only gateway/ask contexts get the one-shot whole-script approval. + # * CLI interactive: the script's terminal() calls are guarded per-call + # (context now propagates into the RPC thread, #33057); a whole-script + # prompt would fire on every execute_code call. + # * Local non-interactive non-gateway: documented limitation above. + if not is_gateway and not is_ask: + return {"approved": True, "message": None} + + session_key = get_current_session_key() + # Built only now (past the early-return gates) so the common non-approval + # paths don't pay to copy a potentially-large script into this string. + command = f"execute_code <<'PY'\n{code}\nPY" + + # Smart mode: ask the aux LLM about the whole script. An APPROVE here only + # suppresses the redundant whole-script prompt; the per-call terminal() + # guards (restored by context propagation) still run independently. + if approval_mode == "smart": + verdict = _smart_approve(command, description) + if verdict == "approve": + logger.debug("Smart approval: auto-approved execute_code for session %s", + session_key) + return {"approved": True, "message": None, + "smart_approved": True, "description": description} + if verdict == "deny": + return { + "approved": False, + "message": ("BLOCKED by smart approval: execute_code script " + "execution was assessed as genuinely dangerous. " + "Do NOT retry."), + "smart_denied": True, + "pattern_key": pattern_key, + "description": description, + "outcome": "denied", + "user_consent": False, + } + # verdict == "escalate" → fall through to manual approval + + notify_cb = None + with _lock: + notify_cb = _gateway_notify_cbs.get(session_key) + + if notify_cb is None: + # No gateway callback registered (e.g. ask-mode without a notifier): + # surface a pending approval for backward compatibility. + submit_pending(session_key, { + "command": command, + "pattern_key": pattern_key, + "pattern_keys": [pattern_key], + "description": description, + }) + return { + "approved": False, + "pattern_key": pattern_key, + "status": "pending_approval", + "approval_pending": True, + "command": command, + "description": description, + "message": ( + f"⚠️ {description}. Asking the user for approval.\n\n" + f"**Code:**\n```python\n{code}\n```" + ), + } + + approval_data = { + "command": command, + "pattern_key": pattern_key, + "pattern_keys": [pattern_key], + "description": description, + } + decision = _await_gateway_decision( + session_key, notify_cb, approval_data, surface="gateway" + ) + if decision.get("notify_failed"): + return { + "approved": False, + "message": ("BLOCKED: Failed to send execute_code approval request " + "to user. Do NOT retry."), + "pattern_key": pattern_key, + "description": description, + "outcome": "notify_failed", + "user_consent": False, + } + + resolved = decision["resolved"] + choice = decision["choice"] + + if not resolved or choice is None or choice == "deny": + reason = "timed out without user response" if not resolved else "denied by user" + addendum = " Silence is not consent." if not resolved else "" + return { + "approved": False, + "message": ( + f"BLOCKED: execute_code script {reason}. The user has NOT " + f"consented to running this code. Do NOT retry, do NOT rephrase " + f"the script, and do NOT attempt the same outcome via a " + f"different tool.{addendum}" + ), + "pattern_key": pattern_key, + "description": description, + "outcome": "timeout" if not resolved else "denied", + "user_consent": False, + } + + # Approved — one-shot only. Deliberately NO approve_session/approve_permanent: + # each execute_code script is distinct arbitrary code, so approval never + # persists to future scripts. + return {"approved": True, "message": None, + "user_approved": True, "description": description} + + # Load permanent allowlist from config on module import load_permanent_allowlist() ``` - `tools/code_execution_tool.py` ```diff @@ -46,6 +46,8 @@ _IS_WINDOWS = platform.system() == "Windows" from typing import Any, Dict, List, Optional +from tools.thread_context import propagate_context_to_thread + # Availability gate. On Windows we fall back to loopback TCP for the # sandbox RPC transport (AF_UNIX is unreliable on Windows Python) — see # ``_use_tcp_rpc`` in ``_execute_local`` below. That makes execute_code ``` ```diff @@ -74,13 +76,30 @@ # Environment variable scrubbing rules (shared between the local + remote # backends). Secret-substring block is applied first; anything left must -# match either a safe prefix or, on Windows, an OS-essential name. +# match a safe prefix, the operational HERMES_ allowlist, or (on Windows) an +# OS-essential name. +# +# NB: the broad "HERMES_" prefix was deliberately removed (#27303) — it leaked +# HERMES_*-named config that lacks a secret substring (e.g. HERMES_BASE_URL, +# HERMES_KANBAN_DB, HERMES_*_WEBHOOK). The child only needs the few +# location/profile vars in _HERMES_CHILD_ALLOWED below; HERMES_RPC_SOCKET / +# HERMES_RPC_DIR / TZ / HOME are injected explicitly after scrubbing. _SAFE_ENV_PREFIXES = ("PATH", "HOME", "USER", "LANG", "LC_", "TERM", "TMPDIR", "TMP", "TEMP", "SHELL", "LOGNAME", - "XDG_", "PYTHONPATH", "VIRTUAL_ENV", "CONDA", - "HERMES_") + "XDG_", "PYTHONPATH", "VIRTUAL_ENV", "CONDA") _SECRET_SUBSTRINGS = ("KEY", "TOKEN", "SECRET", "PASSWORD", "CREDENTIAL", - "PASSWD", "AUTH") + "PASSWD", "AUTH", "DSN", "WEBHOOK") + +# Operational HERMES_* vars the child legitimately needs by exact name — these +# are non-secret runtime-location flags (the same set hermes_cli treats as the +# runtime location) that repo-root modules a sandbox script imports may read at +# import time. None match _SECRET_SUBSTRINGS. +_HERMES_CHILD_ALLOWED = frozenset({ + "HERMES_HOME", + "HERMES_PROFILE", + "HERMES_CONFIG", + "HERMES_ENV", +}) # Windows-only: a handful of variables are required by the OS/CRT itself. # Without them, even stdlib calls like ``socket.socket()`` fail with ``` ```diff @@ -119,9 +138,10 @@ def _scrub_child_env(source_env, is_passthrough=None, is_windows=None): Rules (order matters): 1. Passthrough vars (skill- or config-declared) always pass. - 2. Secret-substring names (KEY/TOKEN/etc.) are blocked. + 2. Secret-substring names (KEY/TOKEN/DSN/WEBHOOK/etc.) are blocked. 3. Names matching a safe prefix pass. - 4. On Windows, a small OS-essential allowlist passes by exact name + 4. Operational HERMES_* vars (_HERMES_CHILD_ALLOWED) pass by exact name. + 5. On Windows, a small OS-essential allowlist passes by exact name — without these the child can't even create a socket or spawn a subprocess. ``` ```diff @@ -138,6 +158,14 @@ def _scrub_child_env(source_env, is_passthrough=None, is_windows=None): is_windows = _IS_WINDOWS scrubbed = {} + # Non-secret HERMES_* vars dropped by the tightened allowlist (#27303). The + # broad "HERMES_" prefix used to pass these through; now only the + # operational set does. The drop is intentional (those vars can carry + # config like HERMES_KANBAN_DB / HERMES_BASE_URL), but a sandbox script + # that imports a repo module reading one at import time would otherwise see + # it silently unset. Surface the drop once so the behavior change is + # diagnosable and points at the env_passthrough opt-in escape hatch. + _dropped_hermes = [] for k, v in source_env.items(): if is_passthrough(k): scrubbed[k] = v ``` ```diff @@ -147,8 +175,25 @@ def _scrub_child_env(source_env, is_passthrough=None, is_windows=None): if any(k.startswith(p) for p in _SAFE_ENV_PREFIXES): scrubbed[k] = v continue + if k in _HERMES_CHILD_ALLOWED: + scrubbed[k] = v + continue if is_windows and k.upper() in _WINDOWS_ESSENTIAL_ENV_VARS: scrubbed[k] = v + continue + if k.startswith("HERMES_"): + # Non-secret (secrets were already dropped above) and not in any + # allowlist — a deliberately-dropped HERMES_* var. + _dropped_hermes.append(k) + if _dropped_hermes: + logger.debug( + "execute_code: dropped %d non-allowlisted HERMES_* var(s) from the " + "sandbox child env (%s). This is intentional hardening (#27303); if " + "a sandbox script legitimately needs one, declare it via " + "env_passthrough in the skill/config so it passes by explicit opt-in.", + len(_dropped_hermes), + ", ".join(sorted(_dropped_hermes)), + ) return scrubbed ``` ```diff @@ -887,9 +932,11 @@ def _execute_remote( _ship_file_to_remote(env, f"{sandbox_dir}/hermes_tools.py", tools_src) _ship_file_to_remote(env, f"{sandbox_dir}/script.py", code) - # Start RPC polling thread + # Wrapped so the thread inherits the turn's approval context + callbacks + # (see tools.thread_context) — else sandbox RPC tool calls lose approval + # routing (#33057). rpc_thread = threading.Thread( - target=_rpc_poll_loop, + target=propagate_context_to_thread(_rpc_poll_loop), args=( env, f"{sandbox_dir}/rpc", effective_task_id, tool_call_log, tool_call_counter, max_tool_calls, ``` ```diff @@ -1049,6 +1096,21 @@ def execute_code( # Dispatch: remote backends use file-based RPC, local uses UDS from tools.terminal_tool import _get_env_config env_type = _get_env_config()["env_type"] + + # execute_code runs arbitrary Python (subprocess/os.system/...) that never + # passes through terminal()/DANGEROUS_PATTERNS, so guard the whole script + # here before either dispatch path spawns it. Runs synchronously in the + # caller (tool-executor) thread, which holds the session context (#30882). + from tools.approval import check_execute_code_guard + _guard = check_execute_code_guard(code, env_type) + if not _guard.get("approved", False): + return json.dumps({ + "status": "error", + "error": _guard.get("message") or "execute_code blocked by approval guard.", + "tool_calls_made": 0, + "duration_seconds": 0, + }, ensure_ascii=False) + if env_type != "local": return _execute_remote(code, task_id, enabled_tools) ``` ```diff @@ -1135,8 +1197,11 @@ def execute_code( os.chmod(sock_path, 0o600) server_sock.listen(1) + # Wrapped so the thread inherits the turn's approval context + callbacks + # (see tools.thread_context) — else gateway sandbox tool calls silently + # auto-approve dangerous commands (#33057, #30882). rpc_thread = threading.Thread( - target=_rpc_server_loop, + target=propagate_context_to_thread(_rpc_server_loop), args=( server_sock, task_id, tool_call_log, tool_call_counter, max_tool_calls, sandbox_tools, ``` - `tools/thread_context.py` ```diff @@ -0,0 +1,120 @@ +#!/usr/bin/env python3 +"""Propagate agent-turn context into worker threads that dispatch Hermes tools. + +A bare ``threading.Thread`` / ``ThreadPoolExecutor`` worker starts with an +empty ``contextvars.Context`` and no thread-local approval/sudo callbacks. +Tool dispatch inside such a thread therefore silently loses: + + * the approval *session/platform* ContextVars (``tools.approval`` / + ``gateway.session_context``) — so gateway sessions fall into + ``check_dangerous_command``'s non-interactive auto-approve branch and + dangerous commands run without prompting (#33057, #30882); + * the thread-local CLI approval/sudo callbacks (``tools.terminal_tool``) — + so ``prompt_dangerous_approval`` cannot reach the user + (GHSA-qg5c-hvr5-hjgr, #15216). + +This helper factors out that capture/install/clear lifecycle so the several +places that fan tool dispatch onto worker threads (``agent.tool_executor`` and +the ``execute_code`` RPC threads) share one audited implementation instead of +divergent copies. + +Usage — call :func:`propagate_context_to_thread` **on the parent thread** +(it snapshots the parent's ContextVars and callbacks at call time) and use the +returned callable as the worker's target:: + + t = threading.Thread(target=propagate_context_to_thread(loop_fn), args=(...)) + # or + executor.submit(propagate_context_to_thread(worker_fn), *args) + +Approval/sudo callbacks are installed for the worker's lifetime and **always +cleared on exit**, so a recycled thread never holds a stale reference to a +disposed CLI instance. +""" + +from __future__ import annotations + +import contextvars +import logging +from typing import Callable + +logger = logging.getLogger(__name__) + + +def _callback_api(): + """Resolve the terminal_tool callback getters/setters. + + Imported lazily: ``tools.terminal_tool`` imports ``tools.approval`` at + module load, so a top-level import here would risk an import cycle for + callers that live in ``tools.approval``. + """ + from tools.terminal_tool import ( + _get_approval_callback, + _get_sudo_password_callback, + set_approval_callback, + set_sudo_password_callback, + ) + return ( + _get_approval_callback, + _get_sudo_password_callback, + set_approval_callback, + set_sudo_password_callback, + ) + + +def propagate_context_to_thread(target: Callable) -> Callable: + """Wrap *target* for execution on a worker thread with the *current* + thread's ContextVars and approval/sudo callbacks propagated. + + Call this on the parent thread; pass the returned callable as the + thread/executor target. The returned callable forwards its positional + and keyword arguments to *target* and returns its result. + + Fail-closed: if callback installation raises, the callbacks are left + unset (``None``). That is the safe outcome — ``prompt_dangerous_approval`` + denies dangerous commands when no callback is registered in an interactive + context, and the gateway approval queue blocks when its notify callback is + absent. + """ + ctx = contextvars.copy_context() + parent_approval_cb = parent_sudo_cb = None + setters = None + try: + get_approval, get_sudo, set_approval, set_sudo = _callback_api() + parent_approval_cb = get_approval() + parent_sudo_cb = get_sudo() + setters = (set_approval, set_sudo) + except Exception: + logger.debug("Could not capture parent approval/sudo callbacks", exc_info=True) + + def _runner(*args, **kwargs): + def _inner(): + if setters is not None: + set_approval, set_sudo = setters + try: + if parent_approval_cb is not None: + set_approval(parent_approval_cb) + if parent_sudo_cb is not None: + set_sudo(parent_sudo_cb) + except Exception: + logger.debug( + "Failed to install propagated approval/sudo callbacks; " + "dangerous-command approval will fail closed", + exc_info=True, + ) + try: + return target(*args, **kwargs) + finally: + if setters is not None: + set_approval, set_sudo = setters + try: + set_approval(None) + set_sudo(None) + except Exception: + logger.debug( + "Failed to clear propagated approval/sudo callbacks", + exc_info=True, + ) + + return ctx.run(_inner) + + return _runner ``` - **#22300: fix(mcp): drop broken approval tools from MCP surface**. open - maintainer direction: category `uncategorized`; implication `needs_llm` - files: `mcp_serve.py`, `tests/test_mcp_serve.py` - **#30893: \[codex\] Guard execute_code behind gateway approvals**. closed without merge - maintainer direction: category `superseded_by_pr`; implication `superseded`; refs `#34497` - **#34131: fix(security): restore approval/sudo context in execute_code RPC threads + guard entry points**. closed without merge - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `21aeefe5f`, `#4146`, `#27303`, `#30882`, `#33057`, `#4146`, `#27303`, `#30882`, `#33057`, `#34594` - **#56971: fix(mcp): wire bridge approval tools to the gateway via a cross-process handshake**. open - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `fe3cb2355` - files: `mcp_serve.py`, `tests/test_mcp_serve.py`, `tests/test_mcp_serve_approvals.py`, `tests/tools/test_approval_external_resolution.py`, `tools/approval.py`, `tools/approval_gateway_wait.py`, `website/docs/user-guide/features/mcp.md` - **#60544: fix(plugins): re-enable pre_tool_call approve escalation with correct gateway notify path (#59201)**. closed without merge - maintainer direction: category `stale_abandoned`; implication `active` - **#63189: fix(kanban): drop approval context env vars from spawned workers**. open - maintainer direction: category `partial_fix`; implication `needs_llm`; refs `#63183`, `#55945`, `#55946`, `#37968`, `#56771`, `#58662`, `#59179`, `#21563`, `#63183` - files: `hermes_cli/kanban_db.py` - **#63342: fix(mcp): unblock bridge approval responses**. open - maintainer direction: category `partial_fix`; implication `needs_llm`; refs `ee5e40c638ec`, `#59260`, `#63342`, `#63342`, `#63342`, `#63342`, `#56971`, `#63342`, `#56971`, `#63342`, `#56971`, `#374151`, `#59260`, `#63342` - files: `mcp_serve.py`, `tests/test_mcp_serve.py`, `tools/approval.py` - **#63421: fix(kanban): broker detached worker approvals across processes**. open - maintainer direction: category `silent_close`; implication `active` - files: `agent/agent_init.py`, `agent/agent_runtime_helpers.py`, `agent/conversation_loop.py`, `agent/curator.py`, `agent/execution_context.py`, `agent/tool_executor.py`, `agent/turn_context.py`, `agent/turn_finalizer.py`, `cli-config.yaml.example`, `cli.py`, `cron/scheduler.py`, `docs/middleware/README.md`, `gateway/kanban_watchers.py`, `gateway/session_context.py`, `gateway/slash_commands.py`, `hermes_cli/_parser.py`, `hermes_cli/cli_agent_setup_mixin.py`, `hermes_cli/commands.py`, `hermes_cli/config.py`, `hermes_cli/kanban.py` (+60 more) - **#65325: fix(delegation): honor worker approval callbacks in gateways**. closed without merge - maintainer direction: category `superseded_by_pr`; implication `superseded`; refs `#65415` - **#83771: fix(mcp): drop broken approval tools from MCP surface**. closed without merge - maintainer direction: category `duplicate_of`; implication `duplicate`; refs `#22300`, `#56971`, `#63342` ## Fix status - Merged to main in PR #34497 (cited above). No release data is tracked. Verify downstream. - Fix last verified (merged) `2026-05-29`. - Subsystem: `agent`