Sub-threshold finding from a focused security review of the CI URL
work — defense-in-depth even though it sits inside the documented
"trust upstream" boundary.
The mirror walker's purgeOrphans deletes local files that aren't in
the upstream's listing. It walked a dirPath built recursively from
upstream-supplied entry names and called os.Remove on the resolved
local path with no containment check. A hostile or compromised
upstream returning ".." in a directory listing could steer the
walker out of cache.root and into the parent — deleting whatever
matches the upstream's "expected to be there" filter in the wrong
directory.
A healthy master never produces such entries (listing.FromDirEntries
filters dot-prefix names), so the bug only fires under an actively
malicious or MITM'd upstream — confidence stayed below the report
threshold. But the fix is small and the cost of being wrong is real
deletion of files outside the cache, so it's worth doing.
Two layers:
1. walker.go walkDir filters upstream listing entries with name ==
"" / "." / ".." or containing "/" / "\" before recursing. Logs
a WARN with the dropped name so an operator can see if their
upstream is misbehaving.
2. purgeOrphans verifies the resolved localDir is contained under
s.cache.root (HasPrefix(root + sep) || == root) before
ReadDir+Remove. Logs a WARN and bails on mismatch.
Either layer alone would fix the original vector; both together
match the defense-in-depth pattern cachePathFor already follows for
single-file writes (line 506).
New TestWalker_HostileUpstreamCannotEscapeCacheRoot constructs a
fake upstream that returns a "../" entry in its listing, places a
sentinel file in the parent of cache.root, runs a mirror walk, and
asserts the sentinel survives. Both filter and containment guard
fire; the sentinel stays put.
Existing mirror tests unchanged — the filter only drops names that
shouldn't appear in healthy listings.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ZDDC
2ce5336289