fix(routing): allow dns.servers on private IPs past the geoip:private block rule (#5774)

* fix(routing): allow dns.servers on private IPs past the geoip:private block rule

Xray's own DNS client traffic is dispatched through the same routing
table as proxied client traffic. When dns.servers points at a private
IP (e.g. a self-hosted AdGuard Home / Pi-hole reachable on the same
Docker network as Xray) and the panel's default geoip:private block
rule is active, Xray's own DNS lookups get silently dropped. Xray then
falls back to dialing destinations by raw hostname once its internal
DNS attempt times out (~4s), so proxied connections still work, just
with a multi-second stall added to every new domain-based connection,
with no error surfaced anywhere.

EnsureDnsServerRouting keeps a managed "direct" allow-rule for any
private literal IP found in dns.servers, inserted immediately before
the geoip:private block rule (matched by shape, not position). It only
acts when both ingredients are present, keeps the managed rule in sync
as dns.servers changes across saves, and never touches manually
authored rules.

Fixes #5773

* fix(routing): scope the DNS allow-rule to its port, guard against reorder/UI drift

Addresses three review findings on the initial fix:

1. The allow-rule now carries a "port" matcher (grouped by the
   dns.servers entries that share it), instead of opening every port
   on the private DNS IP to proxy-client traffic. A private resolver
   that also exposes an unauthenticated admin UI on the same address
   would otherwise become reachable through the proxy too.

2. EnsureDnsServerRouting now strips every previously-managed rule and
   rebuilds the current set fresh, reinserted immediately before the
   (re-indexed) geoip:private block rule on every save. Comparing IP
   content alone missed the case where an admin drags the rule below
   the block rule in the Routing tab (or reorders something else and
   incidentally moves it) — silently reintroducing the exact stall
   this fix addresses, with nothing to notice or correct it.

3. dnsAllowRuleShape now tolerates an "enabled" key as long as it's
   true, matching the existing EnsureStatsRouting precedent
   (xray_setting.go's `delete(apiRule, "enabled")`). The Routing tab's
   rule editor writes that key on every save regardless of whether
   anything changed, and its enabled switch writes it on a plain
   toggle — without this, either action permanently disowns the rule
   from management and a duplicate gets inserted next save. A rule
   explicitly disabled (enabled=false) is left alone and a fresh one
   is (re-)created, respecting the admin's choice instead of silently
   re-enabling it.

No-op detection now compares rebuilt rules against the original
routing.rules JSON (both decoded through encoding/json to a common
type) rather than reflect.DeepEqual on the parsed Go values, which
falsely reported changes for identical content stored as []any vs
[]string.

5 new tests cover multi-port grouping, position drift, and both
enabled-key cases; existing tests updated for the port field.

* fix: avoid size-computation overflow in allocation hint

CodeQL flagged make([]map[string]any, 0, len(clean)+len(managed)) as a
potential integer-overflow risk in the capacity computation. Drop the
addition and hint with len(clean) alone — it already covers most of
the eventual size, and append still grows correctly for the rest.

---------

Co-authored-by: Volov <[email protected]>

internal/web/service/xray_setting.go

@@ -29,6 +29,9 @@ func (s *XraySettingService) SaveXraySetting(newXraySettings string) error {
 	if hoisted, err := EnsureStatsRouting(newXraySettings); err == nil {
 		newXraySettings = hoisted
 	}
+	if synced, err := EnsureDnsServerRouting(newXraySettings); err == nil {
+		newXraySettings = synced
+	}
 	return s.saveSetting("xrayTemplateConfig", newXraySettings)
 }
 

internal/web/service/xray_setting_dns_routing.go

@@ -0,0 +1,386 @@
+package service
+
+import (
+	"encoding/json"
+	"net"
+	"reflect"
+	"sort"
+	"strconv"
+	"strings"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/util/netsafe"
+)
+
+// dnsAllowRuleShape identifies routing rules this file manages: a plain
+// "type=field, ip=[...], port=..., outboundTag=direct" rule with no other
+// matchers. An "enabled" key is tolerated as long as it's true — the
+// Routing tab's rule editor (RuleFormModal.tsx submit()) and its enabled
+// switch (RoutingTab.tsx toggleRule()) always write that key back, even
+// when nothing else changed, so requiring its absence would disown the
+// rule the first time an admin so much as opens it in the UI. A rule
+// toggled off (enabled=false) is treated as no longer ours: the admin
+// explicitly turned it off, and re-enabling it on the next save would
+// silently override that choice.
+//
+// Rules shaped like this are kept in sync with the current dns.servers
+// config on every save; anything else (including rules an admin wrote by
+// hand that happen to also allow-list an IP) is left untouched.
+func dnsAllowRuleShape(rule map[string]any) bool {
+	if t, _ := rule["type"].(string); t != "field" {
+		return false
+	}
+	if out, _ := rule["outboundTag"].(string); out != "direct" {
+		return false
+	}
+	if _, ok := rule["ip"]; !ok {
+		return false
+	}
+	if _, ok := rule["port"]; !ok {
+		return false
+	}
+	for key := range rule {
+		switch key {
+		case "type", "outboundTag", "ip", "port":
+			continue
+		case "enabled":
+			if enabled, ok := rule[key].(bool); !ok || !enabled {
+				return false
+			}
+			continue
+		default:
+			return false
+		}
+	}
+	return true
+}
+
+// findPrivateBlockRule returns the index of a routing rule that blocks
+// geoip:private (the panel's default anti-SSRF rule), or -1 if none is
+// present. Matched by shape (outboundTag=blocked, ip contains
+// "geoip:private") rather than position, since admins can reorder rules.
+func findPrivateBlockRule(rules []map[string]any) int {
+	for i, rule := range rules {
+		if out, _ := rule["outboundTag"].(string); out != "blocked" {
+			continue
+		}
+		for _, ip := range readRuleIPs(rule["ip"]) {
+			if strings.EqualFold(ip, "geoip:private") {
+				return i
+			}
+		}
+	}
+	return -1
+}
+
+func readRuleIPs(raw any) []string {
+	switch v := raw.(type) {
+	case []string:
+		return v
+	case []any:
+		out := make([]string, 0, len(v))
+		for _, item := range v {
+			if s, ok := item.(string); ok {
+				out = append(out, s)
+			}
+		}
+		return out
+	case string:
+		if v == "" {
+			return nil
+		}
+		return []string{v}
+	default:
+		return nil
+	}
+}
+
+// dnsServerEndpoint is a literal (ip, port) pair extracted from a
+// dns.servers entry.
+type dnsServerEndpoint struct {
+	ip   string
+	port int
+}
+
+// privateDnsServerEndpoint extracts a literal, private/internal (ip, port)
+// endpoint from a dns.servers entry, or ok=false if the entry is a domain
+// name, a special Xray keyword (localhost, fakedns, ...), or resolves to a
+// public IP.
+//
+// A dns.servers entry is either a bare string or an object with an
+// "address" field (see frontend/src/schemas/dns.ts DnsServerEntrySchema);
+// the object form may also carry an explicit "port" (default 53 there,
+// per DnsServerObjectInnerSchema), which takes precedence over any port
+// embedded in the address itself.
+func privateDnsServerEndpoint(entry any) (dnsServerEndpoint, bool) {
+	var address string
+	explicitPort := 0
+	switch v := entry.(type) {
+	case string:
+		address = v
+	case map[string]any:
+		address, _ = v["address"].(string)
+		if p, ok := v["port"].(float64); ok && p > 0 {
+			explicitPort = int(p)
+		}
+	default:
+		return dnsServerEndpoint{}, false
+	}
+
+	host, port := splitAddressHostPort(address)
+	if host == "" {
+		return dnsServerEndpoint{}, false
+	}
+	if explicitPort > 0 {
+		port = explicitPort
+	}
+
+	ip := net.ParseIP(host)
+	if ip == nil {
+		// Domain name, or a special keyword like "localhost"/"fakedns" —
+		// neither is something we can safely allow-list by IP here.
+		return dnsServerEndpoint{}, false
+	}
+	if !netsafe.IsBlockedIP(ip) {
+		return dnsServerEndpoint{}, false
+	}
+	return dnsServerEndpoint{ip: ip.String(), port: port}, true
+}
+
+// splitAddressHostPort extracts the bare host and port (defaulting to 53)
+// from an Xray-core DNS server address string. Those may carry a URI
+// scheme (tcp://, tcp+local://, https://, https+local://, quic://,
+// quic+local://) and, for DoH, a path and/or a bracketed IPv6 host — all
+// of that is stripped down to host[:port] before parsing.
+func splitAddressHostPort(address string) (host string, port int) {
+	address = strings.TrimSpace(address)
+	if address == "" {
+		return "", 0
+	}
+
+	if idx := strings.Index(address, "://"); idx != -1 {
+		address = address[idx+3:]
+	}
+	// Drop a DoH path, e.g. "1.1.1.1/dns-query".
+	if idx := strings.Index(address, "/"); idx != -1 {
+		address = address[:idx]
+	}
+
+	port = 53
+	host = address
+	if strings.HasPrefix(host, "[") {
+		// Bracketed IPv6, with or without a port: "[::1]" / "[::1]:53".
+		end := strings.Index(host, "]")
+		if end == -1 {
+			return host, port
+		}
+		rest := host[end+1:]
+		host = host[1:end]
+		if p, ok := strings.CutPrefix(rest, ":"); ok {
+			if n, err := strconv.Atoi(p); err == nil {
+				port = n
+			}
+		}
+		return host, port
+	}
+	if h, p, err := net.SplitHostPort(host); err == nil {
+		host = h
+		if n, err := strconv.Atoi(p); err == nil {
+			port = n
+		}
+	}
+	return host, port
+}
+
+// dnsAllowPortGroup is the set of private literal IPs that share a single
+// port among the configured dns.servers, e.g. two internal resolvers both
+// queried on :53.
+type dnsAllowPortGroup struct {
+	port int
+	ips  []string
+}
+
+// collectPrivateDnsAllowGroups returns the private dns.servers endpoints
+// grouped by port, sorted by port ascending (ips within a group sorted and
+// de-duplicated) for deterministic output.
+func collectPrivateDnsAllowGroups(dnsRaw json.RawMessage) []dnsAllowPortGroup {
+	if len(dnsRaw) == 0 {
+		return nil
+	}
+	var dns struct {
+		Servers []any `json:"servers"`
+	}
+	if err := json.Unmarshal(dnsRaw, &dns); err != nil {
+		return nil
+	}
+
+	byPort := make(map[int]map[string]bool)
+	for _, entry := range dns.Servers {
+		ep, ok := privateDnsServerEndpoint(entry)
+		if !ok {
+			continue
+		}
+		if byPort[ep.port] == nil {
+			byPort[ep.port] = make(map[string]bool)
+		}
+		byPort[ep.port][ep.ip] = true
+	}
+
+	ports := make([]int, 0, len(byPort))
+	for p := range byPort {
+		ports = append(ports, p)
+	}
+	sort.Ints(ports)
+
+	groups := make([]dnsAllowPortGroup, 0, len(ports))
+	for _, p := range ports {
+		ips := make([]string, 0, len(byPort[p]))
+		for ip := range byPort[p] {
+			ips = append(ips, ip)
+		}
+		sort.Strings(ips)
+		groups = append(groups, dnsAllowPortGroup{port: p, ips: ips})
+	}
+	return groups
+}
+
+// EnsureDnsServerRouting keeps a set of managed "direct" allow-rules — one
+// per distinct port among any private/internal dns.servers addresses —
+// in sync, positioned immediately before the panel's default
+// geoip:private block rule.
+//
+// Why this matters: Xray's own DNS client traffic is dispatched through
+// the same routing table as proxied client traffic. If dns.servers points
+// at a private IP (e.g. a self-hosted AdGuard Home / Pi-hole reachable on
+// the same Docker network as Xray — a common self-hosted setup) and the
+// panel's default private-IP block rule is active, Xray's own DNS lookups
+// get silently dropped by that rule. Xray then falls back to dialing
+// destinations by raw hostname once its internal DNS attempt times out
+// (~4s), so proxied connections still work, just with a multi-second stall
+// added to every new domain, with no error surfaced to the client or
+// admin.
+//
+// Each managed rule is scoped to its port (not just the IP), so the
+// exception only reopens the DNS traffic that actually needs it rather
+// than every port on the private host. On every save, all previously
+// managed rules are stripped out and a fresh set is rebuilt from the
+// current dns.servers config and reinserted right before the block rule
+// (recomputing its index after the strip) — this corrects both content
+// drift (dns.servers changed) and position drift (an admin dragged a
+// managed rule below the block rule in the Routing tab, which would
+// otherwise silently reintroduce the stall with nothing to notice or fix
+// it). The rebuilt result is only written back if it actually differs
+// from the input, so well-formed configs aren't churned on every save.
+// Manually-authored rules are never touched — see dnsAllowRuleShape.
+func EnsureDnsServerRouting(raw string) (string, error) {
+	var cfg map[string]json.RawMessage
+	if err := json.Unmarshal([]byte(raw), &cfg); err != nil {
+		return raw, err
+	}
+
+	groups := collectPrivateDnsAllowGroups(cfg["dns"])
+
+	var routing map[string]json.RawMessage
+	if r, ok := cfg["routing"]; ok && len(r) > 0 {
+		if err := json.Unmarshal(r, &routing); err != nil {
+			return raw, err
+		}
+	}
+	if routing == nil {
+		return raw, nil
+	}
+
+	var original []map[string]any
+	if r, ok := routing["rules"]; ok && len(r) > 0 {
+		if err := json.Unmarshal(r, &original); err != nil {
+			return raw, err
+		}
+	}
+
+	rebuilt := rebuildDnsAllowRules(original, groups)
+
+	rulesJSON, err := json.Marshal(rebuilt)
+	if err != nil {
+		return raw, err
+	}
+
+	// Compare against the original rules JSON, not the parsed Go values:
+	// json.Unmarshal into []map[string]any turns "ip" arrays into []any,
+	// while the rules this function builds use []string — those hold
+	// identical content but are different types under reflect.DeepEqual,
+	// which would otherwise report a no-op input as changed and churn the
+	// JSON on every save for no reason.
+	origRulesJSON := routing["rules"]
+	if len(origRulesJSON) == 0 {
+		origRulesJSON = json.RawMessage("[]")
+	}
+	if jsonEqual(origRulesJSON, rulesJSON) {
+		return raw, nil
+	}
+
+	routing["rules"] = rulesJSON
+	routingJSON, err := json.Marshal(routing)
+	if err != nil {
+		return raw, err
+	}
+	cfg["routing"] = routingJSON
+
+	out, err := json.Marshal(cfg)
+	if err != nil {
+		return raw, err
+	}
+	return string(out), nil
+}
+
+// rebuildDnsAllowRules strips any existing managed rules out of rules,
+// then — if a geoip:private block rule is present and groups is non-empty
+// — reinserts a freshly built managed rule per group immediately before
+// it. This uniformly handles content updates, position drift, and removal
+// (an empty groups list just leaves the managed rules stripped).
+func rebuildDnsAllowRules(rules []map[string]any, groups []dnsAllowPortGroup) []map[string]any {
+	clean := make([]map[string]any, 0, len(rules))
+	for _, rule := range rules {
+		if !dnsAllowRuleShape(rule) {
+			clean = append(clean, rule)
+		}
+	}
+
+	blockIdx := findPrivateBlockRule(clean)
+	if blockIdx < 0 || len(groups) == 0 {
+		return clean
+	}
+
+	managed := make([]map[string]any, 0, len(groups))
+	for _, g := range groups {
+		managed = append(managed, map[string]any{
+			"type":        "field",
+			"ip":          g.ips,
+			"port":        strconv.Itoa(g.port),
+			"outboundTag": "direct",
+		})
+	}
+
+	// Capacity hint uses len(clean) alone (not len(clean)+len(managed)):
+	// summing two independent lengths for a make() size risks overflow on
+	// pathological input per static analysis, and clean's length already
+	// covers most of the eventual size on its own.
+	out := make([]map[string]any, 0, len(clean))
+	out = append(out, clean[:blockIdx]...)
+	out = append(out, managed...)
+	out = append(out, clean[blockIdx:]...)
+	return out
+}
+
+// jsonEqual reports whether a and b decode to structurally identical
+// values. Used instead of comparing raw bytes (key order, whitespace) or
+// reflect.DeepEqual on already-parsed Go values (which is type-sensitive
+// to []any vs []string and would misreport identical content as changed).
+func jsonEqual(a, b json.RawMessage) bool {
+	var av, bv any
+	if err := json.Unmarshal(a, &av); err != nil {
+		return false
+	}
+	if err := json.Unmarshal(b, &bv); err != nil {
+		return false
+	}
+	return reflect.DeepEqual(av, bv)
+}

internal/web/service/xray_setting_dns_routing_test.go

@@ -0,0 +1,411 @@
+package service
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+func rulesFromRaw(t *testing.T, raw string) []map[string]any {
+	t.Helper()
+	var cfg map[string]any
+	if err := json.Unmarshal([]byte(raw), &cfg); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	return routingRulesFromCfg(cfg)
+}
+
+func TestEnsureDnsServerRouting_NoOpWithoutDnsServers(t *testing.T) {
+	in := `{"routing":{"rules":[{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}]}}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if out != in {
+		t.Fatalf("expected unchanged input, got: %s", out)
+	}
+}
+
+func TestEnsureDnsServerRouting_NoOpForPublicDnsServer(t *testing.T) {
+	in := `{
+		"dns": {"servers": ["1.1.1.1"]},
+		"routing": {"rules": [{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}]}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if out != in {
+		t.Fatalf("expected unchanged input for a public DNS server, got: %s", out)
+	}
+}
+
+func TestEnsureDnsServerRouting_NoOpWithoutPrivateBlockRule(t *testing.T) {
+	// Private DNS server, but nothing in routing would block it — no rule
+	// needed.
+	in := `{
+		"dns": {"servers": ["172.20.0.53"]},
+		"routing": {"rules": [{"type":"field","inboundTag":["api"],"outboundTag":"api"}]}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if out != in {
+		t.Fatalf("expected unchanged input without a private-block rule, got: %s", out)
+	}
+}
+
+func TestEnsureDnsServerRouting_InsertsAllowRuleBeforeBlock(t *testing.T) {
+	// Reproduces the reported bug: dns.servers on a private docker IP
+	// (e.g. a same-network AdGuard Home) plus the panel's default
+	// geoip:private block rule silently drops Xray's own DNS traffic.
+	in := `{
+		"dns": {"servers": ["172.20.0.53"], "queryStrategy": "UseIPv4"},
+		"routing": {
+			"rules": [
+				{"type":"field","inboundTag":["api"],"outboundTag":"api"},
+				{"type":"field","outboundTag":"blocked","ip":["geoip:private"]},
+				{"type":"field","outboundTag":"blocked","protocol":["bittorrent"]}
+			]
+		}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	rules := rulesFromRaw(t, out)
+	if len(rules) != 4 {
+		t.Fatalf("rules len = %d, want 4: %s", len(rules), out)
+	}
+	if tag, _ := rules[1]["outboundTag"].(string); tag != "direct" {
+		t.Fatalf("expected inserted allow-rule at index 1, got outboundTag %v\nfull: %s", rules[1]["outboundTag"], out)
+	}
+	ips := readRuleIPs(rules[1]["ip"])
+	if len(ips) != 1 || ips[0] != "172.20.0.53" {
+		t.Fatalf("allow-rule ip = %v, want [172.20.0.53]", ips)
+	}
+	if port, _ := rules[1]["port"].(string); port != "53" {
+		t.Fatalf("allow-rule port = %v, want \"53\" (scoped to DNS traffic only)", rules[1]["port"])
+	}
+	if tag, _ := rules[2]["outboundTag"].(string); tag != "blocked" {
+		t.Fatalf("private-block rule should still follow the allow-rule, got %v", rules[2])
+	}
+}
+
+func TestEnsureDnsServerRouting_HandlesObjectServerEntryWithExplicitPort(t *testing.T) {
+	in := `{
+		"dns": {"servers": [{"address": "172.20.0.53", "port": 5353}]},
+		"routing": {"rules": [{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}]}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	rules := rulesFromRaw(t, out)
+	ips := readRuleIPs(rules[0]["ip"])
+	if len(ips) != 1 || ips[0] != "172.20.0.53" {
+		t.Fatalf("allow-rule ip = %v, want [172.20.0.53]", ips)
+	}
+	if port, _ := rules[0]["port"].(string); port != "5353" {
+		t.Fatalf("allow-rule port = %v, want the object's explicit \"5353\"", rules[0]["port"])
+	}
+}
+
+func TestEnsureDnsServerRouting_GroupsDistinctPortsIntoSeparateRules(t *testing.T) {
+	// Two internal resolvers on different ports must not be merged into
+	// one ip+port rule — that would cross-allow ip1:port2 and ip2:port1,
+	// widening the exception beyond what's actually needed.
+	in := `{
+		"dns": {"servers": ["172.20.0.53", "10.0.0.53:5353"]},
+		"routing": {"rules": [{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}]}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	rules := rulesFromRaw(t, out)
+	if len(rules) != 3 {
+		t.Fatalf("rules len = %d, want 3 (one rule per port + the block rule): %s", len(rules), out)
+	}
+	if p, _ := rules[0]["port"].(string); p != "53" {
+		t.Fatalf("rules[0] port = %v, want \"53\" (sorted ascending)", rules[0]["port"])
+	}
+	if ips := readRuleIPs(rules[0]["ip"]); len(ips) != 1 || ips[0] != "172.20.0.53" {
+		t.Fatalf("rules[0] ip = %v, want [172.20.0.53]", ips)
+	}
+	if p, _ := rules[1]["port"].(string); p != "5353" {
+		t.Fatalf("rules[1] port = %v, want \"5353\"", rules[1]["port"])
+	}
+	if ips := readRuleIPs(rules[1]["ip"]); len(ips) != 1 || ips[0] != "10.0.0.53" {
+		t.Fatalf("rules[1] ip = %v, want [10.0.0.53]", ips)
+	}
+}
+
+func TestEnsureDnsServerRouting_StripsSchemePortAndPath(t *testing.T) {
+	in := `{
+		"dns": {"servers": [
+			"tcp://10.0.0.53:5353",
+			"https+local://192.168.1.1/dns-query",
+			"[fd00::53]:53"
+		]},
+		"routing": {"rules": [{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}]}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	rules := rulesFromRaw(t, out)
+	// 192.168.1.1 and fd00::53 share the default port 53; 10.0.0.53:5353
+	// is its own group.
+	if len(rules) != 3 {
+		t.Fatalf("rules len = %d, want 3 (2 port groups + block rule): %s", len(rules), out)
+	}
+	port53 := rules[0]
+	if p, _ := port53["port"].(string); p != "53" {
+		t.Fatalf("rules[0] port = %v, want \"53\"", port53["port"])
+	}
+	want53 := map[string]bool{"192.168.1.1": true, "fd00::53": true}
+	ips := readRuleIPs(port53["ip"])
+	if len(ips) != len(want53) {
+		t.Fatalf("rules[0] ip = %v, want entries matching %v", ips, want53)
+	}
+	for _, ip := range ips {
+		if !want53[ip] {
+			t.Fatalf("unexpected ip %q in port-53 rule %v", ip, ips)
+		}
+	}
+	port5353 := rules[1]
+	if p, _ := port5353["port"].(string); p != "5353" {
+		t.Fatalf("rules[1] port = %v, want \"5353\"", port5353["port"])
+	}
+	if ips := readRuleIPs(port5353["ip"]); len(ips) != 1 || ips[0] != "10.0.0.53" {
+		t.Fatalf("rules[1] ip = %v, want [10.0.0.53]", ips)
+	}
+}
+
+func TestEnsureDnsServerRouting_SkipsSpecialAndDomainAddresses(t *testing.T) {
+	in := `{
+		"dns": {"servers": ["localhost", "fakedns", "dns.google", "8.8.8.8"]},
+		"routing": {"rules": [{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}]}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if out != in {
+		t.Fatalf("expected unchanged input (no private literal IPs present), got: %s", out)
+	}
+}
+
+func TestEnsureDnsServerRouting_IdempotentOnSecondSave(t *testing.T) {
+	in := `{
+		"dns": {"servers": ["172.20.0.53"]},
+		"routing": {"rules": [{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}]}
+	}`
+	first, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	second, err := EnsureDnsServerRouting(first)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if second != first {
+		t.Fatalf("expected no further change on second pass\nfirst:  %s\nsecond: %s", first, second)
+	}
+}
+
+func TestEnsureDnsServerRouting_UpdatesOwnedRuleWhenServersChange(t *testing.T) {
+	in := `{
+		"dns": {"servers": ["172.20.0.53"]},
+		"routing": {
+			"rules": [
+				{"type":"field","ip":["172.20.0.53"],"port":"53","outboundTag":"direct"},
+				{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}
+			]
+		}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if out != in {
+		t.Fatalf("dns servers unchanged, expected no-op, got: %s", out)
+	}
+
+	// Admin adds a second internal resolver on the same port.
+	in2 := `{
+		"dns": {"servers": ["172.20.0.53", "10.0.0.53"]},
+		"routing": {
+			"rules": [
+				{"type":"field","ip":["172.20.0.53"],"port":"53","outboundTag":"direct"},
+				{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}
+			]
+		}
+	}`
+	out2, err := EnsureDnsServerRouting(in2)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	rules := rulesFromRaw(t, out2)
+	if len(rules) != 2 {
+		t.Fatalf("rules len = %d, want 2 (existing rule updated in place): %s", len(rules), out2)
+	}
+	ips := readRuleIPs(rules[0]["ip"])
+	if len(ips) != 2 || ips[0] != "10.0.0.53" || ips[1] != "172.20.0.53" {
+		t.Fatalf("allow-rule ip = %v, want [10.0.0.53 172.20.0.53]", ips)
+	}
+}
+
+func TestEnsureDnsServerRouting_RemovesOwnedRuleWhenNoLongerNeeded(t *testing.T) {
+	// Admin switches dns.servers to a public resolver — our previously
+	// inserted allow-rule is now dead weight and should be dropped.
+	in := `{
+		"dns": {"servers": ["1.1.1.1"]},
+		"routing": {
+			"rules": [
+				{"type":"field","ip":["172.20.0.53"],"port":"53","outboundTag":"direct"},
+				{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}
+			]
+		}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	rules := rulesFromRaw(t, out)
+	if len(rules) != 1 {
+		t.Fatalf("rules len = %d, want 1 (stale allow-rule removed): %s", len(rules), out)
+	}
+	if tag, _ := rules[0]["outboundTag"].(string); tag != "blocked" {
+		t.Fatalf("remaining rule should be the block rule, got %v", rules[0])
+	}
+}
+
+func TestEnsureDnsServerRouting_DoesNotTouchManualRuleWithExtraMatchers(t *testing.T) {
+	// A hand-written rule that also allows the DNS IP but carries an extra
+	// matcher isn't recognized as "ours" and must be left alone; a fresh
+	// managed rule is inserted alongside it instead.
+	in := `{
+		"dns": {"servers": ["172.20.0.53"]},
+		"routing": {
+			"rules": [
+				{"type":"field","ip":["172.20.0.53"],"port":"53","domain":["example.com"],"outboundTag":"direct"},
+				{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}
+			]
+		}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	rules := rulesFromRaw(t, out)
+	if len(rules) != 3 {
+		t.Fatalf("rules len = %d, want 3 (manual rule kept, managed rule inserted): %s", len(rules), out)
+	}
+	if _, ok := rules[0]["domain"]; !ok {
+		t.Fatalf("manual rule with domain matcher should be untouched, got %v", rules[0])
+	}
+}
+
+func TestEnsureDnsServerRouting_RepositionsRuleDraggedAfterBlockRule(t *testing.T) {
+	// The Routing tab lets admins freely drag rules around
+	// (RoutingTab.tsx move-up/move-down and drag-and-drop), and a managed
+	// rule renders as an indistinguishable normal row there. If it ends up
+	// at or after the block rule — directly, or incidentally while
+	// reordering something else — the exact bug this file fixes comes
+	// back silently: the block rule matches first and Xray's own DNS
+	// traffic is dropped again. Detecting only ip/content drift isn't
+	// enough; position must be checked too.
+	in := `{
+		"dns": {"servers": ["172.20.0.53"]},
+		"routing": {
+			"rules": [
+				{"type":"field","outboundTag":"blocked","ip":["geoip:private"]},
+				{"type":"field","ip":["172.20.0.53"],"port":"53","outboundTag":"direct"}
+			]
+		}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	rules := rulesFromRaw(t, out)
+	if len(rules) != 2 {
+		t.Fatalf("rules len = %d, want 2: %s", len(rules), out)
+	}
+	if tag, _ := rules[0]["outboundTag"].(string); tag != "direct" {
+		t.Fatalf("managed rule should be re-homed to index 0 (before the block rule), got %v\nfull: %s", rules[0], out)
+	}
+	if tag, _ := rules[1]["outboundTag"].(string); tag != "blocked" {
+		t.Fatalf("block rule should now be at index 1, got %v\nfull: %s", rules[1], out)
+	}
+}
+
+func TestEnsureDnsServerRouting_TreatsExplicitlyEnabledRuleAsOwned(t *testing.T) {
+	// RuleFormModal.tsx's submit() always writes an "enabled" key, even
+	// when the admin changed nothing and it was already true — merely
+	// opening the auto-generated rule in the editor must not disown it.
+	in := `{
+		"dns": {"servers": ["172.20.0.53"]},
+		"routing": {
+			"rules": [
+				{"type":"field","ip":["172.20.0.53"],"port":"53","outboundTag":"direct","enabled":true},
+				{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}
+			]
+		}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	rules := rulesFromRaw(t, out)
+	if len(rules) != 2 {
+		t.Fatalf("rules len = %d, want 2 (no duplicate inserted): %s", len(rules), out)
+	}
+}
+
+func TestEnsureDnsServerRouting_DisabledRuleIsDisownedAndReplaced(t *testing.T) {
+	// toggleRule() in RoutingTab.tsx writes enabled=false on a plain
+	// switch flip. An admin who explicitly disables the managed rule is
+	// choosing to turn the exception off; re-enabling it on the next save
+	// would silently override that. The disabled rule is left alone and a
+	// fresh, enabled one is (re-)created to keep the fix working.
+	in := `{
+		"dns": {"servers": ["172.20.0.53"]},
+		"routing": {
+			"rules": [
+				{"type":"field","ip":["172.20.0.53"],"port":"53","outboundTag":"direct","enabled":false},
+				{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}
+			]
+		}
+	}`
+	out, err := EnsureDnsServerRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	rules := rulesFromRaw(t, out)
+	if len(rules) != 3 {
+		t.Fatalf("rules len = %d, want 3 (disabled rule kept as-is, fresh managed rule added): %s", len(rules), out)
+	}
+	if enabled, _ := rules[0]["enabled"].(bool); enabled {
+		t.Fatalf("original disabled rule should be untouched, got %v", rules[0])
+	}
+	if _, ok := rules[1]["enabled"]; ok {
+		t.Fatalf("freshly (re-)generated rule shouldn't carry an enabled key, got %v", rules[1])
+	}
+	if tag, _ := rules[1]["outboundTag"].(string); tag != "direct" {
+		t.Fatalf("expected the fresh managed rule at index 1, got %v", rules[1])
+	}
+}
+
+func TestEnsureDnsServerRouting_InvalidJsonReturnsAsIs(t *testing.T) {
+	in := "definitely not json"
+	out, err := EnsureDnsServerRouting(in)
+	if err == nil {
+		t.Fatalf("expected error for invalid json, got none")
+	}
+	if out != in {
+		t.Fatalf("expected raw passthrough on error, got %q", out)
+	}
+}