Make grafana prefer the vault's password, and let the secrets list only shrink (hq issue 342)

grafana's client logged in with a token setting ahead of the vault's admin password,
and its runtime config let a layer set token. The token setting is now ignored and
said, token is protected, and the vault's password comes first.

checks/secrets/known could gain a line hiding a newly unprotected key. merge-check.sh
now judges it against the trunk's copy at the merge base, and refuses an added line.

checks/secrets/known

@@ -1,4 +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).
+# a line that no longer names a finding is refused, and so is one the trunk's copy does not hold (hq issue 342).
 searxng settings server.secret_key

checks/secrets/main.go

@@ -11,10 +11,11 @@
 //
 // 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.
+// beside this check, one line per key, until the controller refuses a nested key. The list only shrinks: a
+// line there that no longer names a finding is refused, and so is a line the trunk's copy (the merge base's,
+// given with -base) does not hold. While the trunk holds no copy, the lines in seed are the trunk's.
 //
-//	go run . <catalogue root>
+//	go run . [-base <the trunk's known>] <catalogue root>
 //
 // Exits 1 on a finding.
 package main
@@ -22,6 +23,7 @@ package main
 import (
 	"bufio"
 	"encoding/json"
+	"flag"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -152,6 +154,23 @@ func Known(path string) (map[string]bool, error) {
 	return out, s.Err()
 }
 
+// seed is the known list as it was first written, standing for the trunk's copy until the trunk holds one.
+var seed = map[string]bool{
+	"searxng settings server.secret_key": true,
+}
+
+// Added answers the lines of known that base does not hold: a line may leave the list, never join it.
+func Added(known, base map[string]bool) []string {
+	var out []string
+	for line := range known {
+		if !base[line] {
+			out = append(out, line)
+		}
+	}
+	sort.Strings(out)
+	return out
+}
+
 // 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{}
@@ -181,9 +200,11 @@ func remedy(key string) string {
 }
 
 func main() {
+	basePath := flag.String("base", "", "the trunk's copy of known (the merge base's); seed when absent")
+	flag.Parse()
 	root := "."
-	if len(os.Args) > 1 {
-		root = os.Args[1]
+	if flag.NArg() > 0 {
+		root = flag.Arg(0)
 	}
 	found, err := Check(root)
 	if err != nil {
@@ -195,6 +216,18 @@ func main() {
 		fmt.Fprintln(os.Stderr, err)
 		os.Exit(1)
 	}
+	base := seed
+	if *basePath != "" {
+		if base, err = Known(*basePath); err != nil {
+			fmt.Fprintln(os.Stderr, err)
+			os.Exit(1)
+		}
+	}
+	added := Added(known, base)
+	for _, line := range added {
+		fmt.Printf("checks/secrets/known gains %q, which the trunk's copy does not hold: the list only shrinks — "+
+			"protect the key instead (novox/hq issue 342)\n", line)
+	}
 	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",
@@ -204,7 +237,7 @@ func main() {
 		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 {
+	if len(refused) > 0 || len(stale) > 0 || len(added) > 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

@@ -76,3 +76,50 @@ func TestTheCatalogueHoldsNoSecretASettingCouldReplace(t *testing.T) {
 		t.Fatalf("refused %v, stale %v", refused, stale)
 	}
 }
+
+func TestALineTheTrunkDoesNotHoldIsRefused(t *testing.T) {
+	trunk := map[string]bool{"b cfg server.secret_key": true}
+	// A key unprotected on the branch, and its line added to the list: refused, though it matches a finding.
+	branch := map[string]bool{"b cfg server.secret_key": true, "a cfg password": true}
+	if added := Added(branch, trunk); len(added) != 1 || added[0] != "a cfg password" {
+		t.Fatalf("added %v", added)
+	}
+	// A line removed is no addition.
+	if added := Added(map[string]bool{}, trunk); len(added) != 0 {
+		t.Fatalf("added %v", added)
+	}
+	// Until the trunk holds a copy, the seed stands for it, and the list as committed is within it.
+	known, err := Known("known")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if added := Added(known, seed); len(added) != 0 {
+		t.Fatalf("known holds lines the seed does not: %v", added)
+	}
+}
+
+func TestTheCheckRefusesALineAddedAgainstTheTrunk(t *testing.T) {
+	root := t.TempDir()
+	manifest(t, root, "a", `{"id":"cfg","type":"file","merge":"json","content":"{\"password\": \"${secret:admin}\"}"}`)
+	dir := t.TempDir()
+	base := filepath.Join(dir, "base")
+	if err := os.WriteFile(base, []byte("# the trunk's\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	branch := map[string]bool{"a cfg password": true}
+	found, err := Check(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	trunk, err := Known(base)
+	if err != nil {
+		t.Fatal(err)
+	}
+	refused, stale := Judge(found, branch)
+	if len(refused) != 0 || len(stale) != 0 {
+		t.Fatalf("the line hides the finding from Judge alone: %v %v", refused, stale)
+	}
+	if added := Added(branch, trunk); len(added) != 1 {
+		t.Fatalf("an added line passed: %v", added)
+	}
+}

merge-check.sh

@@ -38,8 +38,19 @@ set -eu
 (cd checks/words && go test -count=1 . && go run . ../..)
 # The agent's home (novox/hq ADR 0266): no archive, and no file written into, under it.
 (cd checks/agenthome && go test -count=1 . && go run . ../..)
-# The secrets (novox/hq issue 342): a password is not a setting, so no setting can replace the vault's.
-(cd checks/secrets && go test -count=1 . && go run . ../..)
+# The secrets (novox/hq issue 342): a password is not a setting, so no setting can replace the vault's. The
+# known list only shrinks: judged against the trunk's copy at the merge base, a line it does not hold is refused.
+secrets_base=""
+secrets_ref="origin/${MESH_CHECK_BASE:-main}"
+if merge_base=$(git merge-base HEAD "$secrets_ref" 2>/dev/null); then
+	if git cat-file -e "$merge_base:checks/secrets/known" 2>/dev/null; then
+		secrets_base=$(mktemp)
+		git show "$merge_base:checks/secrets/known" >"$secrets_base"
+	fi
+else
+	echo "NOT COMPARED WITH THE TRUNK: no merge base with $secrets_ref; checks/secrets/known is held to the list it was first written with"
+fi
+(cd checks/secrets && go test -count=1 . && if [ -n "$secrets_base" ]; then go run . -base "$secrets_base" ../..; else go run . ../..; fi)
 # The lists that narrow the paths check may not grow unseen: the base branch's copies are what they are
 # judged against (MESH_CHECK_BASE, main when unset), read from the clone the check runs in.
 base_lists=""

modules/grafana/client.ts

@@ -44,6 +44,17 @@ function meshConfig(file?: string): Record<string, string> {
   catch { return {}; }
 }
 
+/**
+ * A password, token or API key is the vault's, never a setting (novox/hq issue 342): a value for one of
+ * them in the settings file is ignored, and that is said once — by its key, never with its value.
+ */
+const saidIgnored = new Set<string>();
+function ignoreSecretSetting(cfg: Record<string, unknown>, key: string): void {
+  if (cfg[key] === undefined || saidIgnored.has(key)) return;
+  saidIgnored.add(key);
+  console.error(`the setting ${key} is ignored: it comes from the vault`);
+}
+
 export class GrafanaClient {
   readonly baseUrl: string;
   private readonly authHeader: string;
@@ -54,21 +65,24 @@ export class GrafanaClient {
   }
 
   /**
-   * Build from the module's resolved environment. Auth is a service-account/API token
-   * (MESH_GRAFANA_TOKEN, sent as Bearer) when present, else HTTP basic with the admin password the
-   * module keeps as its own secret (MESH_GRAFANA_PASSWORD, user MESH_GRAFANA_USER, default admin).
-   * Throws when neither is configured — the module then contributes nothing rather than failing.
+   * Build from the module's resolved environment. Auth is HTTP basic with the vault's admin password,
+   * which the runtime config carries as `password` (kept protected, so no setting replaces it) or
+   * MESH_GRAFANA_PASSWORD (user MESH_GRAFANA_USER, default admin); else a service-account/API token from
+   * the environment (MESH_GRAFANA_TOKEN, sent as Bearer). A `token` in the settings file is never used: a
+   * credential is the vault's, never a setting (hq issue 342). Throws when neither is configured — the
+   * module then contributes nothing rather than failing.
    */
   static fromEnv(env: NodeJS.ProcessEnv = process.env): GrafanaClient {
     const cfg = meshConfig(env.MESH_GRAFANA_CONFIG_FILE);
     const url = cfg.url ?? env.MESH_GRAFANA_URL ?? `http://127.0.0.1:${env.GRAFANA_PORT ?? "3000"}`;
-    const token = cfg.token ?? env.MESH_GRAFANA_TOKEN;
-    if (token) return new GrafanaClient(url, `Bearer ${token}`);
+    ignoreSecretSetting(cfg, "token");
     const password = cfg.password ?? env.MESH_GRAFANA_PASSWORD;
     if (password) {
       const user = cfg.user ?? env.MESH_GRAFANA_USER ?? "admin";
       return new GrafanaClient(url, `Basic ${Buffer.from(`${user}:${password}`).toString("base64")}`);
     }
+    const token = env.MESH_GRAFANA_TOKEN;
+    if (token) return new GrafanaClient(url, `Bearer ${token}`);
     throw new Error("no Grafana auth — set MESH_GRAFANA_TOKEN or MESH_GRAFANA_PASSWORD");
   }
 

modules/grafana/module.json

@@ -142,7 +142,7 @@
       "mode": "0600",
       "content": "{\n  \"user\": \"admin\",\n  \"password\": \"${secret:admin}\"\n}\n",
       "merge": "json",
-      "protected": ["password"]
+      "protected": ["password", "token"]
     }
   ],
   "requires": [

modules/grafana/test/secret-setting.test.ts

@@ -0,0 +1,40 @@
+// A password, a token or an API key is the vault's, never a setting (novox/hq issue 342): grafana logs in
+// with the vault's admin password, a `token` in the settings file is ignored, and that is said once, by the
+// key alone. The runtime config keeps both keys protected, so the controller refuses a layer setting either.
+
+import { test, mock } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, readFileSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+
+import { GrafanaClient } from "../dist/client.js";
+
+const SETTING = "one-a-setting-carried";
+const VAULT = "the-vault-made-this";
+
+test("the vault's admin password is used, a token setting is ignored and said once, without its value", () => {
+  const said: string[] = [];
+  const err = mock.method(console, "error", (...args: unknown[]) => { said.push(args.join(" ")); });
+  try {
+    const dir = mkdtempSync(join(tmpdir(), "grafana-342-"));
+    // What the runtime config holds once the vault filled it, with a token a layer carried beside it.
+    writeFileSync(join(dir, "config.json"), JSON.stringify({ user: "admin", password: VAULT, token: SETTING }));
+    const env = { MESH_GRAFANA_CONFIG_FILE: join(dir, "config.json") };
+    const basic = "Basic " + Buffer.from(`admin:${VAULT}`).toString("base64");
+    assert.equal((GrafanaClient.fromEnv(env) as any).authHeader, basic);
+    assert.equal((GrafanaClient.fromEnv(env) as any).authHeader, basic);
+    assert.deepEqual(said, ["the setting token is ignored: it comes from the vault"]);
+    assert.ok(!said.join("\n").includes(SETTING), "the setting's value is never said");
+  } finally {
+    err.mock.restore();
+  }
+});
+
+test("the runtime config keeps password and token protected", () => {
+  const manifest = JSON.parse(readFileSync(new URL("../module.json", import.meta.url), "utf8")) as {
+    resources: { id: string; protected?: string[] }[];
+  };
+  const runtime = manifest.resources.find((r) => r.id === "runtime-config");
+  assert.deepEqual(runtime?.protected, ["password", "token"]);
+});