Prevent gopass from silently deleting entries when moving (gopasspw#2211

)

Fixes gopasspw#2210

RELEASE_NOTES=[BUGFIX] Stop eating secrets on move

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>

internal/store/root/move.go

@@ -146,13 +146,19 @@ func (r *Store) moveFromTo(ctx context.Context, subFrom *leaf.Store, from, to, f
 
 	debug.Log("Moving (sub) tree %q to %q (entries: %+v)", from, to, entries)
 
+	var moved uint
 	for _, src := range entries {
 		dst := computeMoveDestination(src, from, to, srcIsDir, dstIsDir)
+		if src == dst {
+			debug.Log("skipping %q. src eq dst", src)
 
+			continue
+		}
 		debug.Log("Moving entry %q (%q) => %q (%q) (srcIsDir:%t, dstIsDir:%t, delete:%t)\n", src, from, dst, to, srcIsDir, dstIsDir, del)
 
 		err := r.directMove(ctx, src, dst, del)
 		if err == nil {
+			moved++
 			debug.Log("directly moved from %q to %q", src, dst)
 
 			continue
@@ -176,6 +182,12 @@ func (r *Store) moveFromTo(ctx context.Context, subFrom *leaf.Store, from, to, f
 				return fmt.Errorf("failed to delete secret %q: %w", src, err)
 			}
 		}
+
+		moved++
+	}
+
+	if moved < 1 {
+		return fmt.Errorf("no entries moved")
 	}
 
 	debug.Log("Moved (sub) tree %q to %q", from, to)
@@ -260,6 +272,7 @@ func computeMoveDestination(src, from, to string, srcIsDir, dstIsDir bool) strin
 	if strings.HasSuffix(from, "/") {
 		return path.Join(to, strings.TrimPrefix(src, from))
 	}
+
 	// move a b, where a is a directory but not b, i.e. rename a to b.
 	// this is applied to every child of a, so we need to remove the
 	// old prefix (a) and add the new one (b).

internal/store/root/move_test.go

@@ -317,6 +317,43 @@ func TestCopy(t *testing.T) {
 	})
 }
 
+func TestMoveSelf(t *testing.T) {
+	t.Parallel()
+
+	u := gptest.NewUnitTester(t)
+	u.Entries = []string{
+		"foo/bar/example",
+	}
+	require.NoError(t, u.InitStore(""))
+	defer u.Remove()
+
+	ctx := context.Background()
+	ctx = ctxutil.WithAlwaysYes(ctx, true)
+	ctx = ctxutil.WithHidden(ctx, true)
+
+	rs, err := createRootStore(ctx, u)
+	require.NoError(t, err)
+
+	// Initial state:
+	t.Run("initial state", func(t *testing.T) { //nolint:paralleltest
+		entries, err := rs.List(ctx, tree.INF)
+		require.NoError(t, err)
+		assert.Equal(t, []string{
+			"foo/bar/example",
+		}, entries)
+	})
+
+	// -> move foo/bar/example foo/bar -> no op
+	t.Run("move self", func(t *testing.T) { //nolint:paralleltest
+		assert.Error(t, rs.Move(ctx, "foo/bar/example", "foo/bar"))
+		entries, err := rs.List(ctx, tree.INF)
+		require.NoError(t, err)
+		assert.Equal(t, []string{
+			"foo/bar/example",
+		}, entries)
+	})
+}
+
 func TestComputeMoveDestination(t *testing.T) {
 	t.Parallel()
 
@@ -380,6 +417,15 @@ func TestComputeMoveDestination(t *testing.T) {
 			srcIsDir: false,
 			dstIsDir: true,
 		},
+		{
+			name:     "one level up", // mv foo/bar/example foo/bar
+			src:      "foo/bar/example",
+			from:     "foo/bar",
+			to:       "foo/bar",
+			dst:      "foo/bar/example",
+			srcIsDir: false,
+			dstIsDir: true,
+		},
 	} {
 		tc := tc
 

pkg/clipboard/clipboard.go

@@ -10,6 +10,7 @@ import (
 	"github.com/fatih/color"
 	"github.com/gopasspw/gopass/internal/notify"
 	"github.com/gopasspw/gopass/internal/out"
+	"github.com/gopasspw/gopass/pkg/debug"
 )
 
 var (
@@ -23,6 +24,8 @@ var (
 // CopyTo copies the given data to the clipboard and enqueues automatic
 // clearing of the clipboard.
 func CopyTo(ctx context.Context, name string, content []byte, timeout int) error {
+	debug.Log("Copying to clipboard: %s for %ds", name, timeout)
+
 	clipboardCopyCMD := os.Getenv("GOPASS_CLIPBOARD_COPY_CMD")
 	if clipboardCopyCMD != "" {
 		if err := callCommand(ctx, clipboardCopyCMD, name, content); err != nil {