Abort if gpg binary can not be found (gopasspw#506)

Fixes gopasspw#496

action/action.go

@@ -15,13 +15,18 @@ import (
 )
 
 type gpger interface {
-	FindPublicKeys(context.Context, ...string) (gpg.KeyList, error)
-	FindPrivateKeys(context.Context, ...string) (gpg.KeyList, error)
+	Binary() string
 	ListPublicKeys(context.Context) (gpg.KeyList, error)
+	FindPublicKeys(context.Context, ...string) (gpg.KeyList, error)
 	ListPrivateKeys(context.Context) (gpg.KeyList, error)
 	CreatePrivateKeyBatch(context.Context, string, string, string) error
 	CreatePrivateKey(context.Context) error
+	FindPrivateKeys(context.Context, ...string) (gpg.KeyList, error)
+	GetRecipients(context.Context, string) ([]string, error)
+	Encrypt(context.Context, string, []byte, []string) error
+	Decrypt(context.Context, string) ([]byte, error)
 	ExportPublicKey(context.Context, string, string) error
+	ImportPublicKey(context.Context, string) error
 	Version(context.Context) semver.Version
 }
 
@@ -35,7 +40,7 @@ type Action struct {
 }
 
 // New returns a new Action wrapper
-func New(ctx context.Context, cfg *config.Config, sv semver.Version) *Action {
+func New(ctx context.Context, cfg *config.Config, sv semver.Version) (*Action, error) {
 	name := "gopass"
 	if len(os.Args) > 0 {
 		name = filepath.Base(os.Args[0])
@@ -47,18 +52,22 @@ func New(ctx context.Context, cfg *config.Config, sv semver.Version) *Action {
 		version: sv,
 	}
 
-	store, err := root.New(ctx, cfg)
-	if err != nil {
-		panic(err)
-	}
-	act.Store = store
-
-	act.gpg = gpgcli.New(gpgcli.Config{
+	var err error
+	act.gpg, err = gpgcli.New(gpgcli.Config{
 		Umask: umask(),
 		Args:  gpgOpts(),
 	})
+	if err != nil {
+		return nil, err
+	}
+
+	store, err := root.New(ctx, cfg, act.gpg)
+	if err != nil {
+		return nil, err
+	}
+	act.Store = store
 
-	return act
+	return act, nil
 }
 
 func umask() int {

action/clihelper_test.go

@@ -13,11 +13,14 @@ import (
 func Test_askForGitConfigUser(t *testing.T) {
 	ctx := context.Background()
 
-	s := New(ctx, config.Load(), semver.Version{})
+	s, err := New(ctx, config.Load(), semver.Version{})
+	if err != nil {
+		t.Fatalf("%s", err)
+	}
 
 	ctx = ctxutil.WithTerminal(ctx, true)
 
-	_, _, err := s.askForGitConfigUser(ctx)
+	_, _, err = s.askForGitConfigUser(ctx)
 	if err == nil {
 		t.Error("Did not return error")
 	}
@@ -26,7 +29,10 @@ func Test_askForGitConfigUser(t *testing.T) {
 func Test_askForGitConfigUserNonInteractive(t *testing.T) {
 	ctx := context.Background()
 
-	s := New(ctx, config.Load(), semver.Version{})
+	s, err := New(ctx, config.Load(), semver.Version{})
+	if err != nil {
+		t.Fatalf("%s", err)
+	}
 
 	ctx = ctxutil.WithTerminal(ctx, false)
 

action/errors.go

@@ -49,6 +49,8 @@ const (
 	ExitRecipients
 	// ExitIO is used for misc. I/O errors
 	ExitIO
+	// ExitGPG is used for misc. gpg errors
+	ExitGPG
 )
 
 func (s *Action) exitError(ctx context.Context, exitCode int, err error, format string, args ...interface{}) error {

backend/gpg/cli/gpg.go

@@ -46,7 +46,7 @@ type Config struct {
 }
 
 // New creates a new GPG wrapper
-func New(cfg Config) *GPG {
+func New(cfg Config) (*GPG, error) {
 	// ensure created files don't have group or world perms set
 	// this setting should be inherited by sub-processes
 	umask(cfg.Umask)
@@ -62,9 +62,11 @@ func New(cfg Config) *GPG {
 		binary: "gpg",
 		args:   append(defaultArgs, cfg.Args...),
 	}
-	_ = g.detectBinary(cfg.Binary)
+	if err := g.detectBinary(cfg.Binary); err != nil {
+		return nil, err
+	}
 
-	return g
+	return g, nil
 }
 
 // Binary returns the GPG binary location

backend/gpg/cli/gpg_others.go

@@ -4,21 +4,32 @@ package cli
 
 import (
 	"context"
+	"errors"
 	"os/exec"
 
 	"github.com/blang/semver"
 )
 
 func (g *GPG) detectBinary(bin string) error {
-	for _, b := range []string{bin, "gpg", "gpg2", "gpg1", "gpg"} {
+	bins := []string{"gpg", "gpg2", "gpg1", "gpg"}
+	if bin != "" {
+		bins = append([]string{bin}, bins...)
+	}
+	for i, b := range bins {
 		if p, err := exec.LookPath(b); err == nil {
 			g.binary = p
 			// if we found a GPG 2.x binary we're good, otherwise we try the
 			// others as well
 			if g.Version(context.Background()).GTE(semver.Version{Major: 2}) {
-				break
+				return nil
+			}
+			// in case gpg is version 1 and there may be a gpg2 for version 2 we
+			// don't want to return immedeately on the first gpg try, but afterwards
+			// we take the "best" / first available binary
+			if b != "gpg" && i > len(bins)-2 {
+				return nil
 			}
 		}
 	}
-	return nil
+	return errors.New("no gpg binary found")
 }

main.go

@@ -14,7 +14,7 @@ import (
 
 	"github.com/blang/semver"
 	"github.com/fatih/color"
-	"github.com/justwatchcom/gopass/action"
+	ap "github.com/justwatchcom/gopass/action"
 	"github.com/justwatchcom/gopass/backend/gpg"
 	"github.com/justwatchcom/gopass/config"
 	"github.com/justwatchcom/gopass/store/sub"
@@ -155,7 +155,11 @@ func main() {
 
 	cli.VersionPrinter = makeVersionPrinter(sv)
 
-	action := action.New(ctx, cfg, sv)
+	action, err := ap.New(ctx, cfg, sv)
+	if err != nil {
+		out.Red(ctx, "No gpg binary found: %s", err)
+		os.Exit(ap.ExitGPG)
+	}
 
 	// set some action callbacks
 	if !cfg.Root.AutoImport {

store/root/gpg.go

@@ -4,8 +4,23 @@ import (
 	"context"
 
 	"github.com/blang/semver"
+	"github.com/justwatchcom/gopass/backend/gpg"
 )
 
+type gpger interface {
+	Binary() string
+	ListPublicKeys(context.Context) (gpg.KeyList, error)
+	FindPublicKeys(context.Context, ...string) (gpg.KeyList, error)
+	ListPrivateKeys(context.Context) (gpg.KeyList, error)
+	FindPrivateKeys(context.Context, ...string) (gpg.KeyList, error)
+	GetRecipients(context.Context, string) ([]string, error)
+	Encrypt(context.Context, string, []byte, []string) error
+	Decrypt(context.Context, string) ([]byte, error)
+	ExportPublicKey(context.Context, string, string) error
+	ImportPublicKey(context.Context, string) error
+	Version(context.Context) semver.Version
+}
+
 // GPGVersion returns GPG version information
 func (r *Store) GPGVersion(ctx context.Context) semver.Version {
 	return r.store.GPGVersion(ctx)

store/root/init.go

@@ -13,7 +13,7 @@ func (r *Store) Initialized() bool {
 
 // Init tries to initialize a new password store location matching the object
 func (r *Store) Init(ctx context.Context, alias, path string, ids ...string) error {
-	sub := sub.New(alias, path)
+	sub := sub.New(alias, path, r.gpg)
 	if !r.store.Initialized() && alias == "" {
 		r.store = sub
 	}

store/root/mount.go

@@ -37,7 +37,7 @@ func (r *Store) addMount(ctx context.Context, alias, path string, keys ...string
 	}
 
 	// propagate our config settings to the sub store
-	s := sub.New(alias, path)
+	s := sub.New(alias, path, r.gpg)
 
 	if !s.Initialized() {
 		if len(keys) < 1 {

store/root/store.go

@@ -5,18 +5,12 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/justwatchcom/gopass/backend/gpg"
-	gpgcli "github.com/justwatchcom/gopass/backend/gpg/cli"
 	"github.com/justwatchcom/gopass/config"
 	"github.com/justwatchcom/gopass/store/sub"
 	"github.com/justwatchcom/gopass/utils/fsutil"
 	"github.com/pkg/errors"
 )
 
-type gpger interface {
-	FindPublicKeys(context.Context, ...string) (gpg.KeyList, error)
-}
-
 // Store is the public facing password store
 type Store struct {
 	cfg     *config.Config
@@ -28,7 +22,7 @@ type Store struct {
 }
 
 // New creates a new store
-func New(ctx context.Context, cfg *config.Config) (*Store, error) {
+func New(ctx context.Context, cfg *config.Config, gpg gpger) (*Store, error) {
 	if cfg == nil {
 		cfg = &config.Config{}
 	}
@@ -37,14 +31,14 @@ func New(ctx context.Context, cfg *config.Config) (*Store, error) {
 	}
 	r := &Store{
 		cfg:     cfg,
-		gpg:     gpgcli.New(gpgcli.Config{}),
+		gpg:     gpg,
 		mounts:  make(map[string]*sub.Store, len(cfg.Mounts)),
 		path:    cfg.Root.Path,
 		version: cfg.Version,
 	}
 
 	// create the base store
-	r.store = sub.New("", r.Path())
+	r.store = sub.New("", r.Path(), gpg)
 
 	// initialize all mounts
 	for alias, sc := range cfg.Mounts {

store/root/store_test.go

@@ -12,6 +12,7 @@ import (
 
 	"path"
 
+	gpgmock "github.com/justwatchcom/gopass/backend/gpg/mock"
 	"github.com/justwatchcom/gopass/config"
 	"github.com/justwatchcom/gopass/store/sub"
 )
@@ -32,11 +33,15 @@ func TestSimpleList(t *testing.T) {
 		t.Fatalf("Failed to create store directory: %s", err)
 	}
 
-	rs, err := New(ctx, &config.Config{
-		Root: &config.StoreConfig{
-			Path: tempdir,
+	rs, err := New(
+		ctx,
+		&config.Config{
+			Root: &config.StoreConfig{
+				Path: tempdir,
+			},
 		},
-	})
+		gpgmock.New(),
+	)
 	if err != nil {
 		t.Fatalf("Failed to create root store: %s", err)
 	}
@@ -85,11 +90,15 @@ func TestListMulti(t *testing.T) {
 	}
 	sort.Strings(ents)
 
-	rs, err := New(ctx, &config.Config{
-		Root: &config.StoreConfig{
-			Path: filepath.Join(tempdir, "root"),
+	rs, err := New(
+		ctx,
+		&config.Config{
+			Root: &config.StoreConfig{
+				Path: filepath.Join(tempdir, "root"),
+			},
 		},
-	})
+		gpgmock.New(),
+	)
 	if err != nil {
 		t.Fatalf("Failed to create root store: %s", err)
 	}
@@ -150,11 +159,15 @@ func TestListNested(t *testing.T) {
 	}
 	sort.Strings(ents)
 
-	rs, err := New(ctx, &config.Config{
-		Root: &config.StoreConfig{
-			Path: filepath.Join(tempdir, "root"),
+	rs, err := New(
+		ctx,
+		&config.Config{
+			Root: &config.StoreConfig{
+				Path: filepath.Join(tempdir, "root"),
+			},
 		},
-	})
+		gpgmock.New(),
+	)
 	if err != nil {
 		t.Fatalf("Failed to create root store: %s", err)
 	}

store/sub/recipients_test.go

@@ -198,7 +198,11 @@ func TestListRecipients(t *testing.T) {
 	genRecs, _, err := createStore(tempdir, nil, nil)
 	assert.NoError(t, err)
 
-	s := New("", tempdir)
+	s := New(
+		"",
+		tempdir,
+		gpgmock.New(),
+	)
 
 	rs, err := s.GetRecipients(ctx, "")
 	assert.NoError(t, err)

store/sub/store.go

@@ -8,7 +8,6 @@ import (
 	"strings"
 
 	gitcli "github.com/justwatchcom/gopass/backend/git/cli"
-	gpgcli "github.com/justwatchcom/gopass/backend/gpg/cli"
 	"github.com/justwatchcom/gopass/store"
 	"github.com/justwatchcom/gopass/utils/ctxutil"
 	"github.com/justwatchcom/gopass/utils/fsutil"
@@ -31,9 +30,8 @@ type Store struct {
 }
 
 // New creates a new store, copying settings from the given root store
-func New(alias string, path string) *Store {
+func New(alias string, path string, gpg gpger) *Store {
 	path = fsutil.CleanPath(path)
-	gpg := gpgcli.New(gpgcli.Config{})
 	return &Store{
 		alias: alias,
 		path:  path,