fix(clients): store client group names trimmed

The groups page lists clients.group_name as stored, while every group
action trims the name it is given. A name with surrounding white space,
saved by a node snapshot or by a client edit, was listed as a separate
group, and deleting, renaming or resetting that row acted on the real
group of the trimmed name instead.

Group names are trimmed wherever a client record is built or updated,
and a one-time seeder folds stored names onto their trimmed form.

internal/database/db.go

@@ -1530,7 +1530,7 @@ func runSeeders(isUsersEmpty bool) error {
 	}
 
 	if empty && isUsersEmpty {
-		seeders := []string{"UserPasswordHash", "ClientsTable", "InboundClientsArrayFix", "InboundClientTgIdFix2", "InboundClientSubIdFix", "FreedomFinalRulesReverseFix", "FreedomFinalRulesPrivateEgressBlock", "UppercaseFreedomFinalRulesFix", "InboundRealityFinalmaskTcpStrip", "ApiTokensHash", "LegacyProxySettingsCleanup", "SessionSecretRotation", "OutboundRemovedKeysFix", "FreedomDomainStrategyFix", "DNSOutboundLegacyKeysFix", "DNSOutboundQTypeZeroFix", "WireguardDomainStrategyFix", "XdnsFinalmaskNamesAddrsFix", "UDPHopClientMaskFix", "HostVerifyPeerCertByNameBoolFix", "HostMihomoX25519Reset", "WireguardPeersToClients", "MtprotoSecretsToClients", "NodeInboundsAdopted", "NodeDownNotified", "ResetIpLimitNoFail2ban"}
+		seeders := []string{"UserPasswordHash", "ClientsTable", "InboundClientsArrayFix", "InboundClientTgIdFix2", "InboundClientSubIdFix", "FreedomFinalRulesReverseFix", "FreedomFinalRulesPrivateEgressBlock", "UppercaseFreedomFinalRulesFix", "InboundRealityFinalmaskTcpStrip", "ApiTokensHash", "LegacyProxySettingsCleanup", "SessionSecretRotation", "OutboundRemovedKeysFix", "FreedomDomainStrategyFix", "DNSOutboundLegacyKeysFix", "DNSOutboundQTypeZeroFix", "WireguardDomainStrategyFix", "XdnsFinalmaskNamesAddrsFix", "UDPHopClientMaskFix", "HostVerifyPeerCertByNameBoolFix", "HostMihomoX25519Reset", "WireguardPeersToClients", "MtprotoSecretsToClients", "NodeInboundsAdopted", "NodeDownNotified", "ResetIpLimitNoFail2ban", "ClientGroupNameTrim"}
 		for _, name := range seeders {
 			if err := db.Create(&model.HistoryOfSeeders{SeederName: name}).Error; err != nil {
 				return err
@@ -1713,6 +1713,12 @@ func runSeeders(isUsersEmpty bool) error {
 		}
 	}
 
+	if !slices.Contains(seedersHistory, "ClientGroupNameTrim") {
+		if err := trimClientGroupNames(); err != nil {
+			return err
+		}
+	}
+
 	if err := seedHostsFromExternalProxy(); err != nil {
 		return err
 	}
@@ -1908,6 +1914,26 @@ func clearLegacyProxySettings() error {
 	})
 }
 
+// The groups page addresses a group by its trimmed name, so a padded stored name listed as
+// its own group while every action on it hit the trimmed one.
+func trimClientGroupNames() error {
+	return db.Transaction(func(tx *gorm.DB) error {
+		var names []string
+		if err := tx.Model(&model.ClientRecord{}).Distinct().Pluck("group_name", &names).Error; err != nil {
+			return err
+		}
+		for _, name := range names {
+			if trimmed := strings.TrimSpace(name); trimmed != name {
+				if err := tx.Model(&model.ClientRecord{}).Where("group_name = ?", name).
+					UpdateColumn("group_name", trimmed).Error; err != nil {
+					return err
+				}
+			}
+		}
+		return tx.Create(&model.HistoryOfSeeders{SeederName: "ClientGroupNameTrim"}).Error
+	})
+}
+
 // Secrets stored before v2.8.3 came from math/rand (a 31-bit clock seed up to v2.1.3);
 // GetSecret draws a crypto/rand one on the next start.
 func dropPreUpgradeSessionSecret() error {

internal/database/db_seed_test.go

@@ -345,3 +345,34 @@ func TestNormalizeSettingPaths_RepairsLegacyValues(t *testing.T) {
 		}
 	}
 }
+
+// A padded group name was listed as its own group while every action on it hit the trimmed
+// one, so the upgrade folds stored names onto their trimmed form.
+func TestClientGroupNamesTrimmedOnUpgrade(t *testing.T) {
+	dbPath := filepath.Join(t.TempDir(), "x-ui.db")
+	if err := InitDB(dbPath); err != nil {
+		t.Fatalf("InitDB: %v", err)
+	}
+	t.Cleanup(func() { _ = CloseDB() })
+	for email, group := range map[string]string{"a@x": "vip ", "b@x": "\tvip", "c@x": "vip"} {
+		if err := db.Create(&model.ClientRecord{Email: email, Group: group, Enable: true}).Error; err != nil {
+			t.Fatalf("seed %s: %v", email, err)
+		}
+	}
+	if err := db.Where("seeder_name = ?", "ClientGroupNameTrim").Delete(&model.HistoryOfSeeders{}).Error; err != nil {
+		t.Fatalf("clear seeder history: %v", err)
+	}
+	if err := CloseDB(); err != nil {
+		t.Fatalf("CloseDB: %v", err)
+	}
+	if err := InitDB(dbPath); err != nil {
+		t.Fatalf("reopen InitDB: %v", err)
+	}
+	var groups []string
+	if err := db.Model(&model.ClientRecord{}).Distinct().Pluck("group_name", &groups).Error; err != nil {
+		t.Fatalf("read groups: %v", err)
+	}
+	if len(groups) != 1 || groups[0] != "vip" {
+		t.Fatalf("groups after upgrade = %q, want [vip]", groups)
+	}
+}

internal/database/model/model.go

@@ -1288,7 +1288,7 @@ func (c *Client) ToRecord() *ClientRecord {
 		ExpiryTime:        c.ExpiryTime,
 		Enable:            c.Enable,
 		TgID:              c.TgID,
-		Group:             c.Group,
+		Group:             strings.TrimSpace(c.Group),
 		Comment:           c.Comment,
 		Reset:             c.Reset,
 		ResetDay:          c.ResetDay,

internal/web/service/client_crud.go

@@ -923,7 +923,7 @@ func (s *ClientService) Update(inboundSvc *InboundService, id int, updated model
 	// including the empty string that removes the client from its group.
 	if err := database.GetDB().Model(&model.ClientRecord{}).
 		Where("id = ?", id).
-		UpdateColumn("group_name", updated.Group).Error; err != nil {
+		UpdateColumn("group_name", strings.TrimSpace(updated.Group)).Error; err != nil {
 		return needRestart, err
 	}
 

internal/web/service/client_group_node_sync_test.go

@@ -181,3 +181,22 @@ func TestClientUpdate_ClearsGroup(t *testing.T) {
 		t.Errorf("inbound settings still carry a group key after removal: %s", ibAfter.Settings)
 	}
 }
+
+// The groups page lists stored names but addresses a group by its trimmed name, so a
+// padded name from a node made actions on its row hit the real group.
+func TestSetRemoteTraffic_StoresTrimmedGroupName(t *testing.T) {
+	db := initTrafficTestDB(t)
+	seedNodeRow(t, db, &model.Node{Id: 1, Name: "n1", Address: "127.0.0.1", Port: 2096, ApiToken: "tok", Enable: true})
+	createNodeInbound(t, db, 1, "n1-in", 41001)
+
+	syncNodeWithSettings(t, &InboundService{}, 1, "n1-in",
+		`{"clients":[{"email":"who@x","id":"66666666-6666-4666-8666-666666666666","group":"vip\t","enable":true}]}`)
+
+	var row model.ClientRecord
+	if err := db.Where("email = ?", "who@x").First(&row).Error; err != nil {
+		t.Fatalf("lookup client: %v", err)
+	}
+	if row.Group != "vip" {
+		t.Fatalf("group stored as %q, want %q", row.Group, "vip")
+	}
+}