From 38d7b35c8e6f6b673d96a6948bcb443095c643ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Wed, 23 Sep 2026 10:37:02 +0200 Subject: [PATCH] [client] Reset the updater mode on engine stop Removing SetDownloadOnly from the engine teardown also dropped the reset of the forced-update flag. After netbird down the manager kept the last management directive, so a release published while the client was stopped could still trigger a forced install that management no longer asked for. The same window existed between an engine teardown and the next reconnect attempt, where ResetMode only ran at the start of the attempt. Reset the mode in stopLocked instead, so the updater stays silent from the moment the engine goes down until the first network map of the next connection decides the mode again. --- client/internal/engine.go | 4 ++ client/internal/updater/manager_mode_test.go | 37 +++++++++++++++++++ .../updater/manager_test_helpers_test.go | 36 ++++++++++++++++-- 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/client/internal/engine.go b/client/internal/engine.go index 44bed1e65..f3774e70f 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -455,6 +455,10 @@ func (e *Engine) stopLocked() { e.sessionWatcher.Close() } + if e.updateManager != nil { + e.updateManager.ResetMode() + } + log.Info("cleaning up status recorder states") e.statusRecorder.ReplaceOfflinePeers([]peer.State{}) e.statusRecorder.UpdateDNSStates([]peer.NSGroupState{}) diff --git a/client/internal/updater/manager_mode_test.go b/client/internal/updater/manager_mode_test.go index b7955becc..feca68f0c 100644 --- a/client/internal/updater/manager_mode_test.go +++ b/client/internal/updater/manager_mode_test.go @@ -115,3 +115,40 @@ func Test_SetDownloadOnly_ClearsPendingVersion(t *testing.T) { t.Fatal("Install in download-only mode must not install the staged managed version") } } + +func Test_ResetMode_DropsForceUpdate(t *testing.T) { + tmpFile := path.Join(t.TempDir(), "update-test-force-reset.json") + recorder := peer.NewRecorder("") + sub := recorder.SubscribeToEvents() + defer recorder.UnsubscribeFromEvents(sub) + + mockUpdate := &versionUpdateMock{} + m := NewManager(recorder, statemanager.New(tmpFile)) + m.update = mockUpdate + m.currentVersion = "1.0.0" + m.autoUpdateSupported = func() bool { return true } + m.Start(context.Background()) + defer m.Stop() + + // Management enforces "latest" before the fetcher has reported any version, + // so nothing can be installed while the engine is still up. + m.SetVersion(latestVersion, true) + if event := waitForAnyEvent(sub, 300*time.Millisecond); event != nil { + t.Fatalf("no event expected before the latest version is known, got %v", event) + } + + // The engine stop resets the mode. A release published afterwards must not + // trigger the stale forced install or any notification. + m.ResetMode() + mockUpdate.setLatestVersion(v.Must(v.NewSemver("1.0.1"))) + mockUpdate.onUpdate() + if event := waitForAnyEvent(sub, 300*time.Millisecond); event != nil { + t.Fatalf("stale force directive must stay silent after reset, got %v", event) + } + + m.SetVersion("1.0.1", false) + ver, enforced := waitForUpdateEvent(sub, 500*time.Millisecond) + if ver != "1.0.1" || !enforced { + t.Fatalf("expected enforced event after a fresh directive, got %q enforced=%v", ver, enforced) + } +} diff --git a/client/internal/updater/manager_test_helpers_test.go b/client/internal/updater/manager_test_helpers_test.go index c7faee1f4..430b31ec1 100644 --- a/client/internal/updater/manager_test_helpers_test.go +++ b/client/internal/updater/manager_test_helpers_test.go @@ -2,21 +2,24 @@ package updater import ( "strconv" + "sync" "time" v "github.com/hashicorp/go-version" "github.com/netbirdio/netbird/client/internal/peer" + cProto "github.com/netbirdio/netbird/client/proto" ) type versionUpdateMock struct { latestVersion *v.Version onUpdate func() + mu sync.Mutex } -func (m versionUpdateMock) StopWatch() {} +func (m *versionUpdateMock) StopWatch() {} -func (m versionUpdateMock) SetDaemonVersion(newVersion string) bool { +func (m *versionUpdateMock) SetDaemonVersion(newVersion string) bool { return false } @@ -24,11 +27,19 @@ func (m *versionUpdateMock) SetOnUpdateListener(updateFn func()) { m.onUpdate = updateFn } -func (m versionUpdateMock) LatestVersion() *v.Version { +func (m *versionUpdateMock) LatestVersion() *v.Version { + m.mu.Lock() + defer m.mu.Unlock() return m.latestVersion } -func (m versionUpdateMock) StartFetcher() {} +func (m *versionUpdateMock) StartFetcher() {} + +func (m *versionUpdateMock) setLatestVersion(version *v.Version) { + m.mu.Lock() + defer m.mu.Unlock() + m.latestVersion = version +} // waitForUpdateEvent waits for a new_version_available event, returns the version string or "" on timeout. func waitForUpdateEvent(sub *peer.EventSubscription, timeout time.Duration) (version string, enforced bool) { @@ -54,3 +65,20 @@ func waitForUpdateEvent(sub *peer.EventSubscription, timeout time.Duration) (ver } } } + +// waitForAnyEvent returns the first published event of any kind, or nil on timeout. +// Unlike waitForUpdateEvent it also catches the install-progress events, so a test +// can assert that a forced install never started. +func waitForAnyEvent(sub *peer.EventSubscription, timeout time.Duration) *cProto.SystemEvent { + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case event, ok := <-sub.Events(): + if !ok { + return nil + } + return event + case <-timer.C: + return nil + } +}