From ea926826fbed912a228b7456f3b4103282552d21 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sun, 24 May 2026 22:45:14 +0200 Subject: [PATCH] fix: hash-storage panic on SIGHUP and seeder dup-key on cold restart (#4539) Two bugs that combine into an unrecoverable crash loop after a user enables the Telegram bot in settings on a fresh install. 1. CheckHashStorageJob.Run panics with a nil pointer dereference. The cron job is scheduled whenever settings say the bot is enabled, but the package-level hash storage is only initialized inside Tgbot.Start, which StartPanelOnly intentionally skips (startTgBot=false). Toggling the bot on via the panel triggers SIGHUP, the storage stays nil, and the cron fires 2 minutes later and panics, exiting 2. 2. seedClientsFromInboundJSON is not idempotent. The fresh-install early-return path recorded only UserPasswordHash + ApiTokensTable, never ClientsTable. After the admin adds clients via the panel (which writes to the clients table through SyncInbound), the next start runs the seeder for the first time, finds matching emails already in the table, and fails with SQLSTATE 23505 on idx_clients_email, turning the panic above into an unrecoverable crash loop on PostgreSQL. Fixes: - web/job/check_hash_storage.go: nil-check the storage before calling RemoveExpiredHashes. - database/db.go: in the fresh-install early-return path, also record ClientsTable so the seeder never re-runs against panel-added data. - database/db.go: hydrate seedClientsFromInboundJSON's byEmail cache from existing rows so it merges instead of inserting when a row with the same email already lives in the clients table. Regression tests cover both paths. Closes #4539 --- database/db.go | 18 +++++--- database/db_seed_test.go | 71 ++++++++++++++++++++++++++++++ web/job/check_hash_storage.go | 7 ++- web/job/check_hash_storage_test.go | 12 +++++ 4 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 database/db_seed_test.go create mode 100644 web/job/check_hash_storage_test.go diff --git a/database/db.go b/database/db.go index fba55a3c..10e729c3 100644 --- a/database/db.go +++ b/database/db.go @@ -142,11 +142,11 @@ func runSeeders(isUsersEmpty bool) error { } if empty && isUsersEmpty { - hashSeeder := &model.HistoryOfSeeders{ - SeederName: "UserPasswordHash", - } - if err := db.Create(hashSeeder).Error; err != nil { - return err + seeders := []string{"UserPasswordHash", "ClientsTable"} + for _, name := range seeders { + if err := db.Create(&model.HistoryOfSeeders{SeederName: name}).Error; err != nil { + return err + } } return seedApiTokens() } @@ -237,6 +237,14 @@ func seedClientsFromInboundJSON() error { return db.Transaction(func(tx *gorm.DB) error { byEmail := map[string]*model.ClientRecord{} + var existing []model.ClientRecord + if err := tx.Find(&existing).Error; err != nil { + return err + } + for i := range existing { + byEmail[existing[i].Email] = &existing[i] + } + for _, inbound := range inbounds { if strings.TrimSpace(inbound.Settings) == "" { continue diff --git a/database/db_seed_test.go b/database/db_seed_test.go new file mode 100644 index 00000000..d648774b --- /dev/null +++ b/database/db_seed_test.go @@ -0,0 +1,71 @@ +package database + +import ( + "encoding/json" + "path/filepath" + "testing" + + "github.com/mhsanaei/3x-ui/v3/database/model" +) + +func TestSeedClientsFromInboundJSON_IsIdempotentAgainstExistingClients(t *testing.T) { + dbDir := t.TempDir() + t.Setenv("XUI_DB_FOLDER", dbDir) + if err := InitDB(filepath.Join(dbDir, "3x-ui.db")); err != nil { + t.Fatalf("InitDB failed: %v", err) + } + t.Cleanup(func() { _ = CloseDB() }) + + settings, err := json.Marshal(map[string]any{ + "clients": []any{ + map[string]any{ + "id": "ce8d33df-3a64-4f10-8f9b-91c3a8e0c001", + "email": "alice@example.com", + "enable": true, + "flow": "", + "subId": "alice-sub", + "comment": "from-inbound-json", + }, + }, + }) + if err != nil { + t.Fatalf("marshal settings: %v", err) + } + inbound := model.Inbound{ + UserId: 1, + Port: 12345, + Protocol: model.VLESS, + Settings: string(settings), + Tag: "test-inbound", + } + if err := db.Create(&inbound).Error; err != nil { + t.Fatalf("seed inbound: %v", err) + } + + preExisting := &model.ClientRecord{ + Email: "alice@example.com", + UUID: "ce8d33df-3a64-4f10-8f9b-91c3a8e0c001", + SubID: "alice-sub", + Enable: true, + Comment: "added-via-api", + } + if err := db.Create(preExisting).Error; err != nil { + t.Fatalf("seed client row: %v", err) + } + + if err := db.Where("seeder_name = ?", "ClientsTable").Delete(&model.HistoryOfSeeders{}).Error; err != nil { + t.Fatalf("clear ClientsTable history: %v", err) + } + + if err := seedClientsFromInboundJSON(); err != nil { + t.Fatalf("seedClientsFromInboundJSON should be idempotent against existing rows, got: %v", err) + } + + var count int64 + if err := db.Model(&model.ClientRecord{}).Where("email = ?", "alice@example.com").Count(&count).Error; err != nil { + t.Fatalf("count clients: %v", err) + } + if count != 1 { + t.Fatalf("alice@example.com should resolve to exactly one row, got %d", count) + } +} diff --git a/web/job/check_hash_storage.go b/web/job/check_hash_storage.go index 1489217e..ce836831 100644 --- a/web/job/check_hash_storage.go +++ b/web/job/check_hash_storage.go @@ -16,6 +16,9 @@ func NewCheckHashStorageJob() *CheckHashStorageJob { // Run removes expired hash entries from the Telegram bot's hash storage. func (j *CheckHashStorageJob) Run() { - // Remove expired hashes from storage - j.tgbotService.GetHashStorage().RemoveExpiredHashes() + storage := j.tgbotService.GetHashStorage() + if storage == nil { + return + } + storage.RemoveExpiredHashes() } diff --git a/web/job/check_hash_storage_test.go b/web/job/check_hash_storage_test.go new file mode 100644 index 00000000..e324bcb8 --- /dev/null +++ b/web/job/check_hash_storage_test.go @@ -0,0 +1,12 @@ +package job + +import "testing" + +func TestCheckHashStorageJob_RunWithoutPanicWhenStorageNil(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("CheckHashStorageJob.Run panicked when storage is nil: %v", r) + } + }() + NewCheckHashStorageJob().Run() +}