fix(loader): one duplicate node_id should not disable the entire trusted-agents feed

data.go

@@ -286,15 +286,28 @@ func Load(raw []byte) error {
 		return err
 	}
 	idx := make(map[uint32]entry, len(doc.Agents))
+	voided := make(map[uint32]bool) // node_ids seen more than once
 	for _, a := range doc.Agents {
 		if a.NodeID == 0 {
 			continue // 0 is reserved / would silently match unset fields
 		}
 		if a.Hostname == "" {
 			continue // empty hostname: missing required field — drop
 		}
+		if voided[a.NodeID] {
+			continue // a duplicate already voided this node_id (below)
+		}
 		if other, exists := idx[a.NodeID]; exists {
-			return fmt.Errorf("duplicate node_id %d in trusted-agents list: %q and %q", a.NodeID, other.name, a.Hostname)
+			// Duplicate node_id: drop EVERY entry for it (the one already
+			// indexed and this one) rather than failing the whole list. An
+			// ambiguous node_id must not be trusted — neither the first
+			// entry nor a later pin may silently win — but a single bad row
+			// must not disable the entire feed.
+			slog.Warn("trustedagents: duplicate node_id — dropping all entries for it",
+				"node_id", a.NodeID, "hostnames", []string{other.name, a.Hostname})
+			delete(idx, a.NodeID)
+			voided[a.NodeID] = true
+			continue
 		}
 		pin, err := decodePin(a.PublicKey)
 		if err != nil {

zz_fuzz_test.go

@@ -67,7 +67,7 @@ func FuzzLoad(f *testing.F) {
 		`{"agents":[{"hostname":"a","node_id":1}]}`,
 		`{"agents":[{"hostname":"a","node_id":0}]}`,                              // zero id dropped
 		`{"agents":[{"hostname":"","node_id":5}]}`,                               // empty host dropped
-		`{"agents":[{"hostname":"a","node_id":1},{"hostname":"b","node_id":1}]}`, // duplicate → error
+		`{"agents":[{"hostname":"a","node_id":1},{"hostname":"b","node_id":1}]}`, // duplicate → both dropped, node_id 1 untrusted
 		`{"agents":[{"hostname":"a","node_id":1,"public_key":"` + goodPin + `"}]}`,
 		`{"agents":[{"hostname":"a","node_id":1,"public_key":"!!!"}]}`,    // bad base64
 		`{"agents":[{"hostname":"a","node_id":1,"public_key":"AAAA"}]}`,   // short key
@@ -202,10 +202,11 @@ func TestLoad_OversizedDocDoesNotPanic(t *testing.T) {
 	}
 }
 
-// TestLoad_DuplicateWithPinsRejected confirms the duplicate-node_id
-// guard still fires when the colliding entries carry pins — the loader
-// must reject rather than letting the second pin silently win.
-func TestLoad_DuplicateWithPinsRejected(t *testing.T) {
+// TestLoad_DuplicateWithPinsDropped confirms the duplicate-node_id guard
+// still fires when the colliding entries carry pins: the loader drops every
+// entry for that node_id (neither the first entry nor a later pin may win)
+// while the rest of the list still loads.
+func TestLoad_DuplicateWithPinsDropped(t *testing.T) {
 	restore := SetForTest(nil)
 	t.Cleanup(restore)
 
@@ -216,15 +217,15 @@ func TestLoad_DuplicateWithPinsRejected(t *testing.T) {
 	doc := `{"agents":[` +
 		`{"hostname":"x","node_id":7,"public_key":"` + a + `"},` +
 		`{"hostname":"y","node_id":7,"public_key":"` + bk + `"}]}`
-	if err := Load([]byte(doc)); err == nil {
-		t.Fatal("duplicate node_id with pins must be rejected")
+	if err := Load([]byte(doc)); err != nil {
+		t.Fatalf("duplicate node_id must be dropped, not error the load: %v", err)
 	}
-	// The failed Load must not have trusted either pin.
+	// The dropped node must not have trusted either pin.
 	if _, ok := IsTrustedWithKey(7, pubA); ok {
-		t.Fatal("node_id 7 must not be trusted after a rejected duplicate Load")
+		t.Fatal("node_id 7 must not be trusted after a dropped duplicate Load")
 	}
 	if _, ok := IsTrustedWithKey(7, pubB); ok {
-		t.Fatal("node_id 7 must not be trusted after a rejected duplicate Load")
+		t.Fatal("node_id 7 must not be trusted after a dropped duplicate Load")
 	}
 }
 

zz_test.go

@@ -118,16 +118,18 @@ func TestLoadDuplicateNodeID(t *testing.T) {
 	// then asserts on it. Marking it parallel let a concurrent
 	// global-mutating test (e.g. a fuzz seed run) race the post-Load
 	// assertion. Matches the other Load() tests in this file.
+	// A duplicate node_id is dropped (both entries), not fatal to the list:
+	// one bad row must not disable the whole feed.
 	err := Load([]byte(`{"agents":[
 		{"hostname":"a","node_id":1},
 		{"hostname":"b","node_id":1}
 	]}`))
-	if err == nil {
-		t.Fatal("Load with duplicate node_id must return an error")
+	if err != nil {
+		t.Fatalf("duplicate node_id must be dropped, not error the whole load: %v", err)
 	}
-	// Also verify the list wasn't corrupted by the failed load.
+	// The ambiguous node must NOT be trusted — neither entry wins.
 	if name, ok := IsTrusted(1); ok {
-		t.Fatalf("IsTrusted(1)=%q after failed Load — list must not be updated", name)
+		t.Fatalf("IsTrusted(1)=%q — a duplicated (ambiguous) node_id must stay untrusted", name)
 	}
 	_ = Load(embeddedJSON) // restore
 }