[bugfix] Default to true for core.exportkeys even in substores (gopas…

…spw#2848)

* [bugfix] Default to true for core.exportkeys even in substores

This PR changes the default for core.exportkeys from false to true
in mounted substores to match the default of the global root store.

It also refactors and simplifies the config package a little bit
by removing special typed lookup methods and replacing them with
conversion helpers that can be applied to any string.

Fixes gopasspw#2830

Signed-off-by: Dominik Schulz <[email protected]>

* Fix config tests

Signed-off-by: Dominik Schulz <[email protected]>

---------

Signed-off-by: Dominik Schulz <[email protected]>

internal/action/generate.go

@@ -118,13 +118,13 @@ func (s *Action) generateCopyOrPrint(ctx context.Context, c *cli.Context, name,
 	// copy to clipboard if:
 	// - explicitly requested with -c
 	// - autoclip=true, but only if output is not being redirected.
-	if IsClip(ctx) || (s.cfg.GetBool("generate.autoclip") && ctxutil.IsTerminal(ctx)) {
-		if err := clipboard.CopyTo(ctx, name, []byte(password), s.cfg.GetInt("core.cliptimeout")); err != nil {
+	if IsClip(ctx) || (config.AsBool(s.cfg.Get("generate.autoclip")) && ctxutil.IsTerminal(ctx)) {
+		if err := clipboard.CopyTo(ctx, name, []byte(password), config.AsInt(s.cfg.Get("core.cliptimeout"))); err != nil {
 			return exit.Error(exit.IO, err, "failed to copy to clipboard: %s", err)
 		}
 		// if autoclip is on and we're not printing the password to the terminal
 		// at least leave a notice that we did indeed copy it.
-		if s.cfg.GetBool("generate.autoclip") && !c.Bool("print") {
+		if config.AsBool(s.cfg.Get("generate.autoclip")) && !c.Bool("print") {
 			out.Print(ctx, "Copied to clipboard")
 
 			return nil
@@ -177,7 +177,7 @@ func (s *Action) generatePassword(ctx context.Context, c *cli.Context, length, n
 		symbols = c.Bool("symbols")
 	} else {
 		if cfg.GetM(mp, "generate.symbols") != "" {
-			symbols = cfg.GetBoolM(mp, "generate.symbols")
+			symbols = config.AsBool(cfg.GetM(mp, "generate.symbols"))
 		}
 	}
 
@@ -521,5 +521,5 @@ func isStrict(ctx context.Context, c *cli.Context) bool {
 	}
 
 	// if the config option is not set, GetBoolM will return false by default
-	return cfg.GetBoolM(mp, "generate.strict")
+	return config.AsBool(cfg.GetM(mp, "generate.strict"))
 }

internal/action/otp.go

@@ -181,7 +181,7 @@ func (s *Action) otp(ctx context.Context, name, qrf string, clip, pw, recurse bo
 		debug.Log("OTP period: %ds", two.Period())
 
 		if clip {
-			if err := clipboard.CopyTo(ctx, fmt.Sprintf("token for %s", name), []byte(token), s.cfg.GetInt("core.cliptimeout")); err != nil {
+			if err := clipboard.CopyTo(ctx, fmt.Sprintf("token for %s", name), []byte(token), config.AsInt(s.cfg.Get("core.cliptimeout"))); err != nil {
 				return exit.Error(exit.IO, err, "failed to copy to clipboard: %s", err)
 			}
 

internal/action/show.go

@@ -204,7 +204,7 @@ func (s *Action) showHandleOutput(ctx context.Context, name string, sec gopass.S
 	}
 
 	if (IsClip(ctx) || config.Bool(ctx, "show.autoclip")) && pw != "" {
-		if err := clipboard.CopyTo(ctx, name, []byte(pw), s.cfg.GetInt("core.cliptimeout")); err != nil {
+		if err := clipboard.CopyTo(ctx, name, []byte(pw), config.AsInt(s.cfg.Get("core.cliptimeout"))); err != nil {
 			return err
 		}
 	}

internal/action/sync.go

@@ -219,7 +219,7 @@ func (s *Action) syncMount(ctx context.Context, mp string) error {
 	}
 	syncPrintDiff(ctxno, l, ln)
 
-	exportKeys := s.cfg.GetBoolM(mp, "core.exportkeys")
+	exportKeys := config.AsBool(s.cfg.GetM(mp, "core.exportkeys"))
 	debug.Log("Syncing Mount %s. Exportkeys: %t", mp, exportKeys)
 	if err := syncImportKeys(ctxno, sub, name); err != nil {
 		return err

internal/action/version.go

@@ -53,7 +53,7 @@ func (s *Action) checkVersion(ctx context.Context, u chan string) {
 	}
 
 	// NB: "updater.check" isn't supported as a local config option, hence no mount point here
-	if cfg, _ := config.FromContext(ctx); cfg.IsSet("updater.check") && !cfg.GetBool("updater.check") {
+	if cfg, _ := config.FromContext(ctx); cfg.IsSet("updater.check") && !config.AsBool(cfg.Get("updater.check")) {
 		debug.Log("remote version check disabled by updater.check = false")
 
 		return

internal/config/config.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"os"
 	"strconv"
-	"strings"
 
 	"github.com/gopasspw/gopass/pkg/debug"
 	"github.com/gopasspw/gopass/pkg/gitconfig"
@@ -146,6 +145,19 @@ func (c *Config) IsSet(key string) bool {
 	return c.root.IsSet(key)
 }
 
+// IsSetM returns true if the key is set in the mount or the root config if mount is empty.
+func (c *Config) IsSetM(mount, key string) bool {
+	if mount == "" || mount == "<root>" {
+		return c.root.IsSet(key)
+	}
+
+	if cfg := c.cfgs[mount]; cfg != nil {
+		return cfg.IsSet(key)
+	}
+
+	return false
+}
+
 // Get returns the given key from the root config.
 func (c *Config) Get(key string) string {
 	return c.root.Get(key)
@@ -175,41 +187,6 @@ func (c *Config) GetM(mount, key string) string {
 	return ""
 }
 
-// GetBool returns true if the value of the key evaluates to "true".
-// Otherwise, it returns false.
-func (c *Config) GetBool(key string) bool {
-	return c.GetBoolM("", key)
-}
-
-// GetBoolM returns true if the value of the key evaluates to "true" for the provided mount,
-// or the root config if mount is empty.
-// Otherwise, it returns false.
-func (c *Config) GetBoolM(mount, key string) bool {
-	if strings.ToLower(strings.TrimSpace(c.GetM(mount, key))) == "true" {
-		return true
-	}
-
-	return false
-}
-
-// GetInt returns the integer value of the key if it can be parsed.
-// Otherwise, it returns 0.
-func (c *Config) GetInt(key string) int {
-	return c.GetIntM("", key)
-}
-
-// GetIntM returns the integer value of the key if it can be parsed for the provided mount,
-// or the root config if mount is empty
-// Otherwise, it returns 0.
-func (c *Config) GetIntM(mount, key string) int {
-	iv, err := strconv.Atoi(c.GetM(mount, key))
-	if err != nil {
-		return 0
-	}
-
-	return iv
-}
-
 // Set tries to set the key to the given value.
 // The mount option is necessary to discern between
 // the per-user (global) and possible per-directory (local)
@@ -361,7 +338,7 @@ func DefaultPasswordLengthFromEnv(ctx context.Context) (int, bool) {
 	def := DefaultPasswordLength
 	cfg, mp := FromContext(ctx)
 
-	if l := cfg.GetIntM(mp, "generate.length"); l > 0 {
+	if l := AsInt(cfg.GetM(mp, "generate.length")); l > 0 {
 		def = l
 	}
 

internal/config/config_test.go

@@ -25,8 +25,8 @@ func TestConfig(t *testing.T) {
 	require.NoError(t, cfg.Set("", "core.int", "42"))
 
 	assert.Equal(t, "foo", cfg.Get("core.string"))
-	assert.True(t, cfg.GetBool("core.bool"))
-	assert.Equal(t, 42, cfg.GetInt("core.int"))
+	assert.True(t, AsBool(cfg.Get("core.bool")))
+	assert.Equal(t, 42, AsInt(cfg.Get("core.int")))
 
 	require.NoError(t, cfg.SetEnv("env.string", "foo"))
 	assert.Equal(t, "foo", cfg.Get("env.string"))

internal/config/utils.go

@@ -1,12 +1,55 @@
 package config
 
-import "context"
+import (
+	"context"
+	"strconv"
+)
+
+// AsBool converts a string to a bool value.
+func AsBool(s string) bool {
+	return AsBoolWithDefault(s, false)
+}
+
+// AsBoolWithDefault converts a string to a bool value with a default value.
+func AsBoolWithDefault(s string, def bool) bool {
+	if s == "" {
+		return def
+	}
+
+	switch s {
+	case "1", "true", "yes", "on":
+		return true
+	case "0", "false", "no", "off":
+		return false
+	default:
+		return def
+	}
+}
+
+// AsInt converts a string to an integer value.
+func AsInt(s string) int {
+	return AsIntWithDefault(s, 0)
+}
+
+// AsIntWithDefault converts a string to an integer value with a default value.
+func AsIntWithDefault(s string, def int) int {
+	if s == "" {
+		return def
+	}
+
+	i, err := strconv.Atoi(s)
+	if err != nil {
+		return def
+	}
+
+	return i
+}
 
 // Bool returns a bool value from the config in the context.
 func Bool(ctx context.Context, key string) bool {
 	cfg, mp := FromContext(ctx)
 
-	return cfg.GetBoolM(mp, key)
+	return AsBool(cfg.GetM(mp, key))
 }
 
 // String returns a string value from the config in the context.
@@ -20,5 +63,5 @@ func String(ctx context.Context, key string) string {
 func Int(ctx context.Context, key string) int {
 	cfg, mp := FromContext(ctx)
 
-	return cfg.GetIntM(mp, key)
+	return AsInt(cfg.GetM(mp, key))
 }

internal/config/utils_test.go

@@ -0,0 +1,87 @@
+package config
+
+import (
+	"testing"
+)
+
+func TestAsBoolWithDefault(t *testing.T) {
+	tests := []struct {
+		name     string
+		s        string
+		def      bool
+		expected bool
+	}{
+		{
+			name:     "Empty string with default true",
+			s:        "",
+			def:      true,
+			expected: true,
+		},
+		{
+			name:     "Empty string with default false",
+			s:        "",
+			def:      false,
+			expected: false,
+		},
+		{
+			name:     "Valid string '1' with default true",
+			s:        "1",
+			def:      true,
+			expected: true,
+		},
+		{
+			name:     "Valid string '0' with default true",
+			s:        "0",
+			def:      true,
+			expected: false,
+		},
+		// Add more test cases here
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			result := AsBoolWithDefault(test.s, test.def)
+			if result != test.expected {
+				t.Errorf("Expected %v, but got %v", test.expected, result)
+			}
+		})
+	}
+}
+
+func TestAsIntWithDefault(t *testing.T) {
+	tests := []struct {
+		name     string
+		s        string
+		def      int
+		expected int
+	}{
+		{
+			name:     "Empty string with default 0",
+			s:        "",
+			def:      0,
+			expected: 0,
+		},
+		{
+			name:     "Valid string '123' with default 0",
+			s:        "123",
+			def:      0,
+			expected: 123,
+		},
+		{
+			name:     "Invalid string 'abc' with default 0",
+			s:        "abc",
+			def:      0,
+			expected: 0,
+		},
+		// Add more test cases here
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			result := AsIntWithDefault(test.s, test.def)
+			if result != test.expected {
+				t.Errorf("Expected %v, but got %v", test.expected, result)
+			}
+		})
+	}
+}

internal/store/leaf/recipients.go

@@ -302,7 +302,7 @@ func (s *Store) getRecipients(ctx context.Context, idf string) (*recipients.Reci
 
 	cfg, _ := config.FromContext(ctx)
 	// check recipients hash, global config takes precedence here for security reasons
-	if cfg.GetGlobal("recipients.check") != "true" && !cfg.GetBoolM(s.alias, "recipients.check") {
+	if cfg.GetGlobal("recipients.check") != "true" && !config.AsBool(cfg.GetM(s.alias, "recipients.check")) {
 		return rs, nil
 	}
 
@@ -464,7 +464,7 @@ func (s *Store) saveRecipients(ctx context.Context, rs recipientMarshaler, msg s
 	}
 
 	// save all recipients public keys to the repo if wanted
-	if cfg.GetBoolM(s.alias, "core.exportkeys") {
+	if config.AsBoolWithDefault(cfg.GetM(s.alias, "core.exportkeys"), true) {
 		debug.Log("updating exported keys")
 		if _, err := s.UpdateExportedPublicKeys(ctx, rs.IDs()); err != nil {
 			out.Errorf(ctx, "Failed to export missing public keys: %s", err)
@@ -487,7 +487,7 @@ func (s *Store) saveRecipients(ctx context.Context, rs recipientMarshaler, msg s
 		return fmt.Errorf("failed to commit changes to git: %w", err)
 	}
 
-	if !cfg.GetBoolM(s.alias, "core.autopush") {
+	if !config.AsBool(cfg.GetM(s.alias, "core.autopush")) {
 		debug.Log("not pushing to git remote, core.autopush is false")
 
 		return nil

main.go

@@ -113,7 +113,7 @@ func setupApp(ctx context.Context, sv semver.Version) (context.Context, *cli.App
 	}
 
 	// set some action callbacks
-	if !cfg.GetBool("core.autoimport") {
+	if !config.AsBool(cfg.Get("core.autoimport")) {
 		ctx = ctxutil.WithImportFunc(ctx, termio.AskForKeyImport)
 	}
 
@@ -326,7 +326,7 @@ func initContext(ctx context.Context, cfg *config.Config) context.Context {
 		// on all other platforms we should be able to use color. Only set
 		// this if it's in the config.
 		if cfg.IsSet("core.nocolor") {
-			color.NoColor = cfg.GetBool("core.nocolor")
+			color.NoColor = config.AsBool(cfg.Get("core.nocolor"))
 		}
 	}