fix(xray): refuse a config the running core cannot bind (#6547)

* fix(xray): refuse a config the running core cannot bind RestartXray stopped a working core before handing it a config whose listens collide, so the failed bind exited the whole process (main/run.go:94) and the one-second watchdog retried it in a loop: every protocol down, cause only in the logs. The save-time port guards cannot cover this -- SetInboundEnable, the AmneziaWG relay created on the first peer, template and bridge edits all reach a colliding config with no guard on that path. Probe the generated config at the single restart funnel instead. Collisions the running core already serves are excused, so an established setup is never refused by a static read being wrong about it, and the port-bucketed pass costs nothing on a clean config. * fix(xray): surface a refused config and re-key the bind excuse set Round-1 findings on this PR. Refusing the swap left the running core on its previous config with nothing but a log line to show for it, so the status response now carries the reason while the core runs and the overview marks it; the node list picks the same field up through that response. The excuse set is keyed on the two listens, the port and the shared transports instead of the tag pair, so a pair whose listen moves onto the other's address is refused again, while the same two sockets stay excused however the generator orders them. TestBindConflicts/excused_pair_whose_listen_changed_into_a_real_collision fails without the key change -- watched red first.
2026-09-16 15:17:14 +00:00 · 2026-09-15 15:27:28 +03:00
parent 43e64993fc
commit baef3cdd07
5 changed files with 431 additions and 2 deletions
@@ -627,7 +627,9 @@ func (s *ServerService) GetStatus(lastStatus *Status) *Status {
 	// Xray status
 	if s.xrayService.IsXrayRunning() {
 		status.Xray.State = Running
-		status.Xray.ErrorMsg = ""
+		// A core that runs but was refused the new config is a fault the
+		// operator only ever sees here and in the node list.
+		status.Xray.ErrorMsg = s.xrayService.GetHeldBackConfig()
 	} else {
 		err := s.xrayService.GetXrayErr()
 		if err != nil {
@@ -33,6 +33,8 @@ type xrayLifecycle struct {
 	mu      sync.RWMutex
 	process *xray.Process
 	result  string
+	// heldBack is why the running core still serves the previous config.
+	heldBack string
 }

 func (s *xrayLifecycle) snapshot() (*xray.Process, string) {
@@ -45,9 +47,22 @@ func (s *xrayLifecycle) replace(process *xray.Process) {
 	s.mu.Lock()
 	s.process = process
 	s.result = ""
+	s.heldBack = ""
 	s.mu.Unlock()
 }

+func (s *xrayLifecycle) holdBack(reason string) {
+	s.mu.Lock()
+	s.heldBack = reason
+	s.mu.Unlock()
+}
+
+func (s *xrayLifecycle) heldBackReason() string {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	return s.heldBack
+}
+
 func (s *xrayLifecycle) storeResult(process *xray.Process, result string) {
 	s.mu.Lock()
 	if s.process == process && s.result == "" {
@@ -104,6 +119,12 @@ func (s *XrayService) GetXrayErr() error {
 	return err
 }

+// GetHeldBackConfig returns why the running core still serves its previous
+// config, or "" when the pending config was applied.
+func (s *XrayService) GetHeldBackConfig() string {
+	return xrayState.heldBackReason()
+}
+
 // GetXrayResult returns the result string from the Xray process.
 func (s *XrayService) GetXrayResult() string {
 	process, cachedResult := xrayState.snapshot()
@@ -1373,11 +1394,27 @@ func (s *XrayService) RestartXray(isForce bool) error {
 			logger.Debug("It does not need to restart Xray")
 			return nil
 		}
+		// A config the core cannot bind never replaces one that works: its failed
+		// start exits the core, and the watchdog would then loop on it forever.
+		if conflicts := bindConflicts(xrayConfig, process.GetConfig()); len(conflicts) > 0 {
+			refused := fmt.Sprintf("config refused: %s", conflicts[0])
+			for _, conflict := range conflicts {
+				logger.Error("xray config refused:", conflict.String())
+			}
+			// The refusal is otherwise invisible: the operator's request
+			// succeeded, so the status page has to carry the stale state.
+			xrayState.holdBack(refused)
+			return fmt.Errorf("xray %s", refused)
+		}
 		if !isForce && !configUnchanged && s.tryHotApply(process, xrayConfig) {
 			logger.Info("Xray config changes applied through the core API, no restart needed")
 			return nil
 		}
 		_ = process.Stop()
+	} else if conflicts := bindConflicts(xrayConfig, nil); len(conflicts) > 0 {
+		// Nothing is running to protect and the core is the authority on what it
+		// can bind: start it and let its own error name the port it lost.
+		logger.Warning("xray config may not start:", conflicts[0].String())
 	}

 	process = xray.NewProcess(xrayConfig)
@@ -0,0 +1,147 @@
+package service
+
+import (
+	"cmp"
+	"encoding/json"
+	"fmt"
+	"slices"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+	"github.com/mhsanaei/3x-ui/v3/internal/util/json_util"
+	"github.com/mhsanaei/3x-ui/v3/internal/xray"
+)
+
+// bindConflict names two generated inbounds whose listens cannot coexist.
+type bindConflict struct {
+	tagA    string
+	tagB    string
+	listen  string
+	listenB string
+	port    int
+	shared  transportBits
+}
+
+func (c bindConflict) String() string {
+	return fmt.Sprintf("inbounds %q and %q both bind %s:%d (%s)",
+		c.tagA, c.tagB, displayListen(c.listen), c.port, transportTagSuffix(c.shared))
+}
+
+// bindConflicts reports inbounds of newCfg whose sockets collide. A collision the
+// running config already serves is excused: it is demonstration, not a guess.
+func bindConflicts(newCfg, runningCfg *xray.Config) []bindConflict {
+	conflicts := rawBindConflicts(newCfg)
+	if len(conflicts) == 0 {
+		return nil
+	}
+	excused := runningBindPairs(runningCfg)
+	if len(excused) == 0 {
+		return conflicts
+	}
+	kept := make([]bindConflict, 0, len(conflicts))
+	for _, c := range conflicts {
+		if _, ok := excused[bindPairKey(c)]; !ok {
+			kept = append(kept, c)
+		}
+	}
+	return kept
+}
+
+// rawBindConflicts groups inbounds by port first: only a port two inbounds share
+// is worth parsing transports for, which keeps the probe free on clean configs.
+func rawBindConflicts(cfg *xray.Config) []bindConflict {
+	if cfg == nil {
+		return nil
+	}
+	byPort := make(map[int][]*xray.InboundConfig, len(cfg.InboundConfigs))
+	for i := range cfg.InboundConfigs {
+		ib := &cfg.InboundConfigs[i]
+		if ib.Port > 0 {
+			byPort[ib.Port] = append(byPort[ib.Port], ib)
+		}
+	}
+
+	var conflicts []bindConflict
+	for port, group := range byPort {
+		for i := range group {
+			for j := i + 1; j < len(group); j++ {
+				left, right := group[i], group[j]
+				listenLeft, listenRight := configListen(left.Listen), configListen(right.Listen)
+				if !listenOverlaps(listenLeft, listenRight) {
+					continue
+				}
+				// One port carrying tcp on one inbound and udp on another is a
+				// supported deployment (vless/tcp + hysteria2/udp), never a clash.
+				shared := configTransports(left) & configTransports(right)
+				if shared == 0 {
+					continue
+				}
+				conflicts = append(conflicts, bindConflict{
+					tagA:    left.Tag,
+					tagB:    right.Tag,
+					listen:  listenLeft,
+					listenB: listenRight,
+					port:    port,
+					shared:  shared,
+				})
+			}
+		}
+	}
+	// Port grouping iterates a map: order the report so the first conflict the
+	// caller shows is the same on every restart.
+	slices.SortFunc(conflicts, func(a, b bindConflict) int {
+		return cmp.Or(cmp.Compare(a.port, b.port), cmp.Compare(a.tagA, b.tagA), cmp.Compare(a.tagB, b.tagB))
+	})
+	return conflicts
+}
+
+// runningBindPairs is what the core is demonstrably binding right now, keyed the
+// way a new config's conflicts are, so only the identical one is excused.
+func runningBindPairs(cfg *xray.Config) map[string]struct{} {
+	conflicts := rawBindConflicts(cfg)
+	if len(conflicts) == 0 {
+		return nil
+	}
+	pairs := make(map[string]struct{}, len(conflicts))
+	for _, c := range conflicts {
+		pairs[bindPairKey(c)] = struct{}{}
+	}
+	return pairs
+}
+
+// bindPairKey is the socket set an excuse was granted for: the two listens, the
+// port and the shared transports, never the tags, which the generator reorders.
+func bindPairKey(c bindConflict) string {
+	left, right := c.listen, c.listenB
+	if left > right {
+		left, right = right, left
+	}
+	return fmt.Sprintf("%d\x00%s\x00%s\x00%d", c.port, left, right, c.shared)
+}
+
+func displayListen(listen string) string {
+	if isAnyListen(listen) {
+		return "*"
+	}
+	return listen
+}
+
+// configListen decodes a generated inbound's listen field: absent or empty means
+// every address, which is what the core does with an empty listen too.
+func configListen(raw json_util.RawMessage) string {
+	var listen string
+	if len(raw) == 0 || json.Unmarshal(raw, &listen) != nil {
+		return ""
+	}
+	return listen
+}
+
+// configTransports reads a generated inbound's transports through the same rule
+// the save-time guards use, so the two cannot drift apart.
+func configTransports(ib *xray.InboundConfig) transportBits {
+	// "socks" is the panel's own bridge shape (injectAmneziawgnetSocks and
+	// friends): its udp flag lives in settings like a mixed inbound's.
+	if ib.Protocol == "socks" {
+		return inboundTransports(model.Mixed, string(ib.StreamSettings), string(ib.Settings))
+	}
+	return inboundTransports(model.Protocol(ib.Protocol), string(ib.StreamSettings), string(ib.Settings))
+}
@@ -0,0 +1,237 @@
+package service
+
+import (
+	"encoding/json"
+	"strconv"
+	"strings"
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/amneziawgnet"
+	"github.com/mhsanaei/3x-ui/v3/internal/database"
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+	"github.com/mhsanaei/3x-ui/v3/internal/xray"
+)
+
+// configFromInbounds builds the config the way the panel does, from raw JSON:
+// the probe is only worth anything if it parses what is really written.
+func configFromInbounds(t *testing.T, inbounds string) *xray.Config {
+	t.Helper()
+	var cfg xray.Config
+	if err := json.Unmarshal([]byte(`{"inbounds":[`+inbounds+`]}`), &cfg); err != nil {
+		t.Fatalf("build config: %v", err)
+	}
+	return &cfg
+}
+
+func TestBindConflicts(t *testing.T) {
+	const (
+		relay = `{"listen":"127.0.0.1","port":65101,"protocol":"socks","tag":"relay","settings":{"auth":"password","udp":true,"accounts":[]}}`
+		user  = `{"listen":"0.0.0.0","port":65101,"protocol":"vless","tag":"user","streamSettings":{"network":"tcp"}}`
+	)
+
+	cases := []struct {
+		name     string
+		inbounds string
+		running  string
+		want     int
+	}{
+		{
+			"same listen, port and tcp",
+			`{"listen":"0.0.0.0","port":443,"protocol":"vless","tag":"a"},
+			 {"listen":"0.0.0.0","port":443,"protocol":"vmess","tag":"b"}`,
+			``, 1,
+		},
+		{
+			"tcp and udp on one port are legal",
+			`{"listen":"0.0.0.0","port":443,"protocol":"vless","tag":"a"},
+			 {"listen":"0.0.0.0","port":443,"protocol":"hysteria","tag":"b"}`,
+			``, 0,
+		},
+		{
+			"kcp moves vless to udp and frees the port",
+			`{"listen":"0.0.0.0","port":443,"protocol":"vless","tag":"a","streamSettings":{"network":"kcp"}},
+			 {"listen":"0.0.0.0","port":443,"protocol":"vless","tag":"b","streamSettings":{"network":"tcp"}}`,
+			``, 0,
+		},
+		{
+			"wildcard listen overlaps a loopback one",
+			`{"listen":"0.0.0.0","port":8443,"protocol":"vless","tag":"a"},
+			 {"listen":"127.0.0.1","port":8443,"protocol":"trojan","tag":"b"}`,
+			``, 1,
+		},
+		{
+			"absent listen means wildcard",
+			`{"port":8443,"protocol":"vless","tag":"a"},
+			 {"listen":"127.0.0.1","port":8443,"protocol":"trojan","tag":"b"}`,
+			``, 1,
+		},
+		{
+			"distinct loopback addresses do not overlap",
+			`{"listen":"127.0.0.1","port":8443,"protocol":"vless","tag":"a"},
+			 {"listen":"127.0.0.2","port":8443,"protocol":"vless","tag":"b"}`,
+			``, 0,
+		},
+		{
+			"port zero is not a bind",
+			`{"listen":"0.0.0.0","port":0,"protocol":"tunnel","tag":"a"},
+			 {"listen":"0.0.0.0","port":0,"protocol":"vless","tag":"b"}`,
+			``, 0,
+		},
+		{
+			"clean config",
+			`{"listen":"0.0.0.0","port":443,"protocol":"vless","tag":"a"},
+			 {"listen":"127.0.0.1","port":62789,"protocol":"tunnel","tag":"api"},
+			 ` + relay,
+			``, 0,
+		},
+		{
+			// The relay the AmneziaWG family lives on: loopback tcp+udp, on the
+			// same port as a user inbound's tcp.
+			"amneziawg relay against a tcp inbound",
+			relay + "," + user, ``, 1,
+		},
+		{
+			"amneziawg relay against a udp inbound",
+			relay + `,{"listen":"0.0.0.0","port":65101,"protocol":"hysteria","tag":"user"}`,
+			``, 1,
+		},
+		{
+			// Proves the relay's transports come from settings.udp and not from a
+			// blanket "loopback owns everything" rule.
+			"socks bridge without udp coexists with a udp inbound",
+			`{"listen":"127.0.0.1","port":65101,"protocol":"socks","tag":"bridge","settings":{"auth":"noauth"}},
+			 {"listen":"0.0.0.0","port":65101,"protocol":"hysteria","tag":"user"}`,
+			``, 0,
+		},
+		{
+			"reserved api inbound against a user inbound",
+			`{"listen":"127.0.0.1","port":62789,"protocol":"tunnel","tag":"api","settings":{"rewriteAddress":"127.0.0.1"}},
+			 {"listen":"0.0.0.0","port":62789,"protocol":"vless","tag":"user"}`,
+			``, 1,
+		},
+		{
+			// The core is running this pair right now, so whatever a static read
+			// says about it, it binds: an established setup is never refused.
+			"collision the running config already serves",
+			relay + "," + user, relay + "," + user, 0,
+		},
+		{
+			"collision the running config does not have",
+			relay + "," + user, user, 1,
+		},
+		{
+			// `::` and `0.0.0.0` on one port is what bindv6only=1 makes legal, so the
+			// running config excuses it -- but moving one onto the other is not it.
+			"excused pair whose listen changed into a real collision",
+			`{"listen":"0.0.0.0","port":443,"protocol":"vless","tag":"a"},
+			 {"listen":"0.0.0.0","port":443,"protocol":"vless","tag":"b"}`,
+			`{"listen":"::","port":443,"protocol":"vless","tag":"a"},
+			 {"listen":"0.0.0.0","port":443,"protocol":"vless","tag":"b"}`,
+			1,
+		},
+		{
+			// The same pair of sockets is the same evidence, whichever order the
+			// generator happened to emit them in.
+			"excused pair with its listens swapped stays excused",
+			`{"listen":"0.0.0.0","port":443,"protocol":"vless","tag":"a"},
+			 {"listen":"::","port":443,"protocol":"vless","tag":"b"}`,
+			`{"listen":"::","port":443,"protocol":"vless","tag":"a"},
+			 {"listen":"0.0.0.0","port":443,"protocol":"vless","tag":"b"}`,
+			0,
+		},
+		{
+			"same pair on another port is still new",
+			`{"listen":"127.0.0.1","port":65102,"protocol":"socks","tag":"relay","settings":{"auth":"password","udp":true,"accounts":[]}},
+			 {"listen":"0.0.0.0","port":65102,"protocol":"vless","tag":"user","streamSettings":{"network":"tcp"}}`,
+			relay + "," + user, 1,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			var running *xray.Config
+			if tc.running != "" {
+				running = configFromInbounds(t, tc.running)
+			}
+			got := bindConflicts(configFromInbounds(t, tc.inbounds), running)
+			if len(got) != tc.want {
+				t.Fatalf("bindConflicts = %v, want %d conflict(s)", got, tc.want)
+			}
+			for _, c := range got {
+				if c.tagA == "" || c.tagB == "" || c.tagA == c.tagB {
+					t.Fatalf("a conflict must name both tags, got %+v", c)
+				}
+			}
+		})
+	}
+}
+
+func TestBindConflicts_MessageNamesBothSides(t *testing.T) {
+	conflicts := bindConflicts(configFromInbounds(t, `
+		{"listen":"127.0.0.1","port":65101,"protocol":"socks","tag":"relay","settings":{"auth":"password","udp":true,"accounts":[]}},
+		{"listen":"0.0.0.0","port":65101,"protocol":"vless","tag":"user","streamSettings":{"network":"tcp"}}`), nil)
+	if len(conflicts) != 1 {
+		t.Fatalf("want exactly one conflict, got %v", conflicts)
+	}
+	msg := conflicts[0].String()
+	for _, want := range []string{`"relay"`, `"user"`, "127.0.0.1", "65101"} {
+		if !strings.Contains(msg, want) {
+			t.Fatalf("conflict message %q must contain %q", msg, want)
+		}
+	}
+}
+
+// The probe must read what the panel really emits: the AmneziaWG relay is a
+// loopback "socks" inbound whose udp flag lives in settings, not streamSettings.
+func TestBindConflicts_GeneratedConfig(t *testing.T) {
+	cases := []struct {
+		name    string
+		portOff int
+		running bool
+		want    int
+	}{
+		{"user inbound on the relay port", 0, false, 1},
+		{"already running config", 0, true, 0},
+		{"user inbound on a free port", 1, false, 0},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			setupSettingTestDB(t)
+			seedInboundConflict(t, "awg-1", "0.0.0.0", 51820, model.AmneziaWG, ``, amneziawgRoutedSettings)
+
+			var awg model.Inbound
+			if err := database.GetDB().Where("tag = ?", "awg-1").First(&awg).Error; err != nil {
+				t.Fatalf("read seeded row: %v", err)
+			}
+			relayPort := amneziawgnet.SOCKSPortForInbound(awg.Id)
+			seedInboundConflict(t, "user", "0.0.0.0", relayPort+tc.portOff, model.VLESS, `{"network":"tcp"}`, `{}`)
+
+			svc := &XrayService{}
+			cfg, err := svc.GetXrayConfig()
+			if err != nil {
+				t.Fatalf("GetXrayConfig: %v", err)
+			}
+			running := (*xray.Config)(nil)
+			if tc.running {
+				if running, err = svc.GetXrayConfig(); err != nil {
+					t.Fatalf("second GetXrayConfig: %v", err)
+				}
+			}
+
+			got := bindConflicts(cfg, running)
+			if len(got) != tc.want {
+				t.Fatalf("bindConflicts = %v, want %d", got, tc.want)
+			}
+			if tc.want == 0 {
+				return
+			}
+			msg := got[0].String()
+			for _, want := range []string{`"awg-1"`, `"user"`, strconv.Itoa(relayPort)} {
+				if !strings.Contains(msg, want) {
+					t.Fatalf("conflict message %q must contain %q", msg, want)
+				}
+			}
+		})
+	}
+}