diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 58ca62213..341064b44 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -102,13 +102,13 @@ jobs: exit 1 fi - - name: PostgreSQL subscription Host ordering tests + - name: PostgreSQL subscription Host tests run: | set -o pipefail - go test ./internal/sub -run '^TestSubHostOrder_Postgres$' -count=1 -v | tee /tmp/postgres-host-order.log - passed=$(grep -c -- '^--- PASS' /tmp/postgres-host-order.log || true) + go test ./internal/sub -run '^TestSubHosts_Postgres$' -count=1 -v | tee /tmp/postgres-hosts.log + passed=$(grep -c -- '^--- PASS' /tmp/postgres-hosts.log || true) if [ "$passed" -ne 1 ]; then - echo "expected passing PostgreSQL Host ordering coverage, got $passed" >&2 + echo "expected passing PostgreSQL Host coverage, got $passed" >&2 exit 1 fi diff --git a/internal/database/db.go b/internal/database/db.go index 705469c70..924953a2f 100644 --- a/internal/database/db.go +++ b/internal/database/db.go @@ -32,6 +32,7 @@ import ( "gorm.io/driver/postgres" "gorm.io/driver/sqlite" "gorm.io/gorm" + "gorm.io/gorm/clause" "gorm.io/gorm/logger" ) @@ -78,6 +79,7 @@ func allModels() []any { &model.ClientGroup{}, &model.InboundFallback{}, &model.Host{}, + &model.HostTombstone{}, &model.NodeClientTraffic{}, &model.NodeClientIp{}, &model.ClientGlobalTraffic{}, @@ -873,7 +875,7 @@ func SyncHostsFromExternalProxy(tx *gorm.DB, inboundId int, oldStreamSettings, n continue } if i >= len(added) { - if err := tx.Delete(host).Error; err != nil { + if _, err := DeleteHosts(tx, "id = ?", host.Id); err != nil { return err } continue @@ -1048,17 +1050,43 @@ func anyToNonEmptyStrings(v any) []string { } func pruneOrphanedHosts() error { - res := db.Exec("DELETE FROM hosts WHERE inbound_id NOT IN (SELECT id FROM inbounds)") - if res.Error != nil { - log.Printf("Error pruning orphaned hosts rows: %v", res.Error) - return res.Error + var pruned int64 + err := db.Transaction(func(tx *gorm.DB) error { + var err error + pruned, err = DeleteHosts(tx, "inbound_id NOT IN (SELECT id FROM inbounds)") + return err + }) + if err != nil { + log.Printf("Error pruning orphaned hosts rows: %v", err) + return err } - if res.RowsAffected > 0 { - log.Printf("Pruned %d orphaned hosts row(s)", res.RowsAffected) + if pruned > 0 { + log.Printf("Pruned %d orphaned hosts row(s)", pruned) } return nil } +// DeleteHosts deletes the hosts matching query and keeps each one's inbound in host_tombstones, in +// the caller's transaction, so a client still bound to one fails closed on that inbound alone. +func DeleteHosts(tx *gorm.DB, query any, args ...any) (int64, error) { + var tombstones []model.HostTombstone + if err := tx.Model(&model.Host{}).Select("id AS host_id, inbound_id").Where(query, args...).Scan(&tombstones).Error; err != nil { + return 0, err + } + if len(tombstones) == 0 { + return 0, nil + } + // migrate-db resyncs the hosts sequence to MAX(id), so a deleted id can return; keep its latest inbound. + if err := tx.Clauses(clause.OnConflict{ + Columns: []clause.Column{{Name: "host_id"}}, + DoUpdates: clause.AssignmentColumns([]string{"inbound_id"}), + }).CreateInBatches(&tombstones, 500).Error; err != nil { + return 0, err + } + res := tx.Where(query, args...).Delete(&model.Host{}) + return res.RowsAffected, res.Error +} + func pruneOrphanedClientInbounds() error { res := db.Exec("DELETE FROM client_inbounds WHERE inbound_id NOT IN (SELECT id FROM inbounds)") if res.Error != nil { diff --git a/internal/database/migrate_data.go b/internal/database/migrate_data.go index a6eaf0409..a93e004a4 100644 --- a/internal/database/migrate_data.go +++ b/internal/database/migrate_data.go @@ -53,6 +53,7 @@ func migrationModels() []any { &model.ClientGroup{}, &model.InboundFallback{}, &model.Host{}, + &model.HostTombstone{}, &model.NodeClientTraffic{}, &model.NodeClientIp{}, &model.ClientGlobalTraffic{}, diff --git a/internal/database/model/model.go b/internal/database/model/model.go index 42ac2cc37..ccb1b8afd 100644 --- a/internal/database/model/model.go +++ b/internal/database/model/model.go @@ -1221,6 +1221,15 @@ type Host struct { func (Host) TableName() string { return "hosts" } +// HostTombstone keeps the inbound a deleted host served, so a client still bound to that host +// fails closed on that inbound alone. +type HostTombstone struct { + HostId int `json:"hostId" gorm:"primaryKey;autoIncrement:false;column:host_id"` + InboundId int `json:"inboundId" gorm:"not null;column:inbound_id"` +} + +func (HostTombstone) TableName() string { return "host_tombstones" } + // KeepAliveSeconds returns a scalar value; ranges are emitted by native AWG config. func (c Client) KeepAliveSeconds() int { if c.KeepAlive == nil { diff --git a/internal/sub/host_order_test.go b/internal/sub/host_order_test.go index 2e804b8bb..16e3fe7b0 100644 --- a/internal/sub/host_order_test.go +++ b/internal/sub/host_order_test.go @@ -22,7 +22,7 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/web/service" ) -func TestSubHostOrder_Postgres(t *testing.T) { +func TestSubHosts_Postgres(t *testing.T) { if os.Getenv("XUI_DB_TYPE") != "postgres" || os.Getenv("XUI_DB_DSN") == "" { t.Skip("requires a disposable PostgreSQL database") } @@ -34,6 +34,7 @@ func TestSubHostOrder_Postgres(t *testing.T) { {"filters", TestSub_HostOrderFiltersAndFallbacks}, {"base64", TestSub_HostOrderBase64AndInfoNode}, {"balancer", TestSub_HostOrderBalancerSlots}, + {"deleted binding", TestSub_ClientHostRule_DeletedHostClosesOnlyItsInbound}, } { t.Run(scenario.name, func(t *testing.T) { cleanup, err := testpg.IsolatePackage("host_order") diff --git a/internal/sub/host_sub.go b/internal/sub/host_sub.go index a8ee17b8b..3a27936c8 100644 --- a/internal/sub/host_sub.go +++ b/internal/sub/host_sub.go @@ -20,35 +20,54 @@ const ( hostBindingBound // render only the returned bound hosts of this inbound ) -// clientHostBinding resolves the client's bound host rules against one inbound. A binding whose -// every id is gone fails closed; a missing id beside a live one never silences other inbounds. +// clientHostBinding resolves the client's bound host rules against one inbound. A deleted bound host +// closes only the inbound it served; ids that resolve to no inbound at all fail closed everywhere. func (s *SubService) clientHostBinding(inbound *model.Inbound, client model.Client) (hostBinding, []*model.Host) { ids := model.EffectiveClientHostRuleIds(client) - if len(ids) == 0 || database.GetDB() == nil { + db := database.GetDB() + if len(ids) == 0 || db == nil { return hostBindingNone, nil } var rows []model.Host - if err := database.GetDB().Where("id IN ?", ids).Find(&rows).Error; err != nil { + if err := db.Where("id IN ?", ids).Find(&rows).Error; err != nil { logger.Warning("SubService - clientHostBinding:", err) return hostBindingClosed, nil } - if len(rows) == 0 { - return hostBindingClosed, nil - } byId := make(map[int]*model.Host, len(rows)) for i := range rows { byId[rows[i].Id] = &rows[i] } var here []*model.Host + var gone []int for _, id := range ids { - if h, ok := byId[id]; ok && h.InboundId == inbound.Id { + h, ok := byId[id] + switch { + case !ok: + gone = append(gone, id) + case h.InboundId == inbound.Id: here = append(here, h) } } - if len(here) == 0 { + if len(here) > 0 { + return hostBindingBound, here + } + if len(gone) == 0 { return hostBindingNone, nil } - return hostBindingBound, here + var tombstones []model.HostTombstone + if err := db.Where("host_id IN ?", gone).Find(&tombstones).Error; err != nil { + logger.Warning("SubService - clientHostBinding:", err) + return hostBindingClosed, nil + } + if len(rows) == 0 && len(tombstones) == 0 { + return hostBindingClosed, nil + } + for _, tomb := range tombstones { + if tomb.InboundId == inbound.Id { + return hostBindingClosed, nil + } + } + return hostBindingNone, nil } // clientHostEndpoints returns the endpoints of the client's usable bound hosts on this inbound, or diff --git a/internal/sub/host_sub_test.go b/internal/sub/host_sub_test.go index 1be9c36f5..90fd84d73 100644 --- a/internal/sub/host_sub_test.go +++ b/internal/sub/host_sub_test.go @@ -1160,8 +1160,8 @@ func TestSub_ClientHostRule_MultipleBindings(t *testing.T) { } } -// A deleted binding is skipped; with no valid binding left the client fails closed with zero -// links (no fallback, no dummy link) but stays in traffic accounting. +// A deleted binding is skipped; a binding that resolves to no inbound fails closed with zero links +// (no fallback, no dummy link) but stays in traffic accounting. func TestSub_ClientHostRule_DeletedFallsBack(t *testing.T) { t.Run("deleted and valid binding renders only the valid host", func(t *testing.T) { seedSubDB(t) @@ -1191,7 +1191,7 @@ func TestSub_ClientHostRule_DeletedFallsBack(t *testing.T) { } }) - t.Run("deleted-only binding fails closed with zero links", func(t *testing.T) { + t.Run("binding that resolves to no inbound fails closed with zero links", func(t *testing.T) { seedSubDB(t) ib := seedSubInbound(t, "s1", "fb", 445, 1, tcpTLSStream) seedHost(t, &model.Host{InboundId: ib.Id, SortOrder: 1, Remark: "A", Address: "a.cdn.com", Port: 443, Security: "tls", Sni: "a.sni"}) @@ -1228,8 +1228,8 @@ func TestSub_ClientHostRule_DeletedFallsBack(t *testing.T) { } }) - // A deleted host id must not silence an inbound without the client's other bound hosts: - // relevance comes from rows that still exist, fail-closed only when every bound id is gone. + // An unknown host id must not silence an inbound without the client's other bound hosts: the + // live and deleted hosts the ids resolve to decide which inbounds the binding guards. t.Run("deleted id on one inbound does not silence an unrelated inbound", func(t *testing.T) { seedSubDB(t) ibA := seedSubInbound(t, "s1", "xa", 4471, 1, tcpTLSStream) @@ -1267,6 +1267,65 @@ func TestSub_ClientHostRule_DeletedFallsBack(t *testing.T) { }) } +// Deleting a client's only bound host blanked it on every inbound, since the gone rows no longer +// said which inbound the binding guarded; only the inbound the host served may close. +func TestSub_ClientHostRule_DeletedHostClosesOnlyItsInbound(t *testing.T) { + for _, tc := range []struct { + name string + subId string + portA int + remove func(t *testing.T, ibA *model.Inbound, groupId string) error + }{ + {"by its host group", "dg", 4481, func(_ *testing.T, _ *model.Inbound, groupId string) error { + return (&service.HostService{}).DeleteHostGroup(groupId) + }}, + {"with its inbound", "di", 4483, func(t *testing.T, ibA *model.Inbound, _ string) error { + if err := database.GetDB().Model(ibA).Update("enable", false).Error; err != nil { + t.Fatalf("disable inbound A: %v", err) + } + _, err := (&service.InboundService{}).DelInbound(ibA.Id) + return err + }}, + } { + t.Run(tc.name, func(t *testing.T) { + seedSubDB(t) + ibA := seedSubInbound(t, tc.subId, tc.subId+"a", tc.portA, 1, tcpTLSStream) + ibB := seedSubInbound(t, tc.subId, tc.subId+"b", tc.portA+1, 2, tcpTLSStream) + email, groupId := tc.subId+"b@e", tc.subId+"-grp" + + db := database.GetDB() + var client model.ClientRecord + if err := db.Where("email = ?", email).First(&client).Error; err != nil { + t.Fatalf("load client %s: %v", email, err) + } + if err := db.Create(&model.ClientInbound{ClientId: client.Id, InboundId: ibA.Id}).Error; err != nil { + t.Fatalf("link client to inbound A: %v", err) + } + hostA := seedHost(t, &model.Host{GroupId: groupId, InboundId: ibA.Id, SortOrder: 1, Remark: "A", Address: "a.cdn.com", Port: 443, Security: "tls", Sni: "a.sni"}) + if err := db.Model(&model.ClientRecord{}).Where("email = ?", email). + Update("client_host_rule_ids", model.EncodeClientHostRuleIds([]int{hostA.Id})).Error; err != nil { + t.Fatalf("bind host rule: %v", err) + } + if err := tc.remove(t, ibA, groupId); err != nil { + t.Fatalf("delete the bound host: %v", err) + } + + links, _, _, _, err := NewSubService("").GetSubs(tc.subId, "req.example.com") + if err != nil { + t.Fatalf("GetSubs: %v", err) + } + joined := strings.Join(links, "\n") + uuid := "11111111-2222-4333-8444-" + fmt.Sprintf("%012d", ibB.Port) + if strings.Contains(joined, "a.cdn.com") || strings.Contains(joined, fmt.Sprintf("%s@203.0.113.5:%d", uuid, ibA.Port)) { + t.Fatalf("inbound A lost the client's bound host and must render nothing for it: %s", joined) + } + if !strings.Contains(joined, fmt.Sprintf("%s@203.0.113.5:%d", uuid, ibB.Port)) { + t.Fatalf("inbound B never had a binding and must keep the client's link: %s", joined) + } + }) + } +} + // A client bound to a disabled host fails closed on every format (no links, outbounds or proxies) // but stays in traffic accounting, so quota and expiry headers stay correct. func TestSub_ClientHostRule_DisabledFailsClosed(t *testing.T) { diff --git a/internal/web/service/host.go b/internal/web/service/host.go index 1ad65bc60..c4d2c88a1 100644 --- a/internal/web/service/host.go +++ b/internal/web/service/host.go @@ -337,7 +337,7 @@ func (s *HostService) UpdateHostGroup(groupId string, req *entity.HostGroup) ([] } } if len(toDelete) > 0 { - if err := tx.Where("id IN ?", toDelete).Delete(&model.Host{}).Error; err != nil { + if _, err := database.DeleteHosts(tx, "id IN ?", toDelete); err != nil { return err } } @@ -360,7 +360,10 @@ func (s *HostService) UpdateHostGroup(groupId string, req *entity.HostGroup) ([] } func (s *HostService) DeleteHostGroup(groupId string) error { - return database.GetDB().Where("group_id = ?", groupId).Delete(&model.Host{}).Error + return database.GetDB().Transaction(func(tx *gorm.DB) error { + _, err := database.DeleteHosts(tx, "group_id = ?", groupId) + return err + }) } func (s *HostService) SetHostGroupEnable(groupId string, enable bool) error { @@ -378,7 +381,10 @@ func (s *HostService) DeleteHostsGroup(groupIds []string) error { if len(groupIds) == 0 { return nil } - return database.GetDB().Where("group_id IN ?", groupIds).Delete(&model.Host{}).Error + return database.GetDB().Transaction(func(tx *gorm.DB) error { + _, err := database.DeleteHosts(tx, "group_id IN ?", groupIds) + return err + }) } func (s *HostService) ReorderHostGroups(groupIds []string) error { diff --git a/internal/web/service/inbound.go b/internal/web/service/inbound.go index 872a41669..31d9dd4d6 100644 --- a/internal/web/service/inbound.go +++ b/internal/web/service/inbound.go @@ -1665,7 +1665,7 @@ func (s *InboundService) delInbound(id int) (bool, func(), error) { if err := tx.Delete(model.Inbound{}, id).Error; err != nil { return err } - if err := tx.Where("inbound_id = ?", id).Delete(&model.Host{}).Error; err != nil { + if _, err := database.DeleteHosts(tx, "inbound_id = ?", id); err != nil { return err } // Drop the deleted inbound from any sub-balancer that selects it; a diff --git a/internal/web/service/inbound_traffic.go b/internal/web/service/inbound_traffic.go index c0f76f772..fb8129377 100644 --- a/internal/web/service/inbound_traffic.go +++ b/internal/web/service/inbound_traffic.go @@ -964,7 +964,7 @@ func (s *InboundService) DelDepletedClients(id int) (err error) { if err := s.clientService.DetachInbound(tx, inbound.Id); err != nil { return err } - if err := tx.Where("inbound_id = ?", inbound.Id).Delete(&model.Host{}).Error; err != nil { + if _, err := database.DeleteHosts(tx, "inbound_id = ?", inbound.Id); err != nil { return err } if err := tx.Delete(model.Inbound{}, inbound.Id).Error; err != nil {