Delete empty parent directories on gopass rm -r

This adds house keeping around empty parent directories. They would
have been left over in some cases. This adds more proetections to avoid
that, i.e. removing empty parents on rm -r (not only rm) as well as
fsck checks.

Fixes #1723

RELEASE_NOTES=[BUGFIX] Remove empty parents on gopass rm -r

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

go.sum

@@ -104,22 +104,28 @@ github.com/xrash/smetrics v0.0.0-20200730060457-89a2a8a1fb0b h1:tnWgqoOBmInkt5pb
 github.com/xrash/smetrics v0.0.0-20200730060457-89a2a8a1fb0b/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/crypto v0.0.0-20200930160638-afb6bcd081ae h1:duLSQW+DZ5MsXKX7kc4rXlq6/mmxz4G6ewJuBPlhRe0=
 golang.org/x/crypto v0.0.0-20200930160638-afb6bcd081ae/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3 h1:0GoQqolDA55aaLxZyTzK/Y2ePZzZTUrRacwib7cNsYQ=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
+golang.org/x/net v0.0.0-20201021035429-f5854403a974 h1:IX6qOQeG5uLjB/hjjwjedwfjND0hgjPMMyO1RoIXQNI=
+golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201024232916-9f70ab9862d5 h1:iCaAy5bMeEvwANu3YnJfWwI0kWAGkEa2RXPdweI/ysk=
 golang.org/x/sys v0.0.0-20201024232916-9f70ab9862d5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
+golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20190624222133-a101b041ded4 h1:1mMox4TgefDwqluYCv677yNXwlfTkija4owZve/jr78=
 golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=

internal/action/delete.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/gopasspw/gopass/pkg/ctxutil"
+	"github.com/gopasspw/gopass/pkg/debug"
 	"github.com/gopasspw/gopass/pkg/termio"
 
 	"github.com/urfave/cli/v2"
@@ -39,17 +40,21 @@ func (s *Action) Delete(c *cli.Context) error {
 	}
 
 	if recursive {
+		debug.Log("pruning %q", name)
 		if err := s.Store.Prune(ctx, name); err != nil {
 			return ExitError(ExitUnknown, err, "failed to prune '%s': %s", name, err)
 		}
+		debug.Log("pruned %q", name)
 		return nil
 	}
 
 	// deletes a single key from a YAML doc
 	if key != "" {
+		debug.Log("removing key %q from %q", key, name)
 		return s.deleteKeyFromYAML(ctx, name, key)
 	}
 
+	debug.Log("removing entry %q", name)
 	if err := s.Store.Delete(ctx, name); err != nil {
 		return ExitError(ExitIO, err, "Can not delete '%s': %s", name, err)
 	}

internal/backend/storage/fs/fsck.go

@@ -2,8 +2,11 @@ package fs
 
 import (
 	"context"
+	"io/ioutil"
 	"os"
 	"path/filepath"
+	"sort"
+	"strings"
 	"syscall"
 
 	"github.com/gopasspw/gopass/internal/out"
@@ -23,7 +26,7 @@ func (s *Store) Fsck(ctx context.Context) error {
 	dirs := make(map[string]struct{}, len(entries))
 	for _, entry := range entries {
 		pcb()
-		debug.Log("Checking %s", entry)
+		debug.Log("checking entry %q", entry)
 
 		filename := filepath.Join(s.path, entry)
 		dirs[filepath.Dir(filename)] = struct{}{}
@@ -34,11 +37,18 @@ func (s *Store) Fsck(ctx context.Context) error {
 	}
 
 	for dir := range dirs {
+		debug.Log("checking dir %q", dir)
 		if err := s.fsckCheckDir(ctx, dir); err != nil {
 			return err
 		}
 	}
-	return nil
+
+	if err := s.fsckCheckEmptyDirs(ctx); err != nil {
+		return err
+	}
+
+	debug.Log("checking root dir %q", s.path)
+	return s.fsckCheckDir(ctx, s.path)
 }
 
 func (s *Store) fsckCheckFile(ctx context.Context, filename string) error {
@@ -90,3 +100,54 @@ func (s *Store) fsckCheckDir(ctx context.Context, dirname string) error {
 	}
 	return nil
 }
+
+func (s *Store) fsckCheckEmptyDirs(ctx context.Context) error {
+	v := []string{}
+	if err := filepath.Walk(s.path, func(fp string, fi os.FileInfo, ferr error) error {
+		if ferr != nil {
+			return ferr
+		}
+		if !fi.IsDir() {
+			return nil
+		}
+		if strings.HasPrefix(fi.Name(), ".") {
+			return filepath.SkipDir
+		}
+		if fp == s.path {
+			return nil
+		}
+
+		// add candidate
+		debug.Log("adding candidate %q", fp)
+		v = append(v, fp)
+		return nil
+	}); err != nil {
+		return err
+	}
+
+	// start with longest path (deepest dir)
+	sort.Slice(v, func(i, j int) bool {
+		return len(v[i]) > len(v[j])
+	})
+
+	for _, d := range v {
+		if err := fsckRemoveEmptyDir(d); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func fsckRemoveEmptyDir(fp string) error {
+	ls, err := ioutil.ReadDir(fp)
+	if err != nil {
+		return err
+	}
+	if len(ls) > 0 {
+		debug.Log("dir %q is not empty (%d)", fp, len(ls))
+		return nil
+	}
+
+	debug.Log("removing %q ...", fp)
+	return os.Remove(fp)
+}

internal/backend/storage/fs/store.go

@@ -88,6 +88,7 @@ func (s *Store) removeEmptyParentDirectories(path string) error {
 		return nil
 	}
 
+	debug.Log("removing empty parent dir: %q", parent)
 	err := os.Remove(parent)
 	switch {
 	case err == nil:
@@ -162,7 +163,12 @@ func (s *Store) IsDir(ctx context.Context, name string) bool {
 func (s *Store) Prune(ctx context.Context, prefix string) error {
 	path := filepath.Join(s.path, filepath.Clean(prefix))
 	debug.Log("Purning %s from %s", prefix, path)
-	return os.RemoveAll(path)
+
+	if err := os.RemoveAll(path); err != nil {
+		return err
+	}
+
+	return s.removeEmptyParentDirectories(path)
 }
 
 // Name returns the name of this backend

internal/store/leaf/move.go

@@ -76,6 +76,8 @@ func (s *Store) delete(ctx context.Context, name string, recurse bool) error {
 		}
 	}
 	if err := s.deleteSingle(ctx, path); err != nil {
+		// might fail if we deleted the root of a tree which isn't a secret
+		// itself
 		if !recurse {
 			return err
 		}
@@ -115,6 +117,7 @@ func (s *Store) deleteRecurse(ctx context.Context, name, path string) error {
 
 	debug.Log("Pruning %s", name)
 	if err := s.storage.Prune(ctx, name); err != nil {
+		debug.Log("storage.Prune(%v) failed", name)
 		return err
 	}
 
@@ -124,6 +127,7 @@ func (s *Store) deleteRecurse(ctx context.Context, name, path string) error {
 		}
 		return errors.Wrapf(err, "failed to add '%s' to git", path)
 	}
+	debug.Log("pruned")
 	return nil
 }