fix(agent): import cryptography lazily in the Bitwarden secret source (#86735)

[pittosporum-seu]

What does this PR do?

hermes update fails with exit 2 on Windows on every run:

✗ This updater process has already loaded native venv modules that
  the dependency sync must replace:
    cryptography (_rust.pyd)

Root cause (as located in the issue): every CLI/gateway process imports the secret-source registry, which unconditionally registers the bundled Bitwarden source — and agent/secret_sources/bitwarden.py imported cryptography.hazmat at module level. On Windows a mapped native extension (cryptography._rust) cannot be replaced once loaded, so the updater's self-lock preflight (_detect_self_loaded_native_modules in update_cmd.py) deferred every update before it could even pull commits.

The cryptography imports are now lazy — inside the three functions that actually use them (HKDF key derivation, AESGCM encrypt/decrypt). A process that never reads an encrypted BWS cache never loads cryptography, so the self-lock preflight stays quiet on cold updates. The registry's "lazy, cheap, broken-source-safe" design (documented in _ensure_builtin_sources) is preserved.

Related Issue

Fixes #86735

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• agent/secret_sources/bitwarden.py: removed module-level from cryptography... imports (with a NOTE explaining why); lazy imports in _derive_encrypted_cache_key, _write_encrypted_disk_cache, _read_encrypted_disk_cache
• tests/secret_sources/test_secret_source_registry.py: TestNoEagerCryptographyImport — in a clean subprocess interpreter, _ensure_builtin_sources() must not leave cryptography or cryptography.hazmat.bindings._rust in sys.modules (subprocess so the test runner's own imports cannot mask a regression)

How to Test

1. pytest tests/secret_sources/test_secret_source_registry.py tests/test_bitwarden_secrets.py -q — new regression test passes; encrypted-cache behavior unchanged (pre-existing Windows env failures unrelated: NTFS 0o600, bws.exe naming)
2. ruff check + check-windows-footguns.py --diff — clean
3. Windows manual: hermes update no longer trips the self-lock preflight on a cold process; Bitwarden encrypted cache still works when configured

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix
• I've run pytest and all tests pass
• I've added tests for my changes
• I've tested on my platform: Windows 11, Python 3.11

Documentation & Housekeeping

• N/A for docs/config example (no new config keys)
• N/A for tool descriptions/schemas

This was generated by AI during triage.

Duplicate of #84114: both defer the same module-level Bitwarden cryptography imports into encrypted-cache use paths. #84114 is the earlier open implementation.

Review from SMF Works

Verdict: Comment (support merge) — no blocking issues.

What this does

bitwarden.py imported cryptography.hazmat at module level. The secret-source registry loads that module for every CLI/gateway process, so Windows hermes update always hits _detect_self_loaded_native_modules (cryptography (_rust.pyd)) and exits 2 before it can pull.

The PR moves HKDF/AESGCM imports into the three functions that actually encrypt/decrypt the BWS cache. A cold update that never reads an encrypted cache no longer maps _rust.pyd.

Verified on the files

• Module-level from cryptography... is gone; only a NOTE remains.
• Lazy imports sit in _derive_encrypted_cache_key, _write_encrypted_disk_cache, _read_encrypted_disk_cache — those are the only call sites.
• TestNoEagerCryptographyImport runs _ensure_builtin_sources() in a subprocess so the test runner's own imports cannot mask a regression. That's the right shape.

Residual (non-blocking)

• Current main also landed fix(secrets_cli): defer bitwarden backend import to first attribute access (3f9150e5c). Complementary layer, not a substitute: any other import of bitwarden.py would still have been eager. Keep both.
• If a process does touch the encrypted BWS cache, cryptography still loads and a later in-process hermes update will still self-lock. That's the existing Windows contract, not a regression.
• Test does not re-exercise encrypt/decrypt after the move; existing tests/test_bitwarden_secrets.py should cover that.

Ready for review/merge from our side.

SMF Works review — #86826

Verdict: Reviewed (COMMENT) — no blocking correctness bug in the Bitwarden lazy-import itself. Not an approve: overlapping earlier PRs + one residual Windows-update hole remain.

PR: #86826 — fix(agent): import cryptography lazily in the Bitwarden secret source (#86735)
Author: @pittosporum-seu
Files: 2 (+57 / −3) — agent/secret_sources/bitwarden.py, tests/secret_sources/test_secret_source_registry.py
Base: main@f0c222c7 (opened before #86782 merged)

What this PR does

Moves the three cryptography.hazmat imports off bitwarden.py module import and into the only call sites that need them:

• _derive_encrypted_cache_key → hashes + HKDF
• _write_encrypted_disk_cache → AESGCM.encrypt
• _read_encrypted_disk_cache → AESGCM.decrypt

That matches the #86735 chain: load_hermes_dotenv → secret registry → from agent.secret_sources.bitwarden import BitwardenSource used to map cryptography._rust.pyd before update_cmd._detect_self_loaded_native_modules ran, so Windows hermes update deferred at exit 2 before git fetch.

What I verified on the files

Checklist item	Result
Stops cryptography at module load?	Yes. HEAD bitwarden.py has no module-level from cryptography / import cryptography. Registry _ensure_builtin_sources() can import BitwardenSource without touching hazmat.
AESGCM / HKDF / hashes still available when encryption is used?	Yes. Same constructors, same arguments, same AAD (serialized_key). Write/read still sit in their existing best-effort except Exception envelopes, so an ImportError becomes a cache miss / skipped write — same fail-open as a missing wheel would have been at module import (except now Bitwarden still registers).
Leftover eager import in this module?	None. Only those three functions referenced HKDF / AESGCM / hashes on main; all three now import locally.
Tests?	Partial, but real. TestNoEagerCryptographyImport runs _ensure_builtin_sources() in a clean subprocess and asserts cryptography and cryptography.hazmat.bindings._rust stay out of sys.modules. Subprocess is the right isolation (parent file's autouse fixture stubs _ensure_builtin_sources). Pre-existing tests/test_bitwarden_secrets.py (test_encrypted_cache_writes_without_plaintext, test_encrypted_cache_falls_back_on_network_error) still exercise AES-GCM write/read.
Windows self-lock list still correct?	Yes — and this PR correctly does not touch it. _SELF_LOCKING_NATIVE_MODULES on current main is still only cryptography.hazmat.bindings._rust → cryptography (_rust.pyd). Detector is prefix-in-sys.modules, Windows-only. Keep it as defence-in-depth.
CI (head a0fc72db)	All 12 Python test slices + e2e + Windows-only + ruff + Windows footguns green. Check contributors / check-attribution failed (process/CLA, not a logic defect).

Context the PR description is slightly behind

#86782 merged onto main after this PR opened (3f9150e5, 08:55Z). It already:

1. lazy-loads secrets_cli / the Bitwarden backend
2. gates env_loader registry import on any_enabled

So a cold hermes update with no enabled secret source is already crypto-free on current main. This PR is still the missing half: _ensure_builtin_sources() unconditionally imports bitwarden.py the moment any source is enabled (1Password, command, plugin, or Bitwarden). Without this lazy import, a 1Password-only Windows user still maps _rust.pyd and still cannot update.

The module NOTE ("imported unconditionally … for every CLI/gateway process") is therefore a bit strong against post-#86782 main, but the code change is still the right one.

💡 Suggestions (non-blocking)

1. Earlier open twin: #84114 (2026-08-11, @nanami7777777) does the same three-site lazy import. Its tests are stronger on a different axis: MetaPathFinder blocks cryptography entirely and asserts Bitwarden + command still register. This PR's subprocess sys.modules check is stronger on the actual Windows self-lock predicate. Land one; close the other with credit. If this one lands, consider grafting #84114's "broken optional wheel must not poison registration" test — that is the other half of the registry's "broken-source-safe" contract.

2. Broader sibling: #77517 also lazy-imports Bitwarden crypto and skips external secret fetch on the update subcommand. That is the only approach that keeps _rust.pyd unmapped when secrets.bitwarden.encrypted_cache.enabled is on (see residual risk).

3. Test does not import hermes_cli.main / run cmd_update. Fine for this diff (the registry is the remaining eager edge after #86782). A one-liner comment pointing at that would save the next reviewer a trip.

Residual risk (not a request-changes)

• Encrypted-cache users still self-lock on update. encrypted_cache_enabled defaults false. If it is true, fetch_bitwarden_secrets (dotenv apply, before the preflight at update_cmd.py ~4324) will call _read_encrypted_disk_cache / _write_encrypted_disk_cache and load _rust.pyd in the updater process. This PR does not claim to fix that path; #77517's "don't apply external secrets during update" would.
• Other module-level cryptography imports exist off the update path (plugins/platforms/wecom/wecom_crypto.py; Weixin adapter). Not this PR's job; the self-lock list will still catch them if they ever get imported at CLI startup.
• Plugin discovery's _refresh_secret_sources_after_discovery still imports the registry; after this PR that import is cheap unless someone then uses the encrypted cache.

Looks good

• Minimal, on-target diff — no drive-by update-cmd / env_loader rewrite.
• Comments name the Windows mapped-image failure and #86735.
• Subprocess regression cannot be masked by the test runner already having imported cryptography.

---

SMF Works review — files + GitHub API + current main self-lock / env_loader / #86782 overlay. No competing PR opened.

Thanks @pittosporum-seu — your diagnosis and fix were correct, and the exact change you made here (lazy cryptography imports inside _derive_encrypted_cache_key / the encrypted-cache read/write paths, plus the subprocess registry regression test) landed via #86782, which covered the same bitwarden.py imports along with the secrets_cli/registry import chains, and merged first with full CI green. #86857 then hardened the preflight itself (version-gated, relocated post-code-swap).

Closing as superseded rather than merging a now-conflicting duplicate — but this was a solid, correctly-scoped fix and we'd be glad to see you pick up other issues. Sorry the overlap cost you the merge on this one; two people racing the same urgent regression is a good problem for the project to have.

Thanks for the detailed note — glad the diagnosis held up and #86782/#86857 landed the fix (and hardened the preflight). No hard feelings on the overlap; two people racing the same regression is indeed a good sign. I'll keep picking up issues from the tracker.