fix(update): lazy-import secrets_cli + defer secret_sources registry — prevent Windows self-lock loop

hermes_cli/env_loader.py

@@ -637,6 +637,23 @@ def _apply_external_secret_sources(home_path: Path) -> None:
         # on its next load_hermes_dotenv() call instead of never.
         return
 
+    # Defer the registry import until we know a secrets source is enabled —
+    # agent.secret_sources.bitwarden eagerly loads cryptography._rust.pyd,
+    # which causes the Windows updater to self-lock before its preflight
+    # (the updater itself maps the .pyd before the dependency sync runs).
+    # A config with no enabled sources costs one dict scan; a config with
+    # enabled sources pays the crypto load exactly once, on demand.
+    # NOTE: only keys that smell like a real secret source trigger the import —
+    # a generic dict entry must not force crypto load on every hermes launch.
+    # We whitelist by *shape* (source dict with enabled flag) rather than
+    # hardcoding names, so plugin/test sources pass through unknown keys.
+    any_enabled = any(
+        isinstance(v, dict) and v.get("enabled") is True
+        for v in cfg.values()
+    )
+    if not any_enabled:
+        return
+
     try:
         from agent.secret_sources.registry import apply_all
     except ImportError:

hermes_cli/main.py

@@ -11813,7 +11813,12 @@ def main():
         help="1Password (op:// references) integration",
     )
 
-    # Lazy import — only pays for itself when this subcommand is actually used.
+    # Lazy-import secrets_cli: the module imports agent.secret_sources.bitwarden
+    # which loads cryptography._rust.pyd.  On Windows this maps the native
+    # extension into the updater process, causing the self-lock preflight to
+    # defer (#86781).  secrets_cli defers its backend import to first use
+    # (module-level __getattr__ + handler-level _load_bw()), so register_cli
+    # at parse time only wires argparse structure with no crypto cost.
     from hermes_cli import secrets_cli as _secrets_cli
     from hermes_cli import onepassword_secrets_cli as _op_secrets_cli
 
@@ -11822,14 +11827,10 @@ def main():
 
     def _dispatch_secrets(args):  # noqa: ANN001
         sub = getattr(args, "secrets_command", None)
-        bw_sub = getattr(args, "secrets_bw_command", None)
-        op_sub = getattr(args, "secrets_op_command", None)
-        if sub in ("bitwarden", "bw") and bw_sub is not None:
-            return args.func(args)
-        if sub in ("onepassword", "op", "1password") and op_sub is not None:
-            return args.func(args)
-        secrets_parser.print_help()
-        return 0
+        if sub is None:
+            secrets_parser.print_help()
+            return 0
+        return args.func(args)
 
     secrets_parser.set_defaults(func=_dispatch_secrets)
 

hermes_cli/secrets_cli.py

@@ -23,7 +23,20 @@
 from rich.panel import Panel
 from rich.table import Table
 
-from agent.secret_sources import bitwarden as bw
+# NOTE: the Bitwarden backend (``agent.secret_sources.bitwarden``) pulls in
+# ``cryptography`` at module-import time.  On Windows the resulting
+# ``cryptography._rust.pyd`` is mapped into the running process — and when
+# that process is ``hermes update``, the self-lock preflight detects the
+# loaded native module and defers (#86781).  Keep the backend import lazy:
+# this module is registered parse-time from ``hermes_cli.main`` and must not
+# touch ``bw`` until a handler actually runs.
+#
+# ``_BWS_VERSION`` is duplicated here (as a plain string) so ``register_cli``
+# can render the ``install --help`` text without importing the backend.
+# ``agent.secret_sources.bitwarden._BWS_VERSION`` is the source of truth;
+# bump both together when pinning a new bws release.
+_BWS_VERSION = "2.0.0"
+
 from hermes_cli.config import (
     get_env_path,
     load_config,
@@ -33,6 +46,28 @@
 from hermes_cli.secret_prompt import masked_secret_prompt
 
 
+def _load_bw():
+    """Import ``agent.secret_sources.bitwarden`` on first use (crypto payload)."""
+    from agent.secret_sources import bitwarden as _bw
+
+    return _bw
+
+
+def __getattr__(name: str):
+    """PEP 562 module-level lazy resolver.
+
+    Existing callers (and upstream tests) monkeypatch attributes on
+    ``hermes_cli.secrets_cli.bw`` directly.  Resolving that attribute at
+    module-import time would re-import ``cryptography`` eagerly — the very
+    self-lock we are preventing (#86781).  Defer the backend import until
+    the first actual attribute access, so ``import hermes_cli.secrets_cli``
+    stays crypto-free while ``secrets_cli.bw.find_bws`` still resolves.
+    """
+    if name == "bw":
+        return _load_bw()
+    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
+
+
 # ---------------------------------------------------------------------------
 # Argparse wiring — called from hermes_cli.main
 # ---------------------------------------------------------------------------
@@ -103,7 +138,7 @@ def register_cli(parent_parser: argparse.ArgumentParser) -> None:
 
     install = sub.add_parser(
         "install",
-        help=f"Download and verify the pinned bws binary (v{bw._BWS_VERSION})",
+        help=f"Download and verify the pinned bws binary (v{_BWS_VERSION})",
     )
     install.add_argument(
         "--force",
@@ -119,6 +154,7 @@ def register_cli(parent_parser: argparse.ArgumentParser) -> None:
 
 
 def cmd_setup(args: argparse.Namespace) -> int:
+    bw = _load_bw()
     console = Console()
     console.print(
         Panel.fit(
@@ -308,6 +344,7 @@ def cmd_setup(args: argparse.Namespace) -> int:
 
 
 def cmd_status(args: argparse.Namespace) -> int:
+    bw = _load_bw()
     console = Console()
     cfg = load_config()
     bw_cfg = (cfg.get("secrets") or {}).get("bitwarden") or {}
@@ -373,6 +410,7 @@ def cmd_token(args: argparse.Namespace) -> int:
     token, probes Bitwarden with it (unless ``--no-verify``), and only then
     persists it to .env — so a bad paste never bricks the working token.
     """
+    bw = _load_bw()
     console = Console()
     cfg = load_config()
     bw_cfg = (cfg.get("secrets") or {}).get("bitwarden") or {}
@@ -448,6 +486,7 @@ def cmd_token(args: argparse.Namespace) -> int:
 
 
 def cmd_sync(args: argparse.Namespace) -> int:
+    bw = _load_bw()
     console = Console()
     cfg = load_config()
     bw_cfg = (cfg.get("secrets") or {}).get("bitwarden") or {}
@@ -538,6 +577,7 @@ def cmd_disable(args: argparse.Namespace) -> int:
 
 
 def cmd_install(args: argparse.Namespace) -> int:
+    bw = _load_bw()
     console = Console()
     try:
         path = bw.install_bws(force=bool(args.force))

tests/test_lazy_secrets_dispatch.py

@@ -0,0 +1,208 @@
+"""End-to-end tests for lazy cryptography loading.
+
+These tests invoke the real CLI paths as subprocesses to verify:
+1. `hermes secrets bitwarden setup --help` works (dispatch path)
+2. `hermes update --check` works (update path)
+3. `hermes secrets bitwarden disable` works (handler execution)
+4. `hermes secrets onepassword status` works (lazy backend loads on demand)
+
+Unlike test_lazy_secrets_import.py (which inspects sys.modules), these
+run the actual commands and verify exit codes — the exact paths the
+reviewer flagged as unproven.
+"""
+
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+
+def _run_hermes(args: list[str], timeout: int = 30) -> subprocess.CompletedProcess[str]:
+    """Run hermes CLI as a subprocess from repo root."""
+    repo_root = Path(__file__).parent.parent
+    return subprocess.run(
+        [sys.executable, "-m", "hermes_cli.main"] + args,
+        capture_output=True,
+        text=True,
+        cwd=str(repo_root),
+        timeout=timeout,
+    )
+
+
+class TestSecretsDispatchE2E:
+    """End-to-end secrets dispatch — the path that must not self-lock."""
+
+    def test_bitwarden_setup_help(self) -> None:
+        """`hermes secrets bitwarden setup --help` must exit 0 and print usage.
+
+        This is the exact path that triggered the #86781 self-lock loop on
+        Windows: setup/parser nested under lazy-loaded backend.
+        """
+        result = _run_hermes(["secrets", "bitwarden", "setup", "--help"])
+        assert result.returncode == 0, (
+            f"bitwarden setup --help failed:\n"
+            f"stdout: {result.stdout}\n"
+            f"stderr: {result.stderr}"
+        )
+        assert "usage" in result.stdout.lower()
+
+    def test_bitwarden_status(self) -> None:
+        """`hermes secrets bitwarden status` must exit 0 (runs lazy backend)."""
+        result = _run_hermes(["secrets", "bitwarden", "status"])
+        # status may return non-zero if not configured, but must NOT crash
+        # with import errors, recursion, or missing subcommand
+        assert result.returncode in (0, 1), (
+            f"bitwarden status crashed:\n"
+            f"stdout: {result.stdout}\n"
+            f"stderr: {result.stderr}"
+        )
+        # Must not contain import errors
+        assert "ImportError" not in result.stderr
+        assert "cannot import name" not in result.stderr
+
+    def test_bitwarden_disable(self) -> None:
+        """`hermes secrets bitwarden disable` must exit 0."""
+        result = _run_hermes(["secrets", "bitwarden", "disable"])
+        assert result.returncode == 0, (
+            f"bitwarden disable failed:\n"
+            f"stdout: {result.stdout}\n"
+            f"stderr: {result.stderr}"
+        )
+
+    def test_onepassword_status(self) -> None:
+        """`hermes secrets onepassword status` must exit 0 (1Password lazy backend)."""
+        result = _run_hermes(["secrets", "onepassword", "status"])
+        assert result.returncode in (0, 1), (
+            f"onepassword status crashed:\n"
+            f"stdout: {result.stdout}\n"
+            f"stderr: {result.stderr}"
+        )
+        assert "ImportError" not in result.stderr
+
+    def test_onepassword_setup_help(self) -> None:
+        """`hermes secrets onepassword setup --help` must exit 0."""
+        result = _run_hermes(["secrets", "onepassword", "setup", "--help"])
+        assert result.returncode in (0, 2), (
+            f"onepassword setup --help failed:\n"
+            f"stdout: {result.stdout}\n"
+            f"stderr: {result.stderr}"
+        )
+        assert "ImportError" not in result.stderr
+
+
+class TestUpdatePathE2E:
+    """Update path — must not load cryptography.
+
+    These tests invoke the real `hermes update --check` path as a subprocess.
+    The conftest.py live-system guard blocks this because the command string
+    contains "update"; we bypass with the pytest mark.
+    """
+
+    @pytest.mark.live_system_guard_bypass
+    def test_update_check_clean(self) -> None:
+        """`hermes update --check` must not load cryptography._rust."""
+        result = _run_hermes(["update", "--check"])
+        assert result.returncode in (0, 1, 2), (
+            f"update --check crashed:\n"
+            f"stdout: {result.stdout}\n"
+            f"stderr: {result.stderr}"
+        )
+        # No import errors
+        assert "ImportError" not in result.stderr
+        assert "cannot import name" not in result.stderr
+
+    @pytest.mark.live_system_guard_bypass
+    def test_update_no_self_lock(self) -> None:
+        """Update path must not self-lock (cryptography._rust absent)."""
+        result = _run_hermes(["update", "--check"])
+        # The check itself may return non-zero (e.g. no updates), but
+        # must not contain the self-lock defer message
+        assert "deferred" not in result.stderr.lower()
+        assert "self-lock" not in result.stderr.lower()
+        assert "_rust.pyd" not in result.stderr.lower()
+
+    @pytest.mark.live_system_guard_bypass
+    def test_main_update_check_crypto_absent_in_sys_modules(self) -> None:
+        """Decisive invariant: invoking main() with argv=['hermes','update','--check']
+        leaves cryptography.hazmat.bindings._rust absent from sys.modules.
+
+        This is the exact invariant review flagged as unproven (#86782 review
+        2026-08-15): an import-only test cannot observe lazy failures, because
+        parser construction happens inside main(). Run main() itself in a
+        subprocess, let it execute the update path, then assert sys.modules.
+
+        The check must run before _dispatch_update calls its (potentially
+        lazy) network layer, so we instrument sys.modules immediately after
+        parse_args() and before dispatch returns, using a monkeypatched
+        _cmd_update_check that captures state then short-circuits.
+        """
+        script = """
+import sys
+from unittest.mock import patch
+
+crypto_seen_at_dispatch = []
+
+def capture_update_check(*args, **kwargs):
+    # Run just before the real handler would; record crypto state.
+    crypto_seen_at_dispatch.append(
+        'cryptography.hazmat.bindings._rust' in sys.modules
+    )
+    # Short-circuit: don't actually call the network in tests.
+    return 0
+
+sys.argv = ['hermes', 'update', '--check']
+
+import hermes_cli.main as m
+
+# Patch the update handler so main() exercises its parser + dispatch
+# without doing network I/O.  cmd_update (in main.py) calls
+# _self()._cmd_update_check(branch=..., branch_explicit=...) where _self()
+# resolves the hermes_cli.main module's lazily re-exported attribute —
+# so the patch must land on hermes_cli.main._cmd_update_check.
+with patch('hermes_cli.main._cmd_update_check', capture_update_check):
+    try:
+        m.main()
+    except SystemExit as e:
+        # argparse may sys.exit for --help / bad args; ignore for this probe
+        if e.code not in (0, None):
+            print(f'FAIL: main() exited with code {e.code}')
+            sys.exit(1)
+
+# 1. main() must have dispatched into our capture hook
+if not crypto_seen_at_dispatch:
+    print('FAIL: update --check did not dispatch to _cmd_update_check')
+    sys.exit(1)
+
+# 2. At dispatch time, crypto must NOT be loaded
+if crypto_seen_at_dispatch[0]:
+    print('FAIL: cryptography._rust loaded by main() before update dispatch')
+    sys.exit(1)
+
+# 3. After main() returned, crypto must STILL not be loaded
+if 'cryptography.hazmat.bindings._rust' in sys.modules:
+    print('FAIL: cryptography._rust present in sys.modules after main()')
+    sys.exit(1)
+
+print('PASS: main() update --check path never loaded cryptography._rust')
+sys.exit(0)
+"""
+        repo_root = Path(__file__).parent.parent
+        probe = repo_root / "_test_main_update_crypto_probe.py"
+        probe.write_text(script)
+        try:
+            result = subprocess.run(
+                [sys.executable, probe.name],
+                capture_output=True,
+                text=True,
+                cwd=str(repo_root),
+                timeout=60,
+            )
+            assert result.returncode == 0, (
+                f"Decisive main()-level probe failed:\n"
+                f"stdout: {result.stdout}\n"
+                f"stderr: {result.stderr}"
+            )
+            assert "PASS" in result.stdout
+        finally:
+            probe.unlink(missing_ok=True)