From 8f64173574eebc51198ec1c805095b01c3259dba Mon Sep 17 00:00:00 2001 From: Maycon Santos Date: Mon, 13 Jul 2026 13:39:57 +0200 Subject: [PATCH] [client] Enable launch-on-login by default on fresh GUI installs (#6738) * [client] Add autostart preference marker and MDM disableAutostart key Adds the autostartInitialized marker to the Wails UI preferences store so the one-time autostart default decision can persist per OS user, and a UI-only disableAutostart MDM policy key that suppresses the default and flows into GetConfigResponse.mDMManagedFields like disableAutoConnect. * [client] Enable launch-on-login by default on fresh GUI installs On the first interactive run the GUI persists the autostartInitialized marker before any enable attempt, then enables autostart only when the platform supports it, MDM policy does not disable it, the process was not relaunched by an installer/updater (--post-update), and the installer's fresh-install breadcrumb is present. Upgrading users have no breadcrumb, so an update can never write login items, and a user's disable in Settings is never overridden. * [release] Write fresh-install breadcrumb from installers Installers write a .fresh-install breadcrumb on fresh installs only and delete stale breadcrumbs on upgrade; none of them writes login items or registry Run keys. Windows NSIS detects upgrades via the uninstall registry entry or an existing installed executable; the macOS pkg via the previous pkgutil receipt; Linux deb/rpm via the standard postinstall arguments. Post-update GUI relaunches (macOS open, Linux ui-post-install.sh) pass --post-update so the first-run autostart default cannot fire on updates. * Revert installer breadcrumb changes The real Windows installer does uninstall-then-install and deletes $INSTDIR, so a breadcrumb written there cannot survive or discriminate a fresh install from an upgrade. Restore the three installer files to their main versions; no installer or updater writes an autostart entry. * Detect fresh install from NetBird footprint instead of installer breadcrumb Replace the installer-written breadcrumb discriminator with a GUI-side check. netbirdFootprintExists inspects the daemon config/state files (default.json, legacy config.json, state.json) under profilemanager's default config dir; combined with whether the UI preferences file already existed, this tells a genuinely fresh machine from an existing or upgrading user. Only the signed GUI, via Wails, ever enables launch-on-login, and a user's later manual disable is never overridden. The preferences store now exposes ExistedAtLoad and the --post-update flag is dropped. * Update tests for footprint-based autostart default Table tests for shouldEnableAutostartDefault now cover supported, mdmDisabled, and priorInstall guards plus precedence; breadcrumb and post-update cases are removed. Add a store test asserting ExistedAtLoad is false with no file and true after persisting and reopening. --- client/mdm/canonical_loaders.go | 1 + client/mdm/policy.go | 22 +++-- client/ui/autostart_default.go | 107 ++++++++++++++++++++++++ client/ui/autostart_default_test.go | 125 ++++++++++++++++++++++++++++ client/ui/main.go | 3 + client/ui/preferences/store.go | 50 ++++++++++- client/ui/preferences/store_test.go | 40 +++++++++ client/ui/services/settings.go | 3 +- 8 files changed, 338 insertions(+), 13 deletions(-) create mode 100644 client/ui/autostart_default.go create mode 100644 client/ui/autostart_default_test.go diff --git a/client/mdm/canonical_loaders.go b/client/mdm/canonical_loaders.go index cb9af9ccb..29288b511 100644 --- a/client/mdm/canonical_loaders.go +++ b/client/mdm/canonical_loaders.go @@ -22,6 +22,7 @@ var allKeys = []string{ KeyDisableMetricsCollection, KeyAllowServerSSH, KeyDisableAutoConnect, + KeyDisableAutostart, KeyPreSharedKey, KeyRosenpassEnabled, KeyRosenpassPermissive, diff --git a/client/mdm/policy.go b/client/mdm/policy.go index b76c70a75..1feff28f8 100644 --- a/client/mdm/policy.go +++ b/client/mdm/policy.go @@ -20,10 +20,10 @@ import ( // names (lowerCamelCase) so the daemon can map a Policy key directly to a // configuration field. const ( - KeyManagementURL = "managementURL" - KeyDisableUpdateSettings = "disableUpdateSettings" - KeyDisableProfiles = "disableProfiles" - KeyDisableNetworks = "disableNetworks" + KeyManagementURL = "managementURL" + KeyDisableUpdateSettings = "disableUpdateSettings" + KeyDisableProfiles = "disableProfiles" + KeyDisableNetworks = "disableNetworks" // KeyDisableAdvancedView gates the advanced-view section in the // upcoming UI revision. UI-only: NOT stored on Config, not // applied by applyMDMPolicy, not rejectable via SetConfig. The @@ -37,10 +37,16 @@ const ( KeyDisableMetricsCollection = "disableMetricsCollection" KeyAllowServerSSH = "allowServerSSH" KeyDisableAutoConnect = "disableAutoConnect" - KeyPreSharedKey = "preSharedKey" - KeyRosenpassEnabled = "rosenpassEnabled" - KeyRosenpassPermissive = "rosenpassPermissive" - KeyWireguardPort = "wireguardPort" + // KeyDisableAutostart suppresses the GUI's fresh-install + // launch-on-login default and marks the Settings toggle as + // MDM-managed. UI-only: NOT stored on Config and not applied by + // applyMDMPolicy; the GUI reads it directly and it appears in + // GetConfigResponse.mDMManagedFields when set. + KeyDisableAutostart = "disableAutostart" + KeyPreSharedKey = "preSharedKey" + KeyRosenpassEnabled = "rosenpassEnabled" + KeyRosenpassPermissive = "rosenpassPermissive" + KeyWireguardPort = "wireguardPort" // Split tunnel is modeled as a single conceptual policy with two // registry/plist values. KeySplitTunnelMode is the discriminator diff --git a/client/ui/autostart_default.go b/client/ui/autostart_default.go new file mode 100644 index 000000000..bf1b16a97 --- /dev/null +++ b/client/ui/autostart_default.go @@ -0,0 +1,107 @@ +//go:build !android && !ios && !freebsd && !js + +package main + +import ( + "context" + "os" + "path/filepath" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/internal/profilemanager" + "github.com/netbirdio/netbird/client/mdm" + "github.com/netbirdio/netbird/client/ui/preferences" + "github.com/netbirdio/netbird/client/ui/services" +) + +// autostartDefaultState carries the guard inputs of the one-time autostart +// default decision so the decision itself stays a pure, testable function. +type autostartDefaultState struct { + supported bool + mdmDisabled bool + priorInstall bool +} + +// shouldEnableAutostartDefault applies the first-run guards in order and +// returns whether autostart may be enabled, plus the reason when it may not. +func shouldEnableAutostartDefault(s autostartDefaultState) (bool, string) { + switch { + case !s.supported: + return false, "autostart not supported on this platform" + case s.mdmDisabled: + return false, "autostart disabled by MDM policy" + case s.priorInstall: + return false, "existing NetBird installation" + } + return true, "" +} + +// autostartDisabledByMDM reports whether the MDM policy manages the +// disableAutostart key in a way that must suppress the default. An +// unparseable managed value is treated as disabled to stay on the safe side. +func autostartDisabledByMDM(policy *mdm.Policy) bool { + if !policy.HasKey(mdm.KeyDisableAutostart) { + return false + } + disabled, ok := policy.GetBool(mdm.KeyDisableAutostart) + return !ok || disabled +} + +// netbirdFootprintExists reports whether the machine already carries NetBird +// daemon config or state, meaning this is not a genuinely fresh install. It is +// the update-safety gate for the autostart default: upgrading users always +// have a footprint, so an update can never trigger a login-item write. +func netbirdFootprintExists() bool { + candidates := []string{ + profilemanager.DefaultConfigPath, + filepath.Join(profilemanager.DefaultConfigPathDir, "config.json"), + filepath.Join(profilemanager.DefaultConfigPathDir, "state.json"), + } + for _, path := range candidates { + if path != "" && fileExists(path) { + return true + } + } + return false +} + +// applyAutostartDefault runs the one-time launch-on-login default for genuinely +// fresh installs. The autostartInitialized marker is persisted before any +// enable attempt so a crash mid-flow degrades to "never enabled" instead of +// retrying login-item writes on every launch. A user's later disable in +// Settings is never overridden: the marker guarantees at-most-once, ever. +func applyAutostartDefault(ctx context.Context, autostart *services.Autostart, prefs *preferences.Store, prefsFileExisted bool) { + priorFootprint := netbirdFootprintExists() || prefsFileExisted + + if prefs.Get().AutostartInitialized { + return + } + if err := prefs.SetAutostartInitialized(true); err != nil { + log.Warnf("persist autostart marker, skipping autostart default: %v", err) + return + } + + state := autostartDefaultState{ + supported: autostart.Supported(ctx), + mdmDisabled: autostartDisabledByMDM(mdm.LoadPolicy()), + priorInstall: priorFootprint, + } + enable, reason := shouldEnableAutostartDefault(state) + if !enable { + log.Debugf("skipping autostart default: %s", reason) + return + } + + if err := autostart.SetEnabled(ctx, true); err != nil { + log.Warnf("enable autostart on fresh install: %v", err) + return + } + log.Info("autostart enabled by default on fresh install") +} + +// fileExists reports whether path exists. +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} diff --git a/client/ui/autostart_default_test.go b/client/ui/autostart_default_test.go new file mode 100644 index 000000000..b7bdf9f2a --- /dev/null +++ b/client/ui/autostart_default_test.go @@ -0,0 +1,125 @@ +//go:build !android && !ios && !freebsd && !js + +package main + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/netbirdio/netbird/client/mdm" +) + +func TestShouldEnableAutostartDefault(t *testing.T) { + allPass := autostartDefaultState{ + supported: true, + mdmDisabled: false, + priorInstall: false, + } + + tests := []struct { + name string + mutate func(*autostartDefaultState) + wantEnable bool + wantReason string + }{ + { + name: "fresh install with all guards passing enables", + mutate: func(*autostartDefaultState) {}, + wantEnable: true, + }, + { + name: "unsupported platform skips", + mutate: func(s *autostartDefaultState) { s.supported = false }, + wantReason: "autostart not supported on this platform", + }, + { + name: "MDM disable skips", + mutate: func(s *autostartDefaultState) { s.mdmDisabled = true }, + wantReason: "autostart disabled by MDM policy", + }, + { + name: "existing installation (upgrade) skips", + mutate: func(s *autostartDefaultState) { s.priorInstall = true }, + wantReason: "existing NetBird installation", + }, + { + name: "unsupported wins over every other guard", + mutate: func(s *autostartDefaultState) { + s.supported = false + s.mdmDisabled = true + s.priorInstall = true + }, + wantReason: "autostart not supported on this platform", + }, + { + name: "MDM disable wins over prior install", + mutate: func(s *autostartDefaultState) { + s.mdmDisabled = true + s.priorInstall = true + }, + wantReason: "autostart disabled by MDM policy", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + state := allPass + tc.mutate(&state) + enable, reason := shouldEnableAutostartDefault(state) + assert.Equal(t, tc.wantEnable, enable, "enable decision should match for state %+v", state) + assert.Equal(t, tc.wantReason, reason, "skip reason should identify the failing guard") + }) + } +} + +func TestAutostartDisabledByMDM(t *testing.T) { + tests := []struct { + name string + values map[string]any + want bool + }{ + { + name: "empty policy does not disable", + values: nil, + want: false, + }, + { + name: "unrelated managed keys do not disable", + values: map[string]any{mdm.KeyDisableAutoConnect: true}, + want: false, + }, + { + name: "disableAutostart true disables", + values: map[string]any{mdm.KeyDisableAutostart: true}, + want: true, + }, + { + name: "disableAutostart registry DWORD 1 disables", + values: map[string]any{mdm.KeyDisableAutostart: int64(1)}, + want: true, + }, + { + name: "disableAutostart string true disables", + values: map[string]any{mdm.KeyDisableAutostart: "true"}, + want: true, + }, + { + name: "disableAutostart explicit false allows", + values: map[string]any{mdm.KeyDisableAutostart: false}, + want: false, + }, + { + name: "unparseable managed value is treated as disabled", + values: map[string]any{mdm.KeyDisableAutostart: "not-a-bool"}, + want: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := autostartDisabledByMDM(mdm.NewPolicy(tc.values)) + assert.Equal(t, tc.want, got, "MDM disable decision should match for values %v", tc.values) + }) + } +} diff --git a/client/ui/main.go b/client/ui/main.go index e6b77762c..4889bad79 100644 --- a/client/ui/main.go +++ b/client/ui/main.go @@ -197,6 +197,9 @@ func main() { // daemon may keep the main window from showing, so the OS toast is the // only reliable signal the user gets. go notifyIfDaemonOutdated(compat, notifier, localizer) + // One-time launch-on-login default for fresh installs; gated by the + // NetBird footprint check, MDM policy, and the persisted marker. + go applyAutostartDefault(context.Background(), services.NewAutostart(app.Autostart), prefStore, prefStore.ExistedAtLoad()) }) if err := app.Run(); err != nil { diff --git a/client/ui/preferences/store.go b/client/ui/preferences/store.go index afc854185..df6fbbb16 100644 --- a/client/ui/preferences/store.go +++ b/client/ui/preferences/store.go @@ -54,6 +54,10 @@ type UIPreferences struct { Language i18n.LanguageCode `json:"language"` ViewMode ViewMode `json:"viewMode"` OnboardingCompleted bool `json:"onboardingCompleted"` + // AutostartInitialized records that the one-time autostart default + // decision has run for this OS user. It only ever transitions to true + // and is never reset, so the default-on flow runs at most once, ever. + AutostartInitialized bool `json:"autostartInitialized"` } // LanguageValidator rejects SetLanguage inputs with no shipped bundle. @@ -72,8 +76,9 @@ type Emitter interface { type Store struct { path string - mu sync.RWMutex - current UIPreferences + mu sync.RWMutex + current UIPreferences + existedAtLoad bool subsMu sync.Mutex subs []chan UIPreferences @@ -157,6 +162,27 @@ func (s *Store) SetOnboardingCompleted(done bool) error { return nil } +// SetAutostartInitialized persists the one-time autostart decision marker. +// No-op if unchanged. +func (s *Store) SetAutostartInitialized(done bool) error { + s.mu.Lock() + if s.current.AutostartInitialized == done { + s.mu.Unlock() + return nil + } + next := s.current + next.AutostartInitialized = done + if err := s.persistLocked(next); err != nil { + s.mu.Unlock() + return fmt.Errorf("persist preferences: %w", err) + } + s.current = next + s.mu.Unlock() + + s.broadcast(next) + return nil +} + // SetLanguage validates, persists, and broadcasts. No-op if unchanged. func (s *Store) SetLanguage(lang i18n.LanguageCode) error { if lang == "" { @@ -206,13 +232,29 @@ func (s *Store) Subscribe() (<-chan UIPreferences, func()) { return ch, unsubscribe } +// ExistedAtLoad reports whether the backing preferences file was present on +// disk when the store loaded. It distinguishes a user who ran a prior GUI +// version from a brand-new OS user with no preferences yet. +func (s *Store) ExistedAtLoad() bool { + s.mu.RLock() + defer s.mu.RUnlock() + return s.existedAtLoad +} + // load reads the file into current. A missing file is not an error (the // in-memory default stands); malformed contents return an error. func (s *Store) load() error { - if _, err := os.Stat(s.path); errors.Is(err, os.ErrNotExist) { - return nil + if _, err := os.Stat(s.path); err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil + } + return fmt.Errorf("stat preferences: %w", err) } + s.mu.Lock() + s.existedAtLoad = true + s.mu.Unlock() + var loaded UIPreferences if _, err := util.ReadJson(s.path, &loaded); err != nil { return err diff --git a/client/ui/preferences/store_test.go b/client/ui/preferences/store_test.go index 0d1cc7b54..6384fddb8 100644 --- a/client/ui/preferences/store_test.go +++ b/client/ui/preferences/store_test.go @@ -215,6 +215,46 @@ func TestStore_FileShapeIsJSON(t *testing.T) { assert.Equal(t, i18n.LanguageCode("hu"), parsed.Language) } +func TestStore_SetAutostartInitializedPersistsAcrossReload(t *testing.T) { + withTempConfigDir(t) + emitter := &recordingEmitter{} + s, err := NewStore(nil, emitter) + require.NoError(t, err) + + assert.False(t, s.Get().AutostartInitialized, "marker must default to false when no file is on disk") + + require.NoError(t, s.SetAutostartInitialized(true)) + assert.True(t, s.Get().AutostartInitialized, "Get should reflect the persisted marker") + require.Len(t, emitter.calledWith(EventPreferencesChanged), 1, "first marker write should broadcast") + + // Re-setting the same value must be a no-op: no disk write, no broadcast. + require.NoError(t, s.SetAutostartInitialized(true)) + assert.Len(t, emitter.calledWith(EventPreferencesChanged), 1, "idempotent marker write should not broadcast again") + + // A fresh Store (new GUI launch) must see the marker so the autostart + // default decision never runs twice. + reloaded, err := NewStore(nil, nil) + require.NoError(t, err) + assert.True(t, reloaded.Get().AutostartInitialized, "marker must survive a reload from disk") +} + +func TestStore_ExistedAtLoad(t *testing.T) { + withTempConfigDir(t) + + // Brand-new OS user: no preferences file on disk yet. + fresh, err := NewStore(nil, nil) + require.NoError(t, err) + assert.False(t, fresh.ExistedAtLoad(), "ExistedAtLoad must be false when no file is on disk") + + // Persisting a value writes the file to disk. + require.NoError(t, fresh.SetLanguage("en")) + + // A subsequent GUI launch reopens the now-present file. + reopened, err := NewStore(nil, nil) + require.NoError(t, err) + assert.True(t, reopened.ExistedAtLoad(), "ExistedAtLoad must be true after the store has persisted and is reopened") +} + func TestStore_ErrUnsupportedSentinel(t *testing.T) { // Verifies callers can match on the sentinel error rather than parsing // strings — protects against accidental %v -> %w changes that would diff --git a/client/ui/services/settings.go b/client/ui/services/settings.go index 1c16795ae..3b6f6f81b 100644 --- a/client/ui/services/settings.go +++ b/client/ui/services/settings.go @@ -20,11 +20,12 @@ type MDMFields struct { DisableServerRoutes bool `json:"disableServerRoutes"` AllowServerSSH *bool `json:"allowServerSSH"` DisableAutoConnect bool `json:"disableAutoConnect"` + DisableAutostart bool `json:"disableAutostart"` BlockInbound bool `json:"blockInbound"` DisableMetricsCollection bool `json:"disableMetricsCollection"` SplitTunnelMode bool `json:"splitTunnelMode"` SplitTunnelApps bool `json:"splitTunnelApps"` - DisableAdvancedView bool `json:"disableAdvancedView"` + DisableAdvancedView bool `json:"disableAdvancedView"` } type Features struct {