diff --git a/go/internal/vfs/localvolume.go b/go/internal/vfs/localvolume.go new file mode 100644 index 00000000..38ca2792 --- /dev/null +++ b/go/internal/vfs/localvolume.go @@ -0,0 +1,857 @@ +package vfs + +// This file is the P2 VolumeManager backend: one directory subtree per session +// on the box's fast local storage, under an operator-configured base dir. It is +// the whole of the volume lifecycle at P2 — create, resolve, attach, stamp, +// reconcile, expire — with the snapshot/archive/restore verbs reserved behind +// honest sentinels (see vfs.go). +// +// The load-bearing part is the close-stamp mechanism, which is what bounds the +// storage leak the parent record's 14-day expiry policy exists to bound. A +// stamp is a small JSON marker in the volume's metadata dir written by the +// teardown path (Stamp) and read by the reaper (Expire), and the backend holds +// three invariants over it: +// +// (a) Attach clears the stamp before returning the path, so a reopened +// closed-but-unexpired session never carries a past-deadline stamp into +// its new life. +// (b) Expire takes a per-volume advisory file lock and RE-READS the stamp +// under it, so a volume is never reaped in the window between the reaper +// reading a stamp and a concurrent Attach clearing it. Attach takes the +// same lock around its clear, and RE-VERIFIES under it that the volume +// still exists — an Attach that was blocked behind the Expire which reaped +// the volume returns ErrVolumeNotFound rather than a path to a deleted +// tree. The lock file lives OUTSIDE the volume root, on a stable inode a +// reap cannot unlink, which is what makes that under-lock verdict +// authoritative across reap+recreate (see lockVolume). +// (c) The stamp carries close-vs-suspend intent, supplied by the caller. A +// suspended session's volume is never eligible however old, because D4's +// suspend uses the same stop+remove teardown path a close does — "the +// container is gone" cannot distinguish them. + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "syscall" + "time" +) + +// metaDirName is the per-volume metadata dir inside a volume root: it holds the +// close-stamp, and it is also this package's VOLUME-IDENTITY TOKEN — a +// directory under the base dir that does not contain it is not a volume this +// package owns, and is never scanned, stamped, or reaped (see eachVolume). It +// lives INSIDE the volume root so that reaping the volume reaps its stamp in +// one os.RemoveAll — no orphan stamp can outlive the volume it describes. The +// per-volume lock file deliberately does NOT live here (see lockFileSuffix): a +// lock inside the reaped subtree cannot serialize against the reap itself. The +// dotted, package-prefixed name keeps it clear of any path a checkout would +// write. +const metaDirName = ".compass-vfs-meta" + +const ( + // stampFileName is the close-stamp marker. Written atomically (temp + + // rename) so the reaper can never observe a half-written stamp and + // mis-decide eligibility. + stampFileName = "close-stamp.json" + // lockFileSuffix names the per-volume advisory lock file (invariant (b)), + // appended to the volume root path so the lock is a FILE SIBLING of the + // volume root dir: /. Outside the + // reaped subtree by construction — see lockVolume for why that placement is + // load-bearing rather than incidental. + lockFileSuffix = ".compass-vfs.lock" + // stampTempPattern names the staging file for an atomic stamp write. It + // lives in the same dir as its target so the rename is same-filesystem. + stampTempPattern = "close-stamp-*.json.tmp" +) + +// volumeDirMode is the mode of the base dir and of every per-session volume +// root: private to the invoking host user, who owns the whole subtree. The +// container's keep-id remap maps that user to the agent uid in-container, so +// owner-only is exactly right — no other host user has any business in a +// session's tree. +const volumeDirMode os.FileMode = 0o700 + +// stampFileMode is the mode of the close-stamp and lock files: owner-only, +// matching the volume root. The stamp is Runner state, never agent-readable +// content. +const stampFileMode os.FileMode = 0o600 + +// closeStamp is the on-disk close-stamp: why the session's volume was stamped +// and when. Both fields are read by Expire — the intent decides eligibility at +// all, the timestamp decides whether the retention window has passed. +type closeStamp struct { + // Intent is the caller-supplied close-vs-suspend bit (invariant (c)). + Intent CloseIntent `json:"intent"` + // StampedAt is when the stamp was written. For a discovered orphan this is + // the DISCOVERY time, not the lost close time — the deadline deliberately + // restarts from discovery (see ReconcileOrphans). + StampedAt time.Time `json:"stampedAt"` +} + +// LocalManager is the P2 VolumeManager backend: a local-directory volume store. +// Every session's volume is the subtree /, so a volume's +// host path is a pure function of the base dir and the session id — which is +// what makes the mount path stable across every launch, resume, and burst of +// that session (P2-GC-d). It holds no per-session state: a fresh LocalManager +// on the same base dir after a Runner restart resolves and re-attaches exactly +// the same volumes. +// +// keep-id ownership invariant (load-bearing): the base dir and every +// per-session subtree are created by the Runner as its OWN invoking host user. +// The container's rootless keep-id remap +// (--userns=keep-id:uid=,gid=) maps a Runner-created root +// to agent-owned in-container, which is what satisfies ensureCheckoutDir's +// precondition — "CheckoutDir's parent must be writable by the agent uid" +// (go/internal/runtime/agent.go:354-357). A base dir placed outside the +// Runner's own ownership (a root-owned /var path, a differently-privileged +// installer's dir) breaks every launch on the volume path. This backend does no +// uid remapping itself: it creates dirs as the current process user, and the +// remap is the container runtime's. +type LocalManager struct { + baseDir string +} + +// LocalManager is the P2 backend behind the frozen seam; the assertion keeps the +// two in lockstep at compile time. +var _ VolumeManager = (*LocalManager)(nil) + +// NewLocalManager establishes the operator-configured base dir under which one +// subtree per session lives, keyed by session id, and returns the backend bound +// to it. The base dir is created if absent with plain os.MkdirAll — owned by the +// invoking host user, per the keep-id ownership invariant documented on +// LocalManager; the Runner runs as its own user and never chowns into another. +// It is an error if the base dir is empty, cannot be created, or exists as +// something other than a directory: a misconfigured base dir must fail at +// construction, not at the first launch on the volume path. +func NewLocalManager(baseDir string) (*LocalManager, error) { + if baseDir == "" { + return nil, errors.New("vfs: base dir must not be empty") + } + if err := os.MkdirAll(baseDir, volumeDirMode); err != nil { + return nil, fmt.Errorf("vfs: creating volume base dir %q: %w", baseDir, err) + } + info, err := os.Stat(baseDir) + if err != nil { + return nil, fmt.Errorf("vfs: inspecting volume base dir %q: %w", baseDir, err) + } + if !info.IsDir() { + return nil, fmt.Errorf("vfs: volume base dir %q is not a directory", baseDir) + } + return &LocalManager{baseDir: baseDir}, nil +} + +// BaseDir is the base dir this manager places volumes under. Exposed so an +// operator-facing diagnostic can report the configured location without the +// caller re-deriving it. +func (m *LocalManager) BaseDir() string { return m.baseDir } + +// CreateVolume creates the session's volume subtree (and its metadata dir) and +// returns the resolved Volume. It is idempotent: for a session that already has +// a volume it returns the existing one untouched, because volume destruction is +// Expire's alone (P2-GC-c) — a re-create must never clear a tree. +// +// The subtree is created as the invoking host user, per the keep-id ownership +// invariant documented on LocalManager: the container's keep-id remap makes this +// Runner-owned root agent-owned in-container, satisfying ensureCheckoutDir's +// "CheckoutDir's parent must be writable by the agent uid" precondition +// (go/internal/runtime/agent.go:354-357). +func (m *LocalManager) CreateVolume(ctx context.Context, sessionID string) (Volume, error) { + root, err := m.volumeRoot(sessionID) + if err != nil { + return Volume{}, err + } + if err := ctx.Err(); err != nil { + return Volume{}, err + } + if err := os.MkdirAll(filepath.Join(root, metaDirName), volumeDirMode); err != nil { + return Volume{}, fmt.Errorf("vfs: creating volume root %q: %w", root, err) + } + return Volume{SessionID: sessionID, HostRoot: root}, nil +} + +// Lookup resolves a session's existing volume or returns ErrVolumeNotFound. It +// is the resolve half of the provision path's resolve-or-create: Attach needs a +// resolved Volume, so a caller cannot produce one from a bare session id +// without this verb. A not-found is an error-shaped signal the provision path +// converts into CreateVolume plus a cold materialize — never a silent recreate +// here, so the cold path stays observable. +func (m *LocalManager) Lookup(ctx context.Context, sessionID string) (Volume, error) { + root, err := m.volumeRoot(sessionID) + if err != nil { + return Volume{}, err + } + if err := ctx.Err(); err != nil { + return Volume{}, err + } + info, err := os.Stat(root) + switch { + case errors.Is(err, os.ErrNotExist): + return Volume{}, fmt.Errorf("vfs: session %q: %w", sessionID, ErrVolumeNotFound) + case err != nil: + return Volume{}, fmt.Errorf("vfs: inspecting volume root %q: %w", root, err) + case !info.IsDir(): + return Volume{}, fmt.Errorf("vfs: volume root %q is not a directory: %w", root, ErrVolumeNotFound) + } + return Volume{SessionID: sessionID, HostRoot: root}, nil +} + +// Attach makes the resolved volume available for mounting and returns its host +// path. The path is derived solely from the base dir and the session id, so it +// is identical on every launch, resume, and burst of that session (P2-GC-d) — +// which is what keeps `target/` and sccache valid across a container's death. +// +// Attach also clears any close-stamp before returning (invariant (a)), under +// the per-volume lock Expire uses (invariant (b)): the clear happens BEFORE the +// caller can mount the volume, so no window exists in which the volume is both +// attached-live and still carrying a past-deadline stamp. A stamp that is +// already absent is already-clear, not an error. +// +// Existence is decided UNDER the lock, and that is the load-bearing part. An +// Attach that arrives exactly at a volume's expiry deadline blocks on the lock +// Expire is holding, and the Expire it is waiting behind may reap the volume +// before releasing. The unlocked pre-check below is therefore a fast path only, +// NOT the authority: its verdict is already stale by the time the lock is won. +// Re-stating the check under the lock is what turns that race into an honest +// ErrVolumeNotFound — which the provision path converts into a cold +// CreateVolume plus materialize — rather than a nil error carrying the path of +// a directory that no longer exists. It is only authoritative because the lock +// file lives OUTSIDE the reaped subtree (see lockVolume): a lock inside the +// volume root would be unlinked by the reap, so the winning Attach would hold a +// lock on a dead inode that excludes nobody. +// +// Both error paths JOIN the lock-release error, exactly as Stamp does. A +// release that failed would leave the flock held for this process's lifetime, +// so every later Expire pass would skip this volume forever — an unbounded +// leak in the mechanism built to bound one, and a silent contradiction of +// release()'s own contract. The typed ErrVolumeNotFound stays +// errors.Is-detectable through such a join, so the provision path's +// cold-materialize branch is preserved. +func (m *LocalManager) Attach(ctx context.Context, v Volume) (string, error) { + // Fast pre-check: an Attach of a session that never had a volume fails here + // without touching the lock namespace. Not the authority — see above. + resolved, err := m.Lookup(ctx, v.SessionID) + if err != nil { + return "", err + } + // Block rather than skip: an Attach racing the reaper must win the volume, + // not abandon a launch. Expire holds the lock only for the duration of one + // volume's re-verify-and-reap, so the wait is bounded. + lock, err := lockVolume(resolved.HostRoot, true) + if err != nil { + return "", err + } + if lock == nil { + // Unreachable with block=true, which never reports contention; guard + // anyway so a future non-blocking caller cannot nil-deref. + return "", fmt.Errorf("vfs: session %q: volume lock unavailable", v.SessionID) + } + existErr := requireVolumeRoot(resolved.HostRoot, v.SessionID) + var clearErr error + if existErr == nil { + clearErr = clearStamp(resolved.HostRoot) + } + releaseErr := lock.release() + if existErr != nil { + return "", errors.Join(existErr, releaseErr) + } + if clearErr != nil { + return "", errors.Join(clearErr, releaseErr) + } + if releaseErr != nil { + return "", releaseErr + } + return resolved.HostRoot, nil +} + +// requireVolumeRoot reports ErrVolumeNotFound unless root is still a directory. +// Called under the per-volume lock by Attach and Stamp, it is the authoritative +// existence check: the volume cannot be reaped while the caller holds the lock, +// so a root that is present here stays present for the rest of the critical +// section. A root that is ABSENT here was reaped by the Expire the caller was +// blocked behind, and must surface as the typed not-found signal so the +// provision path cold-materializes instead of mounting a deleted path. +func requireVolumeRoot(root, sessionID string) error { + info, err := os.Stat(root) + switch { + case errors.Is(err, os.ErrNotExist): + return fmt.Errorf("vfs: session %q: volume %q was reaped: %w", sessionID, root, ErrVolumeNotFound) + case err != nil: + return fmt.Errorf("vfs: inspecting volume root %q: %w", root, err) + case !info.IsDir(): + return fmt.Errorf("vfs: volume root %q is not a directory: %w", root, ErrVolumeNotFound) + } + return nil +} + +// Stamp records the caller's close-vs-suspend intent on the volume, with the +// current time as the stamp timestamp. This is the teardown path's half of the +// close-stamp mechanism: the W5 teardown calls it after stopping and removing +// the container, and Expire reads what it wrote. +// +// The intent MUST come from the caller (invariant (c)). D4's suspend uses the +// same stop+remove teardown path a close does, so "the container is gone" +// cannot distinguish them; only the caller knows whether the session is closed +// for good or expected back. Stamping IntentSuspended pins the volume against +// the reaper however old it gets. +// +// The write is atomic (temp file + rename in the metadata dir), so a reaper +// reading concurrently sees either the old stamp or the new one, never a torn +// record. +// +// The write happens under the per-volume lock, like every other stamp mutation +// (invariant (b)): Attach's clear and the reaper's read already serialize on +// that lock, and stamping under it too closes the one remaining gap — a +// teardown stamp landing in the middle of a reap could otherwise re-create the +// metadata dir writeStamp needs inside a subtree Expire is deleting, leaving an +// empty resurrected root behind. Existence is re-checked under the lock for the +// same reason Attach re-checks it: a volume reaped while this call was blocked +// must surface as ErrVolumeNotFound, not be silently resurrected as an empty +// stamped shell the reaper would then have to re-reap. +func (m *LocalManager) Stamp(ctx context.Context, v Volume, intent CloseIntent) error { + resolved, err := m.Lookup(ctx, v.SessionID) + if err != nil { + return err + } + lock, err := lockVolume(resolved.HostRoot, true) + if err != nil { + return err + } + if lock == nil { + // Unreachable with block=true; guarded so a future non-blocking caller + // cannot nil-deref. + return fmt.Errorf("vfs: session %q: volume lock unavailable", v.SessionID) + } + writeErr := requireVolumeRoot(resolved.HostRoot, v.SessionID) + if writeErr == nil { + writeErr = writeStamp(resolved.HostRoot, closeStamp{Intent: intent, StampedAt: time.Now()}) + } + return errors.Join(writeErr, lock.release()) +} + +// ReadStamp reports the volume's current close-stamp: ok is false when the +// volume is unstamped (a live session's volume, or a crash orphan not yet +// reconciled). Exported for the teardown/expiry driver's diagnostics and for +// the tests that assert the three invariants; it takes no lock, so a caller +// deciding to REAP must re-read under the lock as Expire does. +func (m *LocalManager) ReadStamp(ctx context.Context, v Volume) (intent CloseIntent, stampedAt time.Time, ok bool, err error) { + resolved, lookupErr := m.Lookup(ctx, v.SessionID) + if lookupErr != nil { + return 0, time.Time{}, false, lookupErr + } + stamp, readErr := readStamp(resolved.HostRoot) + if readErr != nil { + return 0, time.Time{}, false, readErr + } + if stamp == nil { + return 0, time.Time{}, false, nil + } + return stamp.Intent, stamp.StampedAt, true, nil +} + +// ReconcileOrphans is the startup pass over the base dir: it stamps every +// UNSTAMPED volume closed at discovery time. A crash between container-remove +// and stamp-write leaves an unstamped closed volume, which Expire — which +// treats an unstamped volume as a live session's — would never reach; this pass +// makes every volume reachable by the reaper. +// +// The deadline of a discovered-orphan stamp runs from DISCOVERY, not from the +// lost close, so the volume survives one full retention window past this pass. +// That is the deliberate trade: a crash fails SAFE (reaped one window late), +// never OPEN (some volume the reaper can never see) and never WRONG (invariant +// (a) undoes a discovery stamp for free the moment that session is +// re-provisioned and Attached before the deadline; and a suspended session that +// never crashed was stamped IntentSuspended by its normal teardown, so this +// pass — which only touches UNSTAMPED volumes — leaves it alone). +// +// This needs no Server query and by design cannot want one: the Runner, not the +// Server, is authoritative for live-session truth, RunnerService exposes no +// session-query verb, and the Server's session bindings are cleared at every +// enroll — so the Server's live-session map is empty exactly when a restart +// would consult it. This package therefore takes no RPC or server dependency. +// +// PRECONDITION (normative): this is a STARTUP-ONLY pass, and it MUST complete +// before the manager serves ANY Attach — not merely before the first Expire. A +// crash orphan is by definition a volume with no live session, so running this +// pass while live Attaches are in flight is a CALLER error. Attach holds the +// per-volume lock only across its under-lock existence re-check and its stamp +// clear, so a pass that reaches a volume immediately AFTER that release sees an +// unstamped volume, wins the lock, and stamps a RUNNING session's volume +// IntentClosed — which invariant (a) cannot undo, because the Attach that +// would have cleared it has already happened. Enforcing this at the type level +// (the manager refusing Attach until the pass has run, or folding the pass into +// construction) is the W5/W6 wiring's concern; this method's precondition is +// simply "no concurrent Attach". +// +// The pass is SEPARATE from Expire rather than folded into it, and the ordering +// contract is: the expiry driver (W6) calls ReconcileOrphans once at startup, +// before it serves any Attach and before its first Expire. Keeping them apart +// is what makes "unstamped means live" a single, honest rule inside Expire — +// folding the scan in would make every Expire pass able to stamp a volume it is +// simultaneously judging, and would make a mid-session Expire (the ticker's +// steady state, when unstamped volumes ARE live sessions) stamp live sessions +// closed. +// +// A volume whose lock is held (a concurrent Attach) is skipped: it is being +// attached-live, which is the opposite of orphaned. Like Expire, the pass locks +// first and reads the stamp only under the lock, so it can never stamp a volume +// closed on the strength of a read an in-flight Attach has already invalidated. +func (m *LocalManager) ReconcileOrphans(ctx context.Context) error { + discoveredAt := time.Now() + return m.eachVolume(ctx, func(root string) error { + lock, err := lockVolume(root, false) + if err != nil { + return err + } + if lock == nil { + return nil // held by a live Attach; not an orphan. + } + writeErr := stampOrphanLocked(root, discoveredAt) + releaseErr := lock.release() + return errors.Join(writeErr, releaseErr) + }) +} + +// stampOrphanLocked writes the discovery stamp for an orphan under the held +// per-volume lock, after confirming under that lock that the volume is still +// unstamped. An already-stamped volume is left exactly as its teardown wrote +// it: this pass exists only to make UNSTAMPED volumes reachable by the reaper, +// and rewriting a suspended session's stamp would make it reapable. +func stampOrphanLocked(root string, discoveredAt time.Time) error { + // A volume reaped out from under this pass — between eachVolume's marker + // stat and this lock acquisition — is not an orphan to stamp. Guard the + // mutation the way Attach and Stamp already guard theirs: without this, + // writeStamp's os.MkdirAll would resurrect the reaped root's shell, Lookup + // would then succeed on a reaped session, and the provision path would warm- + // Attach an EMPTY volume instead of cold-materializing — silently defeating + // the not-found-is-an-observable-signal contract. + if err := requireVolumeRoot(root, filepath.Base(root)); err != nil { + if errors.Is(err, ErrVolumeNotFound) { + return nil + } + return err + } + stamp, err := readStamp(root) + if err != nil { + return err + } + if stamp != nil { + return nil + } + return writeStamp(root, closeStamp{Intent: IntentClosed, StampedAt: discoveredAt}) +} + +// Expire reaps volumes whose session is closed AND whose close-stamp is older +// than olderThan. It is the ONLY path that destroys volume contents (P2-GC-c): +// Release, Teardown, eviction, crash, and failed launches never do. +// +// Three volumes are ineligible by construction. An UNSTAMPED volume belongs to +// a live session (a crash orphan is made stamped by ReconcileOrphans, not +// here — see that method for why the passes are separate). A volume stamped +// IntentSuspended is never eligible however old, because its session is +// expected to resume onto exactly this volume at exactly this path. And a +// volume whose stamp is within olderThan is inside its retention window. +// +// Every eligibility decision is made UNDER the per-volume lock (invariant (b)): +// the pass locks a volume first and only then reads its stamp, so there is no +// unlocked read whose verdict could go stale. That ordering is what closes the +// window between reading a stamp and a concurrent Attach clearing it — Attach +// takes the same lock around its clear — and it is structural rather than a +// discipline this call site could lose: there is no code path here that can +// decide to reap from an unlocked read, because no unlocked read exists. +// +// A volume whose lock is held is SKIPPED this pass, not an error: contention +// means someone is attaching it, which is precisely the signal not to reap, and +// the next pass revisits it. +// +// A per-volume failure does not abort the pass: errors are accumulated and +// joined, so one unreadable volume cannot pin the storage of every volume +// behind it. +func (m *LocalManager) Expire(ctx context.Context, olderThan time.Duration) error { + now := time.Now() + return m.eachVolume(ctx, func(root string) error { + lock, err := lockVolume(root, false) + if err != nil { + return err + } + if lock == nil { + return nil // a live Attach holds it; skip, do not reap. + } + reapErr := reapLocked(root, now, olderThan) + releaseErr := lock.release() + return errors.Join(reapErr, releaseErr) + }) +} + +// reapLocked reads the stamp under the held per-volume lock, and deletes the +// volume subtree only if that under-lock read says it is eligible. This is the +// only place a volume is destroyed (P2-GC-c), and it is reachable only with the +// lock held (invariant (b)): reading the stamp here rather than at the call +// site is what guarantees the verdict cannot be stale, because a concurrent +// Attach must hold this same lock to clear a stamp. +// +// It removes ONLY the volume root. The lock file the caller is holding is a +// sibling of that root, not a path inside it, so this removal cannot unlink the +// inode the lock lives on — which is what keeps the lock excluding a blocked +// Attach across the reap, and lets that Attach observe the absence and return +// ErrVolumeNotFound (see lockVolume and Attach). +func reapLocked(root string, now time.Time, olderThan time.Duration) error { + stamp, err := readStamp(root) + if err != nil { + return err + } + if !eligible(stamp, now, olderThan) { + return nil + } + if err := os.RemoveAll(root); err != nil { + return fmt.Errorf("vfs: reaping expired volume %q: %w", root, err) + } + return nil +} + +// eligible reports whether a stamp makes its volume reap-eligible: it must +// exist (an unstamped volume is a live session's), carry IntentClosed exactly +// (IntentSuspended — and any unrecognized intent, which a corrupt stamp could +// carry — pins the volume rather than risking a wrong reap), and be older than +// the retention window. +func eligible(stamp *closeStamp, now time.Time, olderThan time.Duration) bool { + if stamp == nil || stamp.Intent != IntentClosed { + return false + } + return now.Sub(stamp.StampedAt) > olderThan +} + +// Snapshot returns the not-implemented sentinel: the verb is reserved in +// VolumeManager, but the snapshot store, the reflink/rsync copy primitive, and +// the (AgentAccountID, repo) index the provision path reads are W2's to land +// behind this signature. Honest failure over a silent no-op — a caller must not +// read an empty VolumeSnapshotID as a stored snapshot. +func (m *LocalManager) Snapshot(ctx context.Context, v Volume) (VolumeSnapshotID, error) { + return "", ErrSnapshotNotImplemented +} + +// Archive returns the not-implemented sentinel: the verb's consumer is D4's +// cold-idle (OQ-2), so P2 freezes the signature and defers the object-store +// implementation. +func (m *LocalManager) Archive(ctx context.Context, v Volume) (ArchiveRef, error) { + return "", ErrArchiveNotImplemented +} + +// Restore returns the not-implemented sentinel, for the same reason as Archive +// (OQ-2): a caller must not read a zero Volume as a rehydrated one. +func (m *LocalManager) Restore(ctx context.Context, ref ArchiveRef) (Volume, error) { + return Volume{}, ErrRestoreNotImplemented +} + +// volumeRoot resolves a session's volume root, rejecting any session id that +// could escape the base dir or collide with the lock-file namespace. Session +// ids reaching this package are already sanitized internal ids, so this is +// defense in depth on the one operation that deletes a subtree: the base dir is +// the only place this package may ever create or reap, and a `..` or a +// separator in a session id would break that. A session id ENDING in +// lockFileSuffix is rejected too: its volume root would be another session's +// sibling lock-file path, so a reap of one would target the other's lock. +func (m *LocalManager) volumeRoot(sessionID string) (string, error) { + if sessionID == "" { + return "", fmt.Errorf("%w: empty", ErrInvalidSessionID) + } + if sessionID == "." || sessionID == ".." || + strings.ContainsRune(sessionID, '/') || + strings.ContainsRune(sessionID, os.PathSeparator) || + strings.ContainsRune(sessionID, 0) { + return "", fmt.Errorf("%w: %q contains a path separator or traversal element", ErrInvalidSessionID, sessionID) + } + if strings.HasSuffix(sessionID, lockFileSuffix) { + return "", fmt.Errorf("%w: %q collides with the volume lock-file namespace %q", ErrInvalidSessionID, sessionID, lockFileSuffix) + } + return filepath.Join(m.baseDir, sessionID), nil +} + +// eachVolume runs fn against every volume root under the base dir, joining +// per-volume errors instead of aborting on the first: one unreadable or locked +// volume must not pin every volume behind it. +// +// Volume identity here is STRUCTURAL, not positional: an entry is one of this +// package's volumes only if it is a directory AND contains the metaDirName +// marker dir that CreateVolume writes. That marker is the volume-identity +// token, not merely the stamp's container — a directory under the base dir +// without it does not belong to this package and is never stamped or reaped. +// The base dir is not this package's exclusively: the frozen record places W2's +// snapshot store as a sibling subtree under the same base dir keyed by +// VolumeSnapshotID, and without this check the first ReconcileOrphans would +// stamp that store closed and the next Expire past the window would silently +// delete it. Crash orphans keep their marker (CreateVolume wrote it before the +// crash), so reconciliation of genuine orphans is unaffected. Non-directory +// entries are skipped for the same reason — the sibling per-volume lock files +// live in this dir (see lockVolume), and a stray file is not a volume. +func (m *LocalManager) eachVolume(ctx context.Context, fn func(root string) error) error { + entries, err := os.ReadDir(m.baseDir) + if err != nil { + return fmt.Errorf("vfs: scanning volume base dir %q: %w", m.baseDir, err) + } + var errs []error + for _, entry := range entries { + if ctxErr := ctx.Err(); ctxErr != nil { + errs = append(errs, ctxErr) + break + } + if !entry.IsDir() { + continue + } + root := filepath.Join(m.baseDir, entry.Name()) + marker, statErr := os.Stat(filepath.Join(root, metaDirName)) + if statErr != nil { + if !errors.Is(statErr, os.ErrNotExist) { + // Surfaced, not swallowed: an unreadable marker must not be + // silently read as either "a volume" or "not a volume". + errs = append(errs, fmt.Errorf("vfs: inspecting volume marker in %q: %w", root, statErr)) + } + continue + } + if !marker.IsDir() { + continue + } + if err := fn(root); err != nil { + errs = append(errs, err) + } + } + return errors.Join(errs...) +} + +// metaDir is the volume's metadata dir, where the close-stamp lives. The +// per-volume lock file is deliberately NOT here — it is a sibling of the volume +// root (see lockVolume). +func metaDir(root string) string { return filepath.Join(root, metaDirName) } + +// stampPath is the volume's close-stamp file. +func stampPath(root string) string { return filepath.Join(metaDir(root), stampFileName) } + +// readStamp decodes the volume's close-stamp. A nil stamp with a nil error +// means UNSTAMPED — the volume belongs to a live session, or is a crash orphan +// ReconcileOrphans has not yet discovered. Any other read or decode failure is +// returned: a stamp that exists but cannot be understood must not silently +// collapse into "unstamped" (which would look live) or into a defaulted +// IntentClosed (which would look reapable). +func readStamp(root string) (*closeStamp, error) { + path := stampPath(root) + data, err := os.ReadFile(path) //nolint:gosec // G304: path is confined to the operator-configured base dir — either a traversal-checked session id or a ReadDir entry name of that dir, never caller-supplied + if errors.Is(err, os.ErrNotExist) { + return nil, nil //nolint:nilnil // an absent stamp is the UNSTAMPED signal (documented on readStamp), not an error — every caller branches on the nil stamp + } + if err != nil { + return nil, fmt.Errorf("vfs: reading close stamp %q: %w", path, err) + } + var stamp closeStamp + if err := json.Unmarshal(data, &stamp); err != nil { + return nil, fmt.Errorf("vfs: decoding close stamp %q: %w", path, err) + } + return &stamp, nil +} + +// writeStamp writes the volume's close-stamp atomically AND durably: encode, +// stage to a temp file in the same metadata dir, fsync+rename over the target, +// then fsync the containing dir so the rename itself survives a host crash. +// The rename is what makes a concurrent reader see either the old stamp or the +// new one and never a torn record, and staging in the same dir keeps it a +// same-filesystem rename. The durability is load-bearing rather than belt-and- +// braces: losing an IntentClosed stamp in a crash writeback window is benign +// (ReconcileOrphans re-stamps it), but losing an IntentSuspended stamp fails +// WRONG — the volume returns UNSTAMPED, gets stamped IntentClosed at the next +// discovery, and the suspended session's volume that invariant (c) exists to +// pin forever becomes reap-eligible. +func writeStamp(root string, stamp closeStamp) error { + dir := metaDir(root) + if err := os.MkdirAll(dir, volumeDirMode); err != nil { + return fmt.Errorf("vfs: creating volume metadata dir %q: %w", dir, err) + } + data, err := json.Marshal(stamp) + if err != nil { + return fmt.Errorf("vfs: encoding close stamp for %q: %w", root, err) + } + tmp, err := os.CreateTemp(dir, stampTempPattern) + if err != nil { + return fmt.Errorf("vfs: staging close stamp in %q: %w", dir, err) + } + tmpName := tmp.Name() + writeErr := writeAndClose(tmp, data) + if writeErr != nil { + // Best-effort cleanup of the staging file; the write error is the + // actionable one, so a failed unlink is joined rather than masking it. + if rmErr := os.Remove(tmpName); rmErr != nil && !errors.Is(rmErr, os.ErrNotExist) { + return errors.Join(writeErr, fmt.Errorf("vfs: removing staged close stamp %q: %w", tmpName, rmErr)) + } + return writeErr + } + if err := os.Rename(tmpName, stampPath(root)); err != nil { + if rmErr := os.Remove(tmpName); rmErr != nil && !errors.Is(rmErr, os.ErrNotExist) { + return errors.Join( + fmt.Errorf("vfs: committing close stamp for %q: %w", root, err), + fmt.Errorf("vfs: removing staged close stamp %q: %w", tmpName, rmErr), + ) + } + return fmt.Errorf("vfs: committing close stamp for %q: %w", root, err) + } + return syncDir(dir) +} + +// syncDir fsyncs a directory so a rename committed inside it is durable, not +// merely atomic. The staged file's own bytes are fsynced by writeAndClose; the +// directory entry the rename created needs its own fsync, which is the standard +// durable-rename pairing. Both the open and the close are reported: a +// half-reported fsync would put the "never WRONG" claim on ReconcileOrphans +// back on hope. +func syncDir(dir string) error { + d, err := os.Open(dir) //nolint:gosec // G304: path is this package's own metadata dir under the operator-configured base dir, derived from a volume root, never caller-supplied + if err != nil { + return fmt.Errorf("vfs: opening volume metadata dir %q to fsync: %w", dir, err) + } + var errs []error + if syncErr := d.Sync(); syncErr != nil { + errs = append(errs, fmt.Errorf("vfs: fsyncing volume metadata dir %q: %w", dir, syncErr)) + } + if closeErr := d.Close(); closeErr != nil { + errs = append(errs, fmt.Errorf("vfs: closing volume metadata dir %q: %w", dir, closeErr)) + } + return errors.Join(errs...) +} + +// writeAndClose writes data to f, pins the owner-only mode independent of the +// Runner's umask, fsyncs it, and closes it — reporting every failure. The +// fsync is what makes the staged bytes durable before writeStamp renames them +// into place, so a host crash in the writeback window cannot lose a stamp (see +// writeStamp for why losing an IntentSuspended stamp fails WRONG). The Close +// error is handled, not discarded: on a written file it can carry the flush +// failure that means the bytes never landed, which for a stamp would mean a +// volume the reaper mis-judges. +func writeAndClose(f *os.File, data []byte) error { + name := f.Name() + writeErr := func() error { + if _, err := f.Write(data); err != nil { + return fmt.Errorf("vfs: writing close stamp %q: %w", name, err) + } + if err := f.Chmod(stampFileMode); err != nil { + return fmt.Errorf("vfs: pinning close stamp mode %q: %w", name, err) + } + if err := f.Sync(); err != nil { + return fmt.Errorf("vfs: fsyncing close stamp %q: %w", name, err) + } + return nil + }() + closeErr := f.Close() + if closeErr != nil { + closeErr = fmt.Errorf("vfs: closing close stamp %q: %w", name, closeErr) + } + return errors.Join(writeErr, closeErr) +} + +// clearStamp removes the volume's close-stamp (invariant (a)), atomically AND +// durably. An absent stamp is already-clear, not a failure: Attach of a volume +// that was never stamped — the common case, a resume of a still-live session — +// must succeed. +// +// The unlink is fsynced (its containing metadata dir) with the same durability +// writeStamp gives the write, because invariant (a) is exactly as load-bearing +// as (c): a host crash shortly after an Attach that lost the clear would +// resurrect the past-deadline stamp with its ORIGINAL timestamp, and the next +// Expire would reap a volume belonging to the session that just re-attached — +// the loss of the warm tree this package exists to preserve. +func clearStamp(root string) error { + path := stampPath(root) + if err := os.Remove(path); err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil // already clear; nothing to make durable. + } + return fmt.Errorf("vfs: clearing close stamp %q: %w", path, err) + } + return syncDir(filepath.Dir(path)) +} + +// volumeLock is a held per-volume advisory file lock. It owns the open fd the +// lock lives on, so it must be released exactly once (release is idempotent +// against a double call). +type volumeLock struct { + f *os.File +} + +// lockVolume acquires the volume's per-volume advisory lock (invariant (b)) on +// a dedicated lock file that is a FILE SIBLING of the volume root — +// /, never a path inside the root. An OS +// advisory lock — not an in-process mutex — because the two contenders may be +// different processes: a restarted Runner's expiry driver and a live Runner's +// Attach, or a hand-run reaper beside the service. +// +// The placement OUTSIDE the volume root is load-bearing, not cosmetic. flock is +// a lock on an INODE, reached through an open fd, and reapLocked's +// os.RemoveAll(root) unlinks everything inside the root. With the lock file in +// the root's metadata dir, a reap would unlink the very inode a blocked Attach +// was waiting on: that Attach would then unblock holding an exclusive lock on a +// dead inode, and the next lockVolume — after a cold CreateVolume recreated the +// root — would open a NEW inode and lock that, so two actors would hold "the" +// volume lock simultaneously and mutual exclusion would be gone across a +// reap+recreate. On the sibling path the inode is stable across any number of +// reap/recreate cycles, which is what lets Attach and Stamp treat an under-lock +// os.Stat of the root as authoritative. +// +// For the same reason lockVolume creates ONLY the lock file. Its parent, the +// base dir, already exists from NewLocalManager; it must never MkdirAll +// anything inside the volume root, because acquiring a lock that RESURRECTS an +// empty metadata dir inside a reaped root would make the under-lock existence +// check see a live volume that no longer has any contents. +// +// Accepted tradeoff: a sibling lock file outlives its volume's reap — one +// empty, zero-content, owner-only file per session id ever seen on this box, +// which eachVolume skips (it iterates directories only) and which a later +// lockVolume simply reuses. That leak is the price of a stable lock inode, and +// it is the right trade: the alternative loses mutual exclusion. A future +// maintenance pass could unlink orphan lock files whose volume root is absent, +// but only while not under contention (unlinking a lock file someone holds +// recreates exactly the two-inode split described above); that pass is out of +// P2 scope. +func lockVolume(root string, block bool) (*volumeLock, error) { + path := root + lockFileSuffix + f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, stampFileMode) //nolint:gosec // G304: path is this package's own lock file under the operator-configured base dir, derived from a traversal-checked session id, never caller input + if err != nil { + return nil, fmt.Errorf("vfs: opening volume lock %q: %w", path, err) + } + how := syscall.LOCK_EX + if !block { + how |= syscall.LOCK_NB + } + if err := syscall.Flock(int(f.Fd()), how); err != nil { + closeErr := f.Close() + if closeErr != nil { + closeErr = fmt.Errorf("vfs: closing volume lock %q: %w", path, closeErr) + } + if !block && (errors.Is(err, syscall.EWOULDBLOCK) || errors.Is(err, syscall.EAGAIN)) { + // Contended: the caller skips this volume. Report the close failure + // if there was one — a leaked fd is a real defect — but not the + // expected contention. + return nil, closeErr + } + return nil, errors.Join(fmt.Errorf("vfs: locking volume %q: %w", path, err), closeErr) + } + return &volumeLock{f: f}, nil +} + +// release drops the advisory lock and closes its fd. Both failures are +// reported: an unreleased lock would make every later Expire pass skip this +// volume, and a leaked fd is a real defect — neither is "not actionable". +func (l *volumeLock) release() error { + if l == nil || l.f == nil { + return nil + } + f := l.f + l.f = nil + var errs []error + if err := syscall.Flock(int(f.Fd()), syscall.LOCK_UN); err != nil { + errs = append(errs, fmt.Errorf("vfs: unlocking volume lock %q: %w", f.Name(), err)) + } + if err := f.Close(); err != nil { + errs = append(errs, fmt.Errorf("vfs: closing volume lock %q: %w", f.Name(), err)) + } + return errors.Join(errs...) +} diff --git a/go/internal/vfs/localvolume_test.go b/go/internal/vfs/localvolume_test.go new file mode 100644 index 00000000..11d0fa37 --- /dev/null +++ b/go/internal/vfs/localvolume_test.go @@ -0,0 +1,1024 @@ +package vfs + +import ( + "errors" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +// newManager builds a LocalManager over a fresh tempdir base. Every test is +// hermetic: no shared state, no fixed paths, no wall-clock waits — stamp +// timestamps are constructed directly so eligibility is a pure function of +// values the test chose. +func newManager(t *testing.T) *LocalManager { + t.Helper() + m, err := NewLocalManager(filepath.Join(t.TempDir(), "volumes")) + if err != nil { + t.Fatalf("NewLocalManager: %v", err) + } + return m +} + +// mustCreate creates a session's volume, failing the test on error. +func mustCreate(t *testing.T, m *LocalManager, sessionID string) Volume { + t.Helper() + v, err := m.CreateVolume(t.Context(), sessionID) + if err != nil { + t.Fatalf("CreateVolume(%q): %v", sessionID, err) + } + return v +} + +// mustAttach attaches a volume, failing the test on error. +func mustAttach(t *testing.T, m *LocalManager, v Volume) string { + t.Helper() + path, err := m.Attach(t.Context(), v) + if err != nil { + t.Fatalf("Attach(%q): %v", v.SessionID, err) + } + return path +} + +// stampAged writes a close-stamp with an explicitly-constructed timestamp, so +// the test controls eligibility without sleeping. This is the on-disk shape the +// teardown path's Stamp writes; only the clock is the test's. +func stampAged(t *testing.T, v Volume, intent CloseIntent, age time.Duration) { + t.Helper() + if err := writeStamp(v.HostRoot, closeStamp{Intent: intent, StampedAt: time.Now().Add(-age)}); err != nil { + t.Fatalf("writing %s stamp aged %s: %v", intent, age, err) + } +} + +// exists reports whether a path is present, failing the test on any stat error +// other than not-exist (which would otherwise read as a successful reap). +func exists(t *testing.T, path string) bool { + t.Helper() + _, err := os.Stat(path) + switch { + case err == nil: + return true + case errors.Is(err, os.ErrNotExist): + return false + default: + t.Fatalf("stat %q: %v", path, err) + return false + } +} + +// TestNewLocalManagerEstablishesBaseDir pins construction: the base dir is +// created if absent, and a base dir that cannot be a directory fails at +// construction rather than at the first launch on the volume path. +func TestNewLocalManagerEstablishesBaseDir(t *testing.T) { + t.Run("creates an absent base dir", func(t *testing.T) { + base := filepath.Join(t.TempDir(), "nested", "volumes") + m, err := NewLocalManager(base) + if err != nil { + t.Fatalf("NewLocalManager: %v", err) + } + if m.BaseDir() != base { + t.Errorf("BaseDir() = %q, want %q", m.BaseDir(), base) + } + info, err := os.Stat(base) + if err != nil { + t.Fatalf("base dir not created: %v", err) + } + if !info.IsDir() { + t.Errorf("base dir %q is not a directory", base) + } + }) + + t.Run("rejects a base dir that is a file", func(t *testing.T) { + file := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(file, []byte("x"), 0o600); err != nil { + t.Fatalf("writing fixture: %v", err) + } + if _, err := NewLocalManager(file); err == nil { + t.Fatal("NewLocalManager over a regular file succeeded, want an error") + } + }) + + t.Run("rejects an empty base dir", func(t *testing.T) { + if _, err := NewLocalManager(""); err == nil { + t.Fatal("NewLocalManager(\"\") succeeded, want an error") + } + }) +} + +// TestAttachReturnsAStablePath is the P2-GC-d contract: the host path of a +// session's volume is identical on every attach, so `target/` and sccache stay +// valid. A backend that derived any part of the path per-launch would fail here. +func TestAttachReturnsAStablePath(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-stable") + + first := mustAttach(t, m, v) + second := mustAttach(t, m, v) + + if first != second { + t.Errorf("Attach twice returned %q then %q, want one stable path", first, second) + } + if first != v.HostRoot { + t.Errorf("Attach returned %q, want the volume's HostRoot %q", first, v.HostRoot) + } + if !exists(t, first) { + t.Errorf("attached path %q does not exist", first) + } +} + +// TestLookupRoundTripsAndTypesNotFound pins the resolve half of the provision +// path's resolve-or-create: a created volume round-trips with its HostRoot, and +// an absent session is an errors.Is-detectable ErrVolumeNotFound — an +// error-shaped signal the provision path turns into a cold create, never a +// silent recreate here. +func TestLookupRoundTripsAndTypesNotFound(t *testing.T) { + m := newManager(t) + created := mustCreate(t, m, "sess-lookup") + + resolved, err := m.Lookup(t.Context(), "sess-lookup") + if err != nil { + t.Fatalf("Lookup of a created volume: %v", err) + } + if resolved != created { + t.Errorf("Lookup returned %+v, want %+v", resolved, created) + } + + _, err = m.Lookup(t.Context(), "sess-never-created") + if !errors.Is(err, ErrVolumeNotFound) { + t.Errorf("Lookup of an absent session = %v, want ErrVolumeNotFound", err) + } + + // Attach cannot invent a volume either: an unresolvable session id fails + // with the same typed error rather than creating a subtree. + if _, err := m.Attach(t.Context(), Volume{SessionID: "sess-never-created"}); !errors.Is(err, ErrVolumeNotFound) { + t.Errorf("Attach of an absent session = %v, want ErrVolumeNotFound", err) + } +} + +// TestVolumeRootRejectsTraversal guards the one operation that deletes a +// subtree: a session id carrying a separator or a traversal element must never +// resolve to a path outside the base dir, and a session id ENDING in +// lockFileSuffix must never resolve at all — its volume root would BE another +// session's sibling lock-file path, so a reap of one would target the other's +// lock. Each case asserts the typed ErrInvalidSessionID rather than merely a +// non-nil error, so a rejection for an unrelated reason cannot pass for the +// guard. +func TestVolumeRootRejectsTraversal(t *testing.T) { + m := newManager(t) + badIDs := []string{ + "", "..", ".", "../escape", "a/b", "sess/../../etc", + // The lock-namespace collision cases: without volumeRoot's + // HasSuffix check these resolve to a path in the lock namespace. + "sess-x" + lockFileSuffix, + lockFileSuffix, + } + for _, bad := range badIDs { + if _, err := m.CreateVolume(t.Context(), bad); !errors.Is(err, ErrInvalidSessionID) { + t.Errorf("CreateVolume(%q) = %v, want ErrInvalidSessionID", bad, err) + } + if _, err := m.Lookup(t.Context(), bad); !errors.Is(err, ErrInvalidSessionID) { + t.Errorf("Lookup(%q) = %v, want ErrInvalidSessionID", bad, err) + } + } +} + +// TestCreateVolumeIsIdempotent pins P2-GC-c at the create verb: re-creating a +// session's volume returns the existing one with its contents intact. Volume +// destruction is Expire's alone, so a re-create must never clear a tree. +func TestCreateVolumeIsIdempotent(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-idem") + marker := filepath.Join(v.HostRoot, "tree-file") + if err := os.WriteFile(marker, []byte("derived state"), 0o600); err != nil { + t.Fatalf("writing tree fixture: %v", err) + } + + again := mustCreate(t, m, "sess-idem") + if again != v { + t.Errorf("re-CreateVolume returned %+v, want the existing %+v", again, v) + } + if !exists(t, marker) { + t.Error("re-CreateVolume cleared existing volume contents; only Expire may destroy a volume (P2-GC-c)") + } +} + +// TestReattachAfterRunnerRestartIsStable simulates a Runner restart with a +// fresh manager over the same base dir. The manager holds no per-session state, +// so the same session must resolve and attach to the same path — the stable-path +// invariant across a process boundary, not just across calls. +func TestReattachAfterRunnerRestartIsStable(t *testing.T) { + base := filepath.Join(t.TempDir(), "volumes") + first, err := NewLocalManager(base) + if err != nil { + t.Fatalf("NewLocalManager: %v", err) + } + v := mustCreate(t, first, "sess-restart") + before := mustAttach(t, first, v) + + restarted, err := NewLocalManager(base) + if err != nil { + t.Fatalf("NewLocalManager after restart: %v", err) + } + resolved, err := restarted.Lookup(t.Context(), "sess-restart") + if err != nil { + t.Fatalf("Lookup after restart: %v", err) + } + after := mustAttach(t, restarted, resolved) + + if after != before { + t.Errorf("path after restart = %q, want the pre-restart %q", after, before) + } +} + +// TestMountedRootIsWritableByCreatingUser asserts the keep-id ownership +// invariant's observable half: the volume root is created by the invoking host +// user and is writable by it. Under the container's keep-id remap that user maps +// to the agent uid in-container, which is what satisfies ensureCheckoutDir's +// "CheckoutDir's parent must be writable by the agent uid" precondition +// (go/internal/runtime/agent.go:354-357). A base dir or subtree created with a +// mode the creating user cannot write would break every launch on the volume +// path. +func TestMountedRootIsWritableByCreatingUser(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-writable") + root := mustAttach(t, m, v) + + // The agent's first act on the volume is an mkdir of its checkout dir + // (ensureCheckoutDir), then writing the tree into it — so both a dir and a + // file under the mounted root must succeed. + checkout := filepath.Join(root, "checkout") + if err := os.Mkdir(checkout, 0o700); err != nil { + t.Fatalf("creating a checkout dir under the mounted root: %v", err) + } + if err := os.WriteFile(filepath.Join(checkout, "file"), []byte("tree bytes"), 0o600); err != nil { + t.Fatalf("writing under the mounted root: %v", err) + } +} + +// TestStampRecordsCallerIntent pins invariant (c) at the write side: the stamp +// carries the intent the CALLER supplied, because D4's suspend uses the same +// stop+remove teardown path a close does and "the container is gone" cannot +// distinguish them. +func TestStampRecordsCallerIntent(t *testing.T) { + m := newManager(t) + for _, intent := range []CloseIntent{IntentClosed, IntentSuspended} { + v := mustCreate(t, m, "sess-intent-"+intent.String()) + + if _, _, ok, err := m.ReadStamp(t.Context(), v); err != nil || ok { + t.Fatalf("fresh volume ReadStamp ok=%v err=%v, want unstamped", ok, err) + } + before := time.Now() + if err := m.Stamp(t.Context(), v, intent); err != nil { + t.Fatalf("Stamp(%s): %v", intent, err) + } + got, stampedAt, ok, err := m.ReadStamp(t.Context(), v) + if err != nil || !ok { + t.Fatalf("ReadStamp after Stamp: ok=%v err=%v", ok, err) + } + if got != intent { + t.Errorf("stamped intent = %s, want %s", got, intent) + } + if stampedAt.Before(before.Add(-time.Second)) || stampedAt.After(time.Now().Add(time.Second)) { + t.Errorf("stampedAt = %s, want a timestamp from this Stamp call", stampedAt) + } + } +} + +// TestAttachClearsAPastDeadlineStamp is invariant (a): a reopened +// closed-but-unexpired session must not carry a past-deadline stamp into its +// new life. The observable contract is that the volume SURVIVES an Expire that +// would have reaped it before the Attach — a backend that returned the path +// without clearing the stamp would lose a live session's tree on the very next +// reaper pass. +func TestAttachClearsAPastDeadlineStamp(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-reopened") + stampAged(t, v, IntentClosed, 30*24*time.Hour) + + // Pre-condition: without the Attach this volume is reap-eligible. + if stamp, err := readStamp(v.HostRoot); err != nil || stamp == nil { + t.Fatalf("fixture stamp not written: stamp=%v err=%v", stamp, err) + } + + mustAttach(t, m, v) + + if _, _, ok, err := m.ReadStamp(t.Context(), v); err != nil || ok { + t.Fatalf("stamp still present after Attach: ok=%v err=%v", ok, err) + } + if err := m.Expire(t.Context(), 14*24*time.Hour); err != nil { + t.Fatalf("Expire: %v", err) + } + if !exists(t, v.HostRoot) { + t.Error("reopened volume was reaped: Attach did not clear the past-deadline stamp (invariant (a))") + } +} + +// TestExpireReapsOnlyClosedPastDeadline is the reaper's whole eligibility +// contract in one table: five volumes covering every state a volume can be in, +// with exactly one eligible. Each surviving row fails on a distinct plausible +// bug — treating unstamped as closed, ignoring the intent bit, ignoring the +// retention window, or reading a stale stamp across an Attach. +func TestExpireReapsOnlyClosedPastDeadline(t *testing.T) { + const retention = 14 * 24 * time.Hour + old := 30 * 24 * time.Hour + fresh := 1 * time.Hour + + setups := []struct { + name string + sessionID string + // setup puts the volume into its state. + setup func(t *testing.T, m *LocalManager, v Volume) + wantReaped bool + // why names the bug this row catches if the verdict flips. + why string + }{ + { + name: "live session, never stamped", + sessionID: "sess-live", + setup: func(*testing.T, *LocalManager, Volume) {}, + why: "an unstamped volume belongs to a LIVE session; reaping it destroys a running session's tree", + }, + { + name: "suspended past the deadline", + sessionID: "sess-suspended", + setup: func(t *testing.T, _ *LocalManager, v Volume) { + t.Helper() + stampAged(t, v, IntentSuspended, old) + }, + why: "a suspended session is never eligible however old (invariant (c)); its resume expects this exact volume at this exact path", + }, + { + name: "closed inside the retention window", + sessionID: "sess-recent", + setup: func(t *testing.T, _ *LocalManager, v Volume) { + t.Helper() + stampAged(t, v, IntentClosed, fresh) + }, + why: "a recently-closed volume is inside its retention window; reaping it breaks the reopen-a-closed-session path", + }, + { + name: "closed past the deadline then reopened", + sessionID: "sess-reopened", + setup: func(t *testing.T, m *LocalManager, v Volume) { + t.Helper() + stampAged(t, v, IntentClosed, old) + mustAttach(t, m, v) + }, + why: "Attach cleared the stamp (invariant (a)); reaping it means the reaper acted on a stale read", + }, + { + name: "closed past the deadline", + sessionID: "sess-expired", + setup: func(t *testing.T, _ *LocalManager, v Volume) { + t.Helper() + stampAged(t, v, IntentClosed, old) + }, + wantReaped: true, + why: "the ONLY eligible state: closed intent, stamp older than the retention window", + }, + } + + m := newManager(t) + roots := make(map[string]string, len(setups)) + for _, s := range setups { + v := mustCreate(t, m, s.sessionID) + s.setup(t, m, v) + roots[s.sessionID] = v.HostRoot + } + + if err := m.Expire(t.Context(), retention); err != nil { + t.Fatalf("Expire: %v", err) + } + + for _, s := range setups { + t.Run(s.name, func(t *testing.T) { + gone := !exists(t, roots[s.sessionID]) + if gone != s.wantReaped { + t.Errorf("reaped = %v, want %v: %s", gone, s.wantReaped, s.why) + } + if !s.wantReaped { + // A survivor must still be resolvable — Expire must not have + // left it half-deleted. + if _, err := m.Lookup(t.Context(), s.sessionID); err != nil { + t.Errorf("surviving volume no longer resolves: %v", err) + } + } else if _, err := m.Lookup(t.Context(), s.sessionID); !errors.Is(err, ErrVolumeNotFound) { + t.Errorf("Lookup of a reaped volume = %v, want ErrVolumeNotFound", err) + } + }) + } +} + +// TestReconcileOrphansStampsUnstampedVolumesAtDiscovery is the crash- +// reconciliation contract. A crash between container-remove and stamp-write +// leaves an unstamped closed volume that Expire — which reads unstamped as +// live — could never reach. The startup pass stamps it closed at DISCOVERY +// time, so it survives one full retention window from discovery (fails safe, +// never open), invariant (a) still undoes the discovery stamp if the session is +// re-provisioned first, and it is reaped once the discovery deadline passes. +func TestReconcileOrphansStampsUnstampedVolumesAtDiscovery(t *testing.T) { + m := newManager(t) + orphan := mustCreate(t, m, "sess-orphan") + // A suspended volume, stamped by its normal teardown, must be untouched by + // the pass: it is already stamped, so reconciliation has no business in it. + suspended := mustCreate(t, m, "sess-suspended") + stampAged(t, suspended, IntentSuspended, 30*24*time.Hour) + suspendedBefore, _, _, err := m.ReadStamp(t.Context(), suspended) + if err != nil { + t.Fatalf("ReadStamp(suspended): %v", err) + } + + before := time.Now() + if err := m.ReconcileOrphans(t.Context()); err != nil { + t.Fatalf("ReconcileOrphans: %v", err) + } + + intent, stampedAt, ok, err := m.ReadStamp(t.Context(), orphan) + if err != nil || !ok { + t.Fatalf("orphan unstamped after reconcile: ok=%v err=%v", ok, err) + } + if intent != IntentClosed { + t.Errorf("orphan intent = %s, want closed", intent) + } + if stampedAt.Before(before.Add(-time.Second)) { + t.Errorf("orphan stampedAt = %s, want a DISCOVERY-time stamp (>= %s), not the lost close time", stampedAt, before) + } + if got, _, _, err := m.ReadStamp(t.Context(), suspended); err != nil || got != suspendedBefore { + t.Errorf("reconcile rewrote an already-stamped volume: intent %s -> %s (err=%v)", suspendedBefore, got, err) + } + + // Fails safe: the discovery deadline has not passed, so a retention-window + // Expire leaves the orphan alone. + if err := m.Expire(t.Context(), 14*24*time.Hour); err != nil { + t.Fatalf("Expire within the discovery window: %v", err) + } + if !exists(t, orphan.HostRoot) { + t.Fatal("orphan reaped inside its discovery-based retention window; the deadline must run from discovery") + } + + // Invariant (a) still applies to a discovery stamp: re-provisioning the + // session before its deadline undoes it for free. + mustAttach(t, m, orphan) + if _, _, ok, err := m.ReadStamp(t.Context(), orphan); err != nil || ok { + t.Fatalf("Attach did not clear the discovery stamp: ok=%v err=%v", ok, err) + } + // And with the stamp cleared the orphan is now indistinguishable from a + // live session: even a zero-window Expire must not touch it. + if err := m.Expire(t.Context(), 0); err != nil { + t.Fatalf("Expire after re-attach: %v", err) + } + if !exists(t, orphan.HostRoot) { + t.Fatal("re-attached orphan reaped; a cleared stamp means live") + } + + // Re-crash: reconcile stamps it again, and once the deadline has passed it + // is reaped. A negative window is the deterministic way to say "the + // discovery deadline has passed" without sleeping — it makes + // now-stampedAt > olderThan true for the just-written discovery stamp. + if err := m.ReconcileOrphans(t.Context()); err != nil { + t.Fatalf("ReconcileOrphans (second pass): %v", err) + } + if err := m.Expire(t.Context(), -time.Second); err != nil { + t.Fatalf("Expire past the discovery deadline: %v", err) + } + if exists(t, orphan.HostRoot) { + t.Error("orphan survived an Expire past its discovery deadline; a crash orphan must always be reachable by the reaper") + } +} + +// TestReconcileOrphansSkipsAVolumeBeingAttached pins the pass's own contention +// rule: a volume whose per-volume lock is held is being attached-live, which is +// the opposite of orphaned, so the pass must leave it unstamped rather than +// stamping a live session closed. +func TestReconcileOrphansSkipsAVolumeBeingAttached(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-attaching") + + lock, err := lockVolume(v.HostRoot, false) + if err != nil { + t.Fatalf("acquiring the stand-in Attach lock: %v", err) + } + if lock == nil { + t.Fatal("stand-in Attach lock reported contention on a fresh volume") + } + + if err := m.ReconcileOrphans(t.Context()); err != nil { + t.Fatalf("ReconcileOrphans with a held lock returned an error, want a silent skip: %v", err) + } + if _, _, ok, err := m.ReadStamp(t.Context(), v); err != nil || ok { + t.Errorf("reconcile stamped a volume held by a live Attach: ok=%v err=%v", ok, err) + } + + if err := lock.release(); err != nil { + t.Fatalf("releasing the stand-in Attach lock: %v", err) + } + // Once the Attach completes, the same pass does stamp it. + if err := m.ReconcileOrphans(t.Context()); err != nil { + t.Fatalf("ReconcileOrphans after release: %v", err) + } + if _, _, ok, err := m.ReadStamp(t.Context(), v); err != nil || !ok { + t.Errorf("reconcile skipped an unlocked orphan: ok=%v err=%v", ok, err) + } +} + +// TestReaperIgnoresNonVolumeDirs pins volume identity as STRUCTURAL: only a +// directory carrying the metaDirName marker CreateVolume writes is one of this +// package's volumes. A directory under the base dir without that marker belongs +// to someone else and must never be stamped or reaped. +// +// This is not hypothetical. The frozen record places W2's snapshot store as a +// sibling subtree under the same base dir keyed by VolumeSnapshotID — a +// non-volume directory in exactly this position. A scan that treated every +// directory entry as a volume root would have ReconcileOrphans stamp that store +// closed at discovery and Expire silently delete it one retention window later: +// a W1-armed landmine that detonates when W2 lands. +func TestReaperIgnoresNonVolumeDirs(t *testing.T) { + m := newManager(t) + + // A foreign directory in the base dir, with no marker dir — stands in for + // W2's snapshot store. + stray := filepath.Join(m.BaseDir(), "snapshots") + if err := os.Mkdir(stray, 0o700); err != nil { + t.Fatalf("creating the non-volume dir: %v", err) + } + sentinel := filepath.Join(stray, "snapshot-payload") + if err := os.WriteFile(sentinel, []byte("another subtree's bytes"), 0o600); err != nil { + t.Fatalf("writing the non-volume sentinel: %v", err) + } + + // A genuine crash orphan beside it, so the pass is proven to still WORK + // rather than passing because it did nothing at all. + orphan := mustCreate(t, m, "sess-orphan") + + if err := m.ReconcileOrphans(t.Context()); err != nil { + t.Fatalf("ReconcileOrphans with a non-volume dir present: %v", err) + } + // Never stamped: the pass must not have created a metadata dir inside it. + if exists(t, filepath.Join(stray, metaDirName)) { + t.Error("ReconcileOrphans stamped a directory with no volume marker; volume identity must be structural") + } + if _, _, ok, err := m.ReadStamp(t.Context(), orphan); err != nil || !ok { + t.Fatalf("the genuine orphan beside it went unstamped, so this test proves nothing: ok=%v err=%v", ok, err) + } + + // And a zero-window Expire — which reaps anything it considers an eligible + // volume — must leave the stray subtree and its contents untouched. + if err := m.Expire(t.Context(), 0); err != nil { + t.Fatalf("Expire with a non-volume dir present: %v", err) + } + if !exists(t, stray) { + t.Fatal("Expire deleted a directory with no volume marker; the base dir is not this package's exclusively (W2's snapshot store is a sibling subtree)") + } + if !exists(t, sentinel) { + t.Error("Expire emptied a non-volume directory; its contents must survive untouched") + } + // The genuine orphan, by contrast, IS reachable by the reaper. + if exists(t, orphan.HostRoot) { + t.Error("the marked crash orphan survived a zero-window Expire; the marker check must not have made genuine volumes invisible") + } +} + +// TestReconcileOrphansDoesNotResurrectAReapedRoot pins onto the third mutator +// the guard its two siblings (Attach, Stamp) already carry: a volume reaped out +// from under a reconcile pass — between eachVolume's marker stat and this pass's +// lock acquisition — must NOT be re-stamped. Without the guard, writeStamp's +// os.MkdirAll resurrects the reaped root's shell, Lookup then SUCCEEDS on a dead +// session, and the provision path warm-Attaches an EMPTY volume instead of +// cold-materializing — silently defeating the not-found-is-an-observable-signal +// contract Lookup documents. +// +// The reap is applied directly and stampOrphanLocked driven on the absent root: +// a fully-removed root is no longer a base-dir entry eachVolume would visit, so +// the window the guard closes is only reachable by running the mutator against a +// root that vanished after discovery — which is exactly the deterministic form +// of that race. +func TestReconcileOrphansDoesNotResurrectAReapedRoot(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-reaped-mid-reconcile") + + // The reap that landed between discovery and this pass's lock acquisition. + if err := os.RemoveAll(v.HostRoot); err != nil { + t.Fatalf("reaping the volume root: %v", err) + } + + // The mutator the pass would run on the discovered-then-reaped root. A + // reaped volume is not an orphan to stamp: the typed not-found is swallowed, + // so the pass reports success without touching the filesystem. + if err := stampOrphanLocked(v.HostRoot, time.Now()); err != nil { + t.Fatalf("stampOrphanLocked on a reaped root = %v, want nil (a reaped volume is not an orphan to stamp)", err) + } + + // The load-bearing consequence: the root stays absent, so Lookup still + // reports the reap and the provision path cold-materializes rather than + // warm-attaching an empty shell. + if exists(t, v.HostRoot) { + t.Fatal("stampOrphanLocked resurrected a reaped volume root; a reconcile pass must never recreate a volume it did not find live") + } + if _, err := m.Lookup(t.Context(), v.SessionID); !errors.Is(err, ErrVolumeNotFound) { + t.Fatalf("Lookup after a reaped-root reconcile = %v, want ErrVolumeNotFound (else provision warm-attaches an empty volume)", err) + } +} + +// TestExpireSkipsALockedVolume is invariant (b)'s observable half: the +// per-volume advisory lock is what closes the window between the reaper reading +// a stamp and a concurrent Attach clearing it. Holding the lock stands in for +// that in-flight Attach — an eligible-looking volume must be SKIPPED, and the +// skip is not an error (contention is exactly the signal not to reap). +func TestExpireSkipsALockedVolume(t *testing.T) { + m := newManager(t) + held := mustCreate(t, m, "sess-held") + free := mustCreate(t, m, "sess-free") + stampAged(t, held, IntentClosed, 30*24*time.Hour) + stampAged(t, free, IntentClosed, 30*24*time.Hour) + + lock, err := lockVolume(held.HostRoot, false) + if err != nil { + t.Fatalf("acquiring the stand-in Attach lock: %v", err) + } + if lock == nil { + t.Fatal("stand-in Attach lock reported contention on a fresh volume") + } + + if err := m.Expire(t.Context(), 14*24*time.Hour); err != nil { + t.Fatalf("Expire with a contended volume returned an error, want a silent skip: %v", err) + } + if !exists(t, held.HostRoot) { + t.Error("Expire reaped a volume whose per-volume lock was held by a live Attach (invariant (b))") + } + if exists(t, free.HostRoot) { + t.Error("Expire skipped the uncontended eligible volume; one contended volume must not pin the pass") + } + + // With the stand-in Attach finished, the next pass reaps it — the skip is a + // deferral, not a permanent pin. + if err := lock.release(); err != nil { + t.Fatalf("releasing the stand-in Attach lock: %v", err) + } + if err := m.Expire(t.Context(), 14*24*time.Hour); err != nil { + t.Fatalf("Expire after release: %v", err) + } + if exists(t, held.HostRoot) { + t.Error("volume survived the pass after its lock was released; the skip must be a deferral") + } +} + +// TestExpireRejectsACorruptStamp pins the fail-safe reading of an +// unintelligible stamp: it is an error, never a silent collapse into +// "unstamped" (which would look live forever) or into a defaulted IntentClosed +// (which would reap on a stamp nobody can read). +func TestExpireRejectsACorruptStamp(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-corrupt") + if err := os.WriteFile(stampPath(v.HostRoot), []byte("{not json"), stampFileMode); err != nil { + t.Fatalf("writing corrupt stamp: %v", err) + } + + if err := m.Expire(t.Context(), 0); err == nil { + t.Error("Expire over a corrupt stamp succeeded, want a surfaced decode error") + } + if !exists(t, v.HostRoot) { + t.Error("Expire reaped a volume whose stamp could not be decoded") + } +} + +// TestReservedVerbsReturnHonestSentinels pins the reserved-not-implemented +// surface: each verb fails with its own errors.Is-detectable sentinel and a +// zero result, so no caller can read a nil error or an empty id as success. +// W2 replaces Snapshot's body; D4 replaces Archive's and Restore's (OQ-2). +func TestReservedVerbsReturnHonestSentinels(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-reserved") + + if id, err := m.Snapshot(t.Context(), v); !errors.Is(err, ErrSnapshotNotImplemented) || id != "" { + t.Errorf("Snapshot = (%q, %v), want (\"\", ErrSnapshotNotImplemented)", id, err) + } + if ref, err := m.Archive(t.Context(), v); !errors.Is(err, ErrArchiveNotImplemented) || ref != "" { + t.Errorf("Archive = (%q, %v), want (\"\", ErrArchiveNotImplemented)", ref, err) + } + if got, err := m.Restore(t.Context(), ArchiveRef("ref-x")); !errors.Is(err, ErrRestoreNotImplemented) || got != (Volume{}) { + t.Errorf("Restore = (%+v, %v), want (Volume{}, ErrRestoreNotImplemented)", got, err) + } +} + +// TestCloseIntentZeroValueIsClosed pins the enum's deliberate zero value: a +// stamp written with a defaulted intent expires (a bounded storage leak) rather +// than pinning the volume forever (an unbounded one), and a discovered orphan — +// which by construction has no caller intent — wants exactly IntentClosed. +func TestCloseIntentZeroValueIsClosed(t *testing.T) { + var zero CloseIntent + if zero != IntentClosed { + t.Errorf("zero CloseIntent = %v, want IntentClosed", zero) + } + if IntentClosed.String() != "closed" || IntentSuspended.String() != "suspended" { + t.Errorf("intent names = %q/%q, want closed/suspended", IntentClosed, IntentSuspended) + } +} + +// heldFlockField is the `MAJ:MIN:INO` field string the kernel prints for the +// held per-volume lock, read out of /proc/locks itself rather than recomputed +// from a path formula or reassembled from device-number math. +// +// Reading the kernel's own rendering is what makes the gate below an exact +// match. The held lock appears in /proc/locks as its own (non-`->`) FLOCK row +// carrying the very field string its blocked waiters' rows carry, so comparing +// that string end to end compares device AND inode without this test ever +// having to know how the kernel formats a dev_t. The row is identified by the +// inode off the lock's own fd — whatever path the implementation chose for +// it — plus this process's pid, which together cannot match another process's +// unrelated lock. +func heldFlockField(t *testing.T, l *volumeLock) string { + t.Helper() + info, err := l.f.Stat() + if err != nil { + t.Fatalf("stat held volume lock: %v", err) + } + st, ok := info.Sys().(*syscall.Stat_t) + if !ok { + t.Skipf("no syscall.Stat_t for the lock file on this platform (%T); the race gate needs the lock's inode", info.Sys()) + } + inoSuffix := ":" + strconv.FormatUint(st.Ino, 10) + pid := strconv.Itoa(os.Getpid()) + + data, err := os.ReadFile("/proc/locks") + if err != nil { + t.Skipf("cannot read /proc/locks (%v); the race gate needs the kernel's blocked-waiter record", err) + } + // A held row: `301: FLOCK ADVISORY WRITE 3173818 00:3c:775 0 EOF` — the + // MAJ:MIN:INO field is index 5, the owning pid index 4. + for line := range strings.SplitSeq(string(data), "\n") { + fields := strings.Fields(line) + if len(fields) < 6 || fields[1] != "FLOCK" || fields[4] != pid { + continue + } + if strings.HasSuffix(fields[5], inoSuffix) { + return fields[5] + } + } + t.Fatalf("no held FLOCK row in /proc/locks for pid %s inode %s; the race gate cannot identify the lock it must wait on", pid, inoSuffix[1:]) + return "" +} + +// waitForBlockedFlock blocks until the kernel reports a process WAITING on the +// flock identified by field (a `MAJ:MIN:INO` string from heldFlockField), and +// fails the test if that never happens. +// +// This is the event gate that makes TestAttachRacingAReapDoesNotReturnAReaped- +// Path deterministic instead of timing-dependent. A blocked flock has no +// user-space completion signal — the waiter is parked inside the syscall, so +// there is no channel to receive and no WaitGroup to wait — but Linux publishes +// the blocked waiter itself in /proc/locks as a `-> FLOCK` continuation row on +// the lock being contended. Polling until that row appears waits on the actual +// event ("the Attach goroutine is now parked on the flock"), so the test drives +// the exact blocked-then-reap interleaving every run rather than hoping a sleep +// was long enough; a genuine failure to reach that state fails loudly here +// instead of silently degrading into the other interleaving. +// +// The tick between polls is the bounded-poll tick, not a timing assumption: +// the loop re-reads the condition every iteration and still fails loudly at the +// deadline, so nothing here depends on a duration being "long enough". It only +// keeps this from being a core-burning spin whose own contention can delay the +// very scheduling it is waiting for under -race. +func waitForBlockedFlock(t *testing.T, field string) { + t.Helper() + const tick = 500 * time.Microsecond + if _, err := os.ReadFile("/proc/locks"); err != nil { + t.Skipf("cannot read /proc/locks (%v); the race gate needs the kernel's blocked-waiter record", err) + } + deadline := time.Now().Add(30 * time.Second) + for { + data, err := os.ReadFile("/proc/locks") + if err != nil { + t.Fatalf("reading /proc/locks: %v", err) + } + if hasBlockedFlockWaiter(string(data), field) { + return + } + if time.Now().After(deadline) { + t.Fatalf("no process ever blocked on an flock over %s; the race gate never engaged", field) + } + time.Sleep(tick) //nolint:forbidigo // irreducible poll-tick: a blocked flock is parked inside the syscall with NO user-space completion signal (no channel, no WaitGroup), so /proc/locks polling is the only observation of the event; the condition is re-read every iteration and the deadline fails loudly, so the tick paces the poll and is never itself the thing waited on + } +} + +// hasBlockedFlockWaiter reports whether /proc/locks content carries a blocked +// flock waiter for the lock whose kernel-printed MAJ:MIN:INO field is field. A +// waiter row is the `-> ` continuation of the lock it is queued behind, e.g. +// +// 292: -> FLOCK ADVISORY WRITE 1759984 00:20:507753071 0 EOF +// +// The comparison is over the WHOLE MAJ:MIN:INO field, never the inode alone: +// dropping MAJ:MIN would match an unrelated flock waiter on another device +// whose inode number happens to be equal, and a spurious early match here +// would release the reap before the Attach goroutine has parked — silently +// degrading this load-bearing gate into the other (still-passing) interleaving, +// a false green exactly as the test it gates promises it will not produce. +func hasBlockedFlockWaiter(locks, field string) bool { + for line := range strings.SplitSeq(locks, "\n") { + fields := strings.Fields(line) + if len(fields) < 7 || fields[1] != "->" || fields[2] != "FLOCK" { + continue + } + if fields[6] == field { + return true + } + } + return false +} + +// TestAttachRacingAReapDoesNotReturnAReapedPath is the regression test for the +// TOCTOU race between a relaunch and the reaper tick. A session that relaunches +// exactly at its expiry deadline runs Attach's pre-check (root present), then +// blocks on the per-volume lock the reaper holds; the reaper re-verifies +// eligibility under that lock, reaps the tree, and releases. The Attach that +// then wins the lock MUST NOT report success with the path of a directory that +// no longer exists — the provision path would mount a dead path and silently +// lose the warm `target/`/sccache tree with nothing signalling the loss. It +// must return ErrVolumeNotFound so the provision path cold-materializes. +// +// The interleaving is driven deterministically: the stand-in reaper takes the +// lock first, and the reap happens only once the kernel reports the Attach +// goroutine actually parked on that lock (see waitForBlockedFlock). Both +// interleavings are asserted correct, so the gate decides which bug this +// exercises, never whether correct code passes. +func TestAttachRacingAReapDoesNotReturnAReapedPath(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-relaunch-at-deadline") + + // The sentinel stands in for the warm derived state the whole volume + // mechanism exists to preserve: if Attach hands back a path without this + // file, the caller mounted a reaped tree. + sentinel := filepath.Join(v.HostRoot, "target-artifact") + if err := os.WriteFile(sentinel, []byte("warm build cache"), 0o600); err != nil { + t.Fatalf("writing the warm-tree sentinel: %v", err) + } + stampAged(t, v, IntentClosed, 30*24*time.Hour) + + // Stand in for the reaper mid-pass: Expire holds exactly this lock across + // its under-lock re-verify and its os.RemoveAll. + reaper, err := lockVolume(v.HostRoot, false) + if err != nil { + t.Fatalf("acquiring the stand-in reaper lock: %v", err) + } + if reaper == nil { + t.Fatal("stand-in reaper lock reported contention on a fresh volume") + } + lockField := heldFlockField(t, reaper) + + type attachResult struct { + path string + err error + } + done := make(chan attachResult, 1) + go func() { + path, err := m.Attach(t.Context(), v) + done <- attachResult{path: path, err: err} + }() + + // Gate on the kernel's own record that the Attach goroutine is parked on + // the lock, so the reap below lands in the window the bug lives in. + waitForBlockedFlock(t, lockField) + + if err := os.RemoveAll(v.HostRoot); err != nil { + t.Fatalf("reaping the volume root: %v", err) + } + if err := reaper.release(); err != nil { + t.Fatalf("releasing the stand-in reaper lock: %v", err) + } + + got := <-done + switch { + case errors.Is(got.err, ErrVolumeNotFound): + // Correct: the reap is reported as an error-shaped signal the provision + // path turns into a cold CreateVolume plus materialize. + case got.err != nil: + t.Fatalf("Attach racing a reap = %v, want ErrVolumeNotFound", got.err) + case !exists(t, filepath.Join(got.path, "target-artifact")): + t.Fatalf("Attach returned %q with a nil error, but the volume was reaped: the caller would mount a dead path and lose the warm tree with nothing signalling it", got.path) + } +} + +// TestVolumeLockSurvivesAReap pins the placement the fix rests on: the +// per-volume lock file lives OUTSIDE the volume root, so reaping the root +// cannot unlink the inode the lock is held on. +// +// That is what makes mutual exclusion hold across a reap+recreate. flock locks +// an inode; a lock file inside the reaped subtree would be unlinked by the +// reap, so a later lockVolume — after a cold CreateVolume recreated the root — +// would open a NEW inode and lock that, letting two actors hold "the" volume +// lock at once and making any under-lock existence check worthless. Holding the +// lock across a reap must therefore still exclude a second acquisition. +func TestVolumeLockSurvivesAReap(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-reaped-under-lock") + + held, err := lockVolume(v.HostRoot, false) + if err != nil { + t.Fatalf("acquiring the volume lock: %v", err) + } + if held == nil { + t.Fatal("volume lock reported contention on a fresh volume") + } + + if err := os.RemoveAll(v.HostRoot); err != nil { + t.Fatalf("reaping the volume root: %v", err) + } + + // Still held: a second non-blocking acquisition must report contention + // ((nil, nil)), proving it reached the same surviving inode. + second, err := lockVolume(v.HostRoot, false) + if err != nil { + t.Fatalf("second lockVolume after the reap: %v", err) + } + if second != nil { + if releaseErr := second.release(); releaseErr != nil { + t.Errorf("releasing the unexpectedly-acquired second lock: %v", releaseErr) + } + t.Error("a second lockVolume acquired the lock while it was still held across a reap: the lock file was destroyed with the volume root, so mutual exclusion is broken across reap+recreate") + } + + // And a recreate does not split the lock either: the recreated volume's + // lock is the same inode, so it is still contended. + if _, err := m.CreateVolume(t.Context(), v.SessionID); err != nil { + t.Fatalf("recreating the volume: %v", err) + } + afterRecreate, err := lockVolume(v.HostRoot, false) + if err != nil { + t.Fatalf("lockVolume after the recreate: %v", err) + } + if afterRecreate != nil { + if releaseErr := afterRecreate.release(); releaseErr != nil { + t.Errorf("releasing the unexpectedly-acquired post-recreate lock: %v", releaseErr) + } + t.Error("lockVolume acquired the lock after a reap+recreate while it was still held: the recreate produced a second lock inode") + } + + if err := held.release(); err != nil { + t.Fatalf("releasing the held volume lock: %v", err) + } +} + +// TestVolumeLockFileIsOutsideTheVolumeRoot pins the placement structurally, and +// pins that acquiring a lock creates NOTHING inside the volume root. A +// lockVolume that resurrected the metadata dir inside a reaped root would make +// Attach's under-lock existence check see a live volume with no contents. +func TestVolumeLockFileIsOutsideTheVolumeRoot(t *testing.T) { + m := newManager(t) + v := mustCreate(t, m, "sess-lock-placement") + if err := os.RemoveAll(v.HostRoot); err != nil { + t.Fatalf("reaping the volume root: %v", err) + } + + lock, err := lockVolume(v.HostRoot, false) + if err != nil { + t.Fatalf("acquiring the volume lock on a reaped volume: %v", err) + } + if lock == nil { + t.Fatal("volume lock reported contention on an uncontended volume") + } + if exists(t, v.HostRoot) { + t.Error("lockVolume created something inside the volume root; acquiring a lock must not resurrect a reaped root") + } + if !strings.HasPrefix(lock.f.Name(), v.HostRoot+".") { + t.Errorf("lock file %q is not a sibling of the volume root %q", lock.f.Name(), v.HostRoot) + } + if err := lock.release(); err != nil { + t.Fatalf("releasing the volume lock: %v", err) + } + + // The stamp, by contrast, stays INSIDE the volume root (the frozen record + // places it there), and eachVolume iterates directories only, so the + // sibling lock file is never mistaken for a volume. + live := mustCreate(t, m, "sess-stamped") + stampAged(t, live, IntentClosed, 30*24*time.Hour) + if !strings.HasPrefix(stampPath(live.HostRoot), live.HostRoot+string(os.PathSeparator)) { + t.Errorf("stamp path %q is not inside the volume root %q", stampPath(live.HostRoot), live.HostRoot) + } + // Materialize the sibling lock file, then release it: a still-held lock + // would make Expire SKIP the volume, which would pass this check for the + // wrong reason. + siblingLock, err := lockVolume(live.HostRoot, false) + if err != nil { + t.Fatalf("creating the sibling lock file for the iteration check: %v", err) + } + if siblingLock == nil { + t.Fatal("sibling lock reported contention on a fresh volume") + } + if err := siblingLock.release(); err != nil { + t.Fatalf("releasing the sibling lock: %v", err) + } + if !exists(t, live.HostRoot+lockFileSuffix) { + t.Fatalf("no sibling lock file at %q; the iteration check would prove nothing", live.HostRoot+lockFileSuffix) + } + // The sibling .lock FILES must be skipped by volume iteration (eachVolume + // takes directories only) while the eligible volume is still reaped. + if err := m.Expire(t.Context(), 14*24*time.Hour); err != nil { + t.Fatalf("Expire with sibling lock files present: %v", err) + } + if exists(t, live.HostRoot) { + t.Error("Expire did not reap the eligible volume") + } +} diff --git a/go/internal/vfs/vfs.go b/go/internal/vfs/vfs.go new file mode 100644 index 00000000..f40f7f4c --- /dev/null +++ b/go/internal/vfs/vfs.go @@ -0,0 +1,183 @@ +// Package vfs is the per-session persistent-volume seam: where a session's +// working tree and derived state (`target/`, `node_modules`, build caches) live +// so they survive suspend / resume / eviction and mount at a stable absolute +// path on every launch. Today the tree dies with the container because it +// exists only inside it (the agent self-clones post-launch into a +// container-local dir, go/internal/runtime/agent.go:354-358); this package owns +// the volume lifecycle Runner-side, beside the container lifecycle +// (docs/designs/infra/runtime/compass-elastic-session-runtime/p2-persistent-session-volume.md, +// the volume under §Approach and the lifecycle API under W1). +// +// The layering mirrors the elastic-compute seam in internal/compute: +// - vfs.go — the VolumeManager interface plus the value types that cross it +// (Volume, VolumeSnapshotID, ArchiveRef, CloseIntent) and the package's +// typed errors. Every consumer depends on the interface, so a +// network-volume backend can replace the local-directory one without +// touching a caller. +// - localvolume.go — the P2 backend: a directory subtree on the box's fast +// local storage, one per session under an operator-configured base dir. +// Vendor-neutral (Global Constraint 1), no storage fabric; the accepted +// tradeoff — a burst cannot land on a different box until a network-volume +// backend exists — is the parent record's OQ 2 and is not re-opened here. +// +// Two load-bearing constraints shape the surface. Volume destruction is +// **only** via Expire (P2-GC-c): Release, Teardown, eviction, crash, and failed +// launches never delete volume contents, so the sole reclaim path is the policy +// reaper. And the in-container mount path of a session's volume is identical +// across every launch, resume, and burst environment of that session +// (P2-GC-d) — a path change invalidates `target/` and sccache and is a breaking +// bug, which is why Attach returns a path derived solely from the session id +// and the base dir, never from anything per-launch. +// +// Reserved-not-implemented surface: Snapshot, Archive, and Restore are declared +// in the interface now so their consumers land no interface change, but the P2 +// backend returns honest not-implemented sentinels rather than silent no-ops +// (the ErrExecStreamingNotImplemented discipline, +// go/internal/compute/compute.go:26-29; the Resize precedent, +// go/internal/runtime/podman.go:387-396). Snapshot's real body — the snapshot +// store, the reflink/rsync copy primitive, and the (account, repo) index — is +// W2's; Archive/Restore's consumer is D4's cold-idle (OQ-2). +// +// This package imports nothing outside the standard library: the volume +// lifecycle is deliberately independent of the container runtime and of the +// Server. In particular the crash-reconciliation pass needs no session query +// and by design cannot want one — the Runner, not the Server, is authoritative +// for live-session truth, and RunnerService exposes no session-query verb (see +// LocalManager.ReconcileOrphans). +package vfs + +import ( + "context" + "errors" + "time" +) + +// ErrVolumeNotFound is returned by Lookup for a session with no volume on this +// box, and by Attach for a volume that no longer exists. It is an +// error-shaped signal, never a silent recreate: the provision path converts it +// into a fresh CreateVolume plus a cold materialize, so the cold path is +// observable. Callers detect it with errors.Is. +var ErrVolumeNotFound = errors.New("vfs: volume not found") + +// ErrInvalidSessionID rejects a session id that cannot key a volume subtree. +// Session ids reaching this package are already-sanitized internal ids, so this +// is a defense-in-depth guard: a path separator or a `..` element in a session +// id would escape the base dir, and the base dir is the only subtree this +// package is ever allowed to create or reap. +var ErrInvalidSessionID = errors.New("vfs: invalid session id") + +// ErrSnapshotNotImplemented is the honest sentinel the P2 backend returns from +// the reserved Snapshot method: the verb is declared in VolumeManager now, but +// the snapshot store, the reflink/rsync copy primitive, and the +// (AgentAccountID, repo) index the provision path reads are W2's to fill in +// behind this signature. W2 replaces the sentinel body; no interface change +// lands with it. +var ErrSnapshotNotImplemented = errors.New("vfs: Snapshot store is implemented in W2") + +// ErrArchiveNotImplemented is the honest sentinel the P2 backend returns from +// the reserved Archive method: the verb's consumer is D4's cold-idle (OQ-2), so +// the signature is frozen here and the object-store implementation deferred. +var ErrArchiveNotImplemented = errors.New("vfs: Archive is reserved at P2 and implemented in D4") + +// ErrRestoreNotImplemented is the honest sentinel the P2 backend returns from +// the reserved Restore method: like Archive, its consumer is D4's cold-idle +// (OQ-2). +var ErrRestoreNotImplemented = errors.New("vfs: Restore is reserved at P2 and implemented in D4") + +// Volume is a live per-session persistent volume: the session it belongs to and +// its host-side root. Opaque to callers beyond these fields. +type Volume struct { + SessionID string + HostRoot string +} + +// VolumeSnapshotID is the opaque key of a stored volume snapshot (frozen opaque +// by the parent record; never parsed by callers). +type VolumeSnapshotID string + +// ArchiveRef is the opaque reference to an archived volume in the object store +// (consumed by D4's cold-idle; signature frozen here, implementation deferred — +// see OQ-2). +type ArchiveRef string + +// CloseIntent is why a session's volume was last stamped: the intent bit that +// decides whether the expiry reaper may ever touch it. It comes from the +// caller — the teardown path knows whether it is closing or suspending a +// session — and is never inferred from "the container is gone", because D4's +// suspend uses the same stop+remove teardown path a close does. Without the +// caller-supplied bit, every suspended session's volume would look closed and +// be reaped one expiry window into a suspend. +// +// The zero value is IntentClosed, the reap-eligible intent. That direction is +// deliberate: a stamp written with a defaulted intent expires (a bounded +// storage leak) rather than pinning the volume forever (an unbounded one), and +// a discovered orphan — which by construction has no caller intent — wants +// exactly IntentClosed. +type CloseIntent int + +const ( + // IntentClosed marks a session closed for good. Its volume becomes eligible + // for Expire once the stamp is older than the configured retention. + IntentClosed CloseIntent = iota + // IntentSuspended marks a session suspended, not closed. Its volume is + // NEVER eligible for Expire however old the stamp is — the session is + // expected to resume onto exactly this volume, at exactly this path. + IntentSuspended +) + +// String names the intent for diagnostics. An unrecognized value renders +// visibly rather than as a bare integer. +func (i CloseIntent) String() string { + switch i { + case IntentClosed: + return "closed" + case IntentSuspended: + return "suspended" + default: + return "unknown" + } +} + +// VolumeManager owns the session-volume lifecycle Runner-side, beside the +// container lifecycle. An interface so the Runner can hold a VolumeManager and +// tests can substitute a fake, and so a later network-volume backend slots in +// behind it. A backend is constructed with the operator-configured base dir +// (see NewLocalManager), so it has the placement context every verb needs +// without threading it through each call. +type VolumeManager interface { + // CreateVolume creates the session's volume subtree and returns the + // resolved Volume. It is idempotent: creating a volume for a session that + // already has one returns the existing volume rather than clearing it — + // volume destruction is Expire's alone (P2-GC-c). + CreateVolume(ctx context.Context, sessionID string) (Volume, error) + // Lookup resolves a session's existing volume (with its HostRoot) or + // returns ErrVolumeNotFound. It is the "resolve" half of the provision + // path's resolve-or-create: Attach needs a resolved Volume, so a caller + // cannot produce one from a bare session id without this verb. A Runner + // never resolves a volume it does not host — the box-local invariant. + Lookup(ctx context.Context, sessionID string) (Volume, error) + // Attach makes the resolved volume available for mounting and returns its + // host path; it also atomically clears any close-stamp, so a reopened + // closed-but-unexpired session never carries a past-deadline stamp into its + // new life. The returned path depends only on the session id and the base + // dir, which is what makes it stable across every launch (P2-GC-d). + Attach(ctx context.Context, v Volume) (path string, err error) + // Snapshot captures the volume's tree into the snapshot store and returns + // its opaque key. Reserved at P2: the backend returns + // ErrSnapshotNotImplemented until W2 lands the store, the copy primitive, + // and the (account, repo) index. + Snapshot(ctx context.Context, v Volume) (VolumeSnapshotID, error) + // Archive moves the volume to cold object storage and returns its opaque + // reference. Reserved at P2 (OQ-2): the backend returns + // ErrArchiveNotImplemented until D4's cold-idle consumes it. + Archive(ctx context.Context, v Volume) (ArchiveRef, error) + // Restore rehydrates an archived volume and returns the live Volume. + // Reserved at P2 (OQ-2): the backend returns ErrRestoreNotImplemented. + Restore(ctx context.Context, ref ArchiveRef) (Volume, error) + // Expire reaps volumes whose session is closed and whose close-stamp is + // older than olderThan. Never touches live or suspended sessions: an + // unstamped volume belongs to a live session and a stamp carrying + // IntentSuspended is ineligible however old. This is the ONLY path that + // destroys volume contents (P2-GC-c). + Expire(ctx context.Context, olderThan time.Duration) error +}