From d114845e445bf906b4f697ce3dd28e9b9f7eb447 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sun, 1 Nov 2026 00:03:48 +0100 Subject: [PATCH] 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 | 28 ++++++++++++++++- internal/database/db_seed_test.go | 31 +++++++++++++++++++ internal/database/model/model.go | 2 +- internal/web/service/client_crud.go | 2 +- .../service/client_group_node_sync_test.go | 19 ++++++++++++ 5 files changed, 79 insertions(+), 3 deletions(-) diff --git a/internal/database/db.go b/internal/database/db.go index a8a570ab9..66d29d4e5 100644 --- a/internal/database/db.go +++ b/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 { diff --git a/internal/database/db_seed_test.go b/internal/database/db_seed_test.go index 85ceef985..3f928879c 100644 --- a/internal/database/db_seed_test.go +++ b/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) + } +} diff --git a/internal/database/model/model.go b/internal/database/model/model.go index 284d6d08a..cf93ba7fe 100644 --- a/internal/database/model/model.go +++ b/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, diff --git a/internal/web/service/client_crud.go b/internal/web/service/client_crud.go index 5a2081a11..5429167cd 100644 --- a/internal/web/service/client_crud.go +++ b/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 } diff --git a/internal/web/service/client_group_node_sync_test.go b/internal/web/service/client_group_node_sync_test.go index 942d64cad..9f0f569f9 100644 --- a/internal/web/service/client_group_node_sync_test.go +++ b/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") + } +}