fix(vault): carry the unlock prompt onto tool worker threads; honour binary_path in install checks
Live Desktop repro: the fixture model called browser_vault_unlock and got unlock_unavailable although the renderer was interactive. tool_executor runs handlers on a propagated worker thread; thread_context only copied the approval and sudo thread-local callbacks, so the unlock prompt registered by _wire_callbacks was invisible there and can_prompt_here() said nobody could answer. The callback table in thread_context now lists every per-thread prompt (approval, sudo, vault unlock) so a new one cannot silently drop off worker threads again. After the fix the same turn shows the masked card and completes. vault.sources / `hermes vault sources` report a manager as installed when its configured binary_path exists, not only when it is on PATH.
This commit is contained in:
@@ -11,6 +11,7 @@ from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
from abc import ABC, abstractmethod
|
||||
from pathlib import Path
|
||||
from typing import Dict, List, Optional, Sequence
|
||||
|
||||
from agent.vault_store import VaultItemMeta
|
||||
@@ -67,15 +68,32 @@ def _cfg() -> Dict:
|
||||
return cfg if isinstance(cfg, dict) else {}
|
||||
|
||||
|
||||
def external_backend_classes():
|
||||
from agent.vault_backends.bitwarden import BitwardenLoginBackend
|
||||
from agent.vault_backends.onepassword import OnePasswordLoginBackend
|
||||
return (OnePasswordLoginBackend, BitwardenLoginBackend)
|
||||
|
||||
|
||||
def is_installed(name: str) -> bool:
|
||||
"""Is the manager CLI reachable — honouring a configured ``binary_path`` over PATH."""
|
||||
import shutil
|
||||
section = _cfg().get(name) or {}
|
||||
explicit = str(section.get("binary_path") or "") if isinstance(section, dict) else ""
|
||||
if explicit:
|
||||
return Path(explicit).is_file()
|
||||
if name == "onepassword":
|
||||
from agent.secret_sources.onepassword import find_op
|
||||
return find_op() is not None
|
||||
return shutil.which("bw") is not None
|
||||
|
||||
|
||||
def enabled_backends() -> List[LoginBackend]:
|
||||
"""Local first (always on), then each enabled external manager, in config order."""
|
||||
from agent.vault_backends.bitwarden import BitwardenLoginBackend
|
||||
from agent.vault_backends.local import LocalLoginBackend
|
||||
from agent.vault_backends.onepassword import OnePasswordLoginBackend
|
||||
|
||||
cfg = _cfg()
|
||||
out: List[LoginBackend] = [LocalLoginBackend()]
|
||||
for cls in (OnePasswordLoginBackend, BitwardenLoginBackend):
|
||||
for cls in external_backend_classes():
|
||||
section = cfg.get(cls.name) or {}
|
||||
if isinstance(section, dict) and section.get("enabled"):
|
||||
out.append(cls(section))
|
||||
|
||||
@@ -135,16 +135,12 @@ def _cmd_list(args) -> None:
|
||||
|
||||
def _cmd_sources(args) -> None:
|
||||
"""Show/enable/disable the external password managers (`vault.<name>.enabled`)."""
|
||||
import shutil
|
||||
|
||||
from agent.secret_sources.onepassword import find_op
|
||||
from agent.vault_backends import enabled_backends
|
||||
from agent.vault_backends.bitwarden import BitwardenLoginBackend
|
||||
from agent.vault_backends.onepassword import OnePasswordLoginBackend
|
||||
from agent.vault_backends.base import external_backend_classes, is_installed
|
||||
from hermes_cli.config import load_config, save_config
|
||||
|
||||
c = _console()
|
||||
classes = {cls.name: cls for cls in (OnePasswordLoginBackend, BitwardenLoginBackend)}
|
||||
classes = {cls.name: cls for cls in external_backend_classes()}
|
||||
if args.enable or args.disable:
|
||||
name = args.enable or args.disable
|
||||
if name not in classes:
|
||||
@@ -159,10 +155,9 @@ def _cmd_sources(args) -> None:
|
||||
c.print("[dim]Run `bw login` once in a terminal first; Hermes only ever unlocks, never logs in.[/]")
|
||||
return
|
||||
enabled = {b.name for b in enabled_backends()}
|
||||
installed = {"onepassword": find_op() is not None, "bitwarden": shutil.which("bw") is not None}
|
||||
for name, cls in classes.items():
|
||||
status = "[green]on[/]" if name in enabled else "[dim]off[/]"
|
||||
cli = "" if installed[name] else " [yellow](CLI not found)[/]"
|
||||
cli = "" if is_installed(name) else " [yellow](CLI not found)[/]"
|
||||
c.print(f" {cls.display_name:<10} {status}{cli}")
|
||||
c.print("[dim]Toggle with `hermes vault sources --enable onepassword` / `--disable bitwarden`.[/]")
|
||||
|
||||
|
||||
@@ -19,16 +19,21 @@ logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _callback_api():
|
||||
"""Resolve the terminal_tool callback getters/setters (lazy: terminal_tool imports
|
||||
tools.approval at load, so a top-level import risks a cycle for tools.approval callers)."""
|
||||
"""(getter, setter) pairs for every thread-local prompt callback a tool may need mid-dispatch
|
||||
(lazy: terminal_tool imports tools.approval at load, so a top-level import risks a cycle).
|
||||
Add a new per-thread prompt here — a callback missing from this table is silently absent on
|
||||
every parallel/timeout worker, so the tool believes nobody can answer."""
|
||||
from agent.vault_backends import unlock as vault_unlock
|
||||
from tools import terminal_tool as tt
|
||||
|
||||
return (tt._get_approval_callback, tt._get_sudo_password_callback,
|
||||
tt.set_approval_callback, tt.set_sudo_password_callback)
|
||||
return ((tt._get_approval_callback, tt.set_approval_callback),
|
||||
(tt._get_sudo_password_callback, tt.set_sudo_password_callback),
|
||||
(vault_unlock.get_unlock_prompt_callback, vault_unlock.set_unlock_prompt_callback))
|
||||
|
||||
|
||||
def propagate_context_to_thread(target: Callable) -> Callable:
|
||||
"""Wrap *target* to run with the *current* thread's ContextVars and approval/sudo callbacks.
|
||||
"""Wrap *target* to run with the *current* thread's ContextVars and per-thread prompt callbacks
|
||||
(approval, sudo, password-manager unlock).
|
||||
|
||||
Fail-closed: if callback installation raises they stay ``None`` — dangerous commands are then
|
||||
denied by ``prompt_dangerous_approval`` and the gateway approval queue blocks.
|
||||
@@ -37,8 +42,7 @@ def propagate_context_to_thread(target: Callable) -> Callable:
|
||||
# (setter, parent callback) pairs; None when the callback API could not be captured.
|
||||
installs = None
|
||||
try:
|
||||
get_approval, get_sudo, set_approval, set_sudo = _callback_api()
|
||||
installs = ((set_approval, get_approval()), (set_sudo, get_sudo()))
|
||||
installs = tuple((setter, getter()) for getter, setter in _callback_api())
|
||||
except Exception:
|
||||
logger.debug("Could not capture parent approval/sudo callbacks", exc_info=True)
|
||||
|
||||
|
||||
@@ -53,21 +53,17 @@ _EXTERNAL_SOURCES = ("onepassword", "bitwarden")
|
||||
@method("vault.sources")
|
||||
def _(rid, params: dict) -> dict:
|
||||
"""Status of every login source: {name, display_name, enabled, needs_unlock, unlocked, installed}."""
|
||||
import shutil
|
||||
|
||||
from agent.vault_backends import enabled_backends
|
||||
from agent.vault_backends.bitwarden import BitwardenLoginBackend
|
||||
from agent.vault_backends.onepassword import OnePasswordLoginBackend
|
||||
from agent.secret_sources.onepassword import find_op
|
||||
from agent.vault_backends.base import external_backend_classes, is_installed
|
||||
|
||||
enabled = {b.name: b for b in enabled_backends()}
|
||||
rows = [{"name": "local", "display_name": "Hermes vault", "enabled": True, "needs_unlock": False,
|
||||
"unlocked": True, "installed": True}]
|
||||
for cls, installed in ((OnePasswordLoginBackend, find_op() is not None),
|
||||
(BitwardenLoginBackend, shutil.which("bw") is not None)):
|
||||
for cls in external_backend_classes():
|
||||
live = enabled.get(cls.name)
|
||||
rows.append({"name": cls.name, "display_name": cls.display_name, "enabled": live is not None,
|
||||
"needs_unlock": True, "unlocked": bool(live and live.is_unlocked()), "installed": installed})
|
||||
"needs_unlock": True, "unlocked": bool(live and live.is_unlocked()),
|
||||
"installed": is_installed(cls.name)})
|
||||
return _ok(rid, {"sources": rows})
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user