CodeRabbit Generated Unit Tests: Add unit tests for PR changes

client/internal/routemanager/systemops/systemops_darwin_test.go

@@ -0,0 +1,125 @@
+//go:build darwin && !ios
+
+package systemops
+
+import (
+	"net/netip"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	nbnet "github.com/netbirdio/netbird/client/net"
+)
+
+// TestAfOf verifies that afOf returns the correct string for each address family.
+func TestAfOf(t *testing.T) {
+	tests := []struct {
+		name string
+		addr netip.Addr
+		want string
+	}{
+		{
+			name: "IPv4 unspecified",
+			addr: netip.IPv4Unspecified(),
+			want: "IPv4",
+		},
+		{
+			name: "IPv4 private",
+			addr: netip.MustParseAddr("10.0.0.1"),
+			want: "IPv4",
+		},
+		{
+			name: "IPv4 loopback",
+			addr: netip.MustParseAddr("127.0.0.1"),
+			want: "IPv4",
+		},
+		{
+			name: "IPv6 unspecified",
+			addr: netip.IPv6Unspecified(),
+			want: "IPv6",
+		},
+		{
+			name: "IPv6 loopback",
+			addr: netip.MustParseAddr("::1"),
+			want: "IPv6",
+		},
+		{
+			name: "IPv6 unicast",
+			addr: netip.MustParseAddr("2001:db8::1"),
+			want: "IPv6",
+		},
+		{
+			name: "IPv6 link-local",
+			addr: netip.MustParseAddr("fe80::1"),
+			want: "IPv6",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, afOf(tt.addr))
+		})
+	}
+}
+
+// TestIsAddrRouted_AdvancedRoutingBypassesTunnelLookup verifies that when
+// AdvancedRouting is active, IsAddrRouted immediately returns (false, zero)
+// regardless of the provided vpn routes, because the WG socket is bound to
+// the physical interface via IP_BOUND_IF and bypasses the main routing table.
+func TestIsAddrRouted_AdvancedRoutingBypassesTunnelLookup(t *testing.T) {
+	// On darwin, AdvancedRouting returns true unless overridden.
+	// Ensure we reset the state after the test.
+	t.Setenv("NB_USE_LEGACY_ROUTING", "false")
+	t.Setenv("NB_USE_NETSTACK_MODE", "false")
+	nbnet.Init()
+
+	require.True(t, nbnet.AdvancedRouting(), "test requires advanced routing to be active on darwin")
+
+	vpnRoutes := []netip.Prefix{
+		netip.MustParsePrefix("10.0.0.0/8"),
+		netip.MustParsePrefix("192.168.1.0/24"),
+		netip.MustParsePrefix("0.0.0.0/0"),
+	}
+
+	tests := []struct {
+		name string
+		addr netip.Addr
+	}{
+		{"IPv4 in VPN route", netip.MustParseAddr("10.0.0.1")},
+		{"IPv4 in narrow VPN route", netip.MustParseAddr("192.168.1.100")},
+		{"IPv4 default route covered", netip.MustParseAddr("8.8.8.8")},
+		{"IPv6 in VPN route", netip.MustParseAddr("2001:db8::1")},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			routed, prefix := IsAddrRouted(tt.addr, vpnRoutes)
+			assert.False(t, routed, "should not be marked as routed via VPN when advanced routing is active")
+			assert.Equal(t, netip.Prefix{}, prefix, "matched prefix should be zero when advanced routing is active")
+		})
+	}
+}
+
+// TestIsAddrRouted_LegacyModeFallsThroughToTable verifies that when
+// NB_USE_LEGACY_ROUTING=true disables advanced routing, IsAddrRouted
+// performs the normal VPN-route vs local-route comparison.
+func TestIsAddrRouted_LegacyModeFallsThroughToTable(t *testing.T) {
+	t.Setenv("NB_USE_LEGACY_ROUTING", "true")
+	nbnet.Init()
+
+	require.False(t, nbnet.AdvancedRouting(), "test requires advanced routing to be disabled")
+
+	// Use an address that is very unlikely to exist in the host routing table
+	// as a local route, so the VPN route wins.
+	vpnRoutes := []netip.Prefix{
+		netip.MustParsePrefix("198.51.100.0/24"), // TEST-NET-2 – not in normal routing tables
+	}
+
+	addr := netip.MustParseAddr("198.51.100.1")
+	routed, _ := IsAddrRouted(addr, vpnRoutes)
+	// We cannot assert a specific outcome because it depends on the host's
+	// routing table, but we CAN assert that the call did not panic and returned
+	// a consistent pair.
+	_ = routed
+}

client/internal/routemanager/systemops/systemops_isaddrrouted_test.go

@@ -0,0 +1,140 @@
+//go:build !android && !ios
+
+package systemops
+
+import (
+	"net/netip"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	nbnet "github.com/netbirdio/netbird/client/net"
+)
+
+// withLegacyRouting forcibly puts the nbnet package into legacy (non-advanced)
+// routing mode for the duration of the test, then restores the previous value.
+func withLegacyRouting(t *testing.T) {
+	t.Helper()
+	t.Setenv("NB_USE_LEGACY_ROUTING", "true")
+	t.Setenv("NB_USE_NETSTACK_MODE", "false")
+	nbnet.Init()
+	t.Cleanup(func() {
+		// After the test, re-initialise with no overrides so that subsequent
+		// tests start with a clean state.
+		nbnet.Init()
+	})
+}
+
+// withAdvancedRouting attempts to enable advanced routing for the test.
+// On platforms where Init() cannot produce AdvancedRouting()=true (e.g. Linux
+// without root), the calling test is skipped.
+func withAdvancedRouting(t *testing.T) {
+	t.Helper()
+	t.Setenv("NB_USE_LEGACY_ROUTING", "false")
+	t.Setenv("NB_USE_NETSTACK_MODE", "false")
+	nbnet.Init()
+	t.Cleanup(func() {
+		nbnet.Init()
+	})
+	if !nbnet.AdvancedRouting() {
+		t.Skip("advanced routing not available in this environment (need root or darwin)")
+	}
+}
+
+// TestIsAddrRouted_AdvancedRoutingShortCircuit verifies that when
+// AdvancedRouting() returns true, IsAddrRouted immediately returns
+// (false, zero prefix) regardless of any VPN routes, because the WG socket is
+// bound directly to the physical interface and bypasses the kernel routing table.
+func TestIsAddrRouted_AdvancedRoutingShortCircuit(t *testing.T) {
+	withAdvancedRouting(t)
+
+	vpnRoutes := []netip.Prefix{
+		netip.MustParsePrefix("10.0.0.0/8"),
+		netip.MustParsePrefix("192.168.0.0/16"),
+		netip.MustParsePrefix("0.0.0.0/0"),
+		netip.MustParsePrefix("::/0"),
+	}
+
+	tests := []struct {
+		name string
+		addr string
+	}{
+		{"IPv4 matched by 10/8", "10.0.0.1"},
+		{"IPv4 matched by 192.168/16", "192.168.1.1"},
+		{"IPv4 caught by default", "8.8.8.8"},
+		{"IPv6 caught by default", "2001:db8::1"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			addr := netip.MustParseAddr(tt.addr)
+			routed, prefix := IsAddrRouted(addr, vpnRoutes)
+			assert.False(t, routed, "advanced routing must short-circuit VPN route check")
+			assert.Equal(t, netip.Prefix{}, prefix, "returned prefix must be zero under advanced routing")
+		})
+	}
+}
+
+// TestIsAddrRouted_AdvancedRouting_EmptyRoutes verifies the short-circuit with
+// an empty vpnRoutes slice — the result must still be (false, zero).
+func TestIsAddrRouted_AdvancedRouting_EmptyRoutes(t *testing.T) {
+	withAdvancedRouting(t)
+
+	routed, prefix := IsAddrRouted(netip.MustParseAddr("10.0.0.1"), nil)
+	assert.False(t, routed)
+	assert.Equal(t, netip.Prefix{}, prefix)
+}
+
+// TestIsAddrRouted_LegacyMode_NonVPNAddress verifies that under legacy routing
+// an address that is not covered by any VPN prefix returns false.
+func TestIsAddrRouted_LegacyMode_NonVPNAddress(t *testing.T) {
+	withLegacyRouting(t)
+
+	// 198.51.100.x (TEST-NET-2) is not normally in any routing table.
+	vpnRoutes := []netip.Prefix{
+		netip.MustParsePrefix("10.0.0.0/8"),
+	}
+
+	addr := netip.MustParseAddr("198.51.100.1")
+	routed, _ := IsAddrRouted(addr, vpnRoutes)
+	assert.False(t, routed, "address not in VPN routes should not be marked as VPN-routed")
+}
+
+// TestIsAddrRouted_LegacyMode_EmptyVPNRoutes verifies that an empty vpnRoutes
+// slice always yields (false, zero prefix) even under legacy routing.
+func TestIsAddrRouted_LegacyMode_EmptyVPNRoutes(t *testing.T) {
+	withLegacyRouting(t)
+
+	routed, prefix := IsAddrRouted(netip.MustParseAddr("10.0.0.1"), nil)
+	assert.False(t, routed)
+	assert.Equal(t, netip.Prefix{}, prefix)
+}
+
+// TestIsAddrRouted_AdvancedVsLegacy_ContrastiveBehaviour documents the
+// contract difference between the two modes: with a VPN default route and an
+// address that matches it, legacy mode may mark it as VPN-routed while advanced
+// mode must never do so.
+func TestIsAddrRouted_AdvancedVsLegacy_ContrastiveBehaviour(t *testing.T) {
+	vpnRoutes := []netip.Prefix{
+		netip.MustParsePrefix("0.0.0.0/0"),
+	}
+	addr := netip.MustParseAddr("8.8.8.8")
+
+	// --- advanced routing: must always return false ---
+	t.Run("advanced routing", func(t *testing.T) {
+		withAdvancedRouting(t)
+		routed, prefix := IsAddrRouted(addr, vpnRoutes)
+		assert.False(t, routed, "advanced routing must bypass VPN route lookup")
+		assert.Equal(t, netip.Prefix{}, prefix)
+	})
+
+	// --- legacy routing: delegates to kernel table check, does not panic ---
+	t.Run("legacy routing", func(t *testing.T) {
+		withLegacyRouting(t)
+		// We don't assert true/false here because it depends on the host
+		// routing table, but the call must not panic and must return
+		// a valid (bool, prefix) pair.
+		routed, prefix := IsAddrRouted(addr, vpnRoutes)
+		t.Logf("legacy IsAddrRouted(%s, %v) = (%v, %v)", addr, vpnRoutes, routed, prefix)
+	})
+}

client/net/env_bound_iface_test.go

@@ -0,0 +1,121 @@
+//go:build (darwin && !ios) || windows
+
+package net
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// resetAdvancedRoutingState resets the package-level advancedRoutingSupported variable
+// and cleans up after the test.
+func resetAdvancedRoutingState(t *testing.T) {
+	t.Helper()
+	orig := advancedRoutingSupported
+	t.Cleanup(func() { advancedRoutingSupported = orig })
+}
+
+// TestCheckAdvancedRoutingSupport_LegacyRoutingTrue verifies that setting
+// NB_USE_LEGACY_ROUTING=true disables advanced routing.
+func TestCheckAdvancedRoutingSupport_LegacyRoutingTrue(t *testing.T) {
+	t.Setenv(envUseLegacyRouting, "true")
+	assert.False(t, checkAdvancedRoutingSupport())
+}
+
+// TestCheckAdvancedRoutingSupport_LegacyRoutingFalse verifies that
+// NB_USE_LEGACY_ROUTING=false still allows advanced routing when netstack is off.
+func TestCheckAdvancedRoutingSupport_LegacyRoutingFalse(t *testing.T) {
+	t.Setenv(envUseLegacyRouting, "false")
+	t.Setenv("NB_USE_NETSTACK_MODE", "false")
+	assert.True(t, checkAdvancedRoutingSupport())
+}
+
+// TestCheckAdvancedRoutingSupport_LegacyRoutingInvalid verifies that an invalid
+// value for NB_USE_LEGACY_ROUTING is ignored (treated as false), so advanced
+// routing remains enabled when netstack is off.
+func TestCheckAdvancedRoutingSupport_LegacyRoutingInvalid(t *testing.T) {
+	t.Setenv(envUseLegacyRouting, "notabool")
+	t.Setenv("NB_USE_NETSTACK_MODE", "false")
+	// The invalid value is ignored; the default (false) is kept, so advanced routing
+	// is not suppressed by the legacy-routing flag.
+	assert.True(t, checkAdvancedRoutingSupport())
+}
+
+// TestCheckAdvancedRoutingSupport_NetstackEnabled verifies that netstack mode
+// disables advanced routing.
+func TestCheckAdvancedRoutingSupport_NetstackEnabled(t *testing.T) {
+	t.Setenv(envUseLegacyRouting, "false")
+	t.Setenv("NB_USE_NETSTACK_MODE", "true")
+	assert.False(t, checkAdvancedRoutingSupport())
+}
+
+// TestCheckAdvancedRoutingSupport_NoEnvVars verifies that with no env overrides
+// advanced routing is supported (the happy path).
+func TestCheckAdvancedRoutingSupport_NoEnvVars(t *testing.T) {
+	// Unset both controlling variables so we hit the default path.
+	t.Setenv(envUseLegacyRouting, "")
+	t.Setenv("NB_USE_NETSTACK_MODE", "false")
+	assert.True(t, checkAdvancedRoutingSupport())
+}
+
+// TestCheckAdvancedRoutingSupport_LegacyRoutingEmptyString verifies that an
+// empty NB_USE_LEGACY_ROUTING is treated as "not set" and does not disable
+// advanced routing.
+func TestCheckAdvancedRoutingSupport_LegacyRoutingEmptyString(t *testing.T) {
+	t.Setenv(envUseLegacyRouting, "")
+	t.Setenv("NB_USE_NETSTACK_MODE", "false")
+	assert.True(t, checkAdvancedRoutingSupport())
+}
+
+// TestAdvancedRouting_ReflectsInit verifies that after calling Init() with
+// NB_USE_LEGACY_ROUTING=true, AdvancedRouting() returns false.
+func TestAdvancedRouting_ReflectsInit(t *testing.T) {
+	resetAdvancedRoutingState(t)
+
+	t.Setenv(envUseLegacyRouting, "true")
+	Init()
+
+	assert.False(t, AdvancedRouting(), "AdvancedRouting should return false after Init with legacy routing")
+}
+
+// TestAdvancedRouting_ReflectsInit_Advanced verifies that after calling Init()
+// without legacy overrides, AdvancedRouting() returns true.
+func TestAdvancedRouting_ReflectsInit_Advanced(t *testing.T) {
+	resetAdvancedRoutingState(t)
+
+	t.Setenv(envUseLegacyRouting, "false")
+	t.Setenv("NB_USE_NETSTACK_MODE", "false")
+	Init()
+
+	assert.True(t, AdvancedRouting(), "AdvancedRouting should return true after Init without legacy overrides")
+}
+
+// TestSetAndGetVPNInterfaceName verifies SetVPNInterfaceName and GetVPNInterfaceName
+// are consistent.
+func TestSetAndGetVPNInterfaceName(t *testing.T) {
+	orig := GetVPNInterfaceName()
+	t.Cleanup(func() { SetVPNInterfaceName(orig) })
+
+	SetVPNInterfaceName("utun3")
+	assert.Equal(t, "utun3", GetVPNInterfaceName())
+}
+
+// TestSetVPNInterfaceName_Empty verifies that setting an empty name is accepted.
+func TestSetVPNInterfaceName_Empty(t *testing.T) {
+	orig := GetVPNInterfaceName()
+	t.Cleanup(func() { SetVPNInterfaceName(orig) })
+
+	SetVPNInterfaceName("")
+	assert.Equal(t, "", GetVPNInterfaceName())
+}
+
+// TestSetVPNInterfaceName_OverwritesPrevious verifies that the second call wins.
+func TestSetVPNInterfaceName_OverwritesPrevious(t *testing.T) {
+	orig := GetVPNInterfaceName()
+	t.Cleanup(func() { SetVPNInterfaceName(orig) })
+
+	SetVPNInterfaceName("utun1")
+	SetVPNInterfaceName("utun9")
+	assert.Equal(t, "utun9", GetVPNInterfaceName())
+}