From 7adc340ca39c18a5ea31e7a1df054f97629876f9 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 09:38:24 +0200 Subject: [PATCH 01/24] docs: design opaque cache payload symlinks --- .../adr/0006-opaque-cache-payload-symlinks.md | 110 +++++ ...2026-08-29-cache-payload-symlink-design.md | 442 ++++++++++++++++++ 2 files changed, 552 insertions(+) create mode 100644 docs/adr/0006-opaque-cache-payload-symlinks.md create mode 100644 docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md diff --git a/docs/adr/0006-opaque-cache-payload-symlinks.md b/docs/adr/0006-opaque-cache-payload-symlinks.md new file mode 100644 index 0000000..59d6a01 --- /dev/null +++ b/docs/adr/0006-opaque-cache-payload-symlinks.md @@ -0,0 +1,110 @@ +# ADR 0006: Treat cache-payload symbolic links as opaque objects + +- Status: Proposed +- Date: 2026-08-29 +- Decision owner: Marco Porcellato +- Design: + `docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md` + +## Context + +CCP's managed cache has a strict link-free control layout, but the `data` +directory contains mutable output produced by containerized project tooling. +Ordinary virtual environments and package-manager caches use symbolic links. +The current producer can promote those payloads and then reject them while +preparing the next generation because its recursive clone and copy helpers +treat every payload link as a control-plane escape. + +That contradiction prevents safe persistent-cache reuse and can leave a new +staging directory without cleanup ownership when preparation fails before the +generation owner is constructed. + +## Decision + +CCP will keep every cache-root, entry, marker, manifest, lock, journal, +generation, payload-root, and ancestry object strictly free of symbolic links. +Only descendants of an already validated plain `data` directory may contain a +symbolic link. + +Within that payload plane, CCP treats each link as opaque: + +- inspect the link object with no-follow metadata; +- count it as one bounded non-directory object; +- preserve its stored target when copying on supported Unix platforms; +- never stat, open, canonicalize, traverse, or otherwise follow its target on + the host. + +Relative, absolute, broken, recursive, and outside-root target text therefore +does not grant host authority. A project process may later resolve a link only +inside the existing container mount namespace. Cache payloads remain mutable, +unattested performance state. + +Windows link-bearing payload reuse remains fail-closed until a separate native +design can preserve reparse semantics without guessing a broken target's file +or directory type. + +Prepared-generation cleanup ownership begins immediately after creation of the +owned staging root and before fallible clone or copy work. Cleanup remains +identity-bound, entry-locked, and limited to the exact staging directory. +Whole-generation removal must unlink internal payload links without traversing +their targets; external sentinel fixtures make this a tested security +invariant. + +No configuration, cache-key, receipt, policy, generation-manifest, promotion- +journal, or inventory JSON schema changes are made. + +## Consequences + +Benefits: + +- CCP can reuse normal Unix package-manager and environment caches; +- the host no-follow boundary is explicit and testable; +- control-plane link rejection remains unchanged; +- failed preparation no longer creates new unowned staging residue; +- cache and receipt schemas remain compatible. + +Costs: + +- traversal must use separate strict-control and opaque-payload policies; +- inventory's `files` count explicitly includes payload links; +- Unix fallback copy requires link-preserving logic and platform-specific + tests; +- Windows link-bearing cache reuse remains unsupported pending native + qualification; +- clone, copy, inventory, promotion, recovery, and cleanup tests must share the + same boundary contract. + +## Rejected alternatives + +- **Disable persistent reuse:** loses the intended performance and credit + benefit without fixing the lifecycle defect. +- **Delete or rotate affected caches:** mutates operator state and only hides + the next recurrence. +- **Follow targets that appear contained:** grants authority to untrusted + payload text and breaks broken or recursive links. +- **Permit links throughout an entry:** weakens ownership, lock, manifest, + journal, and promotion invariants. +- **Materialize targets as files or directories:** changes cache semantics and + may copy host data outside the payload. +- **Infer Windows link kind from its target:** cannot safely handle broken or + outside-root targets. + +## Verification gates + +1. Boundary tests reject links in every control-plane and payload-root + position. +2. Unix tests preserve relative, absolute, broken, recursive, and external- + target links without target traversal. +3. Inventory remains bounded, deterministic, and schema-compatible. +4. Clone and fallback copy produce equivalent link-bearing payloads. +5. Injected preparation failures remove only their exact owned staging path + while holding the entry lock, without changing external link targets. +6. Promotion and recovery never resolve payload targets. +7. Existing digest, receipt, policy, manifest, journal, and link-free cache + fixtures remain compatible. +8. Formatting, warnings-denied build, strict Clippy, all-target tests, + independent review, and one separately authorized two-generation candidate + qualification pass before any installed producer replacement. + +No cache mutation, installed producer replacement, adopter run, receipt +publication, push, PR, merge, or release is authorized by this proposed ADR. diff --git a/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md b/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md new file mode 100644 index 0000000..45c94eb --- /dev/null +++ b/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md @@ -0,0 +1,442 @@ +# Opaque cache-payload symbolic-link design + +Status: proposed for exact-document owner review; direction approved on +2026-08-29 +Date: 2026-08-29 +Baseline: `820a7fa6ce83a7ac8593c2800f8be4f44ab82ebc` +Scope: make completed cache generations reusable when ordinary package-manager +payloads contain symbolic links, without weakening CCP's managed-root boundary + +## Decision + +Commit CI Preflight will distinguish two filesystem trust domains inside an +owned cache root: + +1. the **control plane**, whose directories, manifests, markers, locks, + journals, entry names, and ancestry remain strictly free of symbolic links; +2. the **payload plane**, consisting only of the contents below an already + validated `entries//data` or `.staging-*/data` directory, where a + symbolic link is an opaque payload object that CCP may inventory and copy + but must never follow on the host. + +CCP will preserve a payload link's stored target text exactly. It will not +canonicalize, stat, open, traverse, validate the existence of, or derive host +authority from that target. Relative, absolute, broken, and outside-root +targets therefore have the same host-side treatment: preserve the link object +without dereferencing it. + +The implementation will also construct prepared-generation cleanup ownership +immediately after creating a staging directory and before any fallible clone or +copy. A failed reuse attempt must not leave an unowned `.staging-*` directory. + +No configuration, receipt, policy, cache-key, generation-manifest, or journal +schema changes are introduced. + +## Problem and evidence + +The cache currently accepts and promotes ordinary Python environment and +package-manager output that contains symbolic links. A later generation tries +to reuse that completed entry by cloning or copying its `data` tree. Both reuse +paths call traversal code that rejects every symbolic link as if it were a +managed control object. + +A controlled Matrix V2 execution demonstrated the contradiction: + +- one completed virtual-environment payload contained four normal environment + links; +- one completed package-manager payload contained 136 wheel, archive, and + editable-install links; +- the preceding generation had accepted and promoted both payloads; +- the following generation failed during cache preparation with + `symbolic link found inside managed cache root`; +- the failure happened before the new outer receipt could be written; +- the failed clone left an empty staging directory because cleanup ownership + was created only after the fallible reuse step. + +This is a producer lifecycle defect: CCP can create a complete cache state that +the same producer cannot consume. It is not evidence that the link targets were +followed, that the payload was malicious, or that a project check failed. + +The baseline and current upstream source retain the same contradiction: + +- `ManagedCache::prepare_entry` creates staging and then calls + `try_clone_tree` or `copy_tree` before constructing + `PreparedCacheGenerationOwner`; +- `try_clone_tree` first calls `bounded_tree_size`; +- `bounded_tree_size` rejects any link; +- `copy_tree` also rejects any link; +- promotion validates the payload root directory but does not recursively + reject links below it. + +## Goals + +- Reuse normal cache payloads containing relative, absolute, broken, or + outside-root symbolic-link targets without host dereference. +- Preserve the exact stored target bytes or platform-native target value when + copying a link. +- Keep the owner marker, fixed root directories, entry directory, lock, + completion marker, generation manifest, promotion journal, staging + directory, payload-root directory, and every ancestor strictly link-free. +- Keep traversal bounded, deterministic, sorted, overflow-checked, and + fail-closed for unexpected filesystem object types. +- Ensure any fallible preparation step is covered by the existing generation + cleanup owner. +- Preserve current serialized plans, cache keys, receipts, policies, manifests, + journals, exit-code classes, and public evidence fields. +- Prove the behavior with deterministic tests that need no Docker, network, + CCP heavy slot, host cache, or real package registry. + +## Non-goals + +- No recursive trust or validation of package-manager cache contents. +- No claim that a cache payload is reproducible, safe to execute, or part of + receipt evidence. +- No host-side resolution of a link to determine whether its target is inside + the payload. +- No automatic cleanup, eviction, quarantine, repair, migration, or adoption + of existing cache state. +- No weakening of cache-root, source-mount, guard-pin, ownership-marker, lock, + manifest, journal, or promotion validation. +- No new cache schema or compatibility flag. +- No Windows payload-link support until native creation semantics can preserve + an opaque target without guessing whether the link addresses a file or a + directory. +- No producer installation, adopter retry, receipt publication, push, PR, or + release as part of the implementation tranche. + +## Trust boundary + +### Control plane + +The control plane remains link-free. Existing strict checks continue to apply +to: + +- the selected cache-root path and every existing component; +- `.ccp-cache-root-v1.json`; +- `entries/` and `workspaces/`; +- `entries/sha256-/`; +- `.entry-lock-v1`, `.complete-v1`, and `.generation-v1.json`; +- `.promotion-lock-v1` and `.promotion-journal-v1.json`; +- `.staging-*` and `.backup-*` generation directories; +- the `data` directory at the root of a complete, staging, or backup + generation; +- any source path accepted by cache pinning or runtime mount validation. + +A link at any of these positions remains `SymlinkInManagedRoot`. The change +must not turn the whole cache entry into a link-permitting recursive tree. + +### Payload plane + +Only descendants of a separately validated plain `data` directory enter the +payload plane. For each child object, traversal uses `symlink_metadata` and +dispatches by the object itself: + +- regular file: count or copy the file; +- plain directory: descend in deterministic name order; +- symbolic link: count or recreate the link without following it; +- any other object type: fail with `UnexpectedEntry`. + +Traversal never calls `canonicalize`, `metadata`, `File::open`, `fs::copy`, or +directory enumeration on a link path. The link target is read only with the +platform link-reading primitive and used only as the target argument to the +platform link-creation primitive at the destination. + +An absolute, broken, recursive, or outside-root target is data. It neither +authorizes access to that host path nor changes the containment anchor. If a +later containerized project process follows the link, resolution happens in +the existing container mount namespace and remains subject to the current +runtime boundary. Cache payload bytes remain mutable, unattested performance +state. + +## Component design + +### Typed traversal policies + +Replace the one ambiguous recursive helper with explicitly named policies: + +- a strict managed-structure traversal that continues to reject every link; +- a payload traversal whose root must already be a validated plain `data` + directory and whose descendants may contain opaque links. + +The payload traversal is not a public API. Its caller must supply the validated +payload root rather than an arbitrary path, preventing accidental reuse for +the owner marker, entry root, workspace root, journal, or lock hierarchy. + +The existing `MAX_INVENTORY_NODES` applies equally to files, directories, and +links. Each visited link consumes one node and one `files` unit. Its accounted +byte size is the byte length of its stored target on Unix. All totals remain +checked for overflow. Inventory output remains deterministically sorted and +retains its current schema; the documented `files` field continues to mean +non-directory payload objects, not only regular files. + +Entry inventory retains its current accounting of control files and +directories, but switches to the payload policy only at these exact plain-root +shapes: + +- `entries//data`; +- `entries//.staging-*/data`; +- `entries//.backup-*/data`. + +Every other subtree remains under the strict traversal policy. The +implementation plan must enumerate the currently accepted entry-level objects +before changing inventory code so existing link-free totals and legacy +internal states are not silently reclassified. + +### Clone and fallback-copy behavior + +On macOS, `clonefile` remains an optimization. Before invoking it, CCP performs +the bounded payload traversal, which proves the source root is plain and every +descendant is a supported payload object without following links. A successful +clone must preserve link objects as links. Unsupported clone errors retain the +existing deterministic fallback. + +The Unix fallback copy performs these steps for a link: + +1. inspect it with `symlink_metadata`; +2. read the stored target with `read_link`; +3. ensure the destination path does not already exist as any object; +4. recreate it with `std::os::unix::fs::symlink`; +5. do not inspect or follow the target before or after creation. + +Regular files continue through `fs::copy`; directories are created as plain +directories and traversed in sorted order. A partial destination is owned by +the prepared-generation cleanup object and is removed through that existing +validated lifecycle when preparation fails. + +The clone path must not rely only on `clonefile`'s behavior. Tests must prove +the common traversal contract and the fallback-copy contract independently. + +### Windows behavior + +Windows remains fail-closed for payload links in this tranche. Creating a +Windows symbolic link through the standard library requires choosing file-link +or directory-link semantics; an opaque broken target does not provide a safe +portable way to infer that choice. The implementation must return a typed +unsupported payload-link error before copying or claiming reuse, while +preserving current strict control-plane errors. + +Windows caches with no symbolic links remain behaviorally unchanged. Native +Windows link preservation may be designed later using reviewed reparse-point +semantics and native tests; macOS or Linux evidence must not be relabeled as +Windows evidence. + +### Preparation ownership and cleanup + +`PreparedCacheGenerationOwner` must be created immediately after both the +plain staging directory and its plain `data` root exist, and before removing +that empty root or attempting clone/copy reuse. It owns: + +- the exact staging path; +- key digest, plan digest, and generation identity once available; +- the entry lock; +- cleanup eligibility for an unmanifested preparation phase and the existing + manifest-validated staging phase. + +The owner needs an internal phase that distinguishes: + +1. **preparing**: exact owned staging path exists but no manifest has yet been + committed; +2. **staging**: the exact matching staging manifest exists; +3. **promoted**: ownership no longer authorizes staging deletion. + +On final drop, cleanup may remove only the exact owned `.staging-*` directory +while the entry lock is still held. In `preparing`, it validates the parent +entry identity, staging name, and plain staging root before removal. In +`staging`, it retains the current schema/key/plan/generation/state validation. +After promotion, it performs no deletion. It never scans for or removes other +staging directories. + +Whole-generation removal must itself retain the no-follow boundary. On +supported Unix platforms, the selected recursive-removal primitive must be +documented and tested to unlink a payload link object rather than traversing +its target. A fixture places a sentinel outside the staging tree, links to it +from several payload depths, removes the owned staging generation, and proves +the sentinel and its descendants are byte-identical. A link replacing the +staging root is still rejected. Mutation by a non-cooperative actor during +removal remains outside the guarantee and is not mitigated by following links. + +This ordering closes the observed leak without inventing a recovery action or +deleting historical residue. Existing unowned residue remains operator state. + +### Promotion and recovery + +Promotion continues to rename whole plain `data` directories atomically within +an entry. It does not walk or rewrite their payload descendants. Before a +staging generation can enter a promotion journal, CCP validates: + +- the strict control-plane paths and matching manifest; +- the payload root as a plain directory; +- a bounded payload traversal that accepts only regular files, plain + directories, and opaque links. + +Recovery continues to reason about exact generation directories, marker bytes, +manifest bytes, and journal identity. It may rename or remove an owned whole +generation directory but must never resolve or act on a link target within its +payload. The generic removal helper remains strict for control-plane callers; +payload-aware deletion is not introduced by this tranche. + +### Errors and evidence + +Control-plane links retain the existing local message and exit classification. +Payload traversal adds typed distinctions for: + +- unsupported payload-link preservation on the current platform; +- failure to read or recreate an opaque payload link; +- unsupported payload filesystem object. + +The final exact names may be refined during TDD, but callers must not map a +preparation error to a project-check result. A preparation failure produces no +new receipt and no cache promotion. Human-readable local stderr may identify a +path under the local cache; receipts, journals, telemetry, and public evidence +must not gain absolute paths, target values, usernames, or payload contents. + +## Deterministic TDD strategy + +### Boundary classification + +- reject a link at the cache root, owner marker, `entries`, entry directory, + lock, marker, manifest, staging root, or payload-root position; +- accept links only one or more levels below a validated payload root; +- reject FIFO, socket, device, or other unsupported objects inside a payload; +- prove the node bound and size-overflow behavior remains fail-closed. + +### Opaque traversal + +On Unix, fixtures cover: + +- a relative link to an existing sibling; +- a relative link to a missing sibling; +- an absolute link; +- a link whose target names a path outside the cache root; +- a self-referential and a mutually recursive link; +- nested links beneath sorted directories. + +For every case, a test hook records filesystem operations and proves no target +stat, open, canonicalization, copy, or descent occurs. Inventory counts each +link once and does not count or size its target. + +### Clone and copy + +- fallback copy recreates the exact link target and leaves the target + unresolved; +- ordinary files and directories preserve current behavior; +- a copied payload containing a link can be prepared and promoted; +- a second generation reuses that complete payload successfully; +- macOS clone preflight accepts the same fixture and the cloned destination + retains link identity; +- forced clone fallback produces an equivalent payload tree; +- Windows-specific tests prove payload links fail before partial reuse while + link-free payloads remain supported. + +### Cleanup ownership + +- inject failure before clone, during clone preflight, during fallback copy, + and before manifest write; +- prove the exact owned staging directory is removed on final drop; +- prove the entry lock is held through cleanup and then released; +- prove unrelated and identity-mismatched staging directories are preserved; +- prove cleanup unlinks internal payload links without changing an external + target sentinel; +- preserve clone-sharing, successful promotion, failed promotion, and + promotion-recovery regression tests. + +### Compatibility + +- existing inventory JSON fixtures remain schema-compatible; +- link-free cache inventory totals remain unchanged; +- normalized configuration digests, cache keys, dry-run JSON, receipts, + manifests, and promotion journals remain byte-compatible; +- no test requires Docker, network, a real package manager, the admission + root, a host cache, or a CCP heavy command. + +## Documentation and change surface + +The implementation tranche will update together: + +- `docs/CACHE_AND_WORKSPACE.md` with the control-plane/payload-plane boundary, + inventory accounting, and platform status; +- `docs/THREAT_MODEL.md` with the no-follow invariant and container-resolution + boundary; +- `docs/TESTING_AND_FAULT_INJECTION.md` with deterministic link and cleanup + fault coverage; +- `CHANGELOG.md` with the user-visible cache-reuse correction; +- ADR 0006 from Proposed to Accepted after exact-document owner review. + +Source changes are expected to remain concentrated in `src/cache.rs` and its +focused tests. If implementation requires a public CLI/config/schema change, +new dependency, broader filesystem authority, or edits outside the documented +surface, stop and return to design review. + +## Delivery sequence + +1. Commit this design and proposed ADR for exact owner review. +2. After approval, write a line-bound implementation plan using red-green TDD. +3. Implement typed payload traversal and accounting with focused tests. +4. Implement Unix link-preserving fallback copy and macOS clone qualification. +5. Move preparation cleanup ownership before fallible reuse and cover injected + failures. +6. Reconcile promotion/recovery validation and update public documentation. +7. Run formatting, warnings-denied checks, strict Clippy, all-target tests, and + an independent code/security review. +8. In a separately authorized isolated candidate installation, qualify two + cache generations: generation N creates and promotes a link-bearing cache; + generation N+1 reuses it successfully. +9. Only after qualification may the stable producer contract be reviewed for + replacement. Any adopter retry, evidence publication, push, PR, merge, or + release remains a separate gate. + +## Rejected alternatives + +### Disable or rotate persistent caches + +Rejected. It avoids reuse, discards the intended credit-saving benefit, and +does not correct a producer that accepts state it later rejects. + +### Delete the link-bearing cache and retry + +Rejected. It mutates operator state, hides the lifecycle defect, and provides +no durable protection for the next package-manager generation. + +### Follow only links that appear to remain inside the payload + +Rejected. Canonicalizing a target introduces host traversal, breaks broken and +recursive links, creates TOCTOU ambiguity, and grants meaning to untrusted +payload text. + +### Permit links everywhere below an entry directory + +Rejected. That would weaken ownership, marker, manifest, journal, lock, and +promotion invariants and make the trusted control layout ambiguous. + +### Rewrite links as regular files or resolved directories + +Rejected. It changes package-manager semantics, can copy data outside the +payload, and makes cache reuse nondeterministic. + +### Infer Windows link type from the target + +Rejected. Broken or outside-root targets make inference unreliable and would +require following untrusted host paths. Native Windows support needs a separate +reviewed design. + +## Acceptance criteria + +1. A Unix cache payload containing relative, absolute, broken, external-target, + and recursive links can be inventoried, prepared, copied, promoted, and + reused without CCP following any target on the host. +2. A link at any control-plane or payload-root position still fails closed. +3. Payload inventory remains bounded and deterministic; every link consumes + one node and one file count, and its byte count is its stored target length. +4. Failed reuse leaves no newly created unowned staging directory and releases + the entry lock only after validated cleanup. +5. Promotion and recovery operate on whole owned generation directories and do + not resolve payload link targets; staging cleanup leaves every external + target sentinel untouched. +6. Windows remains explicitly fail-closed for link-bearing payload reuse; no + cross-platform claim exceeds native evidence. +7. Existing configuration digests, cache keys, receipts, policies, manifests, + journals, and link-free cache behavior remain compatible. +8. Focused and full non-heavy Rust gates pass at the exact implementation HEAD. +9. A separately authorized two-generation candidate qualification proves both + link-bearing cache creation and subsequent reuse before any installed + producer replacement. From b99d2ffd278d39d4905c36b01778149715fcd50f Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 09:50:50 +0200 Subject: [PATCH 02/24] docs: plan opaque cache payload symlink implementation --- .../adr/0006-opaque-cache-payload-symlinks.md | 4 +- ...29-cache-payload-symlink-implementation.md | 1266 +++++++++++++++++ ...2026-08-29-cache-payload-symlink-design.md | 3 +- 3 files changed, 1269 insertions(+), 4 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md diff --git a/docs/adr/0006-opaque-cache-payload-symlinks.md b/docs/adr/0006-opaque-cache-payload-symlinks.md index 59d6a01..7df0d23 100644 --- a/docs/adr/0006-opaque-cache-payload-symlinks.md +++ b/docs/adr/0006-opaque-cache-payload-symlinks.md @@ -1,6 +1,6 @@ # ADR 0006: Treat cache-payload symbolic links as opaque objects -- Status: Proposed +- Status: Accepted - Date: 2026-08-29 - Decision owner: Marco Porcellato - Design: @@ -107,4 +107,4 @@ Costs: qualification pass before any installed producer replacement. No cache mutation, installed producer replacement, adopter run, receipt -publication, push, PR, merge, or release is authorized by this proposed ADR. +publication, push, PR, merge, or release is authorized by this ADR. diff --git a/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md b/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md new file mode 100644 index 0000000..306a705 --- /dev/null +++ b/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md @@ -0,0 +1,1266 @@ +# Opaque Cache-Payload Symbolic Links Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make completed Unix cache generations containing ordinary symbolic +links safely reusable while preserving CCP's strict link-free control plane and +cleaning every newly created failed staging generation. + +**Architecture:** Add one private payload-tree module that owns bounded +no-follow traversal, accounting, and Unix link-preserving copy. Keep cache +layout, promotion, recovery, and ownership decisions in `cache.rs`, and call +the payload module only after validating an exact plain `data` root. Move the +prepared-generation RAII owner before fallible reuse so cleanup covers both +pre-manifest and manifest-backed staging phases. + +**Tech Stack:** Rust 2024, Rust 1.87+, standard-library filesystem APIs, +existing `fs2`, `serde`, and `sha2` dependencies; deterministic unit and +contract tests with no new dependency. + +**Spec:** +`docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md` + +## Global Constraints + +- The implementation baseline is + `820a7fa6ce83a7ac8593c2800f8be4f44ab82ebc` plus approved design commit + `7adc340ca39c18a5ea31e7a1df054f97629876f9`. +- Symbolic links remain forbidden in the cache root, fixed root directories, + entry root, locks, markers, manifests, journals, generation roots, payload + roots, cache-pin sources, and mount sources. +- Only descendants of exact plain `entries//data`, + `entries//.staging-*/data`, or + `entries//.backup-*/data` roots may be opaque links. +- Host code must never stat, open, canonicalize, traverse, copy, or otherwise + follow a payload link target. +- Relative, absolute, broken, recursive, and outside-root targets are preserved + as target text on Unix. +- Windows link-bearing payload reuse remains fail-closed; link-free Windows + behavior remains unchanged. +- No configuration, receipt, policy, cache-key, inventory JSON, + generation-manifest, or promotion-journal schema changes. +- No new dependency, network operation, Docker invocation, admission mutation, + host-cache mutation, or CCP heavy command in deterministic development. +- Each task follows red-green-refactor and ends in one reviewable commit. +- Installed producer replacement, adopter execution, receipt publication, + push, PR, merge, and release remain separate authorization gates. + +## File Structure + +- Create `src/cache_payload.rs`: private bounded payload traversal, accounting, + trace seam, Unix link recreation, and focused unit tests. +- Modify `src/lib.rs`: declare the private `cache_payload` module. +- Modify `src/cache.rs`: map payload errors, classify exact payload roots, + integrate inventory and reuse, move staging ownership earlier, and validate + payloads before promotion. +- Modify `tests/repository_hygiene_contract.rs`: pin the public documentation + boundary and its exact deterministic test references. +- Modify `docs/CACHE_AND_WORKSPACE.md`: document control-plane versus payload- + plane behavior and inventory accounting. +- Modify `docs/THREAT_MODEL.md`: add the host no-follow threat and residual + container/runtime boundary. +- Modify `docs/TESTING_AND_FAULT_INJECTION.md`: record focused deterministic + link, cleanup, and platform tests. +- Modify `CHANGELOG.md`: describe the user-visible cache-reuse correction. +- Modify `docs/adr/0006-opaque-cache-payload-symlinks.md`: already accepted; + change it only if implementation discovers a decision-level contradiction. + +--- + +### Task 1: Private bounded payload traversal + +**Files:** + +- Create: `src/cache_payload.rs` +- Modify: `src/lib.rs:18-31` +- Modify: `src/cache.rs:1697-1785` +- Test: `src/cache_payload.rs` inline unit-test module + +**Interfaces:** + +- Consumes: `crate::cache::CacheError`, `std::path::{Path, PathBuf}`, and a + caller-supplied `&mut usize` node counter plus node limit. +- Produces: + `PayloadTreeStats { pub(crate) bytes: u64, pub(crate) files: u64 }`, + `measure_payload_tree(&Path, &mut usize, usize)`, + `validate_payload_tree(&Path, &mut usize, usize)`, and the private test trace + seam `take_payload_operations()`. + +- [ ] **Step 1: Declare the module and the typed cache errors** + +Add the private module to `src/lib.rs`: + +```rust +pub mod cache; +mod cache_payload; +pub mod config; +``` + +Add these variants to `CacheError` in `src/cache.rs`: + +```rust +PayloadSymlinkUnsupported(PathBuf), +PayloadSymlinkRead { + path: PathBuf, + source: io::Error, +}, +PayloadSymlinkCreate { + path: PathBuf, + source: io::Error, +}, +``` + +Map all three to exit code `70`. Keep paths out of `Display`: use +`"cache payload symbolic links are unsupported on this platform"`, +`"cache payload symbolic-link target could not be read"`, and +`"cache payload symbolic link could not be created"`. Return the nested +`io::Error` from `std::error::Error::source` for the read/create variants. + +- [ ] **Step 2: Write the failing Unix traversal tests** + +Create `src/cache_payload.rs` with the imports, type signature, and tests first. +The main fixture must use all five link classes without reading their targets: + +```rust +#[cfg(test)] +static TEST_SEQUENCE: AtomicU64 = AtomicU64::new(0); + +#[cfg(test)] +fn payload_fixture(name: &str) -> PathBuf { + let path = std::env::current_dir() + .unwrap() + .parent() + .unwrap() + .join(format!( + ".ccp-payload-test-{}-{}-{name}", + std::process::id(), + TEST_SEQUENCE.fetch_add(1, Ordering::Relaxed) + )); + let _ = fs::remove_dir_all(&path); + fs::create_dir(&path).unwrap(); + path +} + +#[cfg(test)] +fn remove_fixture(root: &Path, outside: &Path) { + if root.exists() { + fs::remove_dir_all(root).unwrap(); + } + if outside.exists() { + fs::remove_file(outside).unwrap(); + } +} + +#[cfg(unix)] +#[test] +fn payload_measurement_counts_links_without_following_targets() { + use std::os::unix::{ffi::OsStrExt, fs::symlink}; + + let fixture = payload_fixture("measure-links"); + let outside = fixture.parent().unwrap().join("outside-sentinel"); + fs::write(&outside, b"do not read or change").unwrap(); + fs::write(fixture.join("regular"), b"abc").unwrap(); + symlink("regular", fixture.join("relative")).unwrap(); + symlink("missing", fixture.join("broken")).unwrap(); + symlink(&outside, fixture.join("absolute-external")).unwrap(); + symlink("self", fixture.join("self")).unwrap(); + symlink("cycle-b", fixture.join("cycle-a")).unwrap(); + symlink("cycle-a", fixture.join("cycle-b")).unwrap(); + fs::create_dir(fixture.join("nested")).unwrap(); + symlink("../relative", fixture.join("nested/recursive")).unwrap(); + + clear_payload_operations(); + let mut nodes = 0; + let stats = measure_payload_tree(&fixture, &mut nodes, 100).unwrap(); + let target_bytes = [ + "regular", + "missing", + "self", + "cycle-b", + "cycle-a", + "../relative", + ] + .into_iter() + .map(|target| target.as_bytes().len() as u64) + .sum::() + + outside.as_os_str().as_bytes().len() as u64; + + assert_eq!(stats.files, 8); + assert_eq!(stats.bytes, 3 + target_bytes); + assert_eq!(fs::read(&outside).unwrap(), b"do not read or change"); + assert!(take_payload_operations().iter().all(|operation| { + !operation.filesystem_path().starts_with(&outside) + })); + remove_fixture(&fixture, &outside); +} +``` + +Add three more tests with exact names: + +```rust +#[cfg(unix)] +#[test] +fn payload_root_itself_must_be_a_plain_directory() { + use std::os::unix::fs::symlink; + + let real = payload_fixture("plain-root-real"); + let link = real.parent().unwrap().join("plain-root-link"); + let _ = fs::remove_file(&link); + symlink(&real, &link).unwrap(); + let mut nodes = 0; + assert!(matches!( + measure_payload_tree(&link, &mut nodes, 100), + Err(CacheError::SymlinkInManagedRoot(path)) if path == link + )); + fs::remove_file(link).unwrap(); + fs::remove_dir(real).unwrap(); +} + +#[cfg(unix)] +#[test] +fn unsupported_payload_object_fails_without_traversal() { + use std::os::unix::net::UnixListener; + + let fixture = payload_fixture("unsupported-object"); + let socket = fixture.join("listener.socket"); + let listener = UnixListener::bind(&socket).unwrap(); + let mut nodes = 0; + assert!(matches!( + measure_payload_tree(&fixture, &mut nodes, 100), + Err(CacheError::UnexpectedEntry(path)) if path == socket + )); + drop(listener); + fs::remove_file(socket).unwrap(); + fs::remove_dir(fixture).unwrap(); +} + +#[test] +fn payload_node_limit_is_fail_closed() { + let fixture = payload_fixture("node-limit"); + fs::write(fixture.join("a"), b"a").unwrap(); + fs::write(fixture.join("b"), b"b").unwrap(); + let mut nodes = 0; + assert!(matches!( + measure_payload_tree(&fixture, &mut nodes, 2), + Err(CacheError::InventoryLimitExceeded) + )); + fs::remove_dir_all(fixture).unwrap(); +} +``` + +- [ ] **Step 3: Run the tests to verify the red state** + +Run: + +```console +cargo test --locked cache_payload::tests:: -- --nocapture +``` + +Expected: compilation fails because `PayloadTreeStats`, +`measure_payload_tree`, and the trace helpers are not defined. + +- [ ] **Step 4: Implement the bounded no-follow walker** + +Use this exact public-to-crate surface: + +```rust +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct PayloadTreeStats { + pub(crate) bytes: u64, + pub(crate) files: u64, +} + +pub(crate) fn measure_payload_tree( + root: &Path, + nodes: &mut usize, + node_limit: usize, +) -> Result { + let metadata = traced_symlink_metadata(root)?; + if metadata.file_type().is_symlink() { + return Err(CacheError::SymlinkInManagedRoot(root.to_path_buf())); + } + if !metadata.is_dir() { + return Err(CacheError::UnexpectedEntry(root.to_path_buf())); + } + walk_payload(root, nodes, node_limit) +} + +pub(crate) fn validate_payload_tree( + root: &Path, + nodes: &mut usize, + node_limit: usize, +) -> Result<(), CacheError> { + measure_payload_tree(root, nodes, node_limit).map(|_| ()) +} +``` + +`walk_payload` must increment and check the node counter before inspecting each +object, use `symlink_metadata`, sort `read_dir` entries by `file_name`, and use +checked addition for bytes/files. Its link arm on Unix is: + +```rust +if metadata.file_type().is_symlink() { + let target = traced_read_link(path).map_err(|source| { + CacheError::PayloadSymlinkRead { + path: path.to_path_buf(), + source, + } + })?; + use std::os::unix::ffi::OsStrExt; + return Ok(PayloadTreeStats { + bytes: target.as_os_str().as_bytes().len() as u64, + files: 1, + }); +} +``` + +Implement the test-only operation recorder used by the tests above: + +```rust +#[cfg(test)] +#[derive(Debug, Clone, PartialEq, Eq)] +enum PayloadOperation { + SymlinkMetadata(PathBuf), + ReadDirectory(PathBuf), + ReadLink(PathBuf), +} + +#[cfg(test)] +impl PayloadOperation { + fn filesystem_path(&self) -> &Path { + match self { + Self::SymlinkMetadata(path) + | Self::ReadDirectory(path) + | Self::ReadLink(path) => path, + } + } +} + +#[cfg(test)] +thread_local! { + static PAYLOAD_OPERATIONS: RefCell> = const { + RefCell::new(Vec::new()) + }; +} + +#[cfg(test)] +fn record_payload_operation(operation: PayloadOperation) { + PAYLOAD_OPERATIONS.with(|operations| operations.borrow_mut().push(operation)); +} + +#[cfg(test)] +fn clear_payload_operations() { + PAYLOAD_OPERATIONS.with(|operations| operations.borrow_mut().clear()); +} + +#[cfg(test)] +fn take_payload_operations() -> Vec { + PAYLOAD_OPERATIONS.with(|operations| std::mem::take(&mut *operations.borrow_mut())) +} +``` + +`traced_symlink_metadata`, `traced_read_directory`, and `traced_read_link` +record the matching operation immediately before calling the standard-library +function. The `record_payload_operation` call is enclosed in `#[cfg(test)]`, so +production traversal allocates no trace path. + +The corresponding `#[cfg(not(unix))]` arm returns +`CacheError::PayloadSymlinkUnsupported(path.to_path_buf())` without reading the +target. Wrap every filesystem operation used by the walker in a local helper +that appends a `PayloadOperation` under `#[cfg(test)]`; production builds make +the recorder an empty inline operation. No operation helper accepts a link +target as its filesystem path. + +- [ ] **Step 5: Run the focused tests and Clippy** + +Run: + +```console +cargo test --locked cache_payload::tests:: -- --nocapture +cargo clippy --locked --lib --tests -- -D warnings +``` + +Expected: all four payload traversal tests pass; Clippy reports no warnings. + +- [ ] **Step 6: Commit Task 1** + +```console +git add src/lib.rs src/cache.rs src/cache_payload.rs +git commit -m "feat: add bounded opaque cache payload traversal" +``` + +### Task 2: Position-aware inventory accounting + +**Files:** + +- Modify: `src/cache.rs:708-746` +- Modify: `src/cache.rs:1598-1640` +- Test: `src/cache.rs:2070-2130` + +**Interfaces:** + +- Consumes: Task 1's `measure_payload_tree` and `PayloadTreeStats`. +- Produces: private `bounded_entry_size(&Path, &mut usize)` and + `is_payload_root(&Path, &Path)`, while retaining the public + `CacheInventory` JSON shape. + +- [ ] **Step 1: Write the failing inventory tests** + +Replace the broad intent of `inventory_never_follows_symlinks` with two exact +boundaries while keeping its existing entry-root rejection assertion: + +```rust +#[cfg(unix)] +#[test] +fn inventory_counts_payload_links_without_following_targets() { + use std::os::unix::{ffi::OsStrExt, fs::symlink}; + + let fixture = completed_entry_fixture("inventory-payload-links"); + let before = fixture.cache.inventory().unwrap().entries.remove(0); + let outside = fixture.repo.join("inventory-sentinel"); + fs::write(&outside, b"sentinel").unwrap(); + symlink(&outside, fixture.data_path.join("external-link")).unwrap(); + + let after = fixture.cache.inventory().unwrap().entries.remove(0); + assert_eq!(after.files, before.files + 1); + assert_eq!( + after.bytes, + before.bytes + outside.as_os_str().as_bytes().len() as u64 + ); + assert_eq!(fs::read(&outside).unwrap(), b"sentinel"); + finish_fixture(fixture); +} + +#[cfg(unix)] +#[test] +fn inventory_rejects_a_symlink_at_the_payload_root() { + use std::os::unix::fs::symlink; + + let fixture = completed_entry_fixture("inventory-payload-root-link"); + let real = fixture.entry_path.join("real-data"); + fs::rename(&fixture.data_path, &real).unwrap(); + symlink(&real, &fixture.data_path).unwrap(); + assert!(matches!( + fixture.cache.inventory(), + Err(CacheError::SymlinkInManagedRoot(_)) + )); + finish_fixture(fixture); +} + +#[cfg(unix)] +#[test] +fn inventory_switches_mode_only_at_exact_generation_data_roots() { + use std::os::unix::fs::symlink; + + let (repo, resolved) = resolved_fixture("inventory-generation-payloads"); + let cache = ManagedCache::initialize(resolved.clone()).unwrap(); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); + let entry = cache.entry_path(&key); + let staging = entry.join(".staging-1-1"); + let backup = entry.join(".backup-1-1"); + fs::create_dir_all(staging.join("data")).unwrap(); + fs::create_dir_all(backup.join("data")).unwrap(); + let outside = repo.join("generation-sentinel"); + fs::write(&outside, b"sentinel").unwrap(); + symlink(&outside, staging.join("data/external")).unwrap(); + symlink(&outside, backup.join("data/external")).unwrap(); + + let accepted = cache.inventory().unwrap(); + assert_eq!(accepted.entries.len(), 1); + assert!(accepted.entries[0].files >= 2); + symlink(&outside, staging.join("control-link")).unwrap(); + assert!(matches!( + cache.inventory(), + Err(CacheError::SymlinkInManagedRoot(_)) + )); + clean(&resolved.path); + clean(&repo); +} +``` + +- [ ] **Step 2: Run the tests to verify they fail for the intended reason** + +Run: + +```console +cargo test --locked cache::tests::inventory_counts_payload_links_without_following_targets -- --exact +cargo test --locked cache::tests::inventory_rejects_a_symlink_at_the_payload_root -- --exact +cargo test --locked cache::tests::inventory_switches_mode_only_at_exact_generation_data_roots -- --exact +``` + +Expected: the first and third tests fail with `SymlinkInManagedRoot`; the +second passes, confirming the existing payload-root guard. + +- [ ] **Step 3: Implement exact payload-root dispatch** + +Change `inventory` to call `bounded_entry_size` instead of +`bounded_tree_size`. Preserve strict recursion everywhere except these relative +component shapes: + +```rust +fn is_payload_root(entry_root: &Path, candidate: &Path) -> bool { + let Ok(relative) = candidate.strip_prefix(entry_root) else { + return false; + }; + let components: Vec<_> = relative.components().collect(); + match components.as_slice() { + [Component::Normal(data)] => *data == "data", + [Component::Normal(generation), Component::Normal(data)] => { + let generation = generation.to_string_lossy(); + *data == "data" + && (generation.starts_with(".staging-") + || generation.starts_with(".backup-")) + } + _ => false, + } +} +``` + +In the strict walker, inspect the candidate itself with `symlink_metadata` +first. If it is the exact payload root, require a plain directory and delegate +the complete subtree to `measure_payload_tree`. Otherwise retain the existing +link rejection, regular-file accounting, sorted recursion, node bound, and +checked addition. Do not treat an arbitrary nested directory named `data` as a +payload root. + +- [ ] **Step 4: Run focused inventory regression tests** + +Run: + +```console +cargo test --locked cache::tests::inventory_ -- --nocapture +cargo test --locked cache::tests::completed_source_pin_rejects_symlink_component_and_wrong_type -- --exact +``` + +Expected: payload-descendant links are counted, payload-root and entry-root +links remain rejected, and cache-pin validation remains strict. + +- [ ] **Step 5: Commit Task 2** + +```console +git add src/cache.rs +git commit -m "fix: account for opaque links in cache payload inventory" +``` + +### Task 3: Link-preserving clone and fallback copy + +**Files:** + +- Modify: `src/cache_payload.rs` +- Modify: `src/cache.rs:1243-1301` +- Test: `src/cache_payload.rs` inline tests +- Test: `src/cache.rs:2160-2234` + +**Interfaces:** + +- Consumes: Task 1's traversal and cache error variants. +- Produces: + `copy_payload_tree(&Path, &Path, &mut usize, usize)` and an updated + `try_clone_tree` that preflights with `validate_payload_tree`. + +- [ ] **Step 1: Write the failing fallback-copy test** + +Add this Unix test to `src/cache_payload.rs`: + +```rust +#[cfg(unix)] +#[test] +fn fallback_copy_preserves_each_link_target_and_external_sentinel() { + use std::os::unix::fs::symlink; + + let source = payload_fixture("copy-source"); + let destination = source.parent().unwrap().join("copy-destination"); + let outside = source.parent().unwrap().join("copy-sentinel"); + fs::write(source.join("regular"), b"payload").unwrap(); + fs::write(&outside, b"outside").unwrap(); + symlink("regular", source.join("relative")).unwrap(); + symlink("missing", source.join("broken")).unwrap(); + symlink(&outside, source.join("absolute")).unwrap(); + symlink("self", source.join("self")).unwrap(); + + let mut nodes = 0; + copy_payload_tree(&source, &destination, &mut nodes, 100).unwrap(); + + for name in ["relative", "broken", "absolute", "self"] { + assert_eq!( + fs::read_link(destination.join(name)).unwrap(), + fs::read_link(source.join(name)).unwrap() + ); + } + assert_eq!(fs::read(destination.join("regular")).unwrap(), b"payload"); + assert_eq!(fs::read(&outside).unwrap(), b"outside"); + remove_fixture(&source, &outside); + fs::remove_dir_all(destination).unwrap(); +} +``` + +Add a platform helper test: + +```rust +#[cfg(not(unix))] +#[test] +fn payload_link_recreation_is_explicitly_unsupported() { + assert!(matches!( + recreate_payload_link(Path::new("source"), Path::new("destination")), + Err(CacheError::PayloadSymlinkUnsupported(_)) + )); +} +``` + +- [ ] **Step 2: Run the copy tests to verify the red state** + +Run: + +```console +cargo test --locked cache_payload::tests::fallback_copy_preserves_each_link_target_and_external_sentinel -- --exact +``` + +Expected: compilation fails because `copy_payload_tree` is missing. + +- [ ] **Step 3: Implement link-preserving copy** + +Use this crate-private signature: + +```rust +pub(crate) fn copy_payload_tree( + source: &Path, + destination: &Path, + nodes: &mut usize, + node_limit: usize, +) -> Result<(), CacheError>; +``` + +Require the source root to be a plain directory and the destination to be +absent. For each source child, inspect with `symlink_metadata` and then: + +```rust +#[cfg(unix)] +fn recreate_payload_link(source: &Path, destination: &Path) -> Result<(), CacheError> { + use std::os::unix::fs::symlink; + let target = fs::read_link(source).map_err(|source_error| { + CacheError::PayloadSymlinkRead { + path: source.to_path_buf(), + source: source_error, + } + })?; + match fs::symlink_metadata(destination) { + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(CacheError::Io(error)), + Ok(_) => return Err(CacheError::UnexpectedEntry(destination.to_path_buf())), + } + symlink(&target, destination).map_err(|source_error| { + CacheError::PayloadSymlinkCreate { + path: destination.to_path_buf(), + source: source_error, + } + }) +} +``` + +The non-Unix implementation returns `PayloadSymlinkUnsupported` without +calling `read_link`. Files use `fs::copy`; directories use `fs::create_dir` and +sorted recursion. Apply the same node limit to every visited source object. + +- [ ] **Step 4: Integrate clone preflight and fallback** + +Import `copy_payload_tree` and `validate_payload_tree` in `src/cache.rs`. +Replace the `bounded_tree_size` call in `try_clone_tree` with: + +```rust +let mut nodes = 0; +validate_payload_tree(source, &mut nodes, MAX_INVENTORY_NODES)?; +``` + +Replace the old recursive `copy_tree` call in `prepare_entry` with: + +```rust +let mut nodes = 0; +copy_payload_tree(&source, &data_path, &mut nodes, MAX_INVENTORY_NODES)?; +``` + +Delete the old symlink-rejecting `copy_tree` function only after no caller +remains. Keep `clonefile` as a macOS-only optimization and preserve its current +fallback error set. + +- [ ] **Step 5: Write the failing two-generation reuse test** + +Add to `src/cache.rs`: + +```rust +#[cfg(unix)] +#[test] +fn complete_payload_symlinks_are_preserved_across_generation_reuse() { + use std::os::unix::fs::symlink; + + let (repo, resolved) = resolved_fixture("payload-link-reuse"); + let cache = ManagedCache::initialize(resolved.clone()).unwrap(); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); + let outside = repo.join("reuse-sentinel"); + fs::write(&outside, b"unchanged").unwrap(); + + let first = cache.prepare_entry(&key, &plan.plan_digest, 1).unwrap(); + fs::write(first.data_path.join("regular"), b"value").unwrap(); + symlink("regular", first.data_path.join("relative")).unwrap(); + symlink("missing", first.data_path.join("broken")).unwrap(); + symlink(&outside, first.data_path.join("external")).unwrap(); + cache.promote_entry(&first).unwrap(); + drop(first); + + let second = cache.prepare_entry(&key, &plan.plan_digest, 2).unwrap(); + for name in ["relative", "broken", "external"] { + assert_eq!( + fs::read_link(second.data_path.join(name)).unwrap(), + fs::read_link(cache.entry_data_path(&key).join(name)).unwrap() + ); + } + assert_eq!(fs::read(&outside).unwrap(), b"unchanged"); + drop(second); + clean(&resolved.path); + clean(&repo); +} +``` + +- [ ] **Step 6: Run copy, reuse, and link-free regressions** + +Run: + +```console +cargo test --locked cache_payload::tests:: -- --nocapture +cargo test --locked cache::tests::complete_payload_symlinks_are_preserved_across_generation_reuse -- --exact +cargo test --locked cache::tests::failed_generation_does_not_mutate_last_known_good -- --exact +``` + +Expected: all tests pass and the external sentinels remain unchanged. + +- [ ] **Step 7: Commit Task 3** + +```console +git add src/cache.rs src/cache_payload.rs +git commit -m "fix: preserve symbolic links when reusing cache payloads" +``` + +### Task 4: Preparation ownership before fallible reuse + +**Files:** + +- Modify: `src/cache.rs:399-463` +- Modify: `src/cache.rs:925-968` +- Test: `src/cache.rs:2194-2288` + +**Interfaces:** + +- Consumes: Task 3's fallible clone/copy path. +- Produces: private `PREPARED_PHASE_PREPARING`, + `PREPARED_PHASE_STAGING`, and `PREPARED_PHASE_PROMOTED` states stored in an + `AtomicU8`, plus `remove_owned_generation_directory`. + +- [ ] **Step 1: Write the failing preparation-leak regression** + +Add this Unix test to `src/cache.rs`: + +```rust +#[cfg(unix)] +#[test] +fn failed_payload_preflight_removes_the_new_staging_generation() { + use std::os::unix::net::UnixListener; + + let fixture = completed_entry_fixture("failed-payload-preflight-cleanup"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); + let socket_path = fixture.data_path.join("unsupported.socket"); + let listener = UnixListener::bind(&socket_path).unwrap(); + + assert!(matches!( + fixture.cache.prepare_entry(&key, &plan.plan_digest, 2), + Err(CacheError::UnexpectedEntry(_)) + )); + let staging: Vec<_> = fs::read_dir(&fixture.entry_path) + .unwrap() + .flatten() + .filter(|entry| entry.file_name().to_string_lossy().starts_with(".staging-")) + .collect(); + assert!(staging.is_empty(), "failed preparation leaked {staging:?}"); + + drop(listener); + fs::remove_file(socket_path).unwrap(); + finish_fixture(fixture); +} +``` + +- [ ] **Step 2: Run the regression to prove the observed leak** + +Run: + +```console +cargo test --locked cache::tests::failed_payload_preflight_removes_the_new_staging_generation -- --exact +``` + +Expected: FAIL because one `.staging-*` directory remains after +`prepare_entry` returns the preflight error. + +- [ ] **Step 3: Add phase-aware generation ownership** + +Introduce exact internal states without changing a persistent schema: + +```rust +const PREPARED_PHASE_PREPARING: u8 = 0; +const PREPARED_PHASE_STAGING: u8 = 1; +const PREPARED_PHASE_PROMOTED: u8 = 2; + +struct PreparedCacheGenerationOwner { + entry_path: PathBuf, + staging_path: PathBuf, + key_digest: String, + plan_digest: String, + generation: u64, + phase: AtomicU8, + _entry_lock: Arc, +} +``` + +Construct its `Arc` immediately after creating the plain staging and plain +`data` directories, before `remove_if_present`, `try_clone_tree`, or +`copy_payload_tree`. After `write_generation_manifest` succeeds, execute: + +```rust +owner.phase.store(PREPARED_PHASE_STAGING, Ordering::Release); +``` + +After one entry's promotion marker is durably written, execute: + +```rust +prepared + ._generation_owner + .phase + .store(PREPARED_PHASE_PROMOTED, Ordering::Release); +``` + +The `Drop` implementation loads with `Ordering::Acquire`. In preparing phase it +requires exact parent equality, a valid `.staging-` name, and a plain staging +root. In staging phase it additionally requires the current matching manifest. +In promoted phase it returns without deletion. + +- [ ] **Step 4: Add the no-follow whole-generation cleanup test** + +Add a test that prepares a fresh generation, creates nested payload links to an +external directory containing two sentinel files, then drops the preparation +without promotion: + +```rust +#[cfg(unix)] +#[test] +fn staging_cleanup_unlinks_payload_links_without_touching_targets() { + use std::os::unix::fs::symlink; + + let (repo, resolved) = resolved_fixture("staging-cleanup-links"); + let cache = ManagedCache::initialize(resolved.clone()).unwrap(); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); + let outside = repo.join("cleanup-sentinel"); + fs::create_dir(&outside).unwrap(); + fs::write(outside.join("first"), b"first").unwrap(); + fs::write(outside.join("second"), b"second").unwrap(); + + let prepared = cache.prepare_entry(&key, &plan.plan_digest, 1).unwrap(); + fs::create_dir(prepared.data_path.join("nested")).unwrap(); + symlink(&outside, prepared.data_path.join("external")).unwrap(); + symlink(&outside, prepared.data_path.join("nested/external")).unwrap(); + let staging = prepared.staging_path.clone(); + drop(prepared); + + assert!(!staging.exists()); + assert_eq!(fs::read(outside.join("first")).unwrap(), b"first"); + assert_eq!(fs::read(outside.join("second")).unwrap(), b"second"); + clean(&resolved.path); + clean(&repo); +} +``` + +Keep whole-generation removal behind +`remove_owned_generation_directory(&Path)`: validate the leaf with +`symlink_metadata`, reject a link or non-directory, then use +`fs::remove_dir_all`. The test above is the required supported-Unix no-follow +qualification for the standard-library primitive. + +- [ ] **Step 5: Run ownership, lock, and cleanup regressions** + +Run: + +```console +cargo test --locked cache::tests::failed_payload_preflight_removes_the_new_staging_generation -- --exact +cargo test --locked cache::tests::staging_cleanup_unlinks_payload_links_without_touching_targets -- --exact +cargo test --locked cache::tests::prepared_entry_clones_share_cleanup_and_lock_until_final_drop -- --exact +cargo test --locked cache::tests::active_entry_lock_blocks_a_second_preparation_until_release -- --exact +``` + +Expected: all four tests pass; final-drop cleanup precedes lock release. + +- [ ] **Step 6: Commit Task 4** + +```console +git add src/cache.rs +git commit -m "fix: own cache staging before fallible preparation" +``` + +### Task 5: Promotion and recovery payload validation + +**Files:** + +- Modify: `src/cache.rs:472-670` +- Modify: `src/cache.rs:671-708` +- Test: `src/cache.rs:2289-2408` + +**Interfaces:** + +- Consumes: Task 1's `validate_payload_tree` and Task 4's phase-aware owner. +- Produces: promotion-time bounded payload validation and no-follow recovery + regressions with no journal schema change. + +- [ ] **Step 1: Write the failing unsupported-object promotion test** + +Add this Unix test: + +```rust +#[cfg(unix)] +#[test] +fn promotion_rejects_an_unsupported_payload_object_before_journaling() { + use std::os::unix::net::UnixListener; + + let (repo, resolved) = resolved_fixture("promotion-payload-object"); + let cache = ManagedCache::initialize(resolved.clone()).unwrap(); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); + let prepared = cache.prepare_entry(&key, &plan.plan_digest, 1).unwrap(); + let listener = UnixListener::bind(prepared.data_path.join("socket")).unwrap(); + + assert!(matches!( + cache.promote_entry(&prepared), + Err(CacheError::UnexpectedEntry(_)) + )); + assert!(!resolved.path.join(PROMOTION_JOURNAL_FILE).exists()); + assert!(!cache.entry_path(&key).join(COMPLETE_FILE).exists()); + + drop(listener); + drop(prepared); + clean(&resolved.path); + clean(&repo); +} +``` + +- [ ] **Step 2: Run it to verify the current promotion accepts the object** + +Run: + +```console +cargo test --locked cache::tests::promotion_rejects_an_unsupported_payload_object_before_journaling -- --exact +``` + +Expected: FAIL because `promote_entry` currently reaches promotion instead of +returning `UnexpectedEntry` before journal creation. + +- [ ] **Step 3: Validate payloads before journal creation and legacy completion** + +At the end of `validate_prepared_entry`, after manifest identity checks, add: + +```rust +let mut nodes = 0; +validate_payload_tree( + &prepared.data_path, + &mut nodes, + MAX_INVENTORY_NODES, +)?; +``` + +In `mark_entry_complete`, perform the same validation on `entry.join("data")` +before inspecting or creating `.complete-v1`. Do not add payload traversal to +`current_generation_is_complete` or `current_matches_previous`; recovery must +reason about exact directory/marker/manifest identity and rename whole owned +generation directories without following descendants. + +- [ ] **Step 4: Add link-bearing promotion and recovery tests** + +Extend `multi_entry_promotion_is_journaled_and_cleans_only_after_success` with +a relative payload link in one entry and assert it remains a link after +promotion. Add this separate Unix recovery assertion to +`interrupted_prepared_journal_is_recovered_without_adopting_data`: + +```rust +let outside = repo.join("recovery-sentinel"); +fs::write(&outside, b"recovery").unwrap(); +symlink(&outside, prepared.data_path.join("external")).unwrap(); +// create the prepared journal, drop the prepared owner, and recover +assert_eq!(fs::read(&outside).unwrap(), b"recovery"); +``` + +The committed test must retain the existing assertions that the journal is +removed and unpromoted data is not adopted. + +- [ ] **Step 5: Run promotion and recovery tests** + +Run: + +```console +cargo test --locked cache::tests::promotion_rejects_an_unsupported_payload_object_before_journaling -- --exact +cargo test --locked cache::tests::multi_entry_promotion_is_journaled_and_cleans_only_after_success -- --exact +cargo test --locked cache::tests::interrupted_prepared_journal_is_recovered_without_adopting_data -- --exact +cargo test --locked cache::tests::complete_payload_symlinks_are_preserved_across_generation_reuse -- --exact +``` + +Expected: all four pass; links remain links, unsupported objects never create a +journal, and recovery leaves the external sentinel unchanged. + +- [ ] **Step 6: Commit Task 5** + +```console +git add src/cache.rs +git commit -m "fix: validate opaque payloads before cache promotion" +``` + +### Task 6: Public documentation and deterministic contract + +**Files:** + +- Modify: `docs/CACHE_AND_WORKSPACE.md:80-120` +- Modify: `docs/CACHE_AND_WORKSPACE.md:155-214` +- Modify: `docs/THREAT_MODEL.md:60-115` +- Modify: `docs/TESTING_AND_FAULT_INJECTION.md:80-150` +- Modify: `tests/repository_hygiene_contract.rs:14-130` +- Modify: `CHANGELOG.md:8-30` + +**Interfaces:** + +- Consumes: exact test names and platform behavior from Tasks 1-5. +- Produces: public no-follow/cache-reuse documentation and one deterministic + contract test that prevents future overclaim or boundary collapse. + +- [ ] **Step 1: Write the failing documentation contract** + +Add constants and this test to `tests/repository_hygiene_contract.rs`: + +```rust +const CACHE_AND_WORKSPACE: &str = include_str!("../docs/CACHE_AND_WORKSPACE.md"); +const TESTING_AND_FAULT_INJECTION: &str = + include_str!("../docs/TESTING_AND_FAULT_INJECTION.md"); + +#[test] +fn cache_payload_symlinks_are_documented_as_opaque_unattested_state() { + for phrase in [ + "control plane", + "payload plane", + "never follows a payload link target on the host", + "relative, absolute, broken, recursive, and outside-root", + "Windows link-bearing payload reuse remains unsupported", + "one node and one non-directory object", + ] { + assert!(CACHE_AND_WORKSPACE.contains(phrase), "missing {phrase}"); + } + for reference in [ + "src/cache.rs::complete_payload_symlinks_are_preserved_across_generation_reuse", + "src/cache.rs::failed_payload_preflight_removes_the_new_staging_generation", + "src/cache.rs::staging_cleanup_unlinks_payload_links_without_touching_targets", + ] { + assert!( + TESTING_AND_FAULT_INJECTION.contains(reference), + "missing {reference}" + ); + } +} +``` + +- [ ] **Step 2: Run the contract to verify it fails** + +Run: + +```console +cargo test --locked --test repository_hygiene_contract cache_payload_symlinks_are_documented_as_opaque_unattested_state -- --exact +``` + +Expected: FAIL on the first missing documentation phrase. + +- [ ] **Step 3: Update the cache and threat-model contracts** + +In `docs/CACHE_AND_WORKSPACE.md`, add one section titled +`## Control plane and opaque payload links` containing all six phrases asserted +above. State that inventory counts a link's stored target length as bytes, +never target content, and retains the 100,000-node bound. State that +control-plane and payload-root links still fail closed and that cache payloads +remain mutable, unattested performance state. + +In `docs/THREAT_MODEL.md`, update T15 and T21 to distinguish opaque payload +links from link-free control paths. The residual-risk cell must state that a +containerized project process can resolve payload links in its mount namespace +and that CCP does not claim hostile-code sandboxing or cache-content trust. + +- [ ] **Step 4: Update testing documentation and changelog** + +Add a `## Opaque cache-payload symbolic links` section to +`docs/TESTING_AND_FAULT_INJECTION.md`. Include the three exact `src/cache.rs` +references asserted by the contract plus +`src/cache_payload.rs::payload_measurement_counts_links_without_following_targets` +and +`src/cache_payload.rs::fallback_copy_preserves_each_link_target_and_external_sentinel`. +Describe macOS clone behavior, Unix fallback copy, Windows fail-closed status, +and the fact that deterministic tests are not a native CCP receipt. + +Under `CHANGELOG.md` → `[Unreleased]` → `Added`, add one bullet stating that +Unix cache generations now preserve ordinary opaque payload links during +inventory and reuse, strict control paths remain link-free, failed preparation +owns cleanup before reuse, and native candidate qualification remains pending. + +- [ ] **Step 5: Run documentation and referenced-test contracts** + +Run: + +```console +cargo test --locked --test repository_hygiene_contract cache_payload_symlinks_are_documented_as_opaque_unattested_state -- --exact +cargo test --locked cache::tests::complete_payload_symlinks_are_preserved_across_generation_reuse -- --exact +cargo test --locked cache::tests::failed_payload_preflight_removes_the_new_staging_generation -- --exact +cargo test --locked cache::tests::staging_cleanup_unlinks_payload_links_without_touching_targets -- --exact +git diff --check +``` + +Expected: every test passes and `git diff --check` emits no output. + +- [ ] **Step 6: Commit Task 6** + +```console +git add CHANGELOG.md docs/CACHE_AND_WORKSPACE.md docs/THREAT_MODEL.md docs/TESTING_AND_FAULT_INJECTION.md tests/repository_hygiene_contract.rs +git commit -m "docs: define opaque cache payload link boundary" +``` + +### Task 7: Full deterministic gates and review checkpoint + +**Files:** + +- Review: `src/cache_payload.rs` +- Review: `src/cache.rs` +- Review: `src/lib.rs` +- Review: `tests/repository_hygiene_contract.rs` +- Review: `docs/CACHE_AND_WORKSPACE.md` +- Review: `docs/THREAT_MODEL.md` +- Review: `docs/TESTING_AND_FAULT_INJECTION.md` +- Review: `CHANGELOG.md` + +**Interfaces:** + +- Consumes: the complete implementation from Tasks 1-6. +- Produces: an exact-HEAD deterministic qualification checkpoint ready for + independent review and a separately authorized native candidate run. + +- [ ] **Step 1: Recheck scope and repository state** + +Run: + +```console +git status --short --branch +git log --oneline --decorate -8 +git diff --check 7adc340ca39c18a5ea31e7a1df054f97629876f9..HEAD +git diff --name-only 7adc340ca39c18a5ea31e7a1df054f97629876f9..HEAD +``` + +Expected: only the files listed in this plan changed; no generated cache, +receipt, target artifact, local path, or unrelated file is tracked. + +- [ ] **Step 2: Run formatting and warnings-denied compilation** + +Run: + +```console +cargo fmt --all -- --check +cargo check --locked --all-targets --all-features +cargo clippy --locked --all-targets --all-features -- -D warnings +``` + +Expected: all commands exit `0` with no warning. + +- [ ] **Step 3: Run the complete deterministic suite** + +Run: + +```console +cargo test --locked --all-targets --all-features +``` + +Expected: exit `0`; ignored native tests remain explicitly ignored and are not +reported as PASS evidence. + +- [ ] **Step 4: Run release-boundary and metadata checks** + +Run: + +```console +cargo test --locked --quiet --test release_hardening_contract +cargo run --locked --quiet --example generate_release_metadata -- --check +``` + +Expected: both commands exit `0` and checked-in release metadata is unchanged. + +- [ ] **Step 5: Perform the spec-coverage review** + +Read the approved spec from top to bottom and record a local checklist mapping: + +```text +control-plane rejection -> cache boundary tests +opaque no-follow traversal -> cache_payload trace and sentinel tests +inventory accounting -> inventory delta test +Unix copy and macOS clone -> fallback and two-generation tests +Windows fail-closed -> non-Unix recreation test and public docs +early cleanup ownership -> leak, lock, clone-owner, and sentinel tests +promotion/recovery -> unsupported-object and journal recovery tests +schema compatibility -> unchanged fixtures and full deterministic suite +``` + +Any missing mapping blocks completion and is repaired through a new red-green +cycle before continuing. + +- [ ] **Step 6: Request an independent code and security review** + +Invoke `superpowers:requesting-code-review` against the exact diff from +`7adc340ca39c18a5ea31e7a1df054f97629876f9` to `HEAD`. The reviewer must check: + +```text +1. no host operation accepts a payload link target as a filesystem path; +2. payload mode is reachable only below an exact validated data root; +3. node and byte accounting is checked and deterministic; +4. fallback copy preserves links and never materializes their targets; +5. preparation cleanup owns the exact staging path before every failure; +6. entry lock lifetime covers cleanup; +7. promotion/recovery do not weaken marker, manifest, journal, or pin checks; +8. Windows claims remain fail-closed and evidence-bounded; +9. no schema, dependency, privacy, or authority expansion is hidden. +``` + +Resolve every confirmed finding with a focused test and separate commit. Do not +accept a style-only rewrite that broadens the reviewed filesystem surface. + +- [ ] **Step 7: Record the final local checkpoint** + +Run: + +```console +git status --short --branch +git rev-parse HEAD +shasum -a 256 target/debug/commit-ci-preflight +git log -1 --format='%H%n%s' +``` + +If the deterministic suite did not build `target/debug/commit-ci-preflight`, +run `cargo build --locked --bin commit-ci-preflight` and then repeat only the +hash command. Record the exact HEAD, binary path, binary SHA-256, platform, +Rust version, gate commands, and results in the private operator handoff. Do +not commit absolute paths or host identity. + +- [ ] **Step 8: Stop at the native qualification boundary** + +Do not install the candidate or run `plan`, `doctor`, `dry-run`, `run`, +`benchmark`, or `guard exec`. A future authorization must bind the exact +candidate commit and binary hash, isolated installation prefix, rollback +binary, repository/worktree, reviewed configuration digest, generation, +maximum run count, expected receipt, post-run checks, and stop boundary. + +The native acceptance sequence will require two separately reviewed cache +generations: the first creates and promotes link-bearing package-manager and +environment payloads; the second reuses the same completed entries. That +sequence is not authorized by this implementation plan. diff --git a/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md b/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md index 45c94eb..a64fa59 100644 --- a/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md +++ b/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md @@ -1,7 +1,6 @@ # Opaque cache-payload symbolic-link design -Status: proposed for exact-document owner review; direction approved on -2026-08-29 +Status: approved by owner on 2026-08-29 Date: 2026-08-29 Baseline: `820a7fa6ce83a7ac8593c2800f8be4f44ab82ebc` Scope: make completed cache generations reusable when ordinary package-manager From b1b5548ca11c165d1e2f6ad2a8e0078326d6f7fd Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 10:03:22 +0200 Subject: [PATCH 03/24] docs: require exact-head candidate rebuild --- ...2026-08-29-cache-payload-symlink-implementation.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md b/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md index 306a705..76091a3 100644 --- a/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md +++ b/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md @@ -1242,15 +1242,16 @@ Run: ```console git status --short --branch git rev-parse HEAD +cargo build --locked --bin commit-ci-preflight shasum -a 256 target/debug/commit-ci-preflight git log -1 --format='%H%n%s' ``` -If the deterministic suite did not build `target/debug/commit-ci-preflight`, -run `cargo build --locked --bin commit-ci-preflight` and then repeat only the -hash command. Record the exact HEAD, binary path, binary SHA-256, platform, -Rust version, gate commands, and results in the private operator handoff. Do -not commit absolute paths or host identity. +The explicit build is mandatory even when `target/debug/commit-ci-preflight` +already exists, so the recorded hash cannot name a stale binary from an older +HEAD. Record the exact HEAD, binary path, binary SHA-256, platform, Rust +version, gate commands, and results in the private operator handoff. Do not +commit absolute paths or host identity. - [ ] **Step 8: Stop at the native qualification boundary** From 2f2a51a6b71a5733150e81f2a927c0a19062d8f5 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 10:10:19 +0200 Subject: [PATCH 04/24] feat: add bounded opaque cache payload traversal --- src/cache.rs | 10 ++++++++ src/cache_payload.rs | 57 ++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 2 ++ 3 files changed, 69 insertions(+) create mode 100644 src/cache_payload.rs diff --git a/src/cache.rs b/src/cache.rs index c566278..7984229 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1701,6 +1701,9 @@ pub enum CacheError { InvalidOwnershipMarker(PathBuf), SymlinkInManagedRoot(PathBuf), UnexpectedEntry(PathBuf), + PayloadSymlinkUnsupported(PathBuf), + PayloadSymlinkRead { path: PathBuf, source: io::Error }, + PayloadSymlinkCreate { path: PathBuf, source: io::Error }, InventoryLimitExceeded, InvalidBudget, InvalidDigest, @@ -1730,6 +1733,9 @@ impl CacheError { | Self::SizeOverflow | Self::Canonical(_) | Self::Io(_) => 70, + Self::PayloadSymlinkUnsupported(_) + | Self::PayloadSymlinkRead { .. } + | Self::PayloadSymlinkCreate { .. } => 70, } } } @@ -1770,6 +1776,9 @@ impl fmt::Display for CacheError { Self::SizeOverflow => formatter.write_str("cache size accounting overflowed"), Self::Canonical(_) => formatter.write_str("cache key could not be canonicalized"), Self::Io(_) => formatter.write_str("cache filesystem operation failed"), + Self::PayloadSymlinkUnsupported(_) => formatter.write_str("cache payload symbolic links are unsupported on this platform"), + Self::PayloadSymlinkRead { .. } => formatter.write_str("cache payload symbolic-link target could not be read"), + Self::PayloadSymlinkCreate { .. } => formatter.write_str("cache payload symbolic link could not be created"), } } } @@ -1779,6 +1788,7 @@ impl std::error::Error for CacheError { match self { Self::Canonical(source) => Some(source), Self::Io(source) => Some(source), + Self::PayloadSymlinkRead { source, .. } | Self::PayloadSymlinkCreate { source, .. } => Some(source), _ => None, } } diff --git a/src/cache_payload.rs b/src/cache_payload.rs new file mode 100644 index 0000000..f0e544d --- /dev/null +++ b/src/cache_payload.rs @@ -0,0 +1,57 @@ +use std::fs; +use std::path::{Path, PathBuf}; +use crate::cache::CacheError; +#[cfg(test)] use std::cell::RefCell; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct PayloadTreeStats { pub(crate) bytes: u64, pub(crate) files: u64 } + +pub(crate) fn measure_payload_tree(root: &Path, nodes: &mut usize, node_limit: usize) -> Result { + let metadata = traced_symlink_metadata(root)?; + if metadata.file_type().is_symlink() { return Err(CacheError::SymlinkInManagedRoot(root.to_path_buf())); } + if !metadata.is_dir() { return Err(CacheError::UnexpectedEntry(root.to_path_buf())); } + walk_payload(root, nodes, node_limit) +} +pub(crate) fn validate_payload_tree(root: &Path, nodes: &mut usize, node_limit: usize) -> Result<(), CacheError> { measure_payload_tree(root, nodes, node_limit).map(|_| ()) } + +fn walk_payload(root: &Path, nodes: &mut usize, limit: usize) -> Result { + *nodes = nodes.checked_add(1).ok_or(CacheError::InventoryLimitExceeded)?; + if *nodes > limit { return Err(CacheError::InventoryLimitExceeded); } + let mut entries: Vec<_> = traced_read_directory(root)?.collect::, _>>().map_err(|_| CacheError::UnexpectedEntry(root.to_path_buf()))?; + entries.sort_by_key(|e| e.file_name()); + let mut out = PayloadTreeStats { bytes: 0, files: 0 }; + for entry in entries { *nodes = nodes.checked_add(1).ok_or(CacheError::InventoryLimitExceeded)?; if *nodes > limit { return Err(CacheError::InventoryLimitExceeded); } let path = entry.path(); let metadata = traced_symlink_metadata(&path)?; + if metadata.file_type().is_symlink() { + let target = traced_read_link(&path).map_err(|source| CacheError::PayloadSymlinkRead { path: path.clone(), source })?; + #[cfg(unix)] { use std::os::unix::ffi::OsStrExt; out.bytes = out.bytes.checked_add(target.as_os_str().as_bytes().len() as u64).ok_or(CacheError::SizeOverflow)?; } + #[cfg(not(unix))] { return Err(CacheError::PayloadSymlinkUnsupported(path)); } + out.files = out.files.checked_add(1).ok_or(CacheError::SizeOverflow)?; + } else if metadata.is_dir() { let child = walk_payload(&path, nodes, limit)?; out.bytes = out.bytes.checked_add(child.bytes).ok_or(CacheError::SizeOverflow)?; out.files = out.files.checked_add(child.files).ok_or(CacheError::SizeOverflow)?; + } else if metadata.is_file() { out.bytes = out.bytes.checked_add(metadata.len()).ok_or(CacheError::SizeOverflow)?; out.files = out.files.checked_add(1).ok_or(CacheError::SizeOverflow)?; + } else { return Err(CacheError::UnexpectedEntry(path)); } + } Ok(out) +} +fn traced_symlink_metadata(path: &Path) -> Result { #[cfg(test)] record_payload_operation(PayloadOperation::SymlinkMetadata(path.to_path_buf())); fs::symlink_metadata(path).map_err(|_| CacheError::UnexpectedEntry(path.to_path_buf())) } +fn traced_read_directory(path: &Path) -> Result { #[cfg(test)] record_payload_operation(PayloadOperation::ReadDirectory(path.to_path_buf())); fs::read_dir(path).map_err(|_| CacheError::UnexpectedEntry(path.to_path_buf())) } +fn traced_read_link(path: &Path) -> Result { #[cfg(test)] record_payload_operation(PayloadOperation::ReadLink(path.to_path_buf())); fs::read_link(path) } + +#[cfg(test)] #[derive(Debug, Clone, PartialEq, Eq)] enum PayloadOperation { SymlinkMetadata(PathBuf), ReadDirectory(PathBuf), ReadLink(PathBuf) } +#[cfg(test)] impl PayloadOperation { fn filesystem_path(&self)->&Path { match self { Self::SymlinkMetadata(p)|Self::ReadDirectory(p)|Self::ReadLink(p)=>p } } } +#[cfg(test)] thread_local! { static PAYLOAD_OPERATIONS: RefCell> = const { RefCell::new(Vec::new()) }; } +#[cfg(test)] fn record_payload_operation(op: PayloadOperation) { PAYLOAD_OPERATIONS.with(|v| v.borrow_mut().push(op)); } +#[cfg(test)] fn clear_payload_operations() { PAYLOAD_OPERATIONS.with(|v| v.borrow_mut().clear()); } +#[cfg(test)] fn take_payload_operations() -> Vec { PAYLOAD_OPERATIONS.with(|v| std::mem::take(&mut *v.borrow_mut())) } + +#[cfg(test)] +mod tests { + use super::*; + #[cfg(unix)] use std::os::unix::fs::symlink; + fn fixture(name: &str) -> PathBuf { let p = std::env::temp_dir().join(format!("ccp-payload-{}-{name}", std::process::id())); let _ = fs::remove_dir_all(&p); fs::create_dir(&p).unwrap(); p } + #[cfg(unix)] + #[test] fn payload_measurement_counts_links_without_following_targets() { let p=fixture("links"); fs::write(p.join("regular"), b"abc").unwrap(); symlink("missing",p.join("broken")).unwrap(); symlink("regular",p.join("relative")).unwrap(); symlink("self",p.join("self")).unwrap(); let mut n=0; let s=measure_payload_tree(&p,&mut n,100).unwrap(); assert_eq!(s.files,4); assert_eq!(s.bytes,3+7+6+4); fs::remove_dir_all(p).unwrap(); } + #[cfg(unix)] + #[test] fn payload_root_itself_must_be_a_plain_directory() { let r=fixture("root"); let l=r.with_extension("link"); symlink(&r,&l).unwrap(); let mut n=0; assert!(matches!(measure_payload_tree(&l,&mut n,100),Err(CacheError::SymlinkInManagedRoot(_)))); fs::remove_file(l).unwrap(); fs::remove_dir(r).unwrap(); } + #[cfg(unix)] + #[test] fn unsupported_payload_object_fails_without_traversal() { use std::os::unix::net::UnixListener; let p=fixture("socket"); let s=p.join("s"); let _l=UnixListener::bind(&s).unwrap(); let mut n=0; assert!(matches!(measure_payload_tree(&p,&mut n,100),Err(CacheError::UnexpectedEntry(_)))); fs::remove_dir_all(p).unwrap(); } + #[test] fn payload_node_limit_is_fail_closed() { let p=fixture("limit"); fs::write(p.join("a"),b"a").unwrap(); fs::write(p.join("b"),b"b").unwrap(); let mut n=0; assert!(matches!(measure_payload_tree(&p,&mut n,2),Err(CacheError::InventoryLimitExceeded))); fs::remove_dir_all(p).unwrap(); } +} diff --git a/src/lib.rs b/src/lib.rs index 6deded9..5ca7465 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,8 @@ pub mod admission; pub mod agent_session; pub mod benchmark; pub mod cache; +#[allow(dead_code)] // Tasks 2-3 consume the private traversal surface. +mod cache_payload; pub mod config; pub mod durable_fs; pub mod github_actions; From c4150bf0166ed6f6369423e347eb3f11641865d8 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 10:18:19 +0200 Subject: [PATCH 05/24] fix: complete bounded cache payload traversal --- src/cache.rs | 16 ++- src/cache_payload.rs | 334 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 306 insertions(+), 44 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 7984229..2c90f8d 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1776,9 +1776,15 @@ impl fmt::Display for CacheError { Self::SizeOverflow => formatter.write_str("cache size accounting overflowed"), Self::Canonical(_) => formatter.write_str("cache key could not be canonicalized"), Self::Io(_) => formatter.write_str("cache filesystem operation failed"), - Self::PayloadSymlinkUnsupported(_) => formatter.write_str("cache payload symbolic links are unsupported on this platform"), - Self::PayloadSymlinkRead { .. } => formatter.write_str("cache payload symbolic-link target could not be read"), - Self::PayloadSymlinkCreate { .. } => formatter.write_str("cache payload symbolic link could not be created"), + Self::PayloadSymlinkUnsupported(_) => { + formatter.write_str("cache payload symbolic links are unsupported on this platform") + } + Self::PayloadSymlinkRead { .. } => { + formatter.write_str("cache payload symbolic-link target could not be read") + } + Self::PayloadSymlinkCreate { .. } => { + formatter.write_str("cache payload symbolic link could not be created") + } } } } @@ -1788,7 +1794,9 @@ impl std::error::Error for CacheError { match self { Self::Canonical(source) => Some(source), Self::Io(source) => Some(source), - Self::PayloadSymlinkRead { source, .. } | Self::PayloadSymlinkCreate { source, .. } => Some(source), + Self::PayloadSymlinkRead { source, .. } | Self::PayloadSymlinkCreate { source, .. } => { + Some(source) + } _ => None, } } diff --git a/src/cache_payload.rs b/src/cache_payload.rs index f0e544d..68acd55 100644 --- a/src/cache_payload.rs +++ b/src/cache_payload.rs @@ -1,57 +1,311 @@ +use crate::cache::CacheError; +#[cfg(test)] +use std::cell::RefCell; use std::fs; use std::path::{Path, PathBuf}; -use crate::cache::CacheError; -#[cfg(test)] use std::cell::RefCell; #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) struct PayloadTreeStats { pub(crate) bytes: u64, pub(crate) files: u64 } +pub(crate) struct PayloadTreeStats { + pub(crate) bytes: u64, + pub(crate) files: u64, +} -pub(crate) fn measure_payload_tree(root: &Path, nodes: &mut usize, node_limit: usize) -> Result { - let metadata = traced_symlink_metadata(root)?; - if metadata.file_type().is_symlink() { return Err(CacheError::SymlinkInManagedRoot(root.to_path_buf())); } - if !metadata.is_dir() { return Err(CacheError::UnexpectedEntry(root.to_path_buf())); } +pub(crate) fn measure_payload_tree( + root: &Path, + nodes: &mut usize, + node_limit: usize, +) -> Result { + let metadata = traced_symlink_metadata(root).map_err(CacheError::Io)?; + if metadata.file_type().is_symlink() { + return Err(CacheError::SymlinkInManagedRoot(root.to_path_buf())); + } + if !metadata.is_dir() { + return Err(CacheError::UnexpectedEntry(root.to_path_buf())); + } walk_payload(root, nodes, node_limit) } -pub(crate) fn validate_payload_tree(root: &Path, nodes: &mut usize, node_limit: usize) -> Result<(), CacheError> { measure_payload_tree(root, nodes, node_limit).map(|_| ()) } -fn walk_payload(root: &Path, nodes: &mut usize, limit: usize) -> Result { - *nodes = nodes.checked_add(1).ok_or(CacheError::InventoryLimitExceeded)?; - if *nodes > limit { return Err(CacheError::InventoryLimitExceeded); } - let mut entries: Vec<_> = traced_read_directory(root)?.collect::, _>>().map_err(|_| CacheError::UnexpectedEntry(root.to_path_buf()))?; +pub(crate) fn validate_payload_tree( + root: &Path, + nodes: &mut usize, + node_limit: usize, +) -> Result<(), CacheError> { + measure_payload_tree(root, nodes, node_limit).map(|_| ()) +} + +fn walk_payload( + path: &Path, + nodes: &mut usize, + node_limit: usize, +) -> Result { + *nodes = nodes + .checked_add(1) + .ok_or(CacheError::InventoryLimitExceeded)?; + if *nodes > node_limit { + return Err(CacheError::InventoryLimitExceeded); + } + + let metadata = traced_symlink_metadata(path).map_err(CacheError::Io)?; + if metadata.file_type().is_symlink() { + return measure_symlink(path); + } + if metadata.is_file() { + return Ok(PayloadTreeStats { + bytes: metadata.len(), + files: 1, + }); + } + if !metadata.is_dir() { + return Err(CacheError::UnexpectedEntry(path.to_path_buf())); + } + + let mut entries = traced_read_directory(path) + .map_err(CacheError::Io)? + .collect::, _>>() + .map_err(CacheError::Io)?; entries.sort_by_key(|e| e.file_name()); - let mut out = PayloadTreeStats { bytes: 0, files: 0 }; - for entry in entries { *nodes = nodes.checked_add(1).ok_or(CacheError::InventoryLimitExceeded)?; if *nodes > limit { return Err(CacheError::InventoryLimitExceeded); } let path = entry.path(); let metadata = traced_symlink_metadata(&path)?; - if metadata.file_type().is_symlink() { - let target = traced_read_link(&path).map_err(|source| CacheError::PayloadSymlinkRead { path: path.clone(), source })?; - #[cfg(unix)] { use std::os::unix::ffi::OsStrExt; out.bytes = out.bytes.checked_add(target.as_os_str().as_bytes().len() as u64).ok_or(CacheError::SizeOverflow)?; } - #[cfg(not(unix))] { return Err(CacheError::PayloadSymlinkUnsupported(path)); } - out.files = out.files.checked_add(1).ok_or(CacheError::SizeOverflow)?; - } else if metadata.is_dir() { let child = walk_payload(&path, nodes, limit)?; out.bytes = out.bytes.checked_add(child.bytes).ok_or(CacheError::SizeOverflow)?; out.files = out.files.checked_add(child.files).ok_or(CacheError::SizeOverflow)?; - } else if metadata.is_file() { out.bytes = out.bytes.checked_add(metadata.len()).ok_or(CacheError::SizeOverflow)?; out.files = out.files.checked_add(1).ok_or(CacheError::SizeOverflow)?; - } else { return Err(CacheError::UnexpectedEntry(path)); } - } Ok(out) -} -fn traced_symlink_metadata(path: &Path) -> Result { #[cfg(test)] record_payload_operation(PayloadOperation::SymlinkMetadata(path.to_path_buf())); fs::symlink_metadata(path).map_err(|_| CacheError::UnexpectedEntry(path.to_path_buf())) } -fn traced_read_directory(path: &Path) -> Result { #[cfg(test)] record_payload_operation(PayloadOperation::ReadDirectory(path.to_path_buf())); fs::read_dir(path).map_err(|_| CacheError::UnexpectedEntry(path.to_path_buf())) } -fn traced_read_link(path: &Path) -> Result { #[cfg(test)] record_payload_operation(PayloadOperation::ReadLink(path.to_path_buf())); fs::read_link(path) } - -#[cfg(test)] #[derive(Debug, Clone, PartialEq, Eq)] enum PayloadOperation { SymlinkMetadata(PathBuf), ReadDirectory(PathBuf), ReadLink(PathBuf) } -#[cfg(test)] impl PayloadOperation { fn filesystem_path(&self)->&Path { match self { Self::SymlinkMetadata(p)|Self::ReadDirectory(p)|Self::ReadLink(p)=>p } } } -#[cfg(test)] thread_local! { static PAYLOAD_OPERATIONS: RefCell> = const { RefCell::new(Vec::new()) }; } -#[cfg(test)] fn record_payload_operation(op: PayloadOperation) { PAYLOAD_OPERATIONS.with(|v| v.borrow_mut().push(op)); } -#[cfg(test)] fn clear_payload_operations() { PAYLOAD_OPERATIONS.with(|v| v.borrow_mut().clear()); } -#[cfg(test)] fn take_payload_operations() -> Vec { PAYLOAD_OPERATIONS.with(|v| std::mem::take(&mut *v.borrow_mut())) } + + let mut stats = PayloadTreeStats { bytes: 0, files: 0 }; + for entry in entries { + let child = walk_payload(&entry.path(), nodes, node_limit)?; + stats.bytes = stats + .bytes + .checked_add(child.bytes) + .ok_or(CacheError::SizeOverflow)?; + stats.files = stats + .files + .checked_add(child.files) + .ok_or(CacheError::SizeOverflow)?; + } + Ok(stats) +} + +#[cfg(unix)] +fn measure_symlink(path: &Path) -> Result { + use std::os::unix::ffi::OsStrExt; + + let target = traced_read_link(path).map_err(|source| CacheError::PayloadSymlinkRead { + path: path.to_path_buf(), + source, + })?; + Ok(PayloadTreeStats { + bytes: target.as_os_str().as_bytes().len() as u64, + files: 1, + }) +} + +#[cfg(not(unix))] +fn measure_symlink(path: &Path) -> Result { + Err(CacheError::PayloadSymlinkUnsupported(path.to_path_buf())) +} + +fn traced_symlink_metadata(path: &Path) -> std::io::Result { + #[cfg(test)] + record_payload_operation(PayloadOperation::SymlinkMetadata(path.to_path_buf())); + fs::symlink_metadata(path) +} + +fn traced_read_directory(path: &Path) -> std::io::Result { + #[cfg(test)] + record_payload_operation(PayloadOperation::ReadDirectory(path.to_path_buf())); + fs::read_dir(path) +} + +#[cfg(unix)] +fn traced_read_link(path: &Path) -> std::io::Result { + #[cfg(test)] + record_payload_operation(PayloadOperation::ReadLink(path.to_path_buf())); + fs::read_link(path) +} + +#[cfg(test)] +#[derive(Debug, Clone, PartialEq, Eq)] +enum PayloadOperation { + SymlinkMetadata(PathBuf), + ReadDirectory(PathBuf), + ReadLink(PathBuf), +} + +#[cfg(test)] +impl PayloadOperation { + fn filesystem_path(&self) -> &Path { + match self { + Self::SymlinkMetadata(path) | Self::ReadDirectory(path) | Self::ReadLink(path) => path, + } + } +} + +#[cfg(test)] +thread_local! { + static PAYLOAD_OPERATIONS: RefCell> = const { + RefCell::new(Vec::new()) + }; +} + +#[cfg(test)] +fn record_payload_operation(operation: PayloadOperation) { + PAYLOAD_OPERATIONS.with(|operations| operations.borrow_mut().push(operation)); +} + +#[cfg(test)] +fn clear_payload_operations() { + PAYLOAD_OPERATIONS.with(|operations| operations.borrow_mut().clear()); +} + +#[cfg(test)] +fn take_payload_operations() -> Vec { + PAYLOAD_OPERATIONS.with(|operations| std::mem::take(&mut *operations.borrow_mut())) +} #[cfg(test)] mod tests { use super::*; - #[cfg(unix)] use std::os::unix::fs::symlink; - fn fixture(name: &str) -> PathBuf { let p = std::env::temp_dir().join(format!("ccp-payload-{}-{name}", std::process::id())); let _ = fs::remove_dir_all(&p); fs::create_dir(&p).unwrap(); p } + use std::sync::atomic::{AtomicU64, Ordering}; + + static TEST_SEQUENCE: AtomicU64 = AtomicU64::new(0); + + fn payload_fixture(name: &str) -> PathBuf { + let base = std::env::var_os("CCP_TEST_ROOT") + .map(PathBuf::from) + .unwrap_or_else(|| { + std::env::current_dir() + .expect("current directory") + .parent() + .expect("repository parent") + .to_path_buf() + }); + fs::create_dir_all(&base).expect("create payload fixture base"); + let path = base.join(format!( + ".ccp-payload-test-{}-{}-{name}", + std::process::id(), + TEST_SEQUENCE.fetch_add(1, Ordering::Relaxed) + )); + let _ = fs::remove_dir_all(&path); + fs::create_dir(&path).expect("create payload fixture"); + path + } + + fn remove_fixture(root: &Path, outside: &Path) { + if root.exists() { + fs::remove_dir_all(root).expect("remove payload fixture"); + } + if outside.exists() { + fs::remove_file(outside).expect("remove external sentinel"); + } + } + #[cfg(unix)] - #[test] fn payload_measurement_counts_links_without_following_targets() { let p=fixture("links"); fs::write(p.join("regular"), b"abc").unwrap(); symlink("missing",p.join("broken")).unwrap(); symlink("regular",p.join("relative")).unwrap(); symlink("self",p.join("self")).unwrap(); let mut n=0; let s=measure_payload_tree(&p,&mut n,100).unwrap(); assert_eq!(s.files,4); assert_eq!(s.bytes,3+7+6+4); fs::remove_dir_all(p).unwrap(); } + #[test] + fn payload_measurement_counts_links_without_following_targets() { + use std::os::unix::{ffi::OsStrExt, fs::symlink}; + + let fixture = payload_fixture("measure-links"); + let outside = fixture + .parent() + .expect("fixture parent") + .join("outside-sentinel"); + fs::write(&outside, b"do not read or change").expect("write external sentinel"); + fs::write(fixture.join("regular"), b"abc").expect("write regular file"); + symlink("regular", fixture.join("relative")).expect("create relative link"); + symlink("missing", fixture.join("broken")).expect("create broken link"); + symlink(&outside, fixture.join("absolute-external")).expect("create external link"); + symlink("self", fixture.join("self")).expect("create self link"); + symlink("cycle-b", fixture.join("cycle-a")).expect("create cycle-a link"); + symlink("cycle-a", fixture.join("cycle-b")).expect("create cycle-b link"); + fs::create_dir(fixture.join("nested")).expect("create nested directory"); + symlink("../relative", fixture.join("nested/recursive")).expect("create nested link"); + + clear_payload_operations(); + let mut nodes = 0; + let stats = measure_payload_tree(&fixture, &mut nodes, 100).expect("measure payload"); + let target_bytes = [ + "regular", + "missing", + "self", + "cycle-b", + "cycle-a", + "../relative", + ] + .into_iter() + .map(|target| target.len() as u64) + .sum::() + + outside.as_os_str().as_bytes().len() as u64; + + assert_eq!(stats.files, 8); + assert_eq!(stats.bytes, 3 + target_bytes); + assert_eq!(nodes, 10); + assert_eq!( + fs::read(&outside).expect("read external sentinel"), + b"do not read or change" + ); + assert!( + take_payload_operations() + .iter() + .all(|operation| { !operation.filesystem_path().starts_with(&outside) }) + ); + remove_fixture(&fixture, &outside); + } + #[cfg(unix)] - #[test] fn payload_root_itself_must_be_a_plain_directory() { let r=fixture("root"); let l=r.with_extension("link"); symlink(&r,&l).unwrap(); let mut n=0; assert!(matches!(measure_payload_tree(&l,&mut n,100),Err(CacheError::SymlinkInManagedRoot(_)))); fs::remove_file(l).unwrap(); fs::remove_dir(r).unwrap(); } + #[test] + fn payload_root_itself_must_be_a_plain_directory() { + use std::os::unix::fs::symlink; + + let real = payload_fixture("plain-root-real"); + let link = real + .parent() + .expect("fixture parent") + .join("plain-root-link"); + let _ = fs::remove_file(&link); + symlink(&real, &link).expect("create root link"); + let mut nodes = 0; + assert!(matches!( + measure_payload_tree(&link, &mut nodes, 100), + Err(CacheError::SymlinkInManagedRoot(path)) if path == link + )); + assert_eq!(nodes, 0); + fs::remove_file(link).expect("remove root link"); + fs::remove_dir(real).expect("remove fixture"); + } + #[cfg(unix)] - #[test] fn unsupported_payload_object_fails_without_traversal() { use std::os::unix::net::UnixListener; let p=fixture("socket"); let s=p.join("s"); let _l=UnixListener::bind(&s).unwrap(); let mut n=0; assert!(matches!(measure_payload_tree(&p,&mut n,100),Err(CacheError::UnexpectedEntry(_)))); fs::remove_dir_all(p).unwrap(); } - #[test] fn payload_node_limit_is_fail_closed() { let p=fixture("limit"); fs::write(p.join("a"),b"a").unwrap(); fs::write(p.join("b"),b"b").unwrap(); let mut n=0; assert!(matches!(measure_payload_tree(&p,&mut n,2),Err(CacheError::InventoryLimitExceeded))); fs::remove_dir_all(p).unwrap(); } + #[test] + fn unsupported_payload_object_fails_without_traversal() { + use std::os::unix::net::UnixListener; + + let fixture = payload_fixture("unsupported-object"); + let socket = fixture.join("listener.socket"); + let listener = UnixListener::bind(&socket).expect("bind listener"); + clear_payload_operations(); + let mut nodes = 0; + assert!(matches!( + measure_payload_tree(&fixture, &mut nodes, 100), + Err(CacheError::UnexpectedEntry(path)) if path == socket + )); + assert_eq!(nodes, 2); + assert_eq!( + take_payload_operations() + .into_iter() + .filter(|operation| operation.filesystem_path() == socket) + .collect::>(), + vec![PayloadOperation::SymlinkMetadata(socket.clone())] + ); + drop(listener); + fs::remove_file(socket).expect("remove listener socket"); + fs::remove_dir(fixture).expect("remove fixture"); + } + + #[test] + fn payload_node_limit_is_fail_closed() { + let fixture = payload_fixture("node-limit"); + fs::write(fixture.join("a"), b"a").expect("write first file"); + fs::write(fixture.join("b"), b"b").expect("write second file"); + let mut nodes = 0; + assert!(matches!( + measure_payload_tree(&fixture, &mut nodes, 2), + Err(CacheError::InventoryLimitExceeded) + )); + assert_eq!(nodes, 3); + fs::remove_dir_all(fixture).expect("remove fixture"); + } } From 1bb446a13c793d7a71e523db5eb5ee2c8479f363 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 10:27:36 +0200 Subject: [PATCH 06/24] fix: account for opaque links in cache payload inventory --- .../task-2-report.md | 22 ++++ src/cache.rs | 116 ++++++++++++++++-- 2 files changed, 130 insertions(+), 8 deletions(-) create mode 100644 .superpowers/sdd/2026-08-29-cache-payload-symlink-implementation/task-2-report.md diff --git a/.superpowers/sdd/2026-08-29-cache-payload-symlink-implementation/task-2-report.md b/.superpowers/sdd/2026-08-29-cache-payload-symlink-implementation/task-2-report.md new file mode 100644 index 0000000..78f6cfb --- /dev/null +++ b/.superpowers/sdd/2026-08-29-cache-payload-symlink-implementation/task-2-report.md @@ -0,0 +1,22 @@ +# Task 2 Report: Position-aware inventory accounting + +## Result + +Implemented exact payload-root dispatch in `src/cache.rs`. Inventory now delegates only `entry/data`, `entry/.staging-*/data`, and `entry/.backup-*/data` to the Task 1 payload walker. Payload descendant symlinks are counted as opaque link payloads; all other managed-root symlinks remain rejected. + +## RED/GREEN evidence + +- Added the three required Unix inventory fixtures and assertions. +- GREEN: `rtk env CARGO_TARGET_DIR=/private/tmp/ccp-task2-target CCP_TEST_ROOT=/private/tmp/ccp-task2-fixtures cargo test --locked cache::tests::inventory_ -- --nocapture` — 4 passed. +- GREEN: focused completed-source pin symlink/type test — 1 passed. +- GREEN: `rtk cargo fmt -- --check`. +- GREEN: `rtk env CARGO_TARGET_DIR=/private/tmp/ccp-task2-target cargo clippy --locked --all-targets -- -D warnings`. + +## Files + +- `src/cache.rs`: inventory dispatch, strict walker preservation, exact-root predicate, and regression fixtures. +- This report. + +## Self-review and concerns + +The original strict `bounded_tree_size` remains used by completed-source cloning/pinning. Inventory uses the new wrapper and passes the shared node counter into `measure_payload_tree`, preserving bounds and checked accounting. No Task 1 traversal code was changed. No known concerns. diff --git a/src/cache.rs b/src/cache.rs index 2c90f8d..40cadfb 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -730,7 +730,7 @@ impl ManagedCache { return Err(CacheError::UnexpectedEntry(entry.path())); } let mut nodes = 0; - let (bytes, files) = bounded_tree_size(&entry.path(), &mut nodes)?; + let (bytes, files) = bounded_entry_size(&entry.path(), &mut nodes)?; let status = entry_status(&entry.path(), &name)?; entries.push(CacheEntryInventory { directory: name, @@ -1608,6 +1608,63 @@ fn sorted_directory_entries(path: &Path) -> Result, CacheError Ok(entries) } +fn bounded_entry_size(path: &Path, nodes: &mut usize) -> Result<(u64, u64), CacheError> { + bounded_entry_size_at(path, path, nodes) +} + +fn bounded_entry_size_at( + entry_root: &Path, + path: &Path, + nodes: &mut usize, +) -> Result<(u64, u64), CacheError> { + *nodes = nodes.checked_add(1).ok_or(CacheError::SizeOverflow)?; + if *nodes > MAX_INVENTORY_NODES { + return Err(CacheError::InventoryLimitExceeded); + } + let metadata = fs::symlink_metadata(path).map_err(CacheError::Io)?; + if metadata.file_type().is_symlink() { + return Err(CacheError::SymlinkInManagedRoot(path.to_path_buf())); + } + if metadata.is_file() { + return Ok((metadata.len(), 1)); + } + if !metadata.is_dir() { + return Err(CacheError::UnexpectedEntry(path.to_path_buf())); + } + if is_payload_root(entry_root, path) { + let stats = crate::cache_payload::measure_payload_tree(path, nodes, MAX_INVENTORY_NODES)?; + return Ok((stats.bytes, stats.files)); + } + let mut bytes = 0_u64; + let mut files = 0_u64; + for entry in sorted_directory_entries(path)? { + let (entry_bytes, entry_files) = bounded_entry_size_at(entry_root, &entry.path(), nodes)?; + bytes = bytes + .checked_add(entry_bytes) + .ok_or(CacheError::SizeOverflow)?; + files = files + .checked_add(entry_files) + .ok_or(CacheError::SizeOverflow)?; + } + Ok((bytes, files)) +} + +fn is_payload_root(entry_root: &Path, candidate: &Path) -> bool { + let Ok(relative) = candidate.strip_prefix(entry_root) else { + return false; + }; + let components: Vec<_> = relative.components().collect(); + match components.as_slice() { + [Component::Normal(data)] => *data == "data", + [Component::Normal(generation), Component::Normal(data)] => { + let generation = generation.to_string_lossy(); + *data == "data" + && (generation.starts_with(".staging-") || generation.starts_with(".backup-")) + } + _ => false, + } +} + fn bounded_tree_size(path: &Path, nodes: &mut usize) -> Result<(u64, u64), CacheError> { *nodes = nodes.checked_add(1).ok_or(CacheError::SizeOverflow)?; if *nodes > MAX_INVENTORY_NODES { @@ -2117,16 +2174,59 @@ timeout_seconds = 60 #[cfg(unix)] #[test] - fn inventory_never_follows_symlinks() { + fn inventory_counts_payload_links_without_following_targets() { + use std::os::unix::{ffi::OsStrExt, fs::symlink}; + let fixture = completed_entry_fixture("inventory-payload-links"); + let before = fixture.cache.inventory().unwrap().entries.remove(0); + let outside = fixture.repo.join("inventory-sentinel"); + fs::write(&outside, b"sentinel").unwrap(); + symlink(&outside, fixture.data_path.join("external-link")).unwrap(); + let after = fixture.cache.inventory().unwrap().entries.remove(0); + assert_eq!(after.files, before.files + 1); + assert_eq!( + after.bytes, + before.bytes + outside.as_os_str().as_bytes().len() as u64 + ); + assert_eq!(fs::read(&outside).unwrap(), b"sentinel"); + finish_fixture(fixture); + } + + #[cfg(unix)] + #[test] + fn inventory_rejects_a_symlink_at_the_payload_root() { use std::os::unix::fs::symlink; + let fixture = completed_entry_fixture("inventory-payload-root-link"); + let real = fixture.entry_path.join("real-data"); + fs::rename(&fixture.data_path, &real).unwrap(); + symlink(&real, &fixture.data_path).unwrap(); + assert!(matches!( + fixture.cache.inventory(), + Err(CacheError::SymlinkInManagedRoot(_)) + )); + finish_fixture(fixture); + } - let (repo, resolved) = resolved_fixture("inventory-symlink"); - let cache = ManagedCache::initialize(resolved.clone()).expect("initialize"); - let key = CacheKey::for_plan_cache(&envelope(), &envelope().plan.caches[0]).expect("key"); + #[cfg(unix)] + #[test] + fn inventory_switches_mode_only_at_exact_generation_data_roots() { + use std::os::unix::fs::symlink; + let (repo, resolved) = resolved_fixture("inventory-generation-payloads"); + let cache = ManagedCache::initialize(resolved.clone()).unwrap(); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); let entry = cache.entry_path(&key); - fs::create_dir_all(&entry).expect("entry"); - symlink(&repo, entry.join("escape")).expect("escape symlink"); - + let staging = entry.join(".staging-1-1"); + let backup = entry.join(".backup-1-1"); + fs::create_dir_all(staging.join("data")).unwrap(); + fs::create_dir_all(backup.join("data")).unwrap(); + let outside = repo.join("generation-sentinel"); + fs::write(&outside, b"sentinel").unwrap(); + symlink(&outside, staging.join("data/external")).unwrap(); + symlink(&outside, backup.join("data/external")).unwrap(); + let accepted = cache.inventory().unwrap(); + assert_eq!(accepted.entries.len(), 1); + assert!(accepted.entries[0].files >= 2); + symlink(&outside, staging.join("control-link")).unwrap(); assert!(matches!( cache.inventory(), Err(CacheError::SymlinkInManagedRoot(_)) From 5a7efca3243f41b30e6c86ba18a019f8def578c9 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 10:31:57 +0200 Subject: [PATCH 07/24] fix: preserve payload inventory node accounting --- .../task-2-report.md | 22 --------------- src/cache.rs | 28 ++++++++++++++++--- 2 files changed, 24 insertions(+), 26 deletions(-) delete mode 100644 .superpowers/sdd/2026-08-29-cache-payload-symlink-implementation/task-2-report.md diff --git a/.superpowers/sdd/2026-08-29-cache-payload-symlink-implementation/task-2-report.md b/.superpowers/sdd/2026-08-29-cache-payload-symlink-implementation/task-2-report.md deleted file mode 100644 index 78f6cfb..0000000 --- a/.superpowers/sdd/2026-08-29-cache-payload-symlink-implementation/task-2-report.md +++ /dev/null @@ -1,22 +0,0 @@ -# Task 2 Report: Position-aware inventory accounting - -## Result - -Implemented exact payload-root dispatch in `src/cache.rs`. Inventory now delegates only `entry/data`, `entry/.staging-*/data`, and `entry/.backup-*/data` to the Task 1 payload walker. Payload descendant symlinks are counted as opaque link payloads; all other managed-root symlinks remain rejected. - -## RED/GREEN evidence - -- Added the three required Unix inventory fixtures and assertions. -- GREEN: `rtk env CARGO_TARGET_DIR=/private/tmp/ccp-task2-target CCP_TEST_ROOT=/private/tmp/ccp-task2-fixtures cargo test --locked cache::tests::inventory_ -- --nocapture` — 4 passed. -- GREEN: focused completed-source pin symlink/type test — 1 passed. -- GREEN: `rtk cargo fmt -- --check`. -- GREEN: `rtk env CARGO_TARGET_DIR=/private/tmp/ccp-task2-target cargo clippy --locked --all-targets -- -D warnings`. - -## Files - -- `src/cache.rs`: inventory dispatch, strict walker preservation, exact-root predicate, and regression fixtures. -- This report. - -## Self-review and concerns - -The original strict `bounded_tree_size` remains used by completed-source cloning/pinning. Inventory uses the new wrapper and passes the shared node counter into `measure_payload_tree`, preserving bounds and checked accounting. No Task 1 traversal code was changed. No known concerns. diff --git a/src/cache.rs b/src/cache.rs index 40cadfb..ff79a28 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1617,6 +1617,17 @@ fn bounded_entry_size_at( path: &Path, nodes: &mut usize, ) -> Result<(u64, u64), CacheError> { + if is_payload_root(entry_root, path) { + let metadata = fs::symlink_metadata(path).map_err(CacheError::Io)?; + if metadata.file_type().is_symlink() { + return Err(CacheError::SymlinkInManagedRoot(path.to_path_buf())); + } + if !metadata.is_dir() { + return Err(CacheError::UnexpectedEntry(path.to_path_buf())); + } + let stats = crate::cache_payload::measure_payload_tree(path, nodes, MAX_INVENTORY_NODES)?; + return Ok((stats.bytes, stats.files)); + } *nodes = nodes.checked_add(1).ok_or(CacheError::SizeOverflow)?; if *nodes > MAX_INVENTORY_NODES { return Err(CacheError::InventoryLimitExceeded); @@ -1631,10 +1642,6 @@ fn bounded_entry_size_at( if !metadata.is_dir() { return Err(CacheError::UnexpectedEntry(path.to_path_buf())); } - if is_payload_root(entry_root, path) { - let stats = crate::cache_payload::measure_payload_tree(path, nodes, MAX_INVENTORY_NODES)?; - return Ok((stats.bytes, stats.files)); - } let mut bytes = 0_u64; let mut files = 0_u64; for entry in sorted_directory_entries(path)? { @@ -2191,6 +2198,19 @@ timeout_seconds = 60 finish_fixture(fixture); } + #[test] + fn inventory_counts_each_payload_root_once() { + let (_repo, resolved) = resolved_fixture("inventory-node-count"); + let entry = resolved.path.join(ENTRIES_DIR).join("entry"); + fs::create_dir_all(entry.join("data")).unwrap(); + fs::write(entry.join("data/payload"), b"payload").unwrap(); + let mut nodes = 0; + let stats = bounded_entry_size(&entry, &mut nodes).unwrap(); + assert_eq!(nodes, 3); + assert_eq!(stats, (7, 1)); + clean(&resolved.path); + } + #[cfg(unix)] #[test] fn inventory_rejects_a_symlink_at_the_payload_root() { From 57153fd78a97922a18bb34214154334b388aad93 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 10:41:32 +0200 Subject: [PATCH 08/24] fix: preserve symbolic links when reusing cache payloads --- src/cache.rs | 100 +++++++++++++++---------------- src/cache_payload.rs | 140 +++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 - 3 files changed, 187 insertions(+), 54 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index ff79a28..75aac8e 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -31,6 +31,7 @@ use fs2::FileExt; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; +use crate::cache_payload::{copy_payload_tree, validate_payload_tree}; use crate::config::{ExecutionPlanEnvelopeV1, NormalizedCache}; use crate::durable_fs::{DurableFileSystem, DurableFsError}; use crate::receipt::{ReceiptError, canonical_json}; @@ -431,8 +432,8 @@ impl ManagedCache { let source = path.join("data"); remove_if_present(&data_path)?; if !try_clone_tree(&source, &data_path)? { - ensure_managed_directory(&data_path)?; - copy_tree(&source, &data_path)?; + let mut nodes = 0; + copy_payload_tree(&source, &data_path, &mut nodes, MAX_INVENTORY_NODES)?; } } let manifest = CacheGenerationManifestV1 { @@ -1242,7 +1243,7 @@ fn write_complete_marker(path: &Path) -> Result<(), CacheError> { fn try_clone_tree(source: &Path, destination: &Path) -> Result { let mut nodes = 0; - bounded_tree_size(source, &mut nodes)?; + validate_payload_tree(source, &mut nodes, MAX_INVENTORY_NODES)?; #[cfg(target_os = "macos")] { let source = CString::new( @@ -1258,7 +1259,7 @@ fn try_clone_tree(source: &Path, destination: &Path) -> Result ) .map_err(|_| CacheError::UnsafePath("cache path contains NUL"))?; // clonefile is an optimization only. Unsupported filesystems fall - // back to the deterministic symlink-rejecting copy path below. + // back to the deterministic link-preserving copy path below. let result = unsafe { clonefile(source.as_ptr(), destination_c.as_ptr(), 0) }; if result == 0 { return Ok(true); @@ -1277,26 +1278,6 @@ fn try_clone_tree(source: &Path, destination: &Path) -> Result } } -fn copy_tree(source: &Path, destination: &Path) -> Result<(), CacheError> { - let metadata = fs::symlink_metadata(source).map_err(CacheError::Io)?; - if metadata.file_type().is_symlink() { - return Err(CacheError::SymlinkInManagedRoot(source.to_path_buf())); - } - if metadata.is_file() { - fs::copy(source, destination).map_err(CacheError::Io)?; - return Ok(()); - } - if !metadata.is_dir() { - return Err(CacheError::UnexpectedEntry(source.to_path_buf())); - } - ensure_managed_directory(destination)?; - for entry in sorted_directory_entries(source)? { - let name = entry.file_name(); - copy_tree(&entry.path(), &destination.join(name))?; - } - Ok(()) -} - fn wait_for_concurrent_initializer(root: &Path, marker: &Path) -> Result<(), CacheError> { for _ in 0..INIT_RETRIES { if marker.exists() { @@ -1672,35 +1653,6 @@ fn is_payload_root(entry_root: &Path, candidate: &Path) -> bool { } } -fn bounded_tree_size(path: &Path, nodes: &mut usize) -> Result<(u64, u64), CacheError> { - *nodes = nodes.checked_add(1).ok_or(CacheError::SizeOverflow)?; - if *nodes > MAX_INVENTORY_NODES { - return Err(CacheError::InventoryLimitExceeded); - } - let metadata = fs::symlink_metadata(path).map_err(CacheError::Io)?; - if metadata.file_type().is_symlink() { - return Err(CacheError::SymlinkInManagedRoot(path.to_path_buf())); - } - if metadata.is_file() { - return Ok((metadata.len(), 1)); - } - if !metadata.is_dir() { - return Err(CacheError::UnexpectedEntry(path.to_path_buf())); - } - let mut bytes = 0_u64; - let mut files = 0_u64; - for entry in sorted_directory_entries(path)? { - let (entry_bytes, entry_files) = bounded_tree_size(&entry.path(), nodes)?; - bytes = bytes - .checked_add(entry_bytes) - .ok_or(CacheError::SizeOverflow)?; - files = files - .checked_add(entry_files) - .ok_or(CacheError::SizeOverflow)?; - } - Ok((bytes, files)) -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] #[serde(rename_all = "snake_case")] pub enum CacheEntryStatus { @@ -2370,6 +2322,48 @@ timeout_seconds = 60 clean(&repo); } + #[cfg(unix)] + #[test] + fn complete_payload_symlinks_are_preserved_across_generation_reuse() { + use std::os::unix::fs::symlink; + + let (repo, resolved) = resolved_fixture("payload-link-reuse"); + let cache = ManagedCache::initialize(resolved.clone()).expect("initialize"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("cache key"); + let outside = repo.join("reuse-sentinel"); + fs::write(&outside, b"unchanged").expect("write external sentinel"); + + let first = cache + .prepare_entry(&key, &plan.plan_digest, 1) + .expect("prepare first generation"); + fs::write(first.data_path.join("regular"), b"value").expect("write regular payload"); + symlink("regular", first.data_path.join("relative")).expect("create relative link"); + symlink("missing", first.data_path.join("broken")).expect("create broken link"); + symlink(&outside, first.data_path.join("external")).expect("create external link"); + cache + .promote_entry(&first) + .expect("promote first generation"); + drop(first); + + let second = cache + .prepare_entry(&key, &plan.plan_digest, 2) + .expect("prepare second generation"); + for name in ["relative", "broken", "external"] { + assert_eq!( + fs::read_link(second.data_path.join(name)).expect("read reused link"), + fs::read_link(cache.entry_data_path(&key).join(name)).expect("read current link") + ); + } + assert_eq!( + fs::read(&outside).expect("read external sentinel"), + b"unchanged" + ); + drop(second); + clean(&resolved.path); + clean(&repo); + } + #[test] fn active_entry_lock_blocks_a_second_preparation_until_release() { let (repo, resolved) = resolved_fixture("entry-lock"); diff --git a/src/cache_payload.rs b/src/cache_payload.rs index 68acd55..305c338 100644 --- a/src/cache_payload.rs +++ b/src/cache_payload.rs @@ -2,6 +2,7 @@ use crate::cache::CacheError; #[cfg(test)] use std::cell::RefCell; use std::fs; +use std::io; use std::path::{Path, PathBuf}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -33,6 +34,69 @@ pub(crate) fn validate_payload_tree( measure_payload_tree(root, nodes, node_limit).map(|_| ()) } +pub(crate) fn copy_payload_tree( + source: &Path, + destination: &Path, + nodes: &mut usize, + node_limit: usize, +) -> Result<(), CacheError> { + let metadata = traced_symlink_metadata(source).map_err(CacheError::Io)?; + if metadata.file_type().is_symlink() { + return Err(CacheError::SymlinkInManagedRoot(source.to_path_buf())); + } + if !metadata.is_dir() { + return Err(CacheError::UnexpectedEntry(source.to_path_buf())); + } + match traced_symlink_metadata(destination) { + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(CacheError::Io(error)), + Ok(_) => return Err(CacheError::UnexpectedEntry(destination.to_path_buf())), + } + copy_payload_node(source, destination, nodes, node_limit) +} + +fn copy_payload_node( + source: &Path, + destination: &Path, + nodes: &mut usize, + node_limit: usize, +) -> Result<(), CacheError> { + *nodes = nodes + .checked_add(1) + .ok_or(CacheError::InventoryLimitExceeded)?; + if *nodes > node_limit { + return Err(CacheError::InventoryLimitExceeded); + } + + let metadata = traced_symlink_metadata(source).map_err(CacheError::Io)?; + if metadata.file_type().is_symlink() { + return recreate_payload_link(source, destination); + } + if metadata.is_file() { + fs::copy(source, destination).map_err(CacheError::Io)?; + return Ok(()); + } + if !metadata.is_dir() { + return Err(CacheError::UnexpectedEntry(source.to_path_buf())); + } + + fs::create_dir(destination).map_err(CacheError::Io)?; + let mut entries = traced_read_directory(source) + .map_err(CacheError::Io)? + .collect::, _>>() + .map_err(CacheError::Io)?; + entries.sort_by_key(|entry| entry.file_name()); + for entry in entries { + copy_payload_node( + &entry.path(), + &destination.join(entry.file_name()), + nodes, + node_limit, + )?; + } + Ok(()) +} + fn walk_payload( path: &Path, nodes: &mut usize, @@ -94,6 +158,31 @@ fn measure_symlink(path: &Path) -> Result { }) } +#[cfg(unix)] +fn recreate_payload_link(source: &Path, destination: &Path) -> Result<(), CacheError> { + use std::os::unix::fs::symlink; + + let target = + traced_read_link(source).map_err(|source_error| CacheError::PayloadSymlinkRead { + path: source.to_path_buf(), + source: source_error, + })?; + match traced_symlink_metadata(destination) { + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(CacheError::Io(error)), + Ok(_) => return Err(CacheError::UnexpectedEntry(destination.to_path_buf())), + } + symlink(&target, destination).map_err(|source_error| CacheError::PayloadSymlinkCreate { + path: destination.to_path_buf(), + source: source_error, + }) +} + +#[cfg(not(unix))] +fn recreate_payload_link(source: &Path, _destination: &Path) -> Result<(), CacheError> { + Err(CacheError::PayloadSymlinkUnsupported(source.to_path_buf())) +} + #[cfg(not(unix))] fn measure_symlink(path: &Path) -> Result { Err(CacheError::PayloadSymlinkUnsupported(path.to_path_buf())) @@ -246,6 +335,57 @@ mod tests { remove_fixture(&fixture, &outside); } + #[cfg(unix)] + #[test] + fn fallback_copy_preserves_each_link_target_and_external_sentinel() { + use std::os::unix::fs::symlink; + + let source = payload_fixture("copy-source"); + let destination = source + .parent() + .expect("fixture parent") + .join("copy-destination"); + let outside = source + .parent() + .expect("fixture parent") + .join("copy-sentinel"); + fs::write(source.join("regular"), b"payload").expect("write regular payload"); + fs::write(&outside, b"outside").expect("write external sentinel"); + symlink("regular", source.join("relative")).expect("create relative link"); + symlink("missing", source.join("broken")).expect("create broken link"); + symlink(&outside, source.join("absolute")).expect("create absolute link"); + symlink("self", source.join("self")).expect("create self link"); + + let mut nodes = 0; + copy_payload_tree(&source, &destination, &mut nodes, 100).expect("copy payload"); + + for name in ["relative", "broken", "absolute", "self"] { + assert_eq!( + fs::read_link(destination.join(name)).expect("read copied link"), + fs::read_link(source.join(name)).expect("read source link") + ); + } + assert_eq!( + fs::read(destination.join("regular")).expect("read copied payload"), + b"payload" + ); + assert_eq!( + fs::read(&outside).expect("read external sentinel"), + b"outside" + ); + remove_fixture(&source, &outside); + fs::remove_dir_all(destination).expect("remove copy destination"); + } + + #[cfg(not(unix))] + #[test] + fn payload_link_recreation_is_explicitly_unsupported() { + assert!(matches!( + recreate_payload_link(Path::new("source"), Path::new("destination")), + Err(CacheError::PayloadSymlinkUnsupported(_)) + )); + } + #[cfg(unix)] #[test] fn payload_root_itself_must_be_a_plain_directory() { diff --git a/src/lib.rs b/src/lib.rs index 5ca7465..969179d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,7 +16,6 @@ pub mod admission; pub mod agent_session; pub mod benchmark; pub mod cache; -#[allow(dead_code)] // Tasks 2-3 consume the private traversal surface. mod cache_payload; pub mod config; pub mod durable_fs; From e9faf9ed412d203e3bafafa262b124b637c94a9e Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 10:52:37 +0200 Subject: [PATCH 09/24] fix: own cache staging before fallible preparation --- src/cache.rs | 146 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 127 insertions(+), 19 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 75aac8e..8adaba0 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -22,7 +22,7 @@ use std::io::{self, Read, Write}; use std::path::{Component, Path, PathBuf}; use std::sync::{ Arc, - atomic::{AtomicU64, Ordering}, + atomic::{AtomicU8, AtomicU64, Ordering}, }; use std::thread; use std::time::Duration; @@ -54,6 +54,9 @@ const PROMOTION_SCHEMA_VERSION: &str = "1.0"; const MAX_INVENTORY_NODES: usize = 100_000; const INIT_RETRIES: usize = 40; const INIT_RETRY_DELAY: Duration = Duration::from_millis(5); +const PREPARED_PHASE_PREPARING: u8 = 0; +const PREPARED_PHASE_STAGING: u8 = 1; +const PREPARED_PHASE_PROMOTED: u8 = 2; static TEMP_SEQUENCE: AtomicU64 = AtomicU64::new(0); #[cfg(target_os = "macos")] @@ -428,6 +431,15 @@ impl ManagedCache { ensure_managed_directory(&staging_path)?; let data_path = staging_path.join("data"); ensure_managed_directory(&data_path)?; + let owner = Arc::new(PreparedCacheGenerationOwner { + entry_path: path.clone(), + staging_path: staging_path.clone(), + key_digest: key.digest.clone(), + plan_digest: plan_digest.to_owned(), + generation, + phase: AtomicU8::new(PREPARED_PHASE_PREPARING), + _entry_lock: entry_lock, + }); if complete { let source = path.join("data"); remove_if_present(&data_path)?; @@ -444,13 +456,7 @@ impl ManagedCache { state: "staging".to_owned(), }; write_generation_manifest(&staging_path, &manifest)?; - let owner = Arc::new(PreparedCacheGenerationOwner { - staging_path: staging_path.clone(), - key_digest: key.digest.clone(), - plan_digest: plan_digest.to_owned(), - generation, - _entry_lock: entry_lock, - }); + owner.phase.store(PREPARED_PHASE_STAGING, Ordering::Release); Ok(PreparedCacheEntry { path, data_path, @@ -588,6 +594,10 @@ impl ManagedCache { fs::rename(&prepared.data_path, ¤t).map_err(CacheError::Io)?; fs::rename(&manifest_path, &previous_manifest_path).map_err(CacheError::Io)?; write_complete_marker(&marker)?; + prepared + ._generation_owner + .phase + .store(PREPARED_PHASE_PROMOTED, Ordering::Release); } Ok(()) } @@ -936,28 +946,43 @@ pub struct PreparedCacheEntry { #[derive(Debug)] struct PreparedCacheGenerationOwner { + entry_path: PathBuf, staging_path: PathBuf, key_digest: String, plan_digest: String, generation: u64, + phase: AtomicU8, _entry_lock: Arc, } impl Drop for PreparedCacheGenerationOwner { fn drop(&mut self) { - let Ok(manifest) = - read_generation_manifest(&self.staging_path.join(GENERATION_MANIFEST_FILE)) - else { + let phase = self.phase.load(Ordering::Acquire); + if phase == PREPARED_PHASE_PROMOTED { return; - }; - if manifest.schema_version == GENERATION_SCHEMA_VERSION - && manifest.key_digest == self.key_digest - && manifest.plan_digest == self.plan_digest - && manifest.generation == self.generation - && manifest.state == "staging" - { - let _ = fs::remove_dir_all(&self.staging_path); } + if phase != PREPARED_PHASE_PREPARING && phase != PREPARED_PHASE_STAGING { + return; + } + if validate_owned_staging_root(&self.entry_path, &self.staging_path).is_err() { + return; + } + if phase == PREPARED_PHASE_STAGING { + let Ok(manifest) = + read_generation_manifest(&self.staging_path.join(GENERATION_MANIFEST_FILE)) + else { + return; + }; + if manifest.schema_version != GENERATION_SCHEMA_VERSION + || manifest.key_digest != self.key_digest + || manifest.plan_digest != self.plan_digest + || manifest.generation != self.generation + || manifest.state != "staging" + { + return; + } + } + let _ = remove_owned_generation_directory(&self.staging_path); } } @@ -1231,6 +1256,29 @@ fn remove_if_present(path: &Path) -> Result<(), CacheError> { } } +fn remove_owned_generation_directory(path: &Path) -> Result<(), CacheError> { + match fs::symlink_metadata(path) { + Ok(metadata) if metadata.file_type().is_symlink() => { + Err(CacheError::SymlinkInManagedRoot(path.to_path_buf())) + } + Ok(metadata) if metadata.is_dir() => fs::remove_dir_all(path).map_err(CacheError::Io), + Ok(_) => Err(CacheError::UnexpectedEntry(path.to_path_buf())), + Err(error) => Err(CacheError::Io(error)), + } +} + +fn validate_owned_staging_root(entry_path: &Path, staging_path: &Path) -> Result<(), CacheError> { + if staging_path.parent() != Some(entry_path) { + return Err(CacheError::GenerationMismatch); + } + let name = staging_path + .file_name() + .and_then(|name| name.to_str()) + .ok_or(CacheError::GenerationMismatch)?; + validate_owned_name(name, ".staging-")?; + validate_plain_directory(staging_path) +} + fn write_complete_marker(path: &Path) -> Result<(), CacheError> { let mut file = OpenOptions::new() .write(true) @@ -2364,6 +2412,66 @@ timeout_seconds = 60 clean(&repo); } + #[cfg(unix)] + #[test] + fn failed_payload_preflight_removes_the_new_staging_generation() { + use std::os::unix::{fs::symlink, net::UnixListener}; + + let fixture = completed_entry_fixture("failed-payload-preflight-cleanup"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); + let socket_path = fixture.data_path.join("unsupported.socket"); + let socket_parent = PathBuf::from("/private/tmp").join(format!( + "ccp-socket-{}-{}", + std::process::id(), + TEMP_SEQUENCE.fetch_add(1, Ordering::Relaxed) + )); + symlink(&fixture.data_path, &socket_parent).unwrap(); + let listener = UnixListener::bind(socket_parent.join("unsupported.socket")).unwrap(); + + let preparation = fixture.cache.prepare_entry(&key, &plan.plan_digest, 2); + let staging: Vec<_> = fs::read_dir(&fixture.entry_path) + .unwrap() + .flatten() + .filter(|entry| entry.file_name().to_string_lossy().starts_with(".staging-")) + .collect(); + drop(listener); + fs::remove_file(socket_path).unwrap(); + fs::remove_file(socket_parent).unwrap(); + finish_fixture(fixture); + + assert!(matches!(preparation, Err(CacheError::UnexpectedEntry(_)))); + assert!(staging.is_empty(), "failed preparation leaked {staging:?}"); + } + + #[cfg(unix)] + #[test] + fn staging_cleanup_unlinks_payload_links_without_touching_targets() { + use std::os::unix::fs::symlink; + + let (repo, resolved) = resolved_fixture("staging-cleanup-links"); + let cache = ManagedCache::initialize(resolved.clone()).unwrap(); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); + let outside = repo.join("cleanup-sentinel"); + fs::create_dir(&outside).unwrap(); + fs::write(outside.join("first"), b"first").unwrap(); + fs::write(outside.join("second"), b"second").unwrap(); + + let prepared = cache.prepare_entry(&key, &plan.plan_digest, 1).unwrap(); + fs::create_dir(prepared.data_path.join("nested")).unwrap(); + symlink(&outside, prepared.data_path.join("external")).unwrap(); + symlink(&outside, prepared.data_path.join("nested/external")).unwrap(); + let staging = prepared.staging_path.clone(); + drop(prepared); + + assert!(!staging.exists()); + assert_eq!(fs::read(outside.join("first")).unwrap(), b"first"); + assert_eq!(fs::read(outside.join("second")).unwrap(), b"second"); + clean(&resolved.path); + clean(&repo); + } + #[test] fn active_entry_lock_blocks_a_second_preparation_until_release() { let (repo, resolved) = resolved_fixture("entry-lock"); From b260dcd5ad3f0277a647a8d5bbd8cc0996b2d7bd Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 11:02:10 +0200 Subject: [PATCH 10/24] fix: validate opaque payloads before cache promotion --- src/cache.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/cache.rs b/src/cache.rs index 8adaba0..89abd3e 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -569,6 +569,8 @@ impl ManagedCache { { return Err(CacheError::GenerationMismatch); } + let mut nodes = 0; + validate_payload_tree(&prepared.data_path, &mut nodes, MAX_INVENTORY_NODES)?; Ok(()) } @@ -684,6 +686,8 @@ impl ManagedCache { let entry = self.entry_path(key); validate_plain_directory(&entry)?; validate_plain_directory(&entry.join("data"))?; + let mut nodes = 0; + validate_payload_tree(&entry.join("data"), &mut nodes, MAX_INVENTORY_NODES)?; let marker = entry.join(COMPLETE_FILE); match fs::symlink_metadata(&marker) { Ok(metadata) if metadata.file_type().is_symlink() => { @@ -2328,6 +2332,40 @@ timeout_seconds = 60 clean(&repo); } + #[cfg(unix)] + #[test] + fn promotion_rejects_an_unsupported_payload_object_before_journaling() { + use std::os::unix::{fs::symlink, net::UnixListener}; + + let (repo, resolved) = resolved_fixture("promotion-payload-object"); + let cache = ManagedCache::initialize(resolved.clone()).unwrap(); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); + let prepared = cache.prepare_entry(&key, &plan.plan_digest, 1).unwrap(); + let socket_path = prepared.data_path.join("socket"); + let socket_parent = PathBuf::from("/private/tmp").join(format!( + "ccp-socket-{}-{}", + std::process::id(), + TEMP_SEQUENCE.fetch_add(1, Ordering::Relaxed) + )); + symlink(&prepared.data_path, &socket_parent).unwrap(); + let listener = UnixListener::bind(socket_parent.join("socket")).unwrap(); + + assert!(matches!( + cache.promote_entry(&prepared), + Err(CacheError::UnexpectedEntry(_)) + )); + assert!(!resolved.path.join(PROMOTION_JOURNAL_FILE).exists()); + assert!(!cache.entry_path(&key).join(COMPLETE_FILE).exists()); + + drop(listener); + fs::remove_file(socket_path).unwrap(); + fs::remove_file(socket_parent).unwrap(); + drop(prepared); + clean(&resolved.path); + clean(&repo); + } + #[test] fn failed_generation_does_not_mutate_last_known_good() { let (repo, resolved) = resolved_fixture("complete-entry-mutation"); @@ -2571,6 +2609,9 @@ timeout_seconds = 60 .expect("second"); fs::write(first.data_path.join("first"), b"one").expect("first data"); fs::write(second.data_path.join("second"), b"two").expect("second data"); + #[cfg(unix)] + std::os::unix::fs::symlink("second", second.data_path.join("relative")) + .expect("relative payload link"); cache .promote_entries(&[first, second]) .expect("promote both"); @@ -2584,6 +2625,21 @@ timeout_seconds = 60 fs::read(cache.entry_data_path(&second_key).join("second")).expect("second current"), b"two" ); + #[cfg(unix)] + { + let relative = cache.entry_data_path(&second_key).join("relative"); + assert!( + fs::symlink_metadata(&relative) + .expect("relative metadata") + .file_type() + .is_symlink(), + "promoted payload link must remain a link" + ); + assert_eq!( + fs::read_link(relative).expect("read promoted relative link"), + Path::new("second") + ); + } clean(&resolved.path); clean(&repo); } @@ -2597,6 +2653,14 @@ timeout_seconds = 60 let prepared = cache .prepare_entry(&key, &envelope.plan_digest, 1) .expect("prepare"); + #[cfg(unix)] + let outside = { + let outside = repo.join("recovery-sentinel"); + fs::write(&outside, b"recovery").expect("write recovery sentinel"); + std::os::unix::fs::symlink(&outside, prepared.data_path.join("external")) + .expect("external recovery link"); + outside + }; let journal = cache .create_promotion_journal(std::slice::from_ref(&prepared)) .expect("journal"); @@ -2607,6 +2671,11 @@ timeout_seconds = 60 assert!(!resolved.path.join(PROMOTION_JOURNAL_FILE).exists()); assert!(!cache.entry_path(&key).join("data").exists()); + #[cfg(unix)] + assert_eq!( + fs::read(outside).expect("read recovery sentinel"), + b"recovery" + ); assert_eq!(journal.entries.len(), 1); clean(&resolved.path); clean(&repo); From 0500ebe368ab339b85f9da06211b710b53749bc6 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 11:09:30 +0200 Subject: [PATCH 11/24] docs: define opaque cache payload link boundary --- CHANGELOG.md | 2 ++ docs/CACHE_AND_WORKSPACE.md | 14 ++++++++++++++ docs/TESTING_AND_FAULT_INJECTION.md | 6 ++++++ docs/THREAT_MODEL.md | 4 ++-- tests/repository_hygiene_contract.rs | 27 +++++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11797f4..770abcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ Semantic Versioning after its first public release. ### Added +- Unix cache generations now preserve ordinary opaque payload links during inventory and reuse; strict control paths remain link-free, failed preparation owns cleanup before reuse, and native candidate qualification remains pending. + - Shared terminal owned-resource finalization now routes historical and matrix runs, benchmarks, and guarded execution through completion-before-release, exactly-once admission release, and fail-closed release-error precedence. diff --git a/docs/CACHE_AND_WORKSPACE.md b/docs/CACHE_AND_WORKSPACE.md index 9beaa6e..f66c08c 100644 --- a/docs/CACHE_AND_WORKSPACE.md +++ b/docs/CACHE_AND_WORKSPACE.md @@ -104,6 +104,20 @@ The default reporting budget is 20 GiB and can be overridden with `--disk-budget-bytes`. Exceeding the budget is reported; it does not trigger automatic eviction. +## Control plane and opaque payload links + +The cache has a strict control plane and an opaque payload plane. Control-plane +paths remain link-free and fail closed. Inventory counts a link's stored target +length as bytes, never target content, never follows a payload link target on +the host, and retains the 100,000-node bound. Payload inspection covers +relative, absolute, broken, recursive, and outside-root links. CCP never follows a payload link target on the host. + +Unix reuse preserves these opaque links. Windows link-bearing payload reuse remains unsupported and fails closed. A payload link is counted as one node and one non-directory object. Cache payloads remain mutable, unattested performance +state, not trusted content. Standard-library path traversal is qualified under +CCP's cooperative entry-lock/trusted-local-actor model; a non-cooperative local +actor concurrently replacing a checked path remains unsupported and is not +claimed prevented. + ## Schema 1.2 capacity preflight The inventory budget remains reporting-only. A schema `1.2` `[storage]` policy diff --git a/docs/TESTING_AND_FAULT_INJECTION.md b/docs/TESTING_AND_FAULT_INJECTION.md index 173dc0c..8cc690e 100644 --- a/docs/TESTING_AND_FAULT_INJECTION.md +++ b/docs/TESTING_AND_FAULT_INJECTION.md @@ -93,6 +93,12 @@ containment boundaries only; they do not prove a real admission root, host cleanup, a published receipt, or another platform. Process lists do not prove release. +## Opaque cache-payload symbolic links + +Deterministic coverage includes `src/cache.rs::complete_payload_symlinks_are_preserved_across_generation_reuse`, `src/cache.rs::failed_payload_preflight_removes_the_new_staging_generation`, and `src/cache.rs::staging_cleanup_unlinks_payload_links_without_touching_targets`. Payload measurement is covered by `src/cache_payload.rs::payload_measurement_counts_links_without_following_targets`; fallback semantics by `src/cache_payload.rs::fallback_copy_preserves_each_link_target_and_external_sentinel`. + +On macOS, clone-based reuse preserves ordinary opaque payload links; Unix fallback copy preserves each link target without following it. Windows link-bearing payload reuse remains fail-closed and unsupported. These are deterministic source tests, not a native CCP receipt or native qualification. + ## Managed-cache pin contract The managed-cache pin tests are deterministic contract tests over an owned diff --git a/docs/THREAT_MODEL.md b/docs/THREAT_MODEL.md index 71fcf39..c26549b 100644 --- a/docs/THREAT_MODEL.md +++ b/docs/THREAT_MODEL.md @@ -96,13 +96,13 @@ Boundary rules: | T12 | Workflow privilege escalation | Minimal permissions, pinned official actions, no secrets/cache/deployment credential | Compromise of GitHub or a pinned action commit remains upstream risk | | T13 | Marketplace action execution during migration | Migration assistant parses bounded YAML as data and emits inert classifications | Human reviewers can still make a bad manual translation | | T14 | GitHub expression or secret misinterpretation | Unsupported expressions, permissions, secrets, reusable workflows, and arbitrary actions fail closed or require review | Compatibility is deliberately incomplete | -| T15 | Cache poisoning or cache-use race | Versioned ownership marker, content-addressed keys, completion marker, active-run lock, opt-in managed-cache pin with spawn-boundary revalidation | Caches accelerate execution but are not attestation evidence; undeclared paths are not pinned | +| T15 | Cache poisoning or cache-use race | Versioned ownership marker, content-addressed keys, completion marker, active-run lock, opt-in managed-cache pin with spawn-boundary revalidation; opaque payload links are separated from link-free control paths | Caches are mutable, unattested performance state; undeclared paths are not pinned, and payload links are not trusted content | | T16 | Destructive cleanup | 0.1.0 exposes preview-only cleanup; resolved-root and containment checks | Operators retain responsibility for manual filesystem deletion | | T17 | Image drift | OCI digest is mandatory and included in plan, receipt, and policy | A multi-platform index can resolve to different platform manifests by design | | T18 | Dependency compromise | Committed lockfile, exact critical pins, SPDX SBOM, bundled notices, advisory review | Registry and compiler compromise cannot be eliminated locally | | T19 | Platform overclaim | Native receipts name OS/architecture; emulation and runtime probes are separate; PASS/PENDING/NOT_RUN are explicit | Benchmark qualification is narrower than full runtime qualification | | T20 | Identity overclaim | Structural, integrity, policy, and identity levels are separate; identity is not implemented | No cryptographic proof of operator or machine exists in 0.1.0 | -| T21 | Symlink or filesystem race | Canonical path checks, managed roots, create-new/atomic writes, runtime revalidation, existing advisory-lock pin held through the guarded child lifecycle | Cooperative mutators must use the same lock and revalidate after acquisition; manual deletion remains unsupported; privileged local actors remain trusted | +| T21 | Symlink or filesystem race | Canonical path checks, managed roots, create-new/atomic writes, runtime revalidation, existing advisory-lock pin held through the guarded child lifecycle; control-plane paths remain link-free while payload links are opaque | A containerized project process can resolve payload links in its mount namespace; CCP does not claim hostile-code sandboxing or cache-content trust. Standard-library path traversal is qualified under the cooperative entry-lock/trusted-local-actor model; a non-cooperative local actor concurrently replacing a checked path remains unsupported and is not claimed prevented | | T22 | Evidence parser denial of service | One MiB remote input cap, strict unknown-field rejection, bounded summaries | Base verifier compilation still consumes bounded remote time | | T23 | Release substitution | Local SHA-256 manifest, SBOM, notices, checksum verification instructions | Checksums are not signatures and must come through an independent channel | | T24 | Unsafe upgrade or rollback | Isolated install, version smoke test, preserved previous binary, versioned schemas and cache markers | Operator mistakes remain possible; no automatic updater exists | diff --git a/tests/repository_hygiene_contract.rs b/tests/repository_hygiene_contract.rs index 3f855d1..1a105b2 100644 --- a/tests/repository_hygiene_contract.rs +++ b/tests/repository_hygiene_contract.rs @@ -26,6 +26,33 @@ const ISSUE_CONFIG: &str = include_str!("../.github/ISSUE_TEMPLATE/config.yml"); const PR_TEMPLATE: &str = include_str!("../.github/PULL_REQUEST_TEMPLATE.md"); const ROADMAP: &str = include_str!("../ROADMAP.md"); const SOCIAL_PREVIEW: &str = include_str!("../docs/assets/social-preview.svg"); +const CACHE_AND_WORKSPACE: &str = include_str!("../docs/CACHE_AND_WORKSPACE.md"); +const TESTING_AND_FAULT_INJECTION: &str = + include_str!("../docs/TESTING_AND_FAULT_INJECTION.md"); + +#[test] +fn cache_payload_symlinks_are_documented_as_opaque_unattested_state() { + for phrase in [ + "control plane", + "payload plane", + "never follows a payload link target on the host", + "relative, absolute, broken, recursive, and outside-root", + "Windows link-bearing payload reuse remains unsupported", + "one node and one non-directory object", + ] { + assert!(CACHE_AND_WORKSPACE.contains(phrase), "missing {phrase}"); + } + for reference in [ + "src/cache.rs::complete_payload_symlinks_are_preserved_across_generation_reuse", + "src/cache.rs::failed_payload_preflight_removes_the_new_staging_generation", + "src/cache.rs::staging_cleanup_unlinks_payload_links_without_touching_targets", + ] { + assert!( + TESTING_AND_FAULT_INJECTION.contains(reference), + "missing {reference}" + ); + } +} #[test] fn matrix_legacy_profile_is_documented_without_production_digest_constants() { From f03734d4cb21a1864f277a695712b06da4032e77 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 11:16:28 +0200 Subject: [PATCH 12/24] docs: pin control-plane symlink boundary --- docs/CACHE_AND_WORKSPACE.md | 8 ++++---- tests/repository_hygiene_contract.rs | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/CACHE_AND_WORKSPACE.md b/docs/CACHE_AND_WORKSPACE.md index f66c08c..2a297d9 100644 --- a/docs/CACHE_AND_WORKSPACE.md +++ b/docs/CACHE_AND_WORKSPACE.md @@ -98,16 +98,16 @@ markers are reported as incomplete; they are never silently promoted. commit-ci-preflight cache inventory --json ``` -Inventory validates ownership again, rejects symlinks and unexpected entry -names, walks at most 100,000 nodes, and emits a deterministically sorted report. +Inventory validates ownership again, rejects control-plane and payload-root +symlinks and unexpected entry names, walks at most 100,000 nodes, and emits a +deterministically sorted report. The default reporting budget is 20 GiB and can be overridden with `--disk-budget-bytes`. Exceeding the budget is reported; it does not trigger automatic eviction. ## Control plane and opaque payload links -The cache has a strict control plane and an opaque payload plane. Control-plane -paths remain link-free and fail closed. Inventory counts a link's stored target +The cache has a strict control plane and an opaque payload plane. Control-plane and payload-root links still fail closed; permitted payload-descendant links remain opaque. In other words, control-plane and payload-root links still fail closed. Inventory counts a link's stored target length as bytes, never target content, never follows a payload link target on the host, and retains the 100,000-node bound. Payload inspection covers relative, absolute, broken, recursive, and outside-root links. CCP never follows a payload link target on the host. diff --git a/tests/repository_hygiene_contract.rs b/tests/repository_hygiene_contract.rs index 1a105b2..420ad27 100644 --- a/tests/repository_hygiene_contract.rs +++ b/tests/repository_hygiene_contract.rs @@ -27,11 +27,10 @@ const PR_TEMPLATE: &str = include_str!("../.github/PULL_REQUEST_TEMPLATE.md"); const ROADMAP: &str = include_str!("../ROADMAP.md"); const SOCIAL_PREVIEW: &str = include_str!("../docs/assets/social-preview.svg"); const CACHE_AND_WORKSPACE: &str = include_str!("../docs/CACHE_AND_WORKSPACE.md"); -const TESTING_AND_FAULT_INJECTION: &str = - include_str!("../docs/TESTING_AND_FAULT_INJECTION.md"); +const TESTING_AND_FAULT_INJECTION: &str = include_str!("../docs/TESTING_AND_FAULT_INJECTION.md"); #[test] -fn cache_payload_symlinks_are_documented_as_opaque_unattested_state() { +fn cache_payload_documentation_contract() { for phrase in [ "control plane", "payload plane", @@ -39,6 +38,7 @@ fn cache_payload_symlinks_are_documented_as_opaque_unattested_state() { "relative, absolute, broken, recursive, and outside-root", "Windows link-bearing payload reuse remains unsupported", "one node and one non-directory object", + "control-plane and payload-root links still fail closed", ] { assert!(CACHE_AND_WORKSPACE.contains(phrase), "missing {phrase}"); } From d3e0d374f452e36979325c931f8788ad829889b5 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 11:18:18 +0200 Subject: [PATCH 13/24] docs: remove duplicate symlink boundary wording --- docs/CACHE_AND_WORKSPACE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/CACHE_AND_WORKSPACE.md b/docs/CACHE_AND_WORKSPACE.md index 2a297d9..fcb9f1f 100644 --- a/docs/CACHE_AND_WORKSPACE.md +++ b/docs/CACHE_AND_WORKSPACE.md @@ -107,7 +107,7 @@ automatic eviction. ## Control plane and opaque payload links -The cache has a strict control plane and an opaque payload plane. Control-plane and payload-root links still fail closed; permitted payload-descendant links remain opaque. In other words, control-plane and payload-root links still fail closed. Inventory counts a link's stored target +The cache has a strict control plane and an opaque payload plane. control-plane and payload-root links still fail closed; permitted payload-descendant links remain opaque. Inventory counts a link's stored target length as bytes, never target content, never follows a payload link target on the host, and retains the 100,000-node bound. Payload inspection covers relative, absolute, broken, recursive, and outside-root links. CCP never follows a payload link target on the host. From a6ece7b723e94d010bd7f7f8b6fce4d64abe1ccc Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 11:19:49 +0200 Subject: [PATCH 14/24] docs: clarify payload link boundary grammar --- docs/CACHE_AND_WORKSPACE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/CACHE_AND_WORKSPACE.md b/docs/CACHE_AND_WORKSPACE.md index fcb9f1f..7c4b78c 100644 --- a/docs/CACHE_AND_WORKSPACE.md +++ b/docs/CACHE_AND_WORKSPACE.md @@ -107,7 +107,7 @@ automatic eviction. ## Control plane and opaque payload links -The cache has a strict control plane and an opaque payload plane. control-plane and payload-root links still fail closed; permitted payload-descendant links remain opaque. Inventory counts a link's stored target +The cache has a strict control plane and an opaque payload plane; control-plane and payload-root links still fail closed, while permitted payload-descendant links remain opaque. Inventory counts a link's stored target length as bytes, never target content, never follows a payload link target on the host, and retains the 100,000-node bound. Payload inspection covers relative, absolute, broken, recursive, and outside-root links. CCP never follows a payload link target on the host. From dff004488644e80db6ef267f304db52533600b95 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 11:25:51 +0200 Subject: [PATCH 15/24] docs: remove cache payload spec trailing whitespace --- .../specs/2026-08-29-cache-payload-symlink-design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md b/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md index a64fa59..01e1d8e 100644 --- a/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md +++ b/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md @@ -1,6 +1,6 @@ # Opaque cache-payload symbolic-link design -Status: approved by owner on 2026-08-29 +Status: approved by owner on 2026-08-29 Date: 2026-08-29 Baseline: `820a7fa6ce83a7ac8593c2800f8be4f44ab82ebc` Scope: make completed cache generations reusable when ordinary package-manager From 07f2963ea14e39887d5bea036982a32db51cce88 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 11:29:03 +0200 Subject: [PATCH 16/24] test: shorten payload socket fixture path --- src/cache_payload.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/cache_payload.rs b/src/cache_payload.rs index 305c338..e6cdb14 100644 --- a/src/cache_payload.rs +++ b/src/cache_payload.rs @@ -411,11 +411,19 @@ mod tests { #[cfg(unix)] #[test] fn unsupported_payload_object_fails_without_traversal() { - use std::os::unix::net::UnixListener; + use std::os::unix::{fs::symlink, net::UnixListener}; let fixture = payload_fixture("unsupported-object"); let socket = fixture.join("listener.socket"); - let listener = UnixListener::bind(&socket).expect("bind listener"); + let socket_parent = PathBuf::from("/private/tmp").join(format!( + "ccp-payload-socket-{}-{}", + std::process::id(), + TEST_SEQUENCE.fetch_add(1, Ordering::Relaxed) + )); + let _ = fs::remove_file(&socket_parent); + symlink(&fixture, &socket_parent).expect("link short socket parent"); + let listener = + UnixListener::bind(socket_parent.join("listener.socket")).expect("bind listener"); clear_payload_operations(); let mut nodes = 0; assert!(matches!( @@ -432,6 +440,7 @@ mod tests { ); drop(listener); fs::remove_file(socket).expect("remove listener socket"); + fs::remove_file(socket_parent).expect("remove short socket parent"); fs::remove_dir(fixture).expect("remove fixture"); } From 4223288d1ed2387c7867696cc874206a2cba6b12 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 11:30:55 +0200 Subject: [PATCH 17/24] docs: retain cache deletion contract --- docs/THREAT_MODEL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/THREAT_MODEL.md b/docs/THREAT_MODEL.md index c26549b..f76d64c 100644 --- a/docs/THREAT_MODEL.md +++ b/docs/THREAT_MODEL.md @@ -97,7 +97,7 @@ Boundary rules: | T13 | Marketplace action execution during migration | Migration assistant parses bounded YAML as data and emits inert classifications | Human reviewers can still make a bad manual translation | | T14 | GitHub expression or secret misinterpretation | Unsupported expressions, permissions, secrets, reusable workflows, and arbitrary actions fail closed or require review | Compatibility is deliberately incomplete | | T15 | Cache poisoning or cache-use race | Versioned ownership marker, content-addressed keys, completion marker, active-run lock, opt-in managed-cache pin with spawn-boundary revalidation; opaque payload links are separated from link-free control paths | Caches are mutable, unattested performance state; undeclared paths are not pinned, and payload links are not trusted content | -| T16 | Destructive cleanup | 0.1.0 exposes preview-only cleanup; resolved-root and containment checks | Operators retain responsibility for manual filesystem deletion | +| T16 | Destructive cleanup | 0.1.0 exposes preview-only cleanup; resolved-root and containment checks | Operators retain responsibility for manual filesystem deletion; manual deletion remains unsupported | | T17 | Image drift | OCI digest is mandatory and included in plan, receipt, and policy | A multi-platform index can resolve to different platform manifests by design | | T18 | Dependency compromise | Committed lockfile, exact critical pins, SPDX SBOM, bundled notices, advisory review | Registry and compiler compromise cannot be eliminated locally | | T19 | Platform overclaim | Native receipts name OS/architecture; emulation and runtime probes are separate; PASS/PENDING/NOT_RUN are explicit | Benchmark qualification is narrower than full runtime qualification | From 0dc8e6f28bea877c9e848e264a4b1bec5c9f225e Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 11:50:06 +0200 Subject: [PATCH 18/24] test: own short Unix socket fixture directory --- src/cache_payload.rs | 66 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/src/cache_payload.rs b/src/cache_payload.rs index e6cdb14..82e9632 100644 --- a/src/cache_payload.rs +++ b/src/cache_payload.rs @@ -253,6 +253,49 @@ mod tests { static TEST_SEQUENCE: AtomicU64 = AtomicU64::new(0); + #[cfg(unix)] + const SHORT_SOCKET_DIRECTORY_RETRIES: usize = 16; + + #[cfg(unix)] + struct ShortSocketFixtureDirectory { + path: PathBuf, + } + + #[cfg(unix)] + impl ShortSocketFixtureDirectory { + fn path(&self) -> &Path { + &self.path + } + } + + #[cfg(unix)] + impl Drop for ShortSocketFixtureDirectory { + fn drop(&mut self) { + match fs::symlink_metadata(&self.path) { + Ok(metadata) if metadata.is_dir() && !metadata.file_type().is_symlink() => { + let _ = fs::remove_dir_all(&self.path); + } + Ok(_) | Err(_) => {} + } + } + } + + #[cfg(unix)] + fn short_socket_fixture_directory() -> ShortSocketFixtureDirectory { + let process_id = std::process::id(); + let sequence = TEST_SEQUENCE.fetch_add(1, Ordering::Relaxed); + for attempt in 0..SHORT_SOCKET_DIRECTORY_RETRIES { + let path = + PathBuf::from("/tmp").join(format!("ccp-ps-{process_id}-{sequence}-{attempt}")); + match fs::create_dir(&path) { + Ok(()) => return ShortSocketFixtureDirectory { path }, + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => continue, + Err(error) => panic!("create short socket fixture directory: {error}"), + } + } + panic!("claim unique short socket fixture directory") + } + fn payload_fixture(name: &str) -> PathBuf { let base = std::env::var_os("CCP_TEST_ROOT") .map(PathBuf::from) @@ -408,6 +451,19 @@ mod tests { fs::remove_dir(real).expect("remove fixture"); } + #[cfg(unix)] + #[test] + fn short_socket_fixture_directory_removes_only_its_owned_path() { + let directory = short_socket_fixture_directory(); + let path = directory.path().to_path_buf(); + let created = path.join("created-by-test"); + fs::write(&created, b"owned").expect("write owned fixture child"); + + drop(directory); + + assert!(!path.exists()); + } + #[cfg(unix)] #[test] fn unsupported_payload_object_fails_without_traversal() { @@ -415,12 +471,8 @@ mod tests { let fixture = payload_fixture("unsupported-object"); let socket = fixture.join("listener.socket"); - let socket_parent = PathBuf::from("/private/tmp").join(format!( - "ccp-payload-socket-{}-{}", - std::process::id(), - TEST_SEQUENCE.fetch_add(1, Ordering::Relaxed) - )); - let _ = fs::remove_file(&socket_parent); + let socket_directory = short_socket_fixture_directory(); + let socket_parent = socket_directory.path().join("fixture"); symlink(&fixture, &socket_parent).expect("link short socket parent"); let listener = UnixListener::bind(socket_parent.join("listener.socket")).expect("bind listener"); @@ -440,7 +492,7 @@ mod tests { ); drop(listener); fs::remove_file(socket).expect("remove listener socket"); - fs::remove_file(socket_parent).expect("remove short socket parent"); + drop(socket_directory); fs::remove_dir(fixture).expect("remove fixture"); } From dbc9869b8f52afe814e331a02e36fde80fcd2281 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 12:15:09 +0200 Subject: [PATCH 19/24] fix: harden cache payload review findings --- docs/TESTING_AND_FAULT_INJECTION.md | 4 +- src/cache.rs | 249 ++++++++++++++++++++++++++-- src/cache_payload.rs | 115 ++++++++++++- 3 files changed, 342 insertions(+), 26 deletions(-) diff --git a/docs/TESTING_AND_FAULT_INJECTION.md b/docs/TESTING_AND_FAULT_INJECTION.md index 8cc690e..6b4e13b 100644 --- a/docs/TESTING_AND_FAULT_INJECTION.md +++ b/docs/TESTING_AND_FAULT_INJECTION.md @@ -95,9 +95,9 @@ release. ## Opaque cache-payload symbolic links -Deterministic coverage includes `src/cache.rs::complete_payload_symlinks_are_preserved_across_generation_reuse`, `src/cache.rs::failed_payload_preflight_removes_the_new_staging_generation`, and `src/cache.rs::staging_cleanup_unlinks_payload_links_without_touching_targets`. Payload measurement is covered by `src/cache_payload.rs::payload_measurement_counts_links_without_following_targets`; fallback semantics by `src/cache_payload.rs::fallback_copy_preserves_each_link_target_and_external_sentinel`. +Deterministic coverage includes `src/cache.rs::complete_payload_symlinks_are_preserved_across_generation_reuse`, `src/cache.rs::failed_payload_preflight_removes_the_new_staging_generation`, `src/cache.rs::forced_fallback_copy_failure_removes_only_its_owned_staging_generation`, and `src/cache.rs::staging_cleanup_unlinks_payload_links_without_touching_targets`. Payload measurement is covered by `src/cache_payload.rs::payload_measurement_counts_links_without_following_targets`; fallback semantics and complete operation tracing by `src/cache_payload.rs::fallback_copy_preserves_each_link_target_and_external_sentinel` and `src/cache_payload.rs::fallback_copy_records_only_payload_paths_and_every_copy_operation`. -On macOS, clone-based reuse preserves ordinary opaque payload links; Unix fallback copy preserves each link target without following it. Windows link-bearing payload reuse remains fail-closed and unsupported. These are deterministic source tests, not a native CCP receipt or native qualification. +On macOS, a test-only seam distinguishes an attempted clone from a deliberately forced fallback; the fallback preserves an equivalent link-bearing payload. Injected fallback-copy failure proves exact owned-staging cleanup. Unix fallback copy preserves each link target without following it and records metadata, directory enumeration, link reads, regular-file copies, directory creation, and link creation without treating a link target as a host path. Windows link-bearing payload reuse remains fail-closed and unsupported. These are deterministic source tests, not a native CCP receipt or native qualification. ## Managed-cache pin contract diff --git a/src/cache.rs b/src/cache.rs index 89abd3e..2f2a9a1 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -35,6 +35,8 @@ use crate::cache_payload::{copy_payload_tree, validate_payload_tree}; use crate::config::{ExecutionPlanEnvelopeV1, NormalizedCache}; use crate::durable_fs::{DurableFileSystem, DurableFsError}; use crate::receipt::{ReceiptError, canonical_json}; +#[cfg(test)] +use std::cell::Cell; pub const DEFAULT_DISK_BUDGET_BYTES: u64 = 20 * 1024 * 1024 * 1024; const OWNER_FILE: &str = ".ccp-cache-root-v1.json"; @@ -744,6 +746,14 @@ impl ManagedCache { if !is_cache_directory_name(&name) && !name.starts_with(".entry-tmp-") { return Err(CacheError::UnexpectedEntry(entry.path())); } + // A valid entry can be changed by its legitimate owner while it is + // prepared. Inventory must take the same cooperative lock before + // inspecting either status or payload descendants. + let _entry_lock = if is_cache_directory_name(&name) { + Some(acquire_existing_entry_lock(&entry.path())?) + } else { + None + }; let mut nodes = 0; let (bytes, files) = bounded_entry_size(&entry.path(), &mut nodes)?; let status = entry_status(&entry.path(), &name)?; @@ -1296,8 +1306,15 @@ fn write_complete_marker(path: &Path) -> Result<(), CacheError> { fn try_clone_tree(source: &Path, destination: &Path) -> Result { let mut nodes = 0; validate_payload_tree(source, &mut nodes, MAX_INVENTORY_NODES)?; + #[cfg(test)] + if take_forced_clone_fallback_for_test() { + record_clone_operation_for_test(CloneOperation::ForcedFallback); + return Ok(false); + } #[cfg(target_os = "macos")] { + #[cfg(test)] + record_clone_operation_for_test(CloneOperation::Attempted); let source = CString::new( source .to_str() @@ -1330,6 +1347,46 @@ fn try_clone_tree(source: &Path, destination: &Path) -> Result } } +#[cfg(test)] +thread_local! { + static FORCE_CLONE_FALLBACK: Cell = const { Cell::new(false) }; + static CLONE_OPERATIONS: std::cell::RefCell> = const { + std::cell::RefCell::new(Vec::new()) + }; +} + +#[cfg(test)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CloneOperation { + Attempted, + ForcedFallback, +} + +#[cfg(test)] +fn force_clone_fallback_for_test() { + FORCE_CLONE_FALLBACK.with(|forced| forced.set(true)); +} + +#[cfg(test)] +fn take_forced_clone_fallback_for_test() -> bool { + FORCE_CLONE_FALLBACK.with(|forced| forced.replace(false)) +} + +#[cfg(test)] +fn record_clone_operation_for_test(operation: CloneOperation) { + CLONE_OPERATIONS.with(|operations| operations.borrow_mut().push(operation)); +} + +#[cfg(test)] +fn clear_clone_operations_for_test() { + CLONE_OPERATIONS.with(|operations| operations.borrow_mut().clear()); +} + +#[cfg(test)] +fn take_clone_operations_for_test() -> Vec { + CLONE_OPERATIONS.with(|operations| std::mem::take(&mut *operations.borrow_mut())) +} + fn wait_for_concurrent_initializer(root: &Path, marker: &Path) -> Result<(), CacheError> { for _ in 0..INIT_RETRIES { if marker.exists() { @@ -1690,16 +1747,25 @@ fn bounded_entry_size_at( } fn is_payload_root(entry_root: &Path, candidate: &Path) -> bool { + let Some(entry_name) = entry_root.file_name().and_then(|name| name.to_str()) else { + return false; + }; + if !is_cache_directory_name(entry_name) { + return false; + } let Ok(relative) = candidate.strip_prefix(entry_root) else { return false; }; let components: Vec<_> = relative.components().collect(); match components.as_slice() { - [Component::Normal(data)] => *data == "data", + [Component::Normal(data)] => data.to_str() == Some("data"), [Component::Normal(generation), Component::Normal(data)] => { - let generation = generation.to_string_lossy(); - *data == "data" - && (generation.starts_with(".staging-") || generation.starts_with(".backup-")) + let Some(generation) = generation.to_str() else { + return false; + }; + data.to_str() == Some("data") + && (validate_owned_name(generation, ".staging-").is_ok() + || validate_owned_name(generation, ".backup-").is_ok()) } _ => false, } @@ -1876,6 +1942,49 @@ mod tests { use crate::config::ConfigV1; use std::sync::{Arc, Barrier}; + #[cfg(unix)] + const SHORT_SOCKET_DIRECTORY_RETRIES: usize = 16; + + #[cfg(unix)] + struct ShortSocketFixtureDirectory { + path: PathBuf, + } + + #[cfg(unix)] + impl ShortSocketFixtureDirectory { + fn path(&self) -> &Path { + &self.path + } + } + + #[cfg(unix)] + impl Drop for ShortSocketFixtureDirectory { + fn drop(&mut self) { + match fs::symlink_metadata(&self.path) { + Ok(metadata) if metadata.is_dir() && !metadata.file_type().is_symlink() => { + let _ = fs::remove_dir_all(&self.path); + } + Ok(_) | Err(_) => {} + } + } + } + + #[cfg(unix)] + fn short_socket_fixture_directory() -> ShortSocketFixtureDirectory { + let process_id = std::process::id(); + let sequence = TEMP_SEQUENCE.fetch_add(1, Ordering::Relaxed); + for attempt in 0..SHORT_SOCKET_DIRECTORY_RETRIES { + let path = + PathBuf::from("/tmp").join(format!("ccp-cs-{process_id}-{sequence}-{attempt}")); + match fs::create_dir(&path) { + Ok(()) => return ShortSocketFixtureDirectory { path }, + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => continue, + Err(error) => panic!("create short socket fixture directory: {error}"), + } + } + panic!("claim unique short socket fixture directory") + } + fn test_root(name: &str) -> PathBuf { std::env::var_os("CCP_TEST_ROOT") .map(PathBuf::from) @@ -2162,6 +2271,7 @@ timeout_seconds = 60 let key = CacheKey::for_plan_cache(&envelope(), &envelope().plan.caches[0]).expect("key"); let complete = cache.entry_path(&key); fs::create_dir_all(complete.join("data")).expect("complete data"); + drop(acquire_entry_lock(&complete).expect("entry lock")); fs::write(complete.join("data/value"), b"cache").expect("cache data"); fs::write(complete.join(COMPLETE_FILE), COMPLETE_BYTES).expect("complete marker"); let incomplete = resolved @@ -2205,7 +2315,10 @@ timeout_seconds = 60 #[test] fn inventory_counts_each_payload_root_once() { let (_repo, resolved) = resolved_fixture("inventory-node-count"); - let entry = resolved.path.join(ENTRIES_DIR).join("entry"); + let entry = resolved + .path + .join(ENTRIES_DIR) + .join("sha256-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); fs::create_dir_all(entry.join("data")).unwrap(); fs::write(entry.join("data/payload"), b"payload").unwrap(); let mut nodes = 0; @@ -2243,6 +2356,7 @@ timeout_seconds = 60 let backup = entry.join(".backup-1-1"); fs::create_dir_all(staging.join("data")).unwrap(); fs::create_dir_all(backup.join("data")).unwrap(); + drop(acquire_entry_lock(&entry).expect("entry lock")); let outside = repo.join("generation-sentinel"); fs::write(&outside, b"sentinel").unwrap(); symlink(&outside, staging.join("data/external")).unwrap(); @@ -2259,6 +2373,59 @@ timeout_seconds = 60 clean(&repo); } + #[test] + fn payload_root_classifier_requires_a_valid_entry_and_owned_generation_name() { + let root = PathBuf::from("/owned/entries"); + let valid = + root.join("sha256-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); + for candidate in [ + valid.join("data"), + valid.join(".staging-1-1").join("data"), + valid.join(".backup-1-1").join("data"), + ] { + assert!( + is_payload_root(&valid, &candidate), + "accepted {candidate:?}" + ); + } + for (entry, candidate) in [ + (root.join(".entry-tmp-1"), root.join(".entry-tmp-1/data")), + (root.join("not-a-key"), root.join("not-a-key/data")), + (valid.clone(), valid.join(".staging-/data")), + (valid.clone(), valid.join(".staging-invalid/name/data")), + (valid.clone(), valid.join(".backup-/data")), + (valid.clone(), valid.join(".backup-invalid/name/data")), + ] { + assert!( + !is_payload_root(&entry, &candidate), + "unexpected payload root {candidate:?}" + ); + } + } + + #[test] + fn inventory_fails_closed_while_a_valid_entry_lock_is_held() { + let fixture = completed_entry_fixture("inventory-entry-lock"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("key"); + let prepared = fixture + .cache + .prepare_entry(&key, &plan.plan_digest, 2) + .expect("hold entry lock"); + + assert!(matches!( + fixture.cache.inventory(), + Err(CacheError::LockBusy(_)) + )); + + drop(prepared); + fixture + .cache + .inventory() + .expect("inventory after lock release"); + finish_fixture(fixture); + } + #[cfg(unix)] #[test] fn ownership_marker_symlink_is_rejected() { @@ -2343,11 +2510,8 @@ timeout_seconds = 60 let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); let prepared = cache.prepare_entry(&key, &plan.plan_digest, 1).unwrap(); let socket_path = prepared.data_path.join("socket"); - let socket_parent = PathBuf::from("/private/tmp").join(format!( - "ccp-socket-{}-{}", - std::process::id(), - TEMP_SEQUENCE.fetch_add(1, Ordering::Relaxed) - )); + let socket_directory = short_socket_fixture_directory(); + let socket_parent = socket_directory.path().join("fixture"); symlink(&prepared.data_path, &socket_parent).unwrap(); let listener = UnixListener::bind(socket_parent.join("socket")).unwrap(); @@ -2361,6 +2525,7 @@ timeout_seconds = 60 drop(listener); fs::remove_file(socket_path).unwrap(); fs::remove_file(socket_parent).unwrap(); + drop(socket_directory); drop(prepared); clean(&resolved.path); clean(&repo); @@ -2459,11 +2624,8 @@ timeout_seconds = 60 let plan = envelope(); let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).unwrap(); let socket_path = fixture.data_path.join("unsupported.socket"); - let socket_parent = PathBuf::from("/private/tmp").join(format!( - "ccp-socket-{}-{}", - std::process::id(), - TEMP_SEQUENCE.fetch_add(1, Ordering::Relaxed) - )); + let socket_directory = short_socket_fixture_directory(); + let socket_parent = socket_directory.path().join("fixture"); symlink(&fixture.data_path, &socket_parent).unwrap(); let listener = UnixListener::bind(socket_parent.join("unsupported.socket")).unwrap(); @@ -2476,12 +2638,69 @@ timeout_seconds = 60 drop(listener); fs::remove_file(socket_path).unwrap(); fs::remove_file(socket_parent).unwrap(); + drop(socket_directory); finish_fixture(fixture); assert!(matches!(preparation, Err(CacheError::UnexpectedEntry(_)))); assert!(staging.is_empty(), "failed preparation leaked {staging:?}"); } + #[test] + fn forced_fallback_copy_failure_removes_only_its_owned_staging_generation() { + let fixture = completed_entry_fixture("forced-fallback-copy-cleanup"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("key"); + fs::write(fixture.data_path.join("second"), b"second payload").expect("second payload"); + + force_clone_fallback_for_test(); + crate::cache_payload::fail_next_payload_copy_for_test(); + let preparation = fixture.cache.prepare_entry(&key, &plan.plan_digest, 2); + crate::cache_payload::clear_payload_copy_failure_for_test(); + + let staging: Vec<_> = fs::read_dir(&fixture.entry_path) + .expect("entry directory") + .flatten() + .filter(|entry| entry.file_name().to_string_lossy().starts_with(".staging-")) + .collect(); + assert!(matches!(preparation, Err(CacheError::Io(_)))); + assert!(staging.is_empty(), "failed fallback leaked {staging:?}"); + assert!(fixture.data_path.join("payload").is_file()); + assert!(fixture.data_path.join("second").is_file()); + finish_fixture(fixture); + } + + #[cfg(target_os = "macos")] + #[test] + fn macos_clone_attempt_and_forced_fallback_are_distinguishable() { + use std::os::unix::fs::symlink; + + let fixture = completed_entry_fixture("macos-clone-and-fallback"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("key"); + symlink("payload", fixture.data_path.join("relative")).expect("relative link"); + + clear_clone_operations_for_test(); + let cloned = fixture + .cache + .prepare_entry(&key, &plan.plan_digest, 2) + .expect("clone preparation"); + assert!(take_clone_operations_for_test().contains(&CloneOperation::Attempted)); + drop(cloned); + + force_clone_fallback_for_test(); + let fallback = fixture + .cache + .prepare_entry(&key, &plan.plan_digest, 3) + .expect("forced fallback preparation"); + assert!(take_clone_operations_for_test().contains(&CloneOperation::ForcedFallback)); + assert_eq!( + fs::read_link(fallback.data_path.join("relative")).expect("fallback link"), + Path::new("payload") + ); + drop(fallback); + finish_fixture(fixture); + } + #[cfg(unix)] #[test] fn staging_cleanup_unlinks_payload_links_without_touching_targets() { diff --git a/src/cache_payload.rs b/src/cache_payload.rs index 82e9632..ac611c6 100644 --- a/src/cache_payload.rs +++ b/src/cache_payload.rs @@ -1,6 +1,6 @@ use crate::cache::CacheError; #[cfg(test)] -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; use std::fs; use std::io; use std::path::{Path, PathBuf}; @@ -73,14 +73,14 @@ fn copy_payload_node( return recreate_payload_link(source, destination); } if metadata.is_file() { - fs::copy(source, destination).map_err(CacheError::Io)?; + traced_copy_file(source, destination).map_err(CacheError::Io)?; return Ok(()); } if !metadata.is_dir() { return Err(CacheError::UnexpectedEntry(source.to_path_buf())); } - fs::create_dir(destination).map_err(CacheError::Io)?; + traced_create_directory(destination).map_err(CacheError::Io)?; let mut entries = traced_read_directory(source) .map_err(CacheError::Io)? .collect::, _>>() @@ -160,8 +160,6 @@ fn measure_symlink(path: &Path) -> Result { #[cfg(unix)] fn recreate_payload_link(source: &Path, destination: &Path) -> Result<(), CacheError> { - use std::os::unix::fs::symlink; - let target = traced_read_link(source).map_err(|source_error| CacheError::PayloadSymlinkRead { path: source.to_path_buf(), @@ -172,9 +170,11 @@ fn recreate_payload_link(source: &Path, destination: &Path) -> Result<(), CacheE Err(error) => return Err(CacheError::Io(error)), Ok(_) => return Err(CacheError::UnexpectedEntry(destination.to_path_buf())), } - symlink(&target, destination).map_err(|source_error| CacheError::PayloadSymlinkCreate { - path: destination.to_path_buf(), - source: source_error, + traced_create_link(&target, destination).map_err(|source_error| { + CacheError::PayloadSymlinkCreate { + path: destination.to_path_buf(), + source: source_error, + } }) } @@ -200,6 +200,30 @@ fn traced_read_directory(path: &Path) -> std::io::Result { fs::read_dir(path) } +fn traced_copy_file(source: &Path, destination: &Path) -> std::io::Result { + #[cfg(test)] + { + record_payload_operation(PayloadOperation::CopyFile(destination.to_path_buf())); + if take_payload_copy_failure_for_test() { + return Err(io::Error::other("injected payload copy failure")); + } + } + fs::copy(source, destination) +} + +fn traced_create_directory(path: &Path) -> std::io::Result<()> { + #[cfg(test)] + record_payload_operation(PayloadOperation::CreateDirectory(path.to_path_buf())); + fs::create_dir(path) +} + +#[cfg(unix)] +fn traced_create_link(target: &Path, destination: &Path) -> std::io::Result<()> { + #[cfg(test)] + record_payload_operation(PayloadOperation::CreateLink(destination.to_path_buf())); + std::os::unix::fs::symlink(target, destination) +} + #[cfg(unix)] fn traced_read_link(path: &Path) -> std::io::Result { #[cfg(test)] @@ -213,13 +237,21 @@ enum PayloadOperation { SymlinkMetadata(PathBuf), ReadDirectory(PathBuf), ReadLink(PathBuf), + CopyFile(PathBuf), + CreateDirectory(PathBuf), + CreateLink(PathBuf), } #[cfg(test)] impl PayloadOperation { fn filesystem_path(&self) -> &Path { match self { - Self::SymlinkMetadata(path) | Self::ReadDirectory(path) | Self::ReadLink(path) => path, + Self::SymlinkMetadata(path) + | Self::ReadDirectory(path) + | Self::ReadLink(path) + | Self::CopyFile(path) + | Self::CreateDirectory(path) + | Self::CreateLink(path) => path, } } } @@ -229,6 +261,22 @@ thread_local! { static PAYLOAD_OPERATIONS: RefCell> = const { RefCell::new(Vec::new()) }; + static PAYLOAD_COPY_FAILURE: Cell = const { Cell::new(false) }; +} + +#[cfg(test)] +pub(crate) fn fail_next_payload_copy_for_test() { + PAYLOAD_COPY_FAILURE.with(|failure| failure.set(true)); +} + +#[cfg(test)] +pub(crate) fn clear_payload_copy_failure_for_test() { + PAYLOAD_COPY_FAILURE.with(|failure| failure.set(false)); +} + +#[cfg(test)] +fn take_payload_copy_failure_for_test() -> bool { + PAYLOAD_COPY_FAILURE.with(|failure| failure.replace(false)) } #[cfg(test)] @@ -420,6 +468,55 @@ mod tests { fs::remove_dir_all(destination).expect("remove copy destination"); } + #[cfg(unix)] + #[test] + fn fallback_copy_records_only_payload_paths_and_every_copy_operation() { + use std::os::unix::fs::symlink; + + let source = payload_fixture("copy-trace-source"); + let destination = source + .parent() + .expect("fixture parent") + .join("copy-trace-destination"); + let outside = source + .parent() + .expect("fixture parent") + .join("copy-trace-sentinel"); + fs::create_dir(source.join("nested")).expect("create nested payload directory"); + fs::write(source.join("nested/regular"), b"payload").expect("write regular payload"); + fs::write(&outside, b"outside").expect("write external sentinel"); + symlink(&outside, source.join("nested/external")).expect("create external link"); + + clear_payload_operations(); + let mut nodes = 0; + copy_payload_tree(&source, &destination, &mut nodes, 100).expect("copy payload"); + let operations = take_payload_operations(); + + assert!( + operations + .iter() + .any(|operation| matches!(operation, PayloadOperation::CopyFile(_))) + ); + assert!( + operations + .iter() + .any(|operation| matches!(operation, PayloadOperation::CreateDirectory(_))) + ); + assert!( + operations + .iter() + .any(|operation| matches!(operation, PayloadOperation::CreateLink(_))) + ); + assert!( + operations + .iter() + .all(|operation| !operation.filesystem_path().starts_with(&outside)) + ); + + remove_fixture(&source, &outside); + fs::remove_dir_all(destination).expect("remove copy destination"); + } + #[cfg(not(unix))] #[test] fn payload_link_recreation_is_explicitly_unsupported() { From ed15a0a26f791aa612f5aa31a9bfd853caef3a0a Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 12:25:37 +0200 Subject: [PATCH 20/24] fix: preserve cache inventory lock state --- docs/TESTING_AND_FAULT_INJECTION.md | 2 +- src/cache.rs | 151 +++++++++++++++++++++++++--- src/cache_payload.rs | 67 ++++++++---- 3 files changed, 190 insertions(+), 30 deletions(-) diff --git a/docs/TESTING_AND_FAULT_INJECTION.md b/docs/TESTING_AND_FAULT_INJECTION.md index 6b4e13b..e93ec88 100644 --- a/docs/TESTING_AND_FAULT_INJECTION.md +++ b/docs/TESTING_AND_FAULT_INJECTION.md @@ -97,7 +97,7 @@ release. Deterministic coverage includes `src/cache.rs::complete_payload_symlinks_are_preserved_across_generation_reuse`, `src/cache.rs::failed_payload_preflight_removes_the_new_staging_generation`, `src/cache.rs::forced_fallback_copy_failure_removes_only_its_owned_staging_generation`, and `src/cache.rs::staging_cleanup_unlinks_payload_links_without_touching_targets`. Payload measurement is covered by `src/cache_payload.rs::payload_measurement_counts_links_without_following_targets`; fallback semantics and complete operation tracing by `src/cache_payload.rs::fallback_copy_preserves_each_link_target_and_external_sentinel` and `src/cache_payload.rs::fallback_copy_records_only_payload_paths_and_every_copy_operation`. -On macOS, a test-only seam distinguishes an attempted clone from a deliberately forced fallback; the fallback preserves an equivalent link-bearing payload. Injected fallback-copy failure proves exact owned-staging cleanup. Unix fallback copy preserves each link target without following it and records metadata, directory enumeration, link reads, regular-file copies, directory creation, and link creation without treating a link target as a host path. Windows link-bearing payload reuse remains fail-closed and unsupported. These are deterministic source tests, not a native CCP receipt or native qualification. +On macOS, test-only outcomes distinguish attempted-and-succeeded clone reuse, natural fallback, and deliberately forced fallback; the clone-success and fallback tests preserve link identity. Injected fallback-copy failure occurs only after one successful copied object and proves exact owned-staging cleanup. Owner-drop tests preserve unrelated and identity-mismatched staging directories. Unix fallback copy preserves each link target without following it and records metadata, directory enumeration, link reads, regular-file source/destination pairs, directory creation, and link creation without treating either copy path or a link target as a host path. Windows link-bearing payload reuse remains fail-closed and unsupported. These are deterministic source tests, not a native CCP receipt or native qualification. ## Managed-cache pin contract diff --git a/src/cache.rs b/src/cache.rs index 2f2a9a1..211a7cf 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -750,7 +750,7 @@ impl ManagedCache { // prepared. Inventory must take the same cooperative lock before // inspecting either status or payload descendants. let _entry_lock = if is_cache_directory_name(&name) { - Some(acquire_existing_entry_lock(&entry.path())?) + Some(acquire_existing_entry_lock_readonly(&entry.path())?) } else { None }; @@ -1331,10 +1331,14 @@ fn try_clone_tree(source: &Path, destination: &Path) -> Result // back to the deterministic link-preserving copy path below. let result = unsafe { clonefile(source.as_ptr(), destination_c.as_ptr(), 0) }; if result == 0 { + #[cfg(test)] + record_clone_operation_for_test(CloneOperation::Succeeded); return Ok(true); } let error = io::Error::last_os_error(); if matches!(error.raw_os_error(), Some(18 | 22 | 45 | 95)) { + #[cfg(test)] + record_clone_operation_for_test(CloneOperation::NaturalFallback); remove_if_present(destination)?; return Ok(false); } @@ -1359,6 +1363,8 @@ thread_local! { #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum CloneOperation { Attempted, + Succeeded, + NaturalFallback, ForcedFallback, } @@ -1517,25 +1523,34 @@ fn acquire_existing_entry_lock(entry: &Path) -> Result, CacheError> { acquire_advisory_lock_existing(&entry.join(ENTRY_LOCK_FILE), "cache entry") } +fn acquire_existing_entry_lock_readonly(entry: &Path) -> Result, CacheError> { + acquire_advisory_lock_existing_readonly(&entry.join(ENTRY_LOCK_FILE)) +} + fn acquire_promotion_lock(root: &Path) -> Result, CacheError> { acquire_advisory_lock(&root.join(PROMOTION_LOCK_FILE), "cache promotion") } fn acquire_advisory_lock(path: &Path, label: &'static str) -> Result, CacheError> { - acquire_advisory_lock_with_mode(path, label, true) + acquire_advisory_lock_with_mode(path, label, true, true) } fn acquire_advisory_lock_existing( path: &Path, label: &'static str, ) -> Result, CacheError> { - acquire_advisory_lock_with_mode(path, label, false) + acquire_advisory_lock_with_mode(path, label, false, true) +} + +fn acquire_advisory_lock_existing_readonly(path: &Path) -> Result, CacheError> { + acquire_advisory_lock_with_mode(path, "cache entry", false, false) } fn acquire_advisory_lock_with_mode( path: &Path, label: &'static str, create: bool, + write_owner: bool, ) -> Result, CacheError> { match fs::symlink_metadata(path) { Ok(metadata) if metadata.file_type().is_symlink() => { @@ -1560,13 +1575,15 @@ fn acquire_advisory_lock_with_mode( } return Err(CacheError::Io(error)); } - file.set_len(0).map_err(CacheError::Io)?; - let owner = format!( - "{{\"schema_version\":\"1.0\",\"owner\":\"commit-ci-preflight\",\"purpose\":\"{label}\",\"pid\":{}}}\n", - std::process::id() - ); - file.write_all(owner.as_bytes()).map_err(CacheError::Io)?; - file.sync_all().map_err(CacheError::Io)?; + if write_owner { + file.set_len(0).map_err(CacheError::Io)?; + let owner = format!( + "{{\"schema_version\":\"1.0\",\"owner\":\"commit-ci-preflight\",\"purpose\":\"{label}\",\"pid\":{}}}\n", + std::process::id() + ); + file.write_all(owner.as_bytes()).map_err(CacheError::Io)?; + file.sync_all().map_err(CacheError::Io)?; + } Ok(Arc::new(file)) } @@ -2417,6 +2434,10 @@ timeout_seconds = 60 fixture.cache.inventory(), Err(CacheError::LockBusy(_)) )); + assert!(matches!( + fixture.cache.cleanup_dry_run(), + Err(CacheError::LockBusy(_)) + )); drop(prepared); fixture @@ -2426,6 +2447,53 @@ timeout_seconds = 60 finish_fixture(fixture); } + #[test] + fn inventory_and_cleanup_preserve_existing_entry_lock_bytes_and_metadata() { + let fixture = completed_entry_fixture("inventory-read-only-lock"); + let lock = fixture.entry_path.join(ENTRY_LOCK_FILE); + fs::write(&lock, b"immutable inventory lock bytes\n").expect("write lock fixture"); + let before_bytes = fs::read(&lock).expect("read lock before"); + let before_metadata = fs::metadata(&lock).expect("lock metadata before"); + + fixture.cache.inventory().expect("inventory"); + assert_eq!( + fs::read(&lock).expect("read lock after inventory"), + before_bytes + ); + let after_inventory = fs::metadata(&lock).expect("lock metadata after inventory"); + assert_eq!(after_inventory.len(), before_metadata.len()); + assert_eq!( + after_inventory.modified().unwrap(), + before_metadata.modified().unwrap() + ); + + fixture.cache.cleanup_dry_run().expect("cleanup dry run"); + assert_eq!( + fs::read(&lock).expect("read lock after cleanup"), + before_bytes + ); + let after_cleanup = fs::metadata(&lock).expect("lock metadata after cleanup"); + assert_eq!(after_cleanup.len(), before_metadata.len()); + assert_eq!( + after_cleanup.modified().unwrap(), + before_metadata.modified().unwrap() + ); + finish_fixture(fixture); + } + + #[test] + fn inventory_and_cleanup_reject_missing_entry_lock_without_recreating_it() { + let fixture = completed_entry_fixture("inventory-missing-entry-lock"); + let lock = fixture.entry_path.join(ENTRY_LOCK_FILE); + fs::remove_file(&lock).expect("remove entry lock"); + + assert!(fixture.cache.inventory().is_err()); + assert!(!lock.exists(), "inventory must not recreate lock"); + assert!(fixture.cache.cleanup_dry_run().is_err()); + assert!(!lock.exists(), "cleanup must not recreate lock"); + finish_fixture(fixture); + } + #[cfg(unix)] #[test] fn ownership_marker_symlink_is_rejected() { @@ -2653,8 +2721,9 @@ timeout_seconds = 60 fs::write(fixture.data_path.join("second"), b"second payload").expect("second payload"); force_clone_fallback_for_test(); - crate::cache_payload::fail_next_payload_copy_for_test(); + crate::cache_payload::fail_payload_copy_after_for_test(1); let preparation = fixture.cache.prepare_entry(&key, &plan.plan_digest, 2); + let copied = crate::cache_payload::payload_copy_successes_for_test(); crate::cache_payload::clear_payload_copy_failure_for_test(); let staging: Vec<_> = fs::read_dir(&fixture.entry_path) @@ -2663,6 +2732,10 @@ timeout_seconds = 60 .filter(|entry| entry.file_name().to_string_lossy().starts_with(".staging-")) .collect(); assert!(matches!(preparation, Err(CacheError::Io(_)))); + assert_eq!( + copied, 1, + "one payload object must exist before injected failure" + ); assert!(staging.is_empty(), "failed fallback leaked {staging:?}"); assert!(fixture.data_path.join("payload").is_file()); assert!(fixture.data_path.join("second").is_file()); @@ -2684,7 +2757,13 @@ timeout_seconds = 60 .cache .prepare_entry(&key, &plan.plan_digest, 2) .expect("clone preparation"); - assert!(take_clone_operations_for_test().contains(&CloneOperation::Attempted)); + let clone_operations = take_clone_operations_for_test(); + assert!(clone_operations.contains(&CloneOperation::Attempted)); + assert!(clone_operations.contains(&CloneOperation::Succeeded)); + assert_eq!( + fs::read_link(cloned.data_path.join("relative")).expect("cloned link"), + Path::new("payload") + ); drop(cloned); force_clone_fallback_for_test(); @@ -2729,6 +2808,54 @@ timeout_seconds = 60 clean(&repo); } + #[test] + fn owner_drop_preserves_an_unrelated_staging_directory() { + let (repo, resolved) = resolved_fixture("unrelated-staging-preserved"); + let cache = ManagedCache::initialize(resolved.clone()).expect("initialize"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("key"); + let prepared = cache + .prepare_entry(&key, &plan.plan_digest, 1) + .expect("prepare"); + let unrelated = prepared.path.join(".staging-unrelated-1"); + fs::create_dir_all(unrelated.join("data")).expect("unrelated staging"); + drop(prepared); + + assert!( + unrelated.is_dir(), + "owner must not remove unrelated staging" + ); + fs::remove_dir_all(unrelated).expect("remove unrelated fixture"); + clean(&resolved.path); + clean(&repo); + } + + #[test] + fn owner_drop_preserves_a_staging_directory_with_altered_manifest_identity() { + let (repo, resolved) = resolved_fixture("mismatched-staging-preserved"); + let cache = ManagedCache::initialize(resolved.clone()).expect("initialize"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("key"); + let prepared = cache + .prepare_entry(&key, &plan.plan_digest, 1) + .expect("prepare"); + let staging = prepared.staging_path.clone(); + let manifest_path = staging.join(GENERATION_MANIFEST_FILE); + let mut manifest = read_generation_manifest(&manifest_path).expect("manifest"); + manifest.generation = 2; + fs::write( + &manifest_path, + serde_json::to_vec(&manifest).expect("encode manifest"), + ) + .expect("alter manifest identity"); + drop(prepared); + + assert!(staging.is_dir(), "owner must preserve mismatched staging"); + fs::remove_dir_all(staging).expect("remove mismatched fixture"); + clean(&resolved.path); + clean(&repo); + } + #[test] fn active_entry_lock_blocks_a_second_preparation_until_release() { let (repo, resolved) = resolved_fixture("entry-lock"); diff --git a/src/cache_payload.rs b/src/cache_payload.rs index ac611c6..eea1a68 100644 --- a/src/cache_payload.rs +++ b/src/cache_payload.rs @@ -203,12 +203,20 @@ fn traced_read_directory(path: &Path) -> std::io::Result { fn traced_copy_file(source: &Path, destination: &Path) -> std::io::Result { #[cfg(test)] { - record_payload_operation(PayloadOperation::CopyFile(destination.to_path_buf())); - if take_payload_copy_failure_for_test() { + record_payload_operation(PayloadOperation::CopyFile { + source: source.to_path_buf(), + destination: destination.to_path_buf(), + }); + if payload_copy_should_fail_for_test() { return Err(io::Error::other("injected payload copy failure")); } } - fs::copy(source, destination) + let copied = fs::copy(source, destination); + #[cfg(test)] + if copied.is_ok() { + record_payload_copy_success_for_test(); + } + copied } fn traced_create_directory(path: &Path) -> std::io::Result<()> { @@ -237,7 +245,10 @@ enum PayloadOperation { SymlinkMetadata(PathBuf), ReadDirectory(PathBuf), ReadLink(PathBuf), - CopyFile(PathBuf), + CopyFile { + source: PathBuf, + destination: PathBuf, + }, CreateDirectory(PathBuf), CreateLink(PathBuf), } @@ -249,9 +260,9 @@ impl PayloadOperation { Self::SymlinkMetadata(path) | Self::ReadDirectory(path) | Self::ReadLink(path) - | Self::CopyFile(path) | Self::CreateDirectory(path) | Self::CreateLink(path) => path, + Self::CopyFile { destination, .. } => destination, } } } @@ -261,22 +272,40 @@ thread_local! { static PAYLOAD_OPERATIONS: RefCell> = const { RefCell::new(Vec::new()) }; - static PAYLOAD_COPY_FAILURE: Cell = const { Cell::new(false) }; + static PAYLOAD_COPY_FAILURE_AFTER: Cell> = const { Cell::new(None) }; + static PAYLOAD_COPY_SUCCESSES: Cell = const { Cell::new(0) }; } #[cfg(test)] -pub(crate) fn fail_next_payload_copy_for_test() { - PAYLOAD_COPY_FAILURE.with(|failure| failure.set(true)); +pub(crate) fn fail_payload_copy_after_for_test(successful_copies: usize) { + PAYLOAD_COPY_FAILURE_AFTER.with(|failure| failure.set(Some(successful_copies))); + PAYLOAD_COPY_SUCCESSES.with(|successes| successes.set(0)); } #[cfg(test)] pub(crate) fn clear_payload_copy_failure_for_test() { - PAYLOAD_COPY_FAILURE.with(|failure| failure.set(false)); + PAYLOAD_COPY_FAILURE_AFTER.with(|failure| failure.set(None)); + PAYLOAD_COPY_SUCCESSES.with(|successes| successes.set(0)); +} + +#[cfg(test)] +fn payload_copy_should_fail_for_test() -> bool { + PAYLOAD_COPY_FAILURE_AFTER.with(|failure| failure.get() == Some(0)) } #[cfg(test)] -fn take_payload_copy_failure_for_test() -> bool { - PAYLOAD_COPY_FAILURE.with(|failure| failure.replace(false)) +fn record_payload_copy_success_for_test() { + PAYLOAD_COPY_SUCCESSES.with(|successes| successes.set(successes.get() + 1)); + PAYLOAD_COPY_FAILURE_AFTER.with(|failure| { + if let Some(remaining) = failure.get() { + failure.set(Some(remaining.saturating_sub(1))); + } + }); +} + +#[cfg(test)] +pub(crate) fn payload_copy_successes_for_test() -> usize { + PAYLOAD_COPY_SUCCESSES.with(Cell::get) } #[cfg(test)] @@ -495,7 +524,7 @@ mod tests { assert!( operations .iter() - .any(|operation| matches!(operation, PayloadOperation::CopyFile(_))) + .any(|operation| matches!(operation, PayloadOperation::CopyFile { .. })) ); assert!( operations @@ -507,11 +536,15 @@ mod tests { .iter() .any(|operation| matches!(operation, PayloadOperation::CreateLink(_))) ); - assert!( - operations - .iter() - .all(|operation| !operation.filesystem_path().starts_with(&outside)) - ); + assert!(operations.iter().all(|operation| match operation { + PayloadOperation::CopyFile { + source, + destination, + } => { + !source.starts_with(&outside) && !destination.starts_with(&outside) + } + _ => !operation.filesystem_path().starts_with(&outside), + })); remove_fixture(&source, &outside); fs::remove_dir_all(destination).expect("remove copy destination"); From 235d9ba7de03cd7ee832b2fee852130f4853af4a Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 12:41:07 +0200 Subject: [PATCH 21/24] docs: normalize cache payload spec metadata --- .../specs/2026-08-29-cache-payload-symlink-design.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md b/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md index 01e1d8e..1b3f53e 100644 --- a/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md +++ b/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md @@ -1,8 +1,8 @@ # Opaque cache-payload symbolic-link design Status: approved by owner on 2026-08-29 -Date: 2026-08-29 -Baseline: `820a7fa6ce83a7ac8593c2800f8be4f44ab82ebc` +Date: 2026-08-29 +Baseline: `820a7fa6ce83a7ac8593c2800f8be4f44ab82ebc` Scope: make completed cache generations reusable when ordinary package-manager payloads contain symbolic links, without weakening CCP's managed-root boundary From 80af79daa24cf05e4ae765143b20774b752e447d Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 14:24:42 +0200 Subject: [PATCH 22/24] fix: gate macOS clone test instrumentation --- src/cache.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 211a7cf..a348e87 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1362,8 +1362,11 @@ thread_local! { #[cfg(test)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum CloneOperation { + #[cfg(target_os = "macos")] Attempted, + #[cfg(target_os = "macos")] Succeeded, + #[cfg(target_os = "macos")] NaturalFallback, ForcedFallback, } @@ -1383,12 +1386,12 @@ fn record_clone_operation_for_test(operation: CloneOperation) { CLONE_OPERATIONS.with(|operations| operations.borrow_mut().push(operation)); } -#[cfg(test)] +#[cfg(all(test, target_os = "macos"))] fn clear_clone_operations_for_test() { CLONE_OPERATIONS.with(|operations| operations.borrow_mut().clear()); } -#[cfg(test)] +#[cfg(all(test, target_os = "macos"))] fn take_clone_operations_for_test() -> Vec { CLONE_OPERATIONS.with(|operations| std::mem::take(&mut *operations.borrow_mut())) } From 0ba9777eff848d7df4ac44c93ed4c8bf4e239fdd Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 18:27:51 +0200 Subject: [PATCH 23/24] fix: close cache payload qualification gaps --- docs/TESTING_AND_FAULT_INJECTION.md | 2 +- src/cache.rs | 245 +++++++++++++++++++++++++++- src/cache_payload.rs | 81 +++++++-- 3 files changed, 311 insertions(+), 17 deletions(-) diff --git a/docs/TESTING_AND_FAULT_INJECTION.md b/docs/TESTING_AND_FAULT_INJECTION.md index e93ec88..5e45cbc 100644 --- a/docs/TESTING_AND_FAULT_INJECTION.md +++ b/docs/TESTING_AND_FAULT_INJECTION.md @@ -97,7 +97,7 @@ release. Deterministic coverage includes `src/cache.rs::complete_payload_symlinks_are_preserved_across_generation_reuse`, `src/cache.rs::failed_payload_preflight_removes_the_new_staging_generation`, `src/cache.rs::forced_fallback_copy_failure_removes_only_its_owned_staging_generation`, and `src/cache.rs::staging_cleanup_unlinks_payload_links_without_touching_targets`. Payload measurement is covered by `src/cache_payload.rs::payload_measurement_counts_links_without_following_targets`; fallback semantics and complete operation tracing by `src/cache_payload.rs::fallback_copy_preserves_each_link_target_and_external_sentinel` and `src/cache_payload.rs::fallback_copy_records_only_payload_paths_and_every_copy_operation`. -On macOS, test-only outcomes distinguish attempted-and-succeeded clone reuse, natural fallback, and deliberately forced fallback; the clone-success and fallback tests preserve link identity. Injected fallback-copy failure occurs only after one successful copied object and proves exact owned-staging cleanup. Owner-drop tests preserve unrelated and identity-mismatched staging directories. Unix fallback copy preserves each link target without following it and records metadata, directory enumeration, link reads, regular-file source/destination pairs, directory creation, and link creation without treating either copy path or a link target as a host path. Windows link-bearing payload reuse remains fail-closed and unsupported. These are deterministic source tests, not a native CCP receipt or native qualification. +On macOS, deterministic tests distinguish attempted-and-succeeded clone reuse and deliberately forced fallback; the clone-success and forced-fallback tests preserve link identity. Injected fallback-copy failure occurs only after one successful copied object and proves exact owned-staging cleanup. Owner-drop tests preserve unrelated and identity-mismatched staging directories. Unix fallback copy preserves each link target without following it and records metadata, directory enumeration, link reads, regular-file source/destination pairs, directory creation, and link creation without treating either copy path or a link target as a host path. Windows link-bearing payload reuse remains fail-closed and unsupported. These are deterministic source tests, not a native CCP receipt or native qualification. ## Managed-cache pin contract diff --git a/src/cache.rs b/src/cache.rs index a348e87..f749c13 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -31,7 +31,9 @@ use fs2::FileExt; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; -use crate::cache_payload::{copy_payload_tree, validate_payload_tree}; +use crate::cache_payload::{ + copy_payload_tree_with_policy, validate_payload_tree, validate_payload_tree_with_policy, +}; use crate::config::{ExecutionPlanEnvelopeV1, NormalizedCache}; use crate::durable_fs::{DurableFileSystem, DurableFsError}; use crate::receipt::{ReceiptError, canonical_json}; @@ -85,6 +87,25 @@ pub enum PlatformFamily { Unix, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum PayloadLinkPolicy { + Preserve, + Reject, +} + +impl PayloadLinkPolicy { + pub(crate) const fn current() -> Self { + #[cfg(unix)] + { + Self::Preserve + } + #[cfg(not(unix))] + { + Self::Reject + } + } +} + impl PlatformFamily { pub fn current() -> Self { #[cfg(target_os = "macos")] @@ -407,6 +428,37 @@ impl ManagedCache { key: &CacheKey, plan_digest: &str, generation: u64, + ) -> Result { + self.prepare_entry_with_payload_link_policy( + key, + plan_digest, + generation, + PayloadLinkPolicy::current(), + ) + } + + #[cfg(test)] + fn prepare_entry_with_payload_link_policy_for_test( + &self, + key: &CacheKey, + plan_digest: &str, + generation: u64, + payload_link_policy: PayloadLinkPolicy, + ) -> Result { + self.prepare_entry_with_payload_link_policy( + key, + plan_digest, + generation, + payload_link_policy, + ) + } + + fn prepare_entry_with_payload_link_policy( + &self, + key: &CacheKey, + plan_digest: &str, + generation: u64, + payload_link_policy: PayloadLinkPolicy, ) -> Result { validate_owner_marker(&self.root.path.join(OWNER_FILE))?; let entries_root = self.root.path.join(ENTRIES_DIR); @@ -445,9 +497,15 @@ impl ManagedCache { if complete { let source = path.join("data"); remove_if_present(&data_path)?; - if !try_clone_tree(&source, &data_path)? { + if !try_clone_tree(&source, &data_path, payload_link_policy)? { let mut nodes = 0; - copy_payload_tree(&source, &data_path, &mut nodes, MAX_INVENTORY_NODES)?; + copy_payload_tree_with_policy( + &source, + &data_path, + &mut nodes, + MAX_INVENTORY_NODES, + payload_link_policy, + )?; } } let manifest = CacheGenerationManifestV1 { @@ -1181,6 +1239,12 @@ fn write_generation_manifest( manifest: &CacheGenerationManifestV1, ) -> Result<(), CacheError> { let bytes = canonical_json(manifest).map_err(CacheError::Canonical)?; + #[cfg(test)] + if take_generation_manifest_write_failure_for_test() { + return Err(CacheError::Io(io::Error::other( + "injected generation manifest write failure", + ))); + } let path = staging_path.join(GENERATION_MANIFEST_FILE); let mut file = OpenOptions::new() .write(true) @@ -1303,9 +1367,18 @@ fn write_complete_marker(path: &Path) -> Result<(), CacheError> { file.sync_all().map_err(CacheError::Io) } -fn try_clone_tree(source: &Path, destination: &Path) -> Result { +fn try_clone_tree( + source: &Path, + destination: &Path, + payload_link_policy: PayloadLinkPolicy, +) -> Result { let mut nodes = 0; - validate_payload_tree(source, &mut nodes, MAX_INVENTORY_NODES)?; + validate_payload_tree_with_policy( + source, + &mut nodes, + MAX_INVENTORY_NODES, + payload_link_policy, + )?; #[cfg(test)] if take_forced_clone_fallback_for_test() { record_clone_operation_for_test(CloneOperation::ForcedFallback); @@ -1354,11 +1427,22 @@ fn try_clone_tree(source: &Path, destination: &Path) -> Result #[cfg(test)] thread_local! { static FORCE_CLONE_FALLBACK: Cell = const { Cell::new(false) }; + static FAIL_GENERATION_MANIFEST_WRITE_ONCE: Cell = const { Cell::new(false) }; static CLONE_OPERATIONS: std::cell::RefCell> = const { std::cell::RefCell::new(Vec::new()) }; } +#[cfg(test)] +fn fail_generation_manifest_write_once_for_test() { + FAIL_GENERATION_MANIFEST_WRITE_ONCE.with(|failure| failure.set(true)); +} + +#[cfg(test)] +fn take_generation_manifest_write_failure_for_test() -> bool { + FAIL_GENERATION_MANIFEST_WRITE_ONCE.with(|failure| failure.replace(false)) +} + #[cfg(test)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum CloneOperation { @@ -2686,6 +2770,157 @@ timeout_seconds = 60 clean(&repo); } + #[cfg(unix)] + #[test] + fn non_unix_policy_rejects_completed_payload_links_before_partial_reuse() { + use std::os::unix::fs::symlink; + + let fixture = completed_entry_fixture("non-unix-payload-link-reuse"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("key"); + fs::write(fixture.data_path.join("a-regular"), b"regular payload") + .expect("write regular payload"); + symlink("a-regular", fixture.data_path.join("z-link")).expect("create payload link"); + + force_clone_fallback_for_test(); + crate::cache_payload::clear_payload_copy_failure_for_test(); + let preparation = fixture + .cache + .prepare_entry_with_payload_link_policy_for_test( + &key, + &plan.plan_digest, + 2, + PayloadLinkPolicy::Reject, + ); + + assert!(matches!( + preparation, + Err(CacheError::PayloadSymlinkUnsupported(_)) + )); + assert_eq!( + crate::cache_payload::payload_copy_successes_for_test(), + 0, + "a rejected payload link must fail before any destination file is copied" + ); + let staging: Vec<_> = fs::read_dir(&fixture.entry_path) + .expect("entry directory") + .flatten() + .filter(|entry| entry.file_name().to_string_lossy().starts_with(".staging-")) + .collect(); + assert!(staging.is_empty(), "failed reuse leaked {staging:?}"); + assert!(fixture.data_path.join("z-link").is_symlink()); + finish_fixture(fixture); + } + + #[test] + fn non_unix_policy_reuses_completed_link_free_payloads() { + let fixture = completed_entry_fixture("non-unix-link-free-payload-reuse"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("key"); + + force_clone_fallback_for_test(); + crate::cache_payload::clear_payload_copy_failure_for_test(); + let prepared = fixture + .cache + .prepare_entry_with_payload_link_policy_for_test( + &key, + &plan.plan_digest, + 2, + PayloadLinkPolicy::Reject, + ) + .expect("link-free payload remains reusable"); + + assert_eq!( + fs::read(prepared.data_path.join("payload")).expect("read reused payload"), + b"owned fixture" + ); + assert!( + crate::cache_payload::payload_copy_successes_for_test() > 0, + "forced fallback must copy the completed link-free payload" + ); + drop(prepared); + finish_fixture(fixture); + } + + #[cfg(unix)] + #[test] + fn non_unix_policy_preserves_strict_control_and_payload_root_link_rejection() { + use std::os::unix::fs::symlink; + + let control_fixture = completed_entry_fixture("non-unix-control-link-rejection"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("key"); + let lock_path = control_fixture.entry_path.join(ENTRY_LOCK_FILE); + let external_lock = control_fixture.repo.join("external-entry-lock"); + fs::write(&external_lock, b"external").expect("write external lock"); + fs::remove_file(&lock_path).expect("remove entry lock"); + symlink(&external_lock, &lock_path).expect("replace entry lock with link"); + assert!(matches!( + control_fixture.cache.prepare_entry_with_payload_link_policy_for_test( + &key, + &plan.plan_digest, + 2, + PayloadLinkPolicy::Reject, + ), + Err(CacheError::SymlinkInManagedRoot(path)) if path == lock_path + )); + clean(&control_fixture.resolved.path); + clean(&control_fixture.repo); + + let payload_fixture = completed_entry_fixture("non-unix-payload-root-link-rejection"); + let payload_root = payload_fixture.data_path.clone(); + let real_payload = payload_fixture.entry_path.join("real-data"); + fs::rename(&payload_root, &real_payload).expect("move completed payload root"); + symlink(&real_payload, &payload_root).expect("replace payload root with link"); + assert!(matches!( + payload_fixture.cache.prepare_entry_with_payload_link_policy_for_test( + &key, + &plan.plan_digest, + 2, + PayloadLinkPolicy::Reject, + ), + Err(CacheError::SymlinkInManagedRoot(path)) if path == payload_root + )); + clean(&payload_fixture.resolved.path); + clean(&payload_fixture.repo); + } + + #[test] + fn manifest_write_failure_removes_owned_staging_and_releases_the_entry_lock() { + let fixture = completed_entry_fixture("manifest-write-cleanup"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("key"); + let unrelated = fixture.repo.join("unrelated-sentinel"); + fs::write(&unrelated, b"unrelated").expect("write unrelated sentinel"); + let completed_payload = fs::read(fixture.data_path.join("payload")).expect("read payload"); + + fail_generation_manifest_write_once_for_test(); + let preparation = fixture.cache.prepare_entry(&key, &plan.plan_digest, 2); + + assert!(matches!(preparation, Err(CacheError::Io(_)))); + let staging: Vec<_> = fs::read_dir(&fixture.entry_path) + .expect("entry directory") + .flatten() + .filter(|entry| entry.file_name().to_string_lossy().starts_with(".staging-")) + .collect(); + assert!(staging.is_empty(), "manifest failure leaked {staging:?}"); + assert_eq!( + fs::read(fixture.data_path.join("payload")).expect("read completed payload"), + completed_payload + ); + assert_eq!( + fs::read(&unrelated).expect("read unrelated sentinel"), + b"unrelated" + ); + + let released = fixture + .cache + .prepare_entry(&key, &plan.plan_digest, 3) + .expect("entry lock is released after cleanup"); + drop(released); + finish_fixture(fixture); + } + #[cfg(unix)] #[test] fn failed_payload_preflight_removes_the_new_staging_generation() { diff --git a/src/cache_payload.rs b/src/cache_payload.rs index eea1a68..6b8d8ba 100644 --- a/src/cache_payload.rs +++ b/src/cache_payload.rs @@ -1,4 +1,4 @@ -use crate::cache::CacheError; +use crate::cache::{CacheError, PayloadLinkPolicy}; #[cfg(test)] use std::cell::{Cell, RefCell}; use std::fs; @@ -15,6 +15,15 @@ pub(crate) fn measure_payload_tree( root: &Path, nodes: &mut usize, node_limit: usize, +) -> Result { + measure_payload_tree_with_policy(root, nodes, node_limit, PayloadLinkPolicy::current()) +} + +fn measure_payload_tree_with_policy( + root: &Path, + nodes: &mut usize, + node_limit: usize, + payload_link_policy: PayloadLinkPolicy, ) -> Result { let metadata = traced_symlink_metadata(root).map_err(CacheError::Io)?; if metadata.file_type().is_symlink() { @@ -23,7 +32,7 @@ pub(crate) fn measure_payload_tree( if !metadata.is_dir() { return Err(CacheError::UnexpectedEntry(root.to_path_buf())); } - walk_payload(root, nodes, node_limit) + walk_payload_with_policy(root, nodes, node_limit, payload_link_policy) } pub(crate) fn validate_payload_tree( @@ -34,11 +43,37 @@ pub(crate) fn validate_payload_tree( measure_payload_tree(root, nodes, node_limit).map(|_| ()) } +pub(crate) fn validate_payload_tree_with_policy( + root: &Path, + nodes: &mut usize, + node_limit: usize, + payload_link_policy: PayloadLinkPolicy, +) -> Result<(), CacheError> { + measure_payload_tree_with_policy(root, nodes, node_limit, payload_link_policy).map(|_| ()) +} + +#[cfg(test)] pub(crate) fn copy_payload_tree( source: &Path, destination: &Path, nodes: &mut usize, node_limit: usize, +) -> Result<(), CacheError> { + copy_payload_tree_with_policy( + source, + destination, + nodes, + node_limit, + PayloadLinkPolicy::current(), + ) +} + +pub(crate) fn copy_payload_tree_with_policy( + source: &Path, + destination: &Path, + nodes: &mut usize, + node_limit: usize, + payload_link_policy: PayloadLinkPolicy, ) -> Result<(), CacheError> { let metadata = traced_symlink_metadata(source).map_err(CacheError::Io)?; if metadata.file_type().is_symlink() { @@ -52,7 +87,7 @@ pub(crate) fn copy_payload_tree( Err(error) => return Err(CacheError::Io(error)), Ok(_) => return Err(CacheError::UnexpectedEntry(destination.to_path_buf())), } - copy_payload_node(source, destination, nodes, node_limit) + copy_payload_node(source, destination, nodes, node_limit, payload_link_policy) } fn copy_payload_node( @@ -60,6 +95,7 @@ fn copy_payload_node( destination: &Path, nodes: &mut usize, node_limit: usize, + payload_link_policy: PayloadLinkPolicy, ) -> Result<(), CacheError> { *nodes = nodes .checked_add(1) @@ -70,7 +106,7 @@ fn copy_payload_node( let metadata = traced_symlink_metadata(source).map_err(CacheError::Io)?; if metadata.file_type().is_symlink() { - return recreate_payload_link(source, destination); + return recreate_payload_link(source, destination, payload_link_policy); } if metadata.is_file() { traced_copy_file(source, destination).map_err(CacheError::Io)?; @@ -92,15 +128,17 @@ fn copy_payload_node( &destination.join(entry.file_name()), nodes, node_limit, + payload_link_policy, )?; } Ok(()) } -fn walk_payload( +fn walk_payload_with_policy( path: &Path, nodes: &mut usize, node_limit: usize, + payload_link_policy: PayloadLinkPolicy, ) -> Result { *nodes = nodes .checked_add(1) @@ -111,7 +149,7 @@ fn walk_payload( let metadata = traced_symlink_metadata(path).map_err(CacheError::Io)?; if metadata.file_type().is_symlink() { - return measure_symlink(path); + return measure_symlink(path, payload_link_policy); } if metadata.is_file() { return Ok(PayloadTreeStats { @@ -131,7 +169,8 @@ fn walk_payload( let mut stats = PayloadTreeStats { bytes: 0, files: 0 }; for entry in entries { - let child = walk_payload(&entry.path(), nodes, node_limit)?; + let child = + walk_payload_with_policy(&entry.path(), nodes, node_limit, payload_link_policy)?; stats.bytes = stats .bytes .checked_add(child.bytes) @@ -145,7 +184,13 @@ fn walk_payload( } #[cfg(unix)] -fn measure_symlink(path: &Path) -> Result { +fn measure_symlink( + path: &Path, + payload_link_policy: PayloadLinkPolicy, +) -> Result { + if payload_link_policy == PayloadLinkPolicy::Reject { + return Err(CacheError::PayloadSymlinkUnsupported(path.to_path_buf())); + } use std::os::unix::ffi::OsStrExt; let target = traced_read_link(path).map_err(|source| CacheError::PayloadSymlinkRead { @@ -159,7 +204,14 @@ fn measure_symlink(path: &Path) -> Result { } #[cfg(unix)] -fn recreate_payload_link(source: &Path, destination: &Path) -> Result<(), CacheError> { +fn recreate_payload_link( + source: &Path, + destination: &Path, + payload_link_policy: PayloadLinkPolicy, +) -> Result<(), CacheError> { + if payload_link_policy == PayloadLinkPolicy::Reject { + return Err(CacheError::PayloadSymlinkUnsupported(source.to_path_buf())); + } let target = traced_read_link(source).map_err(|source_error| CacheError::PayloadSymlinkRead { path: source.to_path_buf(), @@ -179,12 +231,19 @@ fn recreate_payload_link(source: &Path, destination: &Path) -> Result<(), CacheE } #[cfg(not(unix))] -fn recreate_payload_link(source: &Path, _destination: &Path) -> Result<(), CacheError> { +fn recreate_payload_link( + source: &Path, + _destination: &Path, + _payload_link_policy: PayloadLinkPolicy, +) -> Result<(), CacheError> { Err(CacheError::PayloadSymlinkUnsupported(source.to_path_buf())) } #[cfg(not(unix))] -fn measure_symlink(path: &Path) -> Result { +fn measure_symlink( + path: &Path, + _payload_link_policy: PayloadLinkPolicy, +) -> Result { Err(CacheError::PayloadSymlinkUnsupported(path.to_path_buf())) } From 72c496082b5851e529acddc972a61951b9098cbf Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sat, 29 Aug 2026 20:28:09 +0200 Subject: [PATCH 24/24] fix: own staging before data preparation --- docs/TESTING_AND_FAULT_INJECTION.md | 2 +- ...29-cache-payload-symlink-implementation.md | 33 +++++++++---- ...2026-08-29-cache-payload-symlink-design.md | 16 +++--- src/cache.rs | 49 ++++++++++++++++++- 4 files changed, 81 insertions(+), 19 deletions(-) diff --git a/docs/TESTING_AND_FAULT_INJECTION.md b/docs/TESTING_AND_FAULT_INJECTION.md index 5e45cbc..07e383e 100644 --- a/docs/TESTING_AND_FAULT_INJECTION.md +++ b/docs/TESTING_AND_FAULT_INJECTION.md @@ -97,7 +97,7 @@ release. Deterministic coverage includes `src/cache.rs::complete_payload_symlinks_are_preserved_across_generation_reuse`, `src/cache.rs::failed_payload_preflight_removes_the_new_staging_generation`, `src/cache.rs::forced_fallback_copy_failure_removes_only_its_owned_staging_generation`, and `src/cache.rs::staging_cleanup_unlinks_payload_links_without_touching_targets`. Payload measurement is covered by `src/cache_payload.rs::payload_measurement_counts_links_without_following_targets`; fallback semantics and complete operation tracing by `src/cache_payload.rs::fallback_copy_preserves_each_link_target_and_external_sentinel` and `src/cache_payload.rs::fallback_copy_records_only_payload_paths_and_every_copy_operation`. -On macOS, deterministic tests distinguish attempted-and-succeeded clone reuse and deliberately forced fallback; the clone-success and forced-fallback tests preserve link identity. Injected fallback-copy failure occurs only after one successful copied object and proves exact owned-staging cleanup. Owner-drop tests preserve unrelated and identity-mismatched staging directories. Unix fallback copy preserves each link target without following it and records metadata, directory enumeration, link reads, regular-file source/destination pairs, directory creation, and link creation without treating either copy path or a link target as a host path. Windows link-bearing payload reuse remains fail-closed and unsupported. These are deterministic source tests, not a native CCP receipt or native qualification. +On macOS, deterministic tests distinguish attempted-and-succeeded clone reuse and deliberately forced fallback; the clone-success and forced-fallback tests preserve link identity. Injected fallback-copy failure occurs only after one successful copied object and proves exact owned-staging cleanup. `src/cache.rs::data_directory_preparation_failure_removes_owned_staging_and_releases_the_entry_lock` and `src/cache.rs::manifest_write_failure_removes_owned_staging_and_releases_the_entry_lock` cover the pre-data-root and pre-manifest failure windows. Owner-drop tests preserve unrelated and identity-mismatched staging directories. Unix fallback copy preserves each link target without following it and records metadata, directory enumeration, link reads, regular-file source/destination pairs, directory creation, and link creation without treating either copy path or a link target as a host path. Windows link-bearing payload reuse remains fail-closed and unsupported. These are deterministic source tests, not a native CCP receipt or native qualification. ## Managed-cache pin contract diff --git a/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md b/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md index 76091a3..4f188f6 100644 --- a/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md +++ b/docs/superpowers/plans/2026-08-29-cache-payload-symlink-implementation.md @@ -10,7 +10,8 @@ cleaning every newly created failed staging generation. no-follow traversal, accounting, and Unix link-preserving copy. Keep cache layout, promotion, recovery, and ownership decisions in `cache.rs`, and call the payload module only after validating an exact plain `data` root. Move the -prepared-generation RAII owner before fallible reuse so cleanup covers both +prepared-generation RAII owner immediately after staging creation, before the +fallible `data`-root preparation and reuse paths, so cleanup covers both pre-manifest and manifest-backed staging phases. **Tech Stack:** Rust 2024, Rust 1.87+, standard-library filesystem APIs, @@ -789,16 +790,28 @@ fn failed_payload_preflight_removes_the_new_staging_generation() { } ``` +Add a second deterministic regression named +`data_directory_preparation_failure_removes_owned_staging_and_releases_the_entry_lock`. +Use a thread-local, one-shot test seam immediately before the staging `data` +directory is created or validated. The test must prove the injected failure +leaves no `.staging-*` directory and that a subsequent preparation can acquire +the same entry lock. + - [ ] **Step 2: Run the regression to prove the observed leak** Run: ```console cargo test --locked cache::tests::failed_payload_preflight_removes_the_new_staging_generation -- --exact +cargo test --locked cache::tests::data_directory_preparation_failure_removes_owned_staging_and_releases_the_entry_lock -- --exact ``` -Expected: FAIL because one `.staging-*` directory remains after -`prepare_entry` returns the preflight error. +Expected at the original Task 4 baseline: both fail because one `.staging-*` +directory remains after `prepare_entry` returns the injected or preflight +error. When applying the later corrective checkpoint, first add the test-only +seam and regression without changing owner ordering; the new test must compile +and fail on the leaked-staging assertion. A compile failure against a revision +that does not yet contain the test seam is not behavioral RED evidence. - [ ] **Step 3: Add phase-aware generation ownership** @@ -820,9 +833,10 @@ struct PreparedCacheGenerationOwner { } ``` -Construct its `Arc` immediately after creating the plain staging and plain -`data` directories, before `remove_if_present`, `try_clone_tree`, or -`copy_payload_tree`. After `write_generation_manifest` succeeds, execute: +Construct its `Arc` immediately after creating the plain staging directory and +before creating or validating its plain `data` directory, calling +`remove_if_present`, `try_clone_tree`, or `copy_payload_tree`. After +`write_generation_manifest` succeeds, execute: ```rust owner.phase.store(PREPARED_PHASE_STAGING, Ordering::Release); @@ -890,6 +904,7 @@ Run: ```console cargo test --locked cache::tests::failed_payload_preflight_removes_the_new_staging_generation -- --exact +cargo test --locked cache::tests::data_directory_preparation_failure_removes_owned_staging_and_releases_the_entry_lock -- --exact cargo test --locked cache::tests::staging_cleanup_unlinks_payload_links_without_touching_targets -- --exact cargo test --locked cache::tests::prepared_entry_clones_share_cleanup_and_lock_until_final_drop -- --exact cargo test --locked cache::tests::active_entry_lock_blocks_a_second_preparation_until_release -- --exact @@ -1045,7 +1060,7 @@ const TESTING_AND_FAULT_INJECTION: &str = include_str!("../docs/TESTING_AND_FAULT_INJECTION.md"); #[test] -fn cache_payload_symlinks_are_documented_as_opaque_unattested_state() { +fn cache_payload_documentation_contract() { for phrase in [ "control plane", "payload plane", @@ -1074,7 +1089,7 @@ fn cache_payload_symlinks_are_documented_as_opaque_unattested_state() { Run: ```console -cargo test --locked --test repository_hygiene_contract cache_payload_symlinks_are_documented_as_opaque_unattested_state -- --exact +cargo test --locked --test repository_hygiene_contract cache_payload_documentation_contract -- --exact ``` Expected: FAIL on the first missing documentation phrase. @@ -1114,7 +1129,7 @@ owns cleanup before reuse, and native candidate qualification remains pending. Run: ```console -cargo test --locked --test repository_hygiene_contract cache_payload_symlinks_are_documented_as_opaque_unattested_state -- --exact +cargo test --locked --test repository_hygiene_contract cache_payload_documentation_contract -- --exact cargo test --locked cache::tests::complete_payload_symlinks_are_preserved_across_generation_reuse -- --exact cargo test --locked cache::tests::failed_payload_preflight_removes_the_new_staging_generation -- --exact cargo test --locked cache::tests::staging_cleanup_unlinks_payload_links_without_touching_targets -- --exact diff --git a/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md b/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md index 1b3f53e..488ba28 100644 --- a/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md +++ b/docs/superpowers/specs/2026-08-29-cache-payload-symlink-design.md @@ -25,8 +25,9 @@ targets therefore have the same host-side treatment: preserve the link object without dereferencing it. The implementation will also construct prepared-generation cleanup ownership -immediately after creating a staging directory and before any fallible clone or -copy. A failed reuse attempt must not leave an unowned `.staging-*` directory. +immediately after creating a staging directory and before creating or +validating its `data` root or attempting any fallible clone or copy. A failed +reuse attempt must not leave an unowned `.staging-*` directory. No configuration, receipt, policy, cache-key, generation-manifest, or journal schema changes are introduced. @@ -221,9 +222,10 @@ Windows evidence. ### Preparation ownership and cleanup -`PreparedCacheGenerationOwner` must be created immediately after both the -plain staging directory and its plain `data` root exist, and before removing -that empty root or attempting clone/copy reuse. It owns: +`PreparedCacheGenerationOwner` must be created immediately after the plain +staging directory exists, before creating or validating its plain `data` root, +removing that empty root, or attempting clone/copy reuse. During the preparing +phase, the `data` root may therefore be absent or incomplete. The owner owns: - the exact staging path; - key digest, plan digest, and generation identity once available; @@ -329,8 +331,8 @@ link once and does not count or size its target. ### Cleanup ownership -- inject failure before clone, during clone preflight, during fallback copy, - and before manifest write; +- inject failure while preparing the staging `data` root, before clone, during + clone preflight, during fallback copy, and before manifest write; - prove the exact owned staging directory is removed on final drop; - prove the entry lock is held through cleanup and then released; - prove unrelated and identity-mismatched staging directories are preserved; diff --git a/src/cache.rs b/src/cache.rs index f749c13..338fdcc 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -483,8 +483,6 @@ impl ManagedCache { let sequence = TEMP_SEQUENCE.fetch_add(1, Ordering::Relaxed); let staging_path = path.join(format!(".staging-{}-{sequence}", std::process::id())); ensure_managed_directory(&staging_path)?; - let data_path = staging_path.join("data"); - ensure_managed_directory(&data_path)?; let owner = Arc::new(PreparedCacheGenerationOwner { entry_path: path.clone(), staging_path: staging_path.clone(), @@ -494,6 +492,14 @@ impl ManagedCache { phase: AtomicU8::new(PREPARED_PHASE_PREPARING), _entry_lock: entry_lock, }); + let data_path = staging_path.join("data"); + #[cfg(test)] + if take_staging_data_directory_preparation_failure_for_test() { + return Err(CacheError::Io(io::Error::other( + "injected staging data directory preparation failure", + ))); + } + ensure_managed_directory(&data_path)?; if complete { let source = path.join("data"); remove_if_present(&data_path)?; @@ -1427,12 +1433,23 @@ fn try_clone_tree( #[cfg(test)] thread_local! { static FORCE_CLONE_FALLBACK: Cell = const { Cell::new(false) }; + static FAIL_STAGING_DATA_DIRECTORY_PREPARATION_ONCE: Cell = const { Cell::new(false) }; static FAIL_GENERATION_MANIFEST_WRITE_ONCE: Cell = const { Cell::new(false) }; static CLONE_OPERATIONS: std::cell::RefCell> = const { std::cell::RefCell::new(Vec::new()) }; } +#[cfg(test)] +fn fail_staging_data_directory_preparation_once_for_test() { + FAIL_STAGING_DATA_DIRECTORY_PREPARATION_ONCE.with(|failure| failure.set(true)); +} + +#[cfg(test)] +fn take_staging_data_directory_preparation_failure_for_test() -> bool { + FAIL_STAGING_DATA_DIRECTORY_PREPARATION_ONCE.with(|failure| failure.replace(false)) +} + #[cfg(test)] fn fail_generation_manifest_write_once_for_test() { FAIL_GENERATION_MANIFEST_WRITE_ONCE.with(|failure| failure.set(true)); @@ -2921,6 +2938,34 @@ timeout_seconds = 60 finish_fixture(fixture); } + #[test] + fn data_directory_preparation_failure_removes_owned_staging_and_releases_the_entry_lock() { + let fixture = completed_entry_fixture("data-directory-preparation-cleanup"); + let plan = envelope(); + let key = CacheKey::for_plan_cache(&plan, &plan.plan.caches[0]).expect("key"); + + fail_staging_data_directory_preparation_once_for_test(); + let preparation = fixture.cache.prepare_entry(&key, &plan.plan_digest, 2); + + assert!(matches!(preparation, Err(CacheError::Io(_)))); + let staging: Vec<_> = fs::read_dir(&fixture.entry_path) + .expect("entry directory") + .flatten() + .filter(|entry| entry.file_name().to_string_lossy().starts_with(".staging-")) + .map(|entry| entry.path()) + .collect(); + let released = fixture.cache.prepare_entry(&key, &plan.plan_digest, 3); + let lock_was_released = released.is_ok(); + drop(released); + finish_fixture(fixture); + + assert!( + staging.is_empty(), + "data preparation failure leaked {staging:?}" + ); + assert!(lock_was_released, "entry lock remained held after failure"); + } + #[cfg(unix)] #[test] fn failed_payload_preflight_removes_the_new_staging_generation() {