install: don't leak package extraction temp directories into $TMPDIR - #33979
Open
robobun wants to merge 7 commits into
Open
install: don't leak package extraction temp directories into $TMPDIR#33979robobun wants to merge 7 commits into
robobun wants to merge 7 commits into
Claude / Claude Code Review
completed
Jul 11, 2026 in 26m 20s
Code review found 2 potential issues
Found 2 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/install/extract_tarball.rs:556-576 |
TOCTOU: invalid-cache-entry pre-delete can remove a concurrently-repaired entry |
| 🟡 Nit | src/sys/lib.rs:9839-9842 |
Unit-test assertion vacuous on Windows: exists_at is file-only |
Annotations
Check warning on line 576 in src/install/extract_tarball.rs
claude / Claude Code Review
TOCTOU: invalid-cache-entry pre-delete can remove a concurrently-repaired entry
The comment's justification ("no concurrent install reads from it") only holds at the moment of the `exists_at` check, not at the moment of `delete_tree` — a concurrent install can delete the corrupt entry, publish its own valid copy, and open it for hardlinking in between, after which this `delete_tree` removes a live entry and the reader fails with `ENOENT`. The preconditions (a pre-existing corrupt cache entry + two installs preempted between adjacent syscalls) are narrow enough that this sho
Check warning on line 9842 in src/sys/lib.rs
claude / Claude Code Review
Unit-test assertion vacuous on Windows: exists_at is file-only
On Windows this assertion is vacuous: `exists_at` is file-only there (per its own comment at src/sys/lib.rs:4388-4396, "Directories yield false"), and `from/sub` is a directory, so `!exists_at(...)` is always `true` regardless of whether the source was actually removed. This is the same helper-semantics issue that was just fixed in the production sad path in 9943a49 — swap to `!directory_exists_at(root, ...).unwrap_or(false)` (or `exists_at_type`) so the source-cleanup assertion is meaningful on
Loading