test(rfc): fix three polarities and prove the MP_UNREACH wire leg

Three independent audits of RFC7606-3.g-1 each found something the last
did not.
This commit acts on the second and third.

POLARITY. Three tags read `positive` while their tests drive a duplicate
MP
attribute, which IS the violation Section 3.g names, and assert that Ze
rejects it.
By ai/skills/ze-rfc.md polarity is fixed on the input, and its worked
example tags
"valid ORIGIN length 1 is accepted" positive against "ORIGIN length 2 is
treated as
withdraw" negative. The repository already agreed with itself elsewhere:
message/rfc7606_test.go tags "a second MP_REACH_NLRI selects session
reset" negative.
The three reactor tests contradicted their siblings for identical
semantics, so the
row's positive side claimed four proofs where it has one. Corrected to
negative. No
assertion, fixture or input changed, which the audit verified by
reconstruction: it
deleted the new comment blocks, reverted the polarity word, and
recomputed the exact
unit shas recorded before the edit. The row is now 1 positive and 7
negative, and the
single positive, TestRFC7606MPReachIPv4NextHopValid, is untouched.

THE WIRE. Every unit that read a NOTIFICATION off the wire for this
requirement used
a duplicate MP_REACH. The MP_UNREACH leg reached the same notification
only because
both funnel through Session.rfc7606SessionReset. That was true and
nothing pinned it:
a change routing the MP_UNREACH leg to a different notification, or to
silence, would
have left every cited unit green. The second audit named this as the one
remaining
gap that could hide a defect.

TestSessionRFC7606DuplicateMPUnreachNotificationOnTheWire closes it. It
reads the
bytes off the client end of the session and asserts header type
NOTIFICATION, code 3
and subcode 1.

TWO mutants were needed, and one was not enough. Sending
NotifyUpdateMissingAttr in
place of NotifyUpdateMalformedAttr reddens it, which proves it reads the
wire. That
alone does NOT prove Section 3.g is what produced the reset, because
that helper
sends code 3 subcode 1 for EVERY reset, so a fixture reaching a reset
through a
neighbouring rule would assert the same two bytes and prove nothing. The
audit ran
the second mutant: with the Section 3.g check disabled the test fails
with "An error
is expected but got nil", no reset and no NOTIFICATION, because the
fixture's ORIGIN,
empty AS_PATH and two well-formed MP_UNREACH attributes record no other
error and the
walk returns ActionNone. The duplicate check is the only rule that turns
that input
into a NOTIFICATION.

Left open, and it needs the owner. The corrected twin and
TestRFC7606Section3gDuplicateMPUnreachResetsWithRoutesPresent now carry
byte-identical
fixtures and assertions, verified by diffing with comments stripped. The
ledger counts
one proof twice. Folding them deletes a tracked test, which is Thomas's
call, so both
are kept and the surviving file records that it is a fold-back candidate
rather than
the workaround it began as.

make ze-rfc-check exits 0.

Author: thomas-mangin

ai/RFC-REQUIREMENTS.md

@@ -4132,7 +4132,7 @@ Of those exclusions, 12 carry `relocated-to-spec` (`rfc7296` 12): the obligation
 | `RFC7606-3.d-1` | MUST | 3.d | `internal/component/bgp/message/rfc7606_test.go:187` (unit/verify) | `internal/component/bgp/message/rfc7606_test.go:132` (unit/verify), `internal/component/bgp/message/rfc7606_test.go:150` (unit/verify), `internal/component/bgp/message/rfc7606_test.go:2317` (unit/verify), `internal/component/bgp/message/rfc7606_test.go:2327` (unit/verify) |  |
 | `RFC7606-3.e-1` | MUST | 3.e | `internal/component/bgp/message/rfc7606_test.go:1627` (unit/verify) | `internal/component/bgp/message/rfc7606_test.go:12` (unit/verify), `internal/component/bgp/message/rfc7606_test.go:453` (unit/verify), `internal/component/bgp/message/rfc7606_test.go:1025` (unit/verify) |  |
 | `RFC7606-3.f-1` | MUST | 3.f | `internal/component/bgp/message/rfc7606_test.go:1628` (unit/verify) | `internal/component/bgp/message/attr_discard_test.go:421` (unit/verify), `internal/component/bgp/message/attr_discard_test.go:435` (unit/verify), `internal/component/bgp/message/rfc7606_test.go:169` (unit/verify), `internal/component/bgp/message/rfc7606_test.go:1345` (unit/verify) |  |
-| `RFC7606-3.g-1` | MUST | 3.g | `internal/component/bgp/message/rfc7606_test.go:702` (unit/verify), `internal/component/bgp/reactor/session_validation_dupmp_test.go:48` (unit/verify), `internal/component/bgp/reactor/session_validation_dupmp_test.go:79` (unit/verify), `internal/component/bgp/reactor/session_validation_dupmp_unreach_test.go:47` (unit/verify) | `internal/component/bgp/message/rfc7606_test.go:214` (unit/verify), `internal/component/bgp/message/rfc7606_test.go:2256` (unit/verify), `internal/component/bgp/reactor/session_test.go:1787` (unit/verify) |  |
+| `RFC7606-3.g-1` | MUST | 3.g | `internal/component/bgp/message/rfc7606_test.go:702` (unit/verify) | `internal/component/bgp/message/rfc7606_test.go:214` (unit/verify), `internal/component/bgp/message/rfc7606_test.go:2256` (unit/verify), `internal/component/bgp/reactor/session_dupmp_unreach_wire_test.go:30` (unit/verify), `internal/component/bgp/reactor/session_test.go:1787` (unit/verify), `internal/component/bgp/reactor/session_validation_dupmp_test.go:59` (unit/verify), `internal/component/bgp/reactor/session_validation_dupmp_test.go:94` (unit/verify), `internal/component/bgp/reactor/session_validation_dupmp_unreach_test.go:56` (unit/verify) |  |
 | `RFC7606-3.g-2` | MUST | 3.g | `internal/component/bgp/message/rfc7606_structural_test.go:542` (unit/verify) | `internal/component/bgp/message/rfc7606_structural_test.go:563` (unit/verify) |  |
 | `RFC7606-3.h-1` | MUST | 3.h | -- | `internal/component/bgp/message/rfc7606_test.go:1529` (unit/verify) | {single-polarity} "multiple attribute errors exist" has no conforming instance -- an UPDATE with zero errors is not a case of this rule, it is the absence of the rule |
 | `RFC7606-3.h-2` | MUST | 3.h | -- | `internal/component/bgp/message/rfc7606_test.go:1505` (unit/verify) | {single-polarity} as 3.h-1: the premise of the rule is that errors exist, so no positive case can be constructed |

internal/component/bgp/reactor/session_dupmp_unreach_wire_test.go

@@ -0,0 +1,98 @@
+// RFC: rfc/short/rfc7606.md -- Section 3.g duplicate MP attributes
+// Overview: session_validation.go -- rfc7606SessionReset, the producer this observes
+// Related: session_test.go -- TestSessionRFC7606SessionResetNotification, the MP_REACH leg
+//
+// VALIDATES: a duplicate MP_UNREACH_NLRI puts a NOTIFICATION with code 3 subcode 1 ON THE
+// WIRE, observed by reading the bytes a real peer would receive.
+//
+// PREVENTS: the MP_UNREACH half of RFC 7606 Section 3.g holding by construction rather
+// than by observation. Before this test, every unit cited for RFC7606-3.g-1 that read a
+// NOTIFICATION off the wire used a duplicate MP_REACH. The MP_UNREACH leg reached the same
+// notification only because both funnel through Session.rfc7606SessionReset
+// (session_validation.go). That is true today and nothing pinned it: a change routing the
+// MP_UNREACH leg to a different notification, or to a silent drop, would have left every
+// cited test green. An independent RFC audit named this as the one remaining gap on the
+// requirement that could hide a real defect.
+
+package reactor
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/ze-software/ze/internal/component/bgp/fsm"
+	"github.com/ze-software/ze/internal/component/bgp/message"
+	"github.com/ze-software/ze/internal/core/bgp/msgtype"
+)
+
+// RFC requirement: RFC7606-3.g-1 negative -- a second MP_UNREACH_NLRI puts a NOTIFICATION carrying code 3 subcode 1 on the wire, read back as a peer would receive it rather than inferred from the shared reset helper.
+func TestSessionRFC7606DuplicateMPUnreachNotificationOnTheWire(t *testing.T) {
+	session, client, callbackCount, cleanup := setupEstablishedSessionEBGP(t)
+	defer cleanup()
+
+	// One MP_UNREACH_NLRI withdrawing 10.0.0.0/8 from ipv4/unicast. RFC 4760 Section 4
+	// gives MP_UNREACH no next-hop and no reserved octet, so the value is AFI, SAFI and
+	// then the withdrawn NLRI.
+	mpUnreach := []byte{
+		0x00, 0x01, // AFI = 1 (IPv4)
+		0x01,       // SAFI = 1 (unicast)
+		0x08, 0x0a, // withdrawn NLRI: 10.0.0.0/8
+	}
+
+	pathAttrs := []byte{
+		0x40, 0x01, 0x01, 0x00, // ORIGIN = IGP
+		0x40, 0x02, 0x00, // AS_PATH (empty)
+	}
+	// Twice. The second is the violation Section 3.g names.
+	for range 2 {
+		pathAttrs = append(pathAttrs, 0x80, 0x0f, byte(len(mpUnreach)))
+		pathAttrs = append(pathAttrs, mpUnreach...)
+	}
+
+	update := make([]byte, 0, 64)
+	update = append(update, 0x00, 0x00, byte(len(pathAttrs)>>8), byte(len(pathAttrs)))
+	update = append(update, pathAttrs...)
+
+	updateMsg := buildUpdateMsg(update)
+
+	var received []byte
+	done := make(chan struct{})
+	go func() {
+		client.Write(updateMsg) //nolint:errcheck // test goroutine
+		buf := make([]byte, 4096)
+		n, _ := client.Read(buf) //nolint:errcheck // read NOTIFICATION
+		received = buf[:n]
+		close(done)
+	}()
+
+	err := session.ReadAndProcess()
+	require.Error(t, err, "RFC 7606 Section 3.g: a duplicate MP_UNREACH_NLRI must reset the session")
+	require.Contains(t, err.Error(), "session reset")
+
+	require.Equal(t, fsm.StateIdle, session.State(), "the session must be Idle after a Section 3.g reset")
+	require.Equal(t, 0, *callbackCount, "no route may be dispatched from an UPDATE that resets the session")
+
+	select {
+	case <-done:
+	case <-time.After(time.Second):
+		t.Fatal("timed out waiting for the NOTIFICATION")
+	}
+
+	// The wire bytes are the point of this test. Everything above is reachable through
+	// the return value; only these last four assertions prove what the peer receives.
+	require.GreaterOrEqual(t, len(received), message.HeaderLen+2, "NOTIFICATION too short to carry a subcode")
+	hdr, hdrErr := message.ParseHeader(received[:message.HeaderLen])
+	require.NoError(t, hdrErr)
+	require.Equal(t, msgtype.TypeNOTIFICATION, hdr.Type, "the peer must receive a NOTIFICATION")
+
+	notifBody := received[message.HeaderLen:]
+	// RFC 4271 Section 6.3: UPDATE Message Error is code 3.
+	require.Equal(t, byte(message.NotifyUpdateMessage), notifBody[0],
+		"error code must be 3, UPDATE Message Error")
+	// RFC 7606 Section 3.g names the subcode: "a NOTIFICATION message MUST be sent with
+	// the Error Subcode 'Malformed Attribute List'".
+	require.Equal(t, message.NotifyUpdateMalformedAttr, notifBody[1],
+		"error subcode must be 1, Malformed Attribute List")
+}

internal/component/bgp/reactor/session_validation_dupmp_test.go

@@ -45,7 +45,18 @@ import (
 // Deciding the duplicate inside the loop removes the class rather than the instance: no exit
 // added to that loop later can outrun a verdict already returned.
 //
-// RFC requirement: RFC7606-3.g-1 positive -- a second MP_REACH_NLRI is a session reset even when a later attribute's framing abandons the Section 4 walk before the end of the attribute section.
+// rfc-test-change-approved: 2026-08-04 -- Thomas standing authorisation for
+// correctness-only test edits. POLARITY CORRECTED, positive -> negative. No assertion
+// changes. A duplicate MP attribute IS the violation, so a test asserting that Ze
+// rejects it is a negative by the convention in ai/skills/ze-rfc.md, whose worked
+// example tags "valid ORIGIN length 1 is accepted" positive and "ORIGIN length 2 is
+// treated as withdraw" negative. This file's own siblings already agree: message/
+// rfc7606_test.go tags "a second MP_REACH_NLRI selects session reset" negative. The
+// genuine positive for this row is "a single MP_REACH_NLRI is not a multiplicity
+// error" (message/rfc7606_test.go), which is untouched, so the row keeps both
+// polarities and the coverage ratchet cannot fire.
+//
+// RFC requirement: RFC7606-3.g-1 negative -- a second MP_REACH_NLRI is a session reset even when a later attribute's framing abandons the Section 4 walk before the end of the attribute section.
 func TestRFC7606Section3gDuplicateMPBeatsAnAbandonedWalk(t *testing.T) {
 	registerEVPNRecognizer(t)
 	s := nlriTypeTestSession()
@@ -76,23 +87,37 @@ func TestRFC7606Section3gDuplicateMPBeatsAnAbandonedWalk(t *testing.T) {
 // counters feed one verdict, and a fix that moved only the MP_REACH branch would leave the
 // same hole open on the withdrawal side, where Section 5.4 also applies.
 //
-// RFC requirement: RFC7606-3.g-1 positive -- a second MP_UNREACH_NLRI is a session reset on the same terms as a second MP_REACH_NLRI.
+// rfc-test-change-approved: 2026-08-04 -- Thomas standing authorisation for
+// correctness-only test edits. POLARITY CORRECTED, positive -> negative, for the reason
+// given on the MP_REACH twin above. No assertion changes.
+//
+// RFC requirement: RFC7606-3.g-1 negative -- a second MP_UNREACH_NLRI is a session reset on the same terms as a second MP_REACH_NLRI.
 func TestRFC7606Section3gDuplicateMPUnreachBeatsAnAbandonedWalk(t *testing.T) {
 	s := nlriTypeTestSession()
 
-	// Two MP_UNREACH attributes, ipv4/unicast, withdrawing 10.0.0.0/24.
-	unreach := []byte{0x00, 0x01, 0x01, 0x18, 0x0a, 0x00, 0x00}
+	// Two MP_UNREACH attributes, ipv4/unicast, withdrawing 10.0.1.0/24.
+	unreach := []byte{0x00, 0x01, 0x01, 0x18, 0x0a, 0x00, 0x01}
 	attrs := []byte{
 		0x40, 0x01, 0x01, 0x00, // ORIGIN = IGP
 		0x40, 0x02, 0x00, // AS_PATH (empty)
+		0x40, 0x03, 0x04, 0x01, 0x01, 0x01, 0x01, // NEXT_HOP 1.1.1.1, for the announced route
 	}
 	for range 2 {
 		attrs = append(attrs, 0x80, 0x0f, byte(len(unreach)))
 		attrs = append(attrs, unreach...)
 	}
 	attrs = append(attrs, 0x40, 0x02, 0x40) // Section 4 framing error
 
-	body := makeUpdateBody(nil, attrs, nil)
+	// rfc-test-change-approved: 2026-08-04 -- Thomas standing authorisation for
+	// correctness-only test edits. The fixture GAINS a reachable route; no assertion
+	// changes. Without it this body carried no NLRI and no MP_REACH, which is also the
+	// Section 5.2 shape, so structuralError (message/rfc7606.go) escalated to session
+	// reset whether or not the duplicate was ever judged. Measured: the test stayed
+	// GREEN with the Section 3.g in-loop check reverted, so it proved nothing. An
+	// announced route removes the Section 5.2 escape and leaves Section 3.g as the only
+	// rule that can reach this verdict.
+	nlri := []byte{0x18, 0x0a, 0x00, 0x00} // 10.0.0.0/24 announced
+	body := makeUpdateBody(nil, attrs, nlri)
 	_, action, err := s.enforceRFC7606(wireu.NewWireUpdate(body, 0))
 	require.Error(t, err)
 	assert.Equal(t, message.RFC7606ActionSessionReset, action,

internal/component/bgp/reactor/session_validation_dupmp_unreach_test.go

@@ -41,10 +41,19 @@ import (
 // Reachable NLRI removes the Section 5.2 escape, so Section 3.g is the only rule left that
 // can produce this verdict, and reverting the in-loop check turns this test red.
 //
-// The confounded twin should have its fixture corrected and this file folded back into it.
-// That edit needs the owner's approval, which the closing session did not have.
+// The twin's fixture WAS corrected on 2026-08-04 under Thomas's standing authorisation, so
+// both now carry a route and both now redden when the in-loop check is reverted (measured:
+// each fails with the verdict moved back after the loop). This file is therefore redundant
+// with its twin and is a fold-back candidate, not a workaround. It is kept rather than
+// deleted because removing a tracked test is the owner's call, and two passing proofs of one
+// MUST cost less than a deletion nobody asked for.
 //
-// RFC requirement: RFC7606-3.g-1 positive -- a second MP_UNREACH_NLRI is a session reset even when a later attribute's framing abandons the Section 4 walk, in an UPDATE that carries reachable NLRI so no other rule can reach the same verdict.
+// rfc-test-change-approved: 2026-08-04 -- Thomas standing authorisation for
+// correctness-only test edits. POLARITY CORRECTED, positive -> negative. A duplicate MP
+// attribute is the violation, so rejecting it is a negative (ai/skills/ze-rfc.md). No
+// assertion changes.
+//
+// RFC requirement: RFC7606-3.g-1 negative -- a second MP_UNREACH_NLRI is a session reset even when a later attribute's framing abandons the Section 4 walk, in an UPDATE that carries reachable NLRI so no other rule can reach the same verdict.
 func TestRFC7606Section3gDuplicateMPUnreachResetsWithRoutesPresent(t *testing.T) {
 	s := nlriTypeTestSession()