Fix CodeRabbit findings: hasIPv6Changed restart loop, empty peerIPs panic, v6 validation

client/iface/wgaddr/address.go

@@ -1,10 +1,7 @@
 package wgaddr
 
 import (
-	"fmt"
 	"net/netip"
-
-	"github.com/netbirdio/netbird/shared/netiputil"
 )
 
 // Address WireGuard parsed address
@@ -59,23 +56,6 @@ func (addr Address) IPv6Prefix() netip.Prefix {
 	return netip.PrefixFrom(addr.IPv6, addr.IPv6Net.Bits())
 }
 
-// SetIPv6FromCompact decodes a compact prefix (5 or 17 bytes) and sets the IPv6 fields.
-// Returns an error if the bytes are invalid. A nil or empty input is a no-op.
-//
-//nolint:recvcheck
-func (addr *Address) SetIPv6FromCompact(raw []byte) error {
-	if len(raw) == 0 {
-		return nil
-	}
-	prefix, err := netiputil.DecodePrefix(raw)
-	if err != nil {
-		return fmt.Errorf("decode v6 overlay address: %w", err)
-	}
-	addr.IPv6 = prefix.Addr()
-	addr.IPv6Net = prefix.Masked()
-	return nil
-}
-
 // ClearIPv6 removes the IPv6 overlay address, leaving only v4.
 //
 //nolint:recvcheck // ClearIPv6 is the only mutating method on this otherwise value-type struct.

client/internal/connect.go

@@ -40,6 +40,7 @@ import (
 	"github.com/netbirdio/netbird/client/system"
 	mgm "github.com/netbirdio/netbird/shared/management/client"
 	mgmProto "github.com/netbirdio/netbird/shared/management/proto"
+	"github.com/netbirdio/netbird/shared/netiputil"
 	"github.com/netbirdio/netbird/shared/relay/auth/hmac"
 	relayClient "github.com/netbirdio/netbird/shared/relay/client"
 	signal "github.com/netbirdio/netbird/shared/signal/client"
@@ -529,8 +530,16 @@ func createEngineConfig(key wgtypes.Key, config *profilemanager.Config, peerConf
 	}
 
 	if !config.DisableIPv6 {
-		if err := wgAddr.SetIPv6FromCompact(peerConfig.GetAddressV6()); err != nil {
-			log.Warn(err)
+		if raw := peerConfig.GetAddressV6(); len(raw) > 0 {
+			prefix, err := netiputil.DecodePrefix(raw)
+			if err != nil {
+				log.Warnf("decode v6 overlay address: %v", err)
+			} else if !prefix.Addr().Is6() {
+				log.Warnf("expected IPv6 overlay address, got %s", prefix.Addr())
+			} else {
+				wgAddr.IPv6 = prefix.Addr()
+				wgAddr.IPv6Net = prefix.Masked()
+			}
 		}
 	}
 

client/internal/engine.go

@@ -1035,22 +1035,24 @@ func (e *Engine) updateConfig(conf *mgmProto.PeerConfig) error {
 }
 
 // hasIPv6Changed reports whether the IPv6 overlay address in the peer config
-// differs from the current interface address (added, removed, or changed).
+// differs from the configured address (added, removed, or changed).
+// Compares against e.config.WgAddr (not the interface address, which may have
+// been cleared by ClearIPv6 if OS assignment failed).
 func (e *Engine) hasIPv6Changed(conf *mgmProto.PeerConfig) bool {
-	current := e.wgInterface.Address()
+	current := e.config.WgAddr
 	raw := conf.GetAddressV6()
 
 	if len(raw) == 0 {
 		return current.HasIPv6()
 	}
 
-	addr, err := netiputil.DecodeAddr(raw)
+	prefix, err := netiputil.DecodePrefix(raw)
 	if err != nil {
 		log.Warnf("decode v6 overlay address: %v", err)
 		return false
 	}
 
-	return !current.HasIPv6() || current.IPv6 != addr
+	return !current.HasIPv6() || current.IPv6 != prefix.Addr() || current.IPv6Net != prefix.Masked()
 }
 
 func (e *Engine) receiveJobEvents() {
@@ -1540,20 +1542,17 @@ func (e *Engine) addNewPeer(peerConfig *mgmProto.RemotePeerConfig) error {
 		peerIPs = append(peerIPs, allowedNetIP)
 	}
 
+	if len(peerIPs) == 0 {
+		return fmt.Errorf("peer %s has no usable AllowedIPs", peerKey)
+	}
+
 	conn, err := e.createPeerConn(peerKey, peerIPs, peerConfig.AgentVersion)
 	if err != nil {
 		return fmt.Errorf("create peer connection: %w", err)
 	}
 
-	var peerIPv6 string
-	ourV6Net := e.wgInterface.Address().IPv6Net
-	for _, pip := range peerIPs {
-		if pip.Addr().Is6() && pip.Bits() == 128 && ourV6Net.Contains(pip.Addr()) {
-			peerIPv6 = pip.Addr().String()
-			break
-		}
-	}
-	err = e.statusRecorder.AddPeer(peerKey, peerConfig.Fqdn, peerIPs[0].Addr().String(), peerIPv6)
+	peerV4, peerV6 := splitAllowedIPs(peerConfig.GetAllowedIps(), e.wgInterface.Address().IPv6Net)
+	err = e.statusRecorder.AddPeer(peerKey, peerConfig.Fqdn, peerV4, peerV6)
 	if err != nil {
 		log.Warnf("error adding peer %s to status recorder, got error: %v", peerKey, err)
 	}

client/internal/engine_test.go

@@ -1728,7 +1728,7 @@ func TestEngine_hasIPv6Changed(t *testing.T) {
 		{
 			name:     "no v6 before, v6 added",
 			current:  v4Only,
-			confV6:   netiputil.EncodeAddr(netip.MustParseAddr("fd00::1")),
+			confV6:   netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/64")),
 			expected: true,
 		},
 		{
@@ -1740,13 +1740,19 @@ func TestEngine_hasIPv6Changed(t *testing.T) {
 		{
 			name:     "had v6, same v6",
 			current:  v4v6,
-			confV6:   netiputil.EncodeAddr(netip.MustParseAddr("fd00::1")),
+			confV6:   netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/64")),
 			expected: false,
 		},
 		{
 			name:     "had v6, different v6",
 			current:  v4v6,
-			confV6:   netiputil.EncodeAddr(netip.MustParseAddr("fd00::2")),
+			confV6:   netiputil.EncodePrefix(netip.MustParsePrefix("fd00::2/64")),
+			expected: true,
+		},
+		{
+			name:     "same v6 addr, different prefix length",
+			current:  v4v6,
+			confV6:   netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/80")),
 			expected: true,
 		},
 		{
@@ -1760,9 +1766,7 @@ func TestEngine_hasIPv6Changed(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			engine := &Engine{
-				wgInterface: &MockWGIface{
-					AddressFunc: func() wgaddr.Address { return tt.current },
-				},
+				config: &EngineConfig{WgAddr: tt.current},
 			}
 			conf := &mgmtProto.PeerConfig{
 				AddressV6: tt.confV6,