Relax config checks when loading (#1797)

To improve compatibility of gopass integrations with gopass itself and
to simplify the upgrade path this PR relaxes config parsing a bit. It
introduces a final attempt where the current config struct is attempted
without the overflow check. Thus it will allow loading configs with
either missing or extra keys. This should allow most forward
compatibility cases. However this might lead us to accepts truly invalid
configs.

RELEASE_NOTES=n/a

internal/config/io.go

@@ -13,10 +13,20 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
+// LoadWithFallbackRelaxed will try to load the config from one of the default
+// locations but also accept a more recent config.
+func LoadWithFallbackRelaxed() *Config {
+	return loadWithFallback(true)
+}
+
 // LoadWithFallback will try to load the config from one of the default locations
 func LoadWithFallback() *Config {
+	return loadWithFallback(false)
+}
+
+func loadWithFallback(relaxed bool) *Config {
 	for _, l := range configLocations() {
-		if cfg := loadConfig(l); cfg != nil {
+		if cfg := loadConfig(l, relaxed); cfg != nil {
 			return cfg
 		}
 	}
@@ -25,15 +35,15 @@ func LoadWithFallback() *Config {
 
 // Load will load the config from the default location or return a default config
 func Load() *Config {
-	if cfg := loadConfig(configLocation()); cfg != nil {
+	if cfg := loadConfig(configLocation(), false); cfg != nil {
 		return cfg
 	}
 	return loadDefault()
 }
 
-func loadConfig(l string) *Config {
+func loadConfig(l string, relaxed bool) *Config {
 	debug.Log("Trying to load config from %s", l)
-	cfg, err := load(l)
+	cfg, err := load(l, relaxed)
 	if err == ErrConfigNotFound {
 		return nil
 	}
@@ -51,7 +61,7 @@ func loadDefault() *Config {
 	return cfg
 }
 
-func load(cf string) (*Config, error) {
+func load(cf string, relaxed bool) (*Config, error) {
 	// deliberately using os.Stat here, a symlinked
 	// config is OK
 	if _, err := os.Stat(cf); err != nil {
@@ -63,7 +73,7 @@ func load(cf string) (*Config, error) {
 		return nil, ErrConfigNotFound
 	}
 
-	cfg, err := decode(buf)
+	cfg, err := decode(buf, relaxed)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "Error reading config from %s: %s\n", cf, err)
 		return nil, ErrConfigNotParsed
@@ -93,12 +103,14 @@ type configer interface {
 	CheckOverflow() error
 }
 
-func decode(buf []byte) (*Config, error) {
+func decode(buf []byte, relaxed bool) (*Config, error) {
+	mostRecent := &Config{
+		ExportKeys: true,
+		Parsing:    true,
+	}
 	cfgs := []configer{
-		&Config{
-			ExportKeys: true,
-			Parsing:    true,
-		},
+		// most recent config must come first
+		mostRecent,
 		&Pre1102{},
 		&Pre193{
 			Root: &Pre193StoreConfig{},
@@ -109,6 +121,11 @@ func decode(buf []byte) (*Config, error) {
 		&Pre140{},
 		&Pre130{},
 	}
+	if relaxed {
+		// most recent config must come last as well, will be tried w/o
+		// overflow checks
+		cfgs = append(cfgs, mostRecent)
+	}
 	var warn string
 	for i, cfg := range cfgs {
 		debug.Log("Trying to unmarshal config into %T", cfg)
@@ -121,7 +138,18 @@ func decode(buf []byte) (*Config, error) {
 			if i == 0 {
 				warn = fmt.Sprintf("Failed to load config %T. Do you need to remove deprecated fields? %s\n", cfg, err)
 			}
-			continue
+			// usually we are strict about extra fields, i.e. any field left
+			// unparsed means this config failed and we try the next one.
+			if i < len(cfgs)-1 {
+				continue
+			}
+			// in relaxed mode we append an extra copy of the most recent
+			// config to the end of the slice and might just ignore these
+			// extra fields.
+			if !relaxed {
+				continue
+			}
+			debug.Log("Ignoring extra config fields for fallback config (only)")
 		}
 		debug.Log("Loaded config: %T: %+v", cfg, cfg)
 		conf := cfg.Config()

internal/config/io_test.go

@@ -51,6 +51,37 @@ mounts:
 					"work":    "/home/johndoe/.password-store-work",
 				},
 			},
+		}, {
+			name: "N+1",
+			cfg: `autoclip: true
+autoimport: false
+cliptimeout: 45
+exportkeys: true
+nopager: false
+foo: bar
+notifications: true
+path: /home/johndoe/.password-store
+safecontent: false
+mounts:
+  foo/sub: /home/johndoe/.password-store-foo-sub
+  work: /home/johndoe/.password-store-work`,
+			want: &Config{
+				AutoClip:      true,
+				AutoImport:    false,
+				ClipTimeout:   45,
+				ExportKeys:    true,
+				NoColor:       false,
+				NoPager:       false,
+				Notifications: true,
+				Parsing:       true,
+				Path:          "/home/johndoe/.password-store",
+				SafeContent:   false,
+				Mounts: map[string]string{
+					"foo/sub": "/home/johndoe/.password-store-foo-sub",
+					"work":    "/home/johndoe/.password-store-work",
+				},
+				XXX: map[string]interface{}{"foo": string("bar")},
+			},
 		}, {
 			name: "1.8.2",
 			cfg: `root:
@@ -310,10 +341,10 @@ version: "1.0.0"`,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			got, err := decode([]byte(tc.cfg))
+			got, err := decode([]byte(tc.cfg), true)
 			require.NoError(t, err)
 			if diff := cmp.Diff(tc.want, got); diff != "" {
-				t.Errorf("decode(%v) mismatch (-want +got):\n%s", tc.cfg, diff)
+				t.Errorf("decode(%s) mismatch for:\n%s\n(-want +got):\n%s", tc.name, tc.cfg, diff)
 			}
 		})
 	}
@@ -369,15 +400,15 @@ func TestLoadError(t *testing.T) {
 	_ = os.Remove(gcfg)
 
 	capture(t, func() error {
-		_, err := load(gcfg)
+		_, err := load(gcfg, false)
 		if err == nil {
 			return fmt.Errorf("should fail")
 		}
 		return nil
 	})
 
 	_ = os.Remove(gcfg)
-	cfg, err := load(gcfg)
+	cfg, err := load(gcfg, false)
 	assert.Error(t, err)
 
 	td, err := ioutil.TempDir("", "gopass-")
@@ -398,7 +429,7 @@ func TestDecodeError(t *testing.T) {
 	require.NoError(t, ioutil.WriteFile(gcfg, []byte(testConfig+"\nfoobar: zab\n"), 0600))
 
 	capture(t, func() error {
-		_, err := load(gcfg)
+		_, err := load(gcfg, false)
 		if err == nil {
 			return fmt.Errorf("should fail")
 		}

pkg/gopass/api/api.go

@@ -25,7 +25,7 @@ var _ gopass.Store = &Gopass{}
 // New creates a new secret store
 // WARNING: This will need to change to accommodate for runtime configuration.
 func New(ctx context.Context) (*Gopass, error) {
-	cfg := config.LoadWithFallback()
+	cfg := config.LoadWithFallbackRelaxed()
 	store := root.New(cfg)
 	initialized, err := store.IsInitialized(ctx)
 	if err != nil {