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. Keys that a
${...} reference brings in are checked once smartconfig has filled it
in, and named without a line. YAML merge keys and a top-level env
section, which smartconfig reads, still load; an anchor that contains
itself is an error. config set refuses such a key before writing, with
the same error. The key-to-field lookup moves from internal/cli to
config.SettingType so both use it.

Model: opus-5-5

internal/cli/config.go

@@ -46,7 +46,6 @@ 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")
@@ -484,9 +483,9 @@ 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 = config.SettingType(keys)
+	if err != nil {
+		return err
 	}
 
 	err = yamlPathSet(root, keys, value)
@@ -746,9 +745,9 @@ func yamlPathSet(root *yaml.Node, keys []string, value string) error {
 // configKeyIsString reports whether the dotted key names a string in
 // config.Config, as s3.access_key_id and snapshots.home.exclude.0 do.
 func configKeyIsString(keys []string) bool {
-	typ, ok := config.SettingType(keys)
+	typ, err := config.SettingType(keys)
 
-	return ok && typ.Kind() == reflect.String
+	return err == nil && typ.Kind() == reflect.String
 }
 
 // yamlSetInMapping resolves (creating if needed) the value node for the

internal/cli/config_test.go

@@ -240,11 +240,11 @@ func TestConfigSetKeepsFileThatWouldNotLoad(t *testing.T) {
 		// 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"},
+			`unknown config key "nosuchkey"`},
 		{configWithoutRecipients, "snapshots.home.exlude", "/tmp",
-			"no setting has this key: snapshots.home.exlude"},
+			`unknown config key "snapshots.home.exlude"`},
 		{defaultConfigText(macOS), "compression_lvl", "9",
-			"no setting has this key: compression_lvl"},
+			`unknown config 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

@@ -300,7 +300,7 @@ func Parse(data []byte) (*Config, error) {
 		return nil, fmt.Errorf("failed to parse config: %w", err)
 	}
 
-	err = checkKeys(&root, reflect.TypeFor[Config](), "")
+	err = checkKeys(&root, reflect.TypeFor[Config](), "", map[*yaml.Node]bool{})
 	if err != nil {
 		return nil, err
 	}

internal/config/keys.go

@@ -19,13 +19,16 @@ const yamlMergeTag = "!!merge"
 // so its keys are not checked.
 const envSection = "env"
 
-var errUnknownKey = errors.New("unknown config key")
+var (
+	errUnknownKey           = errors.New("unknown config key")
+	errAnchorContainsItself = errors.New("anchor contains itself")
+)
 
 // 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) {
+// word. It returns an unknown key error when no setting has that key.
+func SettingType(keys []string) (reflect.Type, error) {
 	typ := reflect.TypeFor[Config]()
 
 	for _, key := range keys {
@@ -35,16 +38,16 @@ func SettingType(keys []string) (reflect.Type, bool) {
 		case typ.Kind() == reflect.Struct:
 			field, ok := yamlField(typ, key)
 			if !ok {
-				return nil, false
+				return nil, fmt.Errorf("%w %q", errUnknownKey, strings.Join(keys, "."))
 			}
 
 			typ = field.Type
 		default:
-			return nil, false
+			return nil, fmt.Errorf("%w %q", errUnknownKey, strings.Join(keys, "."))
 		}
 	}
 
-	return typ, true
+	return typ, nil
 }
 
 // yamlField returns the field of struct type typ whose yaml tag names key.
@@ -62,31 +65,43 @@ func yamlField(typ reflect.Type, key string) (reflect.StructField, bool) {
 // checkKeys returns an error naming the first key in node, a parsed config
 // or part of one at the dotted path, that no setting in typ has. The error
 // gives the key's line in the file, when it has one. 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 {
+// type is left for the YAML decoder to report. following holds the aliases
+// the walk is inside. Meeting one of them again means its anchor contains
+// itself, which is an error.
+func checkKeys(node *yaml.Node, typ reflect.Type, path string,
+	following map[*yaml.Node]bool,
+) error {
 	switch node.Kind {
 	case yaml.DocumentNode:
 		for _, child := range node.Content {
-			err := checkKeys(child, typ, path)
+			err := checkKeys(child, typ, path, following)
 			if err != nil {
 				return err
 			}
 		}
 	case yaml.AliasNode:
-		return checkKeys(node.Alias, typ, path)
+		if following[node] {
+			return fmt.Errorf("%w: %q on line %d",
+				errAnchorContainsItself, node.Value, node.Line)
+		}
+
+		following[node] = true
+		defer delete(following, node)
+
+		return checkKeys(node.Alias, typ, path, following)
 	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)))
+			err := checkKeys(child, typ.Elem(), joinKey(path, strconv.Itoa(i)), following)
 			if err != nil {
 				return err
 			}
 		}
 	case yaml.MappingNode:
-		return checkMappingKeys(node, typ, path)
+		return checkMappingKeys(node, typ, path, following)
 	case yaml.ScalarNode:
 		// A scalar has no keys.
 	}
@@ -105,11 +120,13 @@ func checkFilledKeys(data map[string]any) error {
 		return fmt.Errorf("failed to marshal config data: %w", err)
 	}
 
-	return checkKeys(&filled, reflect.TypeFor[Config](), "")
+	return checkKeys(&filled, reflect.TypeFor[Config](), "", map[*yaml.Node]bool{})
 }
 
 // checkMappingKeys is checkKeys for a map.
-func checkMappingKeys(node *yaml.Node, typ reflect.Type, path string) error {
+func checkMappingKeys(node *yaml.Node, typ reflect.Type, path string,
+	following map[*yaml.Node]bool,
+) error {
 	for i := 0; i+1 < len(node.Content); i += 2 {
 		key, value := node.Content[i], node.Content[i+1]
 
@@ -121,7 +138,7 @@ func checkMappingKeys(node *yaml.Node, typ reflect.Type, path string) error {
 			}
 
 			for _, m := range merged {
-				err := checkKeys(m, typ, path)
+				err := checkKeys(m, typ, path, following)
 				if err != nil {
 					return err
 				}
@@ -157,7 +174,7 @@ func checkMappingKeys(node *yaml.Node, typ reflect.Type, path string) error {
 			return nil
 		}
 
-		err := checkKeys(value, valueType, keyPath)
+		err := checkKeys(value, valueType, keyPath, following)
 		if err != nil {
 			return err
 		}

internal/config/keys_test.go

@@ -65,6 +65,25 @@ func TestParseRefusesUnknownKey(t *testing.T) {
 	}
 }
 
+// TestParseRefusesAnchorThatContainsItself checks that a YAML merge key (<<)
+// naming the anchor of the map it sits in fails the parse with an error.
+func TestParseRefusesAnchorThatContainsItself(t *testing.T) {
+	t.Parallel()
+
+	data := strings.Replace(keysTestConfig, "  test:", "  test: &test", 1) +
+		"    <<: *test\n"
+
+	_, err := Parse([]byte(data))
+	if !errors.Is(err, errAnchorContainsItself) {
+		t.Fatalf("Parse() error = %v, want errAnchorContainsItself", err)
+	}
+
+	want := `anchor contains itself: "test" on line 7`
+	if err.Error() != want {
+		t.Errorf("Parse() error = %q, want %q", err.Error(), want)
+	}
+}
+
 // TestParseAcceptsKnownKeys checks that a config with a ${...} reference
 // and a YAML merge key (<<) still parses.
 func TestParseAcceptsKnownKeys(t *testing.T) {