From 3b9ca47a4ecdda5c81c20fd4a62cd0aafdf41b1c Mon Sep 17 00:00:00 2001 From: n0ctal <4c866w5fn9@privaterelay.appleid.com> Date: Sat, 26 Sep 2026 23:30:25 +0500 Subject: [PATCH] fix(database): keep the legacy tag cleanup from colliding with an existing tag (#6592) * fix(database): avoid legacy inbound tag cleanup collisions * test(database): assert the legacy tag cleanup keeps the migration green The collision guard's test asserted only that the colliding tag was left alone, which an unguarded cleanup also produces: the UPDATE fails on the unique index and the row is unchanged either way. The cleanup shares a transaction with every other requirement, so that failure rolls all of them back on every boot and only reaches the log. Assert the call itself succeeds, which is what actually distinguishes the two. --------- Co-authored-by: n0ctal --- internal/web/service/inbound_migration.go | 16 ++++- .../web/service/inbound_migration_test.go | 60 +++++++++++++++++-- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/internal/web/service/inbound_migration.go b/internal/web/service/inbound_migration.go index a5bedd7f5..3030c4531 100644 --- a/internal/web/service/inbound_migration.go +++ b/internal/web/service/inbound_migration.go @@ -263,11 +263,21 @@ func (s *InboundService) MigrationRequirements() (err error) { // Must be cross-DB: INSTR/REPLACE work on SQLite; Postgres needs position(). tagCleanup := `UPDATE inbounds SET tag = REPLACE(tag, '0.0.0.0:', '') - WHERE INSTR(tag, '0.0.0.0:') > 0;` + WHERE INSTR(tag, '0.0.0.0:') > 0 + AND NOT EXISTS ( + SELECT 1 FROM inbounds AS other + WHERE other.id <> inbounds.id + AND other.tag = REPLACE(inbounds.tag, '0.0.0.0:', '') + );` if database.IsPostgres() { - tagCleanup = `UPDATE inbounds + tagCleanup = `UPDATE inbounds AS i SET tag = REPLACE(tag, '0.0.0.0:', '') - WHERE position('0.0.0.0:' in tag) > 0;` + WHERE position('0.0.0.0:' in tag) > 0 + AND NOT EXISTS ( + SELECT 1 FROM inbounds AS other + WHERE other.id <> i.id + AND other.tag = REPLACE(i.tag, '0.0.0.0:', '') + );` } err = tx.Exec(tagCleanup).Error if err != nil { diff --git a/internal/web/service/inbound_migration_test.go b/internal/web/service/inbound_migration_test.go index c7340c92b..b30867edc 100644 --- a/internal/web/service/inbound_migration_test.go +++ b/internal/web/service/inbound_migration_test.go @@ -144,9 +144,9 @@ func TestMigrationRequirements_CleansLegacyZeroAddrTag(t *testing.T) { db := database.GetDB() legacy := &model.Inbound{ UserId: 1, - Tag: "inbound-0.0.0.0:30002", + Tag: "inbound-0.0.0.0:30003", Enable: true, - Port: 30002, + Port: 30003, Protocol: model.VLESS, Settings: `{"clients":[]}`, StreamSettings: `{"security":"tls","tlsSettings":{"settings":{"domains":[{"domain":"example.com"}]}}}`, @@ -162,8 +162,60 @@ func TestMigrationRequirements_CleansLegacyZeroAddrTag(t *testing.T) { if err := db.First(&got, legacy.Id).Error; err != nil { t.Fatalf("reload inbound: %v", err) } - if got.Tag != "inbound-30002" { - t.Fatalf("legacy 0.0.0.0: tag not stripped: got %q, want %q", got.Tag, "inbound-30002") + if got.Tag != "inbound-30003" { + t.Fatalf("legacy 0.0.0.0: tag not stripped: got %q, want %q", got.Tag, "inbound-30003") + } +} + +func TestMigrationRequirements_SkipsLegacyZeroAddrTagCollision(t *testing.T) { + setupConflictDB(t) + db := database.GetDB() + + existing := &model.Inbound{ + UserId: 1, + Tag: "inbound-30004", + Enable: true, + Port: 30004, + Protocol: model.VLESS, + Settings: `{"clients":[]}`, + StreamSettings: `{"network":"tcp","security":"none"}`, + } + legacy := &model.Inbound{ + UserId: 1, + Tag: "inbound-0.0.0.0:30004", + Enable: true, + Port: 30005, + Protocol: model.VLESS, + Settings: `{"clients":[]}`, + StreamSettings: `{"security":"tls","tlsSettings":{"settings":{"domains":[{"domain":"example.com"}]}}}`, + } + if err := db.Create(existing).Error; err != nil { + t.Fatalf("create existing inbound: %v", err) + } + if err := db.Create(legacy).Error; err != nil { + t.Fatalf("create legacy inbound: %v", err) + } + + svc := InboundService{} + // The cleanup shares a transaction with every other requirement, so a unique + // violation here rolls all of them back and only reaches the log. + if err := svc.MigrationRequirements(); err != nil { + t.Fatalf("MigrationRequirements: %v", err) + } + + var got model.Inbound + if err := db.First(&got, legacy.Id).Error; err != nil { + t.Fatalf("reload legacy inbound: %v", err) + } + if got.Tag != "inbound-0.0.0.0:30004" { + t.Fatalf("colliding legacy tag should be left unchanged, got %q", got.Tag) + } + var count int64 + if err := db.Model(&model.Inbound{}).Where("tag = ?", "inbound-30004").Count(&count).Error; err != nil { + t.Fatalf("count existing tag: %v", err) + } + if count != 1 { + t.Fatalf("target tag count = %d, want 1", count) } }