From f52e59484ac8df31c2984b87cdbc317de121328e Mon Sep 17 00:00:00 2001 From: riccardom Date: Wed, 23 Sep 2026 11:54:35 +0200 Subject: [PATCH] [client] Persist the profile before overlaying MDM on it (review item) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `netbird login` read the config, applied the MDM policy on top, and only then provisioned the identity and wrote the result out. On a profile with no identity yet — a first login — that write persisted the enforced values into the user's own config file: an MDM-managed management URL or pre-shared key became indistinguishable from one the user set, and stayed behind once the policy was withdrawn. Provisioning and its write now come first, and the overlay is applied to the in-memory config afterwards, where it belongs: it is re-derived on every load and never meant to reach disk from here. Server.getConfig already orders the two this way; the two paths now agree. Reported by cubic on the PR. --- client/cmd/login.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/client/cmd/login.go b/client/cmd/login.go index 1d886ed0c..428864375 100644 --- a/client/cmd/login.go +++ b/client/cmd/login.go @@ -326,16 +326,16 @@ func doForegroundLogin(ctx context.Context, cmd *cobra.Command, setupKey string, if err != nil { return fmt.Errorf("read config file %s: %v", configFilePath, err) } - // CLI standalone login: profilemanager no longer auto-applies MDM, - // so layer in the OS-native policy here. Desktop builds construct - // a Loader with no fetcher — the build-tagged loadPlatform reads - // the registry/plist directly. - config.ApplyMDMPolicy(mdm.NewLoader(nil).Load()) - // Reading a config does not provision one: this login is about to dial // management with the profile's identity, so mint the keys if the profile // has none yet and put them on disk — a key that stayed in memory would // come back different on the next run and register a second peer. + // + // Before the MDM overlay below, on purpose: the file must keep the + // profile's own values. The overlay is runtime-only and re-derived on + // every load, so persisting it would turn an enforced management URL or + // pre-shared key into one the user appears to own once the policy is + // withdrawn. if generated, err := config.EnsureIdentity(); err != nil { return fmt.Errorf("ensure profile identity: %v", err) } else if generated { @@ -344,6 +344,12 @@ func doForegroundLogin(ctx context.Context, cmd *cobra.Command, setupKey string, } } + // CLI standalone login: profilemanager no longer auto-applies MDM, + // so layer in the OS-native policy here. Desktop builds construct + // a Loader with no fetcher — the build-tagged loadPlatform reads + // the registry/plist directly. + config.ApplyMDMPolicy(mdm.NewLoader(nil).Load()) + // Mirror runInForegroundMode: recover residual state (DNS, firewall, // ssh config, legacy routing) from a previous unclean shutdown and // enable advanced routing before dialing management.