feat(analyze): safer deletion with Trash and two-key confirm

- Change delete confirmation from double-delete to Delete→Enter
- Move files to macOS Trash instead of permanent deletion
- Allow file recovery from Trash if accidentally deleted
- Update UI prompts to show 'Press Enter to confirm'
- Skip Finder-dependent tests in CI environments
- Update SECURITY_AUDIT.md with new safety mechanisms

Closes #288

SECURITY_AUDIT.md

@@ -123,7 +123,8 @@ The analyzer (`mo analyze`) uses a distinct security model:
 
 - Runs with standard user permissions only.
 - Respects macOS System Integrity Protection (SIP).
-- Requires explicit user confirmation for all deletions.
+- **Two-Key Confirmation:** Deletion requires ⌫ (Delete) to enter confirmation mode, then Enter to confirm. Prevents accidental double-press of the same key.
+- **Trash Instead of Delete:** Files are moved to macOS Trash using Finder's native API, allowing easy recovery if needed.
 - OS-level enforcement (cannot delete `/System` due to Read-Only Volume).
 
 **Code:** `cmd/analyze/*.go`

cmd/analyze/analyze_test.go

@@ -90,6 +90,11 @@ func TestScanPathConcurrentBasic(t *testing.T) {
 }
 
 func TestDeletePathWithProgress(t *testing.T) {
+	// Skip in CI environments where Finder may not be available.
+	if os.Getenv("CI") != "" {
+		t.Skip("Skipping Finder-dependent test in CI")
+	}
+
 	parent := t.TempDir()
 	target := filepath.Join(parent, "target")
 	if err := os.MkdirAll(target, 0o755); err != nil {
@@ -107,18 +112,15 @@ func TestDeletePathWithProgress(t *testing.T) {
 	}
 
 	var counter int64
-	count, err := deletePathWithProgress(target, &counter)
+	count, err := trashPathWithProgress(target, &counter)
 	if err != nil {
-		t.Fatalf("deletePathWithProgress returned error: %v", err)
+		t.Fatalf("trashPathWithProgress returned error: %v", err)
 	}
 	if count != int64(len(files)) {
-		t.Fatalf("expected %d files removed, got %d", len(files), count)
+		t.Fatalf("expected %d files trashed, got %d", len(files), count)
-	}
-	if got := atomic.LoadInt64(&counter); got != count {
-		t.Fatalf("counter mismatch: want %d, got %d", count, got)
 	}
 	if _, err := os.Stat(target); !os.IsNotExist(err) {
-		t.Fatalf("expected target to be removed, stat err=%v", err)
+		t.Fatalf("expected target to be moved to Trash, stat err=%v", err)
 	}
 }
 

cmd/analyze/delete.go

@@ -1,19 +1,24 @@
 package main
 
 import (
-	"io/fs"
+	"context"
+	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"sort"
 	"strings"
 	"sync/atomic"
+	"time"
 
 	tea "github.com/charmbracelet/bubbletea"
 )
 
+const trashTimeout = 30 * time.Second
+
 func deletePathCmd(path string, counter *int64) tea.Cmd {
 	return func() tea.Msg {
-		count, err := deletePathWithProgress(path, counter)
+		count, err := trashPathWithProgress(path, counter)
 		return deleteProgressMsg{
 			done:  true,
 			err:   err,
@@ -23,20 +28,20 @@ func deletePathCmd(path string, counter *int64) tea.Cmd {
 	}
 }
 
-// deleteMultiplePathsCmd deletes paths and aggregates results.
+// deleteMultiplePathsCmd moves paths to Trash and aggregates results.
 func deleteMultiplePathsCmd(paths []string, counter *int64) tea.Cmd {
 	return func() tea.Msg {
 		var totalCount int64
 		var errors []string
 
-		// Delete deeper paths first to avoid parent/child conflicts.
+		// Process deeper paths first to avoid parent/child conflicts.
 		pathsToDelete := append([]string(nil), paths...)
 		sort.Slice(pathsToDelete, func(i, j int) bool {
 			return strings.Count(pathsToDelete[i], string(filepath.Separator)) > strings.Count(pathsToDelete[j], string(filepath.Separator))
 		})
 
 		for _, path := range pathsToDelete {
-			count, err := deletePathWithProgress(path, counter)
+			count, err := trashPathWithProgress(path, counter)
 			totalCount += count
 			if err != nil {
 				if os.IsNotExist(err) {
@@ -72,48 +77,70 @@ func (e *multiDeleteError) Error() string {
 	return strings.Join(e.errors[:min(3, len(e.errors))], "; ")
 }
 
-func deletePathWithProgress(root string, counter *int64) (int64, error) {
+// trashPathWithProgress moves a path to Trash using Finder.
-	var count int64
+// This allows users to recover accidentally deleted files.
-	var firstErr error
+func trashPathWithProgress(root string, counter *int64) (int64, error) {
-
+	// Verify path exists.
-	err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
+	info, err := os.Stat(root)
 	if err != nil {
-			// Skip permission errors but continue.
+		return 0, err
-			if os.IsPermission(err) {
-				if firstErr == nil {
-					firstErr = err
-				}
-				return filepath.SkipDir
-			}
-			if firstErr == nil {
-				firstErr = err
 	}
+
+	// Count items for progress reporting.
+	var count int64
+	if info.IsDir() {
+		_ = filepath.WalkDir(root, func(_ string, d os.DirEntry, err error) error {
+			if err != nil {
 				return nil
 			}
-
 			if !d.IsDir() {
-			if removeErr := os.Remove(path); removeErr == nil {
 				count++
 				if counter != nil {
 					atomic.StoreInt64(counter, count)
 				}
-			} else if firstErr == nil {
-				firstErr = removeErr
 			}
+			return nil
+		})
+	} else {
+		count = 1
+		if counter != nil {
+			atomic.StoreInt64(counter, 1)
+		}
+	}
+
+	// Move to Trash using Finder AppleScript.
+	if err := moveToTrash(root); err != nil {
+		return 0, err
+	}
+
+	return count, nil
+}
+
+// moveToTrash uses macOS Finder to move a file/directory to Trash.
+// This is the safest method as it uses the system's native trash mechanism.
+func moveToTrash(path string) error {
+	absPath, err := filepath.Abs(path)
+	if err != nil {
+		return fmt.Errorf("failed to resolve path: %w", err)
+	}
+
+	// Escape path for AppleScript (handle quotes and backslashes).
+	escapedPath := strings.ReplaceAll(absPath, "\\", "\\\\")
+	escapedPath = strings.ReplaceAll(escapedPath, "\"", "\\\"")
+
+	script := fmt.Sprintf(`tell application "Finder" to delete POSIX file "%s"`, escapedPath)
+
+	ctx, cancel := context.WithTimeout(context.Background(), trashTimeout)
+	defer cancel()
+
+	cmd := exec.CommandContext(ctx, "osascript", "-e", script)
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		if ctx.Err() == context.DeadlineExceeded {
+			return fmt.Errorf("timeout moving to Trash")
+		}
+		return fmt.Errorf("failed to move to Trash: %s", strings.TrimSpace(string(output)))
 	}
 
 	return nil
-	})
-
-	if err != nil && firstErr == nil {
-		firstErr = err
-	}
-
-	if removeErr := os.RemoveAll(root); removeErr != nil {
-		if firstErr == nil {
-			firstErr = removeErr
-		}
-	}
-
-	return count, firstErr
 }

cmd/analyze/delete_test.go

@@ -6,7 +6,47 @@ import (
 	"testing"
 )
 
+func TestTrashPathWithProgress(t *testing.T) {
+	// Skip in CI environments where Finder may not be available.
+	if os.Getenv("CI") != "" {
+		t.Skip("Skipping Finder-dependent test in CI")
+	}
+
+	parent := t.TempDir()
+	target := filepath.Join(parent, "target")
+	if err := os.MkdirAll(target, 0o755); err != nil {
+		t.Fatalf("create target: %v", err)
+	}
+
+	files := []string{
+		filepath.Join(target, "one.txt"),
+		filepath.Join(target, "two.txt"),
+	}
+	for _, f := range files {
+		if err := os.WriteFile(f, []byte("content"), 0o644); err != nil {
+			t.Fatalf("write %s: %v", f, err)
+		}
+	}
+
+	var counter int64
+	count, err := trashPathWithProgress(target, &counter)
+	if err != nil {
+		t.Fatalf("trashPathWithProgress returned error: %v", err)
+	}
+	if count != int64(len(files)) {
+		t.Fatalf("expected %d files trashed, got %d", len(files), count)
+	}
+	if _, err := os.Stat(target); !os.IsNotExist(err) {
+		t.Fatalf("expected target to be moved to Trash, stat err=%v", err)
+	}
+}
+
 func TestDeleteMultiplePathsCmdHandlesParentChild(t *testing.T) {
+	// Skip in CI environments where Finder may not be available.
+	if os.Getenv("CI") != "" {
+		t.Skip("Skipping Finder-dependent test in CI")
+	}
+
 	base := t.TempDir()
 	parent := filepath.Join(base, "parent")
 	child := filepath.Join(parent, "child")
@@ -32,12 +72,16 @@ func TestDeleteMultiplePathsCmdHandlesParentChild(t *testing.T) {
 		t.Fatalf("unexpected error: %v", progress.err)
 	}
 	if progress.count != 2 {
-		t.Fatalf("expected 2 files deleted, got %d", progress.count)
+		t.Fatalf("expected 2 files trashed, got %d", progress.count)
 	}
 	if _, err := os.Stat(parent); !os.IsNotExist(err) {
-		t.Fatalf("expected parent to be removed, err=%v", err)
+		t.Fatalf("expected parent to be moved to Trash, err=%v", err)
-	}
+	}
-	if _, err := os.Stat(child); !os.IsNotExist(err) {
+}
-		t.Fatalf("expected child to be removed, err=%v", err)
+
+func TestMoveToTrashNonExistent(t *testing.T) {
+	err := moveToTrash("/nonexistent/path/that/does/not/exist")
+	if err == nil {
+		t.Fatal("expected error for non-existent path")
 	}
 }

cmd/analyze/main.go

@@ -530,7 +530,7 @@ func (m model) updateKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
 	// Delete confirm flow.
 	if m.deleteConfirm {
 		switch msg.String() {
-		case "delete", "backspace":
+		case "enter":
 			m.deleteConfirm = false
 			m.deleting = true
 			var deleteCount int64

cmd/analyze/view.go

@@ -390,12 +390,12 @@ func (m model) View() string {
 		}
 
 		if deleteCount > 1 {
-			fmt.Fprintf(&b, "%sDelete:%s %d items (%s)  %sPress ⌫ again  |  ESC cancel%s\n",
+			fmt.Fprintf(&b, "%sDelete:%s %d items (%s)  %sPress Enter to confirm  |  ESC cancel%s\n",
 				colorRed, colorReset,
 				deleteCount, humanizeBytes(totalDeleteSize),
 				colorGray, colorReset)
 		} else {
-			fmt.Fprintf(&b, "%sDelete:%s %s (%s)  %sPress ⌫ again  |  ESC cancel%s\n",
+			fmt.Fprintf(&b, "%sDelete:%s %s (%s)  %sPress Enter to confirm  |  ESC cancel%s\n",
 				colorRed, colorReset,
 				m.deleteTarget.Name, humanizeBytes(m.deleteTarget.Size),
 				colorGray, colorReset)