Keep every password, token and API key the vault's, never a setting (hq issue 342)

A setting outranked the vault: baserow, grafana, letta and unifi merged a layer's
password over the ${secret:...} in their runtime config, and the gitea,
home-assistant, icecast, influxdb, mailu and nextcloud clients read the settings
file before the vault's file. A secret in a setting cannot be rotated and is read
back by the settings verb.

The four files now keep their password protected, so the controller refuses a
setting of it. The six clients ignore a secret in the settings file and say so
once, by key and never with the value; icecast reads its admin password from the
vault's file. checks/secrets refuses a mergeable file whose secret a layer could
replace; searxng's nested server.secret_key is listed as known until the
controller protects a nested key.

checks/secrets/go.mod

@@ -0,0 +1,3 @@
+module git.novox.be/novox/mesh-catalog/checks/secrets
+
+go 1.22

checks/secrets/known

@@ -0,0 +1,4 @@
+# Secrets nested below a key a mergeable file takes for its other values, which the controller cannot
+# protect alone yet (it protects top-level keys): "<module> <resource> <dotted key>". This list only shrinks:
+# a line that no longer names a finding is refused (novox/hq issue 342).
+searxng settings server.secret_key

checks/secrets/main.go

@@ -0,0 +1,211 @@
+// Command secrets refuses, in every module's manifest, a mergeable file that a setting could fill with a
+// password, a token or an API key in place of the vault's (novox/hq issue 342).
+//
+// A password is not a setting: it comes from the key vault, and it can be rotated there. A file that says
+// `"merge": "json"` takes the keys of the mesh-wide and the node's layers over its own content, so a key the
+// content fills with `${secret:<name>}` is one a layer could replace, and the vault's value would lose to it
+// until somebody removed it by hand. The controller refuses a setting of a key the file keeps as `protected`,
+// naming the layer and the key, never the value. This check holds every such file to that: each top-level key
+// whose value holds `${secret:…}`, anywhere below it, is either listed in the file's `protected`, or left out
+// of the file's `takes-settings` (novox/hq ADR 0326), so that no layer reaches it.
+//
+// A secret nested below a key the file takes for its other values (`server.secret_key` below `server`) cannot
+// be protected alone today: the controller protects top-level keys. Such a file is named in the file `known`
+// beside this check, one line per key, until the controller refuses a nested key; a line there that no longer
+// names a finding is refused too, so the list only shrinks.
+//
+//	go run . <catalogue root>
+//
+// Exits 1 on a finding.
+package main
+
+import (
+	"bufio"
+	"encoding/json"
+	"fmt"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+)
+
+// Finding is one key of one mergeable file a layer could fill in place of the vault.
+type Finding struct {
+	Module, ID, Key string
+}
+
+func (f Finding) String() string { return f.Module + " " + f.ID + " " + f.Key }
+
+// Check reads every manifest under root/modules and answers each secret-bearing key a layer could replace.
+func Check(root string) ([]Finding, error) {
+	manifests, err := filepath.Glob(filepath.Join(root, "modules", "*", "module.json"))
+	if err != nil {
+		return nil, err
+	}
+	sort.Strings(manifests)
+	var out []Finding
+	for _, path := range manifests {
+		raw, err := os.ReadFile(path)
+		if err != nil {
+			return nil, err
+		}
+		var m struct {
+			Module    string           `json:"module"`
+			Resources []map[string]any `json:"resources"`
+		}
+		if err := json.Unmarshal(raw, &m); err != nil {
+			return nil, fmt.Errorf("%s: %w", path, err)
+		}
+		for _, r := range m.Resources {
+			if how, _ := r["merge"].(string); how == "" {
+				continue
+			}
+			id, _ := r["id"].(string)
+			content, _ := r["content"].(string)
+			var doc map[string]any
+			if strings.TrimSpace(content) == "" {
+				continue
+			}
+			if err := json.Unmarshal([]byte(content), &doc); err != nil {
+				return nil, fmt.Errorf("%s: %s merges as JSON and its content is not JSON: %w", m.Module, id, err)
+			}
+			protected := set(r["protected"])
+			takes, names := r["takes-settings"]
+			taken := set(takes)
+			for _, key := range sortedKeys(doc) {
+				for _, at := range secretsBelow(key, doc[key]) {
+					switch {
+					case protected[key]:
+					case names && !taken[key]:
+					default:
+						out = append(out, Finding{m.Module, id, at})
+					}
+				}
+			}
+		}
+	}
+	return out, nil
+}
+
+// secretsBelow answers the dotted path of every string at or below value that names a vault secret.
+func secretsBelow(at string, value any) []string {
+	switch v := value.(type) {
+	case string:
+		if strings.Contains(v, "${secret:") {
+			return []string{at}
+		}
+	case map[string]any:
+		var out []string
+		for _, k := range sortedKeys(v) {
+			out = append(out, secretsBelow(at+"."+k, v[k])...)
+		}
+		return out
+	case []any:
+		var out []string
+		for i, e := range v {
+			out = append(out, secretsBelow(fmt.Sprintf("%s[%d]", at, i), e)...)
+		}
+		return out
+	}
+	return nil
+}
+
+func set(v any) map[string]bool {
+	out := map[string]bool{}
+	list, _ := v.([]any)
+	for _, e := range list {
+		if s, ok := e.(string); ok {
+			out[s] = true
+		}
+	}
+	return out
+}
+
+func sortedKeys(m map[string]any) []string {
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys
+}
+
+// Known reads the lines of the known list: "<module> <resource> <key>", # comments and blank lines skipped.
+func Known(path string) (map[string]bool, error) {
+	f, err := os.Open(path)
+	if os.IsNotExist(err) {
+		return map[string]bool{}, nil
+	}
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	out := map[string]bool{}
+	s := bufio.NewScanner(f)
+	for s.Scan() {
+		line := strings.TrimSpace(s.Text())
+		if line == "" || strings.HasPrefix(line, "#") {
+			continue
+		}
+		out[strings.Join(strings.Fields(line), " ")] = true
+	}
+	return out, s.Err()
+}
+
+// Judge splits the findings into those refused and the known lines that no longer name one.
+func Judge(found []Finding, known map[string]bool) (refused []Finding, stale []string) {
+	seen := map[string]bool{}
+	for _, f := range found {
+		seen[f.String()] = true
+		if !known[f.String()] {
+			refused = append(refused, f)
+		}
+	}
+	for line := range known {
+		if !seen[line] {
+			stale = append(stale, line)
+		}
+	}
+	sort.Strings(stale)
+	return refused, stale
+}
+
+// remedy says how a finding is put right.
+func remedy(key string) string {
+	top, _, nested := strings.Cut(key, ".")
+	if !nested {
+		return fmt.Sprintf("list %q in the file's \"protected\", so the controller refuses a setting of it", top)
+	}
+	return fmt.Sprintf("the controller protects a top-level key only; list %q in the file's \"protected\" if "+
+		"nothing else below it is the operator's, or leave it out of the file's \"takes-settings\"", top)
+}
+
+func main() {
+	root := "."
+	if len(os.Args) > 1 {
+		root = os.Args[1]
+	}
+	found, err := Check(root)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+	known, err := Known("known")
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+	refused, stale := Judge(found, known)
+	for _, f := range refused {
+		fmt.Printf("%s: %s fills %s with a vault secret, and a setting could replace it (novox/hq issue 342): %s\n",
+			f.Module, f.ID, f.Key, remedy(f.Key))
+	}
+	for _, line := range stale {
+		fmt.Printf("checks/secrets/known names %q, which no longer holds a secret a setting could replace: "+
+			"remove the line\n", line)
+	}
+	if len(refused) > 0 || len(stale) > 0 {
+		os.Exit(1)
+	}
+	fmt.Printf("secrets: no setting can replace a vault secret in a mergeable file (%d known, nested)\n", len(known))
+}

checks/secrets/main_test.go

@@ -0,0 +1,78 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func manifest(t *testing.T, root, module, resources string) {
+	t.Helper()
+	dir := filepath.Join(root, "modules", module)
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	body := `{"module":"` + module + `","resources":[` + resources + `]}`
+	if err := os.WriteFile(filepath.Join(dir, "module.json"), []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestASecretASettingCouldReplaceIsRefused(t *testing.T) {
+	root := t.TempDir()
+	// Refused: the key holds the vault's secret and nothing keeps a layer off it.
+	manifest(t, root, "a", `{"id":"cfg","type":"file","merge":"json","content":"{\"password\": \"${secret:admin}\"}"}`)
+	// Refused: nested below a key the file takes.
+	manifest(t, root, "b", `{"id":"cfg","type":"file","merge":"json","takes-settings":["server"],`+
+		`"content":"{\"server\": {\"secret_key\": \"${secret:s}\", \"port\": 1}}"}`)
+	// Kept: protected.
+	manifest(t, root, "c", `{"id":"cfg","type":"file","merge":"json","protected":["password"],`+
+		`"content":"{\"password\": \"${secret:admin}\", \"user\": \"admin\"}"}`)
+	// Kept: the file names its keys, and not this one.
+	manifest(t, root, "d", `{"id":"cfg","type":"file","merge":"json","takes-settings":["ui"],`+
+		`"content":"{\"token\": \"${secret:t}\", \"ui\": {}}"}`)
+	// Kept: a file that does not merge takes no setting.
+	manifest(t, root, "e", `{"id":"cfg","type":"file","content":"{\"token\": \"${secret:t}\"}"}`)
+	// Refused: protecting another key does not protect this one; a list element counts too.
+	manifest(t, root, "f", `{"id":"cfg","type":"file","merge":"json","protected":["user"],`+
+		`"content":"{\"users\": [{\"password\": \"${secret:p}\"}]}"}`)
+	found, err := Check(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := []string{"a cfg password", "b cfg server.secret_key", "f cfg users[0].password"}
+	if len(found) != len(want) {
+		t.Fatalf("found %v, want %v", found, want)
+	}
+	for i, f := range found {
+		if f.String() != want[i] {
+			t.Fatalf("found %v, want %v", found, want)
+		}
+	}
+}
+
+func TestTheKnownListOnlyShrinks(t *testing.T) {
+	found := []Finding{{"b", "cfg", "server.secret_key"}, {"a", "cfg", "password"}}
+	refused, stale := Judge(found, map[string]bool{"b cfg server.secret_key": true, "gone cfg token": true})
+	if len(refused) != 1 || refused[0].String() != "a cfg password" {
+		t.Fatalf("refused %v", refused)
+	}
+	if len(stale) != 1 || stale[0] != "gone cfg token" {
+		t.Fatalf("stale %v", stale)
+	}
+}
+
+func TestTheCatalogueHoldsNoSecretASettingCouldReplace(t *testing.T) {
+	found, err := Check("../..")
+	if err != nil {
+		t.Fatal(err)
+	}
+	known, err := Known("known")
+	if err != nil {
+		t.Fatal(err)
+	}
+	refused, stale := Judge(found, known)
+	if len(refused) > 0 || len(stale) > 0 {
+		t.Fatalf("refused %v, stale %v", refused, stale)
+	}
+}