Переглянути джерело

fix(amneziawg): stop a disabled inbound's relay slot from being taken (#6540)

* test(amneziawg): pin that a disabled row still owns its relay slot

checkAmneziawgnetSocksConflict filters enable = true, so a disabled AmneziaWG
row is not a candidate when an ordinary inbound's configured port is validated.
SetInboundEnable then flips the column with no port check, so enabling that row
later puts a second inbound on 127.0.0.1:65101 and Xray refuses the whole config.
Expected red on this head; the fix follows.

* fix(amneziawg): count a disabled inbound as owning its relay slot

The forward port check filtered its candidates with enable = true, so a disabled
AmneziaWG row was invisible when an ordinary inbound's configured port was
validated. Nothing else covered the gap: the relay is not a database row, and
SetInboundEnable flips the column with no port check, so re-enabling that row put
a second inbound on 127.0.0.1:65101 and made Xray refuse its whole config,
taking every other protocol on the host down with it.

A row owns the slot its id derives for as long as the row exists, which is the
rule the reverse-direction check already follows. TestCheckPortConflict_
DisabledAmneziawgStillOwnsItsRelaySlot fails without this, on a test-only head
whose go-test run failed on exactly that test, and passes with it.

* test(amneziawg): drop the disabled-row case that asserts the reversed rule

TestCheckPortConflict_AmneziawgnetSocksRelayIgnoredWhenDisabled stated, in its
name and its doc comment, that a disabled AmneziaWG inbound's port must not
block anything -- the rule the parent commit reverses. It also never reached the
predicate it named: its fixture seeds Settings: {}, which
amneziawg.InstanceFromInbound rejects on parsed.Server == nil one statement
before the enable column is read, so it passed with or without the filter.

Leaving it would document both rules for the same operator state with nothing
failing to flag the contradiction. The rule this PR pins is covered for real by
TestCheckPortConflict_DisabledAmneziawgStillOwnsItsRelaySlot, whose fixture
carries a qualifying server block and an enabled peer.
BlindMaster24 10 годин тому
батько
коміт
2d8d304850

+ 27 - 0
internal/web/service/inbound_amneziawg_relay_window_test.go

@@ -141,6 +141,33 @@ func TestCheckPortConflict_LocalAmneziawgRelayCollisionBlocksTheEdit(t *testing.
 	}
 }
 
+// A disabled row still owns the relay slot its id derives: SetInboundEnable
+// flips the column with no port check, so enabling it later would break Xray.
+func TestCheckPortConflict_DisabledAmneziawgStillOwnsItsRelaySlot(t *testing.T) {
+	setupConflictDB(t)
+	owner := addAmneziaWGInbound(t, "awg-disabled", 51820, false)
+	relayPort := amneziawgnet.SOCKSPortForInbound(owner.Id)
+
+	got, err := (&InboundService{}).checkPortConflict(&model.Inbound{
+		Tag:      "takes-the-slot",
+		Enable:   true,
+		Listen:   "0.0.0.0",
+		Port:     relayPort,
+		Protocol: model.VLESS,
+		Settings: `{"clients":[]}`,
+	}, 0)
+	if err != nil {
+		t.Fatalf("checkPortConflict: %v", err)
+	}
+	if got == nil {
+		t.Fatalf("inbound #%d is disabled but still owns relay port %d; the save must be refused",
+			owner.Id, relayPort)
+	}
+	if !strings.Contains(got.String(), owner.Tag) {
+		t.Fatalf("the conflict must name the inbound owning the port, got %q", got.String())
+	}
+}
+
 // A row adopted from a node keeps the protocol it arrived with and its central
 // id (inbound_node.go:737), but gets no relay -- so its slot can never be taken.
 func TestCheckPortConflict_NodeAssignedAmneziawgOwnsNoRelaySlot(t *testing.T) {

+ 4 - 2
internal/web/service/port_conflict.go

@@ -274,7 +274,7 @@ func checkPortConflictTx(db *gorm.DB, inbound *model.Inbound, ignoreId int) (*po
 }
 
 // checkAmneziawgnetSocksConflict reports whether inbound's own port
-// collides with an existing, enabled local AmneziaWG inbound's automatic
+// collides with an existing local AmneziaWG inbound's automatic
 // Xray SOCKS5 relay port. Unlike the retired kernel-module bridge this
 // checks every qualifying AmneziaWG inbound unconditionally: the embedded
 // relay has no RouteThroughXray-style opt-in, every one of them gets a
@@ -286,8 +286,10 @@ func checkPortConflictTx(db *gorm.DB, inbound *model.Inbound, ignoreId int) (*po
 // otherwise two concurrent AmneziaWG creates could both pass this check
 // before either row commits.
 func checkAmneziawgnetSocksConflict(db *gorm.DB, inbound *model.Inbound, ignoreId int, newBits transportBits) (*portConflictDetail, error) {
+	// A disabled row still owns the slot its id derives: SetInboundEnable flips
+	// the column with no port check, so enabling it later must not collide.
 	var candidates []*model.Inbound
-	q := db.Model(model.Inbound{}).Where("protocol = ? AND enable = ? AND node_id IS NULL", model.AmneziaWG, true)
+	q := db.Model(model.Inbound{}).Where("protocol = ? AND node_id IS NULL", model.AmneziaWG)
 	if ignoreId > 0 {
 		q = q.Where("id != ?", ignoreId)
 	}

+ 0 - 23
internal/web/service/port_conflict_test.go

@@ -815,29 +815,6 @@ func TestCheckPortConflict_AmneziawgnetSocksRelayAllowedOnNode(t *testing.T) {
 	}
 }
 
-// A disabled AmneziaWG inbound never gets a relay inbound injected
-// (injectAmneziawgnetSocks skips !inbound.Enable), so its "reserved" port
-// must not block anything.
-func TestCheckPortConflict_AmneziawgnetSocksRelayIgnoredWhenDisabled(t *testing.T) {
-	setupConflictDB(t)
-	awg := &model.Inbound{Tag: "awg-1", Enable: false, Listen: "0.0.0.0", Port: 51820, Protocol: model.AmneziaWG, Settings: `{}`}
-	if err := database.GetDB().Create(awg).Error; err != nil {
-		t.Fatalf("seed disabled awg inbound: %v", err)
-	}
-	relayPort := amneziawgnet.SOCKSPortForInbound(awg.Id)
-
-	svc := &InboundService{}
-	candidate := &model.Inbound{
-		Tag:      "vless-bridge",
-		Listen:   "0.0.0.0",
-		Port:     relayPort,
-		Protocol: model.VLESS,
-	}
-	if got, err := svc.checkPortConflict(candidate, 0); err != nil || got != nil {
-		t.Fatalf("a disabled AmneziaWG inbound's port must not be reserved; got=%v err=%v", got, err)
-	}
-}
-
 // Unlike the retired kernel-module bridge, the embedded relay has no
 // RouteThroughXray-style opt-in -- every qualifying AmneziaWG inbound
 // reserves its relay port regardless of that (now-vestigial) field's value,