cmd/tailmixd: retain WireGuard profile after state sorting

Reacquire an applied WireGuard profile by its stable ID after state normalization reorders the profile slice. Keep its state directory separately so secret cleanup cannot follow a stale profile pointer.

Add a regression test that deterministically invalidates the original pointer by sorting the new profile ahead of an existing profile.

Change-Id: I9009ffb6508576edb29ed5289d838e5cbff50a1e

Author: maisem

cmd/tailmixd/wireguardapi.go

@@ -70,6 +70,8 @@ func (s *supervisor) ApplyWireGuard(_ context.Context, requested wireguardcfg.Co
 		})
 		configured, _ = profileByName(&next, config.Name)
 	}
+	profileID := configured.ID
+	profileStateDir := configured.StateDir
 
 	secrets, err := completeWireGuardSecrets(config, supplied, oldSecrets, create)
 	if err != nil {
@@ -78,14 +80,14 @@ func (s *supervisor) ApplyWireGuard(_ context.Context, requested wireguardcfg.Co
 	if err := validateWireGuardKeyOwnership(config, secrets); err != nil {
 		return controlapi.WireGuardProfile{}, controlapi.NewError("invalid_request", "%v", err)
 	}
-	secretFile, err := writeWireGuardSecrets(configured.StateDir, secrets)
+	secretFile, err := writeWireGuardSecrets(profileStateDir, secrets)
 	if err != nil {
 		return controlapi.WireGuardProfile{}, fmt.Errorf("write WireGuard profile secrets: %w", err)
 	}
 	committed := false
 	defer func() {
 		if !committed {
-			_ = removeWireGuardSecrets(configured.StateDir, secretFile)
+			_ = removeWireGuardSecrets(profileStateDir, secretFile)
 		}
 	}()
 
@@ -98,7 +100,10 @@ func (s *supervisor) ApplyWireGuard(_ context.Context, requested wireguardcfg.Co
 	configured.MagicDNSSuffix = config.DNSSuffix
 	configured.WireGuard = configPointer(config)
 	configured.WireGuardSecretFile = secretFile
-	state.Normalize(&next)
+	configured, err = normalizeProfileByID(&next, profileID)
+	if err != nil {
+		return controlapi.WireGuardProfile{}, controlapi.NewError("invalid_state", "%v", err)
+	}
 
 	var engine *tailmixprofile.WireGuardEngine
 	if wasRunning {
@@ -138,7 +143,7 @@ func (s *supervisor) ApplyWireGuard(_ context.Context, requested wireguardcfg.Co
 	}
 	committed = true
 	if oldSecretFile != "" && oldSecretFile != secretFile {
-		_ = removeWireGuardSecrets(configured.StateDir, oldSecretFile)
+		_ = removeWireGuardSecrets(profileStateDir, oldSecretFile)
 	}
 	return s.wireGuardProfileLocked(*configured)
 }
@@ -229,6 +234,24 @@ func beforeProfileRemoved(st state.State, profileID string) bool {
 	return false
 }
 
+func profileByID(st *state.State, profileID string) *state.Profile {
+	for i := range st.Profiles {
+		if st.Profiles[i].ID == profileID {
+			return &st.Profiles[i]
+		}
+	}
+	return nil
+}
+
+func normalizeProfileByID(st *state.State, profileID string) (*state.Profile, error) {
+	state.Normalize(st)
+	configured := profileByID(st, profileID)
+	if configured == nil {
+		return nil, errors.New("profile disappeared during normalization")
+	}
+	return configured, nil
+}
+
 func validateWireGuardKeyOwnership(config wireguardcfg.Config, secrets wireguardcfg.Secrets) error {
 	if secrets.PrivateKey == nil {
 		return errors.New("WireGuard private key is required")

cmd/tailmixd/wireguardapi_test.go

@@ -62,3 +62,26 @@ func TestValidateWireGuardEndpointNamesRejectsProfileDNSRecursion(t *testing.T)
 		t.Fatalf("rejected independent endpoint: %v", err)
 	}
 }
+
+func TestNormalizeProfileByIDReacquiresSortedProfile(t *testing.T) {
+	config := wireguardcfg.Config{
+		Version: wireguardcfg.Version, Name: "lab", DNSSuffix: "lab.example",
+		Addresses: []netip.Addr{netip.MustParseAddr("10.80.0.1")},
+	}
+	st := state.State{Profiles: []state.Profile{
+		{ID: "z_existing", Name: "existing", Kind: state.ProfileKindTailscale},
+		{ID: "p_new", Name: "lab", Kind: state.ProfileKindWireGuard, WireGuard: &config},
+	}}
+	stale := &st.Profiles[1]
+
+	configured, err := normalizeProfileByID(&st, "p_new")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if stale.ID != "z_existing" {
+		t.Fatalf("test did not force the original pointer to become stale: points to %q", stale.ID)
+	}
+	if configured.ID != "p_new" || configured.WireGuard == nil || configured.WireGuard.Name != "lab" {
+		t.Fatalf("reacquired profile = %+v", configured)
+	}
+}