diff --git a/data.go b/data.go index a9f0f8e..ab74584 100644 --- a/data.go +++ b/data.go @@ -286,6 +286,7 @@ 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 @@ -293,8 +294,20 @@ func Load(raw []byte) error { 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 { diff --git a/zz_fuzz_test.go b/zz_fuzz_test.go index 2fb91b6..9f928c9 100644 --- a/zz_fuzz_test.go +++ b/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") } } diff --git a/zz_test.go b/zz_test.go index 3188ae2..d8fb1ce 100644 --- a/zz_test.go +++ b/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 }