From bc57548a35e3cf8be1cf7083c94e26df27cb6a31 Mon Sep 17 00:00:00 2001 From: Sanaei Date: Wed, 9 Sep 2026 00:17:54 +0200 Subject: [PATCH] fix(clients): withdraw the delete tombstone when the email is re-created Deleting a client tombstones its email for 90s so a node snapshot captured before the deletion cannot resurrect it. Nothing withdrew that tombstone when the operator re-created the same email, so on a master with at least one node the next merge filtered the live client out of the snapshot and SyncInbound pruned its inbound link. The client reappeared only once the tombstone expired, which is the 90-120s detach window reported. Withdraw it on a successful create, single and bulk, so a tombstone can never outlive the identity it was meant to bury. A failed create still leaves it standing, which is what keeps the stale-snapshot guard intact. Closes #6370 --- internal/web/service/client_bulk.go | 5 ++ internal/web/service/client_crud.go | 3 + .../service/client_delete_tombstone_test.go | 62 +++++++++++++++++++ 3 files changed, 70 insertions(+) diff --git a/internal/web/service/client_bulk.go b/internal/web/service/client_bulk.go index 1d7314909..a68d11867 100644 --- a/internal/web/service/client_bulk.go +++ b/internal/web/service/client_bulk.go @@ -1417,6 +1417,7 @@ func (s *ClientService) BulkCreate(inboundSvc *InboundService, payloads []Client } } + createdEmails := make([]string, 0, len(prep)) for idx := range prep { if failed[idx] { skip(prep[idx].client.Email, reason[idx]) @@ -1426,8 +1427,12 @@ func (s *ClientService) BulkCreate(inboundSvc *InboundService, payloads []Client skip(prep[idx].client.Email, err.Error()) continue } + createdEmails = append(createdEmails, prep[idx].client.Email) result.Created++ } + // A re-created email is a live identity again: a delete tombstone left + // standing makes the next node merge prune the new client's inbound links. + withdrawClientTombstones(createdEmails...) return result, needRestart, nil } diff --git a/internal/web/service/client_crud.go b/internal/web/service/client_crud.go index a96f516a6..f6adcfddf 100644 --- a/internal/web/service/client_crud.go +++ b/internal/web/service/client_crud.go @@ -236,6 +236,9 @@ func (s *ClientService) Create(inboundSvc *InboundService, payload *ClientCreate // already existed, and a create the panel reported as failed must not. return needRestart, fanoutErr } + // A re-created email is a live identity again: a delete tombstone left + // standing makes the next node merge prune the new client's inbound links. + withdrawClientTombstones(client.Email) return needRestart, s.setClientLimitHwidByEmail(nil, client.Email, payload.LimitHwid) } diff --git a/internal/web/service/client_delete_tombstone_test.go b/internal/web/service/client_delete_tombstone_test.go index be3e08aca..4d0513ca2 100644 --- a/internal/web/service/client_delete_tombstone_test.go +++ b/internal/web/service/client_delete_tombstone_test.go @@ -1,10 +1,13 @@ package service import ( + "fmt" "testing" "github.com/mhsanaei/3x-ui/v3/internal/database" "github.com/mhsanaei/3x-ui/v3/internal/database/model" + "github.com/mhsanaei/3x-ui/v3/internal/web/runtime" + "github.com/mhsanaei/3x-ui/v3/internal/xray" ) // Delete tombstones up front and keeps the record when an inbound fails. A @@ -42,3 +45,62 @@ func TestFailedDeleteWithdrawsTombstone(t *testing.T) { t.Fatal("delete kept the record but left a live tombstone: the next node sync would finish the deletion it refused") } } + +// A client re-created under a just-deleted email is a live identity again. If +// the tombstone outlives it, the node merge prunes the new client's link. +func TestRecreatedClientSurvivesNodeMerge(t *testing.T) { + db := initTrafficTestDB(t) + svc := &ClientService{} + inboundSvc := &InboundService{} + + const email = "reborn@x" + seedNodeRow(t, db, &model.Node{Id: 1, Name: "n1", Address: "127.0.0.1", Port: 2096, ApiToken: "tok", Enable: true}) + createNodeInboundWithClient(t, db, 1, "n1-in", 41501, email) + nodeSettings := fmt.Sprintf(`{"clients": [{"email": %q, "enable": true}]}`, email) + syncNodeWithSettings(t, inboundSvc, 1, "n1-in", nodeSettings, xray.ClientTraffic{Email: email, Enable: true}) + + var ib model.Inbound + if err := db.Where("tag = ?", "n1-in").First(&ib).Error; err != nil { + t.Fatalf("load inbound: %v", err) + } + + rec := &model.ClientRecord{} + if err := db.Where("email = ?", email).First(rec).Error; err != nil { + t.Fatalf("load adopted client record: %v", err) + } + t.Cleanup(func() { withdrawClientTombstones(email) }) + if _, err := svc.Delete(inboundSvc, rec.Id, false); err != nil { + t.Fatalf("delete client: %v", err) + } + + if _, err := svc.Create(inboundSvc, &ClientCreatePayload{ + Client: model.Client{Email: email, Enable: true, ID: "44444444-4444-4444-4444-444444444444"}, + InboundIds: []int{ib.Id}, + }); err != nil { + t.Fatalf("re-create client: %v", err) + } + + // The create marks the node config-dirty, which parks the client merge; the + // reconcile clears it a tick later, well inside the 90s tombstone window. + if err := db.Model(&model.Node{}).Where("id = ?", 1).Update("config_dirty", false).Error; err != nil { + t.Fatalf("clear config_dirty: %v", err) + } + snap := &runtime.TrafficSnapshot{Inbounds: []*model.Inbound{{ + Tag: "n1-in", Protocol: model.VLESS, Settings: nodeSettings, + ClientStats: []xray.ClientTraffic{{Email: email, Enable: true}}, + }}} + if _, err := inboundSvc.setRemoteTrafficLocked(1, snap, false, false); err != nil { + t.Fatalf("node merge: %v", err) + } + + var links int64 + if err := db.Model(&model.ClientInbound{}). + Joins("JOIN clients ON clients.id = client_inbounds.client_id"). + Where("clients.email = ? AND client_inbounds.inbound_id = ?", email, ib.Id). + Count(&links).Error; err != nil { + t.Fatalf("count client links: %v", err) + } + if links != 1 { + t.Fatalf("re-created client has %d inbound links after the node merge, want 1: the delete tombstone outlived the email and the merge pruned it", links) + } +}