fix(cleanup): move scratch to the macOS Trash without opening it

internal/cleanup/cleanup_test.go

@@ -780,3 +780,29 @@ func TestWhyOmitsMainWorktree(t *testing.T) {
 		}
 	}
 }
+
+// A trash name taken between the free-name check and the rename is retried
+// with the next name instead of failing the removal.
+func TestTrashNameTakenRetries(t *testing.T) {
+	e, _, _, _ := fixture(t)
+	path := scratch(t, e, "taken")
+	c := listed(t, e, path)
+	var tried []string
+	e.Rename = func(from, to string) error {
+		tried = append(tried, to)
+		if len(tried) == 1 {
+			return syscall.EEXIST
+		}
+		return os.Rename(from, to)
+	}
+	r, err := Apply(context.Background(), e, c, ApplyOpts{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(tried) != 2 || tried[0] == tried[1] || r.TrashPath != tried[1] {
+		t.Fatal(tried, r.TrashPath)
+	}
+	if _, err := os.Stat(path); !os.IsNotExist(err) {
+		t.Fatal("scratch still in place", err)
+	}
+}

internal/cleanup/trash.go

@@ -4,10 +4,12 @@ package cleanup
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"github.com/labstack/driga/internal/secfile"
 	"os"
 	"path/filepath"
+	"syscall"
 )
 
 func trash(ctx context.Context, e Env, c Candidate, opts ApplyOpts, r Result) (Result, error) {
@@ -35,27 +37,35 @@ func trash(ctx context.Context, e Env, c Candidate, opts ApplyOpts, r Result) (R
 		return r, err
 	}
 	name := filepath.Base(c.Path)
-	for i := 0; ; i++ {
-		candidate := name
-		if i > 0 {
-			candidate = fmt.Sprintf("%s.%d.%d", name, e.now().UnixNano(), i)
-		}
-		r.TrashPath = filepath.Join(files, candidate)
-		_, err := os.Lstat(r.TrashPath)
-		if os.IsNotExist(err) {
-			if infoDir != "" {
-				if _, err := os.Lstat(filepath.Join(infoDir, candidate+".trashinfo")); err == nil {
-					continue
-				} else if !os.IsNotExist(err) {
-					return r, err
+	// pick sets r.TrashPath to the first free trash name at index i or later
+	// and returns its index.
+	pick := func(i int) (int, error) {
+		for ; ; i++ {
+			candidate := name
+			if i > 0 {
+				candidate = fmt.Sprintf("%s.%d.%d", name, e.now().UnixNano(), i)
+			}
+			r.TrashPath = filepath.Join(files, candidate)
+			_, err := os.Lstat(r.TrashPath)
+			if os.IsNotExist(err) {
+				if infoDir != "" {
+					if _, err := os.Lstat(filepath.Join(infoDir, candidate+".trashinfo")); err == nil {
+						continue
+					} else if !os.IsNotExist(err) {
+						return i, err
+					}
 				}
+				return i, nil
+			}
+			if err != nil {
+				return i, err
 			}
-			break
-		}
-		if err != nil {
-			return r, err
 		}
 	}
+	index, err := pick(0)
+	if err != nil {
+		return r, err
+	}
 	r.Commands = [][]string{{"rename", c.Path, r.TrashPath}}
 	if opts.DryRun {
 		return r, nil
@@ -81,22 +91,35 @@ func trash(ctx context.Context, e Env, c Candidate, opts ApplyOpts, r Result) (R
 	if infoDir != "" {
 		canonicalInfo = filepath.Join(canonicalHome, mustRelative(e.Home, infoDir))
 	}
-	finish, err := trashInfo(canonicalInfo, filepath.Base(r.TrashPath), c.Path, e.now())
-	if err != nil {
-		return r, err
-	}
-	var moveErr error
-	if e.Rename != nil {
-		moveErr = e.Rename(c.Path, r.TrashPath)
-	} else {
-		moveErr = renameScratch(c, filepath.Join(canonicalFiles, filepath.Base(r.TrashPath)))
-	}
-	if err := moveErr; err != nil {
+	// The rename never replaces an item; a name taken since pick is retried
+	// with the next one.
+	for attempt := 0; ; attempt++ {
+		finish, err := trashInfo(canonicalInfo, filepath.Base(r.TrashPath), c.Path, e.now())
+		if err != nil {
+			return r, err
+		}
+		var moveErr error
+		if e.Rename != nil {
+			moveErr = e.Rename(c.Path, r.TrashPath)
+		} else {
+			moveErr = renameScratch(c, filepath.Join(canonicalFiles, filepath.Base(r.TrashPath)))
+		}
+		if moveErr == nil {
+			finish(true)
+			return r, nil
+		}
 		finish(false)
-		return r, fmt.Errorf("trash rename (same filesystem required): %w", err)
+		if errors.Is(moveErr, syscall.EEXIST) && attempt < 8 {
+			if index, err = pick(index + 1); err != nil {
+				return r, err
+			}
+			continue
+		}
+		if errors.Is(moveErr, syscall.EXDEV) {
+			return r, fmt.Errorf("trash rename (same filesystem required): %w", moveErr)
+		}
+		return r, fmt.Errorf("trash rename: %w", moveErr)
 	}
-	finish(true)
-	return r, nil
 }
 
 // Both arguments were already validated as known HOME-relative OS paths.

internal/cleanup/trash_darwin_test.go

@@ -0,0 +1,67 @@
+package cleanup
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// macOS refuses to open ~/.Trash to a process without Full Disk Access, as
+// the Driga service is, but lets it rename into it. A trash directory that
+// cannot be read stands in for that here: scratch still moves into it, and an
+// item already there keeps its name.
+func TestScratchTrashUnreadable(t *testing.T) {
+	e, _, _, _ := fixture(t)
+	trash := filepath.Join(e.Home, ".Trash")
+	put(t, filepath.Join(trash, "unreadable", "kept"), "keep")
+	if err := os.Chmod(trash, 0o300); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.Chmod(trash, 0o700) })
+	if _, err := os.ReadDir(trash); err == nil {
+		t.Skip("trash still readable; running as root?")
+	}
+	path := scratch(t, e, "unreadable")
+	r, err := Apply(context.Background(), e, listed(t, e, path), ApplyOpts{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := os.Stat(path); !os.IsNotExist(err) {
+		t.Fatal("scratch still in place", err)
+	}
+	if filepath.Dir(r.TrashPath) != trash || filepath.Base(r.TrashPath) == "unreadable" {
+		t.Fatal(r.TrashPath)
+	}
+	if _, err := os.Stat(filepath.Join(r.TrashPath, "file")); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := os.Stat(filepath.Join(trash, "unreadable", "kept")); err != nil {
+		t.Fatal("existing trash item replaced", err)
+	}
+}
+
+// A .Trash swapped for a symlink after the checks is refused by the rename
+// itself, so scratch never lands in the symlink's target.
+func TestRenameIntoTrashRefusesSymlink(t *testing.T) {
+	home := t.TempDir()
+	elsewhere := t.TempDir()
+	if err := os.Symlink(elsewhere, filepath.Join(home, ".Trash")); err != nil {
+		t.Fatal(err)
+	}
+	src := t.TempDir()
+	if err := os.Mkdir(filepath.Join(src, "item"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	source, err := openDir(resolved(src))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer source.Close()
+	if err := renameIntoTrash(source, "item", nil, filepath.Join(resolved(home), ".Trash", "item")); err == nil {
+		t.Fatal("rename followed a .Trash symlink")
+	}
+	if _, err := os.Stat(filepath.Join(elsewhere, "item")); !os.IsNotExist(err) {
+		t.Fatal("scratch moved through the symlink", err)
+	}
+}

internal/cleanup/trash_move_unix.go

@@ -7,7 +7,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"syscall"
 
 	"golang.org/x/sys/unix"
 )
@@ -62,17 +61,5 @@ func renameScratch(c Candidate, destination string) error {
 	if !os.SameFile(info, c.identity.info) {
 		return fmt.Errorf("scratch identity changed")
 	}
-	target, err := openDir(filepath.Dir(destination))
-	if err != nil {
-		return err
-	}
-	defer target.Close()
-	targetInfo, err := target.Stat()
-	if err != nil {
-		return err
-	}
-	if !sameDevice(rootInfo.Sys().(*syscall.Stat_t), targetInfo.Sys().(*syscall.Stat_t)) {
-		return syscall.EXDEV
-	}
-	return unix.Renameat(int(source.Fd()), name, int(target.Fd()), filepath.Base(destination))
+	return renameIntoTrash(source, name, rootInfo, destination)
 }

internal/cleanup/trash_rename_darwin.go

@@ -0,0 +1,24 @@
+package cleanup
+
+import (
+	"os"
+	"path/filepath"
+
+	"golang.org/x/sys/unix"
+)
+
+// renameIntoTrash moves name from source into ~/.Trash. macOS privacy
+// protection refuses to open ~/.Trash to a process without Full Disk Access,
+// as the Driga service is, but lets it rename an item into it. So the move
+// goes through a handle on the trash's parent, refuses a symlink anywhere in
+// the destination path, and never replaces an existing item. The kernel
+// refuses a cross-device move with EXDEV.
+func renameIntoTrash(source *os.File, name string, _ os.FileInfo, destination string) error {
+	trashDir := filepath.Dir(destination)
+	parent, err := openDir(filepath.Dir(trashDir))
+	if err != nil {
+		return err
+	}
+	defer parent.Close()
+	return unix.RenameatxNp(int(source.Fd()), name, int(parent.Fd()), filepath.Join(filepath.Base(trashDir), filepath.Base(destination)), unix.RENAME_EXCL|unix.RENAME_NOFOLLOW_ANY)
+}

internal/cleanup/trash_rename_linux.go

@@ -0,0 +1,35 @@
+package cleanup
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"syscall"
+
+	"golang.org/x/sys/unix"
+)
+
+// renameIntoTrash moves name from source into the trash directory through a
+// handle on that directory, so no component of it can be swapped for a
+// symlink in between, and never replaces an existing item.
+func renameIntoTrash(source *os.File, name string, rootInfo os.FileInfo, destination string) error {
+	target, err := openDir(filepath.Dir(destination))
+	if err != nil {
+		return err
+	}
+	defer target.Close()
+	targetInfo, err := target.Stat()
+	if err != nil {
+		return err
+	}
+	if !sameDevice(rootInfo.Sys().(*syscall.Stat_t), targetInfo.Sys().(*syscall.Stat_t)) {
+		return syscall.EXDEV
+	}
+	// Never replace an existing item. A filesystem without RENAME_NOREPLACE
+	// falls back to the plain rename the free-name check guards.
+	err = unix.Renameat2(int(source.Fd()), name, int(target.Fd()), filepath.Base(destination), unix.RENAME_NOREPLACE)
+	if errors.Is(err, unix.EINVAL) || errors.Is(err, unix.ENOSYS) {
+		err = unix.Renameat(int(source.Fd()), name, int(target.Fd()), filepath.Base(destination))
+	}
+	return err
+}