From 1d373cf67eeeca253cfc30be8ea0ef7781eaf5c5 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sun, 1 Nov 2026 13:13:10 +0100 Subject: [PATCH] fix(node): let a node take back a client orphaned before the upgrade The migration backfills sync_orphaned_by = 0 for every client a node merge had already soft-orphaned, and the gate refused a detached record unless its mark named the reporting node. A client inside its orphan grace window at upgrade time was therefore refused when its node reported it again, and ReapSyncOrphans then deleted it on the master while the node kept serving it. A mark that names no node predates the column and may be taken back by any node, as before. --- internal/web/service/inbound_node_claims.go | 3 ++- internal/web/service/node_foreign_client_test.go | 11 ++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/web/service/inbound_node_claims.go b/internal/web/service/inbound_node_claims.go index 82d7064f4..df0acc534 100644 --- a/internal/web/service/inbound_node_claims.go +++ b/internal/web/service/inbound_node_claims.go @@ -160,7 +160,8 @@ func loadNodeClientClaims(tx *gorm.DB, nodeID int, snap *runtime.TrafficSnapshot case scope.elsewhere: claims.refused[email] = "it is attached only to inbounds this node does not host" claims.usageOnly[email] = struct{}{} - case !scope.here && (rec.SyncOrphanedAt == 0 || rec.SyncOrphanedBy != nodeID): + // A mark from before sync_orphaned_by existed names no node, so any node may take it back. + case !scope.here && (rec.SyncOrphanedAt == 0 || (rec.SyncOrphanedBy != 0 && rec.SyncOrphanedBy != nodeID)): claims.refused[email] = "it is detached from every inbound on the master" case subID != rec.SubID && subID != "" && subIDTaken(email, subID): claims.storedSubID[email] = rec.SubID diff --git a/internal/web/service/node_foreign_client_test.go b/internal/web/service/node_foreign_client_test.go index d581969b2..21b9f78f3 100644 --- a/internal/web/service/node_foreign_client_test.go +++ b/internal/web/service/node_foreign_client_test.go @@ -243,11 +243,13 @@ func TestNodeCannotAdoptDetachedClient(t *testing.T) { master := seedMasterInbound(t, db, "master-in", 40001, `{"email":"detached@x","id":"11111111-1111-4111-8111-111111111111","subId":"d-sub","enable":true},`+ `{"email":"mine@x","id":"22222222-2222-4222-8222-222222222222","enable":true},`+ - `{"email":"theirs@x","id":"33333333-3333-4333-8333-333333333333","enable":true}`) + `{"email":"theirs@x","id":"33333333-3333-4333-8333-333333333333","enable":true},`+ + `{"email":"legacy@x","id":"44444444-4444-4444-8444-444444444444","enable":true}`) if err := db.Exec("DELETE FROM client_inbounds WHERE inbound_id = ?", master.Id).Error; err != nil { t.Fatalf("detach: %v", err) } - for email, node := range map[string]int{"mine@x": 1, "theirs@x": 2} { + // legacy@x was marked before sync_orphaned_by existed, so its mark names no node. + for email, node := range map[string]int{"mine@x": 1, "theirs@x": 2, "legacy@x": 0} { if err := db.Model(&model.ClientRecord{}).Where("email = ?", email). Updates(map[string]any{"sync_orphaned_at": 1, "sync_orphaned_by": node}).Error; err != nil { t.Fatalf("mark %s orphaned: %v", email, err) @@ -256,7 +258,7 @@ func TestNodeCannotAdoptDetachedClient(t *testing.T) { createNodeInbound(t, db, 1, "n1-in", 41001) syncNodeWithSettings(t, svc, 1, "n1-in", - `{"clients":[{"email":"detached@x","subId":"picked","enable":true},{"email":"mine@x","enable":true},{"email":"theirs@x","subId":"picked2","enable":true}]}`) + `{"clients":[{"email":"detached@x","subId":"picked","enable":true},{"email":"mine@x","enable":true},{"email":"theirs@x","subId":"picked2","enable":true},{"email":"legacy@x","enable":true}]}`) if row := readClientRecord(t, db, "detached@x"); row.SubID != "d-sub" || nodeHostsClient(t, db, "n1-in", "detached@x") { t.Fatalf("node adopted a detached client: subId=%q", row.SubID) @@ -267,6 +269,9 @@ func TestNodeCannotAdoptDetachedClient(t *testing.T) { if !nodeHostsClient(t, db, "n1-in", "mine@x") { t.Fatal("node could not take back a client its own merge orphaned") } + if !nodeHostsClient(t, db, "n1-in", "legacy@x") { + t.Fatal("a client orphaned before the upgrade could not be taken back and would be reaped") + } } // The subscription server selects by subId alone, so a node's client must not join