fix(security): add auth.json to build_write_denied_paths

[toprakeker]

Fixes #70942

Summary

build_write_denied_paths() in agent/file_safety.py protects .env,
.anthropic_oauth.json, SSH keys, and a handful of other credential
files against writes/deletes/moves by the agent's own file tools.
auth.json — the actual provider credential store (OAuth refresh
tokens, API keys, the whole credential_pool) — was missing from that
list entirely.

The asymmetry that gave this away: get_read_block_error() in the
same module already treats auth.json as a credential store and
blocks reads of it. So the agent could not read auth.json, but
could freely overwrite or delete it via write_file, patch_replace,
delete_path, move_file, or the ACP fs/write_text_file shim —
the read-deny arguably made this worse, since the agent can't inspect
what it's about to destroy before destroying it.

Fix

Mirrors the existing .env / .anthropic_oauth.json handling:

• Adds auth.json at both the active-profile path and the top-level
  root path (same root-widening shape as write_file tool bypasses credential protection for global ~/.hermes/.env #15981, so the store stays
  protected whether or not a profile is active).
• Also adds auth.lock, auth/google_oauth.json, and
  webhook_subscriptions.json — these were already read-denied
  alongside auth.json in get_read_block_error()'s
  credential_file_names tuple but had the same write-side gap.

Tests

• Updated test_hermes_control_files_requested_writable — it
  previously asserted auth.json and webhook_subscriptions.json
  were writable, which encoded the bug. Now only asserts config.yaml
  is writable (still correct — config isn't a credential store).
• Added TestWriteDenyCredentialStore with 5 new tests: each new path
  individually, plus a root-widening-under-profile case matching the
  existing .env coverage.

Ran the full file-safety/credential test surface locally, no
regressions:

tests/agent/test_file_safety_sandbox_mirror.py
tests/agent/test_file_safety_credentials.py
tests/agent/test_file_safety.py
tests/agent/test_file_safety_container_mirror.py
tests/agent/test_file_safety_session_state.py
tests/agent/test_file_safety_cross_profile.py
tests/tools/test_credential_files.py
tests/tools/test_cross_profile_guard.py
tests/tools/test_write_deny.py
tests/tools/test_file_write_safety.py

213 passed in 4.28s

[teknium1]

Thanks for addressing the verified auth.json write-side gap. Current main routes write, patch, delete, move, and ACP writes through get_write_denied_error() (tools/file_operations.py:1291-1391, agent/copilot_acp_client.py:732-739), so this shared denylist change covers the reported paths.

Problems

• The same read/write asymmetry remains for cache/bws_cache.json: it is read-denied as a credential store in agent/file_safety.py:284, and it stores plaintext secret values in agent/secret_sources/bitwarden.py:93-100. The PR retains only the existing bws_cache.enc.json write-deny entries.

Suggested changes

• Add both active-profile and global-root cache/bws_cache.json paths to build_write_denied_paths() and add a focused regression test, including profile/root widening.

Automated hermes-sweeper review.

[toprakeker]

Thanks — good catch. I added cache/bws_cache.json to the same active-profile and global-root write-deny coverage as the encrypted Bitwarden cache, plus focused regression tests for the plaintext cache and profile/root widening case.\n\nValidation run locally:\n\n\npython -m pytest tests/tools/test_write_deny.py -q -o 'addopts='\n# 27 passed\n\npython -m pytest tests/agent/test_file_safety_sandbox_mirror.py tests/agent/test_file_safety_credentials.py tests/agent/test_file_safety.py tests/agent/test_file_safety_container_mirror.py tests/agent/test_file_safety_session_state.py tests/agent/test_file_safety_cross_profile.py tests/tools/test_credential_files.py tests/tools/test_cross_profile_guard.py tests/tools/test_write_deny.py tests/tools/test_file_write_safety.py -q -o 'addopts='\n# 215 passed\n

[GottZ]

This was generated by AI during triage.

Summary

One PR directly addresses and closes #70942. #70967 prevents agent-mediated destruction of active-profile and global-root credential stores, while its auth.json protection reverses the deliberate write policy documented for #45947.

Related pull requests

• fix(security): add auth.json to build_write_denied_paths #70967 best fix — (+98/-2) — best fix, pending maintainer policy resolution: The diff write-denies auth.json and related credential stores across active-profile and global-root paths, with focused regression coverage; it also satisfies the keep_open review on fix(security): add auth.json to build_write_denied_paths #70967 by adding the requested plaintext cache/bws_cache.json protection and tests, but the auth.json policy reversal still requires an explicit maintainer decision.

Suggested consolidation

Keep open with a salvage path: #70967 is the sole Verify-selected best fix, and its cache/bws_cache.json protection plus profile/root regression coverage are independently useful, but maintainers must explicitly decide whether to reverse #45947's deliberate auth.json writability before the broader change can proceed. There are no competing PRs to close as duplicates.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I70942(["issue #70942 (open)"])
    P70967["PR #70967 (open)"]
    P70967 -->|best fix| I70942
    class I70942 open
    class P70967 open
    class P70967 best
    class P70967 target
    click I70942 "https://github.com/NousResearch/hermes-agent/issues/70942"
    click P70967 "https://github.com/NousResearch/hermes-agent/pull/70967"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 6 kB of PR diffs, 13 kB of issue/PR text, 3 kB of discussion (5 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[toprakeker]

Rebased onto current main and resolved the test conflict while preserving the requested plaintext Bitwarden cache coverage.

New head:
872322ae213943c5d8b4ce5dc3960964e37f73ba

Verification:

uv run --extra dev pytest tests/tools/test_write_deny.py -q
19 passed in 0.67s

uv run ruff check agent/file_safety.py tools/file_operations.py tests/tools/test_write_deny.py
All checks passed!

git diff --check origin/main...HEAD
PASS

The remaining non-technical blocker is still the policy decision called out by triage: whether maintainers want to reverse the prior auth.json writability behavior from #45947. The rebased branch keeps the cache/bws_cache.json protections and active-profile/global-root regression coverage intact for review.

[toprakeker]

Closing this older pre-contributor branch rather than asking maintainers to approve stale fork CI. The codebase has moved substantially since it was opened; if the underlying issue remains, I will re-evaluate it against current main and submit a fresh, current PR.