From 152c84efa2daada5907d1ae8d3af7e2f215bbf0f Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sun, 1 Nov 2026 00:36:45 +0100 Subject: [PATCH] fix(inbounds): delete an inbound's dependent rows on every delete path The node sync's sweep of an inbound the node stopped reporting deleted the inbound but left its host rows, and the depleted-clients delete left its sub-balancer references. SQLite hands a freed inbound id out again once the table empties, so a node's host overrides could bind to a new inbound it never hosted and point that inbound's subscribers at an address, port and TLS settings the node chose. delInbound's cleanup moves into deleteInboundRow (detach clients, delete the inbound, its host rows and its sub-balancer references), which the sweep and the depleted-clients delete now call as well. --- internal/web/service/inbound.go | 55 +++++++++++-------- internal/web/service/inbound_node.go | 6 +- internal/web/service/inbound_traffic.go | 8 +-- internal/web/service/node_host_adopt_test.go | 49 +++++++++++++++++ .../web/service/traffic_runtime_apply_test.go | 10 ++++ 5 files changed, 93 insertions(+), 35 deletions(-) diff --git a/internal/web/service/inbound.go b/internal/web/service/inbound.go index 408fa0cd2..940fedbab 100644 --- a/internal/web/service/inbound.go +++ b/internal/web/service/inbound.go @@ -1689,31 +1689,9 @@ func (s *InboundService) delInbound(id int) (bool, func(), error) { } if err := runSerializedTx(func(tx *gorm.DB) error { - if err := s.clientService.DetachInbound(tx, id); err != nil { + if err := s.deleteInboundRow(tx, id); err != nil { return err } - if err := tx.Delete(model.Inbound{}, id).Error; err != nil { - return err - } - if _, err := database.DeleteHosts(tx, "inbound_id = ?", id); err != nil { - return err - } - // Drop the deleted inbound from any sub-balancer that selects it; a - // dangling id would emit a member no subscriber can resolve (#5648). - var balancers []model.SubBalancer - if err := tx.Find(&balancers).Error; err != nil { - return err - } - for i := range balancers { - before := balancers[i].InboundIds - balancers[i].InboundIds = slices.DeleteFunc(before, func(b int) bool { return b == id }) - if len(balancers[i].InboundIds) == len(before) { - continue - } - if err := tx.Save(&balancers[i]).Error; err != nil { - return err - } - } if loadErr == nil && ib.NodeID != nil { return (&NodeService{}).MarkNodeDirtyTx(tx, *ib.NodeID) } @@ -1749,6 +1727,37 @@ func (s *InboundService) delInbound(id int) (bool, func(), error) { return needRestart, nodePush, nil } +// deleteInboundRow removes an inbound and every row keyed by its id; SQLite +// hands a freed id out again, so anything left behind binds to the next inbound. +func (s *InboundService) deleteInboundRow(tx *gorm.DB, id int) error { + if err := s.clientService.DetachInbound(tx, id); err != nil { + return err + } + if err := tx.Delete(model.Inbound{}, id).Error; err != nil { + return err + } + if _, err := database.DeleteHosts(tx, "inbound_id = ?", id); err != nil { + return err + } + // Drop the deleted inbound from any sub-balancer that selects it; a + // dangling id would emit a member no subscriber can resolve (#5648). + var balancers []model.SubBalancer + if err := tx.Find(&balancers).Error; err != nil { + return err + } + for i := range balancers { + before := balancers[i].InboundIds + balancers[i].InboundIds = slices.DeleteFunc(before, func(b int) bool { return b == id }) + if len(balancers[i].InboundIds) == len(before) { + continue + } + if err := tx.Save(&balancers[i]).Error; err != nil { + return err + } + } + return nil +} + type BulkDelInboundResult struct { Deleted int `json:"deleted"` Skipped []BulkDelInboundReport `json:"skipped,omitempty"` diff --git a/internal/web/service/inbound_node.go b/internal/web/service/inbound_node.go index a17787e21..76c352389 100644 --- a/internal/web/service/inbound_node.go +++ b/internal/web/service/inbound_node.go @@ -1010,11 +1010,7 @@ func (s *InboundService) setRemoteTrafficLocked(nodeID int, snap *runtime.Traffi } } } - if err := s.clientService.DetachInbound(tx, c.Id); err != nil { - return false, err - } - if err := tx.Where("id = ?", c.Id). - Delete(&model.Inbound{}).Error; err != nil { + if err := s.deleteInboundRow(tx, c.Id); err != nil { return false, err } delete(tagToCentral, c.Tag) diff --git a/internal/web/service/inbound_traffic.go b/internal/web/service/inbound_traffic.go index 6456aa1a9..6722b0e20 100644 --- a/internal/web/service/inbound_traffic.go +++ b/internal/web/service/inbound_traffic.go @@ -961,13 +961,7 @@ func (s *InboundService) DelDepletedClients(id int) (err error) { } if len(newClients) == 0 { deletedInbounds = append(deletedInbounds, *inbound) - if err := s.clientService.DetachInbound(tx, inbound.Id); err != nil { - return err - } - if _, err := database.DeleteHosts(tx, "inbound_id = ?", inbound.Id); err != nil { - return err - } - if err := tx.Delete(model.Inbound{}, inbound.Id).Error; err != nil { + if err := s.deleteInboundRow(tx, inbound.Id); err != nil { return err } if inbound.NodeID != nil { diff --git a/internal/web/service/node_host_adopt_test.go b/internal/web/service/node_host_adopt_test.go index 7ab57fc17..53906744b 100644 --- a/internal/web/service/node_host_adopt_test.go +++ b/internal/web/service/node_host_adopt_test.go @@ -75,3 +75,52 @@ func TestSetRemoteTraffic_AdoptsNodeHostRows(t *testing.T) { t.Fatalf("total host rows = %d, want 1 (group member for un-adopted node inbound 99 must not materialize)", total) } } + +func TestSetRemoteTraffic_SweptInboundTakesItsDependentsAlong(t *testing.T) { + setupConflictDB(t) + db := database.GetDB() + + const nodeID = 6 + if err := db.Create(&model.Node{Id: nodeID, Name: "host-node", Address: "10.0.0.6", Port: 2053, ApiToken: "t", Guid: "host-node-guid"}).Error; err != nil { + t.Fatalf("create node: %v", err) + } + inbound := func(id, port int, tag string) *model.Inbound { + return &model.Inbound{Id: id, Tag: tag, Enable: true, Port: port, Protocol: model.VLESS, Settings: `{"clients":[]}`} + } + group := func(groupId string, inboundId int) *entity.HostGroup { + return &entity.HostGroup{GroupId: groupId, InboundIds: []int{inboundId}, Hosts: []string{"cdn.example.com:8443"}, Remark: "cdn"} + } + + svc := InboundService{} + if _, err := svc.setRemoteTrafficLocked(nodeID, &runtime.TrafficSnapshot{ + Inbounds: []*model.Inbound{inbound(1, 443, "swept-443"), inbound(2, 444, "kept-444")}, + HostGroups: []*entity.HostGroup{group("g-swept", 1), group("g-kept", 2)}, + }, false, false); err != nil { + t.Fatalf("adopting merge: %v", err) + } + swept, kept := loadInboundByTag(t, "swept-443"), loadInboundByTag(t, "kept-444") + balancer := &model.SubBalancer{Remark: "both", InboundIds: []int{swept.Id, kept.Id}, Enabled: true} + if err := db.Create(balancer).Error; err != nil { + t.Fatalf("create balancer: %v", err) + } + + if _, err := svc.setRemoteTrafficLocked(nodeID, &runtime.TrafficSnapshot{ + Inbounds: []*model.Inbound{inbound(2, 444, "kept-444")}, + }, false, false); err != nil { + t.Fatalf("sweeping merge: %v", err) + } + + var groups []string + if err := db.Model(&model.Host{}).Order("group_id").Pluck("group_id", &groups).Error; err != nil { + t.Fatalf("load hosts: %v", err) + } + if len(groups) != 1 || groups[0] != "g-kept" { + t.Fatalf("host groups after the sweep = %v, want [g-kept]", groups) + } + if err := db.First(balancer, balancer.Id).Error; err != nil { + t.Fatalf("reload balancer: %v", err) + } + if len(balancer.InboundIds) != 1 || balancer.InboundIds[0] != kept.Id { + t.Fatalf("balancer members after the sweep = %v, want [%d]", balancer.InboundIds, kept.Id) + } +} diff --git a/internal/web/service/traffic_runtime_apply_test.go b/internal/web/service/traffic_runtime_apply_test.go index d8afc1620..77ac7165c 100644 --- a/internal/web/service/traffic_runtime_apply_test.go +++ b/internal/web/service/traffic_runtime_apply_test.go @@ -76,6 +76,10 @@ func TestDelDepletedClientsCleansRuntimeAfterCommit(t *testing.T) { Updates(map[string]any{"up": 100, "total": 100, "reset": 0}).Error; err != nil { t.Fatalf("deplete traffic: %v", err) } + balancer := &model.SubBalancer{Remark: "depleted", InboundIds: []int{ib.Id}, Enabled: true} + if err := database.GetDB().Create(balancer).Error; err != nil { + t.Fatalf("create balancer: %v", err) + } if err := (&InboundService{}).DelDepletedClients(-1); err != nil { t.Fatalf("DelDepletedClients: %v", err) @@ -83,4 +87,10 @@ func TestDelDepletedClientsCleansRuntimeAfterCommit(t *testing.T) { if got := fake.delInbound.Load(); got != 1 { t.Fatalf("runtime DelInbound calls = %d, want 1 after commit", got) } + if err := database.GetDB().First(balancer, balancer.Id).Error; err != nil { + t.Fatalf("reload balancer: %v", err) + } + if len(balancer.InboundIds) != 0 { + t.Fatalf("balancer members after the inbound went = %v, want none", balancer.InboundIds) + } }