[client] Do not panic on a config with no sync message version

apply() wrote the incoming sync message version through the stored pointer,
without checking it was there: a config that carries no version yet made it
dereference nil. Reachable from the update-settings dry run, which runs inside
a request handler — where failing closed is the worst acceptable outcome, and a
panic is not one.

The field is now reassigned like every other optional one, which also means
apply() no longer mutates anything the caller still holds through a pointer, so
the dry run's copy has one less field to detach.

Reported by cubic-dev-ai on PR #7398.

client/internal/profilemanager/config.go

@@ -682,9 +682,12 @@ func (config *Config) apply(input ConfigInput) (updated bool, err error) {
 		updated = true
 	}
 
-	if input.SyncMessageVersion != nil && *input.SyncMessageVersion != *config.SyncMessageVersion {
+	// Assigning the pointer, not writing through it: a config that carries no
+	// version yet would otherwise be a nil dereference, and a panic inside a
+	// request handler is not a way to fail.
+	if input.SyncMessageVersion != nil && (config.SyncMessageVersion == nil || *input.SyncMessageVersion != *config.SyncMessageVersion) {
 		log.Infof("setting SyncMessageVersion to %v", *input.SyncMessageVersion)
-		*config.SyncMessageVersion = *input.SyncMessageVersion
+		config.SyncMessageVersion = input.SyncMessageVersion
 		updated = true
 	}
 
@@ -1032,12 +1035,11 @@ func newDryRunBaseline(configPath string) (*Config, error) {
 	return baseline, nil
 }
 
-// clone returns a copy of the config that apply can be run against without
-// the original observing the writes, or nil for a nil receiver. Only what
-// apply mutates in place needs detaching: the slices it replaces or appends
-// to, and SyncMessageVersion, which it writes through the pointer. The
-// remaining pointer fields are reassigned, not written through, and
-// ClientCertKeyPair is only overwritten.
+// clone returns a copy of the config that apply can be run against without the
+// original observing the writes, or nil for a nil receiver. Only what apply
+// mutates in place needs detaching, which is the slices it replaces or appends
+// to: every pointer field it touches is reassigned rather than written through,
+// and ClientCertKeyPair is only overwritten.
 func (config *Config) clone() *Config {
 	if config == nil {
 		return nil
@@ -1047,10 +1049,6 @@ func (config *Config) clone() *Config {
 	probe.IFaceBlackList = slices.Clone(config.IFaceBlackList)
 	probe.NATExternalIPs = slices.Clone(config.NATExternalIPs)
 	probe.DNSLabels = slices.Clone(config.DNSLabels)
-	if config.SyncMessageVersion != nil {
-		version := *config.SyncMessageVersion
-		probe.SyncMessageVersion = &version
-	}
 	return &probe
 }
 

client/internal/profilemanager/config_would_change_test.go

@@ -108,7 +108,7 @@ func TestReadsDoNotWriteTheConfigBack(t *testing.T) {
 	denormalized := []byte(`{"WgIface":"wt0"}`)
 
 	for name, read := range map[string]func(string) (*Config, error){
-		"GetExistingConfig":   GetExistingConfig,
+		"GetExistingConfig":    GetExistingConfig,
 		"ReadOrGenerateConfig": ReadOrGenerateConfig,
 	} {
 		t.Run(name, func(t *testing.T) {
@@ -261,3 +261,17 @@ func TestUpdateConfigProvisionsAMissingIdentity(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, cfg.PrivateKey, persisted.PrivateKey, "the provisioned identity was not persisted")
 }
+
+// A config that carries no sync message version must not make the dry run
+// panic: the gate runs inside a request handler, where failing closed is the
+// worst acceptable outcome.
+func TestWouldChangeWithoutAStoredSyncMessageVersion(t *testing.T) {
+	cfg := seededConfig(t)
+	require.Nil(t, cfg.SyncMessageVersion, "the fixture is only useful while the field starts out unset")
+
+	version := 2
+	changed, err := cfg.WouldChange(ConfigInput{SyncMessageVersion: &version})
+	require.NoError(t, err)
+	require.True(t, changed)
+	require.Nil(t, cfg.SyncMessageVersion, "the dry run set the version on the stored config")
+}