Merge branch 'main' into fix/pkce-flow-session-extend

management.pb.go conflicted because both sides added to it: main gave
ResourceCompact an id and GroupCompact a resources list, this branch added
PKCEAuthorizationFlowRequest.SessionExtend. The .proto merged cleanly, so
the generated file was regenerated from it rather than resolved by hand.

client/internal/dns_test.go

@@ -8,7 +8,9 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/netbirdio/netbird/client/iface/wgaddr"
 	nbdns "github.com/netbirdio/netbird/dns"
+	mgmProto "github.com/netbirdio/netbird/shared/management/proto"
 )
 
 func TestCreatePTRRecord_IPv4(t *testing.T) {
@@ -136,3 +138,88 @@ func TestAddReverseZone_IPv6(t *testing.T) {
 	assert.Len(t, reverseZone.Records, 1)
 	assert.Equal(t, int(dns.TypePTR), reverseZone.Records[0].Type)
 }
+
+// TestToDNSConfig_ZoneFlagsPreserved pins the per-zone NonAuthoritative flag
+// through the legacy DNSConfig path. A non-authoritative zone is match-only:
+// the local resolver falls through to the upstream for an in-zone name it does
+// not define. The built-in peer zone is the authoritative one and must stay
+// that way, so the flag has to travel per zone rather than be derived.
+func TestToDNSConfig_ZoneFlagsPreserved(t *testing.T) {
+	config := toDNSConfig(&mgmProto.DNSConfig{
+		ServiceEnable: true,
+		CustomZones: []*mgmProto.CustomZone{
+			{
+				Domain: "netbird.cloud.",
+				Records: []*mgmProto.SimpleRecord{
+					{Name: "peer1.netbird.cloud.", Type: int64(dns.TypeA), Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.1"},
+				},
+			},
+			{
+				Domain:               "corp.internal.",
+				NonAuthoritative:     true,
+				SearchDomainDisabled: true,
+				Records: []*mgmProto.SimpleRecord{
+					{Name: "db.corp.internal.", Type: int64(dns.TypeA), Class: nbdns.DefaultClass, TTL: 300, RData: "10.10.0.5"},
+				},
+			},
+		},
+	}, wgaddr.Address{
+		IP:      netip.MustParseAddr("100.64.0.1"),
+		Network: netip.MustParsePrefix("100.64.0.0/16"),
+	})
+
+	zones := make(map[string]nbdns.CustomZone, len(config.CustomZones))
+	for _, zone := range config.CustomZones {
+		zones[zone.Domain] = zone
+	}
+
+	peerZone, ok := zones["netbird.cloud."]
+	require.True(t, ok, "peer zone must survive")
+	assert.False(t, peerZone.NonAuthoritative, "the built-in peer zone owns the account domain and stays authoritative")
+
+	accountZone, ok := zones["corp.internal."]
+	require.True(t, ok, "account zone must survive")
+	assert.True(t, accountZone.NonAuthoritative, "an account zone stays match-only, else undefined in-zone names get black-holed")
+	assert.True(t, accountZone.SearchDomainDisabled)
+}
+
+// TestToDNSConfig_SingleZoneForcedAuthoritative pins the compatibility clause
+// in toDNSConfig: a config carrying exactly one zone is treated as
+// authoritative no matter what the server said, because servers that predate
+// the NonAuthoritative field send only the peer FQDN zone.
+//
+// The clause can only ever downgrade an explicit true to false, so a server
+// that legitimately sends a single non-authoritative zone — an account whose
+// only zone is a custom one, with no peer records to build the built-in zone
+// from — gets that zone's whole apex black-holed on the client. Real accounts
+// always carry the peer zone alongside, which is why this is latent. Narrowing
+// it needs a way to tell "unset" from "false" on the wire, or the account
+// domain passed down here; until then this test states the contract so a
+// change to it is deliberate.
+func TestToDNSConfig_SingleZoneForcedAuthoritative(t *testing.T) {
+	config := toDNSConfig(&mgmProto.DNSConfig{
+		ServiceEnable: true,
+		CustomZones: []*mgmProto.CustomZone{
+			{
+				Domain:           "corp.internal.",
+				NonAuthoritative: true,
+				Records: []*mgmProto.SimpleRecord{
+					{Name: "db.corp.internal.", Type: int64(dns.TypeA), Class: nbdns.DefaultClass, TTL: 300, RData: "10.10.0.5"},
+				},
+			},
+		},
+	}, wgaddr.Address{
+		IP:      netip.MustParseAddr("100.64.0.1"),
+		Network: netip.MustParsePrefix("100.64.0.0/16"),
+	})
+
+	require.NotEmpty(t, config.CustomZones)
+	assert.Equal(t, "corp.internal.", config.CustomZones[0].Domain)
+	assert.False(t, config.CustomZones[0].NonAuthoritative,
+		"a lone zone is forced authoritative for pre-NonAuthoritative servers")
+
+	// The reverse zone the config gains afterwards must not feed back into the
+	// decision: the compat gate counts the zones the server sent.
+	require.Len(t, config.CustomZones, 2, "a reverse zone is appended for the overlay prefix")
+	assert.Equal(t, "64.100.in-addr.arpa.", config.CustomZones[1].Domain)
+}

client/internal/engine_privileged_test.go

@@ -519,7 +519,7 @@ func startManagement(t *testing.T, dataDir, testFile string) (*grpc.Server, stri
 
 	updateManager := update_channel.NewPeersUpdateManager(metrics)
 	requestBuffer := server.NewAccountRequestBuffer(context.Background(), store)
-	networkMapController := controller.NewController(context.Background(), store, metrics, updateManager, requestBuffer, server.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", port_forwarding.NewControllerMock(), manager.NewEphemeralManager(store, peersManager), config)
+	networkMapController := controller.NewController(context.Background(), store, metrics, updateManager, requestBuffer, server.MockIntegratedValidator{}, settingsMockManager, "netbird.selfhosted", port_forwarding.NewControllerMock(), manager.NewEphemeralManager(store, peersManager), config, nil)
 	accountManager, err := server.BuildManager(context.Background(), config, store, networkMapController, jobManager, nil, "", eventStore, nil, false, ia, metrics, port_forwarding.NewControllerMock(), settingsMockManager, permissionsManager, false, cacheStore)
 	if err != nil {
 		return nil, "", err

client/internal/updater/installer/doc.go

@@ -109,6 +109,10 @@
 //   - Does NOT remove result.json (cleaned by ResultHandler after read)
 //   - Does NOT remove msi.log (kept for debugging)
 //
+// On Windows the updater copy is often still locked when the daemon it restarted
+// runs cleanup, so removing it is retried briefly and otherwise left in place for
+// the next update to overwrite rather than reported as a failure.
+//
 // # Dry-Run Mode
 //
 // Dry-run mode allows testing the update process without actually installing:

client/internal/updater/installer/installer_cleanup_windows_test.go

@@ -0,0 +1,67 @@
+package installer
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"golang.org/x/sys/windows"
+)
+
+// lockFile opens path without FILE_SHARE_DELETE, so os.Remove fails the way it does
+// while the updater process still holds its own image.
+func lockFile(t *testing.T, path string) windows.Handle {
+	t.Helper()
+
+	p, err := windows.UTF16PtrFromString(path)
+	if err != nil {
+		t.Fatalf("convert path: %v", err)
+	}
+
+	handle, err := windows.CreateFile(p, windows.GENERIC_READ, windows.FILE_SHARE_READ, nil, windows.OPEN_EXISTING, windows.FILE_ATTRIBUTE_NORMAL, 0)
+	if err != nil {
+		t.Fatalf("lock %s: %v", path, err)
+	}
+	return handle
+}
+
+// releaseAfter closes the handle once the delay has passed, standing in for the
+// updater process finally exiting.
+func releaseAfter(t *testing.T, handle windows.Handle, delay time.Duration) {
+	t.Helper()
+
+	released := make(chan struct{})
+	t.Cleanup(func() { <-released })
+
+	go func() {
+		defer close(released)
+		time.Sleep(delay)
+		if err := windows.CloseHandle(handle); err != nil {
+			t.Errorf("close handle: %v", err)
+		}
+	}()
+}
+
+// TestCleanUpInstallerFilesLockedUpdater covers the post-update cleanup race: the
+// daemon cleans up at startup while the updater that restarted it is still exiting,
+// so the updater image is locked and Windows refuses the delete. Cleanup must wait
+// the lock out instead of reporting a failure and leaving the binary behind.
+func TestCleanUpInstallerFilesLockedUpdater(t *testing.T) {
+	tempDir := t.TempDir()
+	path := filepath.Join(tempDir, updaterBinary)
+	if err := os.WriteFile(path, []byte("x"), 0o600); err != nil {
+		t.Fatalf("write updater: %v", err)
+	}
+
+	releaseAfter(t, lockFile(t, path), 300*time.Millisecond)
+
+	u := NewWithDir(tempDir)
+	if err := u.CleanUpInstallerFiles(); err != nil {
+		t.Fatalf("cleanup must tolerate a still-locked updater: %v", err)
+	}
+
+	if _, err := os.Stat(path); !os.IsNotExist(err) {
+		t.Errorf("updater binary still present (stat err: %v)", err)
+	}
+}

client/internal/updater/installer/installer_common.go

@@ -152,8 +152,8 @@ func (u *Installer) CleanUpInstallerFiles() error {
 
 	var merr *multierror.Error
 
-	if err := os.Remove(filepath.Join(u.tempDir, updaterBinary)); err != nil && !os.IsNotExist(err) {
-		merr = multierror.Append(merr, fmt.Errorf("failed to remove updater binary: %w", err))
+	if err := removeUpdaterBinary(filepath.Join(u.tempDir, updaterBinary)); err != nil {
+		merr = multierror.Append(merr, fmt.Errorf("remove updater binary: %w", err))
 	}
 
 	entries, err := os.ReadDir(u.tempDir)
@@ -167,10 +167,16 @@ func (u *Installer) CleanUpInstallerFiles() error {
 		}
 
 		name := entry.Name()
+		// The updater copy is handled above; on Windows its name also matches the
+		// extension sweep, which would report the same file twice.
+		if strings.EqualFold(name, updaterBinary) {
+			continue
+		}
+
 		for _, ext := range binaryExtensions {
 			if strings.HasSuffix(strings.ToLower(name), strings.ToLower(ext)) {
 				if err := os.Remove(filepath.Join(u.tempDir, name)); err != nil {
-					merr = multierror.Append(merr, fmt.Errorf("failed to remove %s: %w", name, err))
+					merr = multierror.Append(merr, fmt.Errorf("remove %s: %w", name, err))
 				}
 				break
 			}

client/internal/updater/installer/installer_common_test.go

@@ -0,0 +1,52 @@
+//go:build windows || darwin
+
+package installer
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// TestCleanUpInstallerFiles checks that cleanup removes the updater copy and the
+// downloaded installer while leaving the logs and the result file for the daemon.
+func TestCleanUpInstallerFiles(t *testing.T) {
+	tempDir := t.TempDir()
+
+	installers := make([]string, 0, len(binaryExtensions))
+	for _, ext := range binaryExtensions {
+		installers = append(installers, "netbird_installer."+ext)
+	}
+
+	kept := []string{"installer.log", "result.json"}
+
+	for _, name := range append(append([]string{updaterBinary}, installers...), kept...) {
+		if err := os.WriteFile(filepath.Join(tempDir, name), []byte("x"), 0o600); err != nil {
+			t.Fatalf("write %s: %v", name, err)
+		}
+	}
+
+	u := NewWithDir(tempDir)
+	if err := u.CleanUpInstallerFiles(); err != nil {
+		t.Fatalf("CleanUpInstallerFiles: %v", err)
+	}
+
+	for _, name := range append([]string{updaterBinary}, installers...) {
+		if _, err := os.Stat(filepath.Join(tempDir, name)); !os.IsNotExist(err) {
+			t.Errorf("%s was not removed (stat err: %v)", name, err)
+		}
+	}
+
+	for _, name := range kept {
+		if _, err := os.Stat(filepath.Join(tempDir, name)); err != nil {
+			t.Errorf("%s should have been kept: %v", name, err)
+		}
+	}
+}
+
+func TestCleanUpInstallerFilesMissingTempDir(t *testing.T) {
+	u := NewWithDir(filepath.Join(t.TempDir(), "does-not-exist"))
+	if err := u.CleanUpInstallerFiles(); err != nil {
+		t.Errorf("a missing temp dir is not a cleanup failure, got: %v", err)
+	}
+}

client/internal/updater/installer/remove_updater_darwin.go

@@ -0,0 +1,12 @@
+package installer
+
+import "os"
+
+// removeUpdaterBinary deletes the updater copy left in the temp dir. On darwin a
+// running binary can be unlinked, so no retry is needed.
+func removeUpdaterBinary(path string) error {
+	if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	return nil
+}

client/internal/updater/installer/remove_updater_windows.go

@@ -0,0 +1,45 @@
+package installer
+
+import (
+	"errors"
+	"os"
+	"time"
+
+	log "github.com/sirupsen/logrus"
+	"golang.org/x/sys/windows"
+)
+
+const (
+	// The updater is the process that restarted the daemon, so when the daemon
+	// cleans up at startup the updater is often still exiting and Windows refuses
+	// to delete its locked image. These bound how long cleanup waits for it.
+	updaterRemoveAttempts = 5
+	updaterRemoveDelay    = 200 * time.Millisecond
+)
+
+// removeUpdaterBinary deletes the updater copy left in the temp dir, retrying
+// while the still-exiting updater process holds its image. A binary that stays
+// locked for the whole window is left in place and reported at info level: the
+// next update overwrites it, so it is not worth failing cleanup over.
+func removeUpdaterBinary(path string) error {
+	for attempt := 0; attempt < updaterRemoveAttempts; attempt++ {
+		if attempt > 0 {
+			time.Sleep(updaterRemoveDelay)
+		}
+
+		err := os.Remove(path)
+		if err == nil || os.IsNotExist(err) {
+			return nil
+		}
+		if !isFileLocked(err) {
+			return err
+		}
+	}
+
+	log.Infof("updater binary %s is still locked, leaving it for the next update to overwrite", path)
+	return nil
+}
+
+func isFileLocked(err error) bool {
+	return errors.Is(err, windows.ERROR_ACCESS_DENIED) || errors.Is(err, windows.ERROR_SHARING_VIOLATION)
+}

client/internal/updater/installer/remove_updater_windows_test.go

@@ -0,0 +1,59 @@
+package installer
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"golang.org/x/sys/windows"
+)
+
+func TestRemoveUpdaterBinaryRetriesWhileLocked(t *testing.T) {
+	path := filepath.Join(t.TempDir(), updaterBinary)
+	if err := os.WriteFile(path, []byte("x"), 0o600); err != nil {
+		t.Fatalf("write updater: %v", err)
+	}
+
+	releaseAfter(t, lockFile(t, path), updaterRemoveDelay+50*time.Millisecond)
+
+	if err := removeUpdaterBinary(path); err != nil {
+		t.Fatalf("removeUpdaterBinary: %v", err)
+	}
+
+	if _, err := os.Stat(path); !os.IsNotExist(err) {
+		t.Errorf("updater binary still present (stat err: %v)", err)
+	}
+}
+
+// TestRemoveUpdaterBinaryStaysLocked covers an updater that never releases its
+// image within the retry window. Cleanup gives up quietly and leaves the file
+// behind rather than reporting a failure.
+func TestRemoveUpdaterBinaryStaysLocked(t *testing.T) {
+	path := filepath.Join(t.TempDir(), updaterBinary)
+	if err := os.WriteFile(path, []byte("x"), 0o600); err != nil {
+		t.Fatalf("write updater: %v", err)
+	}
+
+	handle := lockFile(t, path)
+	t.Cleanup(func() {
+		if err := windows.CloseHandle(handle); err != nil {
+			t.Errorf("close handle: %v", err)
+		}
+	})
+
+	if err := removeUpdaterBinary(path); err != nil {
+		t.Fatalf("a permanently locked updater is not a cleanup failure, got: %v", err)
+	}
+
+	if _, err := os.Stat(path); err != nil {
+		t.Errorf("locked updater binary should be left in place, stat: %v", err)
+	}
+}
+
+func TestRemoveUpdaterBinaryMissingFile(t *testing.T) {
+	path := filepath.Join(t.TempDir(), updaterBinary)
+	if err := removeUpdaterBinary(path); err != nil {
+		t.Errorf("a missing updater binary is not a failure, got: %v", err)
+	}
+}