From 79e65f63df97ec79fe1144e97290c636937fd2c6 Mon Sep 17 00:00:00 2001 From: Seyed Ali Shahrokhi Date: Tue, 21 Jul 2026 17:27:34 +0330 Subject: [PATCH] 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. --- internal/web/service/xray.go | 87 +++++--- .../web/service/xray_config_inject_test.go | 186 +++++++++++++++++- 2 files changed, 243 insertions(+), 30 deletions(-) diff --git a/internal/web/service/xray.go b/internal/web/service/xray.go index ba0453c75..59aef9aa0 100644 --- a/internal/web/service/xray.go +++ b/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{ diff --git a/internal/web/service/xray_config_inject_test.go b/internal/web/service/xray_config_inject_test.go index 8c770a5c5..456a37cc2 100644 --- a/internal/web/service/xray_config_inject_test.go +++ b/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) + } +}