fix(security): symlinks and Grep bypassed the sensitive-path deny-list - #16
Merged
Conversation
Phase 2 (filesystem tool surface, 6 files, 1,019 LOC). Two live bypasses, both
reproduced before fixing and re-verified after.
## Symlinks defeated every sensitive-path protection (CRITICAL)
`check_sensitive_path` matches on the *file name* of the path it is handed, and
nothing resolved symlinks first. Measured, on the real tools:
read .ssh/id_rsa directly -> refused ("private-key file")
read notes.md -> .ssh/id_rsa -> PRIVATE KEY CONTENTS RETURNED
write config.json -> .aws/credentials -> CREDENTIALS OVERWRITTEN
So the guard demonstrably worked when the name matched and demonstrably did not
when it did not — every protection it offers was one innocuously-named link away
from nothing.
This needs no unusual privileges and no suspicious command. A repository can
simply *ship* a symlink called `README.md`; asking the agent to read it
exfiltrates the target. The agent can also create one itself with `ln -s`.
`check_sensitive_path_resolved` now checks both the supplied path and its
canonicalised target — either looking sensitive is a refusal. For paths that do
not exist yet (a fresh write) it canonicalises the parent instead, which also
catches a symlinked parent directory. Applied at all four filesystem entry
points: FileRead, FileWrite, FileEdit, MultiEdit.
Benign symlinks still work; a guard that blocks real work gets turned off, which
is worse than no guard.
## Grep returned the contents of files FileRead refuses (HIGH)
Grep never consulted the deny-list at all, so searching for a string inside
`server.pem` returned the key material verbatim — a read primitive that walked
straight past the guard on FileRead.
Grep has **two** backends and the first fix only covered one of them. The
pure-Rust fallback can check each file as it opens it; `rg` opens files itself
and had to be told up front via `--glob` exclusions derived from the same
deny-list. Patching only the fallback left the leak fully open on any machine
with ripgrep installed — which is most of them, and was this one.
Both backends are now covered, and the tests exercise them independently (the
fallback test clears PATH so `rg` cannot be found).
## Not changed
Reading `credentials.json` and `.env` stays permitted — that is a deliberate
design decision in the deny-list (only key material is read-blocked), not an
oversight, and it is unaffected here.
QA: 593 tests, 0 failures. Clippy clean under the CI gate. Release 19.06 MB.
Zero panics in production code across all six Phase 2 files. Each fix verified
by reverting it: reverting symlink resolution fails 2 tests, reverting the rg
globs fails the rg test, reverting the fallback check fails the fallback test.
Co-Authored-By: Arch Linux <noreply@archlinux.org>
ForkedInTime
added a commit
that referenced
this pull request
Aug 3, 2026
#17) Completes Phase 2 — the data-integrity half that #16 left open. - MultiEdit documented its edits as atomic but wrote inside the loop, so a batch where edit 3 of 5 failed left the other four on disk: a half-finished refactor reported as one '✗' among several '✓'. Edits are now staged and committed per file, all-or-nothing. - No atomic writes anywhere: fs::write truncates first, so a crash mid-write left the file empty with the original gone. atomic_write does temp+fsync+rename in the same directory, and carries the original mode across — without that, editing a 0600 file would silently republish it at 0644. - glob built an unbounded result Vec on broad patterns. Capped at 1000 after the mtime sort, with truncation stated rather than silent. - Encoding checked and closed with no change needed: CRLF and BOM round-trip byte-identically, non-UTF-8 is refused with bytes untouched. 601 tests, 0 failures. Clippy clean. Zero panics in production code across all six Phase 2 files plus tools/mod.rs. #16's security tests re-run green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 (filesystem tool surface, 6 files, 1,019 LOC). Two live bypasses, both reproduced before fixing and re-verified after.
Symlinks defeated every sensitive-path protection (CRITICAL)
check_sensitive_pathmatches on the file name of the path it is handed, and nothing resolved symlinks first. Measured against the real tools:.ssh/id_rsadirectlynotes.md→.ssh/id_rsaconfig.json→.aws/credentialsThe guard demonstrably worked when the name matched and demonstrably didn't when it didn't — every protection it offers was one innocuously-named link away from nothing.
This needs no unusual privileges and no suspicious command. A repository can simply ship a symlink called
README.md; asking the agent to read it exfiltrates the target. The agent can also create one itself withln -s.check_sensitive_path_resolvednow checks both the supplied path and its canonicalised target — either looking sensitive is a refusal. For paths that don't exist yet (a fresh write) it canonicalises the parent, which also catches a symlinked parent directory. Applied at all four entry points: FileRead, FileWrite, FileEdit, MultiEdit.Benign symlinks still work — a guard that blocks real work gets turned off, which is worse than no guard.
Grep returned the contents of files FileRead refuses (HIGH)
Grep never consulted the deny-list, so searching for a string inside
server.pemreturned the key material verbatim — a read primitive that walked past the guard on FileRead.Grep has two backends and my first fix only covered one. The pure-Rust fallback can check each file as it opens it;
rgopens files itself and had to be told up front via--globexclusions derived from the same deny-list. Patching only the fallback left the leak fully open on any machine with ripgrep installed — which is most of them, and was this one.Both are now covered, and the tests exercise them independently (the fallback test clears
PATHsorgcan't be found).Not changed
Reading
credentials.jsonand.envstays permitted — that's a deliberate deny-list decision (only key material is read-blocked), not an oversight.QA
593 tests, 0 failures. Clippy clean under the gate. Release 19.06 MB. Zero panics in production code across all six Phase 2 files.
Each fix verified by reverting it: reverting symlink resolution fails 2 tests, reverting the rg globs fails the rg test, reverting the fallback check fails the fallback test.