test(ingestor): pin clientRxCoverage.sources through LoadConfig (#278)

cmd/ingestor/client_rx_sources.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"fmt"
 	"log"
 	"strings"
 	"sync"
@@ -91,24 +92,29 @@ func (c *Config) warnClientRxSourceDrop(tag, name string, now time.Time) {
 // checkClientRxSources reports, once at startup, every clientRxCoverage.sources
 // entry that matches no configured mqttSources[].name — an allowlist of names
 // that can never match would silently drop all coverage. It also logs the
-// effective restriction so the gate is visible in the boot log. Returns the
+// effective restriction so the gate is visible in the boot log, and warns when
+// one entry matches more than one configured source (#278). Returns the
 // unknown names (nil when there is no allowlist or every name matches).
 func checkClientRxSources(cfg *Config, sources []MQTTSource) []string {
 	allow := cfg.ClientRxCoverageSources()
 	if len(allow) == 0 {
 		return nil
 	}
-	var unknown []string
+	var unknown, ambiguous []string
 	for _, want := range allow {
-		found := false
+		var matched []string
 		for _, src := range sources {
 			if strings.EqualFold(strings.TrimSpace(src.Name), want) {
-				found = true
-				break
+				matched = append(matched, fmt.Sprintf("%q", src.Name))
 			}
 		}
-		if !found {
+		switch {
+		case len(matched) == 0:
 			unknown = append(unknown, want)
+		case len(matched) > 1:
+			// mqttSources[].name is not required to be unique and matching is
+			// case-insensitive, so one entry can admit several brokers (#278).
+			ambiguous = append(ambiguous, fmt.Sprintf("%s matches %d sources (%s)", want, len(matched), strings.Join(matched, ", ")))
 		}
 	}
 	state := ""
@@ -122,5 +128,9 @@ func checkClientRxSources(cfg *Config, sources []MQTTSource) []string {
 		log.Printf("[client-rx] WARNING: %d clientRxCoverage.sources name(s) match no configured mqttSources[].name: %s — coverage will never be accepted for those names; check the spelling",
 			len(unknown), strings.Join(unknown, ", "))
 	}
+	if len(ambiguous) > 0 {
+		log.Printf("[client-rx] WARNING: clientRxCoverage.sources name(s) match more than one configured mqttSources[].name (case-insensitive): %s — coverage is accepted from every one of them; give each source a unique name",
+			strings.Join(ambiguous, "; "))
+	}
 	return unknown
 }

cmd/ingestor/client_rx_sources_278_test.go

@@ -0,0 +1,121 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// Follow-ups to the #265 client-RX source allowlist (#278).
+
+// TestLoadConfigClientRxCoverageSources pins the documented JSON key
+// clientRxCoverage.sources through the real LoadConfig. Every other allowlist
+// test builds ClientRxCoverageConfig directly, so a typo in the struct tag
+// would leave them green while the key silently did nothing — and an ignored
+// allowlist fails open (every source accepted).
+func TestLoadConfigClientRxCoverageSources(t *testing.T) {
+	t.Setenv("MQTT_BROKER", "")
+	cfgPath := filepath.Join(t.TempDir(), "config.json")
+	if err := os.WriteFile(cfgPath, []byte(`{
+		"mqttSources": [
+			{"name": "a", "broker": "tcp://a.invalid:1883", "topics": ["meshcore/#"]},
+			{"name": "b", "broker": "tcp://b.invalid:1883", "topics": ["meshcore/#"]}
+		],
+		"clientRxCoverage": { "enabled": true, "sources": ["a"] }
+	}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	cfg, err := LoadConfig(cfgPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if cfg.ClientRxCoverage == nil || !cfg.ClientRxCoverageEnabled() {
+		t.Fatalf("clientRxCoverage.enabled not loaded: %+v", cfg.ClientRxCoverage)
+	}
+	if got := cfg.ClientRxCoverage.Sources; len(got) != 1 || got[0] != "a" {
+		t.Fatalf("clientRxCoverage.sources = %q, want [a]", got)
+	}
+	if !cfg.ClientRxSourceAllowed("a") {
+		t.Error("listed source a must be allowed")
+	}
+	if cfg.ClientRxSourceAllowed("b") {
+		t.Error("unlisted source b must be rejected once sources is loaded")
+	}
+}
+
+// TestCheckClientRxSourcesDuplicateName: matching is case-insensitive and
+// mqttSources[].name is not required to be unique, so one allowlisted name can
+// admit several brokers. That widens a trust boundary without the operator
+// noticing, so startup warns once per such name and lists the matching sources.
+func TestCheckClientRxSourcesDuplicateName(t *testing.T) {
+	sources := []MQTTSource{{Name: "auth"}, {Name: " Auth "}, {Name: "legacy"}, {Name: "LEGACY"}}
+
+	cfg := coverageCfgWithSources("AUTH")
+	var unknown []string
+	out := captureLog(t, func() { unknown = checkClientRxSources(cfg, sources) })
+
+	if len(unknown) != 0 {
+		t.Fatalf("a duplicated name is not unknown: unknown = %v", unknown)
+	}
+	var warn []string
+	for _, l := range strings.Split(out, "\n") {
+		if strings.Contains(l, "WARNING") {
+			warn = append(warn, l)
+		}
+	}
+	if len(warn) != 1 {
+		t.Fatalf("expected exactly one warning line, got %d:\n%s", len(warn), out)
+	}
+	for _, want := range []string{"AUTH", `"auth"`, `" Auth "`, "2"} {
+		if !strings.Contains(warn[0], want) {
+			t.Errorf("warning must mention %s:\n%s", want, warn[0])
+		}
+	}
+	if strings.Contains(strings.ToLower(warn[0]), "legacy") {
+		t.Errorf("legacy is duplicated but not allowlisted, it must not be reported:\n%s", warn[0])
+	}
+}
+
+// TestCheckClientRxSourcesUniqueNamesNoDuplicateWarning: one match per
+// allowlisted name ⇒ no warning at all.
+func TestCheckClientRxSourcesUniqueNamesNoDuplicateWarning(t *testing.T) {
+	sources := []MQTTSource{{Name: "auth"}, {Name: "legacy"}}
+	out := captureLog(t, func() { checkClientRxSources(coverageCfgWithSources("Auth", "legacy"), sources) })
+	if strings.Contains(out, "WARNING") {
+		t.Fatalf("unique names must not warn:\n%s", out)
+	}
+}
+
+// TestClientRxCoverageDocListsDropLogLimits: AGENTS.md rule 8 — the hardcoded
+// drop-warning interval and line cap are listed under "Configurable values
+// (future customizer)", with their current values taken from the constants so
+// the docs cannot drift from the code.
+func TestClientRxCoverageDocListsDropLogLimits(t *testing.T) {
+	data, err := os.ReadFile(filepath.Join("..", "..", "docs", "client-rx-coverage.md"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	const heading = "## Configurable values (future customizer)"
+	doc := string(data)
+	i := strings.Index(doc, heading)
+	if i < 0 {
+		t.Fatalf("docs/client-rx-coverage.md has no %q section", heading)
+	}
+	section := doc[i+len(heading):]
+	if j := strings.Index(section, "\n## "); j >= 0 {
+		section = section[:j]
+	}
+	section = strings.Join(strings.Fields(section), " ") // tolerate line wrapping
+
+	for _, want := range []string{
+		fmt.Sprintf("`clientRxSourceWarnInterval`, %d minutes", int(clientRxSourceWarnInterval.Minutes())),
+		fmt.Sprintf("`clientRxSourceWarnMax`, %d lines", clientRxSourceWarnMax),
+	} {
+		if !strings.Contains(section, want) {
+			t.Errorf("configurable-values section must list %q; got:\n%s", want, section)
+		}
+	}
+}

docs/client-rx-coverage.md

@@ -65,6 +65,9 @@ Notes:
   can never be listed, so it is rejected whenever an allowlist is set.
 - A name that matches no configured source is logged once at startup — the allowlist would otherwise
   silently drop all coverage from that (mistyped) name.
+- Source names are not required to be unique, so a name that matches more than one configured source
+  (for example `auth` and `Auth`) admits all of them; that is also logged once at startup. Give each
+  source a unique name.
 - Drops are logged per source, throttled and capped at a fixed number of lines, so a busy unlisted
   broker cannot flood the log.
 - The option gates only the client namespace. The observer blacklist and every other ingest rule stay
@@ -251,3 +254,10 @@ on-device signing) — not required for the MVP, tracked as a follow-up.
 
 Hardcoded initially, tracked for the customizer per AGENTS.md rule 8: hex resolution per zoom
 (`zoomToHexRes`), colour SNR thresholds (`coverageColorVar`), and any `rx_at` max-age validation.
+
+The source-allowlist drop warning ([Restricting which MQTT sources may
+contribute](#restricting-which-mqtt-sources-may-contribute)) is also hardcoded, in
+`cmd/ingestor/client_rx_sources.go`: the re-log interval per source (`clientRxSourceWarnInterval`,
+10 minutes) and the per-source line cap for the lifetime of the process (`clientRxSourceWarnMax`,
+10 lines). These are ingestor settings, so they would become config keys rather than customizer
+controls.