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

This commit is contained in:
Viktor Liu
2026-03-25 10:18:06 +01:00
parent 878dc45abf
commit baf2c03508
4 changed files with 28 additions and 22 deletions
+2 -2
View File
@@ -61,8 +61,8 @@ jobs:
echo "Size: ${SIZE} bytes (${SIZE_MB} MB)" echo "Size: ${SIZE} bytes (${SIZE_MB} MB)"
if [ ${SIZE} -gt 57671680 ]; then if [ ${SIZE} -gt 58720256 ]; then
echo "Wasm binary size (${SIZE_MB}MB) exceeds 55MB limit!" echo "Wasm binary size (${SIZE_MB}MB) exceeds 56MB limit!"
exit 1 exit 1
fi fi
+4 -1
View File
@@ -71,6 +71,9 @@ func (addr *Address) SetIPv6FromCompact(raw []byte) error {
if err != nil { if err != nil {
return fmt.Errorf("decode v6 overlay address: %w", err) return fmt.Errorf("decode v6 overlay address: %w", err)
} }
if !prefix.Addr().Is6() {
return fmt.Errorf("expected IPv6 address, got %s", prefix.Addr())
}
addr.IPv6 = prefix.Addr() addr.IPv6 = prefix.Addr()
addr.IPv6Net = prefix.Masked() addr.IPv6Net = prefix.Masked()
return nil return nil
@@ -78,7 +81,7 @@ func (addr *Address) SetIPv6FromCompact(raw []byte) error {
// ClearIPv6 removes the IPv6 overlay address, leaving only v4. // ClearIPv6 removes the IPv6 overlay address, leaving only v4.
// //
//nolint:recvcheck // ClearIPv6 is the only mutating method on this otherwise value-type struct. //nolint:recvcheck
func (addr *Address) ClearIPv6() { func (addr *Address) ClearIPv6() {
addr.IPv6 = netip.Addr{} addr.IPv6 = netip.Addr{}
addr.IPv6Net = netip.Prefix{} addr.IPv6Net = netip.Prefix{}
+12 -13
View File
@@ -1035,22 +1035,24 @@ func (e *Engine) updateConfig(conf *mgmProto.PeerConfig) error {
} }
// hasIPv6Changed reports whether the IPv6 overlay address in the peer config // 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 { func (e *Engine) hasIPv6Changed(conf *mgmProto.PeerConfig) bool {
current := e.wgInterface.Address() current := e.config.WgAddr
raw := conf.GetAddressV6() raw := conf.GetAddressV6()
if len(raw) == 0 { if len(raw) == 0 {
return current.HasIPv6() return current.HasIPv6()
} }
addr, err := netiputil.DecodeAddr(raw) prefix, err := netiputil.DecodePrefix(raw)
if err != nil { if err != nil {
log.Warnf("decode v6 overlay address: %v", err) log.Warnf("decode v6 overlay address: %v", err)
return false return false
} }
return !current.HasIPv6() || current.IPv6 != addr return !current.HasIPv6() || current.IPv6 != prefix.Addr() || current.IPv6Net != prefix.Masked()
} }
func (e *Engine) receiveJobEvents() { func (e *Engine) receiveJobEvents() {
@@ -1540,20 +1542,17 @@ func (e *Engine) addNewPeer(peerConfig *mgmProto.RemotePeerConfig) error {
peerIPs = append(peerIPs, allowedNetIP) 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) conn, err := e.createPeerConn(peerKey, peerIPs, peerConfig.AgentVersion)
if err != nil { if err != nil {
return fmt.Errorf("create peer connection: %w", err) return fmt.Errorf("create peer connection: %w", err)
} }
var peerIPv6 string peerV4, peerV6 := splitAllowedIPs(peerConfig.GetAllowedIps(), e.wgInterface.Address().IPv6Net)
ourV6Net := e.wgInterface.Address().IPv6Net err = e.statusRecorder.AddPeer(peerKey, peerConfig.Fqdn, peerV4, peerV6)
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)
if err != nil { if err != nil {
log.Warnf("error adding peer %s to status recorder, got error: %v", peerKey, err) log.Warnf("error adding peer %s to status recorder, got error: %v", peerKey, err)
} }
+10 -6
View File
@@ -1728,7 +1728,7 @@ func TestEngine_hasIPv6Changed(t *testing.T) {
{ {
name: "no v6 before, v6 added", name: "no v6 before, v6 added",
current: v4Only, current: v4Only,
confV6: netiputil.EncodeAddr(netip.MustParseAddr("fd00::1")), confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/64")),
expected: true, expected: true,
}, },
{ {
@@ -1740,13 +1740,19 @@ func TestEngine_hasIPv6Changed(t *testing.T) {
{ {
name: "had v6, same v6", name: "had v6, same v6",
current: v4v6, current: v4v6,
confV6: netiputil.EncodeAddr(netip.MustParseAddr("fd00::1")), confV6: netiputil.EncodePrefix(netip.MustParsePrefix("fd00::1/64")),
expected: false, expected: false,
}, },
{ {
name: "had v6, different v6", name: "had v6, different v6",
current: v4v6, 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, expected: true,
}, },
{ {
@@ -1760,9 +1766,7 @@ func TestEngine_hasIPv6Changed(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
engine := &Engine{ engine := &Engine{
wgInterface: &MockWGIface{ config: &EngineConfig{WgAddr: tt.current},
AddressFunc: func() wgaddr.Address { return tt.current },
},
} }
conf := &mgmtProto.PeerConfig{ conf := &mgmtProto.PeerConfig{
AddressV6: tt.confV6, AddressV6: tt.confV6,