Refuse config keys that no setting has (closes #419)

config.Parse walks the file as written and fails on the first key that
matches no setting, naming its dotted path and line, so a typo in
exclude or prune_grace_period no longer falls back silently. ${...}
references and YAML merge keys still load. config set refuses such a key
before writing. The key-to-field lookup moves from internal/cli to
config.SettingType so both use it.

Model: opus-5-5

README.md

@@ -311,14 +311,15 @@ existing file. Created with mode `0600` since it will contain credentials.
 in the file are preserved. Missing keys along the path are created: as a list
 when the next path element is a number (so
 `vaultik config set age_recipients.0 age1...` works on a file without
-`age_recipients`), otherwise as a map. If the config loads, a change after which
+`age_recipients`), otherwise as a map. A key that no setting has, such as a
-it would not, such as `vaultik config set compression_level 99`, is refused and
+misspelled one, is refused. If the config loads, a change after which it would
-the file is left unchanged. A config that does not load, such as the one
+not, such as `vaultik config set compression_level 99`, is refused and the file
-`config init` writes before `storage_url` is set or one with a `${ENV:...}`
+is left unchanged. A config that does not load, such as the one `config init`
-reference to a variable not set in this shell, is checked one setting at a time:
+writes before `storage_url` is set or one with a `${ENV:...}` reference to a
-only a change after which the changed setting cannot be read, such as one giving
+variable not set in this shell, is checked one setting at a time: a change after
-it the wrong type (a single value where a list belongs or a word where a number
+which the changed setting cannot be read, such as one giving it the wrong type
-belongs), is refused.
+(a single value where a list belongs or a word where a number belongs), is
+refused.
 
 **`snapshot create`**: Perform incremental backup of configured snapshots.
 

internal/cli/config.go

@@ -46,6 +46,7 @@ var (
 	errNeedNumericIndex = errors.New("key is a list; use a numeric index")
 	errIndexOutOfRange  = errors.New("index out of range")
 	errNotMapOrList     = errors.New("key is not a map or list")
+	errNoSuchSetting    = errors.New("no setting has this key")
 	errWouldNotLoad     = errors.New(
 		"vaultik would not load the config after this change, " +
 			"so the file was not changed")
@@ -439,15 +440,15 @@ func newConfigSetCommand() *cobra.Command {
 		Long: `Sets a scalar config value addressed by dotted YAML path and writes
 the file back, preserving comments and formatting. Missing keys along
 the path are created: as a list when the next path element is a
-number, otherwise as a map.
+number, otherwise as a map. A key that no setting has is refused.
 
 If the config loads, a change after which it would not, such as
 compression_level 99, is refused and the file is left unchanged. A
 config that does not load, such as the one config init writes before
 storage_url is set or one with a ${ENV:...} reference to a variable
-not set here, is checked one setting at a time: only a change after
+not set here, is checked one setting at a time: a change after which
-which the changed setting cannot be read, such as one giving it the
+the changed setting cannot be read, such as one giving it the wrong
-wrong type, is refused.
+type, is refused.
 
 Examples:
   vaultik config set storage_url "file:///mnt/backups"
@@ -484,6 +485,11 @@ func writeConfigSet(out *ui.Writer, path, key, value string) error {
 
 	keys := strings.Split(key, ".")
 
+	_, ok := config.SettingType(keys)
+	if !ok {
+		return fmt.Errorf("%w: %s", errNoSuchSetting, key)
+	}
+
 	err = yamlPathSet(root, keys, value)
 	if err != nil {
 		return err
@@ -739,41 +745,11 @@ func yamlPathSet(root *yaml.Node, keys []string, value string) error {
 }
 
 // configKeyIsString reports whether the dotted key names a string in
-// config.Config, following the fields' yaml tags, as s3.access_key_id and
+// config.Config, as s3.access_key_id and snapshots.home.exclude.0 do.
-// snapshots.home.exclude.0 do.
 func configKeyIsString(keys []string) bool {
-	typ := reflect.TypeFor[config.Config]()
+	typ, ok := config.SettingType(keys)
 
-	for _, key := range keys {
+	return ok && typ.Kind() == reflect.String
-		switch {
-		case typ.Kind() == reflect.Map || typ.Kind() == reflect.Slice:
-			// The key is a snapshot name or a list index.
-			typ = typ.Elem()
-		case typ.Kind() == reflect.Struct:
-			field, ok := yamlField(typ, key)
-			if !ok {
-				return false
-			}
-
-			typ = field.Type
-		default:
-			return false
-		}
-	}
-
-	return typ.Kind() == reflect.String
-}
-
-// yamlField returns the field of struct type typ whose yaml tag names key.
-func yamlField(typ reflect.Type, key string) (reflect.StructField, bool) {
-	for field := range typ.Fields() {
-		name, _, _ := strings.Cut(field.Tag.Get("yaml"), ",")
-		if name == key {
-			return field, true
-		}
-	}
-
-	return reflect.StructField{}, false
 }
 
 // yamlSetInMapping resolves (creating if needed) the value node for the

internal/cli/config_test.go

@@ -237,6 +237,14 @@ func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
 		value  string
 		reason string
 	}{
+		// Keys that no setting has, on a config that loads and on one
+		// that does not.
+		{configWithoutRecipients, "nosuchkey", "1",
+			"no setting has this key: nosuchkey"},
+		{configWithoutRecipients, "snapshots.home.exlude", "/tmp",
+			"no setting has this key: snapshots.home.exlude"},
+		{defaultConfigText(macOS), "compression_lvl", "9",
+			"no setting has this key: compression_lvl"},
 		// Index 1 of the empty list the missing key gives.
 		{configWithoutRecipients, "age_recipients.1", testRecipient,
 			"index out of range"},

internal/config/config.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strings"
 	"time"
@@ -286,9 +287,24 @@ func Load(path string) (*Config, error) {
 }
 
 // Parse reads YAML config data as Load does, without validating it. It
-// applies default values for optional fields, fills in ${...} references
+// refuses a key that no setting has, applies default values for optional
-// using smartconfig, and expands ~ in paths.
+// fields, fills in ${...} references using smartconfig, and expands ~ in
+// paths.
 func Parse(data []byte) (*Config, error) {
+	// smartconfig drops line numbers, so keys are checked in the data as
+	// written.
+	var root yaml.Node
+
+	err := yaml.Unmarshal(data, &root)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse config: %w", err)
+	}
+
+	err = checkKeys(&root, reflect.TypeFor[Config](), "")
+	if err != nil {
+		return nil, err
+	}
+
 	// Load config using smartconfig for interpolation
 	sc, err := smartconfig.NewFromReader(bytes.NewReader(data))
 	if err != nil {

internal/config/keys.go

@@ -0,0 +1,149 @@
+package config
+
+import (
+	"errors"
+	"fmt"
+	"reflect"
+	"strconv"
+	"strings"
+
+	"gopkg.in/yaml.v3"
+)
+
+// yamlMergeTag is the tag YAML gives a merge key (<<), which adds the keys
+// of the map it names to the map it sits in.
+const yamlMergeTag = "!!merge"
+
+var errUnknownKey = errors.New("unknown config key")
+
+// SettingType returns the type of the setting that the dotted key names in
+// Config, following the fields' yaml tags, as for s3.bucket or
+// snapshots.home.exclude.0. A snapshot name or a list index may be any
+// word. It returns false when no setting has that key.
+func SettingType(keys []string) (reflect.Type, bool) {
+	typ := reflect.TypeFor[Config]()
+
+	for _, key := range keys {
+		switch {
+		case typ.Kind() == reflect.Map || typ.Kind() == reflect.Slice:
+			typ = typ.Elem()
+		case typ.Kind() == reflect.Struct:
+			field, ok := yamlField(typ, key)
+			if !ok {
+				return nil, false
+			}
+
+			typ = field.Type
+		default:
+			return nil, false
+		}
+	}
+
+	return typ, true
+}
+
+// yamlField returns the field of struct type typ whose yaml tag names key.
+func yamlField(typ reflect.Type, key string) (reflect.StructField, bool) {
+	for field := range typ.Fields() {
+		name, _, _ := strings.Cut(field.Tag.Get("yaml"), ",")
+		if name == key && name != "-" {
+			return field, true
+		}
+	}
+
+	return reflect.StructField{}, false
+}
+
+// checkKeys returns an error naming the first key in node, a parsed config
+// file or part of one at the dotted path, that no setting in typ has. The
+// error gives the key's line in the file. A value of the wrong type is left
+// for the YAML decoder to report.
+func checkKeys(node *yaml.Node, typ reflect.Type, path string) error {
+	switch node.Kind {
+	case yaml.DocumentNode:
+		for _, child := range node.Content {
+			err := checkKeys(child, typ, path)
+			if err != nil {
+				return err
+			}
+		}
+	case yaml.AliasNode:
+		return checkKeys(node.Alias, typ, path)
+	case yaml.SequenceNode:
+		if typ.Kind() != reflect.Slice {
+			return nil
+		}
+
+		for i, child := range node.Content {
+			err := checkKeys(child, typ.Elem(), joinKey(path, strconv.Itoa(i)))
+			if err != nil {
+				return err
+			}
+		}
+	case yaml.MappingNode:
+		return checkMappingKeys(node, typ, path)
+	case yaml.ScalarNode:
+		// A scalar has no keys.
+	}
+
+	return nil
+}
+
+// checkMappingKeys is checkKeys for a map in the file.
+func checkMappingKeys(node *yaml.Node, typ reflect.Type, path string) error {
+	for i := 0; i+1 < len(node.Content); i += 2 {
+		key, value := node.Content[i], node.Content[i+1]
+
+		if key.Tag == yamlMergeTag {
+			// The value is one map or a list of maps.
+			merged := []*yaml.Node{value}
+			if value.Kind == yaml.SequenceNode {
+				merged = value.Content
+			}
+
+			for _, m := range merged {
+				err := checkKeys(m, typ, path)
+				if err != nil {
+					return err
+				}
+			}
+
+			continue
+		}
+
+		keyPath := joinKey(path, key.Value)
+
+		var valueType reflect.Type
+
+		switch {
+		case typ.Kind() == reflect.Map:
+			valueType = typ.Elem()
+		case typ.Kind() == reflect.Struct:
+			field, ok := yamlField(typ, key.Value)
+			if !ok {
+				return fmt.Errorf("%w %q on line %d",
+					errUnknownKey, keyPath, key.Line)
+			}
+
+			valueType = field.Type
+		default:
+			return nil
+		}
+
+		err := checkKeys(value, valueType, keyPath)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// joinKey appends key to the dotted path.
+func joinKey(path, key string) string {
+	if path == "" {
+		return key
+	}
+
+	return path + "." + key
+}

internal/config/keys_test.go

@@ -0,0 +1,86 @@
+package config //nolint:testpackage // checks the unexported errUnknownKey
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// keysTestConfig loads; each case below adds one line to it.
+const keysTestConfig = "storage_url: file:///tmp/vaultik-test-storage\n" +
+	"s3:\n" +
+	"  region: us-east-1\n" +
+	"snapshots:\n" +
+	"  test:\n" +
+	"    paths: [/tmp/vaultik-test-source]\n"
+
+// TestParseRefusesUnknownKey checks that a key no setting has, at any
+// depth, fails the parse with an error naming the key and its line.
+func TestParseRefusesUnknownKey(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		yaml string
+		want string
+	}{
+		{"top level", keysTestConfig + "compression_lvl: 9\n",
+			`unknown config key "compression_lvl" on line 7`},
+		{"in a snapshot", keysTestConfig + "    exlude: [/tmp/cache]\n",
+			`unknown config key "snapshots.test.exlude" on line 7`},
+		{"in s3", strings.Replace(keysTestConfig, "  region:",
+			"  bukcet: b\n  region:", 1),
+			`unknown config key "s3.bukcet" on line 3`},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			_, err := Parse([]byte(tt.yaml))
+			if !errors.Is(err, errUnknownKey) {
+				t.Fatalf("Parse() error = %v, want errUnknownKey", err)
+			}
+
+			if err.Error() != tt.want {
+				t.Errorf("Parse() error = %q, want %q", err.Error(), tt.want)
+			}
+		})
+	}
+}
+
+// TestParseAcceptsKnownKeys checks that a config with a ${...} reference
+// and a YAML merge key (<<) still parses.
+func TestParseAcceptsKnownKeys(t *testing.T) {
+	t.Parallel()
+
+	levelPath := filepath.Join(t.TempDir(), "level")
+
+	err := os.WriteFile(levelPath, []byte("9"), 0o600)
+	if err != nil {
+		t.Fatalf("write level: %v", err)
+	}
+
+	data := strings.Replace(keysTestConfig, "  test:", "  test: &test", 1) +
+		"  other:\n" +
+		"    <<: *test\n" +
+		"    exclude: [/tmp/vaultik-test-source/cache]\n" +
+		"compression_level: ${FILE:" + levelPath + "}\n"
+
+	cfg, err := Parse([]byte(data))
+	if err != nil {
+		t.Fatalf("Parse() unexpected error: %v", err)
+	}
+
+	if cfg.CompressionLevel != 9 {
+		t.Errorf("compression_level = %d, want 9", cfg.CompressionLevel)
+	}
+
+	other := cfg.Snapshots["other"]
+	if len(other.Paths) != 1 || len(other.Exclude) != 1 {
+		t.Errorf("snapshot other = %+v, want the paths of test and one exclude",
+			other)
+	}
+}