Browse Source

fix(xray): validate generated egress targets (#5989)

* fix(xray): validate panel egress target

Avoid generating a loopback panel bridge and routing rule when a saved panel outbound disappears after an outbound subscription refresh. Preserve routing unchanged and log the missing target instead.

* fix(xray): guard node and mtproto egress

Apply the fail-closed target check to every generated egress bridge. Skip node and MTProto bridge injection when a selected outbound disappears or the relevant JSON cannot be parsed.

* test(xray): complete node egress coverage

Cover tag and port collisions plus absent and malformed routing. Clarify the fail-closed bridge behavior in the panel and MTProto egress documentation.
Seyed Ali Shahrokhi 17 hours ago
parent
commit
79e65f63df
2 changed files with 243 additions and 30 deletions
  1. 58 29
      internal/web/service/xray.go
  2. 185 1
      internal/web/service/xray_config_inject_test.go

+ 58 - 29
internal/web/service/xray.go

@@ -354,10 +354,10 @@ const PanelEgressInboundTag = "panel-egress"
 // already taken by other inbounds in the generated config are skipped.
 const panelEgressBasePort = 62790
 
-// injectPanelEgress appends a loopback SOCKS inbound to the generated config
-// and prepends a routing rule sending it to outboundTag. Both live only in the
-// generated config — the stored template is never modified — and both are
-// hot-appliable, so changing the panel outbound never restarts the core.
+// injectPanelEgress appends a loopback SOCKS inbound and routing rule only when
+// outboundTag resolves in the final outbound or balancer set. Otherwise the
+// entire injection is skipped. Generated state is hot-appliable and never
+// modifies the stored template or restarts the core.
 func injectPanelEgress(cfg *xray.Config, outboundTag string) {
 	for i := range cfg.InboundConfigs {
 		if cfg.InboundConfigs[i].Tag == PanelEgressInboundTag {
@@ -375,6 +375,10 @@ func injectPanelEgress(cfg *xray.Config, outboundTag string) {
 			return
 		}
 	}
+	if !routingTargetExists(routing, cfg.OutboundConfigs, outboundTag) {
+		logger.Warning("panel egress: target tag [", outboundTag, "] not found, skipping injection")
+		return
+	}
 	rules, _ := routing["rules"].([]any)
 	rule := map[string]any{
 		"type":       "field",
@@ -418,6 +422,25 @@ func injectPanelEgress(cfg *xray.Config, outboundTag string) {
 	})
 }
 
+func outboundTagExists(outbounds json_util.RawMessage, tag string) bool {
+	var parsed []struct {
+		Tag string `json:"tag"`
+	}
+	if tag == "" || json.Unmarshal(outbounds, &parsed) != nil {
+		return false
+	}
+	for _, outbound := range parsed {
+		if outbound.Tag == tag {
+			return true
+		}
+	}
+	return false
+}
+
+func routingTargetExists(routing map[string]any, outbounds json_util.RawMessage, tag string) bool {
+	return routingTagIsBalancer(routing, tag) || outboundTagExists(outbounds, tag)
+}
+
 // NodeEgressInboundTag returns the loopback SOCKS inbound tag for a given node.
 func NodeEgressInboundTag(nodeID int) string {
 	return fmt.Sprintf("node-egress-%d", nodeID)
@@ -452,6 +475,10 @@ func injectNodeEgresses(cfg *xray.Config, nodes []*model.Node) {
 		if !n.Enable || n.OutboundTag == "" {
 			continue
 		}
+		if !routingTargetExists(routing, cfg.OutboundConfigs, n.OutboundTag) {
+			logger.Warning("node egress: target tag [", n.OutboundTag, "] not found, skipping node [", n.Id, "]")
+			continue
+		}
 		tag := NodeEgressInboundTag(n.Id)
 		if _, exists := usedTags[tag]; exists {
 			logger.Warning("node egress: inbound tag [", tag, "] already exists, skipping")
@@ -529,11 +556,11 @@ func routingTagIsBalancer(routing map[string]any, tag string) bool {
 const mtprotoEgressSocksSettings = `{"auth":"noauth","udp":false}`
 
 // injectMtprotoEgress wires one routed mtproto inbound into the generated
-// config: it appends a loopback SOCKS inbound (tagged with the inbound's own tag,
-// on the egress port persisted in settings) and, when an outbound is selected,
-// prepends a routing rule sending that tag to it. Both live only in the generated
-// config — the stored template is untouched — and both are hot-appliable, so
-// toggling routing never forces a full Xray restart. Mirrors injectPanelEgress.
+// config after any selected outbound resolves in the final target set. Invalid
+// selected targets or routing data skip the entire injection; without a selected
+// outbound, the bridge retains default-route behavior. Generated state remains
+// hot-appliable, leaves the stored template untouched, and never forces a full
+// Xray restart. Mirrors injectPanelEgress.
 func injectMtprotoEgress(cfg *xray.Config, inbound *model.Inbound) {
 	var parsed struct {
 		RouteThroughXray bool   `json:"routeThroughXray"`
@@ -556,31 +583,33 @@ func injectMtprotoEgress(cfg *xray.Config, inbound *model.Inbound) {
 
 	if parsed.OutboundTag != "" {
 		routing := map[string]any{}
-		parseOK := true
 		if len(cfg.RouterConfig) > 0 {
 			if err := json.Unmarshal(cfg.RouterConfig, &routing); err != nil {
-				logger.Warning("mtproto egress: routing section is unparsable, skipping rule:", err)
-				parseOK = false
+				logger.Warning("mtproto egress: routing section is unparsable, skipping injection:", err)
+				return
 			}
 		}
-		if parseOK {
-			rules, _ := routing["rules"].([]any)
-			rule := map[string]any{
-				"type":       "field",
-				"inboundTag": []any{tag},
-			}
-			if routingTagIsBalancer(routing, parsed.OutboundTag) {
-				rule["balancerTag"] = parsed.OutboundTag
-			} else {
-				rule["outboundTag"] = parsed.OutboundTag
-			}
-			routing["rules"] = append([]any{rule}, rules...)
-			if newRouting, err := json.Marshal(routing); err == nil {
-				cfg.RouterConfig = json_util.RawMessage(newRouting)
-			} else {
-				logger.Warning("mtproto egress: failed to rebuild routing section, skipping rule:", err)
-			}
+		if !routingTargetExists(routing, cfg.OutboundConfigs, parsed.OutboundTag) {
+			logger.Warning("mtproto egress: target tag [", parsed.OutboundTag, "] not found, skipping injection")
+			return
+		}
+		rules, _ := routing["rules"].([]any)
+		rule := map[string]any{
+			"type":       "field",
+			"inboundTag": []any{tag},
+		}
+		if routingTagIsBalancer(routing, parsed.OutboundTag) {
+			rule["balancerTag"] = parsed.OutboundTag
+		} else {
+			rule["outboundTag"] = parsed.OutboundTag
+		}
+		routing["rules"] = append([]any{rule}, rules...)
+		newRouting, err := json.Marshal(routing)
+		if err != nil {
+			logger.Warning("mtproto egress: failed to rebuild routing section, skipping injection:", err)
+			return
 		}
+		cfg.RouterConfig = json_util.RawMessage(newRouting)
 	}
 
 	cfg.InboundConfigs = append(cfg.InboundConfigs, xray.InboundConfig{

+ 185 - 1
internal/web/service/xray_config_inject_test.go

@@ -127,7 +127,8 @@ func TestEnsureStatsPolicy(t *testing.T) {
 
 func egressTestConfig() *xray.Config {
 	return &xray.Config{
-		RouterConfig: json_util.RawMessage(`{"domainStrategy":"AsIs","rules":[{"type":"field","inboundTag":["api"],"outboundTag":"api"}]}`),
+		RouterConfig:    json_util.RawMessage(`{"domainStrategy":"AsIs","rules":[{"type":"field","inboundTag":["api"],"outboundTag":"api"}]}`),
+		OutboundConfigs: json_util.RawMessage(`[{"protocol":"freedom","tag":"direct"},{"protocol":"socks","tag":"warp"}]`),
 		InboundConfigs: []xray.InboundConfig{
 			{Port: 62789, Protocol: "tunnel", Tag: "api", Listen: json_util.RawMessage(`"127.0.0.1"`)},
 		},
@@ -278,6 +279,146 @@ func TestInjectPanelEgress_BadRoutingSkips(t *testing.T) {
 	}
 }
 
+func TestInjectPanelEgress_MissingTargetSkips(t *testing.T) {
+	cfg := egressTestConfig()
+	before := string(cfg.RouterConfig)
+	injectPanelEgress(cfg, "removed-subscription-outbound")
+	if len(cfg.InboundConfigs) != 1 {
+		t.Fatalf("a missing target must not expose the panel bridge, got %+v", cfg.InboundConfigs)
+	}
+	if string(cfg.RouterConfig) != before {
+		t.Fatalf("a missing target must leave routing untouched, got %s", cfg.RouterConfig)
+	}
+}
+
+func TestInjectPanelEgress_BadOutboundsSkips(t *testing.T) {
+	cfg := egressTestConfig()
+	cfg.OutboundConfigs = json_util.RawMessage(`{not json`)
+	before := string(cfg.RouterConfig)
+	injectPanelEgress(cfg, "direct")
+	if len(cfg.InboundConfigs) != 1 {
+		t.Fatalf("unparsable outbounds must not expose the panel bridge, got %+v", cfg.InboundConfigs)
+	}
+	if string(cfg.RouterConfig) != before {
+		t.Fatalf("unparsable outbounds must leave routing untouched, got %s", cfg.RouterConfig)
+	}
+}
+
+func TestInjectNodeEgresses_MissingTargetSkips(t *testing.T) {
+	cfg := egressTestConfig()
+	injectNodeEgresses(cfg, []*model.Node{
+		{Id: 1, Enable: true, OutboundTag: "removed-subscription-outbound"},
+		{Id: 2, Enable: true, OutboundTag: "warp"},
+	})
+
+	if len(cfg.InboundConfigs) != 2 {
+		t.Fatalf("only the node with a valid target should get a bridge, got %+v", cfg.InboundConfigs)
+	}
+	bridge := cfg.InboundConfigs[1]
+	if bridge.Tag != NodeEgressInboundTag(2) || bridge.Port != nodeEgressBasePort+2 {
+		t.Fatalf("unexpected node egress bridge: %+v", bridge)
+	}
+
+	var routing egressRouting
+	if err := json.Unmarshal(cfg.RouterConfig, &routing); err != nil {
+		t.Fatal(err)
+	}
+	if len(routing.Rules) != 2 || routing.Rules[0].OutboundTag != "warp" ||
+		len(routing.Rules[0].InboundTag) != 1 || routing.Rules[0].InboundTag[0] != NodeEgressInboundTag(2) {
+		t.Fatalf("only the valid node egress rule should be prepended, got %+v", routing.Rules)
+	}
+}
+
+func TestInjectNodeEgresses_BadOutboundsSkips(t *testing.T) {
+	cfg := egressTestConfig()
+	cfg.OutboundConfigs = json_util.RawMessage(`{not json`)
+	before := string(cfg.RouterConfig)
+	injectNodeEgresses(cfg, []*model.Node{{Id: 1, Enable: true, OutboundTag: "direct"}})
+
+	if len(cfg.InboundConfigs) != 1 {
+		t.Fatalf("unparsable outbounds must not expose a node bridge, got %+v", cfg.InboundConfigs)
+	}
+	if string(cfg.RouterConfig) != before {
+		t.Fatalf("unparsable outbounds must leave routing untouched, got %s", cfg.RouterConfig)
+	}
+}
+
+func TestInjectNodeEgresses_BalancerTarget(t *testing.T) {
+	cfg := egressTestConfig()
+	cfg.RouterConfig = json_util.RawMessage(`{"rules":[],"balancers":[{"tag":"lb","selector":["warp"]}]}`)
+	injectNodeEgresses(cfg, []*model.Node{{Id: 1, Enable: true, OutboundTag: "lb"}})
+
+	var routing struct {
+		Rules []struct {
+			OutboundTag string `json:"outboundTag"`
+			BalancerTag string `json:"balancerTag"`
+		} `json:"rules"`
+	}
+	if err := json.Unmarshal(cfg.RouterConfig, &routing); err != nil {
+		t.Fatal(err)
+	}
+	if len(cfg.InboundConfigs) != 2 || len(routing.Rules) != 1 ||
+		routing.Rules[0].BalancerTag != "lb" || routing.Rules[0].OutboundTag != "" {
+		t.Fatalf("a valid balancer target must create the node bridge and rule, got %+v", routing.Rules)
+	}
+}
+
+func TestInjectNodeEgresses_TagCollisionSkips(t *testing.T) {
+	cfg := egressTestConfig()
+	cfg.InboundConfigs = append(cfg.InboundConfigs,
+		xray.InboundConfig{Port: 1234, Protocol: "socks", Tag: NodeEgressInboundTag(1)},
+	)
+	before := string(cfg.RouterConfig)
+	injectNodeEgresses(cfg, []*model.Node{{Id: 1, Enable: true, OutboundTag: "direct"}})
+
+	if len(cfg.InboundConfigs) != 2 || string(cfg.RouterConfig) != before {
+		t.Fatal("an existing node egress tag must make that node injection a no-op")
+	}
+}
+
+func TestInjectNodeEgresses_PortCollision(t *testing.T) {
+	cfg := egressTestConfig()
+	cfg.InboundConfigs = append(cfg.InboundConfigs,
+		xray.InboundConfig{Port: nodeEgressBasePort + 1, Protocol: "vless", Tag: "in-1"},
+		xray.InboundConfig{Port: nodeEgressBasePort + 2, Protocol: "vless", Tag: "in-2"},
+	)
+	injectNodeEgresses(cfg, []*model.Node{{Id: 1, Enable: true, OutboundTag: "direct"}})
+
+	bridge := cfg.InboundConfigs[len(cfg.InboundConfigs)-1]
+	if bridge.Tag != NodeEgressInboundTag(1) || bridge.Port != nodeEgressBasePort+3 {
+		t.Fatalf("node egress must skip taken ports, got %+v", bridge)
+	}
+}
+
+func TestInjectNodeEgresses_NoRoutingSection(t *testing.T) {
+	cfg := egressTestConfig()
+	cfg.RouterConfig = nil
+	injectNodeEgresses(cfg, []*model.Node{{Id: 1, Enable: true, OutboundTag: "direct"}})
+
+	var routing egressRouting
+	if err := json.Unmarshal(cfg.RouterConfig, &routing); err != nil {
+		t.Fatal(err)
+	}
+	if len(cfg.InboundConfigs) != 2 || len(routing.Rules) != 1 ||
+		routing.Rules[0].OutboundTag != "direct" ||
+		len(routing.Rules[0].InboundTag) != 1 || routing.Rules[0].InboundTag[0] != NodeEgressInboundTag(1) {
+		t.Fatalf("a routing section must be created with the node egress rule, got %+v", routing.Rules)
+	}
+}
+
+func TestInjectNodeEgresses_BadRoutingSkips(t *testing.T) {
+	cfg := egressTestConfig()
+	cfg.RouterConfig = json_util.RawMessage(`{not json`)
+	injectNodeEgresses(cfg, []*model.Node{{Id: 1, Enable: true, OutboundTag: "direct"}})
+
+	if len(cfg.InboundConfigs) != 1 {
+		t.Fatalf("unparsable routing must not expose a node bridge, got %+v", cfg.InboundConfigs)
+	}
+	if string(cfg.RouterConfig) != `{not json` {
+		t.Fatalf("unparsable routing must be left untouched, got %s", cfg.RouterConfig)
+	}
+}
+
 func mtprotoInbound(tag string, settings string) *model.Inbound {
 	return &model.Inbound{Tag: tag, Protocol: model.MTProto, Enable: true, Settings: settings}
 }
@@ -373,3 +514,46 @@ func TestInjectMtprotoEgress_TagCollisionSkips(t *testing.T) {
 		t.Fatal("a real inbound already owning the tag must make the bridge a no-op")
 	}
 }
+
+func TestInjectMtprotoEgress_MissingTargetSkips(t *testing.T) {
+	cfg := egressTestConfig()
+	before := string(cfg.RouterConfig)
+	injectMtprotoEgress(cfg, mtprotoInbound("inbound-443",
+		`{"routeThroughXray":true,"routeXrayPort":50004,"outboundTag":"removed-subscription-outbound"}`))
+
+	if len(cfg.InboundConfigs) != 1 {
+		t.Fatalf("a missing target must not expose the mtproto bridge, got %+v", cfg.InboundConfigs)
+	}
+	if string(cfg.RouterConfig) != before {
+		t.Fatalf("a missing target must leave routing untouched, got %s", cfg.RouterConfig)
+	}
+}
+
+func TestInjectMtprotoEgress_BadOutboundsSkips(t *testing.T) {
+	cfg := egressTestConfig()
+	cfg.OutboundConfigs = json_util.RawMessage(`{not json`)
+	before := string(cfg.RouterConfig)
+	injectMtprotoEgress(cfg, mtprotoInbound("inbound-443",
+		`{"routeThroughXray":true,"routeXrayPort":50005,"outboundTag":"direct"}`))
+
+	if len(cfg.InboundConfigs) != 1 {
+		t.Fatalf("unparsable outbounds must not expose the mtproto bridge, got %+v", cfg.InboundConfigs)
+	}
+	if string(cfg.RouterConfig) != before {
+		t.Fatalf("unparsable outbounds must leave routing untouched, got %s", cfg.RouterConfig)
+	}
+}
+
+func TestInjectMtprotoEgress_BadRoutingSkips(t *testing.T) {
+	cfg := egressTestConfig()
+	cfg.RouterConfig = json_util.RawMessage(`{not json`)
+	injectMtprotoEgress(cfg, mtprotoInbound("inbound-443",
+		`{"routeThroughXray":true,"routeXrayPort":50006,"outboundTag":"direct"}`))
+
+	if len(cfg.InboundConfigs) != 1 {
+		t.Fatalf("unparsable routing must not expose the mtproto bridge, got %+v", cfg.InboundConfigs)
+	}
+	if string(cfg.RouterConfig) != `{not json` {
+		t.Fatalf("unparsable routing must be left untouched, got %s", cfg.RouterConfig)
+	}
+}