fix(client): stop duplicate client entries accumulating in inbound settings

Adding a user to multi-node inbounds could leave 3-6 identical entries
in one inbound's settings.clients array: addInboundClient appended
incoming clients unconditionally, and the duplicate-email precheck
exempts a matching subId (so one identity can span several inbounds),
so a retried or raced add of the same client re-appended it to an
inbound that already carried it - on the master and, since nodes run
the same code, on every node, whose snapshot adoption then copied the
duplicates back verbatim. The normalized clients/client_inbounds tables
stayed clean (unique constraints), which is why the phantom rows only
showed in settings-driven views like the Detach clients modal, where
duplicate React keys also broke the selection counter.

Three layers: addInboundClient now skips incoming clients whose email
is already on the target inbound (idempotent re-adds instead of
duplication), node snapshot adoption collapses duplicate emails before
writing the central row, and an idempotent startup repair rewrites any
inbound whose settings still carry duplicates from older builds.

Closes #5770
This commit is contained in:
MHSanaei
2026-07-05 21:17:25 +02:00
parent 9d1a21b484
commit 5a7b3b7370
6 changed files with 321 additions and 5 deletions
@@ -0,0 +1,95 @@
package service
import (
"encoding/json"
"testing"
"github.com/google/uuid"
"github.com/mhsanaei/3x-ui/v3/internal/database"
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
)
func mustUnmarshal(t *testing.T, raw string, v any) {
t.Helper()
if err := json.Unmarshal([]byte(raw), v); err != nil {
t.Fatalf("unmarshal %q: %v", raw, err)
}
}
func settingsClientEmails(t *testing.T, inboundId int) []string {
t.Helper()
var ib model.Inbound
if err := database.GetDB().First(&ib, inboundId).Error; err != nil {
t.Fatalf("load inbound %d: %v", inboundId, err)
}
clients, err := (&InboundService{}).GetClients(&ib)
if err != nil {
t.Fatalf("GetClients: %v", err)
}
emails := make([]string, 0, len(clients))
for _, c := range clients {
emails = append(emails, c.Email)
}
return emails
}
// Re-adding a client that is already on the inbound must be an idempotent
// no-op, not a second settings entry: checkEmailsExistForClients exempts a
// matching subId (so one identity can span inbounds), which let retried or
// raced adds duplicate the same email inside one settings array (#5770).
func TestAddInboundClient_SkipsClientsAlreadyOnInbound(t *testing.T) {
setupBulkDB(t)
nodeID, _ := setupNodeRuntime(t)
alice := model.Client{ID: uuid.NewString(), Email: "alice@dup", SubID: "alice-sub-1234567", Enable: true}
ib := nodeInbound(t, nodeID, 33001, []model.Client{alice})
svc := &ClientService{}
inboundSvc := &InboundService{}
if _, err := svc.AddInboundClient(inboundSvc, &model.Inbound{Id: ib.Id, Protocol: model.VLESS, Settings: clientsSettings(t, []model.Client{alice})}); err != nil {
t.Fatalf("re-add of existing client should be a no-op, got error: %v", err)
}
if emails := settingsClientEmails(t, ib.Id); len(emails) != 1 || emails[0] != "alice@dup" {
t.Fatalf("settings after duplicate re-add: expected exactly [alice@dup], got %v", emails)
}
bob := model.Client{ID: uuid.NewString(), Email: "bob@dup", SubID: "bob-sub-123456789", Enable: true}
if _, err := svc.AddInboundClient(inboundSvc, &model.Inbound{Id: ib.Id, Protocol: model.VLESS, Settings: clientsSettings(t, []model.Client{alice, bob})}); err != nil {
t.Fatalf("mixed add (one duplicate, one new): %v", err)
}
if emails := settingsClientEmails(t, ib.Id); len(emails) != 2 || emails[0] != "alice@dup" || emails[1] != "bob@dup" {
t.Fatalf("settings after mixed add: expected [alice@dup bob@dup], got %v", emails)
}
}
func TestDedupeSettingsClients(t *testing.T) {
dup := `{"clients": [` +
`{"id": "u1", "email": "a@x", "subId": "s1"},` +
`{"id": "u2", "email": "b@x", "subId": "s2"},` +
`{"id": "u1", "email": "a@x", "subId": "s1"},` +
`{"id": "u1", "email": "A@X", "subId": "s1"}]}`
out, changed := dedupeSettingsClients(dup)
if !changed {
t.Fatal("expected duplicates to be removed")
}
var parsed struct {
Clients []model.Client `json:"clients"`
}
mustUnmarshal(t, out, &parsed)
if len(parsed.Clients) != 2 || parsed.Clients[0].Email != "a@x" || parsed.Clients[1].Email != "b@x" {
t.Fatalf("expected first occurrences [a@x b@x], got %+v", parsed.Clients)
}
clean := `{"clients": [{"id": "u1", "email": "a@x"}, {"id": "u2", "email": "b@x"}]}`
if _, changed := dedupeSettingsClients(clean); changed {
t.Fatal("clean settings must not be rewritten")
}
if _, changed := dedupeSettingsClients(""); changed {
t.Fatal("empty settings must not be rewritten")
}
if _, changed := dedupeSettingsClients("{not json"); changed {
t.Fatal("invalid settings must not be rewritten")
}
}
+39 -5
View File
@@ -319,12 +319,46 @@ func (s *ClientService) addInboundClient(inboundSvc *InboundService, data *model
return false, err
}
if oldInbound.Protocol == model.WireGuard {
existing, gcErr := inboundSvc.GetClients(oldInbound)
if gcErr != nil {
return false, gcErr
existingClients, err := inboundSvc.GetClients(oldInbound)
if err != nil {
return false, err
}
// A client already on this inbound is skipped instead of appended again:
// checkEmailsExistForClients exempts a matching subId so one identity can
// live on several inbounds, which let retried or raced adds duplicate the
// same email inside a single settings array (#5770). clients and
// interfaceClients are parsed from the same data.Settings array, so they
// stay index-aligned while filtering.
if len(existingClients) > 0 && len(clients) > 0 {
existingEmails := make(map[string]struct{}, len(existingClients))
for _, c := range existingClients {
if c.Email != "" {
existingEmails[strings.ToLower(c.Email)] = struct{}{}
}
}
if dErr := defaultWireguardClients(existing, clients, interfaceClients); dErr != nil {
keptClients := make([]model.Client, 0, len(clients))
keptWire := make([]any, 0, len(interfaceClients))
for i, c := range clients {
if c.Email != "" {
if _, dup := existingEmails[strings.ToLower(c.Email)]; dup {
continue
}
}
keptClients = append(keptClients, c)
if i < len(interfaceClients) {
keptWire = append(keptWire, interfaceClients[i])
}
}
if len(keptClients) == 0 {
return false, nil
}
clients = keptClients
interfaceClients = keptWire
}
if oldInbound.Protocol == model.WireGuard {
if dErr := defaultWireguardClients(existingClients, clients, interfaceClients); dErr != nil {
return false, dErr
}
}
+43
View File
@@ -2,6 +2,7 @@ package service
import (
"encoding/json"
"strings"
"sync"
"time"
@@ -141,6 +142,48 @@ func isClientEmailTombstoned(email string) bool {
return true
}
// dedupeSettingsClients collapses duplicate same-email client entries inside a
// settings JSON blob, keeping the first occurrence. Node snapshots produced by
// builds without the addInboundClient duplicate guard can carry duplicates
// (#5770); adopting them verbatim would copy the duplication into the central
// inbound. Returns the filtered JSON and whether anything was removed.
func dedupeSettingsClients(settings string) (string, bool) {
if settings == "" {
return settings, false
}
var parsed map[string]any
if err := json.Unmarshal([]byte(settings), &parsed); err != nil {
return settings, false
}
clients, _ := parsed["clients"].([]any)
if len(clients) < 2 {
return settings, false
}
seen := make(map[string]struct{}, len(clients))
kept := make([]any, 0, len(clients))
for _, c := range clients {
if cm, ok := c.(map[string]any); ok {
if email, _ := cm["email"].(string); email != "" {
key := strings.ToLower(email)
if _, dup := seen[key]; dup {
continue
}
seen[key] = struct{}{}
}
}
kept = append(kept, c)
}
if len(kept) == len(clients) {
return settings, false
}
parsed["clients"] = kept
b, err := json.MarshalIndent(parsed, "", " ")
if err != nil {
return settings, false
}
return string(b), true
}
// stripTombstonedClients drops just-deleted client entries from a node
// snapshot's settings JSON so adopting a stale snapshot can't re-add them to
// the central inbound while the delete tombstone is live. Returns the filtered
+3
View File
@@ -534,6 +534,9 @@ func (s *InboundService) setRemoteTrafficLocked(nodeID int, snap *runtime.Traffi
if stripped, changed := stripTombstonedClients(adoptedSettings); changed {
adoptedSettings = stripped
}
if deduped, changed := dedupeSettingsClients(adoptedSettings); changed {
adoptedSettings = deduped
}
updates := map[string]any{}
if !dirty {