Revert "[client] Skip engine restart when default route is unchanged"

This reverts commit a071b55f35.

client/internal/engine.go

@@ -82,12 +82,6 @@ const (
 	PeerConnectionTimeoutMax = 45000 // ms
 	PeerConnectionTimeoutMin = 30000 // ms
 	disableAutoUpdate        = "disabled"
-
-	// systemInfoTimeout bounds how long the sync loop waits for system info / posture
-	// check gathering. The gathering runs uncancellable system calls (process scan,
-	// exec, os.Stat); without this bound a single stuck call freezes handleSync, and
-	// thus syncMsgMux, for as long as the call hangs (observed multi-minute freezes).
-	systemInfoTimeout = 15 * time.Second
 )
 
 var ErrResetConnection = fmt.Errorf("reset connection")
@@ -1072,22 +1066,11 @@ func (e *Engine) updateChecksIfNew(checks []*mgmProto.Checks) error {
 	}
 	e.checks = checks
 
-	info, ok := system.GetInfoWithChecksTimeout(e.ctx, systemInfoTimeout, checks)
-	if !ok {
-		// Gathering timed out; skip the meta sync this cycle rather than blocking the
-		// sync loop (and syncMsgMux) on a stuck system call. A later sync will retry.
-		return nil
+	info, err := system.GetInfoWithChecks(e.ctx, checks, e.overlayAddresses()...)
+	if err != nil {
+		log.Warnf("failed to get system info with checks: %v", err)
+		info = system.GetInfo(e.ctx)
 	}
-	e.applyInfoFlags(info)
-
-	if err := e.mgmClient.SyncMeta(info); err != nil {
-		return fmt.Errorf("could not sync meta: error %s", err)
-	}
-	return nil
-}
-
-// applyInfoFlags sets the engine's config-derived feature flags on the gathered system info.
-func (e *Engine) applyInfoFlags(info *system.Info) {
 	info.SetFlags(
 		e.config.RosenpassEnabled,
 		e.config.RosenpassPermissive,
@@ -1106,6 +1089,26 @@ func (e *Engine) applyInfoFlags(info *system.Info) {
 		e.config.EnableSSHRemotePortForwarding,
 		e.config.DisableSSHAuth,
 	)
+
+	if err := e.mgmClient.SyncMeta(info); err != nil {
+		log.Errorf("could not sync meta: error %s", err)
+		return err
+	}
+	return nil
+}
+
+// overlayAddresses returns our own WireGuard overlay address (v4 and v6) so it
+// can be excluded from the reported network addresses; the interface coming and
+// going otherwise churns the peer meta on the management server.
+func (e *Engine) overlayAddresses() []netip.Addr {
+	var ips []netip.Addr
+	if e.config.WgAddr.IP.IsValid() {
+		ips = append(ips, e.config.WgAddr.IP)
+	}
+	if e.config.WgAddr.HasIPv6() {
+		ips = append(ips, e.config.WgAddr.IPv6)
+	}
+	return ips
 }
 
 func (e *Engine) updateConfig(conf *mgmProto.PeerConfig) error {
@@ -1251,15 +1254,31 @@ func (e *Engine) receiveManagementEvents() {
 	e.shutdownWg.Add(1)
 	go func() {
 		defer e.shutdownWg.Done()
-		info, ok := system.GetInfoWithChecksTimeout(e.ctx, systemInfoTimeout, e.checks)
-		if !ok {
-			// Gathering timed out; connect the stream with base info so management
-			// connectivity still comes up rather than blocking here.
+		info, err := system.GetInfoWithChecks(e.ctx, e.checks, e.overlayAddresses()...)
+		if err != nil {
+			log.Warnf("failed to get system info with checks: %v", err)
 			info = system.GetInfo(e.ctx)
 		}
-		e.applyInfoFlags(info)
+		info.SetFlags(
+			e.config.RosenpassEnabled,
+			e.config.RosenpassPermissive,
+			&e.config.ServerSSHAllowed,
+			e.config.DisableClientRoutes,
+			e.config.DisableServerRoutes,
+			e.config.DisableDNS,
+			e.config.DisableFirewall,
+			e.config.BlockLANAccess,
+			e.config.BlockInbound,
+			e.config.DisableIPv6,
+			e.config.LazyConnectionEnabled,
+			e.config.EnableSSHRoot,
+			e.config.EnableSSHSFTP,
+			e.config.EnableSSHLocalPortForwarding,
+			e.config.EnableSSHRemotePortForwarding,
+			e.config.DisableSSHAuth,
+		)
 
-		err := e.mgmClient.Sync(e.ctx, info, e.handleSync)
+		err = e.mgmClient.Sync(e.ctx, info, e.handleSync)
 		if err != nil {
 			// happens if management is unavailable for a long time.
 			// We want to cancel the operation of the whole client

client/system/info.go

@@ -2,10 +2,9 @@ package system
 
 import (
 	"context"
-	"errors"
 	"net/netip"
+	"slices"
 	"strings"
-	"time"
 
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/metadata"
@@ -123,6 +122,23 @@ func (i *Info) SetFlags(
 	}
 }
 
+// removeAddresses drops network addresses whose IP matches any of the given
+// addresses, regardless of prefix length. Used to exclude the NetBird overlay
+// address, which otherwise churns the meta as the interface comes and goes.
+func (i *Info) removeAddresses(ips ...netip.Addr) {
+	if len(ips) == 0 {
+		return
+	}
+	filtered := i.NetworkAddresses[:0]
+	for _, addr := range i.NetworkAddresses {
+		if slices.Contains(ips, addr.NetIP.Addr()) {
+			continue
+		}
+		filtered = append(filtered, addr)
+	}
+	i.NetworkAddresses = filtered
+}
+
 // extractUserAgent extracts Netbird's agent (client) name and version from the outgoing context
 func extractUserAgent(ctx context.Context) string {
 	md, hasMeta := metadata.FromOutgoingContext(ctx)
@@ -149,14 +165,16 @@ func extractDeviceName(ctx context.Context, defaultName string) string {
 }
 
 // GetInfoWithChecks retrieves and parses the system information with applied checks.
-func GetInfoWithChecks(ctx context.Context, checks []*proto.Checks) (*Info, error) {
+// excludeIPs are dropped from the reported network addresses (e.g. our own
+// WireGuard overlay address, which otherwise churns the peer meta).
+func GetInfoWithChecks(ctx context.Context, checks []*proto.Checks, excludeIPs ...netip.Addr) (*Info, error) {
 	log.Debugf("gathering system information with checks: %d", len(checks))
 	processCheckPaths := make([]string, 0)
 	for _, check := range checks {
 		processCheckPaths = append(processCheckPaths, check.GetFiles()...)
 	}
 
-	files, err := checkFileAndProcess(ctx, processCheckPaths)
+	files, err := checkFileAndProcess(processCheckPaths)
 	if err != nil {
 		return nil, err
 	}
@@ -164,43 +182,8 @@ func GetInfoWithChecks(ctx context.Context, checks []*proto.Checks) (*Info, erro
 
 	info := GetInfo(ctx)
 	info.Files = files
+	info.removeAddresses(excludeIPs...)
 
 	log.Debugf("all system information gathered successfully")
 	return info, nil
 }
-
-// GetInfoWithChecksTimeout is GetInfoWithChecks bounded by timeout. Posture-check gathering
-// runs uncancellable system calls (process enumeration, os.Stat), so calling it inline can
-// block the caller for as long as such a call hangs. It runs in a goroutine instead: if it
-// does not return within timeout the caller gets (nil, false) and should proceed with
-// degraded behavior rather than block. On a gathering error it falls back to base GetInfo.
-//
-// The buffered channel lets the abandoned goroutine finish and exit once its blocking call
-// returns, so it does not leak beyond the duration of that call.
-func GetInfoWithChecksTimeout(ctx context.Context, timeout time.Duration, checks []*proto.Checks) (*Info, bool) {
-	ctx, cancel := context.WithTimeout(ctx, timeout)
-	defer cancel()
-
-	infoCh := make(chan *Info, 1)
-	go func() {
-		info, err := GetInfoWithChecks(ctx, checks)
-		if err != nil {
-			log.Warnf("failed to get system info with checks: %v", err)
-			info = GetInfo(ctx)
-		}
-		infoCh <- info
-	}()
-
-	select {
-	case info := <-infoCh:
-		return info, true
-	case <-ctx.Done():
-		if errors.Is(ctx.Err(), context.DeadlineExceeded) {
-			log.Warnf("gathering system info with checks timed out after %s", timeout)
-		} else {
-			// Parent context canceled (e.g. shutdown), not a timeout.
-			log.Warnf("gathering system info with checks canceled: %v", ctx.Err())
-		}
-		return nil, false
-	}
-}

client/system/info_android.go

@@ -50,7 +50,7 @@ func GetInfo(ctx context.Context) *Info {
 }
 
 // checkFileAndProcess checks if the file path exists and if a process is running at that path.
-func checkFileAndProcess(_ context.Context, _ []string) ([]File, error) {
+func checkFileAndProcess(paths []string) ([]File, error) {
 	return []File{}, nil
 }
 

client/system/info_darwin.go

@@ -32,7 +32,7 @@ func GetInfo(ctx context.Context) *Info {
 	sysName := string(bytes.Split(utsname.Sysname[:], []byte{0})[0])
 	machine := string(bytes.Split(utsname.Machine[:], []byte{0})[0])
 	release := string(bytes.Split(utsname.Release[:], []byte{0})[0])
-	swVersion, err := exec.CommandContext(ctx, "sw_vers", "-productVersion").Output()
+	swVersion, err := exec.Command("sw_vers", "-productVersion").Output()
 	if err != nil {
 		log.Warnf("got an error while retrieving macOS version with sw_vers, error: %s. Using darwin version instead.\n", err)
 		swVersion = []byte(release)

client/system/info_ios.go

@@ -105,7 +105,7 @@ func isDuplicated(addresses []NetworkAddress, addr NetworkAddress) bool {
 }
 
 // checkFileAndProcess checks if the file path exists and if a process is running at that path.
-func checkFileAndProcess(_ context.Context, _ []string) ([]File, error) {
+func checkFileAndProcess(paths []string) ([]File, error) {
 	return []File{}, nil
 }
 

client/system/info_js.go

@@ -103,7 +103,7 @@ func collectLocationInfo(info *Info) {
 	}
 }
 
-func checkFileAndProcess(_ context.Context, _ []string) ([]File, error) {
+func checkFileAndProcess(_ []string) ([]File, error) {
 	return []File{}, nil
 }
 

client/system/info_test.go

@@ -2,8 +2,8 @@ package system
 
 import (
 	"context"
+	"net/netip"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"google.golang.org/grpc/metadata"
@@ -35,20 +35,6 @@ func Test_CustomHostname(t *testing.T) {
 	assert.Equal(t, want, got.Hostname)
 }
 
-func TestGetInfoWithChecksTimeout_Success(t *testing.T) {
-	info, ok := GetInfoWithChecksTimeout(context.Background(), 30*time.Second, nil)
-	assert.True(t, ok, "expected gathering to complete within the timeout")
-	assert.NotNil(t, info)
-}
-
-func TestGetInfoWithChecksTimeout_Timeout(t *testing.T) {
-	// A 1ns budget expires before the (real) system-info gathering can finish, so the
-	// caller must get (nil, false) instead of blocking on the in-flight goroutine.
-	info, ok := GetInfoWithChecksTimeout(context.Background(), time.Nanosecond, nil)
-	assert.False(t, ok, "expected timeout to be reported")
-	assert.Nil(t, info)
-}
-
 func Test_NetAddresses(t *testing.T) {
 	addr, err := networkAddresses()
 	if err != nil {
@@ -58,3 +44,42 @@ func Test_NetAddresses(t *testing.T) {
 		t.Errorf("no network addresses found")
 	}
 }
+
+func TestInfo_RemoveAddresses(t *testing.T) {
+	addr := func(cidr string) NetworkAddress {
+		return NetworkAddress{NetIP: netip.MustParsePrefix(cidr)}
+	}
+
+	info := &Info{
+		NetworkAddresses: []NetworkAddress{
+			addr("192.168.1.7/24"),
+			addr("100.76.70.97/32"),                          // overlay v4 (host mask /32)
+			addr("2001:818:c51b:4800:845:a65d:ae6f:623f/64"), // real global v6
+			addr("fd00:1234::1/64"),                          // overlay v6
+		},
+	}
+
+	// Overlay addresses as the engine knows them, with a different mask (/16, /64).
+	info.removeAddresses(
+		netip.MustParseAddr("100.76.70.97"),
+		netip.MustParseAddr("fd00:1234::1"),
+	)
+
+	want := []string{"192.168.1.7/24", "2001:818:c51b:4800:845:a65d:ae6f:623f/64"}
+	if len(info.NetworkAddresses) != len(want) {
+		t.Fatalf("got %d addresses, want %d: %v", len(info.NetworkAddresses), len(want), info.NetworkAddresses)
+	}
+	for i, w := range want {
+		if got := info.NetworkAddresses[i].NetIP.String(); got != w {
+			t.Errorf("address[%d] = %s, want %s", i, got, w)
+		}
+	}
+}
+
+func TestInfo_RemoveAddresses_NoOp(t *testing.T) {
+	info := &Info{NetworkAddresses: []NetworkAddress{{NetIP: netip.MustParsePrefix("10.0.0.1/24")}}}
+	info.removeAddresses()
+	if len(info.NetworkAddresses) != 1 {
+		t.Errorf("expected no change with empty input, got %v", info.NetworkAddresses)
+	}
+}

client/system/network_addr.go

@@ -46,7 +46,9 @@ func toNetworkAddress(address net.Addr, mac string) (NetworkAddress, bool) {
 	if !ok {
 		return NetworkAddress{}, false
 	}
-	if ipNet.IP.IsLoopback() {
+	// Skip link-local and multicast: they carry no routable peer info and the
+	// IPv6 link-local of a flapping NIC churns the meta on every up/down.
+	if ipNet.IP.IsLoopback() || ipNet.IP.IsLinkLocalUnicast() || ipNet.IP.IsMulticast() {
 		return NetworkAddress{}, false
 	}
 	prefix, err := netip.ParsePrefix(ipNet.String())

client/system/network_addr_test.go

@@ -0,0 +1,45 @@
+//go:build !ios
+
+package system
+
+import (
+	"net"
+	"testing"
+)
+
+func mustIPNet(t *testing.T, cidr string) *net.IPNet {
+	t.Helper()
+	ip, ipNet, err := net.ParseCIDR(cidr)
+	if err != nil {
+		t.Fatalf("parse %q: %v", cidr, err)
+	}
+	ipNet.IP = ip
+	return ipNet
+}
+
+func TestToNetworkAddress_Filtering(t *testing.T) {
+	const mac = "c8:4b:d6:b6:04:ac"
+
+	tests := []struct {
+		name string
+		cidr string
+		want bool
+	}{
+		{"ipv4 global", "10.65.16.181/23", true},
+		{"ipv6 global", "2620:52:0:4110:102d:6a98:ee75:8b92/64", true},
+		{"ipv4 loopback", "127.0.0.1/8", false},
+		{"ipv6 loopback", "::1/128", false},
+		{"ipv6 link-local", "fe80::871:4c25:23d7:2529/64", false},
+		{"ipv4 link-local", "169.254.1.2/16", false},
+		{"ipv6 multicast", "ff02::1/128", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, got := toNetworkAddress(mustIPNet(t, tt.cidr), mac)
+			if got != tt.want {
+				t.Errorf("toNetworkAddress(%s) ok = %v, want %v", tt.cidr, got, tt.want)
+			}
+		})
+	}
+}

client/system/process.go

@@ -3,30 +3,24 @@
 package system
 
 import (
-	"context"
 	"os"
 	"slices"
 
 	"github.com/shirou/gopsutil/v3/process"
 )
 
-// getRunningProcesses returns a list of running process paths. The context bounds the work:
-// the per-PID loop bails as soon as ctx is done, and the gopsutil calls honor it where they
-// can, so a stuck enumeration cannot run unbounded.
-func getRunningProcesses(ctx context.Context) ([]string, error) {
-	processIDs, err := process.PidsWithContext(ctx)
+// getRunningProcesses returns a list of running process paths.
+func getRunningProcesses() ([]string, error) {
+	processIDs, err := process.Pids()
 	if err != nil {
 		return nil, err
 	}
 
 	processMap := make(map[string]bool)
 	for _, pID := range processIDs {
-		if err := ctx.Err(); err != nil {
-			return nil, err
-		}
 		p := &process.Process{Pid: pID}
 
-		path, _ := p.ExeWithContext(ctx)
+		path, _ := p.Exe()
 		if path != "" {
 			processMap[path] = false
 		}
@@ -41,21 +35,18 @@ func getRunningProcesses(ctx context.Context) ([]string, error) {
 }
 
 // checkFileAndProcess checks if the file path exists and if a process is running at that path.
-func checkFileAndProcess(ctx context.Context, paths []string) ([]File, error) {
+func checkFileAndProcess(paths []string) ([]File, error) {
 	files := make([]File, len(paths))
 	if len(paths) == 0 {
 		return files, nil
 	}
 
-	runningProcesses, err := getRunningProcesses(ctx)
+	runningProcesses, err := getRunningProcesses()
 	if err != nil {
 		return nil, err
 	}
 
 	for i, path := range paths {
-		if err := ctx.Err(); err != nil {
-			return nil, err
-		}
 		file := File{Path: path}
 
 		_, err := os.Stat(path)

client/system/process_test.go

@@ -1,7 +1,6 @@
 package system
 
 import (
-	"context"
 	"testing"
 
 	"github.com/shirou/gopsutil/v3/process"
@@ -10,7 +9,7 @@ import (
 func Benchmark_getRunningProcesses(b *testing.B) {
 	b.Run("getRunningProcesses new", func(b *testing.B) {
 		for i := 0; i < b.N; i++ {
-			ps, err := getRunningProcesses(context.Background())
+			ps, err := getRunningProcesses()
 			if err != nil {
 				b.Fatalf("unexpected error: %v", err)
 			}
@@ -30,38 +29,12 @@ func Benchmark_getRunningProcesses(b *testing.B) {
 			}
 		}
 	})
-	s, _ := getRunningProcesses(context.Background())
+	s, _ := getRunningProcesses()
 	b.Logf("getRunningProcesses returned %d processes", len(s))
 	s, _ = getRunningProcessesOld()
 	b.Logf("getRunningProcessesOld returned %d processes", len(s))
 }
 
-func TestCheckFileAndProcess_ContextCanceled(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel()
-
-	// With a canceled context and non-empty paths the gathering must bail with an error
-	// instead of running the (potentially blocking) process scan / stat loop.
-	if _, err := checkFileAndProcess(ctx, []string{"/does/not/exist"}); err == nil {
-		t.Fatal("expected error on canceled context, got nil")
-	}
-}
-
-func TestCheckFileAndProcess_EmptyPaths(t *testing.T) {
-	// No check paths means no work to do: it must return immediately with no error,
-	// even on a canceled context (nothing to scan or stat).
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel()
-
-	files, err := checkFileAndProcess(ctx, nil)
-	if err != nil {
-		t.Fatalf("unexpected error for empty paths: %v", err)
-	}
-	if len(files) != 0 {
-		t.Fatalf("expected no files, got %d", len(files))
-	}
-}
-
 func getRunningProcessesOld() ([]string, error) {
 	processes, err := process.Processes()
 	if err != nil {

management/server/peer.go

@@ -1170,7 +1170,7 @@ func (am *DefaultAccountManager) LoginPeer(ctx context.Context, login types.Peer
 	}
 
 	// This is needed to keep in memory for the peer config. Otherwise browser client will end in a retry loop
-	peer.Meta = login.Meta
+	peer.UpdateMetaIfNew(ctx, login.Meta)
 
 	peerGroupIDs, err = getPeerGroupIDs(ctx, am.Store, accountID, peer.ID)
 	if err != nil {