Skip to content

fix(secrets): 1Password encrypted-only cache; strip OP_* auth from child envs - #77168

Open
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:fix/onepassword-encrypted-cache-env-hygiene
Open

fix(secrets): 1Password encrypted-only cache; strip OP_* auth from child envs#77168
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:fix/onepassword-encrypted-cache-env-hygiene

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Related #77008 #77027 #77039 #77164

What changed and why

The 1Password secret source stored every resolved value in a plaintext disk cache at <hermes_home>/cache/op_cache.json — the same plaintext-at-rest vulnerability class the Bitwarden series (#77008) eliminated. The disk cache is now encrypted-only, and 1Password auth material no longer reaches spawned child processes.

1. Encrypted-only disk cache (mirrors the Bitwarden #77008 pattern)

  • Values persist only in op_cache.enc.json under AES-GCM, keyed off the auth material (service-account token, OP_ACCOUNT, OP_CONNECT_*, OP_SESSION_*); the raw token never touches disk — it only derives the key.
  • Random salt + nonce per write; the serialized cache key is authenticated as AAD; version field checked on read.
  • A plaintext cache file is never written. A legacy op_cache.json from older Hermes versions is migrated into the encrypted cache and removed on first read.
  • cache_ttl_seconds: 0 disables cache reuse entirely (fresh fetch every time; nothing read from or written to disk) — and still removes any legacy plaintext file.

2. Child-process env hygiene (mirrors the Bitwarden #77027 pattern)

OP_SERVICE_ACCOUNT_TOKEN, OP_CONNECT_TOKEN, and every OP_SESSION_* var are now stripped from all spawned children via _is_hermes_internal_secret — terminal, browser worker, ACP executor, computer-use driver, TUI/Node host. Previously they leaked to every child by default. The op child still receives them explicitly through its own minimal allowlisted env (_op_child_env), which is the one legitimate consumer.

3. Tests (5 new, 16 total in the file)

  • test_disk_cache_is_encrypted_only_no_plaintext — default path writes ONLY op_cache.enc.json; raw secret value absent from the on-disk payload.
  • test_encrypted_cache_round_trip — disk cache serves a second fetch with no op read.
  • test_legacy_plaintext_cache_is_migrated_and_removed — pre-hardening plaintext is re-encrypted and deleted.
  • test_memory_only_mode_removes_legacy_plaintext — ttl 0: fresh fetch, no plaintext consulted, legacy removed.
  • test_clear_caches_removes_encrypted_file.

Verification

  • tests/test_onepassword_secrets.py: 16 passed, 0 failed; with tests/agent/test_secret_scope.py + tests/secret_sources/: 80 passed, 0 failed.
  • Ruff clean on all three changed files.
  • Independent QA critique (separate agent): SAFE TO SHIP — cryptographic correctness verified adversarially (tamper/AAD/rotation checks), env-strip contract verified end-to-end (OP_ACCOUNT/OP_CONNECT_HOST/PATH/HOME retained), no blockers, MINOR/NIT only.
  • The op child env allowlist still carries OP_CONNECT_TOKEN/session vars (its one legitimate consumer), verified end-to-end.

Related

Part of #77473

…ild envs

The 1Password secret source stored every resolved value in a plaintext
disk cache (op_cache.json) at <hermes_home>/cache/ — the same
plaintext-at-rest vulnerability class the Bitwarden series (NousResearch#77008)
eliminated. The disk cache is now encrypted-only (AES-GCM, op_cache.enc.json,
keyed off the auth material; the raw token never touches disk), legacy
plaintext is migrated and removed on first read, and cache_ttl_seconds: 0
fully disables cache reuse (fresh fetch every time, nothing on disk).

Also closes the child-process leak: OP_SERVICE_ACCOUNT_TOKEN,
OP_CONNECT_TOKEN, and every OP_SESSION_* var are stripped from all
spawned children via _is_hermes_internal_secret (the op child still
receives them explicitly through its own allowlisted env).
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/auth Authentication, OAuth, credential pools backend/local Local shell execution sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data needs-repro Bug needs reproduction steps labels Aug 3, 2026
@egilewski

Copy link
Copy Markdown
Contributor

suggesting changes

This replay improves 1Password cache protection but is not safe to merge yet. Four source-backed security issues remain:

  • [P1] Cache identity does not include every effective 1Password selector and profile dimension. A long-lived process can change the selector while credentials stay the same, allowing a cached secret from the prior profile to be returned. Include the resolved cache context and all selectors in the cache and encryption identity, or disable caching for ambient desktop authentication, and add selector and context-isolation tests.
  • [P1] Some 1Password authentication variable names bypass child-process scrubbing. Exact session and configured custom token names can remain in model-facing child environments. Treat every supported authentication name as secret material, pass it only to the 1Password child, and add negative tests across child-process entry points.
  • [P2] Legacy plaintext cache data can survive a mismatched or failed migration. Remove or safely migrate recognized plaintext on every fetch, clear, and cache-disabled operation, including stale or mismatched entries, and test fetch and write failures.
  • [P1] A browser-use installer receives the parent environment instead of the centralized scrubbed environment, exposing internal 1Password credentials to a third-party child. Route installer and related helper spawns through the shared scrubber and add a regression test ensuring internal secrets are absent.

Security evidence:

  • trust boundary: 1Password credentials and references enter the secret source; cache and child-process boundaries must prevent cross-profile reuse, plaintext persistence, and credential inheritance.
  • source/sink/invariant: Checked that cache identity, migration cleanup, authentication-name scrubbing, and child-process environment construction preserve those boundaries; the four gaps above remain.
  • current-main reproduction: Checked the pre-change cache and environment behavior against the stated encrypted-cache and child-scrub invariants.
  • PR-head or patch-replay validation: Checked the reviewed change set and focused validation evidence for the cache, migration, authentication-scrub, and installer behavior.
  • positive/negative cases: Checked successful encrypted caching, migration, cleanup, and standard scrubbing; negative cases cover selector changes, alternate authentication names, failed migration, and unsanitized installer inheritance.
  • residual bypass search: Checked direct child-process environment construction for remaining unsanitized flows.
  • reviewer validation: Independently confirmed the four issues against the supplied source evidence.

Review setup: I reviewed a run-owned local rebase or patch replay against current GitHub main because the submitted branch is stale or conflicted; this does not mean the submitted branch itself merges cleanly.

Not checked:

  • Full repository test suite
  • CodeRabbit review

Signed: GPT-5.6-luna-max in Codex

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 backend/local Local shell execution comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint needs-repro Bug needs reproduction steps P3 Low — cosmetic, nice to have 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