From 91fbf3272a66e77c9677f87296ab0c0e3e6e316f Mon Sep 17 00:00:00 2001 From: rysweet Date: Sun, 26 Jul 2026 23:53:03 +0000 Subject: [PATCH 1/5] wip: checkpoint after implementation (steps 7-8) Automatic checkpoint to preserve work in progress. Tests and implementation saved before refactoring phase. --- docs/concepts/agentic-disk-reclamation.md | 2 + docs/howto/configure-disk-reclamation.md | 2 + docs/reference/disk-reclaim-api.md | 2 + .../disk-reclaim-build-cache-producer.md | 361 +++++++++ src/disk_reclaim/build_cache.rs | 220 ++++++ src/disk_reclaim/executor.rs | 1 + src/disk_reclaim/guard.rs | 62 +- src/disk_reclaim/mod.rs | 74 +- tests/disk_reclaim_build_cache_producer.rs | 746 ++++++++++++++++++ 9 files changed, 1465 insertions(+), 5 deletions(-) create mode 100644 docs/reference/disk-reclaim-build-cache-producer.md create mode 100644 src/disk_reclaim/build_cache.rs create mode 100644 tests/disk_reclaim_build_cache_producer.rs diff --git a/docs/concepts/agentic-disk-reclamation.md b/docs/concepts/agentic-disk-reclamation.md index ec627721f..c02386136 100644 --- a/docs/concepts/agentic-disk-reclamation.md +++ b/docs/concepts/agentic-disk-reclamation.md @@ -8,6 +8,7 @@ doc_type: concept related: - ../howto/configure-disk-reclamation.md - ../reference/disk-reclaim-api.md + - ../reference/disk-reclaim-build-cache-producer.md - ../reference/disk-reclaim-telemetry.md - ../reference/engineer-worktree-sweep-safety.md - ./automated-disk-health.md @@ -257,6 +258,7 @@ reused unchanged. - [Configure disk reclamation (how-to)](../howto/configure-disk-reclamation.md) — operator guide, CLI, env config - [Disk reclaim API (reference)](../reference/disk-reclaim-api.md) — module API, the guard, the executor, the recipe contract +- [Disk reclaim build-cache producer (reference)](../reference/disk-reclaim-build-cache-producer.md) — the deterministic sub-artifact `StaleBuildCache` producer that lets routine reclaim hold steady-state disk below the emergency threshold - [Disk reclaim telemetry (reference)](../reference/disk-reclaim-telemetry.md) — emitted metrics - [Worktree reaping safety guards](../reference/engineer-worktree-sweep-safety.md) — the shared liveness/uncommitted-work primitives the guard composes - [Automated disk health (concept)](./automated-disk-health.md) — the superseded per-cycle check diff --git a/docs/howto/configure-disk-reclamation.md b/docs/howto/configure-disk-reclamation.md index cf119b56a..5d9da8b60 100644 --- a/docs/howto/configure-disk-reclamation.md +++ b/docs/howto/configure-disk-reclamation.md @@ -8,6 +8,7 @@ doc_type: howto related: - ../concepts/agentic-disk-reclamation.md - ../reference/disk-reclaim-api.md + - ../reference/disk-reclaim-build-cache-producer.md - ../reference/disk-reclaim-telemetry.md - ./configure-disk-health-check.md - ./reclaim-disk-space-and-run-low-space-rust-builds.md @@ -322,6 +323,7 @@ du -sh /home/azureuser/* 2>/dev/null | sort -h | tail -10 - [The simard disk tool (reference)](../reference/simard-disk-tool.md) — CLI grammar, exit codes, guard reasons - [Agentic disk reclamation (concept)](../concepts/agentic-disk-reclamation.md) — design rationale, the rails, "agent proposes, Rust disposes" - [Disk reclaim API (reference)](../reference/disk-reclaim-api.md) — module API, guard, executor, recipe contract +- [Disk reclaim build-cache producer (reference)](../reference/disk-reclaim-build-cache-producer.md) — the deterministic sub-artifact producer that makes routine reclaim free build caches so the emergency net stops firing every cycle - [Disk reclaim telemetry (reference)](../reference/disk-reclaim-telemetry.md) — emitted metrics - [Configure the disk health check](./configure-disk-health-check.md) — the superseded per-cycle check - [Reclaim disk space and run low-space Rust builds](./reclaim-disk-space-and-run-low-space-rust-builds.md) — manual build-artifact scripts diff --git a/docs/reference/disk-reclaim-api.md b/docs/reference/disk-reclaim-api.md index 32141e7a9..7fb8d9d8f 100644 --- a/docs/reference/disk-reclaim-api.md +++ b/docs/reference/disk-reclaim-api.md @@ -9,6 +9,7 @@ related: - ../concepts/agentic-disk-reclamation.md - ../howto/configure-disk-reclamation.md - ./disk-reclaim-telemetry.md + - ./disk-reclaim-build-cache-producer.md - ./disk-health-api.md - ./engineer-worktree-sweep-safety.md --- @@ -38,6 +39,7 @@ for operator usage. | `daemon_dir.rs` | `resolve_daemon_working_dirs` — the protected daemon-directory union | | `executor.rs` | `exec_reclaim` — largest-first, threshold-stop, TOCTOU-reasserting executor + `ReclaimReport` | | `recipe.rs` | Invoke `disk-reclaim.yaml`, strict marker parse, no-fallback error path | +| `build_cache.rs` | Deterministic sub-artifact `StaleBuildCache` producer + the leaf allowlist threaded into the guard — see [Disk reclaim build-cache producer](./disk-reclaim-build-cache-producer.md) | ## Data flow diff --git a/docs/reference/disk-reclaim-build-cache-producer.md b/docs/reference/disk-reclaim-build-cache-producer.md new file mode 100644 index 000000000..8175bc92b --- /dev/null +++ b/docs/reference/disk-reclaim-build-cache-producer.md @@ -0,0 +1,361 @@ +--- +title: Disk reclaim build-cache producer +description: Reference for the deterministic build-cache candidate producer in src/disk_reclaim/build_cache.rs — the sub-artifact StaleBuildCache candidates it emits under target/debug/{incremental,deps,build} and target/llvm-cov-target, the leaf-only allow-scope widening and exact-canonical build-cache-leaf guard exemption that admit them, and how routine reclaim now holds steady-state disk below the emergency threshold instead of the daemon nuking target/debug wholesale every cycle. +last_updated: 2026-07-26 +review_schedule: as-needed +owner: simard +doc_type: reference +related: + - ../concepts/agentic-disk-reclamation.md + - ../howto/configure-disk-reclamation.md + - ./disk-reclaim-api.md + - ./disk-reclaim-telemetry.md + - ./engineer-worktree-sweep-safety.md +--- + +# Disk reclaim build-cache producer + +**Module:** `src/disk_reclaim/build_cache.rs` + +The build-cache producer is the **deterministic** half of routine disk +reclamation. Where the [`disk-reclaim.yaml`](./disk-reclaim-api.md#recipers--disk-reclaimyaml--the-analysis-only-recipe) +agent *proposes* candidates non-deterministically, this module enumerates +regenerable cargo build-cache leaf directories itself — the same set every run, +no LLM in the loop — and contributes them to the candidate list the guarded +executor disposes of. It exists so **routine** reclaim (85 % ≤ used < 95 %) +actually frees space in Apply mode instead of reporting `freed 0 bytes, +everything skipped for review`, which is what forced the `emergency_cleanup` +hard stop to fire on every ~30-minute cycle and wholesale-delete `target/debug`. + +This is a reference for the module API, the leaf allowlist it produces, and the +narrow guard changes that admit its candidates. For *why* the split exists see +[Agentic disk reclamation](../concepts/agentic-disk-reclamation.md); for +operator usage see [Configure disk reclamation](../howto/configure-disk-reclamation.md). + +## The defect it fixes + +Before this producer, routine and emergency reclaim used **disjoint +allow-scopes**: + +- The routine path's `allow_roots` (§[`disk-reclaim-api.md`](./disk-reclaim-api.md#allow_roots--the-reclamation-scope)) + admitted `/worktrees` and the shared cargo target dirs, but **not** a + managed repo's own `/target`. Every `repo_root/target/*` candidate the + agent proposed was rejected `OutsideAllowRoot` → `freed 0 bytes, 0 paths + removed, K skipped for review` on every cycle while disk stayed pinned at + 94–99 %. +- Only the deterministic `emergency_cleanup` net (`disk_health.rs`, invoked from + `daemon/mod.rs` at ≥ 95 %) ever freed the build target — and only by + `rm -rf`-ing all of `target/debug` (~718 MB), which the next `cargo build` + immediately regrew. A delete-rebuild-delete loop that wasted compute and + risked build failure at 100 %. + +The producer closes the gap by making the routine path **capable of freeing the +build cache incrementally**, so steady-state disk settles comfortably below the +emergency threshold and `emergency_cleanup` stops firing. + +## What it emits + +`build_cache_candidates` returns granular [`ReclaimCandidate`](./disk-reclaim-api.md#candidaters--the-proposal-contract)s +of kind `StaleBuildCache`, one per **existing** regenerable leaf directory, for +every managed repo **and each of its `worktrees/*`**: + +| Leaf (relative to a `target/` root) | Contents | Regenerated by | +| ----------------------------------- | -------- | -------------- | +| `debug/incremental` | Incremental compilation cache | next `cargo build` | +| `debug/deps` | Compiled dependency artifacts | next `cargo build` | +| `debug/build` | Build-script outputs | next `cargo build` | +| `llvm-cov-target/debug/incremental` | Coverage-instrumented incremental cache | next `cargo llvm-cov` | +| `llvm-cov-target/debug/deps` | Coverage-instrumented deps | next `cargo llvm-cov` | +| `llvm-cov-target/debug/build` | Coverage build-script outputs | next `cargo llvm-cov` | + +The exact leaf set is the const `EVICTABLE_CACHE_DIRS`. Every emitted candidate +carries `kind = StaleBuildCache`, `est_bytes = None` (the executor re-measures; +the producer never trusts an estimate for a size decision), and a fixed, +low-cardinality `reason` string. Non-existent leaves are silently omitted +(nothing to reclaim), so a repo that has never built coverage contributes only +the `debug/*` leaves. + +### Why sub-artifact granularity, not `target/debug` + +The executor is **all-or-nothing per candidate path** — it either removes the +whole path or none of it — but it stops as soon as `%-used` drops under +`target_pct` (largest-first, minimum-necessary; see +[`exec_reclaim`](./disk-reclaim-api.md#executorrs--the-disposer)). Emitting one +`target/debug` candidate would therefore reproduce the wholesale nuke. Emitting +the **leaves** lets the executor evict the fewest cache classes needed to get +back under target — typically just `debug/incremental` — and leaves the linked +final binaries (including `target/debug/simard`, which is **never** a leaf and +never emitted) and warm `deps` in place. That is precisely what breaks the +nuke-then-rebuild loop. + +### What it never emits + +- **`target/debug` itself** — only its regenerable children. The allow-scope + widening below is leaf-only, so `starts_with` structurally forbids a + wholesale-`target/debug` candidate from ever passing containment. +- **`target/debug/simard`** (or any final binary) — excluded by construction so + the daemon's own auto-reload image is never deleted. +- **Symlinks, non-directories, or foreign-owned paths** — rejected during + enumeration (see [Safety](#safety-properties)). +- **Anything outside `managed_repos()`** — roots derive only from the hardcoded + managed-repo set and their worktrees; no external path can be injected. + +## API + +```rust +// src/disk_reclaim/build_cache.rs + +/// The regenerable cargo cache leaf dirs, relative to a `target/` root. +/// `target/debug/{incremental,deps,build}` and the coverage mirror under +/// `target/llvm-cov-target/debug/*`. `debug/` itself and final binaries +/// (`debug/simard`) are deliberately absent. +pub const EVICTABLE_CACHE_DIRS: &[&str]; + +/// The `target/` roots to enumerate: `/target` for each managed repo and +/// `/worktrees/*/target` for each of its worktrees. +pub fn target_debug_roots(repos: &[PathBuf]) -> Vec; + +/// The canonicalized, vetted leaf directories that actually exist for the given +/// managed `repos`. Internally derives the `target/` roots via +/// `target_debug_roots`, then for each root admits only the existing +/// `EVICTABLE_CACHE_DIRS` leaves. Each entry is canonicalized; symlinks, +/// non-directories, and paths not owned by the effective UID are dropped +/// (fail-closed). This is the exact allowlist threaded into the guard as +/// `build_cache_leaves`. +pub fn build_cache_leaf_dirs(repos: &[PathBuf]) -> Vec; + +/// The deterministic `StaleBuildCache` candidates for the given repos — one per +/// existing leaf, `est_bytes = None`. Merged with the recipe's proposal by +/// `run_disk_reclaim` (dedup by path). +pub fn build_cache_candidates(repos: &[PathBuf]) -> Vec; +``` + +The module is registered as `pub mod build_cache;` in +[`mod.rs`](./disk-reclaim-api.md#module-layout) and re-exported alongside the +other producers. + +## How it wires into the guard + +The producer emits candidates; it does **not** delete. Everything it emits is +still re-vetted by [`vet_candidate`](./disk-reclaim-api.md#guardrs--the-non-bypassable-rail) +at the syscall boundary. Two narrow, additive changes let the leaves through +**without** widening any other rail: + +### 1. Leaf-only allow-scope widening + +`GuardContext` gains a `build_cache_leaves: &[PathBuf]` field. `reclaim_candidates` +computes the canonicalized leaves once (via `build_cache_leaf_dirs`) and extends +the Rail-2 positive containment scope with **the leaves themselves** — not +`target/`, not `target/debug`. Because containment is component-wise +`starts_with`, admitting `…/target/debug/incremental` as an allow-root permits +that leaf and its children while **structurally forbidding** `…/target/debug` +(which is a parent, not a descendant) from ever being contained. The wholesale +nuke cannot be expressed through this scope. + +**Scope note.** A managed repo's `worktrees/*` leaves already sit under the +pre-existing `/worktrees` allow-root, so the widening's *new* reach is +effectively the repo's **own** `/target` leaves (plus the coverage +mirror) — the exact set the old `allow_roots` rejected `OutsideAllowRoot`. The +widening is still expressed leaf-by-leaf (never `target/`) for both, so the +containment guarantee is uniform. + +### 2. Exact-canonical build-cache-leaf exemption (Rail 1) + +The managed repos sit under the protected deny-set in some deployments, so the +leaves need a **surgical** carve-out from Rail 1. Concretely, the deny-set +([`ProtectedDenySet::resolve`](./disk-reclaim-api.md#protecteddenyset)) contains +the daemon's resolved `WorkingDirectory` entries; when the daemon runs *from* a +managed repo checkout, that repo root — and therefore its `target/debug/*` +leaves — is deny-set-protected, and without the carve-out routine reclaim would +reject them `ProtectedPath`. `vet_candidate` grants the exemption only +via exact **canonical equality** against the computed `build_cache_leaves` +allowlist — never a prefix, substring, or pattern match. A path is exempt from +the deny-set **iff** its canonical form equals a registered leaf exactly. +`target/debug`, a worktree root, or a sibling like `target/debug/simard` is not +in the set, so the deny-set still rejects it. + +Crucially, **Rails 2–4 remain in force for exempted leaves**: + +| Rail | Still applies to an exempted leaf? | +| ---- | ---------------------------------- | +| Rail 1 — protected deny-set | Exempt **only** on exact-canonical leaf match; everything else denied | +| Rail 2 — allow-root + symlink/canonicalize | Yes — must `starts_with` a canonical leaf allow-root; symlink/canonicalize failure → `OutsideAllowRoot` | +| Rail 3 — live process | Yes — a leaf with a live PID at/under it → `LiveProcess` (interlocks against an in-flight `cargo build`) | +| Rail 4 — `.git`/tracked-worktree vetoes | Yes — unchanged | + +The exemption is **fail-closed**: a canonicalize error means "not a registered +leaf," so the path stays protected. This double-lock — leaf-only allow-root +*and* exact-canonical deny-set exemption — is why admitting build caches does +not weaken the guard. + +## How it wires into orchestration + +`run_disk_reclaim` merges the deterministic candidates with the recipe's +proposal and de-duplicates by path: + +``` +run_disk_reclaim + ├─ run_reclaim_recipe() → Vec (agent proposal) + ├─ build_cache_candidates(repos) → Vec (deterministic) + ├─ merge + dedup by path (HashSet) + └─ reclaim_candidates(...) → every candidate re-vetted by the guard +``` + +The recipe contract is **unchanged** — the agent may still nominate build caches +and everything else it inspects; the deterministic set simply guarantees the +build-cache leaves are always present regardless of what the agent returns. This +removes the steady-state relief's dependence on non-deterministic LLM output +(the "no silent fallbacks" requirement) while keeping the recipe's broader +reasoning. There is **no fallback path**: a recipe failure still surfaces as +`SimardError::AdapterInvocationFailed`; the deterministic candidates do not mask +a broken recipe. + +`reclaim_candidates` (the shared CLI + daemon entry point) computes the leaves +and threads them into both `allow_roots` and `GuardContext.build_cache_leaves`, +so **CLI callers inherit the same safe behavior** as the daemon — no signature +change at the call sites. + +## Steady-state behavior (the guarantee, and its condition) + +With the producer wired in, when the home partition is at +`85 % ≤ used < 95 %` **and the executor is in Apply mode**, routine reclaim +frees **non-zero** bytes by evicting the minimum-necessary build-cache leaves and +drives usage down to `target_pct` (≤ the emergency `95 %` gate). Disk settles in +a comfortable band below the emergency threshold instead of oscillating 94–99 %, +so `emergency_cleanup` stops firing every cycle. + +This guarantee is **conditional on Apply mode** — the same condition under which +real deletion is intended. The producer makes routine reclaim *capable and +correct*; it does **not** force-enable apply. The daemon still ships in +**dry-run + human-review by default** and only deletes when an operator sets +`SIMARD_DISK_RECLAIM_DAEMON_APPLY=1` (see +[Configuration](#configuration)). In dry-run each leaf appears in +`would_remove[]` carrying its own measured `bytes`, but the report-level +`bytes_freed` stays **0** (only an actual apply-mode removal increments it) and +nothing is deleted. + +The emergency safety net is **untouched**: `emergency_cleanup` / `disk_health.rs` +and the `daemon/mod.rs` reclaim-threshold block remain the deterministic +`≥ 95 %` hard stop for the case where the host is too starved to run the agent. +The producer only tightens the routine path so that stop is rarely reached. + +## Configuration + +The producer introduces **no new configuration surface**. It reuses the existing +disk-reclaim knobs: + +| Variable | Effect on the build-cache producer | Default | +| -------- | ---------------------------------- | ------- | +| `SIMARD_DISK_RECLAIM_PCT` | Routine trigger + the `target_pct` the executor evicts leaves down to. Clamped `[1, 99]`. | `85` | +| `SIMARD_DISK_RECLAIM_DAEMON_APPLY` | Whether the **daemon** routine path may actually delete leaves. Unset/`0` = dry-run + human-review (leaves shown in `would_remove[]`, nothing deleted). `1` = daemon evicts leaves for real. No effect on the CLI (`--apply` is always honored there). | unset (dry-run) | +| `SIMARD_GIT_PROTECTED_REPOS` | Extra repo roots added to the protected deny-set. Preserved and honored — a protected repo's leaves are **not** exempted unless they exactly match a computed `build_cache_leaves` entry. | unset | + +All existing overrides and thresholds are preserved; the DryRun default is +retained. Enabling `SIMARD_DISK_RECLAIM_DAEMON_APPLY=1` is what actually converts +the observed loop into steady-state self-healing. + +## Examples + +### Preview what routine reclaim would evict (dry-run) + +The default CLI run now surfaces the deterministic build-cache leaves alongside +any agent proposals: + +```bash +simard disk-reclaim --report-json | jq '.would_remove[] | select(.kind == "stale_build_cache")' +``` + +```json +{ + "path": "/home/azureuser/src/Simard/target/debug/incremental", + "kind": "stale_build_cache", + "bytes": 512114688, + "primitive": "remove_dir" +} +``` + +`target/debug` itself and `target/debug/simard` never appear — only the +regenerable leaves. + +### Reclaim by hand (leaf-granular) + +```bash +simard disk-reclaim --apply +``` + +```text +disk-reclaim (apply) — home partition 96% used, target 85% +REMOVED stale_build_cache /home/azureuser/src/Simard/target/debug/incremental 488M (rm -rf) +REMOVED stale_build_cache /home/azureuser/src/Simard/target/debug/deps 402M (rm -rf) +reclaimed 890M — home partition now 84% used (target 85% reached); 0 candidates left for human review +``` + +The executor stops at the target — it evicted `incremental` and `deps` but left +`build/`, the final binaries, and everything else, because usage was already +under target. Compare with the pre-fix behavior, where the only relief was +`emergency_cleanup` nuking all ~718 MB of `target/debug`. + +### Confirm the emergency net stopped firing + +After enabling apply on the daemon, the routine one-liner shows non-zero freed +bytes and the `EMERGENCY disk cleanup` line disappears from the steady state: + +```bash +grep -E "disk reclaim|EMERGENCY disk cleanup" ~/.simard/ooda.log | tail -5 +``` + +```text +[2026-07-26T22:14:01Z] disk reclaim: 88% -> 84% used, freed 934281216 bytes, 2 paths removed, 0 skipped for review +[2026-07-26T22:44:02Z] disk reclaim: 86% -> 84% used, freed 512114688 bytes, 1 paths removed, 0 skipped for review +``` + +Routine reclaim now holds the band; no `[simard] EMERGENCY disk cleanup: N% -> +freed bytes` entries follow. + +## Safety properties + +- **Fail-closed by construction.** A canonicalize failure means "not a + registered leaf" → the path stays protected. An ambiguous apply-gate parse → + DryRun. +- **Symlinks and non-directories rejected.** Enumeration uses + `symlink_metadata` and acts only on canonical, resolved directories — closing + symlink-swap and `..`-traversal into a protected or foreign location. +- **Exact-canonical deny-set exemption only.** The Rail-1 carve-out is never a + `starts_with`/pattern; it is exact canonical equality against the computed + leaf allowlist, double-locked by the leaf-only allow-root. +- **Ownership check.** Leaves not owned by the effective UID are dropped — no + cross-user or root deletion. The apply path is still refused entirely under + `geteuid() == 0`. +- **TOCTOU re-assert.** At the syscall boundary `RealPathRemover` already + re-canonicalizes the path and re-asserts allow-root containment + (`under_any_root(canon, allow_roots)`) immediately before `remove_dir_all`, + refusing anything that no longer resolves under a live allow-root. For a + `StaleBuildCache` leaf this is tightened with a **type/owner re-stat** — the + path must still be a real directory (not a symlink swapped in after + enumeration) owned by the effective UID, or the candidate is aborted. Because + vetting is per-candidate, one abort skips only that leaf and never fails the + run. +- **Live-process interlock.** Rail 3 vetoes any leaf with a live PID at/under it, + so an in-flight `cargo build` writing `deps`/`build` is never deleted from + under itself (worst case it regenerates next cycle — strictly safer than the + prior wholesale nuke). +- **Auditability.** Every apply-mode eviction emits structured `tracing` + + OTel (path, bytes, `reason = stale_build_cache`, `dry_run`). No + `print!`/`println!`; no silent fallbacks. + +## Telemetry + +The producer adds **no new metric series and no new `RejectReason`**. Evicted +leaves increment the existing `simard.disk.reclaim.paths_removed` / +`simard.disk.reclaim.bytes_freed` counters with `kind = stale_build_cache`, and +any refused leaf increments `simard.disk.reclaim.candidates_skipped` with its +existing `reject_reason`. See +[Disk reclaim telemetry](./disk-reclaim-telemetry.md). + +## Related + +- [Agentic disk reclamation (concept)](../concepts/agentic-disk-reclamation.md) — the "agent proposes, Rust disposes" split and the hard rails +- [Configure disk reclamation (how-to)](../howto/configure-disk-reclamation.md) — operator CLI, thresholds, the daemon trigger +- [Disk reclaim API (reference)](./disk-reclaim-api.md) — the candidate contract, `vet_candidate`, `exec_reclaim`, the recipe contract +- [Disk reclaim telemetry (reference)](./disk-reclaim-telemetry.md) — emitted metrics +- [Worktree reaping safety guards (reference)](./engineer-worktree-sweep-safety.md) — the shared liveness/uncommitted-work primitives the guard composes diff --git a/src/disk_reclaim/build_cache.rs b/src/disk_reclaim/build_cache.rs new file mode 100644 index 000000000..cdb5b4a1c --- /dev/null +++ b/src/disk_reclaim/build_cache.rs @@ -0,0 +1,220 @@ +//! The **deterministic** half of routine disk reclamation (issue #4810). +//! +//! Where the `disk-reclaim.yaml` agent *proposes* candidates non-deterministically, +//! this module enumerates the regenerable cargo build-cache leaf directories +//! itself — the same set every run, no LLM in the loop — and contributes them to +//! the candidate list the guarded executor disposes of. +//! +//! It exists because ROUTINE reclaim (`85 % ≤ used < 95 %`) never freed the build +//! target: the routine `allow_roots` admitted `/worktrees` and the shared +//! cargo target dirs but **not** a managed repo's own `/target`, so every +//! `repo_root/target/*` candidate was rejected `OutsideAllowRoot` → `freed 0 +//! bytes, everything skipped for review`. Only the emergency net (`disk_health.rs` +//! at `≥ 95 %`) ever freed the target, and only by `rm -rf`-ing all of +//! `target/debug` (~718 MB) which the next `cargo build` immediately regrew — a +//! delete-rebuild-delete loop. +//! +//! The producer closes the gap **incrementally**: because the executor is +//! all-or-nothing per candidate path but stops as soon as usage drops under +//! `target_pct`, emitting the regenerable **leaves** (`target/debug/incremental`, +//! `…/deps`, `…/build`, and the `llvm-cov-target` mirror) lets the executor evict +//! the fewest cache classes needed to get back under target — never the wholesale +//! `target/debug`, and never the final `simard` binary. +//! +//! See `docs/reference/disk-reclaim-build-cache-producer.md` for the full +//! contract and the two narrow guard changes (leaf-only allow-scope + the +//! exact-canonical deny-set exemption) that admit these candidates. + +use std::os::unix::fs::MetadataExt; +use std::path::{Path, PathBuf}; + +use super::candidate::{CandidateKind, ReclaimCandidate}; + +/// The regenerable cargo cache leaf dirs, relative to a `target/` root. These are +/// `target/debug/{incremental,deps,build}` and the coverage mirror under +/// `target/llvm-cov-target/debug/*`. `debug/` itself and final binaries +/// (`debug/simard`) are deliberately absent — evicting those is the wholesale +/// nuke this fix removes. +pub const EVICTABLE_CACHE_DIRS: &[&str] = &[ + "debug/incremental", + "debug/deps", + "debug/build", + "llvm-cov-target/debug/incremental", + "llvm-cov-target/debug/deps", + "llvm-cov-target/debug/build", +]; + +/// Fixed, low-cardinality rationale carried on every emitted candidate. The +/// guard re-derives the real primitive and the executor re-measures the size, so +/// this string is purely informational (and never a telemetry attribute). +const BUILD_CACHE_REASON: &str = "regenerable cargo build cache (routine reclaim)"; + +/// The `target/` roots to enumerate for a set of managed repos: `/target` +/// for each repo plus `/worktrees/*/target` for each of its worktrees. +/// +/// Worktree subdirectories are read via `read_dir` (which does not follow the +/// final symlink component), and a symlinked `worktrees/*` entry is skipped +/// (`file_type().is_dir()` is false for a symlink) so a swapped link cannot +/// redirect enumeration into a foreign tree. The returned roots are **not** +/// filtered for existence here — [`build_cache_leaf_dirs`] does the existence, +/// symlink, and ownership vetting per leaf. +pub fn target_debug_roots(repos: &[PathBuf]) -> Vec { + let mut roots = Vec::new(); + for repo in repos { + roots.push(repo.join("target")); + + let worktrees = repo.join("worktrees"); + if let Ok(entries) = std::fs::read_dir(&worktrees) { + let mut wt_targets: Vec = entries + .flatten() + .filter(|e| e.file_type().map(|t| t.is_dir()).unwrap_or(false)) + .map(|e| e.path().join("target")) + .collect(); + // Deterministic ordering regardless of directory iteration order. + wt_targets.sort(); + roots.extend(wt_targets); + } + } + roots +} + +/// The canonicalized, vetted build-cache leaf directories that actually exist +/// for the given managed `repos`. Internally derives the `target/` roots via +/// [`target_debug_roots`], then for each root checks every entry in +/// [`EVICTABLE_CACHE_DIRS`] and admits it **only** when it is a real, non-symlink +/// directory owned by the effective UID; it is then canonicalized. Everything +/// else (missing leaf, symlink, non-directory, foreign owner, canonicalize +/// failure) is silently dropped — **fail-closed**. The result is deduplicated and +/// is the exact allowlist threaded into the guard as `build_cache_leaves`. +pub fn build_cache_leaf_dirs(repos: &[PathBuf]) -> Vec { + let mut leaves: Vec = Vec::new(); + for root in target_debug_roots(repos) { + for rel in EVICTABLE_CACHE_DIRS { + if let Some(canon) = vetted_leaf(&root.join(rel)) + && !leaves.contains(&canon) + { + leaves.push(canon); + } + } + } + leaves +} + +/// The deterministic `StaleBuildCache` candidates for the given managed repos — +/// one per existing leaf, `est_bytes = None` (the executor re-measures; the +/// producer never trusts an estimate for a size decision). Deterministic and +/// env-independent: candidate selection is a pure function of the on-disk layout. +pub fn build_cache_candidates(repos: &[PathBuf]) -> Vec { + build_cache_leaf_dirs(repos) + .into_iter() + .map(|path| ReclaimCandidate { + path, + kind: CandidateKind::StaleBuildCache, + parent_repo: None, + reason: Some(BUILD_CACHE_REASON.to_string()), + est_bytes: None, + }) + .collect() +} + +/// Vet one candidate leaf: it must be a real, non-symlink directory owned by the +/// effective UID. Returns its canonical path on success, `None` otherwise +/// (fail-closed). Uses `symlink_metadata` so the final component is not followed, +/// closing symlink-swap into a foreign or protected location. +fn vetted_leaf(path: &Path) -> Option { + let meta = std::fs::symlink_metadata(path).ok()?; + if meta.file_type().is_symlink() || !meta.is_dir() { + return None; + } + // SAFETY: `geteuid` takes no arguments, reads no memory, and cannot fail. + let euid = unsafe { libc::geteuid() }; + if meta.uid() != euid { + return None; + } + std::fs::canonicalize(path).ok() +} + +#[cfg(test)] +mod tests { + use super::*; + use std::os::unix::fs::symlink; + use tempfile::TempDir; + + /// Create the three regenerable `target/debug/*` leaves plus a `simard` + /// binary file under `root`. + fn basic_repo(root: &Path) { + for leaf in [ + "target/debug/incremental", + "target/debug/deps", + "target/debug/build", + ] { + std::fs::create_dir_all(root.join(leaf)).expect("leaf dir"); + } + std::fs::write(root.join("target/debug/simard"), b"ELF").expect("binary"); + } + + #[test] + fn evictable_dirs_exclude_debug_root_and_binary() { + assert!(EVICTABLE_CACHE_DIRS.contains(&"debug/incremental")); + assert!(EVICTABLE_CACHE_DIRS.contains(&"llvm-cov-target/debug/deps")); + assert!( + !EVICTABLE_CACHE_DIRS + .iter() + .any(|d| *d == "debug" || d.ends_with("/debug")), + "`debug` itself must never be an evictable leaf", + ); + assert!( + !EVICTABLE_CACHE_DIRS.iter().any(|d| d.contains("simard")), + "the final binary must never be an evictable leaf", + ); + } + + #[test] + fn leaf_dirs_are_existing_debug_leaves_only() { + let tmp = TempDir::new().unwrap(); + basic_repo(tmp.path()); + let leaves = build_cache_leaf_dirs(std::slice::from_ref(&tmp.path().to_path_buf())); + + for present in [ + "target/debug/incremental", + "target/debug/deps", + "target/debug/build", + ] { + let want = tmp.path().join(present).canonicalize().unwrap(); + assert!(leaves.contains(&want), "missing {present}: {leaves:?}"); + } + // Neither `target/debug` nor the binary is ever a leaf. + let debug = tmp.path().join("target/debug").canonicalize().unwrap(); + assert!(!leaves.contains(&debug)); + } + + #[test] + fn leaf_dirs_reject_symlinked_leaf() { + let tmp = TempDir::new().unwrap(); + std::fs::create_dir_all(tmp.path().join("target/debug/incremental")).unwrap(); + let elsewhere = tmp.path().join("evil"); + std::fs::create_dir_all(&elsewhere).unwrap(); + symlink(&elsewhere, tmp.path().join("target/debug/deps")).unwrap(); + + let leaves = build_cache_leaf_dirs(std::slice::from_ref(&tmp.path().to_path_buf())); + let evil = elsewhere.canonicalize().unwrap(); + assert!( + !leaves.contains(&evil), + "a symlinked leaf must never be admitted: {leaves:?}", + ); + } + + #[test] + fn candidates_are_stale_build_cache_without_estimate() { + let tmp = TempDir::new().unwrap(); + basic_repo(tmp.path()); + let candidates = build_cache_candidates(std::slice::from_ref(&tmp.path().to_path_buf())); + assert!(!candidates.is_empty()); + for c in &candidates { + assert_eq!(c.kind, CandidateKind::StaleBuildCache); + assert_eq!(c.est_bytes, None); + assert!(c.reason.is_some()); + assert!(!c.path.ends_with("simard")); + } + } +} diff --git a/src/disk_reclaim/executor.rs b/src/disk_reclaim/executor.rs index 1b8dd2c8f..e819da1c1 100644 --- a/src/disk_reclaim/executor.rs +++ b/src/disk_reclaim/executor.rs @@ -362,6 +362,7 @@ mod tests { ) -> GuardContext<'a> { GuardContext { allow_roots: &env.allow_roots, + build_cache_leaves: &[], protected, live_probe: live, wt_probe: wt, diff --git a/src/disk_reclaim/guard.rs b/src/disk_reclaim/guard.rs index da13950f7..ade02a629 100644 --- a/src/disk_reclaim/guard.rs +++ b/src/disk_reclaim/guard.rs @@ -210,6 +210,14 @@ impl SizeMeasurer for CachingSizeMeasurer<'_> { pub struct GuardContext<'a> { /// The positive containment allow-list (reclamation scope). pub allow_roots: &'a [PathBuf], + /// The registered build-cache leaf allowlist (issue #4810). Each entry is a + /// canonical `target/debug/{incremental,deps,build}` (or `llvm-cov-target` + /// mirror) leaf that routine reclaim may evict. A candidate whose canonical + /// path **exactly** equals one of these is exempted from the protected + /// deny-set and serves as its own allow-root anchor — never a prefix, + /// substring, or pattern match. Rails 3–4 still apply. Empty for callers + /// that do not manage build caches (behaviour is then identical to before). + pub build_cache_leaves: &'a [PathBuf], /// The protected deny-set. pub protected: &'a ProtectedDenySet, /// Live-PID probe (fail-closed). @@ -230,16 +238,33 @@ pub struct GuardContext<'a> { pub fn vet_candidate(candidate: &ReclaimCandidate, ctx: &GuardContext<'_>) -> Verdict { let path = candidate.path.as_path(); - // Rail 1 — protected deny-set (absolute; checked first so it always wins). - if ctx.protected.contains(path) { + // A registered build-cache leaf (exact canonical match) gets a surgical, + // double-locked exemption (issue #4810): it bypasses the protected deny-set + // (Rail 1) and serves as its own allow-root anchor (Rail 2). This is what + // lets ROUTINE reclaim evict `/target/debug/{incremental,deps,build}` + // even when the managed repo root itself is deny-set-protected (the daemon + // runs from a managed checkout), instead of only the emergency net nuking + // `target/debug` wholesale. The exemption is **fail-closed**: a symlink, + // non-directory, or canonicalize failure means "not a registered leaf", and + // the path then falls through to the normal rails. Rails 3–4 still apply. + let is_build_cache_leaf = is_registered_build_cache_leaf(path, ctx.build_cache_leaves); + + // Rail 1 — protected deny-set (absolute; checked first so it always wins), + // except for an exact-canonical registered build-cache leaf. + if !is_build_cache_leaf && ctx.protected.contains(path) { return Verdict::Reject { reason: RejectReason::ProtectedPath, }; } // Rail 2 — allow-root containment + symlink/canonicalize refusal. Reuses the - // audited component-wise primitive (no string-prefix confusion). - if !is_safe_to_delete(path, ctx.allow_roots, ctx.protected.as_paths()) { + // audited component-wise primitive (no string-prefix confusion). A registered + // leaf equals (not strictly-inside) its leaf allow-root, which + // `is_safe_to_delete` refuses by design; the exemption above already proved + // the leaf is a real, non-symlink directory canonically equal to a registered + // leaf, so it satisfies containment on its own — never `target/debug`, which + // is a parent (not a descendant) and can never match a leaf. + if !is_build_cache_leaf && !is_safe_to_delete(path, ctx.allow_roots, ctx.protected.as_paths()) { return Verdict::Reject { reason: RejectReason::OutsideAllowRoot, }; @@ -281,6 +306,34 @@ pub fn vet_candidate(candidate: &ReclaimCandidate, ctx: &GuardContext<'_>) -> Ve } } +/// `true` iff `path` is an **exact-canonical** registered build-cache leaf +/// (issue #4810). Fail-closed by construction: the path must be a real, +/// non-symlink directory whose canonical form exactly equals one of `leaves`. +/// A symlink (symlink-swap defense), a non-directory, a canonicalize failure, or +/// any prefix/substring relationship is **not** a match, so `target/debug`, a +/// worktree root, or a sibling like `target/debug/simard` is never exempted. +fn is_registered_build_cache_leaf(path: &Path, leaves: &[PathBuf]) -> bool { + if leaves.is_empty() { + return false; + } + let meta = match std::fs::symlink_metadata(path) { + Ok(m) => m, + Err(_) => return false, + }; + if meta.file_type().is_symlink() || !meta.is_dir() { + return false; + } + let canon = match std::fs::canonicalize(path) { + Ok(c) => c, + Err(_) => return false, + }; + leaves.iter().any(|leaf| { + std::fs::canonicalize(leaf) + .map(|l| l == canon) + .unwrap_or(false) + }) +} + #[cfg(test)] mod tests { use super::*; @@ -354,6 +407,7 @@ mod tests { ) -> Verdict { let ctx = GuardContext { allow_roots, + build_cache_leaves: &[], protected, live_probe: live, wt_probe: wt, diff --git a/src/disk_reclaim/mod.rs b/src/disk_reclaim/mod.rs index 623d835f0..5b0b93b44 100644 --- a/src/disk_reclaim/mod.rs +++ b/src/disk_reclaim/mod.rs @@ -29,6 +29,11 @@ pub mod guard; pub mod prod; pub mod recipe; +pub mod build_cache; + +pub use build_cache::{ + EVICTABLE_CACHE_DIRS, build_cache_candidates, build_cache_leaf_dirs, target_debug_roots, +}; pub use candidate::{CandidateKind, MAX_CANDIDATES, ReclaimCandidate, parse_candidates}; pub use daemon_dir::resolve_daemon_working_dirs; pub use executor::{ @@ -191,7 +196,16 @@ pub fn reclaim_candidates( effective }; - let allow = allow_roots(state_root); + // Deterministic build-cache leaves for the managed repos (issue #4810). + // Threaded into BOTH the Rail-2 allow-scope and the guard's registered-leaf + // allowlist so routine reclaim can evict `/target/debug/*` without + // widening any other rail. The widening is expressed leaf-by-leaf (never + // `target/`), so `starts_with` structurally forbids a wholesale-`target/debug` + // candidate from ever being contained. Empty when nothing is built yet. + let build_cache_leaves = build_cache::build_cache_leaf_dirs(&managed_repos()); + let mut allow = allow_roots(state_root); + allow.extend(build_cache_leaves.iter().cloned()); + let protected = ProtectedDenySet::resolve(Path::new("/proc")); let live = crate::worktree_gc::ProcfsLiveProcessProbe::new(); let wt = RealTrackedWorktreeProbe; @@ -201,6 +215,7 @@ pub fn reclaim_candidates( let measurer = CachingSizeMeasurer::new(&du); let ctx = GuardContext { allow_roots: &allow, + build_cache_leaves: &build_cache_leaves, protected: &protected, live_probe: &live, wt_probe: &wt, @@ -249,11 +264,40 @@ pub fn run_disk_reclaim( }; let (candidates, _used_pct) = run_reclaim_recipe(&invoker)?; + // Merge the deterministic build-cache leaves with the agent's proposal, + // de-duplicating by path (issue #4810). The recipe contract is unchanged — + // the agent may still nominate build caches and everything else — but the + // deterministic set guarantees the regenerable leaves are always present + // regardless of what the agent returns, removing steady-state relief's + // dependence on non-deterministic LLM output. There is NO fallback: a recipe + // failure still surfaces above as `AdapterInvocationFailed`; the deterministic + // candidates never mask a broken recipe. + let candidates = merge_dedup_by_path(candidates, build_cache_candidates(&managed_repos())); + Ok(reclaim_candidates( candidates, state_root, mode, target_pct, source, )) } +/// Merge two candidate lists, keeping the first occurrence of each path. The +/// recipe proposal wins on a path collision (its `kind`/`reason` are advisory and +/// re-derived by the guard anyway), and each deterministic leaf is appended only +/// when no candidate already targets that path. +fn merge_dedup_by_path( + primary: Vec, + extra: Vec, +) -> Vec { + let mut seen: std::collections::HashSet = + primary.iter().map(|c| c.path.clone()).collect(); + let mut out = primary; + for candidate in extra { + if seen.insert(candidate.path.clone()) { + out.push(candidate); + } + } + out +} + /// Whether the daemon self-heal trigger should fire: the measured home-partition /// `%-used` is at or above the configured reclaim threshold. Kept as a named, /// tested predicate so the trigger semantics live in one place rather than @@ -567,4 +611,32 @@ mod tests { "unknown_pr_state" ); } + + #[test] + fn merge_dedup_by_path_appends_only_new_paths() { + let mk = |p: &str, kind: CandidateKind| ReclaimCandidate { + path: PathBuf::from(p), + kind, + parent_repo: None, + reason: None, + est_bytes: None, + }; + // The recipe proposal already includes `/a`; the deterministic set adds + // `/a` (a collision, dropped) and `/b` (new, appended). + let recipe = vec![mk("/a", CandidateKind::OrphanDir)]; + let deterministic = vec![ + mk("/a", CandidateKind::StaleBuildCache), + mk("/b", CandidateKind::StaleBuildCache), + ]; + + let merged = merge_dedup_by_path(recipe, deterministic); + let paths: Vec<_> = merged.iter().map(|c| c.path.clone()).collect(); + assert_eq!( + paths, + vec![PathBuf::from("/a"), PathBuf::from("/b")], + "collision keeps the first (recipe) entry; only the new path is appended", + ); + // The surviving `/a` is the recipe's, proving first-wins on collision. + assert_eq!(merged[0].kind, CandidateKind::OrphanDir); + } } diff --git a/tests/disk_reclaim_build_cache_producer.rs b/tests/disk_reclaim_build_cache_producer.rs new file mode 100644 index 000000000..0fb090eca --- /dev/null +++ b/tests/disk_reclaim_build_cache_producer.rs @@ -0,0 +1,746 @@ +//! TDD contract (Step 7 — tests first) for the deterministic build-cache +//! candidate producer that makes ROUTINE disk reclaim actually free the cargo +//! build cache, so the daemon stops oscillating 94–99 % and nuking +//! `target/debug` wholesale on every ~30-minute cycle (issue #4810). +//! +//! These tests are written **before** the implementation and MUST fail until +//! `src/disk_reclaim/build_cache.rs` exists and the guard is wired for +//! build-cache leaves. This whole test crate references the not-yet-existing +//! `simard::disk_reclaim::build_cache` module and the new +//! `GuardContext.build_cache_leaves` field, so it will not compile — and thus +//! every test fails — until the feature is built. Because integration test +//! files are separate compilation units, this does not break the rest of the +//! suite. +//! +//! Contract sources: `docs/reference/disk-reclaim-build-cache-producer.md` +//! (the retcon reference) plus the finalized Step-2c requirements and design +//! spec. What is asserted here: +//! 1. The producer emits granular `StaleBuildCache` leaves under +//! `target/debug/{incremental,deps,build}` (+ the llvm-cov mirror) — never +//! `target/debug` itself, never `target/debug/simard`, never symlinks. +//! 2. The guard admits a registered leaf (Allow{RemoveDir}) via the leaf-only +//! allow-scope and the exact-canonical deny-set exemption, while every +//! non-leaf sibling / parent / worktree-root stays rejected, and Rails 2–4 +//! (allow-root, live-process) remain in force for exempted leaves. +//! 3. The executor, fed the producer's candidates, frees non-zero bytes and +//! stops at `target_pct` (minimum-necessary) in Apply mode, frees 0 in +//! DryRun, and never removes the final binary. +//! 4. The producer is deterministic and env-independent; existing thresholds +//! / apply-gate defaults are preserved. + +use std::cell::RefCell; +use std::os::unix::fs::symlink; +use std::path::{Path, PathBuf}; + +use serial_test::serial; +use tempfile::TempDir; + +use simard::disk_pressure::check::{DiskStat, DiskStatProvider}; +use simard::disk_reclaim::build_cache::{ + EVICTABLE_CACHE_DIRS, build_cache_candidates, build_cache_leaf_dirs, target_debug_roots, +}; +use simard::disk_reclaim::{ + CandidateKind, GuardContext, PathRemover, ProtectedDenySet, ReclaimCandidate, ReclaimMode, + ReclaimPrimitive, RejectReason, SizeMeasurer, TrackedWorktreeProbe, Verdict, WorktreeVerdict, + exec_reclaim, vet_candidate, +}; +use simard::worktree_gc::liveness::LiveProcessProbe; + +// --------------------------------------------------------------------------- +// Test doubles (all target traits are `pub`, so integration tests can supply +// their own hermetic implementations — the crate's internal `#[cfg(test)]` +// fakes are not reachable from here). +// --------------------------------------------------------------------------- + +/// Live-process probe: reports "live" for any candidate at/under a listed path. +struct FakeLive { + live: Vec, +} +impl FakeLive { + fn none() -> Self { + Self { live: vec![] } + } + fn at(path: &Path) -> Self { + Self { + live: vec![path.to_path_buf()], + } + } +} +impl LiveProcessProbe for FakeLive { + fn worktree_has_live_process(&self, dir: &Path) -> bool { + let d = dir.canonicalize().unwrap_or_else(|_| dir.to_path_buf()); + self.live.iter().any(|p| { + let c = p.canonicalize().unwrap_or_else(|_| p.clone()); + d.starts_with(&c) || c.starts_with(&d) + }) + } +} + +/// Tracked-worktree probe returning a fixed verdict. For a non-git leaf (no +/// `.git`) the guard must NOT consult this at all; we return a hard `Reject` +/// so a leaf that still ends up `Allow` proves the tracked-worktree rail was +/// (correctly) not applied to it. +struct FixedWt(WorktreeVerdict); +impl TrackedWorktreeProbe for FixedWt { + fn assess(&self, _worktree: &Path) -> WorktreeVerdict { + self.0 + } +} + +/// Size measurer backed by a path→bytes map (default 0). Keyed by the exact +/// candidate path so ordering in the executor is deterministic. +#[derive(Default)] +struct MapMeasurer(RefCell>); +impl MapMeasurer { + fn set(&self, p: &Path, b: u64) { + self.0.borrow_mut().insert(p.to_path_buf(), b); + } +} +impl SizeMeasurer for MapMeasurer { + fn measure(&self, p: &Path) -> u64 { + *self.0.borrow().get(p).unwrap_or(&0) + } +} + +/// Disk provider returning a scripted sequence of `%-used` values; each `stat` +/// consumes the next, the last repeats (mirrors the crate's own executor test +/// double so the stop-at-target semantics are exercised identically). +struct ScriptedDisk(RefCell>); +impl ScriptedDisk { + fn new(pcts: Vec) -> Self { + Self(RefCell::new(pcts)) + } +} +impl DiskStatProvider for ScriptedDisk { + fn stat(&self, _path: &Path) -> Result { + let mut v = self.0.borrow_mut(); + let pct = if v.len() > 1 { v.remove(0) } else { v[0] }; + Ok(DiskStat { + free_bytes: 100 - pct as u64, + total_bytes: 100, + }) + } +} + +/// Records every remove call and reports success. Used to observe *which* +/// leaves the executor evicts without touching real system paths. +#[derive(Default)] +struct RecordingRemover(RefCell>); +impl PathRemover for RecordingRemover { + fn remove(&self, primitive: ReclaimPrimitive, path: &Path) -> Result<(), String> { + self.0.borrow_mut().push((primitive, path.to_path_buf())); + Ok(()) + } +} + +// --------------------------------------------------------------------------- +// Fixture: a fake managed repo with a cargo `target/` tree. +// --------------------------------------------------------------------------- + +/// A managed-repo fixture rooted at a tempdir, with a realistic `target/debug` +/// layout (regenerable leaves + the final `simard` binary) and, optionally, a +/// coverage mirror and worktrees. +struct RepoFixture { + _tmp: TempDir, + root: PathBuf, +} +impl RepoFixture { + /// A repo with `target/debug/{incremental,deps,build}` dirs and a + /// `target/debug/simard` binary file. No coverage mirror, no worktrees. + fn basic() -> Self { + let tmp = TempDir::new().expect("tempdir"); + let root = tmp.path().to_path_buf(); + for leaf in [ + "target/debug/incremental", + "target/debug/deps", + "target/debug/build", + ] { + std::fs::create_dir_all(root.join(leaf)).expect("leaf dir"); + } + // The final binary must never be a leaf / candidate. + std::fs::write(root.join("target/debug/simard"), b"ELF").expect("binary"); + Self { _tmp: tmp, root } + } + + fn leaf(&self, rel: &str) -> PathBuf { + self.root.join(rel) + } +} + +/// True iff `paths` contains a path that canonically equals `expect`. +fn contains_canon(paths: &[PathBuf], expect: &Path) -> bool { + let e = match expect.canonicalize() { + Ok(e) => e, + Err(_) => return false, + }; + paths + .iter() + .any(|p| p.canonicalize().map(|c| c == e).unwrap_or(false)) +} + +/// Vet a single candidate against a hermetic guard context wired the way +/// production's `reclaim_candidates` wires it: the build-cache leaves are added +/// both to the allow-scope and to `build_cache_leaves`. +#[allow(clippy::too_many_arguments)] +fn vet( + candidate: &ReclaimCandidate, + allow_roots: &[PathBuf], + build_cache_leaves: &[PathBuf], + protected: &ProtectedDenySet, + live: &dyn LiveProcessProbe, + wt: &dyn TrackedWorktreeProbe, + measurer: &dyn SizeMeasurer, +) -> Verdict { + let ctx = GuardContext { + allow_roots, + build_cache_leaves, + protected, + live_probe: live, + wt_probe: wt, + measurer, + }; + vet_candidate(candidate, &ctx) +} + +fn cand(path: &Path, kind: CandidateKind) -> ReclaimCandidate { + ReclaimCandidate { + path: path.to_path_buf(), + kind, + parent_repo: None, + reason: Some("agent advisory — ignored by the guard".to_string()), + est_bytes: Some(9_999_999), + } +} + +// =========================================================================== +// 1. Producer: EVICTABLE_CACHE_DIRS constant +// =========================================================================== + +#[test] +fn evictable_cache_dirs_are_the_regenerable_leaves_only() { + let set: Vec<&str> = EVICTABLE_CACHE_DIRS.to_vec(); + + // The six regenerable leaves (debug + coverage mirror), relative to a + // `target/` root. + for expected in [ + "debug/incremental", + "debug/deps", + "debug/build", + "llvm-cov-target/debug/incremental", + "llvm-cov-target/debug/deps", + "llvm-cov-target/debug/build", + ] { + assert!( + set.contains(&expected), + "EVICTABLE_CACHE_DIRS must include the regenerable leaf {expected:?}; got {set:?}", + ); + } + + // `debug` itself and the final binary must never be evictable leaves — + // that is exactly the wholesale-nuke the fix removes. + assert!( + !set.iter().any(|d| *d == "debug" || d.ends_with("/debug")), + "EVICTABLE_CACHE_DIRS must not contain `debug` itself (only its children): {set:?}", + ); + assert!( + !set.iter().any(|d| d.contains("simard")), + "EVICTABLE_CACHE_DIRS must never target the final `simard` binary: {set:?}", + ); +} + +// =========================================================================== +// 2. Producer: target_debug_roots — repo target + worktree targets +// =========================================================================== + +#[test] +fn target_debug_roots_covers_repo_and_its_worktrees() { + let repo = RepoFixture::basic(); + // Give the repo a worktree so the producer reaches worktrees/*/target too. + let wt = repo.root.join("worktrees/wt1"); + std::fs::create_dir_all(wt.join("target/debug")).expect("worktree target"); + + let roots = target_debug_roots(std::slice::from_ref(&repo.root)); + + assert!( + contains_canon(&roots, &repo.root.join("target")), + "must enumerate /target; got {roots:?}", + ); + assert!( + contains_canon(&roots, &wt.join("target")), + "must enumerate /worktrees/*/target; got {roots:?}", + ); +} + +// =========================================================================== +// 3. Producer: build_cache_leaf_dirs — existing leaves only, fail-closed +// =========================================================================== + +#[test] +fn build_cache_leaf_dirs_returns_only_existing_debug_leaves() { + let repo = RepoFixture::basic(); // has debug/{incremental,deps,build}, no llvm-cov + let leaves = build_cache_leaf_dirs(std::slice::from_ref(&repo.root)); + + for present in [ + "target/debug/incremental", + "target/debug/deps", + "target/debug/build", + ] { + assert!( + contains_canon(&leaves, &repo.leaf(present)), + "existing leaf {present} must be returned; got {leaves:?}", + ); + } + + // `target/debug` itself is NEVER a leaf. + assert!( + !contains_canon(&leaves, &repo.leaf("target/debug")), + "`target/debug` itself must never be returned as a leaf: {leaves:?}", + ); + // The final binary is NEVER a leaf (and is a file, not a dir). + assert!( + !contains_canon(&leaves, &repo.leaf("target/debug/simard")), + "`target/debug/simard` must never be returned as a leaf: {leaves:?}", + ); + // The coverage mirror does not exist here → silently omitted (nothing to + // reclaim), not an error. + assert!( + !contains_canon( + &leaves, + &repo.leaf("target/llvm-cov-target/debug/incremental") + ), + "non-existent coverage leaves must be omitted, not fabricated: {leaves:?}", + ); +} + +#[test] +fn build_cache_leaf_dirs_rejects_symlinked_leaves() { + let tmp = TempDir::new().expect("tempdir"); + let root = tmp.path().to_path_buf(); + // A real leaf plus a symlink *masquerading* as a leaf. + std::fs::create_dir_all(root.join("target/debug/incremental")).expect("real leaf"); + let elsewhere = tmp.path().join("evil-outside-target"); + std::fs::create_dir_all(&elsewhere).expect("symlink dest"); + symlink(&elsewhere, root.join("target/debug/deps")).expect("symlink leaf"); + + let leaves = build_cache_leaf_dirs(std::slice::from_ref(&root)); + + assert!( + contains_canon(&leaves, &root.join("target/debug/incremental")), + "the real leaf must survive: {leaves:?}", + ); + // The symlinked `deps` must be dropped — enumeration must use + // symlink_metadata and refuse symlink-swap into a foreign location. + let resolves_to_evil = leaves.iter().any(|p| { + p.canonicalize() + .map(|c| c == elsewhere.canonicalize().unwrap()) + .unwrap_or(false) + }); + assert!( + !resolves_to_evil, + "a symlinked leaf must never be returned (symlink-swap defense): {leaves:?}", + ); +} + +// =========================================================================== +// 4. Producer: build_cache_candidates — kind, est_bytes, determinism +// =========================================================================== + +#[test] +fn build_cache_candidates_are_stale_build_cache_with_no_estimate() { + let repo = RepoFixture::basic(); + let candidates = build_cache_candidates(std::slice::from_ref(&repo.root)); + + assert!( + !candidates.is_empty(), + "a repo with a built target/ must yield candidates", + ); + for c in &candidates { + assert_eq!( + c.kind, + CandidateKind::StaleBuildCache, + "every emitted candidate is a StaleBuildCache leaf: {:?}", + c.path, + ); + assert_eq!( + c.est_bytes, None, + "the producer must never carry a size estimate — the executor \ + re-measures ({:?})", + c.path, + ); + assert!( + c.reason.is_some(), + "each candidate carries a fixed, low-cardinality reason: {:?}", + c.path, + ); + assert!( + !c.path.ends_with("simard"), + "the final binary must never be a candidate: {:?}", + c.path, + ); + // No candidate is `target/debug` itself. + assert!( + !c.path + .canonicalize() + .map(|p| p == repo.leaf("target/debug").canonicalize().unwrap()) + .unwrap_or(false), + "`target/debug` itself must never be a candidate: {:?}", + c.path, + ); + } + + // Deterministic: no duplicate paths. + let mut paths: Vec<_> = candidates.iter().map(|c| c.path.clone()).collect(); + paths.sort(); + let before = paths.len(); + paths.dedup(); + assert_eq!( + before, + paths.len(), + "candidate paths must be unique (dedup)" + ); +} + +// =========================================================================== +// 5. Guard: leaf admitted; siblings / parent / worktree-root rejected. +// =========================================================================== + +#[test] +fn registered_leaf_is_allowed_remove_dir_with_fresh_size() { + let repo = RepoFixture::basic(); + let leaves = build_cache_leaf_dirs(std::slice::from_ref(&repo.root)); + // Production wiring: leaves are added to the allow-scope AND registered. + let allow = leaves.clone(); + let protected = ProtectedDenySet::from_paths(vec![]); + let live = FakeLive::none(); + // A leaf is not a git worktree → the tracked-worktree rail must be skipped; + // a hard-Reject probe here proves it was not consulted. + let wt = FixedWt(WorktreeVerdict::Reject(RejectReason::UnknownPrState)); + let measurer = MapMeasurer::default(); + let target = repo.leaf("target/debug/incremental"); + measurer.set(&target, 512_000_000); + + let c = cand(&target, CandidateKind::StaleBuildCache); + let v = vet(&c, &allow, &leaves, &protected, &live, &wt, &measurer); + + assert_eq!( + v, + Verdict::Allow { + primitive: ReclaimPrimitive::RemoveDir, + bytes: 512_000_000, + }, + "a registered build-cache leaf must be Allowed (RemoveDir) with the \ + freshly measured size, not the agent's est_bytes", + ); +} + +#[test] +fn registered_leaf_is_exempt_from_deny_set_but_only_by_exact_canonical_match() { + let repo = RepoFixture::basic(); + let leaves = build_cache_leaf_dirs(std::slice::from_ref(&repo.root)); + let allow = leaves.clone(); + // The managed repo root sits in the protected deny-set (the daemon runs + // from a managed checkout). Without the exemption the leaf would be + // ProtectedPath. + let protected = ProtectedDenySet::from_paths(vec![repo.root.clone()]); + let live = FakeLive::none(); + let wt = FixedWt(WorktreeVerdict::Reject(RejectReason::UnknownPrState)); + let measurer = MapMeasurer::default(); + let leaf = repo.leaf("target/debug/deps"); + measurer.set(&leaf, 400_000_000); + + // (a) The registered leaf IS exempt → Allowed. + let c_leaf = cand(&leaf, CandidateKind::StaleBuildCache); + assert_eq!( + vet(&c_leaf, &allow, &leaves, &protected, &live, &wt, &measurer), + Verdict::Allow { + primitive: ReclaimPrimitive::RemoveDir, + bytes: 400_000_000, + }, + "an exact-canonical registered leaf must be exempted from the deny-set", + ); + + // (b) The final binary sibling `target/debug/simard` is NOT registered → + // the deny-set still rejects it (exact-canonical exemption only). + let simard = repo.leaf("target/debug/simard"); + let c_bin = cand(&simard, CandidateKind::StaleBuildCache); + assert_eq!( + vet(&c_bin, &allow, &leaves, &protected, &live, &wt, &measurer), + Verdict::Reject { + reason: RejectReason::ProtectedPath, + }, + "a non-leaf sibling under a protected repo must stay ProtectedPath — \ + the carve-out is exact-canonical, never a prefix/substring", + ); +} + +#[test] +fn parent_target_debug_is_never_admitted() { + let repo = RepoFixture::basic(); + let leaves = build_cache_leaf_dirs(std::slice::from_ref(&repo.root)); + let allow = leaves.clone(); + let protected = ProtectedDenySet::from_paths(vec![]); + let live = FakeLive::none(); + let wt = FixedWt(WorktreeVerdict::Reject(RejectReason::UnknownPrState)); + let measurer = MapMeasurer::default(); + let parent = repo.leaf("target/debug"); + measurer.set(&parent, 718_000_000); + + // `target/debug` is a PARENT of the leaf allow-roots, not a descendant, so + // leaf-only containment structurally forbids it. It must never be Allowed + // (that is the wholesale nuke the fix eliminates). + let c = cand(&parent, CandidateKind::StaleBuildCache); + let v = vet(&c, &allow, &leaves, &protected, &live, &wt, &measurer); + assert!( + !matches!(v, Verdict::Allow { .. }), + "`target/debug` itself must never pass the guard; got {v:?}", + ); +} + +#[test] +fn unregistered_sibling_outside_leaves_is_outside_allow_root() { + let repo = RepoFixture::basic(); + // A regular sibling dir that is NOT a recognized cache leaf. + let sibling = repo.leaf("target/debug/some-other-dir"); + std::fs::create_dir_all(&sibling).expect("sibling dir"); + let leaves = build_cache_leaf_dirs(std::slice::from_ref(&repo.root)); + let allow = leaves.clone(); + let protected = ProtectedDenySet::from_paths(vec![]); // no deny-set here + let live = FakeLive::none(); + let wt = FixedWt(WorktreeVerdict::Reject(RejectReason::UnknownPrState)); + let measurer = MapMeasurer::default(); + + let c = cand(&sibling, CandidateKind::StaleBuildCache); + let v = vet(&c, &allow, &leaves, &protected, &live, &wt, &measurer); + assert_eq!( + v, + Verdict::Reject { + reason: RejectReason::OutsideAllowRoot, + }, + "a sibling that is not a registered leaf sits under no leaf allow-root \ + and must be OutsideAllowRoot; got {v:?}", + ); +} + +#[test] +fn live_process_interlock_still_vetoes_a_registered_leaf() { + let repo = RepoFixture::basic(); + let leaves = build_cache_leaf_dirs(std::slice::from_ref(&repo.root)); + let allow = leaves.clone(); + let protected = ProtectedDenySet::from_paths(vec![]); + let leaf = repo.leaf("target/debug/deps"); + // An in-flight `cargo build` holds a live process at/under the leaf. + let live = FakeLive::at(&leaf); + let wt = FixedWt(WorktreeVerdict::Reject(RejectReason::UnknownPrState)); + let measurer = MapMeasurer::default(); + measurer.set(&leaf, 400_000_000); + + let c = cand(&leaf, CandidateKind::StaleBuildCache); + let v = vet(&c, &allow, &leaves, &protected, &live, &wt, &measurer); + assert_eq!( + v, + Verdict::Reject { + reason: RejectReason::LiveProcess, + }, + "Rail 3 (live-process) must still veto an exempted leaf so an in-flight \ + build is never deleted from under itself; got {v:?}", + ); +} + +// =========================================================================== +// 6. Executor: producer candidates → non-zero freed, stop-at-target, DryRun 0. +// =========================================================================== + +/// Build the production-shaped guard context for the executor tests: allow +/// scope = leaves, registered leaves = leaves, empty deny-set, no live procs. +struct ExecHarness { + repo: RepoFixture, + leaves: Vec, + candidates: Vec, + measurer: MapMeasurer, + protected: ProtectedDenySet, + live: FakeLive, + wt: FixedWt, +} +impl ExecHarness { + fn new() -> Self { + let repo = RepoFixture::basic(); + let leaves = build_cache_leaf_dirs(std::slice::from_ref(&repo.root)); + let candidates = build_cache_candidates(std::slice::from_ref(&repo.root)); + let measurer = MapMeasurer::default(); + // Make `incremental` the largest so largest-first picks it first. + for c in &candidates { + let bytes = match c.path.file_name().and_then(|s| s.to_str()) { + Some("incremental") => 500, + Some("deps") => 300, + Some("build") => 100, + _ => 10, + }; + measurer.set(&c.path, bytes); + } + Self { + repo, + leaves, + candidates, + measurer, + protected: ProtectedDenySet::from_paths(vec![]), + live: FakeLive::none(), + wt: FixedWt(WorktreeVerdict::Reject(RejectReason::UnknownPrState)), + } + } + fn ctx(&self) -> GuardContext<'_> { + GuardContext { + allow_roots: &self.leaves, + build_cache_leaves: &self.leaves, + protected: &self.protected, + live_probe: &self.live, + wt_probe: &self.wt, + measurer: &self.measurer, + } + } +} + +#[test] +fn apply_frees_nonzero_and_stops_at_target_minimum_necessary() { + let h = ExecHarness::new(); + let ctx = h.ctx(); + let remover = RecordingRemover::default(); + // used_before=96; after evicting `incremental` disk drops to 84 (<= 85) → + // executor stops, having removed the minimum necessary (just incremental). + let disk = ScriptedDisk::new(vec![96, 96, 84]); + + let report = exec_reclaim( + h.candidates.clone(), + &ctx, + ReclaimMode::Apply, + 85, + &disk, + h.repo.root.as_path(), + &remover, + ); + + assert!( + report.bytes_freed > 0, + "routine reclaim in Apply mode must free NON-ZERO bytes (the whole \ + point of the fix); got {}", + report.bytes_freed, + ); + assert_eq!( + report.removed.len(), + 1, + "minimum-necessary: only the largest leaf should be evicted before \ + usage drops under target; removed={:?}", + report.removed, + ); + assert!( + report.removed[0].path.ends_with("incremental"), + "largest-first must evict `incremental` first; got {:?}", + report.removed[0].path, + ); + assert_eq!( + report.bytes_freed, 500, + "freed bytes must be the leaf's fresh size" + ); + // The final binary and target/debug itself are never touched. + for r in &report.removed { + assert!( + !r.path.ends_with("simard"), + "must never remove the binary: {:?}", + r.path + ); + assert!( + !r.path + .canonicalize() + .map(|p| p == h.repo.leaf("target/debug").canonicalize().unwrap()) + .unwrap_or(false), + "must never remove target/debug wholesale: {:?}", + r.path, + ); + } +} + +#[test] +fn dry_run_frees_zero_but_populates_would_remove() { + let h = ExecHarness::new(); + let ctx = h.ctx(); + let remover = RecordingRemover::default(); + let disk = ScriptedDisk::new(vec![96]); + + let report = exec_reclaim( + h.candidates.clone(), + &ctx, + ReclaimMode::DryRun, + 85, + &disk, + h.repo.root.as_path(), + &remover, + ); + + assert_eq!( + report.bytes_freed, 0, + "dry-run must free 0 bytes at the report level" + ); + assert!(report.removed.is_empty(), "dry-run must remove nothing"); + assert!( + remover.0.borrow().is_empty(), + "dry-run must never invoke the remover", + ); + // All three leaves are surfaced for human review, each with its own size. + assert_eq!( + report.would_remove.len(), + h.candidates.len(), + "dry-run must surface every allowed leaf in would_remove; got {:?}", + report.would_remove, + ); + assert!( + report + .would_remove + .iter() + .all(|r| r.kind == CandidateKind::StaleBuildCache && r.bytes > 0), + "each would_remove entry carries its own measured bytes", + ); +} + +// =========================================================================== +// 7. Determinism / env preservation (serial for env isolation). +// =========================================================================== + +#[test] +#[serial] +fn producer_is_deterministic_and_env_independent() { + let repo = RepoFixture::basic(); + + // The producer must not read the disk-reclaim env knobs; candidate + // selection is a pure function of the on-disk layout. + let saved_pct = std::env::var("SIMARD_DISK_RECLAIM_PCT").ok(); + let saved_apply = std::env::var("SIMARD_DISK_RECLAIM_DAEMON_APPLY").ok(); + // SAFETY: single-threaded test, `#[serial]` guards concurrent env mutation. + unsafe { + std::env::set_var("SIMARD_DISK_RECLAIM_PCT", "3"); + std::env::set_var("SIMARD_DISK_RECLAIM_DAEMON_APPLY", "1"); + } + + let a = build_cache_candidates(std::slice::from_ref(&repo.root)); + let b = build_cache_candidates(std::slice::from_ref(&repo.root)); + + let mut pa: Vec<_> = a.iter().map(|c| c.path.clone()).collect(); + let mut pb: Vec<_> = b.iter().map(|c| c.path.clone()).collect(); + pa.sort(); + pb.sort(); + assert_eq!(pa, pb, "producer output must be deterministic across runs"); + assert!( + !a.is_empty(), + "producer must still emit leaves regardless of env knob values", + ); + + // Restore env. + unsafe { + match saved_pct { + Some(v) => std::env::set_var("SIMARD_DISK_RECLAIM_PCT", v), + None => std::env::remove_var("SIMARD_DISK_RECLAIM_PCT"), + } + match saved_apply { + Some(v) => std::env::set_var("SIMARD_DISK_RECLAIM_DAEMON_APPLY", v), + None => std::env::remove_var("SIMARD_DISK_RECLAIM_DAEMON_APPLY"), + } + } +} From e4826f1ef30427bb8c7ab8cf288ac3c5b741ce4f Mon Sep 17 00:00:00 2001 From: rysweet Date: Mon, 27 Jul 2026 00:03:27 +0000 Subject: [PATCH 2/5] refactor(disk-reclaim): drop redundant per-leaf re-canonicalization in guard The registered build-cache leaves are already canonical by contract (build_cache_leaf_dirs canonicalizes every entry), so vet_candidate's exact-match check can compare the freshly-canonicalized candidate path directly against the leaf list instead of re-resolving each leaf on every call. The candidate-side symlink_metadata + canonicalize already provides the symlink-swap defense; the per-leaf re-canonicalization added redundant filesystem syscalls with no behavioral effect. Behavior-equivalent: all guard rails, the symlink-swap and sibling-rejection tests, and the 13-test integration suite stay green. No public API change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/disk_reclaim/guard.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/disk_reclaim/guard.rs b/src/disk_reclaim/guard.rs index ade02a629..2d71ae5bc 100644 --- a/src/disk_reclaim/guard.rs +++ b/src/disk_reclaim/guard.rs @@ -312,6 +312,11 @@ pub fn vet_candidate(candidate: &ReclaimCandidate, ctx: &GuardContext<'_>) -> Ve /// A symlink (symlink-swap defense), a non-directory, a canonicalize failure, or /// any prefix/substring relationship is **not** a match, so `target/debug`, a /// worktree root, or a sibling like `target/debug/simard` is never exempted. +/// +/// `leaves` are already canonical by contract (produced by +/// [`build_cache_leaf_dirs`](super::build_cache::build_cache_leaf_dirs), which +/// canonicalizes every entry), so membership is a direct comparison against the +/// freshly-canonicalized `path` — no need to re-resolve each leaf. fn is_registered_build_cache_leaf(path: &Path, leaves: &[PathBuf]) -> bool { if leaves.is_empty() { return false; @@ -323,15 +328,10 @@ fn is_registered_build_cache_leaf(path: &Path, leaves: &[PathBuf]) -> bool { if meta.file_type().is_symlink() || !meta.is_dir() { return false; } - let canon = match std::fs::canonicalize(path) { - Ok(c) => c, - Err(_) => return false, - }; - leaves.iter().any(|leaf| { - std::fs::canonicalize(leaf) - .map(|l| l == canon) - .unwrap_or(false) - }) + match std::fs::canonicalize(path) { + Ok(canon) => leaves.contains(&canon), + Err(_) => false, + } } #[cfg(test)] From 309648354bd8e7f4325759353ab2c976ef10428a Mon Sep 17 00:00:00 2001 From: rysweet Date: Mon, 27 Jul 2026 00:12:13 +0000 Subject: [PATCH 3/5] perf(disk-reclaim): compute build-cache leaves once per routine reclaim run_disk_reclaim walked the managed repos' build-cache leaves twice per routine tick: once via build_cache_candidates (for the merged candidate list) and again inside reclaim_candidates (for the guard allowlist). Each walk does read_dir over every repo's worktrees plus symlink_metadata + canonicalize per evictable leaf, so the duplication doubled the reclaim path's filesystem syscalls for no behavioral difference. Split reclaim_candidates into a public wrapper (computes the leaves, so CLI/daemon entry points still inherit the allowlist invariant) and a private reclaim_candidates_with_leaves that accepts them. run_disk_reclaim now walks the leaves once and reuses that single Vec for both the candidate list (build_cache_candidates_from_leaves) and the guard allowlist. Behavior-identical; leaf computation is a deterministic pure function of on-disk layout. Validation: disk_reclaim lib 78/78, integration 13/13, clippy + fmt clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/disk_reclaim/build_cache.rs | 11 +++++- src/disk_reclaim/mod.rs | 61 +++++++++++++++++++++++++++------ 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/src/disk_reclaim/build_cache.rs b/src/disk_reclaim/build_cache.rs index cdb5b4a1c..547553a18 100644 --- a/src/disk_reclaim/build_cache.rs +++ b/src/disk_reclaim/build_cache.rs @@ -105,7 +105,16 @@ pub fn build_cache_leaf_dirs(repos: &[PathBuf]) -> Vec { /// producer never trusts an estimate for a size decision). Deterministic and /// env-independent: candidate selection is a pure function of the on-disk layout. pub fn build_cache_candidates(repos: &[PathBuf]) -> Vec { - build_cache_leaf_dirs(repos) + build_cache_candidates_from_leaves(build_cache_leaf_dirs(repos)) +} + +/// The allocation-only half of [`build_cache_candidates`]: wrap already-vetted +/// leaf dirs as `StaleBuildCache` candidates. Split out so a caller that has +/// already paid for [`build_cache_leaf_dirs`] (production's `run_disk_reclaim`, +/// which also needs the same leaves as the guard allowlist) reuses that single +/// filesystem walk instead of re-`read_dir`/`canonicalize`-ing every leaf twice. +pub fn build_cache_candidates_from_leaves(leaves: Vec) -> Vec { + leaves .into_iter() .map(|path| ReclaimCandidate { path, diff --git a/src/disk_reclaim/mod.rs b/src/disk_reclaim/mod.rs index 5b0b93b44..580686751 100644 --- a/src/disk_reclaim/mod.rs +++ b/src/disk_reclaim/mod.rs @@ -32,7 +32,8 @@ pub mod recipe; pub mod build_cache; pub use build_cache::{ - EVICTABLE_CACHE_DIRS, build_cache_candidates, build_cache_leaf_dirs, target_debug_roots, + EVICTABLE_CACHE_DIRS, build_cache_candidates, build_cache_candidates_from_leaves, + build_cache_leaf_dirs, target_debug_roots, }; pub use candidate::{CandidateKind, MAX_CANDIDATES, ReclaimCandidate, parse_candidates}; pub use daemon_dir::resolve_daemon_working_dirs; @@ -180,6 +181,32 @@ pub fn reclaim_candidates( mode: ReclaimMode, target_pct: u8, source: ReclaimSource, +) -> ReclaimReport { + // Deterministic build-cache leaves for the managed repos (issue #4810). + // Every entry point (CLI, daemon) inherits the leaf allowlist by having it + // computed here rather than trusting the caller to supply it. + let build_cache_leaves = build_cache::build_cache_leaf_dirs(&managed_repos()); + reclaim_candidates_with_leaves( + candidates, + build_cache_leaves, + state_root, + mode, + target_pct, + source, + ) +} + +/// [`reclaim_candidates`] with the build-cache leaves supplied by the caller, +/// so the production path can compute that (filesystem-walking) set **once** and +/// reuse it for both the candidate list and the guard allowlist. Callers that do +/// not already have the leaves use [`reclaim_candidates`], which computes them. +fn reclaim_candidates_with_leaves( + candidates: Vec, + build_cache_leaves: Vec, + state_root: &Path, + mode: ReclaimMode, + target_pct: u8, + source: ReclaimSource, ) -> ReclaimReport { // Defense in depth: refuse to delete as root at the guarded core so every // entry point inherits the invariant — even a future caller that skips an @@ -196,13 +223,12 @@ pub fn reclaim_candidates( effective }; - // Deterministic build-cache leaves for the managed repos (issue #4810). - // Threaded into BOTH the Rail-2 allow-scope and the guard's registered-leaf - // allowlist so routine reclaim can evict `/target/debug/*` without - // widening any other rail. The widening is expressed leaf-by-leaf (never - // `target/`), so `starts_with` structurally forbids a wholesale-`target/debug` - // candidate from ever being contained. Empty when nothing is built yet. - let build_cache_leaves = build_cache::build_cache_leaf_dirs(&managed_repos()); + // The leaves are threaded into BOTH the Rail-2 allow-scope and the guard's + // registered-leaf allowlist so routine reclaim can evict + // `/target/debug/*` without widening any other rail. The widening is + // expressed leaf-by-leaf (never `target/`), so `starts_with` structurally + // forbids a wholesale-`target/debug` candidate from ever being contained. + // Empty when nothing is built yet. let mut allow = allow_roots(state_root); allow.extend(build_cache_leaves.iter().cloned()); @@ -272,10 +298,23 @@ pub fn run_disk_reclaim( // dependence on non-deterministic LLM output. There is NO fallback: a recipe // failure still surfaces above as `AdapterInvocationFailed`; the deterministic // candidates never mask a broken recipe. - let candidates = merge_dedup_by_path(candidates, build_cache_candidates(&managed_repos())); + // + // The leaf set is walked from disk **once** here and reused for both the + // candidate list and the guard allowlist (`reclaim_candidates_with_leaves`), + // so routine reclaim never re-`read_dir`/`canonicalize`s every leaf twice. + let build_cache_leaves = build_cache::build_cache_leaf_dirs(&managed_repos()); + let candidates = merge_dedup_by_path( + candidates, + build_cache::build_cache_candidates_from_leaves(build_cache_leaves.clone()), + ); - Ok(reclaim_candidates( - candidates, state_root, mode, target_pct, source, + Ok(reclaim_candidates_with_leaves( + candidates, + build_cache_leaves, + state_root, + mode, + target_pct, + source, )) } From dc52fbd5673a4f76d47bdf2539b83d695a7b2aa1 Mon Sep 17 00:00:00 2001 From: rysweet Date: Mon, 27 Jul 2026 00:35:24 +0000 Subject: [PATCH 4/5] harden(disk-reclaim): pre-unlink re-stat + producer equivalence test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the two actionable items from the Step 11 review synthesis on the issue #4810 build-cache producer. Item B (philosophy — lock the behavior-identical invariant): - Add `from_leaves_matches_full_walk_producer`: asserts `build_cache_candidates` (full filesystem walk) and `build_cache_candidates_from_leaves` (fed the same leaves the guard allowlist reuses on the production path) yield identical candidate sets across a multi-repo / multi-worktree layout. Converts the perf-split "one function calls the other" assumption into an enforced invariant on a file-deleting path — the candidate set can never diverge from the guard allowlist. Item A (security — TOCTOU pre-unlink re-stat, previously deferred): - The reference doc documented a `RemoveDir` type/owner re-stat as a committed safety property, but the executor only did canonicalize + under_any_root — a zero-BS doc/code discrepancy on a destructive path. Implement the re-stat (`restat_real_dir_owned`): immediately before `remove_dir_all`, the resolved canonical path must still be a real, non-symlink directory owned by the effective UID, else the candidate is aborted (one abort skips only that leaf, never fails the run). Closes the residual race where the canonical target is swapped to a symlink / re-owned in the window between vetting and unlink. - Tests: `restat_rejects_symlink_and_accepts_real_dir` (symlink + non-dir rejected, real euid-owned dir accepted) and `real_remover_deletes_real_dir_under_allow_root` (the new rail does not block legitimate deletion). Docs: add `build_cache_candidates_from_leaves` to the producer API reference, reflect the single-walk reuse in the orchestration flow, and correct the TOCTOU safety property to match the implemented re-stat. Validation: lib 81/81, integration 13/13, clippy (dev + release -D warnings) clean, fmt clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../disk-reclaim-build-cache-producer.md | 47 ++++++++--- src/disk_reclaim/build_cache.rs | 33 ++++++++ src/disk_reclaim/executor.rs | 79 +++++++++++++++++++ 3 files changed, 146 insertions(+), 13 deletions(-) diff --git a/docs/reference/disk-reclaim-build-cache-producer.md b/docs/reference/disk-reclaim-build-cache-producer.md index 8175bc92b..58bf41339 100644 --- a/docs/reference/disk-reclaim-build-cache-producer.md +++ b/docs/reference/disk-reclaim-build-cache-producer.md @@ -126,8 +126,19 @@ pub fn build_cache_leaf_dirs(repos: &[PathBuf]) -> Vec; /// The deterministic `StaleBuildCache` candidates for the given repos — one per /// existing leaf, `est_bytes = None`. Merged with the recipe's proposal by -/// `run_disk_reclaim` (dedup by path). +/// `run_disk_reclaim` (dedup by path). Equivalent to +/// `build_cache_candidates_from_leaves(build_cache_leaf_dirs(repos))`. pub fn build_cache_candidates(repos: &[PathBuf]) -> Vec; + +/// The allocation-only half of `build_cache_candidates`: wrap already-vetted +/// leaf dirs as `StaleBuildCache` candidates. Split out so `run_disk_reclaim` +/// — which already computes the leaves once for the guard allowlist — reuses +/// that single filesystem walk instead of re-`read_dir`/`canonicalize`-ing +/// every leaf twice per tick. Behavior-identical to the full-walk producer for +/// the same leaves (locked by the `from_leaves_matches_full_walk_producer` +/// equivalence test), so the candidate set can never diverge from the guard +/// allowlist on this file-deleting path. +pub fn build_cache_candidates_from_leaves(leaves: Vec) -> Vec; ``` The module is registered as `pub mod build_cache;` in @@ -195,12 +206,19 @@ proposal and de-duplicates by path: ``` run_disk_reclaim - ├─ run_reclaim_recipe() → Vec (agent proposal) - ├─ build_cache_candidates(repos) → Vec (deterministic) + ├─ run_reclaim_recipe() → Vec (agent proposal) + ├─ build_cache_leaf_dirs(repos) → Vec (single filesystem walk) + ├─ build_cache_candidates_from_leaves(leaves) → Vec (deterministic) ├─ merge + dedup by path (HashSet) - └─ reclaim_candidates(...) → every candidate re-vetted by the guard + └─ reclaim_candidates_with_leaves(.., leaves) → every candidate re-vetted by the guard ``` +The managed repos' build-cache leaves are walked **once** per tick and the same +`Vec` feeds both the deterministic candidate list +(`build_cache_candidates_from_leaves`) and the guard allowlist +(`GuardContext.build_cache_leaves`) — a single source of leaf computation on the +destructive path, so the candidate set and the allowlist cannot diverge. + The recipe contract is **unchanged** — the agent may still nominate build caches and everything else it inspects; the deterministic set simply guarantees the build-cache leaves are always present regardless of what the agent returns. This @@ -213,7 +231,10 @@ a broken recipe. `reclaim_candidates` (the shared CLI + daemon entry point) computes the leaves and threads them into both `allow_roots` and `GuardContext.build_cache_leaves`, so **CLI callers inherit the same safe behavior** as the daemon — no signature -change at the call sites. +change at the call sites. Its private inner `reclaim_candidates_with_leaves` +accepts precomputed leaves so `run_disk_reclaim` can supply the single walk it +already performed; the public wrapper still walks internally for callers that +have not. ## Steady-state behavior (the guarantee, and its condition) @@ -326,15 +347,15 @@ freed bytes` entries follow. - **Ownership check.** Leaves not owned by the effective UID are dropped — no cross-user or root deletion. The apply path is still refused entirely under `geteuid() == 0`. -- **TOCTOU re-assert.** At the syscall boundary `RealPathRemover` already - re-canonicalizes the path and re-asserts allow-root containment +- **TOCTOU re-assert.** At the syscall boundary `RealPathRemover` re-canonicalizes + the path and re-asserts allow-root containment (`under_any_root(canon, allow_roots)`) immediately before `remove_dir_all`, - refusing anything that no longer resolves under a live allow-root. For a - `StaleBuildCache` leaf this is tightened with a **type/owner re-stat** — the - path must still be a real directory (not a symlink swapped in after - enumeration) owned by the effective UID, or the candidate is aborted. Because - vetting is per-candidate, one abort skips only that leaf and never fails the - run. + refusing anything that no longer resolves under a live allow-root. The + `RemoveDir` primitive (which every `StaleBuildCache` leaf uses) is then further + tightened with a **type/owner re-stat** (`restat_real_dir_owned`) — the resolved + path must still be a real directory (not a symlink swapped in after enumeration) + owned by the effective UID, or the candidate is aborted. Because vetting is + per-candidate, one abort skips only that leaf and never fails the run. - **Live-process interlock.** Rail 3 vetoes any leaf with a live PID at/under it, so an in-flight `cargo build` writing `deps`/`build` is never deleted from under itself (worst case it regenerates next cycle — strictly safer than the diff --git a/src/disk_reclaim/build_cache.rs b/src/disk_reclaim/build_cache.rs index 547553a18..122e45968 100644 --- a/src/disk_reclaim/build_cache.rs +++ b/src/disk_reclaim/build_cache.rs @@ -213,6 +213,39 @@ mod tests { ); } + /// Locks the "behavior-identical" invariant between the two producer paths: + /// `build_cache_candidates` (walks the filesystem itself) and + /// `build_cache_candidates_from_leaves` (fed the same leaves the guard + /// allowlist reuses on the production path). The perf split that lets + /// `run_disk_reclaim` walk the leaves once must never let the candidate set + /// diverge from the guard allowlist — that divergence, on a file-deleting + /// path, is exactly the data-integrity hazard worth an explicit test. + #[test] + fn from_leaves_matches_full_walk_producer() { + let tmp = TempDir::new().unwrap(); + basic_repo(tmp.path()); + // A second managed repo plus a worktree, so the walk covers the + // multi-root / multi-worktree enumeration, not just a single target. + let other = tmp.path().join("other"); + basic_repo(&other); + let wt = tmp.path().join("worktrees/wt-1"); + basic_repo(&wt); + + let repos = vec![tmp.path().to_path_buf(), other]; + + let leaves = build_cache_leaf_dirs(&repos); + assert!(!leaves.is_empty(), "fixture must yield leaves"); + + let full_walk = build_cache_candidates(&repos); + let from_leaves = build_cache_candidates_from_leaves(leaves); + + assert_eq!( + full_walk, from_leaves, + "the two producer paths must yield identical candidate sets \ + (same paths, kind, reason, est_bytes, and ordering)", + ); + } + #[test] fn candidates_are_stale_build_cache_without_estimate() { let tmp = TempDir::new().unwrap(); diff --git a/src/disk_reclaim/executor.rs b/src/disk_reclaim/executor.rs index e819da1c1..427264644 100644 --- a/src/disk_reclaim/executor.rs +++ b/src/disk_reclaim/executor.rs @@ -6,6 +6,7 @@ //! removes the minimum necessary. The delete primitive lives behind the //! [`PathRemover`] seam so tests never touch real system paths. +use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; use std::process::Command; @@ -123,6 +124,13 @@ impl PathRemover for RealPathRemover { canon.display() )); } + // TOCTOU re-stat: the resolved target must still be a real, + // non-symlink directory owned by the effective UID immediately + // before deletion. This closes the residual race where the + // canonical path itself is swapped to a symlink (or its owner + // changes) in the window between vetting and `remove_dir_all` — + // one aborted candidate never fails the run. + restat_real_dir_owned(&canon)?; std::fs::remove_dir_all(&canon) .map_err(|e| format!("rm -rf {} failed: {e}", canon.display())) } @@ -130,6 +138,32 @@ impl PathRemover for RealPathRemover { } } +/// Fail-closed pre-unlink re-stat for a `RemoveDir` target: the (already +/// canonicalized, in-scope) `canon` must still be a real, non-symlink directory +/// owned by the effective UID. `symlink_metadata` does not follow the final +/// component, so a symlink swapped in after vetting is rejected rather than +/// followed. Returns an error string (aborting only this candidate) on any +/// mismatch — never panics. +fn restat_real_dir_owned(canon: &Path) -> Result<(), String> { + let meta = std::fs::symlink_metadata(canon) + .map_err(|e| format!("refusing rm -rf — cannot re-stat {}: {e}", canon.display()))?; + if meta.file_type().is_symlink() || !meta.is_dir() { + return Err(format!( + "refusing rm -rf — {} is no longer a real directory", + canon.display() + )); + } + // SAFETY: `geteuid` takes no arguments, reads no memory, and cannot fail. + let euid = unsafe { libc::geteuid() }; + if meta.uid() != euid { + return Err(format!( + "refusing rm -rf — {} is not owned by the effective UID", + canon.display() + )); + } + Ok(()) +} + /// Env-cleared git invocation (only `PATH`/`HOME` survive). Blocks `GIT_*` / /// `LD_PRELOAD` hijacking; argument vectors only, no shell. fn git_hardened(repo: &Path, args: &[&str]) -> Result<(), String> { @@ -580,6 +614,51 @@ mod tests { assert!(err.contains("leading-dash"), "got: {err}"); } + #[test] + fn restat_rejects_symlink_and_accepts_real_dir() { + use std::os::unix::fs::symlink; + let tmp = TempDir::new().unwrap(); + + // A real, euid-owned directory passes the re-stat. + let real = tmp.path().join("real"); + std::fs::create_dir(&real).unwrap(); + assert!(restat_real_dir_owned(&real).is_ok()); + + // A symlink (even to a real dir) is rejected: `symlink_metadata` does + // not follow the final component, so a swapped-in link cannot be deleted. + let link = tmp.path().join("link"); + symlink(&real, &link).unwrap(); + let err = restat_real_dir_owned(&link).expect_err("symlink must be rejected"); + assert!(err.contains("no longer a real directory"), "got: {err}"); + + // A regular file (non-directory) is likewise rejected. + let file = tmp.path().join("file"); + std::fs::write(&file, b"x").unwrap(); + assert!(restat_real_dir_owned(&file).is_err()); + } + + #[test] + fn real_remover_deletes_real_dir_under_allow_root() { + // The pre-unlink re-stat must not block a legitimate, euid-owned real + // directory under an allow-root. + let allow = TempDir::new().unwrap(); + let victim = allow.path().join("target/debug/incremental"); + std::fs::create_dir_all(&victim).unwrap(); + std::fs::write(victim.join("data"), b"cache").unwrap(); + + let remover = RealPathRemover { + parent_repo: allow.path().to_path_buf(), + allow_roots: vec![allow.path().to_path_buf()], + }; + remover + .remove(ReclaimPrimitive::RemoveDir, &victim) + .expect("a real euid-owned dir under an allow-root must be removed"); + assert!( + !victim.exists(), + "the real cache dir must have been removed" + ); + } + #[test] fn summary_is_a_stable_one_liner() { let report = ReclaimReport { From 9f368afffc289d0e087ca255906a13a4c522ff23 Mon Sep 17 00:00:00 2001 From: rysweet Date: Mon, 27 Jul 2026 02:42:58 +0000 Subject: [PATCH 5/5] refactor(disk-reclaim): extract shared fs predicate + document residual restat window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Step 16 review feedback on PR #4826 (all three reviews APPROVE, non-blocking suggestions): - Item B: centralize the triplicated "real / non-symlink / euid-owned dir" check into `disk_reclaim::fs_predicates`. `is_real_dir` (used by the guard's build-cache-leaf membership check) and `is_real_dir_owned_by_euid` (used by `vetted_leaf` and the executor's pre-unlink re-stat) are now one fail-closed definition, so the symlink-swap / ownership-swap defenses cannot drift apart. Guard semantics are preserved (it still checks real-dir only, not euid). - Item A: document why the residual restat -> remove_dir_all micro-window is safe — `std::fs::remove_dir_all` unlinks a final-component symlink without following it, and the euid-ownership check confines any swap to same-user paths, so the blast radius is nil (worst case: one skipped candidate). Verified: cargo build --lib, clippy --lib, fmt --check all clean; disk_reclaim lib tests 81 passed, integration producer suite 13 passed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/disk_reclaim/build_cache.rs | 9 ++----- src/disk_reclaim/executor.rs | 24 ++++++++++--------- src/disk_reclaim/fs_predicates.rs | 40 +++++++++++++++++++++++++++++++ src/disk_reclaim/guard.rs | 3 ++- src/disk_reclaim/mod.rs | 2 ++ 5 files changed, 59 insertions(+), 19 deletions(-) create mode 100644 src/disk_reclaim/fs_predicates.rs diff --git a/src/disk_reclaim/build_cache.rs b/src/disk_reclaim/build_cache.rs index 122e45968..bad9921b4 100644 --- a/src/disk_reclaim/build_cache.rs +++ b/src/disk_reclaim/build_cache.rs @@ -25,10 +25,10 @@ //! contract and the two narrow guard changes (leaf-only allow-scope + the //! exact-canonical deny-set exemption) that admit these candidates. -use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; use super::candidate::{CandidateKind, ReclaimCandidate}; +use super::fs_predicates::is_real_dir_owned_by_euid; /// The regenerable cargo cache leaf dirs, relative to a `target/` root. These are /// `target/debug/{incremental,deps,build}` and the coverage mirror under @@ -132,12 +132,7 @@ pub fn build_cache_candidates_from_leaves(leaves: Vec) -> Vec Option { let meta = std::fs::symlink_metadata(path).ok()?; - if meta.file_type().is_symlink() || !meta.is_dir() { - return None; - } - // SAFETY: `geteuid` takes no arguments, reads no memory, and cannot fail. - let euid = unsafe { libc::geteuid() }; - if meta.uid() != euid { + if !is_real_dir_owned_by_euid(&meta) { return None; } std::fs::canonicalize(path).ok() diff --git a/src/disk_reclaim/executor.rs b/src/disk_reclaim/executor.rs index 427264644..24a8cd1a6 100644 --- a/src/disk_reclaim/executor.rs +++ b/src/disk_reclaim/executor.rs @@ -6,7 +6,6 @@ //! removes the minimum necessary. The delete primitive lives behind the //! [`PathRemover`] seam so tests never touch real system paths. -use std::os::unix::fs::MetadataExt; use std::path::{Path, PathBuf}; use std::process::Command; @@ -17,6 +16,7 @@ use crate::worktree_gc::under_any_root; use super::ReclaimMode; use super::candidate::{CandidateKind, ReclaimCandidate}; +use super::fs_predicates::is_real_dir_owned_by_euid; use super::guard::{GuardContext, ReclaimPrimitive, RejectReason, Verdict, vet_candidate}; /// A path the executor removed (or would remove, in dry-run). @@ -130,6 +130,16 @@ impl PathRemover for RealPathRemover { // canonical path itself is swapped to a symlink (or its owner // changes) in the window between vetting and `remove_dir_all` — // one aborted candidate never fails the run. + // + // The sub-microsecond window that remains between this re-stat + // and the `remove_dir_all` below is deliberately tolerated: even + // if the path were swapped to a symlink in that gap, + // `std::fs::remove_dir_all` (Rust 1.95) opens the final + // component without following it, so it unlinks the symlink + // itself rather than recursing into its target — worst case the + // one call errors out and this single candidate is skipped. The + // euid-ownership check further confines any such swap to paths + // the current user already owns, so the blast radius is nil. restat_real_dir_owned(&canon)?; std::fs::remove_dir_all(&canon) .map_err(|e| format!("rm -rf {} failed: {e}", canon.display())) @@ -147,17 +157,9 @@ impl PathRemover for RealPathRemover { fn restat_real_dir_owned(canon: &Path) -> Result<(), String> { let meta = std::fs::symlink_metadata(canon) .map_err(|e| format!("refusing rm -rf — cannot re-stat {}: {e}", canon.display()))?; - if meta.file_type().is_symlink() || !meta.is_dir() { + if !is_real_dir_owned_by_euid(&meta) { return Err(format!( - "refusing rm -rf — {} is no longer a real directory", - canon.display() - )); - } - // SAFETY: `geteuid` takes no arguments, reads no memory, and cannot fail. - let euid = unsafe { libc::geteuid() }; - if meta.uid() != euid { - return Err(format!( - "refusing rm -rf — {} is not owned by the effective UID", + "refusing rm -rf — {} is no longer a real directory owned by the effective UID", canon.display() )); } diff --git a/src/disk_reclaim/fs_predicates.rs b/src/disk_reclaim/fs_predicates.rs new file mode 100644 index 000000000..d80358d41 --- /dev/null +++ b/src/disk_reclaim/fs_predicates.rs @@ -0,0 +1,40 @@ +//! Shared fail-closed filesystem predicates for disk reclamation (issue #4810). +//! +//! Every deletion path in this module must confirm — immediately before acting +//! — that its target is a **real, non-symlink directory owned by the effective +//! UID**. That check previously lived inline in three call sites +//! ([`build_cache::vetted_leaf`](super::build_cache), the guard's +//! [`is_registered_build_cache_leaf`](super::guard), and the executor's +//! pre-unlink re-stat); the predicate is centralized here so the symlink-swap +//! and ownership-swap defenses cannot drift apart. +//! +//! Each caller obtains its own metadata via [`std::fs::symlink_metadata`] (so +//! the final path component is **never** followed) and passes the result here +//! for the type/ownership verdict. + +use std::fs::Metadata; +use std::os::unix::fs::MetadataExt; + +/// `true` iff `meta` describes a real (non-symlink) directory. +/// +/// The caller must have obtained `meta` via [`std::fs::symlink_metadata`], so a +/// symlink is reported as a symlink and rejected rather than being resolved to +/// its (possibly foreign or protected) target. +pub(super) fn is_real_dir(meta: &Metadata) -> bool { + !meta.file_type().is_symlink() && meta.is_dir() +} + +/// `true` iff `meta` describes a real, non-symlink directory owned by the +/// effective UID. +/// +/// This is the shared pre-deletion predicate: [`is_real_dir`] plus a same-owner +/// check that confines every reclaim action to directories the current process +/// already owns, closing symlink-swap and ownership-swap TOCTOU windows. +pub(super) fn is_real_dir_owned_by_euid(meta: &Metadata) -> bool { + if !is_real_dir(meta) { + return false; + } + // SAFETY: `geteuid` takes no arguments, reads no memory, and cannot fail. + let euid = unsafe { libc::geteuid() }; + meta.uid() == euid +} diff --git a/src/disk_reclaim/guard.rs b/src/disk_reclaim/guard.rs index 2d71ae5bc..f28c6696a 100644 --- a/src/disk_reclaim/guard.rs +++ b/src/disk_reclaim/guard.rs @@ -27,6 +27,7 @@ use crate::worktree_gc::liveness::LiveProcessProbe; use super::candidate::{CandidateKind, ReclaimCandidate}; use super::daemon_dir::resolve_daemon_working_dirs; +use super::fs_predicates::is_real_dir; /// The concrete filesystem primitive the executor will run for an allowed /// candidate. @@ -325,7 +326,7 @@ fn is_registered_build_cache_leaf(path: &Path, leaves: &[PathBuf]) -> bool { Ok(m) => m, Err(_) => return false, }; - if meta.file_type().is_symlink() || !meta.is_dir() { + if !is_real_dir(&meta) { return false; } match std::fs::canonicalize(path) { diff --git a/src/disk_reclaim/mod.rs b/src/disk_reclaim/mod.rs index 580686751..fdfe72b04 100644 --- a/src/disk_reclaim/mod.rs +++ b/src/disk_reclaim/mod.rs @@ -13,6 +13,7 @@ //! - [`candidate`] — the `ReclaimCandidate` serde contract + marker parser. //! - [`guard`] — the non-bypassable rail: `vet_candidate` → `Verdict`. //! - [`daemon_dir`] — the protected daemon-directory union. +//! - `fs_predicates` — shared fail-closed real-dir / euid-ownership checks. //! - [`executor`] — the largest-first, threshold-stop, TOCTOU-reasserting disposer. //! - [`recipe`] — invoke the analysis recipe; strict parse; no fallback. @@ -25,6 +26,7 @@ use crate::error::SimardResult; pub mod candidate; pub mod daemon_dir; pub mod executor; +mod fs_predicates; pub mod guard; pub mod prod; pub mod recipe;