From 2d8d304850f718b462a1b473dc5b7ceedc8cbef4 Mon Sep 17 00:00:00 2001 From: BlindMaster24 <375291171150z@gmail.com> Date: Tue, 15 Sep 2026 11:33:02 +0300 Subject: [PATCH] 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. --- .../inbound_amneziawg_relay_window_test.go | 27 +++++++++++++++++++ internal/web/service/port_conflict.go | 6 +++-- internal/web/service/port_conflict_test.go | 23 ---------------- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/internal/web/service/inbound_amneziawg_relay_window_test.go b/internal/web/service/inbound_amneziawg_relay_window_test.go index c61297cb7..08f4de9c6 100644 --- a/internal/web/service/inbound_amneziawg_relay_window_test.go +++ b/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) { diff --git a/internal/web/service/port_conflict.go b/internal/web/service/port_conflict.go index db4340235..ced33a368 100644 --- a/internal/web/service/port_conflict.go +++ b/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) } diff --git a/internal/web/service/port_conflict_test.go b/internal/web/service/port_conflict_test.go index 718b686fa..3891a7709 100644 --- a/internal/web/service/port_conflict_test.go +++ b/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,