Disable possible git concurrency during fsck (#2486)

By using the default queue some git operations might still have been
running when gopass did attempt to update exported keys. Disabling
the queue in the inner loop makes more sense since it's more predictable
and less brittle and using the queue there wouldn't help much anyway.

Fixes #2459

RELEASE_NOTES=[BUGFIX] Fix possible concurrency issues in fsck.

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

internal/queue/background.go

@@ -29,15 +29,16 @@ type Queuer interface {
 	Idle(time.Duration) error
 }
 
-// WithQueue adds the given queue to the context.
+// WithQueue adds the given queue to the context. Add a nil
+// queue to disable queuing in this context.
 func WithQueue(ctx context.Context, q *Queue) context.Context {
 	return context.WithValue(ctx, ctxKeyQueue, q)
 }
 
 // GetQueue returns an existing queue from the context or
 // returns a noop one.
 func GetQueue(ctx context.Context) Queuer {
-	if q, ok := ctx.Value(ctxKeyQueue).(*Queue); ok {
+	if q, ok := ctx.Value(ctxKeyQueue).(*Queue); ok && q != nil {
 		return q
 	}
 

internal/store/leaf/fsck.go

@@ -10,6 +10,7 @@ import (
 	"github.com/gopasspw/gopass/internal/backend"
 	"github.com/gopasspw/gopass/internal/diff"
 	"github.com/gopasspw/gopass/internal/out"
+	"github.com/gopasspw/gopass/internal/queue"
 	"github.com/gopasspw/gopass/internal/store"
 	"github.com/gopasspw/gopass/pkg/ctxutil"
 	"github.com/gopasspw/gopass/pkg/debug"
@@ -33,26 +34,49 @@ func (s *Store) Fsck(ctx context.Context, path string) error {
 		return fmt.Errorf("storage backend compaction failed: %w", err)
 	}
 
-	pcb := ctxutil.GetProgressCallback(ctx)
-
 	// then we'll make sure all the secrets are readable by us and every
 	// valid recipient
 	if path != "" {
 		out.Printf(ctx, "Checking all secrets matching %s", path)
 	}
 
+	if err := s.fsckLoop(ctx, path); err != nil {
+		return err
+	}
+
+	if err := s.storage.Push(ctx, "", ""); err != nil {
+		if !errors.Is(err, store.ErrGitNoRemote) {
+			out.Printf(ctx, "RCS Push failed: %s", err)
+		}
+	}
+
+	return nil
+}
+
+func (s *Store) fsckLoop(ctx context.Context, path string) error {
+	pcb := ctxutil.GetProgressCallback(ctx)
+
+	// disable network ops, we will push at the end. pushing on possibly
+	// every single secret could be terribly slow.
+	ctx = ctxutil.WithNoNetwork(ctx, true)
+
+	// disable the queue, for batch operations this is not necessary / wanted
+	// since different git processes might step onto each others toes.
+	ctx = queue.WithQueue(ctx, nil)
+
 	names, err := s.List(ctx, path)
 	if err != nil {
 		return fmt.Errorf("failed to list entries for %s: %w", path, err)
 	}
 	debug.Log("names (%d): %q", len(names), names)
 	sort.Strings(names)
+
 	for _, name := range names {
 		pcb()
 		if strings.HasPrefix(name, s.alias+"/") {
 			name = strings.TrimPrefix(name, s.alias+"/")
 		}
-		ctx := ctxutil.WithNoNetwork(ctx, true)
+
 		debug.Log("[%s] Checking %s", path, name)
 
 		if err := s.fsckCheckEntry(ctx, name); err != nil {
@@ -64,12 +88,6 @@ func (s *Store) Fsck(ctx context.Context, path string) error {
 		out.Errorf(ctx, "Failed to update public keys: %s", err)
 	}
 
-	if err := s.storage.Push(ctx, "", ""); err != nil {
-		if !errors.Is(err, store.ErrGitNoRemote) {
-			out.Printf(ctx, "RCS Push failed: %s", err)
-		}
-	}
-
 	return nil
 }