fix(gate): [5/7] hid the runner's exit code exactly when it explained everything

The line naming a nonzero `go test` status was printed only when every
privileged test had produced a verdict — on the reasoning that a named FAILED
already explains the status. The case that actually happens is the opposite
one: the run dies at package level, so it names no test, so the loop above
prints MISSING for all of them, and the one line pointing at the real cause was
the one suppressed. A reader then goes hunting for three vanished tests instead
of at the build error above.

To be exact about what was and was not broken, because the framing matters: the
exit status was never SWALLOWED. priv_bad is set by the MISSING branch, so
FAILED is set and the gate fails either way — this was a diagnosis bug, not a
correctness one. What changes is whether the log says why.

Verified on the branch a green run never reaches, by driving the edited block
with all four (priv_rc, priv_bad) combinations: the new message appears only for
(1,1), the old one only for (1,0), and priv_bad/FAILED come out 1 in both. The
full gate is green with the change in, which covers the (0,0) path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS

scripts/run-tests.sh

@@ -721,9 +721,25 @@ else
 			priv_bad=1
 		fi
 	done <<<"$priv_expect"
-	if [ "$priv_rc" -ne 0 ] && [ "$priv_bad" -eq 0 ]; then
-		echo "  FAILED [privileged]: go test exited $priv_rc with every named test accounted for —" >&2
-		echo "          a build or package-level failure, see the log above." >&2
+	# The runner's own exit status is reported WHATEVER the per-name verdicts say.
+	# It used to be reported only when every name was accounted for, on the
+	# reasoning that a named failure already explains the status — but the case
+	# that actually happens is the opposite one: the run dies at package level, the
+	# loop above prints MISSING for every name because none of them produced a
+	# verdict, and the one line naming the cause was the one suppressed. A reader
+	# then goes looking for three vanished tests instead of at the build error.
+	# Same verdict either way (priv_bad, hence FAILED, is set in both branches) —
+	# what changes is whether the log says why.
+	if [ "$priv_rc" -ne 0 ]; then
+		if [ "$priv_bad" -eq 0 ]; then
+			echo "  FAILED [privileged]: go test exited $priv_rc with every named test accounted for —" >&2
+			echo "          a build or package-level failure, see the log above." >&2
+		else
+			echo "  FAILED [privileged]: go test exited $priv_rc as well — the verdicts above are" >&2
+			echo "          the symptom. A run that dies at package level names no test, so" >&2
+			echo "          MISSING lines are what that looks like from here; the cause is in" >&2
+			echo "          the log above, not in the tests they name." >&2
+		fi
 		priv_bad=1
 	fi
 	if [ "$priv_bad" -ne 0 ]; then

shater/apply/privatedstseverity_test.go

@@ -0,0 +1,120 @@
+package apply
+
+// THE PRIVATE-DESTINATION CHANNEL CARRIES TWO DIFFERENT ANSWERS.
+//
+// netplane's private-destination mechanism (netplane/coverage.go
+// privateRoutedPlan) produces two shapes of news, and the netplane channel grades
+// everything critical by default:
+//
+//	a REFUSAL   a rule names a private subnet and this plan will NOT route it —
+//	            because it collides with one of the router's own networks, or
+//	            because the router could not be enumerated at all. The rule looks
+//	            configured and does nothing. Critical, and the default is right.
+//	a DISCLOSURE
+//	            an address list this plan never reads MIGHT contain private
+//	            destinations. It is unconditional by construction — "warn on
+//	            suspicion" would mean reading the list, which is precisely what
+//	            that stage cannot do — so it fires for rule-sets that are very
+//	            probably fine. Nothing is leaking and nothing is cut off; grading
+//	            it critical would spend the alarm banner on a maybe, every apply.
+//
+// WHY THIS TEST DRIVES THE REAL RENDERER. The discriminator is a substring of
+// netplane's prose, and this package already carries the scar of markers that
+// matched no living text. Nothing here is hand-typed prose: both cases build a
+// model, call netplane.RenderNftWithWarnings, and grade whatever comes back, so a
+// reworded producer fails this test BY NAME instead of silently reverting the
+// severity to critical.
+//
+// THE CONTROL IS THE SPLIT. An instrument that answered "warning" to everything
+// would pass a test that only checked the disclosure, and the wholesale-critical
+// code would pass a test that only checked the refusal. Only asserting that the
+// SAME instrument gives DIFFERENT answers to the two real texts distinguishes the
+// grading from either failure.
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/sagernet/sing-box/shater/model"
+	"github.com/sagernet/sing-box/shater/netplane"
+)
+
+// privSevModel is one LAN plus whatever destination rule-set the case needs.
+// There is no ubus on a build machine, so netplane cannot enumerate this
+// router's networks — which is exactly the state the REFUSAL case needs, and is
+// irrelevant to the DISCLOSURE case.
+func privSevModel(sets []model.Ruleset) *model.Model {
+	return &model.Model{
+		Globals: model.Globals{
+			Enabled: true, KillSwitch: "closed", IPv6: false,
+			FwmarkBase: 0x2000, TableBase: 0x2000,
+		},
+		Inbounds: []model.Inbound{
+			{Name: "lan", Enabled: true, Type: "tproxy", Network: "lan", TproxyPort: 12345, TCP: true, UDP: true},
+		},
+		Rulesets: sets,
+		Rules: []model.Rule{
+			{Name: "lab", Enabled: true, Order: 10, DstRuleset: []string{"lab"}, Target: "node:awghome"},
+		},
+	}
+}
+
+// privSevFinding renders m through the REAL netplane renderer, pushes the result
+// through the real gatherWarnings, and returns the one finding whose message
+// contains phrase.
+func privSevFinding(t *testing.T, m *model.Model, phrase string) Warning {
+	t.Helper()
+	_, netWarns, err := netplane.RenderNftWithWarnings(m)
+	if err != nil {
+		t.Fatalf("RenderNftWithWarnings: %v", err)
+	}
+	var hits []Warning
+	for _, w := range gatherWarnings(m, nil, netWarns, nil) {
+		if strings.Contains(w.Message, phrase) {
+			hits = append(hits, w)
+		}
+	}
+	if len(hits) != 1 {
+		t.Fatalf("want exactly one finding containing %q, got %d; netplane said: %v", phrase, len(hits), netWarns)
+	}
+	return hits[0]
+}
+
+func TestPrivateDestinationRefusalIsCriticalDisclosureIsNot(t *testing.T) {
+	// REFUSAL: an explicitly named private subnet this plan will not route,
+	// because it cannot prove the subnet is not one of the router's own.
+	refusal := privSevFinding(t,
+		privSevModel([]model.Ruleset{{
+			Name: "lab", Type: "ipcidr", Source: "inline", Entries: []string{"10.10.10.0/24"},
+		}}),
+		"network inventory could not be read")
+	if refusal.Severity != SeverityCritical {
+		t.Fatalf("a rule that names a private subnet and does NOT route it graded %q, want %q: %s",
+			refusal.Severity, SeverityCritical, refusal.Message)
+	}
+	if refusal.Section != "rule" || refusal.Name != "lab" {
+		t.Fatalf("refusal is not attributed to the rule it is about: section=%q name=%q",
+			refusal.Section, refusal.Name)
+	}
+
+	// DISCLOSURE: a list this plan cannot read. Same channel, same instrument.
+	disclosure := privSevFinding(t,
+		privSevModel([]model.Ruleset{{
+			Name: "lab", Type: "ipcidr", Source: "url", URL: "https://example.invalid/lab.srs",
+		}}),
+		"address list whose contents this data plane never sees")
+	if disclosure.Severity != SeverityWarning {
+		t.Fatalf("a blind-spot disclosure graded %q, want %q — a red that means \"probably fine\" is how "+
+			"the next red stops being read: %s", disclosure.Severity, SeverityWarning, disclosure.Message)
+	}
+	if disclosure.Section != "ruleset" || disclosure.Name != "lab" {
+		t.Fatalf("disclosure is not attributed to the rule-set it is about: section=%q name=%q",
+			disclosure.Section, disclosure.Name)
+	}
+
+	// The split itself. Two texts, one instrument, two answers.
+	if refusal.Severity == disclosure.Severity {
+		t.Fatalf("the netplane channel gave both private-destination texts the same grade (%q), so this "+
+			"test would pass against an instrument that grades nothing", refusal.Severity)
+	}
+}

shater/apply/warnings.go

@@ -272,11 +272,29 @@ var infoMarkers = []string{"cache:"}
 // file its trustworthiness once already.
 
 // netplaneDeliberateMarkers are the producer's OWN statement that the state it is
-// describing may be what the operator asked for. Quoted from
-// netplane/coverage.go oneProtocolWarning's closed branch, which is the only
-// place it appears.
+// describing may be what the operator asked for — or, for the second entry, that
+// it may not be a state at all. Both are quoted verbatim from netplane:
+//
+//   - oneProtocolWarning's closed branch (netplane/coverage.go), the original;
+//   - opaqueAddressListWarning (netplane/coverage.go), the disclosure that an
+//     address list this plan cannot read MIGHT contain private destinations the
+//     private-destination bypass will swallow. It is unconditional by design —
+//     "warn on suspicion" would mean reading the list, which is exactly what
+//     that stage cannot do — so it fires for a rule-set that is very probably
+//     fine. It describes a BLIND SPOT, not a leak and not an outage: nothing the
+//     operator configured as protection is missing, and if the list holds only
+//     public addresses there is nothing to do at all. Grading that critical
+//     would spend the panel's alarm banner on a maybe, every apply, which is how
+//     the next critical stops being read.
+//
+// It stays a `warning` rather than `info` for the same reason the first entry
+// does: the panel's attentionFindings keeps critical and warning and DROPS info,
+// so an info here would never reach the operator, and the state it describes —
+// a rule that names a private destination and silently never fires — is one they
+// have to be able to find out about.
 var netplaneDeliberateMarkers = []string{
 	"If you meant it, nothing to do.",
+	"If it contains only public addresses, nothing to do.",
 }
 
 // netplaneNeverDowngrade is the guard on the guard: text that says traffic is

shater/engine/observatory_test.go

@@ -37,6 +37,22 @@ func obsFixture() option.Options {
 	}
 }
 
+// obsSelectorFixture is the same two nodes behind a used SELECTOR group. A
+// selector has no checker of its own, so both members are the OBSERVATORY's to
+// dial — which obsFixture's urltest group's members are not (they are
+// SelfChecked, TestPlanHandsURLTestMembersToTheirGroup). Any test that must
+// observe the observatory actually dialling needs this shape, not that one.
+func obsSelectorFixture() option.Options {
+	return option.Options{
+		Outbounds: []option.Outbound{
+			fixNode("n1", ""),
+			fixNode("n2", ""),
+			fixGroup(C.TypeSelector, "sel", "n1", "n2"),
+		},
+		Route: fixRoute("sel"),
+	}
+}
+
 func obsState(e *Engine) (plan []ProbeJob, cursor int, enabled bool) {
 	e.obsMu.Lock()
 	defer e.obsMu.Unlock()
@@ -60,6 +76,19 @@ func TestConfigureObservatoryLifecycle(t *testing.T) {
 
 	cfg := ObservatoryConfig{Enabled: true, Options: obsFixture(), ProbeURL: "u", ProbeInterval: time.Minute}
 	e.ConfigureObservatory(cfg)
+	// An enabled configure must have armed the ticker — asserted HERE because the
+	// next line takes that goroutine out of the test's way, and an assertion
+	// removed by a fixture is an assertion lost. Everything below is about what
+	// ConfigureObservatory writes, which only reads deterministically with no
+	// second writer; see detachObservatoryLoop.
+	e.obsMu.Lock()
+	armed := e.obs.stop != nil && e.obs.done != nil && e.obs.nudge != nil
+	e.obsMu.Unlock()
+	if !armed {
+		t.Fatal("an enabled configure did not start the observatory loop")
+	}
+	detachObservatoryLoop(e)
+
 	plan, _, enabled := obsState(e)
 	if !enabled || len(plan) != 2 {
 		t.Fatalf("after enable: enabled=%v plan=%d jobs, want true/2 (n1, n2)", enabled, len(plan))
@@ -105,6 +134,10 @@ func TestObservatoryTickStoppedEngine(t *testing.T) {
 	e := New()
 	defer e.StopObservatory()
 	e.ConfigureObservatory(ObservatoryConfig{Enabled: true, Options: obsFixture(), ProbeInterval: time.Minute})
+	// The loop's own first pass would have advanced the cursor for us, and the
+	// assertion below would then hold whether or not the hand-driven tick did
+	// anything at all. Quiesce so the tick under test is the only one there is.
+	quiesceObservatoryLoop(e)
 
 	e.observatoryTickOnce()
 	if _, cursor, _ := obsState(e); cursor == 0 {
@@ -122,6 +155,11 @@ func TestObservatoryTicksDuringManualRun(t *testing.T) {
 	e := New()
 	defer e.StopObservatory()
 	e.ConfigureObservatory(ObservatoryConfig{Enabled: true, Options: obsFixture(), ProbeInterval: time.Minute})
+	// Same reason as above, and it bites harder here: if the loop's first pass
+	// were still in flight the hand-driven tick would hit the busy guard and do
+	// NOTHING, while the cursor it reads would already be non-zero — the test
+	// would pass for the exact behaviour it exists to forbid.
+	quiesceObservatoryLoop(e)
 
 	e.groupTestRunning.Store(true)
 	e.observatoryTickOnce()
@@ -156,6 +194,12 @@ func TestRefreshObservatoryForcesOnePass(t *testing.T) {
 	}
 
 	e.ConfigureObservatory(ObservatoryConfig{Enabled: true, Options: obsFixture(), ProbeInterval: time.Minute})
+	// The flag lifecycle below is driven by hand, one tick at a time, so the loop
+	// must not be running a pass of its own alongside it — it would consume the
+	// force flag, move the cursor, or (holding the busy guard) turn the tick under
+	// test into a no-op. That collision IS a real production sequence, and it has
+	// its own test: TestRefreshDuringInFlightTickRunsAFullForcedPass.
+	detachObservatoryLoop(e)
 
 	// Fresh observations on every planned tag: the polite gate would skip them
 	// all, which is exactly what a forced pass must NOT do.
@@ -195,6 +239,197 @@ func TestRefreshObservatoryForcesOnePass(t *testing.T) {
 	// TestGroups depends on.
 }
 
+// A refresh raised while a tick is ALREADY IN FLIGHT must still produce a full
+// forced pass. This is the LIVE-loop half of the contract the test above pins by
+// hand, and it is the sequence the panel actually generates: a human presses
+// "Test" whenever they like, which is very often mid-tick.
+//
+// The collision is real. observatoryTickOnce holds the busy guard for the whole
+// duration of a batch, so a tick started while one is running would be a silent
+// no-op — the request must survive being made at exactly that moment.
+//
+// TWO independent wakeups carry it across, and this was measured rather than
+// read off the code: the buffered nudge RefreshObservatory sends (parked in the
+// channel while the loop is inside its tick, picked up the instant that tick
+// returns), and the tick's own deferred re-nudge. Disabling EITHER one leaves
+// this test green; disabling BOTH makes it fail with "force is still raised" and
+// 2 attempts instead of 4. So the assertion below is on the observable contract,
+// not on a mechanism — which is the right level for it, but it does mean neither
+// send is individually pinned here. The defer has its own subject in
+// TestForcedPassChainsItsBatchesWithoutWaitingForTheTick.
+//
+// This is also the sequence the flaky version of the tests above kept stumbling
+// into without ever asserting anything about it: it was the accident, never the
+// subject. Here it is the subject, and it is deterministic by construction — the
+// stub probe holds pass 1 open until the test has raised the flag, so "in
+// flight" is a fact rather than a hope.
+func TestRefreshDuringInFlightTickRunsAFullForcedPass(t *testing.T) {
+	e := New()
+	t.Cleanup(e.StopObservatory)
+
+	var mu sync.Mutex
+	var attempts []string
+	started := make(chan struct{})
+	release := make(chan struct{})
+	var startOnce, releaseOnce sync.Once
+	// Registered AFTER StopObservatory so it runs BEFORE it (cleanups are LIFO):
+	// any t.Fatal below would otherwise leave a probe parked in the stub, and
+	// StopObservatory waits for the tick holding it — a failing test would hang
+	// instead of reporting.
+	releaseAll := func() { releaseOnce.Do(func() { close(release) }) }
+	t.Cleanup(releaseAll)
+	hist := e.URLTestHistory()
+
+	// Installed BEFORE the observatory is configured, because the pass this test
+	// parks is the loop's own immediate first one.
+	e.setProbeFn(func(j ProbeJob) {
+		startOnce.Do(func() { close(started) })
+		<-release
+		mu.Lock()
+		attempts = append(attempts, j.Dial)
+		mu.Unlock()
+		// A real probe leaves a FRESH observation behind, and that is what makes
+		// the second pass proof of anything: the polite freshness gate would skip
+		// every job on it, so a second round of attempts can only come from force.
+		for _, tag := range j.Store {
+			hist.StoreURLTestHistory(tag, &adapter.URLTestHistory{LastOK: time.Now(), Delay: 10})
+		}
+	})
+	e.ConfigureObservatory(ObservatoryConfig{
+		Enabled: true, Options: obsSelectorFixture(), ProbeURL: "u", ProbeInterval: time.Minute,
+	})
+	// Checked before anything waits on the stub: a plan the observatory does not
+	// dial would park nothing, and the wait below would hang instead of failing.
+	plan, _, _ := obsState(e)
+	if len(plan) != 2 {
+		t.Fatalf("plan = %d jobs, want 2 (n1, n2)", len(plan))
+	}
+	for _, j := range plan {
+		if j.SelfChecked {
+			t.Fatalf("%s is self-checked; this test needs jobs the observatory itself dials", j.Dial)
+		}
+	}
+
+	// The loop's first pass parks inside the stub, holding the busy guard.
+	select {
+	case <-started:
+	case <-time.After(5 * time.Second):
+		t.Fatal("the observatory's first pass never reached the probe stub")
+	}
+	e.RefreshObservatory()
+	e.obsMu.Lock()
+	force := e.obs.force
+	e.obsMu.Unlock()
+	if !force {
+		t.Fatal("precondition: the refresh did not raise the force flag")
+	}
+	releaseAll()
+
+	// The forced pass must arrive on its own — nothing below ticks anything.
+	deadline := time.Now().Add(5 * time.Second)
+	var n int
+	for {
+		mu.Lock()
+		n = len(attempts)
+		mu.Unlock()
+		e.obsMu.Lock()
+		force = e.obs.force
+		e.obsMu.Unlock()
+		if (n >= 4 && !force) || !time.Now().Before(deadline) {
+			break
+		}
+		time.Sleep(2 * time.Millisecond)
+	}
+	if force {
+		t.Error("force is still raised: a refresh made during a busy tick was never carried over to a pass")
+	}
+	if n != 4 {
+		t.Fatalf("%d probe attempts, want 4 — the loop's own pass over the 2-job plan, then the FORCED pass the refresh asked for. "+
+			"2 means the refresh was dropped by the busy guard and never re-nudged; more means the flag outlived its one walk", n)
+	}
+}
+
+// obsWideSelectorFixture is n nodes behind one used selector — a plan LONGER
+// than one batch (observatoryBatch), which is the ordinary shape on a
+// subscription-sized config and the only one where a forced pass needs more than
+// a single tick to finish.
+func obsWideSelectorFixture(n int) option.Options {
+	var obs []option.Outbound
+	var members []string
+	for i := 0; i < n; i++ {
+		tag := fmt.Sprintf("w%02d", i)
+		obs = append(obs, fixNode(tag, ""))
+		members = append(members, tag)
+	}
+	obs = append(obs, fixGroup(C.TypeSelector, "wide", members...))
+	return option.Options{Outbounds: obs, Route: fixRoute("wide")}
+}
+
+// A forced pass that does not fit in one batch must CHAIN its batches instead of
+// taking one per 10s tick — the deferred re-nudge in observatoryTickOnce.
+//
+// This is the claim that comment makes, and until now nothing held it. It is not
+// cosmetic: a manual run polls the board and gives up after 120s, so on a
+// subscription-sized plan (hundreds of measurements, a dozen batches) a pass at
+// one batch per tick would report "not reached" about nodes nobody got round to
+// dialling.
+//
+// THREE batches, not two, and the third one is the whole point. Two would prove
+// nothing: the loop's own unconditional first tick runs one batch, and
+// RefreshObservatory's wakeup is still sitting unconsumed in the buffered nudge
+// channel to pay for a second — so a two-batch plan finishes even with the
+// chaining removed. Measured that way round before this constant was raised.
+func TestForcedPassChainsItsBatchesWithoutWaitingForTheTick(t *testing.T) {
+	const nodes = 2*observatoryBatch + 6 // three batches, the last a short one
+
+	e := New()
+	t.Cleanup(e.StopObservatory)
+	var mu sync.Mutex
+	dialled := map[string]bool{}
+	hist := e.URLTestHistory()
+	e.setProbeFn(func(j ProbeJob) {
+		mu.Lock()
+		dialled[j.Dial] = true
+		mu.Unlock()
+		for _, tag := range j.Store {
+			hist.StoreURLTestHistory(tag, &adapter.URLTestHistory{LastOK: time.Now(), Delay: 10})
+		}
+	})
+	e.ConfigureObservatory(ObservatoryConfig{
+		Enabled: true, Options: obsWideSelectorFixture(nodes), ProbeURL: "u", ProbeInterval: time.Minute,
+	})
+	if plan, _, _ := obsState(e); len(plan) != nodes {
+		t.Fatalf("plan = %d jobs, want %d", len(plan), nodes)
+	}
+	e.RefreshObservatory()
+
+	// Half of observatoryTick (10s), which is the only number that matters here:
+	// generous enough that a loaded runner cannot miss three chained batches of
+	// instant stub probes, and still strictly less than the tick a batch would
+	// otherwise wait for. Both directions measured — see the mutation note above.
+	deadline := time.Now().Add(5 * time.Second)
+	var got int
+	for {
+		mu.Lock()
+		got = len(dialled)
+		mu.Unlock()
+		if got >= nodes || !time.Now().Before(deadline) {
+			break
+		}
+		time.Sleep(2 * time.Millisecond)
+	}
+	if got != nodes {
+		t.Fatalf("%d of %d planned targets dialled within 5s; a forced pass must chain its batches, "+
+			"not spend one 10s tick each — a manual run times out at 120s", got, nodes)
+	}
+	e.obsMu.Lock()
+	force := e.obs.force
+	e.obsMu.Unlock()
+	if force {
+		t.Error("force is still raised after the whole plan was walked; it must be one-shot")
+	}
+}
+
 // The freshness gate: a job whose every covered tag has an observation younger
 // than the global probe interval is skipped — that set is exactly what an ACTIVE
 // group is measuring itself, and the observatory must not duplicate or suppress
@@ -385,25 +620,58 @@ func newChainProberOf(t *testing.T, opts option.Options, dead func(string) bool)
 	return e, rec
 }
 
-// quiesceObservatoryLoop stops the background ticker while leaving the plan, the
-// hop index and the used-set installed, and rewinds the cursor to the top.
+// detachObservatoryLoop takes the observatory's ticker goroutine out of the way
+// and KEEPS it out, leaving the plan, the hop index, the used-set and the CURSOR
+// exactly as ConfigureObservatory left them.
 //
-// The tests below drive observatoryTickOnce by hand and assert on an exact list
-// of dial attempts, which a ticker running its own passes in parallel would make
-// nondeterministic. StopObservatory cannot be used for this: it also throws the
-// plan away, which is the very thing under test. ConfigureObservatory still built
-// everything — only the goroutine is taken out.
-func quiesceObservatoryLoop(e *Engine) {
+// # Why every hand-driven observatory test needs this
+//
+// ConfigureObservatory starts the loop, and the loop's first tick fires
+// IMMEDIATELY — by contract, so an applied config gets its first verdicts in
+// seconds. That tick advances the cursor and consumes the force flag, and a
+// plan change additionally nudges the loop into a pass on purpose. Those are
+// precisely the fields the lifecycle tests read, so reading them straight after
+// a Configure is reading a value another goroutine is entitled to rewrite in the
+// same instant.
+//
+// That is the whole of the 2026-07-28 release-blocking flake: four assertions,
+// two tests, one cause. Each failed on its own engine's loop — no cross-test
+// state, no leaked goroutine — at roughly 1 run in 200 alone and far more often
+// under a loaded `-shuffle=on` package run, which is why CI saw it and a quiet
+// laptop did not.
+//
+// # Why a PLACEHOLDER stop channel rather than nil
+//
+// ConfigureObservatory starts a loop only when e.obs.stop is nil, so leaving a
+// non-nil one behind means the reconfigures these tests make run their whole
+// state machine — plan rebuild, cursor policy, nudge — with no goroutine racing
+// the reader. e.obs.nudge is left nil and every send to it is a select with a
+// default, so a nudge is dropped instead of blocking. StopObservatory and a
+// later disable close the placeholder exactly once, as they would the real one.
+func detachObservatoryLoop(e *Engine) {
 	e.obsMu.Lock()
 	stop, done := e.obs.stop, e.obs.done
-	e.obs.stop, e.obs.done, e.obs.nudge = nil, nil, nil
+	e.obs.stop, e.obs.done, e.obs.nudge = make(chan struct{}), nil, nil
+	e.obsMu.Unlock()
 	if stop != nil {
 		close(stop)
 	}
-	e.obsMu.Unlock()
 	if done != nil {
 		<-done // the loop's own first tick may still be in flight
 	}
+}
+
+// quiesceObservatoryLoop detaches the loop as above and additionally rewinds the
+// cursor to the top of the plan.
+//
+// The tests below drive observatoryTickOnce by hand and assert on an exact list
+// of dial attempts, which a ticker running its own passes in parallel would make
+// nondeterministic — and which the loop's own first pass would have half-walked
+// before the test started. StopObservatory cannot be used for this: it also
+// throws the plan away, which is the very thing under test. ConfigureObservatory
+// still built everything — only the goroutine is taken out.
+func quiesceObservatoryLoop(e *Engine) {
+	detachObservatoryLoop(e)
 	e.obsMu.Lock()
 	e.obs.cursor = 0
 	e.obsMu.Unlock()

shater/generate/ruleset.go

@@ -37,6 +37,7 @@ import (
 	"github.com/sagernet/sing/common/json/badoption"
 
 	"github.com/sagernet/sing-box/shater/model"
+	"github.com/sagernet/sing-box/shater/netplane"
 )
 
 // routeRulesetTagPrefix tags a materialised routing rule-set. Distinct from the
@@ -1424,12 +1425,16 @@ func (b *builder) peelDomainRegexes(diag string, entries []string) (rest, regexe
 // "ipcidr" is the model's spelling; "ip_cidr" mirrors the engine's field name and
 // "ip" is the shorthand. All three are exact synonyms. Everything else (including
 // the empty default) means a DOMAIN list.
+//
+// The vocabulary itself lives in netplane, which needs the identical answer to
+// decide whether a rule-set can name a private DESTINATION the data plane would
+// otherwise bypass (netplane.privateRoutedPlan). Two packages asking the same
+// question of the same field must not each carry their own list of spellings —
+// that is how "ip" ends up accepted on one side and not the other, and the
+// symptom is a rule that silently matches nothing. This is a delegation, not a
+// copy.
 func ruleSetTypeIsIPCIDR(rsType string) bool {
-	switch strings.ToLower(strings.TrimSpace(rsType)) {
-	case "ipcidr", "ip_cidr", "ip":
-		return true
-	}
-	return false
+	return netplane.IsIPCIDRRulesetType(rsType)
 }
 
 // warnUnknownRuleSetType reports a Ruleset.Type that is neither a domain nor an

shater/netplane/coverage.go

@@ -58,7 +58,10 @@ package netplane
 // probe fails, the gate closes, and rendering behaves exactly as it did before.
 
 import (
+	"encoding/json"
 	"fmt"
+	"net/netip"
+	"sort"
 	"strings"
 
 	"github.com/sagernet/sing-box/shater/model"
@@ -451,3 +454,660 @@ func parseInternetForwardedZones(exportText string) map[string]bool {
 	}
 	return fwd
 }
+
+// ---------------------------------------------------------------------------
+// EXPLICITLY NAMED PRIVATE DESTINATIONS
+//
+// The plane bypasses every private destination before the divert can see it —
+// three copies of `ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16,
+// 127.0.0.0/8, 169.254.0.0/16 } accept`, one per chain. That default is right:
+// LAN-to-LAN traffic, the router's own services and every local plane must not
+// be dragged through a tunnel, and a catch-all rule must never quietly acquire
+// them.
+//
+// It is wrong in exactly one case. When an operator writes a rule whose
+// destination rule-set names a private subnet OUTRIGHT — 10.10.10.0/24 ->
+// node:awghome, a second site behind a WireGuard node — the accept above fires
+// first and the rule NEVER MATCHES. Nothing said so: no warning, no counter, no
+// difference in the rendered plan. The rule simply did nothing, for ever.
+//
+// So: the bypass stays the default, and naming a subnet outright overrides it.
+// "Outright" is the whole contract, and it is deliberately narrow:
+//
+//   - the CIDR must be an ENTRY of an INLINE `config ruleset` with type=ipcidr —
+//     the only destination list whose contents this stage can actually read;
+//   - it must be CONTAINED in 10.0.0.0/8, 172.16.0.0/12 or 192.168.0.0/16. A
+//     prefix that merely overlaps one (0.0.0.0/0, 10.0.0.0/7) is not a naming,
+//     it is a catch-all that happens to include private space;
+//   - it must be referenced by an ENABLED rule (profile-effective) whose target
+//     actually routes somewhere other than plain direct — see
+//     privateOverrideTarget;
+//   - it must not overlap ANY network this router itself carries. That is not
+//     routing, it is self-amputation, and it is refused BY NAME, not silently;
+//   - 127.0.0.0/8, 169.254.0.0/16, 224.0.0.0/4 and 255.255.255.255 are never
+//     taken, whatever a list says.
+//
+// # Why the doubt always falls back to the bypass
+//
+// The two errors are not symmetrical. Failing to route a named subnet costs the
+// operator a feature and shows up the moment they test it. Routing a subnet we
+// should not have touched can take the router's own management network into a
+// tunnel that may not even be up — an outage reachable only over the console. So
+// every uncertain input (an unreadable list, an inventory we could not
+// enumerate, an address family we cannot check the router's own addresses in)
+// resolves to "leave it on the bypass", and says so.
+//
+// # IPv4 only, and the reason is the inventory, not the effort
+//
+// The router-network guard is built on `ubus call network.interface dump`, which
+// reports `ipv4-address` and nothing else. With no v6 inventory there is no way
+// to tell a named ULA apart from this router's OWN v6 LAN, so honouring a v6
+// private prefix would be exactly the guess the paragraph above forbids. Named
+// v6 private space is therefore left on the bypass and reported.
+//
+// # What it carries, and what it does not
+//
+// The override is rendered as TPROXY divert lines, so by itself it moves TCP and
+// UDP and nothing else: the kernel needs a socket to hand the packet to, and
+// ICMP has not got one. That is not an acceptable place to stop, because `ping`
+// is how anyone checks whether a route works — a named subnet whose TCP
+// succeeds while ping reports 100% loss reads as a broken route, which is this
+// same defect one protocol down.
+//
+// So the L3 ingress carries the echo, using the mechanism that already exists:
+// with `l3_tunnel` on, the renderer stamps the L3 mark on ICMP bound for these
+// destinations (again ABOVE the bypass, which is the only reason it was not
+// already happening), `ip rule` delivers it into the engine's TUN, the SAME
+// route rules pick the outbound there, and a WireGuard/AmneziaWG one moves it
+// for real. See model.Globals.L3Tunnel: TCP, UDP and ICMP echo are exactly what
+// sing-tun's dispatcher takes, and an outbound that cannot carry L3 drops the
+// echo honestly rather than forging a reply.
+//
+// With `l3_tunnel` OFF nothing carries ICMP and no line pretends otherwise —
+// and the rule that named the subnet is told so by name, because silence there
+// would rebuild the defect this file exists to remove.
+//
+// ESP, AH, GRE, IGMP and SCTP to a named private subnet are carried by nothing
+// here, exactly as they are to any other destination; Globals.Untunnelable and
+// Globals.UntunnelableEgress remain the only answers for them.
+
+// privateOverridable are the blocks inside which an explicitly named prefix may
+// override the bypass. Positive and closed: a prefix that is not CONTAINED in
+// one of these is never taken.
+var privateOverridable = []netip.Prefix{
+	netip.MustParsePrefix("10.0.0.0/8"),
+	netip.MustParsePrefix("172.16.0.0/12"),
+	netip.MustParsePrefix("192.168.0.0/16"),
+}
+
+// privateNeverRoutable is the part of the bypass no configuration may override.
+// Loopback, link-local, multicast and the limited broadcast address are not
+// destinations that survive a tunnel, and they carry planes the LAN needs to
+// stay up. A rule naming them is a mistake, and the mistake is reported rather
+// than obeyed.
+var privateNeverRoutable = []netip.Prefix{
+	netip.MustParsePrefix("127.0.0.0/8"),
+	netip.MustParsePrefix("169.254.0.0/16"),
+	netip.MustParsePrefix("224.0.0.0/4"),
+	netip.MustParsePrefix("255.255.255.255/32"),
+}
+
+// privateV6Bypassed mirrors the `ip6 daddr { ::1, fc00::/7, fe80::/10, ff00::/8
+// } accept` line. Used ONLY to recognise a named v6 private prefix so it can be
+// reported as not-overridden — never to route one. See the file comment.
+var privateV6Bypassed = []netip.Prefix{
+	netip.MustParsePrefix("::1/128"),
+	netip.MustParsePrefix("fc00::/7"),
+	netip.MustParsePrefix("fe80::/10"),
+	netip.MustParsePrefix("ff00::/8"),
+}
+
+// IsIPCIDRRulesetType reports whether a `config ruleset` type names an ADDRESS
+// list rather than a domain list. "ipcidr" is the model's spelling, "ip_cidr"
+// mirrors the engine's field name, "ip" is the shorthand; all three are exact
+// synonyms, and everything else (including the empty default) means domains.
+//
+// Exported because generate needs the identical answer when it builds the
+// engine's headless rule (generate.ruleSetTypeIsIPCIDR delegates here) and that
+// package cannot be imported from this one. One definition, two callers — the
+// alternative is a fourth copy of a vocabulary this project has already paid for.
+func IsIPCIDRRulesetType(rsType string) bool {
+	switch strings.ToLower(strings.TrimSpace(rsType)) {
+	case "ipcidr", "ip_cidr", "ip":
+		return true
+	}
+	return false
+}
+
+// addrListKind classifies a `config ruleset` by what this stage can learn from
+// it about ADDRESSES.
+type addrListKind int
+
+const (
+	// addrListNone: nothing here can name an address we could read or miss — a
+	// domain list, a geosite list, a geo IP list of country codes, or a source
+	// this build does not know.
+	addrListNone addrListKind = iota
+	// addrListInline: the Entries ARE the content, and we can read them.
+	addrListInline
+	// addrListOpaque: an address list whose content is a file, a download or a
+	// geo database — real addresses, invisible here.
+	addrListOpaque
+)
+
+// rulesetAddrListKind is the closed positive classification of a rule-set.
+//
+// There is no open default that routes: the last return is addrListNone, so an
+// unrecognised source contributes no CIDR and makes no claim. That direction is
+// the recoverable one — an unknown source is already reported by generate
+// ("unknown source %q, skipped"), and treating it as readable would mean
+// honouring `Entries` that the engine itself ignores.
+func rulesetAddrListKind(rs model.Ruleset) addrListKind {
+	switch strings.ToLower(strings.TrimSpace(rs.Source)) {
+	case "inline", "":
+		// "" is the UCI parser's own default for `source` (model/uci.go), so a
+		// section written without one is an inline list, and a model built in code
+		// that leaves the field blank means the same thing. Nothing is risked
+		// either way: only `Entries` are ever read here, and a non-inline
+		// rule-set's Entries are dead text the engine never looks at.
+		if IsIPCIDRRulesetType(rs.Type) {
+			return addrListInline
+		}
+		return addrListNone
+	case "url", "file":
+		if IsIPCIDRRulesetType(rs.Type) {
+			return addrListOpaque
+		}
+		return addrListNone
+	case "geoip":
+		// A geo IP list is an address list by construction, whatever its `type`
+		// says. Two cases are NOT opaque: one with no category at all builds
+		// nothing (generate warns and skips it, so it can neither route nor hide
+		// anything), and one whose categories are all ISO country codes holds
+		// public routable space only. Everything else — `private` above all, which
+		// IS the private-space list — is opaque.
+		cats := geoipCategories(rs)
+		switch {
+		case len(cats) == 0:
+			return addrListNone
+		case len(geoipCountryCategories(cats)) == len(cats):
+			return addrListNone
+		}
+		return addrListOpaque
+	case "geosite", "subscription":
+		return addrListNone
+	}
+	return addrListNone
+}
+
+// geoipCategories is the rule-set's non-empty category list.
+func geoipCategories(rs model.Ruleset) []string {
+	var out []string
+	for _, c := range rs.Categories {
+		if c = strings.TrimSpace(c); c != "" {
+			out = append(out, c)
+		}
+	}
+	return out
+}
+
+// geoipCountryCategories returns the categories that are ISO-3166 alpha-2
+// country codes. Closed and positive: a category this function does not
+// recognise is not trusted, so a typo errs towards the disclosure rather than
+// towards silence.
+func geoipCountryCategories(cats []string) []string {
+	var out []string
+	for _, c := range cats {
+		c = strings.ToLower(strings.TrimSpace(c))
+		if len(c) != 2 || c[0] < 'a' || c[0] > 'z' || c[1] < 'a' || c[1] > 'z' {
+			continue
+		}
+		out = append(out, c)
+	}
+	return out
+}
+
+// privateOverrideTarget answers whether rule r asks for something the private
+// bypass would deny it.
+//
+// Closed positive list. `node:`, `group:`, `chain:` and `egress:` send the
+// traffic somewhere the kernel's own routing would not, and `block` is a verdict
+// the bypass silently overrules — all four are real requests. `direct` and the
+// empty target are NOT: direct means "leave without the engine", which is
+// precisely what the bypass already does, and diverting a private destination
+// into the engine to reach the same verdict would be strictly worse — the
+// engine's direct outbound follows the DEFAULT route, so LAN-to-LAN traffic
+// could be pushed out the WAN. An unrecognised target routes to block anyway
+// (generate's ruleKillFallback) but is not honoured here: a target we cannot
+// name is not an explicit request.
+func privateOverrideTarget(r model.Rule) bool {
+	kind, _ := model.SplitTarget(model.EffectiveRuleTarget(r))
+	switch strings.ToLower(strings.TrimSpace(kind)) {
+	case "node", "group", "chain", "egress", "block":
+		return true
+	}
+	return false
+}
+
+// routerNet is one IPv4 network this router carries, with the two things the
+// guard has to tell apart: which interface it belongs to (so a refusal can name
+// it) and whether that interface is an UPLINK or one of our own networks.
+type routerNet struct {
+	Prefix netip.Prefix
+	Iface  string
+	// WAN is true when the interface sits in a firewall zone conventionally used
+	// for the uplink (model.WANZoneNames — the same list rule-source validation
+	// and the coverage check use). Unknown zone means NOT wan, which is the
+	// conservative reading: an unreadable firewall config must not downgrade a
+	// network into "just an uplink".
+	WAN bool
+}
+
+// routerIPv4Networks enumerates every IPv4 network this router itself carries,
+// from the same `ubus call network.interface dump` the interface picker reads —
+// but ALL addresses of ALL interfaces, loopback and secondaries included, where
+// IfaceInfo keeps only the primary of each non-loopback one. The guard must be
+// able to see an address the panel has no reason to show.
+//
+// ok=false means "could not be enumerated", and that includes an inventory that
+// parsed but holds no IPv4 address at all: a list of zero networks cannot prove
+// a named subnet is not one of ours, and pretending otherwise is the guess this
+// whole mechanism exists to avoid.
+func routerIPv4Networks() ([]routerNet, bool) {
+	out, err := execCommand("ubus", "call", "network.interface", "dump").Output()
+	if err != nil {
+		return nil, false
+	}
+	var doc struct {
+		Interface []struct {
+			Interface string `json:"interface"`
+			IPv4      []struct {
+				Address string `json:"address"`
+				Mask    int    `json:"mask"`
+			} `json:"ipv4-address"`
+		} `json:"interface"`
+	}
+	if json.Unmarshal(out, &doc) != nil {
+		return nil, false
+	}
+	zones := zoneMap()
+	var nets []routerNet
+	for _, i := range doc.Interface {
+		name := strings.TrimSpace(i.Interface)
+		wan := model.WANZoneNames[strings.ToLower(strings.TrimSpace(zones[name]))]
+		for _, a := range i.IPv4 {
+			addr, err := netip.ParseAddr(strings.TrimSpace(a.Address))
+			if err != nil || !addr.Is4() || a.Mask < 0 || a.Mask > 32 {
+				continue
+			}
+			nets = append(nets, routerNet{
+				Prefix: netip.PrefixFrom(addr, a.Mask).Masked(),
+				Iface:  name,
+				WAN:    wan,
+			})
+		}
+	}
+	return nets, len(nets) > 0
+}
+
+// parsePrivatePrefix reads one rule-set entry the way the engine does: a CIDR,
+// or a bare address treated as a full-length prefix. Unparseable entries are
+// skipped silently — generate already reports them by name ("bad ip_cidr entry
+// %q, skipped") and a second copy of that message from a second package would
+// only teach the operator to skim.
+func parsePrivatePrefix(entry string) (netip.Prefix, bool) {
+	e := strings.TrimSpace(entry)
+	if e == "" {
+		return netip.Prefix{}, false
+	}
+	if p, err := netip.ParsePrefix(e); err == nil {
+		return p.Masked(), true
+	}
+	if a, err := netip.ParseAddr(e); err == nil {
+		return netip.PrefixFrom(a, a.BitLen()), true
+	}
+	return netip.Prefix{}, false
+}
+
+// prefixWithinAny reports whether p lies wholly inside one of blocks.
+func prefixWithinAny(p netip.Prefix, blocks []netip.Prefix) bool {
+	for _, b := range blocks {
+		if b.Bits() <= p.Bits() && b.Contains(p.Addr()) {
+			return true
+		}
+	}
+	return false
+}
+
+func prefixOverlapsAny(p netip.Prefix, blocks []netip.Prefix) bool {
+	for _, b := range blocks {
+		if p.Overlaps(b) {
+			return true
+		}
+	}
+	return false
+}
+
+// privateRoutedPlan resolves the explicitly named private destinations of this
+// model into (a) the IPv4 prefixes the ruleset must divert BEFORE the private
+// bypass and (b) everything it refused to take, said out loud.
+//
+// rules must be the PROFILE-EFFECTIVE rules at the render instant (nftPlanRules)
+// — the same list the divert-device set and the engine plan are built from, so
+// the three can never disagree about which rules are live.
+//
+// The returned prefixes are canonical (masked), de-duplicated, pairwise DISJOINT
+// and deterministically ordered; see renderablePrefixSet for why each of those
+// is load-bearing.
+func privateRoutedPlan(m *model.Model, rules []model.Rule) ([]string, []string) {
+	if m == nil {
+		return nil, nil
+	}
+	sets := make(map[string]model.Ruleset, len(m.Rulesets))
+	for _, rs := range m.Rulesets {
+		n := strings.TrimSpace(rs.Name)
+		if n == "" {
+			continue
+		}
+		if _, seen := sets[n]; !seen {
+			sets[n] = rs
+		}
+	}
+
+	// Read ONCE per plan: the answer cannot change inside one render, and the
+	// probe is a process spawn.
+	routerNets, haveInventory := routerIPv4Networks()
+
+	var warnings []string
+	var accepted []netip.Prefix
+	seenAccepted := map[netip.Prefix]bool{}
+	// Rules that actually contributed a routed prefix, in order, once each — the
+	// audience for the ICMP note below.
+	var routing []string
+	seenRouting := map[string]bool{}
+	// One disclosure per opaque list, however many rules point at it.
+	seenOpaque := map[string]bool{}
+	// One verdict per (rule, rule-set, prefix): a list that repeats an entry, or a
+	// rule that names the same set twice, must not say the same thing twice.
+	seenVerdict := map[string]bool{}
+
+	for _, r := range rules {
+		if !r.Enabled || !privateOverrideTarget(r) {
+			continue
+		}
+		for _, ref := range r.DstRuleset {
+			rs, ok := sets[strings.TrimSpace(ref)]
+			if !ok {
+				// A dangling reference: generate reports it, and it names no address.
+				continue
+			}
+			switch rulesetAddrListKind(rs) {
+			case addrListOpaque:
+				if !seenOpaque[rs.Name] {
+					seenOpaque[rs.Name] = true
+					warnings = append(warnings, opaqueAddressListWarning(rs))
+				}
+			case addrListInline:
+				for _, e := range rs.Entries {
+					p, parsed := parsePrivatePrefix(e)
+					if !parsed {
+						continue
+					}
+					key := r.Name + "\x00" + rs.Name + "\x00" + p.String()
+					if seenVerdict[key] {
+						continue
+					}
+					seenVerdict[key] = true
+					w, take := privatePrefixVerdict(r, rs, p, routerNets, haveInventory)
+					if w != "" {
+						warnings = append(warnings, w)
+					}
+					if take {
+						if !seenAccepted[p] {
+							seenAccepted[p] = true
+							accepted = append(accepted, p)
+						}
+						if !seenRouting[r.Name] {
+							seenRouting[r.Name] = true
+							routing = append(routing, r.Name)
+						}
+					}
+				}
+			case addrListNone:
+				// Nothing in this list can name an address; nothing to read, nothing
+				// to disclose.
+			}
+		}
+	}
+	cidrs := renderablePrefixSet(accepted)
+	// PING IS HOW ANYONE CHECKS THIS, and with l3_tunnel off nothing carries it.
+	//
+	// The divert is a TPROXY divert, so it moves TCP and UDP: kernel TPROXY hands
+	// the packet to a socket and there is no socket for ICMP. The L3 ingress is
+	// what carries an echo into the engine (see model.Globals.L3Tunnel), and when
+	// it is switched off the ruleset emits no ICMP line for these destinations at
+	// all. Saying nothing here would reproduce this whole defect one protocol
+	// down: `curl` works, `ping` reports 100% loss, and the operator concludes the
+	// route is broken.
+	if len(cidrs) > 0 && !L3Enabled(m.Globals) {
+		for _, name := range routing {
+			warnings = append(warnings, fmt.Sprintf(
+				"rule %q: the private destinations this rule names (%s) ARE routed into the engine, but for "+
+					"TCP and UDP only — the divert is a TPROXY divert and the kernel needs a socket to hand "+
+					"the packet to, which ICMP has not got. `l3_tunnel` is OFF, so nothing carries ping or "+
+					"traceroute to them either: TCP and UDP will work and a ping into %s will not, which "+
+					"reads as a broken route rather than as a setting. Set `option l3_tunnel '1'` to carry "+
+					"ICMP echo through the same rules — an L3-capable outbound (WireGuard/AmneziaWG, or "+
+					"direct) moves it for real; a vless/trojan/ss one still cannot, and drops it honestly.",
+				name, strings.Join(cidrs, ", "), cidrs[0]))
+		}
+	}
+	return cidrs, warnings
+}
+
+// privatePrefixVerdict decides ONE named prefix. It returns the sentence to
+// report (empty when there is nothing to say) and whether the prefix is taken.
+//
+// Every branch that does not take the prefix says why, because a rule that
+// cannot fire and reports nothing is the defect this whole mechanism was written
+// for. The only branch that takes it is the last one, reached after every
+// refusal has had its chance.
+func privatePrefixVerdict(r model.Rule, rs model.Ruleset, p netip.Prefix, routerNets []routerNet, haveInventory bool) (string, bool) {
+	if p.Addr().Is6() {
+		if prefixOverlapsAny(p, privateV6Bypassed) {
+			return fmt.Sprintf(
+				"rule %q: destination %s (rule-set %q) is IPv6 private space, and the private-destination "+
+					"bypass is overridden for IPv4 only. This router's interface inventory (`ubus call "+
+					"network.interface dump`) reports IPv4 addresses and nothing else, so there is no way to "+
+					"tell a named ULA apart from this router's OWN IPv6 LAN — and taking it on a guess could "+
+					"route the LAN's own v6 into the tunnel. It stays on the bypass: this rule does NOT route "+
+					"%s.", r.Name, p, rs.Name, p), false
+		}
+		return "", false
+	}
+	// ELIGIBILITY, and the order of the alternatives is load-bearing.
+	//
+	// A prefix is eligible only if it lies WHOLLY inside one of the three RFC1918
+	// blocks and touches no never-routable space. The second half of that
+	// conjunction is unreachable today — those blocks are disjoint from loopback,
+	// link-local, multicast and the broadcast address — and it stays because the
+	// day somebody widens privateOverridable is the day it stops being unreachable,
+	// and multicast must not become divertible by an edit to a different list.
+	//
+	// When it is NOT eligible, "is it a catch-all?" is asked BEFORE "is it
+	// reserved?", because 0.0.0.0/0 is both and only one of those is the answer
+	// the operator needs: they wrote a catch-all, and the news is that it does not
+	// acquire private space, not that it contains 127.0.0.0/8.
+	if !prefixWithinAny(p, privateOverridable) || prefixOverlapsAny(p, privateNeverRoutable) {
+		switch {
+		case prefixOverlapsAny(p, privateOverridable):
+			return fmt.Sprintf(
+				"rule %q: destination %s (rule-set %q) covers private address space without naming it: it is "+
+					"not CONTAINED in 10.0.0.0/8, 172.16.0.0/12 or 192.168.0.0/16, so it is a catch-all that "+
+					"happens to include them, not an explicit destination. The private-destination bypass is "+
+					"NOT overridden by it, and the private addresses it covers go to plain routing before this "+
+					"rule is ever consulted. Add the subnet you actually mean (e.g. 10.10.10.0/24) as its own "+
+					"entry.", r.Name, p, rs.Name), false
+		case prefixOverlapsAny(p, privateNeverRoutable):
+			return fmt.Sprintf(
+				"rule %q: destination %s (rule-set %q) is loopback, link-local, multicast or broadcast space. "+
+					"That is never diverted into the engine whatever a rule says — it is not a destination "+
+					"that survives a tunnel, and it carries planes the LAN needs to stay up. It stays on the "+
+					"bypass: this rule does NOT route %s.", r.Name, p, rs.Name, p), false
+		}
+		// Ordinary public space: the catch-all divert already carries it, and this
+		// mechanism has nothing to say about it.
+		return "", false
+	}
+	if !haveInventory {
+		return fmt.Sprintf(
+			"rule %q: destination %s (rule-set %q) is an explicitly named private subnet, but this router's "+
+				"network inventory could not be read (`ubus call network.interface dump` returned nothing "+
+				"usable), so there is no way to prove %s is not one of this router's OWN networks. Overriding "+
+				"the private-destination bypass on that guess could route this router's LAN into the tunnel "+
+				"and lock you out of it, so it is REFUSED: this rule does NOT route %s. Nothing needs fixing "+
+				"if netifd was simply not up yet — reapply once it is.",
+			r.Name, p, rs.Name, p, p), false
+	}
+	// THE SELF-AMPUTATION GUARD, and the distinction inside it is the difference
+	// between a safety device and an obstacle.
+	//
+	// A collision with one of the router's OWN networks — a LAN, a guest VLAN, a
+	// management link, anything not in a WAN zone — is refused outright. Those are
+	// the networks whose devices this router serves; diverting them would take the
+	// LAN away from the LAN, and the operator would discover it over the console.
+	//
+	// A collision with an UPLINK subnet is a different fact and must not be
+	// treated as the same one. ISPs hand out RFC1918 WANs routinely (this router's
+	// own default gateway is 10.0.0.1), and on a /8 or /12 uplink EVERY private
+	// subnet in the world "collides" — so refusing on that basis would disable the
+	// whole feature on exactly the routers most likely to want it, and the reason
+	// would look like a bug rather than a decision. Nothing of ours lives there:
+	// the router's own addresses are already accepted by `fib daddr type local`
+	// above these lines, and the router's OWN egress never traverses them because
+	// every divert line is scoped to LAN ingress. What is actually lost is LAN
+	// clients reaching hosts inside the ISP's subnet, which is worth a sentence,
+	// not a refusal.
+	//
+	// A WAN collision therefore ROUTES and DISCLOSES; and it is recorded rather
+	// than returned immediately, so a second, non-WAN collision on the same prefix
+	// still wins and refuses.
+	var wanHit *routerNet
+	for i, own := range routerNets {
+		if !p.Overlaps(own.Prefix) {
+			continue
+		}
+		if own.WAN {
+			if wanHit == nil {
+				wanHit = &routerNets[i]
+			}
+			continue
+		}
+		return fmt.Sprintf(
+			"rule %q: destination %s (rule-set %q) overlaps %s, which is a network THIS ROUTER carries "+
+				"(interface %q). Diverting it would take the router's own network away from the devices on "+
+				"it — local services, the panel and SSH on that subnet would be pushed into the tunnel — so "+
+				"it is REFUSED and stays on the private-destination bypass: this rule does NOT route %s. "+
+				"Name the subnet you actually mean, or renumber the network that collides with it.",
+			r.Name, p, rs.Name, own.Prefix, own.Iface, p), false
+	}
+	if wanHit != nil {
+		return fmt.Sprintf(
+			"rule %q: destination %s (rule-set %q) IS routed into the engine, and you should know it also "+
+				"overlaps %s — the subnet this router's own uplink %q sits on. Nothing of this router's is "+
+				"lost (its own addresses are accepted before these lines, and its own traffic never enters "+
+				"them), but LAN clients can no longer reach hosts inside that uplink subnet directly: those "+
+				"destinations now go to this rule's target instead. If that is not what you meant, narrow "+
+				"the rule-set entry to the subnet you actually want.",
+			r.Name, p, rs.Name, wanHit.Prefix, wanHit.Iface), true
+	}
+	return "", true
+}
+
+// opaqueAddressListWarning is the disclosure required by the one thing this
+// mechanism cannot do: read a list it does not hold.
+//
+// WHY IT IS UNCONDITIONAL. The alternative — warn only once the plan already
+// contains a named private subnet — is silent in precisely the case that
+// matters: an operator whose ONLY destination list is a downloaded file
+// containing 10.10.10.0/24, wondering why the rule never fires. And "warn on
+// suspicion" is not available: suspicion would mean reading the list, which is
+// the thing this stage cannot do.
+//
+// WHY IT IS NOT NOISE. It fires only for an ADDRESS list — a `url`/`file`
+// rule-set declared type=ipcidr, or a geo IP list whose category is not an ISO
+// country code — referenced by an ENABLED rule that actually asks to route
+// somewhere. A configuration of domain rule-sets and geoip country lists, which
+// is the ordinary shape, produces none of these. The domain blind spot is named
+// inside the sentence rather than given a warning of its own, for the same
+// reason: one warning per domain list would be a warning on every config, and a
+// warning on every config is read by nobody.
+func opaqueAddressListWarning(rs model.Ruleset) string {
+	return fmt.Sprintf(
+		"ruleset %q: this is an address list whose contents this data plane never sees (source=%s), and "+
+			"there is exactly one consequence. Destinations in private space (10.0.0.0/8, 172.16.0.0/12, "+
+			"192.168.0.0/16) are bypassed BEFORE the engine is consulted, and that bypass is overridden only "+
+			"for a private subnet written out as an entry of an INLINE type=ipcidr rule-set. If this list "+
+			"contains private addresses, the rules pointing at it do NOT route them — and this plan cannot "+
+			"tell you whether it does, because it never reads the list. The same blind spot covers domain "+
+			"rule-sets whose names resolve into private space. To route one, add it as an inline type=ipcidr "+
+			"entry. If it contains only public addresses, nothing to do.",
+		rs.Name, strings.ToLower(strings.TrimSpace(rs.Source)))
+}
+
+// renderablePrefixSet turns the accepted prefixes into the deterministic,
+// pairwise-disjoint, canonical string list the ruleset renders.
+//
+// Determinism is load-bearing: the applier's idempotence check compares the
+// rendered TEXT, so a set whose order depended on map iteration would reload the
+// data plane on every reconcile.
+//
+// Disjointness is a smaller claim than it looks, and it is worth stating what
+// was actually measured rather than what one would assume. `nft -f` on this very
+// ruleset with { 10.10.10.0/24, 10.10.0.0/16 } in the set LOADED — nftables
+// 1.0.9 auto-merges overlapping members of an ANONYMOUS set rather than
+// rejecting them. So this is not rescuing a table that would otherwise fail to
+// load. It is here because (a) a covered prefix contributes nothing, so emitting
+// it is noise the operator would have to read past in `nft list ruleset`, and
+// (b) relying on auto-merge would make the rendered text's fate depend on an
+// nftables version, and this text is compared byte-for-byte by the applier.
+// Whether a NAMED interval set (`flags interval`, no `auto-merge`) would reject
+// the same input was not tested; nothing here renders one.
+func renderablePrefixSet(in []netip.Prefix) []string {
+	if len(in) == 0 {
+		return nil
+	}
+	ps := append([]netip.Prefix(nil), in...)
+	// Widest first, so a covering prefix is always already kept when the prefixes
+	// it covers are examined.
+	sort.Slice(ps, func(i, j int) bool {
+		if ps[i].Bits() != ps[j].Bits() {
+			return ps[i].Bits() < ps[j].Bits()
+		}
+		return ps[i].Addr().Compare(ps[j].Addr()) < 0
+	})
+	var kept []netip.Prefix
+	for _, p := range ps {
+		if prefixWithinAny(p, kept) {
+			continue
+		}
+		kept = append(kept, p)
+	}
+	sort.Slice(kept, func(i, j int) bool {
+		if c := kept[i].Addr().Compare(kept[j].Addr()); c != 0 {
+			return c < 0
+		}
+		return kept[i].Bits() < kept[j].Bits()
+	})
+	out := make([]string, 0, len(kept))
+	for _, p := range kept {
+		out = append(out, p.String())
+	}
+	return out
+}
+
+// nftPrivateSetExpr renders the accepted prefixes as an nft anonymous set. The
+// caller has already established that the list is non-empty.
+func nftPrivateSetExpr(cidrs []string) string {
+	return "{ " + strings.Join(cidrs, ", ") + " }"
+}

shater/netplane/nft.go

@@ -925,7 +925,8 @@ func RenderHoldNftAt(m *model.Model, now time.Time) (string, error) {
 	// one. The failure is not lost, only late — the first full render reports it
 	// (uncoveredNetworkWarnings), and that is the honest description of this
 	// branch rather than a claim that the hold plane is complete.
-	devs, _ := nftDivertDevs(m, nftPlanRules(m, now))
+	planRules := nftPlanRules(m, now)
+	devs, _ := nftDivertDevs(m, planRules)
 	validDevs, _ := nftSplitDevs(devs)
 	// The holding plane has no divert, so nothing here can make one APPEAR: record
 	// an empty set, or the next ApplyNft would inherit the previous full plan's
@@ -960,6 +961,25 @@ func RenderHoldNftAt(m *model.Model, now time.Time) (string, error) {
 		}
 		sb.WriteString(fmt.Sprintf("\t\tmeta mark 0x%x oifname %q accept\n", EgressMark(m.Globals, i), dev))
 	}
+	// A private subnet an enabled rule names OUTRIGHT is NOT part of "keep the LAN
+	// usable" — the operator asked for it to be routed, and with the engine down
+	// there is nothing to route it. Dropped here, ABOVE the LAN-to-LAN accept, so
+	// it fails closed like every other destination this plan would have diverted;
+	// letting it through would hand it to the main table, i.e. out the default
+	// WAN, which is the leak this whole plane exists to prevent.
+	//
+	// The warnings are discarded, as the zone-resolution error above is and for
+	// the same reason: this path is called from the boot armor, where there is no
+	// apply and no panel to report to. The first full render says everything.
+	//
+	// HONEST LIMITATION: privateRoutedPlan needs the router's interface inventory
+	// to prove a named subnet is not one of ours, and at boot netifd may not have
+	// answered yet. When it has not, the list is empty, no drop is emitted, and
+	// that destination behaves exactly as it did before this existed — accepted
+	// as LAN-to-LAN. Failing the other way would mean dropping traffic on a guess.
+	if priv, _ := privateRoutedPlan(m, planRules); len(priv) > 0 {
+		sb.WriteString("\t\t" + iif + " ip daddr " + nftPrivateSetExpr(priv) + " drop\n")
+	}
 	// Keep the LAN itself usable.
 	sb.WriteString("\t\t" + iif + " ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16 } accept\n")
 	sb.WriteString("\t\t" + iif + " icmpv6 type { nd-router-solicit, nd-router-advert, nd-neighbor-solicit, nd-neighbor-advert } accept\n")
@@ -1104,6 +1124,18 @@ func renderNft(m *model.Model, plan *UntunnelablePlan, now time.Time) (string, [
 	// picker (or nowhere at all). Collected BEFORE the refusal below, so a refused
 	// plan still tells the operator everything that is wrong with it.
 	warnings = append(warnings, coverageWarnings(m, divertRefs)...)
+	// Destinations in private space that an enabled rule names OUTRIGHT, and every
+	// named private destination this plan REFUSED to take, with the reason. See
+	// the block comment above privateOverridable: the bypass stays the default and
+	// only an explicit naming overrides it, so a rule that cannot fire is now said
+	// out loud instead of rendering as a plan indistinguishable from a working one.
+	//
+	// Collected here, next to the coverage warnings, because it is the same kind
+	// of statement — what this plan does NOT do — and it must survive the refusals
+	// below for the same reason: a refused plan should still tell the operator
+	// everything that is wrong with it.
+	privCIDRs, privWarnings := privateRoutedPlan(m, planRules)
+	warnings = append(warnings, privWarnings...)
 	// What this plan covers for ONE protocol and not the other. Deliberately NOT
 	// inside coverageWarnings: everything in there is gated on the router
 	// inventory, and this is derived from the model alone — see
@@ -1339,6 +1371,72 @@ func renderNft(m *model.Model, plan *UntunnelablePlan, now time.Time) (string, [
 		}
 	}
 	sb.WriteString("\t\tfib daddr type local accept\n")
+	// --- explicitly named private destinations (see privateOverridable) ---
+	// ORDER IS THE WHOLE MECHANISM. The `ip daddr { 10.0.0.0/8, ... } accept`
+	// immediately below is what makes a rule like `10.10.10.0/24 -> node:awghome`
+	// never fire: the packet is accepted before any divert line is reached, so the
+	// engine — which holds the rule — never sees it. Putting the divert for the
+	// NAMED subnets one line higher is the entire fix, and it is deliberately not
+	// done by subtracting them from that set: an interval set minus a member is a
+	// second thing to keep in step with three chains, while first-match order is
+	// already the semantics nft gives us.
+	//
+	// BELOW `fib daddr type local accept` on purpose. Every address the router
+	// itself holds keeps reaching the router, whatever a list says, so a named
+	// subnet can never swallow the panel or SSH even if the overlap guard in
+	// privatePrefixVerdict were somehow wrong. Two independent defences, and this
+	// one costs a line.
+	//
+	// Per DEVICE, exactly like the DNS force-intercept above: the port and the
+	// tcp/udp flags come from the inbound that owns each ingress device. A device
+	// whose inbound does not carry a protocol gets no line for it, rather than a
+	// divert to a socket the engine never opened (`tproxy` would return NFT_BREAK,
+	// the rule would abort before its accept, and the packet would fall through to
+	// the bypass anyway — the behaviour of not emitting the line, plus a false
+	// claim in the ruleset).
+	//
+	// No counter. This set is the UNION of what several rules named, possibly
+	// through a shared rule-set, so no packet here can be attributed to one rule;
+	// a `c_rule_x` on these lines would be a fabricated measurement, which this
+	// plane refuses to produce (see the appendLines comment).
+	if len(privCIDRs) > 0 && hasPrimary && dnsIif != "" {
+		privSet := "ip daddr " + nftPrivateSetExpr(privCIDRs)
+		for _, g := range nftGroupDevs(m, inboundParams(primary), validDevs) {
+			giif := nftIifExpr(g.devs)
+			if giif == "" {
+				continue
+			}
+			if g.params.tcp {
+				sb.WriteString(nftDivertLine(giif, "tcp", privSet, 4, g.params.port, tproxyMark, ""))
+			}
+			if g.params.udp {
+				sb.WriteString(nftDivertLine(giif, "udp", privSet, 4, g.params.port, tproxyMark, ""))
+			}
+		}
+		// PING, which is how anyone checks whether this worked. TPROXY needs a
+		// socket and there is none for ICMP, so the two lines above carry
+		// connections and nothing else — a named subnet whose TCP works while
+		// `ping` reports 100% loss looks exactly like a broken feature.
+		//
+		// The L3 ingress is the mechanism that already exists for this: the mark
+		// plus addL3Routing's `ip rule` deliver the echo into the engine's TUN,
+		// where the SAME route rules pick the outbound, and a WireGuard/AmneziaWG
+		// one carries it for real (see model.Globals.L3Tunnel — the limit is
+		// sing-tun's dispatcher, which takes TCP, UDP and ICMP echo). Nothing new
+		// is invented here; the existing L3 line simply sits BELOW the private
+		// bypass and so never sees these destinations.
+		//
+		// icmp only, never `l4proto != { tcp, udp }`, for the reason spelled out
+		// at the L3 block below: a marked ESP/GRE/SCTP packet enters the TUN and
+		// vanishes instead of receiving the untunnelable policy's honest verdict.
+		//
+		// With l3_tunnel OFF nothing is emitted and nothing is claimed — the
+		// operator is told so by name (see privateRoutedPlan's l3 note).
+		if L3Enabled(m.Globals) {
+			sb.WriteString(fmt.Sprintf("\t\t%s %s ip protocol icmp meta mark set 0x%x accept\n",
+				dnsIif, privSet, L3Mark(m.Globals)))
+		}
+	}
 	sb.WriteString("\t\tip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16, 224.0.0.0/4, 255.255.255.255 } accept\n")
 	sb.WriteString("\t\tip6 daddr { ::1, fc00::/7, fe80::/10, ff00::/8 } accept\n")
 	// --- L3 ingress (Globals.L3Tunnel): route what TPROXY cannot carry ---
@@ -1526,7 +1624,31 @@ func renderNft(m *model.Model, plan *UntunnelablePlan, now time.Time) (string, [
 				sb.WriteString(fmt.Sprintf("\t\tmeta mark 0x%x oifname %q accept\n", EgressMark(m.Globals, i), dev))
 			}
 			// 2) LAN-to-LAN / link-local (v4): the LAN itself must keep working
-			//    (same private-range bypass sets the prerouting chain uses).
+			//    (same private-range bypass sets the prerouting chain uses) —
+			//    EXCEPT the private subnets an enabled rule named outright, which
+			//    are diverted in prerouting and therefore have to be droppable
+			//    here like every other diverted destination. This chain sees a
+			//    diverted packet only when the divert FAILED (the engine's tproxy
+			//    socket is down, `tproxy` returned NFT_BREAK), and accepting it
+			//    then would send it to the main table, i.e. out the default WAN in
+			//    the clear. The drop must precede the accept: first match wins.
+			//
+			//    THE L3 LEG MUST SURVIVE IT. With l3_tunnel on, prerouting marks
+			//    ICMP for these destinations into the engine's TUN, and that leg
+			//    DOES traverse this chain (iifname LAN, oifname the TUN) — unlike
+			//    the tproxy legs, which are delivered locally and never appear
+			//    here. The `oifname "shater-l3*" accept` that normally rescues it
+			//    sits four steps below this line, so without an exclusion this
+			//    drop would eat exactly the packets prerouting just routed. The
+			//    mark is the discriminator rather than the device name because it
+			//    is what prerouting actually stamped, and it needs no wildcard.
+			if len(privCIDRs) > 0 {
+				drop := "\t\t" + dnsIif + " ip daddr " + nftPrivateSetExpr(privCIDRs)
+				if L3Enabled(m.Globals) {
+					drop += fmt.Sprintf(" meta mark != 0x%x", L3Mark(m.Globals))
+				}
+				sb.WriteString(drop + " drop\n")
+			}
 			sb.WriteString("\t\t" + dnsIif + " ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16 } accept\n")
 			// 3) IPv6 plane. Public v6 escapes the divert exactly like v4, so it is
 			//    dropped below in BOTH cases; only the surviving accepts differ:

shater/netplane/privatedst_test.go

@@ -0,0 +1,838 @@
+package netplane
+
+// EXPLICITLY NAMED PRIVATE DESTINATIONS — the rule that could not fire.
+//
+// The defect: `ip daddr { 10.0.0.0/8, ... } accept` sits ABOVE every divert line,
+// so a rule whose destination rule-set names 10.10.10.0/24 is never consulted —
+// the packet is accepted and handed to plain routing before the engine sees it.
+// The plan rendered identically whether the rule existed or not, and nothing said
+// so.
+//
+// Every test here drives the REAL RenderNftPlanAt / RenderHoldNftAt, because the
+// question is what the KERNEL is handed and in what order, and a test of the
+// helper alone would prove nothing about the rendered chain.
+//
+// THE CONTROL IS THE POINT. Two of these tests are negative on purpose:
+// TestPrivateDestinationBypassIsStillTheDefault (no explicit naming => private is
+// still bypassed) and TestPrivateDestinationCatchAllDoesNotTakePrivate. Without
+// them a "green" positive test would be satisfied by a plane that had simply
+// stopped bypassing private space at all — which is the outage this whole
+// mechanism is written to avoid, not the feature.
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/sagernet/sing-box/shater/model"
+)
+
+// privDstIfaceDump is a router shaped like the one this was found on: a LAN on
+// 10.67.0.0/24 (so a rule naming 10.67.x collides with the router's OWN network),
+// a second LAN, and a public-addressed uplink that cannot accidentally collide
+// with the RFC1918 fixtures below.
+const privDstIfaceDump = `{"interface":[
+	{"interface":"loopback","up":true,"l3_device":"lo","ipv4-address":[{"address":"127.0.0.1","mask":8}]},
+	{"interface":"lan","up":true,"l3_device":"br-lan","ipv4-address":[{"address":"10.67.0.1","mask":24}]},
+	{"interface":"cam","up":true,"l3_device":"br-cam","ipv4-address":[{"address":"172.20.5.1","mask":24}]},
+	{"interface":"wan0","up":true,"l3_device":"eth1","ipv4-address":[{"address":"198.51.100.7","mask":24}]}
+]}`
+
+// privDstExec answers the two probes this mechanism makes — the interface dump
+// (the router-network guard) and IfaceDevice's per-interface status — and returns
+// empty output for everything else, which is what every other netplane test sees.
+//
+// dump == "" means "this router could not be enumerated": the ubus call still
+// succeeds but yields nothing parseable, which is the state the guard must treat
+// as "cannot prove the subnet is not ours".
+func privDstExec(dump, firewall string) func(string, ...string) *exec.Cmd {
+	return func(name string, arg ...string) *exec.Cmd {
+		joined := strings.Join(append([]string{name}, arg...), " ")
+		out := ""
+		switch {
+		case joined == "ubus call network.interface dump":
+			out = dump
+		case joined == "uci -q export firewall":
+			out = firewall
+		case joined == "ubus call network.interface.lan status":
+			out = `{"l3_device":"br-lan"}`
+		}
+		cs := append([]string{"-test.run=TestSysctlRevertHelperProcess", "--", name}, arg...)
+		cmd := exec.Command(os.Args[0], cs...)
+		cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1", "GO_HELPER_STDOUT="+out)
+		return cmd
+	}
+}
+
+func withPrivDstRouter(t *testing.T, dump string) {
+	t.Helper()
+	withPrivDstRouterFW(t, dump, "")
+}
+
+// withPrivDstRouterFW also answers `uci -q export firewall`, which is what makes
+// the zone of each interface knowable — and therefore what tells an UPLINK
+// subnet apart from one of the router's own networks.
+func withPrivDstRouterFW(t *testing.T, dump, firewall string) {
+	t.Helper()
+	orig := execCommand
+	execCommand = privDstExec(dump, firewall)
+	t.Cleanup(func() { execCommand = orig })
+	fakeSysClassNet(t, "br-lan", "br-cam", "eth1")
+}
+
+// privDstModel is the owner's shape: one LAN, kill-switch closed, and whatever
+// rule-sets/rules the caller wants to point at a WireGuard node.
+func privDstModel(sets []model.Ruleset, rules []model.Rule) *model.Model {
+	return &model.Model{
+		Globals: model.Globals{
+			Enabled: true, KillSwitch: "closed", IPv6: false,
+			FwmarkBase: 0x2000, TableBase: 0x2000,
+		},
+		Inbounds: []model.Inbound{{
+			Name: "lan", Enabled: true, Type: "tproxy", Network: "lan",
+			TproxyPort: 12345, TCP: true, UDP: true,
+		}},
+		Rulesets: sets,
+		Rules:    rules,
+	}
+}
+
+func privDstInlineSet(name string, entries ...string) model.Ruleset {
+	return model.Ruleset{Name: name, Type: "ipcidr", Source: "inline", Entries: entries}
+}
+
+func privDstRule(name, set, target string) model.Rule {
+	return model.Rule{Name: name, Enabled: true, Order: 10, DstRuleset: []string{set}, Target: target}
+}
+
+func privDstRender(t *testing.T, m *model.Model) (string, []string) {
+	t.Helper()
+	rs, ws, err := RenderNftPlanAt(m, nil, time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC))
+	if err != nil {
+		t.Fatalf("RenderNftPlanAt: %v", err)
+	}
+	return rs, ws
+}
+
+// lineIndex returns the index of the first line CONTAINING sub, or -1.
+func privDstLineIndex(ruleset, sub string) int {
+	for i, l := range strings.Split(ruleset, "\n") {
+		if strings.Contains(l, sub) {
+			return i
+		}
+	}
+	return -1
+}
+
+// privDstWarningWith returns the first warning containing sub, or "".
+func privDstWarningWith(ws []string, sub string) string {
+	for _, w := range ws {
+		if strings.Contains(w, sub) {
+			return w
+		}
+	}
+	return ""
+}
+
+const privDstBypassV4 = "ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16, 224.0.0.0/4, 255.255.255.255 } accept"
+
+// ---------------------------------------------------------------------------
+// THE CONTROL: no explicit naming => private space is STILL bypassed.
+//
+// RED IF THE FEATURE OVERREACHES: if the plane ever stopped bypassing private
+// destinations by default — or started taking them from a domain list, a public
+// CIDR list, or a rule with no destination at all — this fails. A positive test
+// alone cannot tell "routes what was named" apart from "routes everything".
+func TestPrivateDestinationBypassIsStillTheDefault(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	m := privDstModel(
+		[]model.Ruleset{
+			// A domain list — cannot name an address, whatever it contains.
+			{Name: "sites", Type: "domain", Source: "inline", Entries: []string{"example.com"}},
+			// A PUBLIC address list: readable, named, and none of this plane's business.
+			privDstInlineSet("pub", "198.18.0.0/15"),
+		},
+		[]model.Rule{
+			privDstRule("byname", "sites", "node:awghome"),
+			privDstRule("bypub", "pub", "node:awghome"),
+			// A catch-all: no destination selector at all.
+			{Name: "rest", Enabled: true, Order: 90, Target: "node:awghome"},
+		},
+	)
+	rs, ws := privDstRender(t, m)
+
+	if strings.Contains(rs, "ip daddr {") && !strings.Contains(rs, privDstBypassV4) {
+		t.Fatalf("the private bypass line disappeared:\n%s", rs)
+	}
+	if i := privDstLineIndex(rs, privDstBypassV4); i < 0 {
+		t.Fatalf("private bypass line missing entirely:\n%s", rs)
+	}
+	// Not one divert line may carry an `ip daddr` match: nothing was named.
+	for _, l := range strings.Split(rs, "\n") {
+		if strings.Contains(l, "tproxy") && strings.Contains(l, "ip daddr") {
+			t.Fatalf("a destination-matched divert was emitted for a config that names no private subnet: %q", l)
+		}
+	}
+	// And the forward chain must still ACCEPT LAN-to-LAN, with no drop above it.
+	fwdAccept := privDstLineIndex(rs, `iifname "br-lan" ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16 } accept`)
+	if fwdAccept < 0 {
+		t.Fatalf("forward-chain LAN-to-LAN accept missing:\n%s", rs)
+	}
+	if strings.Contains(rs, "ip daddr { 198.18.0.0/15 }") {
+		t.Fatalf("a PUBLIC named CIDR was rendered into the plane; it needs no override:\n%s", rs)
+	}
+	if w := privDstWarningWith(ws, "does NOT route"); w != "" {
+		t.Fatalf("nothing was named, so nothing should be refused; got: %s", w)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// THE FEATURE: an explicitly named private subnet is diverted, ABOVE the bypass.
+//
+// RED BEFORE: no `ip daddr { 10.10.10.0/24 }` line existed anywhere; the rendered
+// plan was byte-identical to the same config without the rule.
+func TestPrivateDestinationNamedSubnetIsDiverted(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	m := privDstModel(
+		[]model.Ruleset{privDstInlineSet("lab", "10.10.10.0/24")},
+		[]model.Rule{privDstRule("lab", "lab", "node:awghome")},
+	)
+	rs, ws := privDstRender(t, m)
+
+	wantTCP := `iifname "br-lan" meta l4proto tcp ip daddr { 10.10.10.0/24 } update @clients { ip saddr } tproxy ip to :12345 meta mark set 0x2000 accept`
+	wantUDP := `iifname "br-lan" meta l4proto udp ip daddr { 10.10.10.0/24 } update @clients { ip saddr } tproxy ip to :12345 meta mark set 0x2000 accept`
+	tcpAt := privDstLineIndex(rs, wantTCP)
+	udpAt := privDstLineIndex(rs, wantUDP)
+	if tcpAt < 0 || udpAt < 0 {
+		t.Fatalf("named private subnet not diverted (tcp=%d udp=%d):\n%s", tcpAt, udpAt, rs)
+	}
+
+	// ORDER IS THE MECHANISM. Below fib-local (the router keeps its own
+	// addresses), above the private bypass (or the rule still never fires).
+	fibAt := privDstLineIndex(rs, "fib daddr type local accept")
+	bypassAt := privDstLineIndex(rs, privDstBypassV4)
+	if fibAt < 0 || bypassAt < 0 {
+		t.Fatalf("prerouting skeleton missing (fib=%d bypass=%d):\n%s", fibAt, bypassAt, rs)
+	}
+	if !(fibAt < tcpAt && tcpAt < bypassAt) {
+		t.Fatalf("divert is in the wrong place: fib=%d divert=%d bypass=%d\n%s", fibAt, tcpAt, bypassAt, rs)
+	}
+
+	// FAIL-CLOSED MIRROR: with the engine down the divert breaks and the packet
+	// lands in forward, where the LAN-to-LAN accept would wave it out the default
+	// WAN. The drop must sit above that accept.
+	dropAt := privDstLineIndex(rs, `iifname "br-lan" ip daddr { 10.10.10.0/24 } drop`)
+	fwdAcceptAt := privDstLineIndex(rs, `iifname "br-lan" ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16 } accept`)
+	if dropAt < 0 || fwdAcceptAt < 0 {
+		t.Fatalf("forward-chain fail-closed pair missing (drop=%d accept=%d):\n%s", dropAt, fwdAcceptAt, rs)
+	}
+	if dropAt > fwdAcceptAt {
+		t.Fatalf("fail-closed drop is BELOW the LAN-to-LAN accept, so it never runs: drop=%d accept=%d\n%s", dropAt, fwdAcceptAt, rs)
+	}
+	if w := privDstWarningWith(ws, "does NOT route"); w != "" {
+		t.Fatalf("a subnet that was taken must not also be reported as refused: %s", w)
+	}
+}
+
+// The holding plane (engine down, boot armor) must fail closed on the same
+// destination, for the same reason the forward chain does.
+func TestPrivateDestinationHoldPlaneDropsNamedSubnet(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	m := privDstModel(
+		[]model.Ruleset{privDstInlineSet("lab", "10.10.10.0/24")},
+		[]model.Rule{privDstRule("lab", "lab", "node:awghome")},
+	)
+	rs, err := RenderHoldNftAt(m, time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC))
+	if err != nil {
+		t.Fatalf("RenderHoldNftAt: %v", err)
+	}
+	dropAt := privDstLineIndex(rs, `iifname "br-lan" ip daddr { 10.10.10.0/24 } drop`)
+	acceptAt := privDstLineIndex(rs, `iifname "br-lan" ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16 } accept`)
+	if dropAt < 0 || acceptAt < 0 {
+		t.Fatalf("holding plane pair missing (drop=%d accept=%d):\n%s", dropAt, acceptAt, rs)
+	}
+	if dropAt > acceptAt {
+		t.Fatalf("holding plane drop is below the LAN-to-LAN accept: drop=%d accept=%d\n%s", dropAt, acceptAt, rs)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// THE GUARD: a subnet that overlaps one of THIS ROUTER's own networks is refused
+// by name. Taking it would push the router's own LAN into the tunnel.
+func TestPrivateDestinationRefusesRouterOwnNetwork(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	for _, tc := range []struct {
+		name, cidr, owner string
+	}{
+		{"exactly the LAN", "10.67.0.0/24", "10.67.0.0/24"},
+		{"a slice of the LAN", "10.67.0.128/25", "10.67.0.0/24"},
+		{"a block containing the LAN", "10.64.0.0/12", "10.67.0.0/24"},
+		{"the second LAN", "172.20.5.0/24", "172.20.5.0/24"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			m := privDstModel(
+				[]model.Ruleset{privDstInlineSet("oops", tc.cidr)},
+				[]model.Rule{privDstRule("oops", "oops", "node:awghome")},
+			)
+			rs, ws := privDstRender(t, m)
+
+			if strings.Contains(rs, "ip daddr { "+tc.cidr+" }") {
+				t.Fatalf("REFUSED subnet %s was rendered into the plane anyway:\n%s", tc.cidr, rs)
+			}
+			w := privDstWarningWith(ws, "which is a network THIS ROUTER carries")
+			if w == "" {
+				t.Fatalf("self-amputation was refused SILENTLY for %s; warnings=%v", tc.cidr, ws)
+			}
+			for _, must := range []string{`rule "oops"`, tc.cidr, tc.owner, "REFUSED", "does NOT route"} {
+				if !strings.Contains(w, must) {
+					t.Fatalf("refusal does not name %q: %s", must, w)
+				}
+			}
+		})
+	}
+}
+
+// ---------------------------------------------------------------------------
+// AN RFC1918 UPLINK IS NOT ONE OF "OUR NETWORKS", and treating it as one would
+// disable this feature on exactly the routers that need it. The production
+// router's own default gateway is 10.0.0.1; a /8 there makes EVERY private
+// subnet in the world "collide".
+//
+// The split has to be measured in both directions from the SAME instrument, or
+// it proves nothing: a guard that refused everything and a guard that refused
+// nothing would each pass one half of it.
+const privDstCGNATDump = `{"interface":[
+	{"interface":"lan","up":true,"l3_device":"br-lan","ipv4-address":[{"address":"10.67.0.1","mask":24}]},
+	{"interface":"wan0","up":true,"l3_device":"eth1","ipv4-address":[{"address":"10.0.0.53","mask":8}]}
+]}`
+
+const privDstCGNATFirewall = `package firewall
+
+config zone
+	option name 'lan'
+	list network 'lan'
+
+config zone
+	option name 'wan'
+	list network 'wan0'
+	option masq '1'
+
+config forwarding
+	option src 'lan'
+	option dest 'wan'
+`
+
+func TestPrivateDestinationUplinkSubnetIsRoutedAndDisclosed(t *testing.T) {
+	withPrivDstRouterFW(t, privDstCGNATDump, privDstCGNATFirewall)
+
+	// SUBJECT: 10.10.10.0/24 lies inside the uplink's 10.0.0.0/8 and nowhere near
+	// the LAN. It must be routed, and the overlap must be disclosed.
+	rs, ws := privDstRender(t, privDstModel(
+		[]model.Ruleset{privDstInlineSet("lab", "10.10.10.0/24")},
+		[]model.Rule{privDstRule("lab", "lab", "node:awghome")},
+	))
+	if privDstLineIndex(rs, "ip daddr { 10.10.10.0/24 }") < 0 {
+		t.Fatalf("an RFC1918 UPLINK made the whole feature refuse itself:\n%s", rs)
+	}
+	w := privDstWarningWith(ws, "the subnet this router's own uplink")
+	if w == "" {
+		t.Fatalf("the uplink overlap was not disclosed; warnings=%v", ws)
+	}
+	for _, must := range []string{`rule "lab"`, "10.0.0.0/8", `"wan0"`, "IS routed"} {
+		if !strings.Contains(w, must) {
+			t.Fatalf("uplink disclosure does not name %q: %s", must, w)
+		}
+	}
+
+	// CONTROL, same instrument, same router: the LAN still refuses. Without this
+	// the test above would also pass against a guard that had simply stopped
+	// guarding.
+	rs2, ws2 := privDstRender(t, privDstModel(
+		[]model.Ruleset{privDstInlineSet("oops", "10.67.0.0/24")},
+		[]model.Rule{privDstRule("oops", "oops", "node:awghome")},
+	))
+	if strings.Contains(rs2, "ip daddr { 10.67.0.0/24 }") {
+		t.Fatalf("the router's own LAN was routed into the tunnel:\n%s", rs2)
+	}
+	if privDstWarningWith(ws2, "which is a network THIS ROUTER carries") == "" {
+		t.Fatalf("self-amputation was not refused by name; warnings=%v", ws2)
+	}
+}
+
+// A named subnet cannot be taken when this router could not be enumerated: with
+// no inventory there is no way to prove it is not ours, and the doubt falls to
+// the bypass.
+func TestPrivateDestinationRefusesWithoutRouterInventory(t *testing.T) {
+	withPrivDstRouter(t, "") // ubus answers, with nothing usable in it
+
+	m := privDstModel(
+		[]model.Ruleset{privDstInlineSet("lab", "10.10.10.0/24")},
+		[]model.Rule{privDstRule("lab", "lab", "node:awghome")},
+	)
+	rs, ws := privDstRender(t, m)
+
+	if strings.Contains(rs, "ip daddr { 10.10.10.0/24 }") {
+		t.Fatalf("a private subnet was taken on a guess, with no router inventory:\n%s", rs)
+	}
+	w := privDstWarningWith(ws, "network inventory could not be read")
+	if w == "" {
+		t.Fatalf("refused for lack of an inventory, and said nothing; warnings=%v", ws)
+	}
+	if !strings.Contains(w, "10.10.10.0/24") || !strings.Contains(w, "does NOT route") {
+		t.Fatalf("inventory refusal does not name the subnet or the consequence: %s", w)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// Loopback / link-local / multicast / broadcast are never taken, whatever a list
+// says — and saying so is part of the contract.
+func TestPrivateDestinationNeverTakesReservedSpace(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	for _, cidr := range []string{"127.0.0.0/8", "169.254.0.0/16", "224.0.0.0/4", "255.255.255.255"} {
+		t.Run(cidr, func(t *testing.T) {
+			m := privDstModel(
+				[]model.Ruleset{privDstInlineSet("bad", cidr)},
+				[]model.Rule{privDstRule("bad", "bad", "node:awghome")},
+			)
+			rs, ws := privDstRender(t, m)
+			for _, l := range strings.Split(rs, "\n") {
+				if strings.Contains(l, "tproxy") && strings.Contains(l, "ip daddr") {
+					t.Fatalf("reserved space %s produced a divert: %q", cidr, l)
+				}
+			}
+			if privDstWarningWith(ws, "loopback, link-local, multicast or broadcast space") == "" {
+				t.Fatalf("reserved space %s was ignored silently; warnings=%v", cidr, ws)
+			}
+		})
+	}
+}
+
+// A catch-all does not acquire private space, and says so rather than looking
+// like it worked. This is the owner's explicit requirement: "при 0.0.0.0 не
+// роутился приватный сабнет".
+func TestPrivateDestinationCatchAllDoesNotTakePrivate(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	for _, cidr := range []string{"0.0.0.0/0", "10.0.0.0/7"} {
+		t.Run(cidr, func(t *testing.T) {
+			m := privDstModel(
+				[]model.Ruleset{privDstInlineSet("all", cidr)},
+				[]model.Rule{privDstRule("all", "all", "node:awghome")},
+			)
+			rs, ws := privDstRender(t, m)
+			for _, l := range strings.Split(rs, "\n") {
+				if strings.Contains(l, "tproxy") && strings.Contains(l, "ip daddr") {
+					t.Fatalf("catch-all %s took private space: %q", cidr, l)
+				}
+			}
+			if privDstWarningWith(ws, "covers private address space without naming it") == "" {
+				t.Fatalf("catch-all %s silently failed to route private space; warnings=%v", cidr, ws)
+			}
+		})
+	}
+}
+
+// `direct` is what the bypass already does. Diverting a private destination into
+// the engine to reach the same verdict would be strictly worse — the engine's
+// direct outbound follows the DEFAULT route — so it is not an override.
+func TestPrivateDestinationDirectTargetIsNotAnOverride(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	for _, target := range []string{"direct", ""} {
+		t.Run("target="+target, func(t *testing.T) {
+			m := privDstModel(
+				[]model.Ruleset{privDstInlineSet("lab", "10.10.10.0/24")},
+				[]model.Rule{privDstRule("lab", "lab", target)},
+			)
+			rs, _ := privDstRender(t, m)
+			if strings.Contains(rs, "ip daddr { 10.10.10.0/24 }") {
+				t.Fatalf("target %q diverted a private destination into the engine:\n%s", target, rs)
+			}
+		})
+	}
+}
+
+// A DISABLED rule names nothing: the plane must be identical to the one with no
+// rule at all.
+func TestPrivateDestinationDisabledRuleTakesNothing(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	off := privDstRule("lab", "lab", "node:awghome")
+	off.Enabled = false
+	with, _ := privDstRender(t, privDstModel(
+		[]model.Ruleset{privDstInlineSet("lab", "10.10.10.0/24")},
+		[]model.Rule{off},
+	))
+	without, _ := privDstRender(t, privDstModel(
+		[]model.Ruleset{privDstInlineSet("lab", "10.10.10.0/24")}, nil,
+	))
+	if with != without {
+		t.Fatalf("a disabled rule changed the plane:\n--- with ---\n%s\n--- without ---\n%s", with, without)
+	}
+}
+
+// nft refuses an anonymous interval set holding overlapping members, and the
+// failure takes the WHOLE table down. Overlapping named prefixes must collapse.
+func TestPrivateDestinationSetIsDisjointAndOrdered(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	m := privDstModel(
+		[]model.Ruleset{privDstInlineSet("lab",
+			"192.168.5.0/24", "192.168.0.0/16", "10.10.10.0/24", "10.10.10.0/24", "192.168.9.9")},
+		[]model.Rule{privDstRule("lab", "lab", "node:awghome")},
+	)
+	rs, _ := privDstRender(t, m)
+	want := "ip daddr { 10.10.10.0/24, 192.168.0.0/16 }"
+	if !strings.Contains(rs, want) {
+		t.Fatalf("expected the collapsed, ordered set %q:\n%s", want, rs)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// THE BLIND SPOT, DISCLOSED. A list this plan never reads may hold private
+// addresses; the plan cannot know, and must not pretend it does.
+func TestPrivateDestinationOpaqueListIsDisclosed(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	for _, tc := range []struct {
+		name string
+		rs   model.Ruleset
+		want bool
+	}{
+		{"downloaded ip list", model.Ruleset{Name: "feed", Type: "ipcidr", Source: "url", URL: "https://x/y.srs"}, true},
+		{"local ip list file", model.Ruleset{Name: "feed", Type: "ipcidr", Source: "file", Path: "/etc/x.srs"}, true},
+		{"geoip PRIVATE category", model.Ruleset{Name: "feed", Source: "geoip", Categories: []string{"private"}}, true},
+		{"geoip unknown category", model.Ruleset{Name: "feed", Source: "geoip", Categories: []string{"nowhere"}}, true},
+		{"geoip country codes", model.Ruleset{Name: "feed", Source: "geoip", Categories: []string{"ru", "by"}}, false},
+		{"geosite", model.Ruleset{Name: "feed", Source: "geosite", Categories: []string{"youtube"}}, false},
+		{"downloaded DOMAIN list", model.Ruleset{Name: "feed", Type: "domain", Source: "url", URL: "https://x/y.txt"}, false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			m := privDstModel([]model.Ruleset{tc.rs},
+				[]model.Rule{privDstRule("r", "feed", "node:awghome")})
+			_, ws := privDstRender(t, m)
+			got := privDstWarningWith(ws, "address list whose contents this data plane never sees") != ""
+			if got != tc.want {
+				t.Fatalf("disclosure=%v, want %v; warnings=%v", got, tc.want, ws)
+			}
+		})
+	}
+}
+
+// The disclosure is a statement about a blind spot, not about a leak: it must
+// carry the marker apply/warnings.go grades as `warning`, and must not carry the
+// phrases that can never be downgraded.
+func TestPrivateDestinationDisclosureWordingIsGradable(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	m := privDstModel(
+		[]model.Ruleset{{Name: "feed", Type: "ipcidr", Source: "url", URL: "https://x/y.srs"}},
+		[]model.Rule{privDstRule("r", "feed", "node:awghome")},
+	)
+	_, ws := privDstRender(t, m)
+	w := privDstWarningWith(ws, "address list whose contents this data plane never sees")
+	if w == "" {
+		t.Fatalf("no disclosure at all; warnings=%v", ws)
+	}
+	if !strings.HasSuffix(w, "If it contains only public addresses, nothing to do.") {
+		t.Fatalf("disclosure lost the marker apply/warnings.go keys on: %s", w)
+	}
+	for _, never := range []string{"IN THE CLEAR", "CUT OFF"} {
+		if strings.Contains(w, never) {
+			t.Fatalf("disclosure contains the never-downgrade phrase %q, so it can never be graded a warning: %s", never, w)
+		}
+	}
+	if !strings.HasPrefix(w, `ruleset "feed": `) {
+		t.Fatalf("disclosure is not attributable to the rule-set it is about: %s", w)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// The vocabulary of `type` is ONE definition. This is the check that a future
+// edit adding "cidr" to one side and not the other cannot pass.
+func TestPrivateDestinationIPCIDRVocabularyIsClosed(t *testing.T) {
+	for _, tc := range []struct {
+		in   string
+		want bool
+	}{
+		{"ipcidr", true}, {"ip_cidr", true}, {"ip", true},
+		{" IPCIDR ", true}, {"IP_CIDR", true},
+		{"", false}, {"domain", false}, {"cidr", false}, {"ipv4", false}, {"ips", false},
+	} {
+		if got := IsIPCIDRRulesetType(tc.in); got != tc.want {
+			t.Fatalf("IsIPCIDRRulesetType(%q) = %v, want %v", tc.in, got, tc.want)
+		}
+	}
+}
+
+// v6 private space is NOT taken, because the router-network guard has no v6
+// inventory to check it against — and that is said, not left silent.
+func TestPrivateDestinationIPv6PrivateIsReportedNotTaken(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	m := privDstModel(
+		[]model.Ruleset{privDstInlineSet("lab", "fd00:dead::/48")},
+		[]model.Rule{privDstRule("lab", "lab", "node:awghome")},
+	)
+	m.Globals.IPv6 = true
+	rs, ws := privDstRender(t, m)
+	if strings.Contains(rs, "fd00:dead::/48") {
+		t.Fatalf("a v6 ULA was routed without any way to check it against this router's own v6:\n%s", rs)
+	}
+	w := privDstWarningWith(ws, "IPv6 private space")
+	if w == "" {
+		t.Fatalf("named v6 private space was ignored silently; warnings=%v", ws)
+	}
+	if !strings.Contains(w, "does NOT route") {
+		t.Fatalf("v6 report does not state the consequence: %s", w)
+	}
+}
+
+// ---------------------------------------------------------------------------
+// PING. The owner's acceptance criterion is `ping 10.10.10.10` from a LAN client,
+// and a TPROXY divert carries TCP and UDP only — the kernel needs a socket to
+// hand the packet to and ICMP has not got one. The L3 ingress is the existing
+// mechanism that carries an echo into the engine; the defect was that its
+// marking line sits BELOW the private bypass and so never saw these
+// destinations.
+//
+// RED BEFORE: with the tproxy divert alone, TCP to 10.10.10.10 works and ping
+// reports 100% loss — which reads as a broken route, not as a protocol boundary.
+func TestPrivateDestinationICMPIsRoutedWhenL3IsOn(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	m := privDstModel(
+		[]model.Ruleset{privDstInlineSet("lab", "10.10.10.0/24")},
+		[]model.Rule{privDstRule("lab", "lab", "node:awghome")},
+	)
+	m.Globals.L3Tunnel = true
+	rs, ws := privDstRender(t, m)
+
+	mark := L3Mark(m.Globals)
+	want := fmt.Sprintf(`iifname "br-lan" ip daddr { 10.10.10.0/24 } ip protocol icmp meta mark set 0x%x accept`, mark)
+	icmpAt := privDstLineIndex(rs, want)
+	bypassAt := privDstLineIndex(rs, privDstBypassV4)
+	if icmpAt < 0 {
+		t.Fatalf("no ICMP route into the engine for a named private subnet, so ping cannot work:\n%s", rs)
+	}
+	if icmpAt > bypassAt {
+		t.Fatalf("the ICMP mark is BELOW the private bypass, where it can never run: icmp=%d bypass=%d\n%s",
+			icmpAt, bypassAt, rs)
+	}
+
+	// The fail-closed drop must not eat the leg prerouting just routed. Unlike
+	// the tproxy legs (delivered locally, never seen in forward), the LAN->TUN
+	// ICMP leg DOES traverse the forward chain.
+	wantDrop := fmt.Sprintf(`iifname "br-lan" ip daddr { 10.10.10.0/24 } meta mark != 0x%x drop`, mark)
+	if privDstLineIndex(rs, wantDrop) < 0 {
+		t.Fatalf("the forward-chain drop does not exclude the L3-marked leg, so ping is dropped by our own "+
+			"kill switch; wanted %q in:\n%s", wantDrop, rs)
+	}
+	if w := privDstWarningWith(ws, "`l3_tunnel` is OFF"); w != "" {
+		t.Fatalf("l3_tunnel is ON; the ping note must not fire: %s", w)
+	}
+}
+
+// With l3_tunnel OFF the ICMP line cannot be emitted — and the operator has to be
+// told, or this defect simply reappears one protocol down.
+func TestPrivateDestinationICMPGapIsNamedWhenL3IsOff(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+
+	m := privDstModel(
+		[]model.Ruleset{privDstInlineSet("lab", "10.10.10.0/24")},
+		[]model.Rule{privDstRule("lab", "lab", "node:awghome")},
+	)
+	m.Globals.L3Tunnel = false
+	rs, ws := privDstRender(t, m)
+
+	if strings.Contains(rs, "ip protocol icmp") {
+		t.Fatalf("an ICMP route was emitted with l3_tunnel off, where nothing receives it:\n%s", rs)
+	}
+	// The TCP/UDP half must still be there: this is a protocol gap, not a refusal.
+	if privDstLineIndex(rs, `meta l4proto tcp ip daddr { 10.10.10.0/24 }`) < 0 {
+		t.Fatalf("the TCP divert disappeared along with the ICMP one:\n%s", rs)
+	}
+	w := privDstWarningWith(ws, "`l3_tunnel` is OFF")
+	if w == "" {
+		t.Fatalf("ping does not reach a routed private subnet and nothing said so; warnings=%v", ws)
+	}
+	for _, must := range []string{`rule "lab"`, "10.10.10.0/24", "a ping into 10.10.10.0/24", "l3_tunnel '1'"} {
+		if !strings.Contains(w, must) {
+			t.Fatalf("the ping note does not name %q: %s", must, w)
+		}
+	}
+}
+
+// ---------------------------------------------------------------------------
+// BYTE-FOR-BYTE: a configuration that names no private subnet must render EXACTLY
+// the text it rendered before this mechanism existed. The applier's idempotence
+// check compares the rendered text, so one stray byte turns every reconcile into
+// a live reload of the data plane.
+//
+// The golden below was produced by the pre-change renderer (git HEAD before this
+// commit) for privDstUntouchedModel; see the commit message for the command.
+func privDstUntouchedModel() *model.Model {
+	m := privDstModel(
+		[]model.Ruleset{
+			{Name: "sites", Type: "domain", Source: "inline", Entries: []string{"example.com"}},
+			{Name: "geo", Source: "geoip", Categories: []string{"ru"}},
+			privDstInlineSet("pub", "198.18.0.0/15"),
+		},
+		[]model.Rule{
+			{Name: "kids", Enabled: true, Order: 10, Src: []string{"192.168.1.0/24"}, Target: "block"},
+			privDstRule("byname", "sites", "node:awghome"),
+			privDstRule("bygeo", "geo", "node:awghome"),
+			privDstRule("bypub", "pub", "node:awghome"),
+		},
+	)
+	m.Globals.DNSIntercept = true
+	return m
+}
+
+// The same fixture with the L3 ingress ON — the shape the router actually ships
+// (model defaults seed l3_tunnel true). It renders the prerouting ICMP mark, the
+// L3 mark bypass and the forward-chain TUN accepts, so it pins the text AROUND
+// every line this change inserts, not just the plain one.
+func privDstUntouchedL3Model() *model.Model {
+	m := privDstUntouchedModel()
+	m.Globals.L3Tunnel = true
+	return m
+}
+
+func TestPrivateDestinationRenderIsUnchangedWithL3Ingress(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+	got, _ := privDstRender(t, privDstUntouchedL3Model())
+	if got != privDstGoldenL3 {
+		t.Fatalf("the L3 plane changed for a config that names no private subnet.\n--- got ---\n%s\n--- want ---\n%s",
+			got, privDstGoldenL3)
+	}
+	for _, must := range []string{"ip protocol icmp meta mark set", `oifname "shater-l3`} {
+		if !strings.Contains(got, must) {
+			t.Fatalf("the L3 golden fixture no longer renders the L3 plane (missing %q)", must)
+		}
+	}
+}
+
+func TestPrivateDestinationRenderIsUnchangedWithoutNamedSubnets(t *testing.T) {
+	withPrivDstRouter(t, privDstIfaceDump)
+	got, _ := privDstRender(t, privDstUntouchedModel())
+	if got != privDstGoldenUntouched {
+		t.Fatalf("the plane changed for a config that names no private subnet.\n--- got ---\n%s\n--- want ---\n%s",
+			got, privDstGoldenUntouched)
+	}
+	// A golden is only as good as its liveness: if the fixture ever stopped
+	// producing a real plane, the comparison would still pass on two empty
+	// strings.
+	for _, must := range []string{"table inet shater {", "tproxy ip to :12345", privDstBypassV4} {
+		if !strings.Contains(got, must) {
+			t.Fatalf("the golden fixture no longer renders a real plane (missing %q)", must)
+		}
+	}
+}
+
+// REGENERATING THE TWO GOLDENS, when the ruleset skeleton changes on purpose.
+//
+// NOT from this package's own output — a golden captured from the code it is
+// meant to police proves only that the code is self-consistent. Capture it from
+// a worktree at the commit BEFORE the change:
+//
+//	git worktree add /tmp/head-wt HEAD --detach
+//	# copy privDstUntouchedModel + privDstExec into a throwaway _test.go there,
+//	# render it with RenderNftPlanAt and write the bytes to a file
+//	cd /tmp/head-wt && go test ./shater/netplane -run <that test>
+//	git worktree remove /tmp/head-wt
+//
+// privDstGoldenRaw is the EXACT text the PRE-CHANGE renderer produced for
+// privDstUntouchedModel. It was captured from a git worktree at the parent commit
+// with a throwaway dump test — not by copying this package's own output, because a
+// golden taken from the code it is meant to police proves nothing. The leading
+// newline is an artefact of the raw literal and is stripped below.
+const privDstGoldenRaw = `
+#!/usr/sbin/nft -f
+table inet shater
+delete table inet shater
+table inet shater {
+	set clients { type ipv4_addr; flags dynamic; counter; }
+	counter c_rule_kids { }
+	counter c_in_lan { }
+	chain prerouting {
+		type filter hook prerouting priority mangle; policy accept;
+		meta mark 0xff accept
+		iifname "br-lan" meta l4proto tcp th dport 53 update @clients { ip saddr } tproxy ip to :12345 meta mark set 0x2000 accept
+		iifname "br-lan" meta l4proto udp th dport 53 update @clients { ip saddr } tproxy ip to :12345 meta mark set 0x2000 accept
+		fib daddr type local accept
+		ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16, 224.0.0.0/4, 255.255.255.255 } accept
+		ip6 daddr { ::1, fc00::/7, fe80::/10, ff00::/8 } accept
+		iifname "br-lan" meta l4proto { tcp, udp } th dport 853 accept
+		iifname "br-lan" meta l4proto tcp ip saddr 192.168.1.0/24 update @clients { ip saddr } counter name "c_rule_kids" tproxy ip to :12345 meta mark set 0x2000 accept
+		iifname "br-lan" meta l4proto udp ip saddr 192.168.1.0/24 update @clients { ip saddr } counter name "c_rule_kids" tproxy ip to :12345 meta mark set 0x2000 accept
+		iifname "br-lan" meta l4proto tcp update @clients { ip saddr } counter name "c_in_lan" tproxy ip to :12345 meta mark set 0x2000 accept
+		iifname "br-lan" meta l4proto udp update @clients { ip saddr } counter name "c_in_lan" tproxy ip to :12345 meta mark set 0x2000 accept
+	}
+	chain forward {
+		type filter hook forward priority filter; policy accept;
+		iifname "br-lan" meta l4proto { tcp, udp } th dport 853 reject
+		meta mark 0xff accept
+		iifname "br-lan" ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16 } accept
+		iifname "br-lan" icmpv6 type { nd-router-solicit, nd-router-advert, nd-neighbor-solicit, nd-neighbor-advert } accept
+		iifname "br-lan" ip6 daddr fe80::/10 accept
+		iifname "br-lan" ip6 daddr ff00::/8 accept
+		iifname "br-lan" meta nfproto ipv4 drop
+		iifname "br-lan" meta nfproto ipv6 drop
+	}
+}
+`
+
+var privDstGoldenUntouched = strings.TrimPrefix(privDstGoldenRaw, "\n")
+
+// privDstGoldenL3Raw is privDstGoldenRaw's sibling for the L3-ingress shape,
+// captured the same way from the same pre-change worktree.
+const privDstGoldenL3Raw = `
+#!/usr/sbin/nft -f
+table inet shater
+delete table inet shater
+table inet shater {
+	set clients { type ipv4_addr; flags dynamic; counter; }
+	counter c_rule_kids { }
+	counter c_in_lan { }
+	chain prerouting {
+		type filter hook prerouting priority mangle; policy accept;
+		meta mark 0xff accept
+		meta mark 0x2080 accept
+		iifname "br-lan" meta l4proto tcp th dport 53 update @clients { ip saddr } tproxy ip to :12345 meta mark set 0x2000 accept
+		iifname "br-lan" meta l4proto udp th dport 53 update @clients { ip saddr } tproxy ip to :12345 meta mark set 0x2000 accept
+		fib daddr type local accept
+		ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16, 224.0.0.0/4, 255.255.255.255 } accept
+		ip6 daddr { ::1, fc00::/7, fe80::/10, ff00::/8 } accept
+		iifname "br-lan" ip protocol icmp meta mark set 0x2080 accept
+		iifname "br-lan" meta l4proto { tcp, udp } th dport 853 accept
+		iifname "br-lan" meta l4proto tcp ip saddr 192.168.1.0/24 update @clients { ip saddr } counter name "c_rule_kids" tproxy ip to :12345 meta mark set 0x2000 accept
+		iifname "br-lan" meta l4proto udp ip saddr 192.168.1.0/24 update @clients { ip saddr } counter name "c_rule_kids" tproxy ip to :12345 meta mark set 0x2000 accept
+		iifname "br-lan" meta l4proto tcp update @clients { ip saddr } counter name "c_in_lan" tproxy ip to :12345 meta mark set 0x2000 accept
+		iifname "br-lan" meta l4proto udp update @clients { ip saddr } counter name "c_in_lan" tproxy ip to :12345 meta mark set 0x2000 accept
+	}
+	chain forward {
+		type filter hook forward priority filter; policy accept;
+		iifname "br-lan" meta l4proto { tcp, udp } th dport 853 reject
+		meta mark 0xff accept
+		iifname "br-lan" ip daddr { 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16 } accept
+		iifname "br-lan" icmpv6 type { nd-router-solicit, nd-router-advert, nd-neighbor-solicit, nd-neighbor-advert } accept
+		iifname "br-lan" ip6 daddr fe80::/10 accept
+		iifname "br-lan" ip6 daddr ff00::/8 accept
+		oifname "shater-l3*" accept
+		iifname "shater-l3*" accept
+		iifname "br-lan" meta nfproto ipv4 drop
+		iifname "br-lan" meta nfproto ipv6 drop
+	}
+}
+`
+
+var privDstGoldenL3 = strings.TrimPrefix(privDstGoldenL3Raw, "\n")