diff --git a/internal/catalogue/settings.go b/internal/catalogue/settings.go index 71357453..e20b61a2 100644 --- a/internal/catalogue/settings.go +++ b/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:}` 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 "" +} diff --git a/internal/catalogue/settings_test.go b/internal/catalogue/settings_test.go index b2cc392b..6fe6d2a0 100644 --- a/internal/catalogue/settings_test.go +++ b/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) + } + } +}