Merge pull request 'settings: no layer replaces a value the vault fills, at any depth (issue 610)' (#308) from fix/610-no-layer-replaces-a-vault-secret into main

internal/catalogue/settings.go

@@ -192,6 +192,16 @@ func settle(base map[string]any, layers []Layer, protected map[string]bool, what
 					"%s sets %q on %v, and that module keeps %q for itself — it is not settable",
 					layer.From, key, what, key)
 			}
+			// **No layer replaces a secret the vault fills, at any depth** (novox/hq issue 610): a
+			// mergeable file's `${secret:<name>}` is the vault's value, and a layer that reached it —
+			// `server.secret_key` under a `server` the file takes — would put a value the operator or an
+			// agent chose where the vault's belongs. Judged against the file's own content, so it holds
+			// whatever a module's `protected` list says.
+			if at := secretReplacedBy(base[key], value, key); at != "" {
+				return nil, fmt.Errorf(
+					"%s sets %q on %v, which the vault fills (${secret:…}); a secret comes from the vault, "+
+						"never from a setting (novox/hq issues 342, 610)", layer.From, at, what)
+			}
 			merged[key] = mergeValue(merged[key], value)
 		}
 	}
@@ -576,3 +586,54 @@ func JudgeSettings(m Manifest, layers []Layer, adopted bool) error {
 	}
 	return nil
 }
+
+// secretReplacedBy is the path in a mergeable file's own content that a layer's value would replace while the
+// vault fills it — a string holding `${secret:` — or "" when the value leaves every such string as it is
+// (novox/hq issue 610). Maps merge key by key (mergeValue), so only the keys the value names are judged; any
+// other value replaces the whole of what it lands on.
+func secretReplacedBy(base, value any, at string) string {
+	if baseMap, isMap := base.(map[string]any); isMap {
+		if valueMap, also := value.(map[string]any); also {
+			keys := make([]string, 0, len(valueMap))
+			for k := range valueMap {
+				keys = append(keys, k)
+			}
+			sort.Strings(keys)
+			for _, k := range keys {
+				if found := secretReplacedBy(baseMap[k], valueMap[k], at+"."+k); found != "" {
+					return found
+				}
+			}
+			return ""
+		}
+	}
+	return secretIn(base, at)
+}
+
+// secretIn is the first path under v holding a vault placeholder, or "".
+func secretIn(v any, at string) string {
+	switch t := v.(type) {
+	case string:
+		if strings.Contains(t, "${secret:") {
+			return at
+		}
+	case map[string]any:
+		keys := make([]string, 0, len(t))
+		for k := range t {
+			keys = append(keys, k)
+		}
+		sort.Strings(keys)
+		for _, k := range keys {
+			if found := secretIn(t[k], at+"."+k); found != "" {
+				return found
+			}
+		}
+	case []any:
+		for i, item := range t {
+			if found := secretIn(item, fmt.Sprintf("%s[%d]", at, i)); found != "" {
+				return found
+			}
+		}
+	}
+	return ""
+}

internal/catalogue/settings_test.go

@@ -234,3 +234,29 @@ func TestAServedValueMayBeTheOperators(t *testing.T) {
 		t.Errorf("a setting a served fact asks for is not stray: %v", unused)
 	}
 }
+
+// No layer replaces a value the vault fills, at any depth (novox/hq issue 610): searxng's server.secret_key
+// sits under `server`, a key its file takes, and a layer reaching it was let through by a top-level-only check.
+func TestNoLayerReplacesAVaultSecretAtAnyDepth(t *testing.T) {
+	file := map[string]any{"id": "settings", "type": "file", "merge": MergeJSON,
+		"content": `{"server": {"secret_key": "${secret:secret}", "port": 8080}, "ui": {"theme": "simple"}, "token": "${secret:token}"}`}
+	layer := func(values map[string]any) []Layer { return []Layer{{From: "anchor", Values: values}} }
+
+	merged, err := ApplySettings(file, layer(map[string]any{"server": map[string]any{"port": float64(9000)}, "ui": map[string]any{"theme": "dark"}}))
+	if err != nil {
+		t.Fatalf("a key beside the secret was refused: %v", err)
+	}
+	if c := merged["content"].(string); !strings.Contains(c, `"secret_key": "${secret:secret}"`) || !strings.Contains(c, `"port": 9000`) {
+		t.Fatalf("the secret or the setting did not land as meant:\n%s", c)
+	}
+	for name, values := range map[string]map[string]any{
+		"the nested secret":        {"server": map[string]any{"secret_key": "chosen"}},
+		"the whole map holding it": {"server": "elsewhere"},
+		"a top-level secret":       {"token": "chosen"},
+		"the secret, emptied":      {"server": map[string]any{"secret_key": ""}},
+	} {
+		if _, err := ApplySettings(file, layer(values)); err == nil || !strings.Contains(err.Error(), "vault") {
+			t.Errorf("%s: %v, want a refusal naming the vault", name, err)
+		}
+	}
+}