From e9988879773ba5541b68306367aec1c699808d77 Mon Sep 17 00:00:00 2001
From: riccardom <riccardomanfrin@gmail.com>
Date: Mon, 7 Sep 2026 16:52:25 +0200
Subject: [PATCH] [client] Normalize the config before diffing it in
 WouldChange
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

apply() reports two different things through one bool: an input that changed
a value, and a field it had to fill in because the config carried none. The
update-settings gate reads that bool as "the caller asked for a change", so
any config still missing a default answered a request that asks for nothing
with a refusal.

Readers already hand out normalized configs — readConfig applies an empty
input for exactly this reason — which is why the gate got away with it. But a
handler that refuses a request must not depend on where its caller obtained
the config, and it must not start reading "this profile predates a field" as
"the caller asked for a change" the day someone adds one with a default.

WouldChange now runs the filling-in as a pass of its own and discards its
verdict, so the pass that answers the caller measures only what the input
did.
---
 client/internal/profilemanager/config.go      | 16 ++++++++++
 .../config_would_change_test.go               | 29 +++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go
index 9ef0873f2..036ed0e76 100644
--- a/client/internal/profilemanager/config.go
+++ b/client/internal/profilemanager/config.go
@@ -1090,6 +1090,22 @@ func (config *Config) WouldChange(input ConfigInput) (bool, error) {
 		probe = baseline
 	}
 
+	// Normalize before measuring. apply() reports two different things through
+	// one bool: an input that changed a value, and a field it had to fill in
+	// because the config carried none. Only the first is a settings change, so
+	// the filling-in gets a pass of its own whose verdict is discarded, and the
+	// pass that answers the caller runs against a config with nothing left to
+	// fill in.
+	//
+	// Readers already hand out normalized configs — readConfig applies an empty
+	// input for this very reason — so this is normally a no-op. But a gate that
+	// refuses a request must not depend on where its caller got the config
+	// from, and it must not start reading "this profile predates a field" as
+	// "the caller asked for a change" the day someone adds one.
+	if _, err := probe.apply(ConfigInput{ConfigPath: input.ConfigPath}); err != nil {
+		return true, fmt.Errorf("normalize the config to diff against: %w", err)
+	}
+
 	if isPreSharedKeyHidden(input.PreSharedKey) {
 		input.PreSharedKey = nil
 	}
diff --git a/client/internal/profilemanager/config_would_change_test.go b/client/internal/profilemanager/config_would_change_test.go
index f0458f0d5..d01cd6b8a 100644
--- a/client/internal/profilemanager/config_would_change_test.go
+++ b/client/internal/profilemanager/config_would_change_test.go
@@ -448,6 +448,35 @@ func TestWouldChangeIgnoresRestatedDefaultsOfUnsetFields(t *testing.T) {
 	}
 }
 
+// The verdict must not depend on where the caller got the config from. Readers
+// normalize what they hand out, but apply() signals "I filled in a default"
+// through the same bool as "the input changed something", so a config that
+// never passed through a read would otherwise report a change for an input
+// that asks for nothing.
+func TestWouldChangeNormalizesBeforeMeasuring(t *testing.T) {
+	rawConfig := func(t *testing.T) *Config {
+		t.Helper()
+
+		cfg := &Config{WgIface: iface.WgInterfaceDefault}
+		require.Nil(t, cfg.ServerSSHAllowed, "the fixture is only useful while the config is not normalized")
+		require.Nil(t, cfg.EnableSSHRoot)
+		require.Empty(t, cfg.IFaceBlackList)
+		return cfg
+	}
+
+	changed, err := rawConfig(t).WouldChange(ConfigInput{})
+	require.NoError(t, err)
+	require.False(t, changed, "an input carrying nothing cannot change anything")
+
+	changed, err = rawConfig(t).WouldChange(ConfigInput{EnableSSHRoot: boolPtr(false)})
+	require.NoError(t, err)
+	require.False(t, changed, "the default of a field the config never held is not a change")
+
+	changed, err = rawConfig(t).WouldChange(ConfigInput{EnableSSHRoot: boolPtr(true)})
+	require.NoError(t, err)
+	require.True(t, changed, "a non-default value is still a change")
+}
+
 // A zero-padded port addresses the same port.
 func TestServiceURLPortIsNormalizedNumerically(t *testing.T) {
 	padded, err := ParseServiceURL("padded", "https://mgmt.example.com:0443")