bug
says links counted at link size, but `fs::metadata` follows symlinks and returns the *target's* size, not the link's size. `fs::symlink_metadata` returns the link's own size. So the code contradicts its comment: a symlinked blob is counted at the target's size. Consequence: on a store where blobs are hardlinked (hardlinks don't matter — metadata is same file, fine) or symlinked from elsewhere, `held` counts target bytes, possibly double-counting... Actually if blobs are symlinked into the store from a shared volume, each blob's metadata is the target's length — the total counts real bytes, which is arguably what you want for space, but the comment says the opposite ("counted at the size of the link"). The consequence per their rationale: the total should be "the number of bytes this directory is answerable for" — with `fs::metadata`, symlink targets' sizes are counted, which for a moved store linked (hardlinked, likely — moving between volumes uses hardlinks, not symlinks; hardlink metadata len is the file size, so fine). If they meant symlink, metadata follows and gives file size. Hmm: for hardlinks, `fs::metadata` len == file size, correct. The comment says "A link is counted at the size of the link" — for a hardlink, size of link == size of file. So comment makes sense for hardlinks. For symlinks it follows. Is this a defect? If it followed a symlink out of the store... they'd delete a file outside the store? `fs::remove_file` on a symlink removes the symlink only, not the target — safe. And `meta.is_dir()` following a symlink into a directory outside the store would recurse outside the blob dir — collect would walk outside the blob directory via a symlinked dir! `fs::metadata` follows symlinks, so a symlink to a directory makes `meta.is_dir()` true and it recurses into it. Consequence: a symlink in the blobs dir pointing elsewhere causes the sweep to walk (and potentially unlink files in) a directory outside the blob store. But would an attacker plant a symlink? The blob store is written only by the proxy... A malicious upstream can't create symlinks (writes bytes). Local operator could. `BlobStore::writer`/commit never creates symlinks. Edge case. I might report as low: `collect` follows symlinks, so a symlinked directory under blobs is walked (and a symlink to a file counts at target size, contradicting the comment). This is a genuine TOCTOU-ish/symlink-traversal defect in code whose only job is filesystem walking. Severity low/medium. Given the deploy described ("store moved between volumes often has blobs linked into it") — if they symlink a directory (e.g. `blobs/sha256/ab/cd` → old store), the sweep would recurse into the old store and delete blobs there too — outside `CAIRN_BLOB_DIR`. That's concrete: line 164-168. Medium maybe. I'd rate yellow (medium) or blue (low). I'll say for the symlinked-directory traversal (deletes outside the blob dir), since their own comment anticipates linked stores. Hmm, but is symlinking a directory realistic? Their comment says "blobs linked into it rather than copied" — typical is hardlinks for files. With hardlinks, no traversal. With `cp -al` you get hardlinks. With symlinks (`ln -s`), `fs::metadata` follows. The defect: comment claims one behavior (size of the link), code does another (size of target); and directory symlinks are traversed, letting a sweep unlink files outside the blob directory. I'll report as one finding, medium or low. Let me be precise: lines 160-168. 10. **`forget` race**: index entries removed for digests that were swept; but between `referenced()` and `forget`, a fetch could have just linked an index entry for a digest the sweep decided was unreferenced? No — if the sweep decided unreferenced, that means at index-read time no entry existed; then a fetch concurrently commits blob + links entry. The blob is deleted (unreferenced at scan time... wait order: index read first, then blob walk. Fetch commit blob at T1 after blob walk already passed that leaf? If blob walk happens after commit, blob seen; index entry written at T2 after index read; sweep deletes blob, then `forget` — index map was read before T2, so `gone` hex not in map, entry not removed. Left: stale index entry pointing at deleted blob — handled by resolve (refetch). The request itself fails at open_blob with 500 though. That's finding 3. 11. **admin.rs:73 `reclaim` is sync in async handler** — same as finding 4; the blocking walk in an async fn without spawn_blocking. Report at admin.rs:76 or sweep.rs:90. I'll report once, pointing at sweep.rs:90 (the function) or admin.rs:76 (the call in async context). Both lines. Policy says name both lines. I'll cite sweep.rs:90 with mention of admin.rs:76 and main.rs:116.
sweep.rs:160-163
Evidence quote
Not recorded
Suggested fix
Not recorded