fix(security): add bws_cache.json to file_safety read guard - #32092
fix(security): add bws_cache.json to file_safety read guard#32092AhmetArif0 wants to merge 1 commit into
Conversation
The Bitwarden Secrets Manager disk cache introduced in NousResearch#31968 stores plaintext secret values at <hermes_home>/cache/bws_cache.json to avoid re-fetching across back-to-back CLI invocations. The file was not added to get_read_block_error()'s credential_file_names list, leaving the agent able to read it directly via the read_file tool. Add os.path.join("cache", "bws_cache.json") to credential_file_names so both HERMES_HOME and the global root are covered, matching the existing pattern used for auth.json, .anthropic_oauth.json, etc. Other files under cache/ (images, documents, audio) are unaffected — the check is an exact-file match, not a prefix match. Verified: 11/11 exploit/regression scenarios pass; 38/38 existing file_safety tests pass.
hclsys
left a comment
There was a problem hiding this comment.
Verified the fix against main. Correct and tight:
- Path is real —
agent/secret_sources/bitwarden.py:431writes the cache to<hermes_home>/cache/bws_cache.json(_DISK_CACHE_BASENAME = "bws_cache.json",:86), soos.path.join("cache", "bws_cache.json")matches the actual on-disk layout. - Matching works for the nested path — the credential loop does
(hd / name).resolve()thenresolved == blocked(file_safety.py:253-258); the nested-join entry resolves to the exact absolute file, exactly like the existingos.path.join("auth", "google_oauth.json")entry. No allowlist/SAFE_ROOTS short-circuitscache/ahead of this branch (only the skills.hubdeny runs earlier), so the guard takes effect. - Temp file is a non-issue — the writer uses
mkstemp(prefix=".bws_cache_", suffix=".tmp")+chmod 0600+ atomicos.replace(bitwarden.py:155-162), so the plaintext temp has an unpredictable name and a sub-ms window; not a realisticread_filetarget, and the "defense-in-depth, not a boundary" framing already covers it.
One consistency observation, not a blocker: this is an exact-file entry. If BSM ever grows a second cache artifact under cache/ (per-project file, rotation, etc.), this won't cover it — the same enumeration-vs-prefix gap that motivated #32090 to switch credential dirs to a whole-tree deny. Given cache/ legitimately holds non-secret files too, exact-match is the right call here; just flagging that the basename is now load-bearing if the cache layout changes.
LGTM.
|
Salvaged in PR #34421 → merged to main. Thanks — this closed a real defense-in-depth gap from #31968. |
Summary
PR #31968 introduced a Bitwarden Secrets Manager disk cache at
<hermes_home>/cache/bws_cache.jsonto avoid re-fetching secrets across back-to-back CLI invocations. The cache stores plaintext secret values (API keys, database passwords, etc. pulled from BSM projects).get_read_block_error()inagent/file_safety.pywas not updated to include this path, leaving the agent able to read it directly via theread_filetool — the same tool that already cannot readauth.json,.anthropic_oauth.json, ormcp-tokens/*.Fix
Add
os.path.join("cache", "bws_cache.json")tocredential_file_namesinget_read_block_error(), using the same exact-file match pattern already in place forauth.json,.anthropic_oauth.json, andauth/google_oauth.json. BothHERMES_HOMEand the global Hermes root are covered, consistent with the existing per-profile + root-level guard shape (#15981).Other files under
cache/(images, documents, audio) are unaffected — the check is an exact-file match, not a prefix match.credential_file_names = ( "auth.json", "auth.lock", ".anthropic_oauth.json", ".env", "webhook_subscriptions.json", os.path.join("auth", "google_oauth.json"), + # Bitwarden Secrets Manager disk cache: stores plaintext secret values + # to avoid re-fetching across back-to-back CLI invocations. The file + # was introduced by #31968 but not added to this guard. + os.path.join("cache", "bws_cache.json"), )Test plan
cache/bws_cache.jsonunder HERMES_HOME → BLOCKEDcredential(consistent with existing messages)auth.json,.anthropic_oauth.json,.env,webhook_subscriptions.json,mcp-tokens/*→ still BLOCKED (no regressions)cache/images/photo.png,cache/documents/report.pdf,cache/audio/clip.mp3→ still ALLOWEDbws_cache.jsonoutside HERMES_HOME → ALLOWED (per-location gate, not a basename block)test_file_safety.py+test_file_safety_credentials.pytests pass