Skip to content

fix(secrets): add encrypted Bitwarden stale cache - #65711

Closed
andyylin wants to merge 3 commits into
NousResearch:mainfrom
andyylin:fix/encrypted-bitwarden-stale-cache
Closed

fix(secrets): add encrypted Bitwarden stale cache#65711
andyylin wants to merge 3 commits into
NousResearch:mainfrom
andyylin:fix/encrypted-bitwarden-stale-cache

Conversation

@andyylin

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds an opt-in encrypted last-good cache for Bitwarden Secrets Manager secrets so Hermes can still start during transient BWS/network outages without storing the cached secret payload in plaintext-equivalent JSON.

This is related to #41938, but intentionally different: #41938 serves the existing plaintext disk cache when BWS fails. This PR adds an encrypted cache path with explicit stale-window controls and keeps fallback limited to network/timeout failures so auth failures do not bypass revocation.

Changes made

  • Adds bws_cache.enc.json, written atomically with mode 0600 under <hermes_home>/cache/.
  • Encrypts cached payloads with AES-GCM using a key derived from the local bootstrap BWS_ACCESS_TOKEN plus a random per-cache salt via HKDF-SHA256.
  • Keeps the BWS access token out of the cache file; it is only used locally to derive the encryption key.
  • Adds opt-in config:
secrets:
  bitwarden:
    encrypted_cache:
      enabled: false
      max_stale_seconds: 0
  • Keeps stale fallback separate from cache_ttl_seconds, so operators can set cache_ttl_seconds: 0 while still retaining an encrypted break-glass cache.
  • Falls back to encrypted cache only for classified NETWORK / TIMEOUT BWS failures.
  • Does not fall back for auth failures, malformed output, or cache corruption.
  • Updates cache reset test helpers so encrypted cache files are cleaned too.

Security notes

This does not eliminate the bootstrap-token trust boundary: a local attacker who can read BWS_ACCESS_TOKEN can derive the cache key and already has enough material to fetch secrets when the network is available. It does improve the common failure/theft case where a cache file is copied or exposed without the bootstrap token: the cached secret payload is no longer plaintext JSON.

Tests

uv run --with pytest pytest tests/test_bitwarden_secrets.py
python3 -m py_compile agent/secret_sources/bitwarden.py hermes_cli/config.py tests/test_bitwarden_secrets.py
git diff --check upstream/main...HEAD

All passed locally.

Runtime smoke

Also verified on a live Hermes profile without printing secret values:

  • encrypted cache file was created with mode 0600;
  • plaintext cache file was absent;
  • encrypted cache JSON contained only metadata + ciphertext;
  • a fake bws returning network is unreachable successfully loaded secrets from the encrypted stale cache;
  • the warning reported: Using stale encrypted Bitwarden cache after network fetching BWS secrets.

AI Disclosure

This change was implemented with AI assistance.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 16, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for building a narrow, opt-in encrypted alternative to the plaintext stale-cache approach discussed on #41938. The stale-fallback premise is still present on current main: agent/secret_sources/bitwarden.py:410 directly calls _run_bws_list with no fallback.

Problems

  • agent/secret_sources/bitwarden.py:464 validates expiry from unauthenticated outer JSON metadata before decrypting. AES-GCM binds only serialized_key at line 472, so changing outer fetched_at can extend valid stale ciphertext past max_stale_seconds during a network failure.
  • agent/secret_sources/bitwarden.py:586 writes the encrypted cache without removing a prior plaintext cache/bws_cache.json; enabling the option does not clean an existing plaintext cache.
  • bws_cache.enc.json is absent from the parallel credential protections in agent/file_safety.py:271-282, gateway/platforms/base.py:1174-1192, and hermes_cli/web_server.py:1342-1355.

Suggested changes

  • Authenticate or decrypt-before-check the timestamp, and add a metadata-tampering regression test.
  • Remove the legacy plaintext cache after a successful encrypted write and test that migration.
  • Extend all credential guards and their tests; document the new config in the example and Bitwarden guide.

Automated hermes-sweeper review.

Comment thread agent/secret_sources/bitwarden.py Outdated
fetched_at = payload.get("fetched_at")
if not isinstance(fetched_at, (int, float)):
return None
entry_age = time.time() - float(fetched_at)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stale-window check trusts unauthenticated outer JSON metadata. AES-GCM below authenticates only serialized_key, so changing fetched_at to now makes old valid ciphertext acceptable during a network failure. Check the decrypted inner timestamp instead, or authenticate this metadata, and add a tampering regression test.

Comment thread agent/secret_sources/bitwarden.py Outdated
_DISK_CACHE.write(cache_key, entry, cache_ttl_seconds, home_path)
if cache_ttl_seconds > 0:
_CACHE[cache_key] = entry
if encrypted_cache_enabled and encrypted_cache_max_stale_seconds > 0:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a user enables this after normal caching, an existing plaintext cache/bws_cache.json is neither read nor removed, so the old secret payload remains on disk. Remove or migrate that legacy cache after a successful encrypted write and add a transition test.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 18, 2026
@andyylin
andyylin force-pushed the fix/encrypted-bitwarden-stale-cache branch from fad3146 to 686652e Compare July 19, 2026 02:52
# Conflicts:
#	agent/secret_sources/bitwarden.py
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #69251 — both your commits landed as-is (rebase-merged, authorship preserved). We reworked the fallback control flow onto the stale-cache path that merged in the meantime (#69051): one transport-only gate, encrypted tier replaces plaintext when enabled, plus one addition — hermes secrets bitwarden token rotation now clears the encrypted file too, since its key derives from the rotated token (the rotation command postdates your PR). Your defense-in-depth wiring (write-deny, media-deny, dashboard deny lists) came through untouched. Thanks @andyylin!

@teknium1 teknium1 closed this Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants