Skip to content

Lane TTL cleanup can recursively delete an unverified path #5824

Description

@wuisabel-gif

Problem

Lane TTL cleanup trusts the persisted worktree_path and can recursively delete that path even when it cannot verify that the path is a managed CodeWhale Git worktree.

This creates a destructive failure mode for corrupted, stale, externally modified, or otherwise invalid local lane state.

Evidence

crates/lane/src/runtime.rs:130-138 passes the persisted path directly into cleanup:

if let Some(path) = record.worktree_path.as_ref() {
    remove_worktree_if_expired(
        path,
        record.worktree_ttl_secs,
        record.stopped_at.as_deref(),
    )?;
}

crates/lane/src/worktree.rs:104-123 attempts to identify the worktree, but then removes the path regardless of whether identification succeeded:

let details = worktree_details(worktree_path);

// Best-effort: git worktree remove --force, then rm -rf.
let removed = details.as_ref().is_some_and(|details| { /* ... */ });
if worktree_path.exists() {
    fs::remove_dir_all(worktree_path)?;
}

The details == None case still falls through to remove_dir_all.

Failure mode

If a persisted LaneRecord contains an existing unrelated directory as worktree_path and the TTL is zero, stopping the lane can recursively delete that directory when worktree_details cannot establish managed-worktree identity.

A similar risk exists if the path is changed between validation and removal, or if Git metadata becomes unavailable while cleanup is running.

This is primarily a defense against corrupted or externally modified local state. It should not require an attacker with arbitrary process execution to be considered a bug: a stale or malformed persisted record must not turn cleanup into an unbounded recursive delete.

Proposed bounded fix

  • Require successful managed-worktree identity before any recursive removal.
  • Verify the candidate path is beneath the owning/expected worktree root and still resolves to the identified worktree.
  • Do not fall back to fs::remove_dir_all for an unverified path.
  • Preserve the existing safe behavior for missing paths, TTL checks, Git cleanup, and unmerged lane branches.
  • Add focused tests proving malformed records, unrelated directories, non-worktree paths, and path changes between validation and deletion remain intact.

Do not change lane retention policy or broaden this into a general filesystem sandbox rewrite.

Acceptance criteria

  • A path that Git cannot identify as a managed worktree is never recursively deleted by TTL cleanup.
  • A verified managed worktree can still be removed after its TTL expires.
  • The existing branch-retention rule remains intact: unmerged lane branches are kept.
  • Tests cover TTL zero, expired timestamps, unrelated directories, missing Git metadata, and validation/removal identity checks.
  • Errors and diagnostics do not echo secret-shaped path data beyond the existing sanitized path conventions.

Related work

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions