[BUGFIX] Try to always honor local config for mounts (#2724)

* [BUGFIX] Try to always honor local config for mounts Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com> * [BUGFIX] Better propagate mount to config Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com> * [n/a] CR nit fix Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com> --------- Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>
gopasspw · Nov 25, 2023 · 5f18942 · 5f18942
1 parent d168602
commit 5f18942
Show file tree

Hide file tree

Showing 29 changed files with 208 additions and 135 deletions.
diff --git a/docs/config.md b/docs/config.md
diff --git a/helpers/man/main.go b/helpers/man/main.go
@@ -27,7 +27,7 @@ import (
 )
 
 func main() {
-	// this is a workaround for the man helper getting accidentially
+	// this is a workaround for the man helper getting accidentally
 	// installed into my $GOBIN dir and me not being able to figure out
 	// why. So instead of being greeted with an ugly panic message
 	// every now and then when I need to open a man page I decided

diff --git a/internal/action/aliases.go b/internal/action/aliases.go
@@ -9,7 +9,7 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
-// AliasesPrint prints all cofigured aliases.
+// AliasesPrint prints all configured aliases.
 func (s *Action) AliasesPrint(c *cli.Context) error {
 	out.Printf(c.Context, "Configured aliases:")
 	aliases := pwrules.AllAliases(c.Context)

diff --git a/internal/action/convert.go b/internal/action/convert.go
@@ -4,6 +4,7 @@ import (
 	"github.com/gopasspw/gopass/internal/action/exit"
 	"github.com/gopasspw/gopass/internal/backend"
 	"github.com/gopasspw/gopass/internal/backend/crypto/age"
+	"github.com/gopasspw/gopass/internal/config"
 	"github.com/gopasspw/gopass/internal/out"
 	"github.com/gopasspw/gopass/pkg/ctxutil"
 	"github.com/gopasspw/gopass/pkg/debug"
@@ -24,6 +25,9 @@ func (s *Action) Convert(c *cli.Context) error {
 		return exit.Error(exit.NotFound, err, "mount %q not found: %s", store, err)
 	}
 
+	// we know it's a valid mount at this point
+	ctx = config.WithMount(ctx, store)
+
 	oldStorage := sub.Storage().Name()
 
 	storage, err := backend.StorageRegistry.Backend(oldStorage)

diff --git a/internal/action/create.go b/internal/action/create.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/gopasspw/gopass/internal/action/exit"
+	"github.com/gopasspw/gopass/internal/config"
 	"github.com/gopasspw/gopass/internal/create"
 	"github.com/gopasspw/gopass/internal/cui"
 	"github.com/gopasspw/gopass/internal/hook"
@@ -50,7 +51,7 @@ func (s *Action) createPrintOrCopy(ctx context.Context, c *cli.Context, name, pa
 		return nil
 	}
 
-	if err := clipboard.CopyTo(ctx, name, []byte(password), s.cfg.GetInt("core.cliptimeout")); err != nil {
+	if err := clipboard.CopyTo(ctx, name, []byte(password), config.Int(ctx, "core.cliptimeout")); err != nil {
 		return exit.Error(exit.IO, err, "failed to copy to clipboard: %s", err)
 	}
 

diff --git a/internal/action/generate.go b/internal/action/generate.go
@@ -58,6 +58,9 @@ func (s *Action) Generate(c *cli.Context) error {
 		}
 	}
 
+	mp := s.Store.MountPoint(name)
+	ctx = config.WithMount(ctx, mp)
+
 	// generate password.
 	password, err := s.generatePassword(ctx, c, length, name)
 	if err != nil {
@@ -167,17 +170,18 @@ func (s *Action) generatePassword(ctx context.Context, c *cli.Context, length, n
 		return s.generatePasswordForRule(ctx, c, length, name, domain, rule)
 	}
 
-	cfg := config.FromContext(ctx)
+	cfg, mp := config.FromContext(ctx)
+
 	symbols := false
 	if c.IsSet("symbols") {
 		symbols = c.Bool("symbols")
 	} else {
-		if cfg.IsSet("generate.symbols") {
-			symbols = cfg.GetBool("generate.symbols")
+		if cfg.GetM(mp, "generate.symbols") != "" {
+			symbols = cfg.GetBoolM(mp, "generate.symbols")
 		}
 	}
 
-	generator := cfg.Get("generate.generator")
+	generator := cfg.GetM(mp, "generate.generator")
 	if c.IsSet("generator") {
 		generator = c.String("generator")
 	}
@@ -510,15 +514,12 @@ func filterPrefix(in []string, prefix string) []string {
 }
 
 func isStrict(ctx context.Context, c *cli.Context) bool {
-	cfg := config.FromContext(ctx)
+	cfg, mp := config.FromContext(ctx)
 
 	if c.Bool("strict") {
 		return true
 	}
 
-	if cfg.IsSet("generate.strict") {
-		return cfg.GetBool("generate.strict")
-	}
-
-	return false
+	// if the config option is not set, GetBoolM will return false by default
+	return cfg.GetBoolM(mp, "generate.strict")
 }
diff --git a/internal/action/init.go b/internal/action/init.go
@@ -36,6 +36,9 @@ func (s *Action) IsInitialized(c *cli.Context) error {
 
 	if inited {
 		debug.Log("Store is fully initialized and ready to go\n\nAll systems go. 🚀\n")
+		name := c.Args().First()
+		// setting the mount point here is not enough when we're using the REPL mode
+		ctx = config.WithMount(ctx, s.Store.MountPoint(name))
 		s.printReminder(ctx)
 		if c.Command.Name != "sync" && !c.Bool("nosync") {
 			_ = s.autoSync(ctx)

diff --git a/internal/action/otp.go b/internal/action/otp.go
@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/gopasspw/gopass/internal/action/exit"
+	"github.com/gopasspw/gopass/internal/config"
 	"github.com/gopasspw/gopass/internal/out"
 	"github.com/gopasspw/gopass/internal/store"
 	"github.com/gopasspw/gopass/pkg/clipboard"
@@ -110,6 +111,7 @@ func (s *Action) otp(ctx context.Context, name, qrf string, clip, pw, recurse bo
 		return s.otpHandleError(ctx, name, qrf, clip, pw, recurse, err)
 	}
 
+	ctx = config.WithMount(ctx, s.Store.MountPoint(name))
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 

diff --git a/internal/action/recipients.go b/internal/action/recipients.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/gopasspw/gopass/internal/action/exit"
+	"github.com/gopasspw/gopass/internal/config"
 	"github.com/gopasspw/gopass/internal/cui"
 	"github.com/gopasspw/gopass/internal/out"
 	"github.com/gopasspw/gopass/internal/set"
@@ -112,6 +113,7 @@ func (s *Action) RecipientsAdd(c *cli.Context) error {
 			out.Warningf(ctx, "Failed to list public key %q: %s", r, err)
 			var imported bool
 			if sub, err := s.Store.GetSubStore(store); err == nil {
+				ctx = config.WithMount(ctx, store)
 				if err := sub.ImportMissingPublicKeys(ctx, r); err != nil {
 					out.Warningf(ctx, "Failed to import missing public keys: %s", err)
 				}

diff --git a/internal/action/reminder.go b/internal/action/reminder.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"os"
 
+	"github.com/gopasspw/gopass/internal/config"
 	"github.com/gopasspw/gopass/internal/env"
 	"github.com/gopasspw/gopass/internal/out"
 	"github.com/gopasspw/gopass/pkg/ctxutil"
@@ -18,7 +19,7 @@ func (s *Action) printReminder(ctx context.Context) {
 		return
 	}
 
-	if sv := os.Getenv("GOPASS_NO_REMINDER"); sv != "" {
+	if sv := os.Getenv("GOPASS_NO_REMINDER"); sv != "" || config.Bool(ctx, "core.noreminder") {
 		return
 	}
 

diff --git a/internal/action/repl.go b/internal/action/repl.go
@@ -133,7 +133,7 @@ func (s *Action) REPL(c *cli.Context) error {
 
 READ:
 	for {
-		// check for context cancelation
+		// check for context cancellation
 		select {
 		case <-c.Context.Done():
 			return fmt.Errorf("user aborted")

diff --git a/internal/action/show.go b/internal/action/show.go
@@ -105,6 +105,9 @@ func (s *Action) show(ctx context.Context, c *cli.Context, name string, recurse
 		out.Warningf(ctx, "%s is a secret and a folder. Use 'gopass show %s' to display the secret and 'gopass list %s' to show the content of the folder", name, name, name)
 	}
 
+	mp := s.Store.MountPoint(name)
+	ctx = config.WithMount(ctx, mp)
+
 	if HasRevision(ctx) {
 		return s.showHandleRevision(ctx, c, name, GetRevision(ctx))
 	}

diff --git a/internal/action/sync.go b/internal/action/sync.go
@@ -109,10 +109,10 @@ func (s *Action) sync(ctx context.Context, store string) error {
 	// sync all stores (root and all mounted sub stores).
 	for _, mp := range mps {
 		if store != "" {
-			if store != "root" && mp != store {
+			if store != "<root>" && mp != store {
 				continue
 			}
-			if store == "root" && mp != "" {
+			if store == "<root>" && mp != "" {
 				continue
 			}
 		}
@@ -141,6 +141,7 @@ func (s *Action) sync(ctx context.Context, store string) error {
 	}
 
 	if numEntries != 0 {
+		ctx = config.WithMount(ctx, store)
 		_ = notify.Notify(ctx, "gopass - sync", fmt.Sprintf("Finished. Synced %d remotes.%s", numMPs, diff))
 	}
 
@@ -210,7 +211,7 @@ func (s *Action) syncMount(ctx context.Context, mp string) error {
 	}
 	syncPrintDiff(ctxno, l, ln)
 
-	exportKeys := s.cfg.GetBool("core.exportkeys")
+	exportKeys := s.cfg.GetBoolM(mp, "core.exportkeys")
 	debug.Log("Syncing Mount %s. Exportkeys: %t", mp, exportKeys)
 	if err := syncImportKeys(ctxno, sub, name); err != nil {
 		return err

diff --git a/internal/action/unclip.go b/internal/action/unclip.go
@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/gopasspw/gopass/internal/action/exit"
+	"github.com/gopasspw/gopass/internal/config"
 	"github.com/gopasspw/gopass/pkg/clipboard"
 	"github.com/gopasspw/gopass/pkg/ctxutil"
 	"github.com/urfave/cli/v2"
@@ -19,6 +20,10 @@ func (s *Action) Unclip(c *cli.Context) error {
 	checksum := os.Getenv("GOPASS_UNCLIP_CHECKSUM")
 
 	time.Sleep(time.Second * time.Duration(timeout))
+
+	mp := s.Store.MountPoint(name)
+	ctx = config.WithMount(ctx, mp)
+
 	if err := clipboard.Clear(ctx, name, checksum, force); err != nil {
 		return exit.Error(exit.IO, err, "Failed to clear clipboard: %s", err)
 	}

diff --git a/internal/action/version.go b/internal/action/version.go
@@ -52,7 +52,8 @@ func (s *Action) checkVersion(ctx context.Context, u chan string) {
 		return
 	}
 
-	if cfg := config.FromContext(ctx); cfg.IsSet("updater.check") && !cfg.GetBool("updater.check") {
+	// 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") {
 		debug.Log("remote version check disabled by updater.check = false")
 
 		return

diff --git a/internal/backend/rcs_test.go b/internal/backend/rcs_test.go
@@ -14,8 +14,6 @@ import (
 )
 
 func TestClone(t *testing.T) {
-	t.Parallel()
-
 	ctx := context.Background()
 
 	td := t.TempDir()
@@ -35,8 +33,6 @@ func TestClone(t *testing.T) {
 }
 
 func TestInitRCS(t *testing.T) {
-	t.Parallel()
-
 	ctx := context.Background()
 
 	td := t.TempDir()

diff --git a/internal/config/config.go b/internal/config/config.go
@@ -150,9 +150,15 @@ func (c *Config) GetAll(key string) []string {
 	return c.root.GetAll(key)
 }
 
+// GetGlobal returns the given key from the root global config.
+// This is typically used to prevent a local config override of sensitive config items, e.g. used for integrity checks.
+func (c *Config) GetGlobal(key string) string {
+	return c.root.GetGlobal(key)
+}
+
 // GetM returns the given key from the mount or the root config if mount is empty.
 func (c *Config) GetM(mount, key string) string {
-	if mount == "" {
+	if mount == "" || mount == "<root>" {
 		return c.root.Get(key)
 	}
 
@@ -164,19 +170,33 @@ func (c *Config) GetM(mount, key string) string {
 }
 
 // GetBool returns true if the value of the key evaluates to "true".
-// Otherwise it returns false.
+// Otherwise, it returns false.
 func (c *Config) GetBool(key string) bool {
-	if strings.ToLower(strings.TrimSpace(c.Get(key))) == "true" {
+	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.
+// Otherwise, it returns 0.
 func (c *Config) GetInt(key string) int {
-	iv, err := strconv.Atoi(c.Get(key))
+	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
 	}
@@ -274,7 +294,7 @@ func (c *Config) Unset(mount, key string) error {
 
 // Keys returns all keys in the given config.
 func (c *Config) Keys(mount string) []string {
-	if mount == "" {
+	if mount == "" || mount == "<root>" {
 		return c.root.Keys()
 	}
 
@@ -333,9 +353,9 @@ func (c *Config) migrateOptions(migrations map[string]string) {
 // will be true, otherwise it will be false.
 func DefaultPasswordLengthFromEnv(ctx context.Context) (int, bool) {
 	def := DefaultPasswordLength
-	cfg := FromContext(ctx)
+	cfg, mp := FromContext(ctx)
 
-	if l := cfg.GetInt("generate.length"); l > 0 {
+	if l := cfg.GetIntM(mp, "generate.length"); l > 0 {
 		def = l
 	}
 

diff --git a/internal/config/context.go b/internal/config/context.go
@@ -3,28 +3,42 @@ package config
 import (
 	"context"
 
+	"github.com/gopasspw/gopass/pkg/debug"
 	"github.com/gopasspw/gopass/pkg/gitconfig"
 )
 
 type contextKey int
 
 const (
 	ctxKeyConfig contextKey = iota
+	ctxKeyMountPoint
 )
 
 func (c *Config) WithConfig(ctx context.Context) context.Context {
 	return context.WithValue(ctx, ctxKeyConfig, c)
 }
 
-func FromContext(ctx context.Context) *Config {
+func WithMount(ctx context.Context, mp string) context.Context {
+	return context.WithValue(ctx, ctxKeyMountPoint, mp)
+}
+
+// FromContext returns a config from a context, as well as the current mount point (store name) if found.
+func FromContext(ctx context.Context) (*Config, string) {
+	mount := ""
+	if m, found := ctx.Value(ctxKeyMountPoint).(string); found && m != "" {
+		mount = m
+	}
+
 	if c, found := ctx.Value(ctxKeyConfig).(*Config); found && c != nil {
-		return c
+		return c, mount
 	}
 
-	c := &Config{
+	debug.Log("no config in context, loading anew")
+
+	cfg := &Config{
 		root: newGitconfig().LoadAll(""),
 	}
-	c.root.Preset = gitconfig.NewFromMap(defaults)
+	cfg.root.Preset = gitconfig.NewFromMap(defaults)
 
-	return c
+	return cfg, mount
 }
diff --git a/internal/config/docs_test.go b/internal/config/docs_test.go
@@ -76,7 +76,7 @@ func TestConfigOptsInDocs(t *testing.T) {
 func usedOpts(t *testing.T) map[string]bool {
 	t.Helper()
 
-	optRE := regexp.MustCompile(`(?:\.Get(?:|Int|Bool)\(\"([a-z]+\.[a-z-]+)\"\)|\.GetM\([^,]+, \"([a-z]+\.[a-z-]+)\"\)|config\.(?:Bool|Int|String)\((?:ctx|c\.Context), \"([a-z]+\.[a-z-]+)\"\)|hook\.Invoke(?:Root)?\(ctx, \"([a-z]+\.[a-z-]+)\")`)
+	optRE := regexp.MustCompile(`(?:\.Get(?:|Int|Bool|All|Global)\(\"([a-z]+\.[a-z-]+)\"\)|\.Get(?:|Int|Bool)M\([^,]+, \"([a-z]+\.[a-z-]+)\"\)|config\.(?:Bool|Int|String)\((?:ctx|c\.Context), \"([a-z]+\.[a-z-]+)\"\)|hook\.Invoke(?:Root)?\(ctx, \"([a-z]+\.[a-z-]+)\")`)
 	opts := make(map[string]bool, 42)
 
 	dir := filepath.Join("..", "..")