fix(security): add missing credential paths to write denylist - #32262
fix(security): add missing credential paths to write denylist#32262AhmetArif0 wants to merge 1 commit into
Conversation
The write denylist already protects SSH keys, AWS, GPG, npm, PyPI, Docker, Azure, GitHub CLI, git-credentials, and gcloud credentials. Four more common credential stores were unprotected: ~/.vault-token stores the HashiCorp Vault bootstrap token — directly analogous to ~/.netrc, a single-file credential bearer. ~/.cargo/credentials.toml holds the crates.io registry API token used by `cargo publish` — analogous to ~/.npmrc and ~/.pypirc, both already denied. ~/.config/hub/ is the GitHub Hub CLI OAuth token store — the direct sibling of ~/.config/gh/ which was already protected. ~/.terraform.d/ holds Terraform Cloud credentials.tfrc.json with the API token used to interact with Terraform Cloud remote state and plan/apply operations — analogous to ~/.aws/. Under prompt injection, an agent could be instructed to overwrite any of these files to destroy credentials or plant malicious ones. Adds four test methods covering each new path.
alt-glitch
added
type/security
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying four credential stores omitted from the shared denylist. The premise remains valid on current main: agent/file_safety.py:49-56 and agent/file_safety.py:66-76 still do not include any of the proposed locations, while agent/file_safety.py:103-106 is the shared enforcement path used by file tools and ACP.
Problems
- The production hunks apply cleanly, but
git apply --checkrejects the test hunk because its anchor,test_package_manager_configs, no longer exists. Current main consolidated those assertions intotests/tools/test_write_deny.py:72-75.
Suggested changes
- Salvage the four assertions into the current test layout: add the exact-path checks beside
test_credential_config_files_deniedand retain the directory checks inTestWriteDenyPrefixes. The source additions belong inbuild_write_denied_paths()andbuild_write_denied_prefixes()as proposed.
Automated hermes-sweeper review.
| for name in [".npmrc", ".pypirc", ".pgpass"]: | ||
| assert _is_write_denied(os.path.join(home, name)) is True, f"{name} should be denied" | ||
|
|
||
| def test_vault_token(self): |
There was a problem hiding this comment.
This test hunk needs a manual transplant onto current main: its test_package_manager_configs anchor was consolidated into test_credential_config_files_denied at current tests/tools/test_write_deny.py:72-75, so git apply --check rejects this portion of the patch.
teknium1
added
sweeper:risk-security-boundary
GottZ
left a comment
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two PRs address the same credential-denylist gap class but cover different credential stores: #27217 added Git and Google Cloud paths, while #32262 adds Vault, Cargo, Hub CLI, and Terraform paths plus regression tests.
Related pull requests
- #27217 [merged]
related— (+2/-0) — merged reference implementation: added exact-file protection for ~/.git-credentials and prefix protection for ~/.config/gcloud, directly closing two earlier credential-store omissions; it remains relevant as the established pattern and prerequisite baseline for #32262. - #32262
related— (+24/-0) — keep open and refresh tests: the production diff directly covers four additional omissions via exact-file rules for ~/.vault-token and ~/.cargo/credentials.toml and prefix rules for ~/.config/hub and ~/.terraform.d. Consistent with the keep_open review on #32262, the source hunks remain applicable, but the test hunk must be moved to the current consolidated test layout before merge.
Suggested consolidation
Update #32262 by rebasing or relocating its four assertions into the current test structure, then merge #32262; #27217 is already merged and complementary rather than duplicative, so no PR should be closed as a duplicate.
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 3 kB of PR diffs, 2 kB of issue/PR text, 3 kB of discussion (4 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
4 participants
Problem
The write denylist in
agent/file_safety.pyalready protects SSH keys, AWS, GPG, npm, PyPI, Docker, Azure, GitHub CLI (~/.config/gh/),~/.git-credentials, and~/.config/gcloud/. Four analogous credential stores were left unprotected:~/.vault-token~/.netrc~/.cargo/credentials.toml~/.npmrc/~/.pypirc~/.config/hub/~/.config/gh/~/.terraform.d/~/.aws/Under prompt injection, an agent could be instructed to overwrite any of these files to destroy or backdoor credentials (e.g. planting a malicious crates.io token or destroying a Vault bootstrap token).
Fix
~/.vault-tokenand~/.cargo/credentials.tomltobuild_write_denied_paths()(exact-file block)~/.config/huband~/.terraform.dtobuild_write_denied_prefixes()(directory block)Testing
tests/tools/test_write_deny.py— 4 new test methods, all pass (24/24 total)TestWriteAllowedsuite unchanged and green