fix(l3): guard the ICMP honest-drop at PreMatch, not inside the walk

The drop that keeps a ping from reading as tunnelled lived in
preMatchFlow, overriding the pre-declared continueResult. That covered
every exit of THAT function and none of the walk above it: the
prepareMatchMetadata error return (which arrived later, with the shared
metadata refactor), the sniff bail-outs, and the default: arm of the
rule-action switch all returned PreMatchContinue on their own.
adapter.JudgeFlow maps Continue to tun.ActionAccept, and sing-tun answers
Accept by rewriting Echo into EchoReply itself -- the exact forgery this
delta exists to remove. Narrow paths, but paths.

PreMatch is now a funnel over the renamed preMatch walk, so the guard
sits on the single return value and cannot be outgrown by a new exit.
PreMatchBypass joins the drop: sing-tun implements ActionBypass on the
nfqueue plane only, so on the TUN path it lands in the same default: arm
as Accept and forges too.

Every ICMP case has an explicit TCP/UDP twin; the JudgeFlow mapping
table is pinned outright, including the one fix that must NOT be made
there -- refusing ActionFlow for a port whose address is not unspecified
would drop every ping through WireGuard/AWG, because the forward
dispatcher and the ICMP forwarder share that function with identical
arguments and only the latter needs an unspecified address.

That leaves a real hole open, now named in D25 rather than papered over:
a FRAGMENTED echo to a WireGuard/AWG outbound is still answered by the
router. The dispatcher returns before asking for a verdict at all when
the packet is a fragment, and the reassembled packet reaches the ICMP
forwarder, whose installFlow demands the unspecified address a WireGuard
endpoint never has. The two fixes that would close it both live outside
pre-match and are written down; the Consequence paragraph is scoped
until one lands.

The stack comment in generate/inbound.go repeated the "only gvisor
really forwards ICMP" argument that D25 itself retracts -- both stacks
run the same ForwardDispatcher first. Brought in line.

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

adapter/judgeflow_icmp_lx_test.go

@@ -0,0 +1,145 @@
+// lx:begin l3-honest-drop
+package adapter
+
+import (
+	"net/netip"
+	"testing"
+
+	"github.com/sagernet/sing-tun"
+	"github.com/sagernet/sing-tun/gtcpip/header"
+
+	"github.com/stretchr/testify/require"
+)
+
+// judgeFlowRouter answers PreMatch with a canned verdict; JudgeFlow reads
+// nothing else off the Router.
+type judgeFlowRouter struct {
+	Router
+	result PreMatchResult
+}
+
+func (r *judgeFlowRouter) PreMatch(InboundContext, []byte) PreMatchResult { return r.result }
+
+// judgeFlowPort is the tun.Port half of a FlowOutbound. inet4 is what
+// PortAddresses reports for IPv4 — the one field the two ICMP consumers in
+// sing-tun disagree about (see the comment on
+// TestJudgeFlowICMPToBoundPortStaysAFlow).
+type judgeFlowPort struct {
+	Outbound
+	inet4 netip.Addr
+}
+
+func (o *judgeFlowPort) Tag() string  { return "wg-out" }
+func (o *judgeFlowPort) Type() string { return "wireguard" }
+func (o *judgeFlowPort) PortAddresses() (netip.Addr, netip.Addr) {
+	return o.inet4, netip.Addr{}
+}
+func (o *judgeFlowPort) PortMTU() uint32                     { return 1420 }
+func (o *judgeFlowPort) AttachReturn(tun.Return) error       { return nil }
+func (o *judgeFlowPort) DetachReturn(tun.Return) error       { return nil }
+func (o *judgeFlowPort) WritePackets(packets [][]byte) error { return nil }
+
+// judgeFlowNonPort is a FlowOutbound-shaped result that is NOT a tun.Port — the
+// interface drift the second line of defense in JudgeFlow exists for.
+type judgeFlowNonPort struct {
+	Outbound
+}
+
+func (o *judgeFlowNonPort) Tag() string  { return "drifted" }
+func (o *judgeFlowNonPort) Type() string { return "drifted" }
+
+func judgeFlow(t *testing.T, protocol uint8, result PreMatchResult) tun.FlowVerdict {
+	t.Helper()
+	return JudgeFlow(
+		&judgeFlowRouter{result: result},
+		"l3-in", "tun", protocol,
+		netip.MustParseAddrPort("192.168.1.2:1234"),
+		netip.MustParseAddrPort("1.1.1.1:1234"),
+		nil,
+	)
+}
+
+const (
+	judgeFlowICMP = uint8(header.ICMPv4ProtocolNumber)
+	judgeFlowTCP  = uint8(header.TCPProtocolNumber)
+)
+
+// TestJudgeFlowICMPToBoundPortStaysAFlow is the guard on the ONE fix that must
+// not be made here.
+//
+// sing-tun has two ICMP consumers with different requirements on the port:
+//
+//   - ForwardDispatcher.createFlow (flow_dispatch.go) needs only a VALID port
+//     address — it NATs the echo identifier and rewrites the source to that
+//     address. This is the path every unfragmented LAN ping takes, and it is
+//     what makes ping-through-WireGuard/AWG work at all.
+//   - ICMPForwarder.installFlow (stack_gvisor_icmp.go) additionally requires the
+//     address to be UNSPECIFIED, because it writes the packet to the port
+//     unmodified. A WireGuard endpoint reports its concrete interface address
+//     (transport/wireguard/port.go), so installFlow declines and HandlePacket
+//     falls through to forging the echo reply.
+//
+// The tempting fix — "for ICMP, refuse ActionFlow when PortAddresses() is not
+// unspecified, so the verdict becomes a drop and the forgery is unreachable" —
+// is applied HERE, in the one function both consumers share, with byte-identical
+// arguments from either. It would therefore kill the working path too: every
+// ping through WireGuard/AWG, fragmented or not, would drop, and l3_tunnel would
+// carry nothing but `direct`. Keep this test failing loudly if anyone tries.
+func TestJudgeFlowICMPToBoundPortStaysAFlow(t *testing.T) {
+	t.Parallel()
+	port := &judgeFlowPort{inet4: netip.MustParseAddr("10.2.0.2")}
+	verdict := judgeFlow(t, judgeFlowICMP, PreMatchResult{Action: PreMatchFlow, Outbound: port})
+	require.Equal(t, tun.ActionFlow, verdict.Action,
+		"ICMP to a WireGuard/AWG endpoint must stay a flow: the forward dispatcher NATs it by echo identifier and this is the whole point of l3_tunnel")
+	require.Same(t, tun.Port(port), verdict.Port)
+}
+
+// The `direct` shape: an unspecified port address. Both consumers accept it.
+func TestJudgeFlowICMPToUnspecifiedPortStaysAFlow(t *testing.T) {
+	t.Parallel()
+	port := &judgeFlowPort{inet4: netip.IPv4Unspecified()}
+	verdict := judgeFlow(t, judgeFlowICMP, PreMatchResult{Action: PreMatchFlow, Outbound: port})
+	require.Equal(t, tun.ActionFlow, verdict.Action)
+	require.Same(t, tun.Port(port), verdict.Port)
+}
+
+// PreMatchDrop is the honest verdict and must arrive as ActionDrop: it is the
+// only value (besides Reject) that stops ICMPForwarder.HandlePacket before the
+// Echo -> EchoReply rewrite.
+func TestJudgeFlowICMPDropReachesTheStackAsDrop(t *testing.T) {
+	t.Parallel()
+	verdict := judgeFlow(t, judgeFlowICMP, PreMatchResult{Action: PreMatchDrop})
+	require.Equal(t, tun.ActionDrop, verdict.Action)
+}
+
+// The second line of defense: a PreMatchFlow whose outbound is not a tun.Port
+// must not degrade ICMP to ActionAccept, because Accept is the forged reply.
+func TestJudgeFlowICMPNonPortOutboundDrops(t *testing.T) {
+	t.Parallel()
+	verdict := judgeFlow(t, judgeFlowICMP, PreMatchResult{Action: PreMatchFlow, Outbound: &judgeFlowNonPort{}})
+	require.Equal(t, tun.ActionDrop, verdict.Action,
+		"FlowOutbound and tun.Port are distinct interfaces; a drift between them must not silently re-enable the echo forger")
+}
+
+func TestJudgeFlowTCPNonPortOutboundAccepts(t *testing.T) {
+	t.Parallel()
+	verdict := judgeFlow(t, judgeFlowTCP, PreMatchResult{Action: PreMatchFlow, Outbound: &judgeFlowNonPort{}})
+	require.Equal(t, tun.ActionAccept, verdict.Action,
+		"for TCP, falling back to Accept is upstream behaviour and must stay untouched")
+}
+
+// TCP keeps every mapping it had, including the Continue -> Accept default that
+// is a forgery only for ICMP.
+func TestJudgeFlowTCPContinueStaysAccept(t *testing.T) {
+	t.Parallel()
+	verdict := judgeFlow(t, judgeFlowTCP, PreMatchResult{Action: PreMatchContinue})
+	require.Equal(t, tun.ActionAccept, verdict.Action)
+}
+
+func TestJudgeFlowTCPBypassStaysBypass(t *testing.T) {
+	t.Parallel()
+	verdict := judgeFlow(t, judgeFlowTCP, PreMatchResult{Action: PreMatchBypass})
+	require.Equal(t, tun.ActionBypass, verdict.Action)
+}
+
+// lx:end l3-honest-drop

docs-lx/lx-changelog.md

@@ -21,23 +21,39 @@ whenever the flow judgment comes back Accept (`stack_gvisor_icmp.go`) — so a
 ping routed to vless/vmess/… would read as a working tunnel while the packet
 never left the router.
 
-* **`route/route.go` (`preMatchFlow`)** — one hunk: for `N.NetworkICMP` the
-  pre-declared `continueResult` is overridden to `PreMatchDrop`, which covers
-  every `return continueResult` exit point of the function at once (outbound
-  not loaded, dead group selection, ICMP absent from `outbound.Network()` —
-  the vless/vmess/… case — and non-`FlowOutbound` alike).
+* **`route/route.go` (`PreMatch`)** — the pre-match walk was renamed to
+  `preMatch` and the exported `PreMatch` became a thin FUNNEL that rewrites
+  `PreMatchContinue` and `PreMatchBypass` to `PreMatchDrop` for
+  `N.NetworkICMP`. An earlier version overrode `continueResult` inside
+  `preMatchFlow` instead; that covered only the exits reaching that function and
+  left three of the walk's own exits forging — the `prepareMatchMetadata` error
+  return, the sniff bail-outs, and the `default:` arm of the rule-action switch
+  (every action pre-match has no arm for: `hijack-dns`, `direct`, …). A guard on
+  the single return value cannot be outgrown by a new exit. `PreMatchBypass` is
+  folded in because sing-tun implements `ActionBypass` on the nfqueue plane only
+  — on the TUN path it lands in the same `default:` arm as Accept, i.e. forges.
 * **`adapter/router.go` (`JudgeFlow`, the `!isPort` branch)** — ICMP returns
   `ActionDrop` where it fell through to `ActionAccept`. Second line of defense:
   `adapter.FlowOutbound` and `tun.Port` are distinct interfaces, and a drift
   between them must not quietly re-enable the forged reply.
 * **TCP/UDP behaviour is unchanged** — `PreMatchContinue` still means "take the
-  ordinary connection route" for both, and the `!isPort` fallthrough still
-  returns `ActionAccept` for them; pinned by `route/prematch_icmp_lx_test.go`
-  (itself inside the marker).
+  ordinary connection route" for both, `PreMatchBypass` still means bypass, and
+  the `!isPort` fallthrough still returns `ActionAccept` for them; pinned by
+  `route/prematch_icmp_lx_test.go` and `adapter/judgeflow_icmp_lx_test.go`
+  (both inside the marker), each ICMP case having an explicit TCP/UDP twin.
+* **NOT covered: a FRAGMENTED echo to a WireGuard/AWG outbound is still
+  forged** — sing-tun's `ForwardDispatcher.Dispatch` returns before asking for a
+  verdict at all when `parsed.fragment`, and the reassembled packet reaches
+  `ICMPForwarder.HandlePacket`, whose `installFlow` demands an UNSPECIFIED port
+  address that a WireGuard endpoint never has. Fixing it inside `JudgeFlow`
+  is NOT possible — both consumers call it with identical arguments and the
+  working path needs the concrete address. Full chain, the two viable fixes and
+  the trap are in `docs-shater/DECISIONS.md` D25, "KNOWN HOLE".
 * **Rebase cost: two small marked blocks** (`lx:begin/end l3-honest-drop`, a
-  3-line conditional in `route/route.go` and one branch body in
-  `adapter/router.go`) plus the self-contained test file — carried across an
-  upstream rebase by eye.
+  wrapper function in `route/route.go` and one branch body in
+  `adapter/router.go`) plus the two self-contained test files — carried across
+  an upstream rebase by eye. Note that `PreMatch`'s own body now lives in
+  `preMatch`, so an upstream change to the walk applies to that function.
 
 **Fork-layer + control-plane rework of proxy health** — ships with `shaterd`
 (the shater router daemon), not as an lx release tag; recorded here because the

docs-shater/DECISIONS.md

@@ -883,15 +883,97 @@ the echo ITSELF (Echo→EchoReply + address swap) whenever the flow judgment com
 back Accept (`stack_gvisor_icmp.go:120`), and upstream maps "no flow route" to
 exactly that Accept — so a ping routed to vless would read as tunnelled while
 the packet died on the router. Two small marked hunks make the truth observable:
-`route/route.go` (`preMatchFlow`) overrides the pre-declared `continueResult` to
-`PreMatchDrop` for `N.NetworkICMP`, covering every exit point of the function at
-once, and `adapter/router.go` (`JudgeFlow`, the `!isPort` branch) returns
-`ActionDrop` for ICMP where it fell through to `ActionAccept` — the second line
-of defense, because `FlowOutbound` and `tun.Port` are distinct interfaces and a
-drift between them must not quietly re-enable the forger. TCP/UDP verdicts are
-byte-identical; `route/prematch_icmp_lx_test.go` pins both directions. The
-operator-facing text says the same out loud (`shater/apply/warnings.go`):
-proxy-routed addresses "cannot be pinged at all — deliberately".
+`route/route.go` wraps the whole pre-match walk — the walk itself became
+`preMatch`, and the exported `PreMatch` is now a FUNNEL that rewrites
+`PreMatchContinue` and `PreMatchBypass` to `PreMatchDrop` for `N.NetworkICMP` —
+and `adapter/router.go` (`JudgeFlow`, the `!isPort` branch) returns `ActionDrop`
+for ICMP where it fell through to `ActionAccept` — the second line of defense,
+because `FlowOutbound` and `tun.Port` are distinct interfaces and a drift
+between them must not quietly re-enable the forger. TCP/UDP verdicts are
+byte-identical; `route/prematch_icmp_lx_test.go` and
+`adapter/judgeflow_icmp_lx_test.go` pin both directions. The operator-facing
+text says the same out loud (`shater/apply/warnings.go`): proxy-routed addresses
+"cannot be pinged at all — deliberately".
+
+> **Why a funnel and not an override inside the walk.** The first version of
+> this delta overrode the pre-declared `continueResult` inside `preMatchFlow`
+> and claimed to cover "every exit point of the function at once". It covered
+> every exit of THAT function; the walk above it has exits of its own that never
+> reach it — the `prepareMatchMetadata` error return (which arrived later, with
+> the shared-metadata refactor, upstream `b911fb078`), the sniff bail-outs, and
+> the `default:` arm of the rule-action switch, which catches every action
+> pre-match has no arm for (`hijack-dns`, `direct`, and whatever upstream adds
+> next). Each of those returned `PreMatchContinue`, i.e. `tun.ActionAccept`,
+> i.e. the forged reply. A guard on the single return value cannot be outgrown
+> by a new exit. `PreMatchBypass` joined the drop for the same reason: sing-tun
+> implements `ActionBypass` on the nfqueue plane only — the name appears nowhere
+> in `flow_dispatch.go` or `stack_gvisor_icmp.go` — so on the TUN path it lands
+> in the same `default:` arm as Accept and forges too. There is no honest bypass
+> for a packet that is already inside the engine's TUN.
+
+**KNOWN HOLE, not closed: a FRAGMENTED echo request to a WireGuard/AWG outbound
+still gets a forged reply.** The honest drop above covers the verdict; it does
+not cover the packets on which the working path never asks for one. Chain, read
+off the source — no reproducing unit test exists, because every link lives in
+the dependency:
+
+1. `ForwardDispatcher.Dispatch` (`flow_dispatch.go:176-177`) bails out on
+   `parsed.fragment` BEFORE calling `JudgeFlow` at all, so the packet is handed
+   on to the gVisor stack.
+2. The NIC is promiscuous and spoofing (`stack_gvisor.go:219-223`), so the stack
+   reassembles the fragments and delivers the echo to
+   `ICMPForwarder.HandlePacket` (`stack_gvisor_icmp.go:105+`).
+3. There `JudgeFlow` IS called, returns `ActionFlow` with the WireGuard port, and
+   `installFlow` (`:233-244`) refuses it: it writes the packet to the port
+   UNMODIFIED, so it requires `PortAddresses()` to be VALID **and UNSPECIFIED**,
+   and `transport/wireguard/port.go:13` reports the endpoint's concrete
+   interface address.
+4. `HandlePacket` falls past the switch and forges: `SetType(ICMPv4EchoReply)` +
+   address swap.
+
+The asymmetry is exactly backwards from what one would want: `direct` has no
+hole (`ping.Port.PortAddresses()` is `IPv4Unspecified()`, so `installFlow`
+accepts and the packet really leaves), while WireGuard/AWG — the outbounds the
+feature exists for — is where the forgery lives. Upstream applies precisely this
+unspecified test itself, and answers it honestly, in the cloudflared ICMP
+handler (`protocol/cloudflare/inbound.go:163-167`: "forwarding ICMP to
+outbound/… is not supported", drop) — the TUN path is the one that forges
+instead. And it is reachable well below the "jumbo ping" intuition: anything
+larger than the TUN's own MTU is fragmented by the KERNEL on the way in, so the
+threshold is `ping -s 1393`, not `ping -s 2000`.
+
+**The obvious fix is wrong and must not be applied.** "In `adapter.JudgeFlow`,
+refuse `ActionFlow` for ICMP when `PortAddresses()` is not unspecified" fails
+because `JudgeFlow` is the ONE function both consumers share, and they call it
+with byte-identical arguments (`flow_parse.go parseTransport` and
+`ICMPForwarder.HandlePacket` both use the echo identifier as source AND
+destination port; `firstPacket` is nil for ICMP from either). But
+`ForwardDispatcher.createFlow` (`flow_dispatch.go:325-335`) needs only a VALID
+address — it NATs by echo identifier and rewrites the source — so that check
+would drop every ping through WireGuard/AWG, fragmented or not, and leave
+`l3_tunnel` carrying nothing but `direct`.
+`TestJudgeFlowICMPToBoundPortStaysAFlow` exists to fail loudly if anyone tries;
+it was verified by applying the proposed fix and watching exactly that test go
+red while the `direct`-shaped twin stayed green.
+
+The two fixes that WOULD work, both outside the pre-match code:
+
+- **Stop the kernel from fragmenting into the TUN.** `forwardToPort`
+  (`flow_dispatch.go:445-481`) already does the MTU work correctly once it gets
+  a whole packet: for IPv4 without DF it fragments to `Port.PortMTU()` itself,
+  and with DF it emits a proper `fragmentation needed` quoting the tunnel MTU.
+  Raising the `l3-in` MTU above the LAN MTU would hand it whole packets and
+  close the common case — at the price of the "MTU 1420 = the WG payload budget"
+  contract, of the per-read buffer size on the router, and of nothing at all for
+  packets the CLIENT already fragmented onto the wire.
+- **Refuse to mark fragments into the TUN at all** — an `ip frag-off & 0x3fff
+  != 0` carve-out in the prerouting L3 block (`netplane/nft.go`), leaving them to
+  the `untunnelable` policy's honest verdict. That closes the client-fragmented
+  case only; it cannot see the fragmentation the kernel performs AFTER the
+  routing decision, so it is a complement to the first, not a substitute.
+
+Neither is a pre-match change, so neither is folded in here. Until one lands,
+the Consequence below is scoped accordingly.
 
 **fw4 has to be told about the device, and `list device` is the only spelling
 that works.** nftables runs EVERY table on every packet and a drop in any one of
@@ -962,10 +1044,14 @@ That was measured on a throwaway harness, not on aarch64, not under load, and
 with an empty ICMP NAT table, so it bounds nothing on the router. Neither
 item is folded into any claim above.
 
-Consequence: ping from the LAN either genuinely travels through the tunnel
-(WireGuard/AWG, direct) or fails honestly — nothing in the path forges liveness
-— and a router that never opts in renders the pre-feature plane byte-for-byte
-(`TestL3IngressOptIn` pins the off-state render).
+Consequence: an UNFRAGMENTED ping from the LAN either genuinely travels through
+the tunnel (WireGuard/AWG, direct) or fails honestly — no ROUTING verdict in the
+path forges liveness — and a router that never opts in renders the pre-feature
+plane byte-for-byte (`TestL3IngressOptIn` pins the off-state render). The
+qualifier is load-bearing: a fragmented echo to a WireGuard/AWG outbound still
+receives a forged reply from the TUN stack, per the KNOWN HOLE above. It was
+overclaimed here before; do not restore the unqualified sentence until one of
+the two named fixes has landed.
 
 ## D26 — What the engine cannot carry, the kernel carries: `untunnelable_egress`
 Decided 2026-07-26. D25 ended with ESP/AH/GRE/IGMP/SCTP still owned by the D17

route/prematch_icmp_lx_test.go

@@ -7,10 +7,28 @@ import (
 	"testing"
 
 	"github.com/sagernet/sing-box/adapter"
+	C "github.com/sagernet/sing-box/constant"
+	"github.com/sagernet/sing-box/log"
+	"github.com/sagernet/sing-box/option"
+	R "github.com/sagernet/sing-box/route/rule"
+	"github.com/sagernet/sing/common/json/badoption"
 	M "github.com/sagernet/sing/common/metadata"
 	N "github.com/sagernet/sing/common/network"
+
+	"github.com/stretchr/testify/require"
 )
 
+// The contract under test: PreMatch never answers "continue" (nor "bypass") for
+// an ICMP flow. adapter.JudgeFlow maps both to tun.ActionAccept, and the TUN
+// stack answers Accept by FORGING the echo reply itself
+// (sing-tun stack_gvisor_icmp.go ICMPForwarder.HandlePacket, the fallthrough
+// under the Flow/Reject/Drop switch). A verdict of "continue" therefore reads to
+// the operator as a working ping off a tunnel that never carried the packet.
+//
+// Every test below has a TCP/UDP twin: the honest drop must not leak into the
+// protocols where "continue" really does mean "take the ordinary connection
+// route".
+
 // icmpL4Outbound is a minimal L4-only outbound (the vless/vmess/... shape): it
 // does NOT implement adapter.FlowOutbound, and Network() lists only TCP/UDP.
 // Unused Outbound methods come from the embedded nil interface and are never
@@ -21,63 +39,230 @@ type icmpL4Outbound struct {
 }
 
 func (o *icmpL4Outbound) Tag() string       { return o.tag }
+func (o *icmpL4Outbound) Type() string      { return "vless" }
 func (o *icmpL4Outbound) Network() []string { return []string{N.NetworkTCP, N.NetworkUDP} }
 
-// icmpOutboundManager resolves tags from a fixed map; the rest of the
-// OutboundManager surface is never touched by preMatchFlow.
+// icmpOutboundManager resolves tags from a fixed map and hands the same L4-only
+// outbound out as the default; the rest of the OutboundManager surface is never
+// touched by the pre-match walk.
 type icmpOutboundManager struct {
 	adapter.OutboundManager
-	outbounds map[string]adapter.Outbound
+	defaultOutbound adapter.Outbound
+	outbounds       map[string]adapter.Outbound
 }
 
+func (m *icmpOutboundManager) Default() adapter.Outbound { return m.defaultOutbound }
+
 func (m *icmpOutboundManager) Outbound(tag string) (adapter.Outbound, bool) {
 	outbound, loaded := m.outbounds[tag]
 	return outbound, loaded
 }
 
-func icmpTestRouter(outbounds ...adapter.Outbound) *Router {
-	manager := &icmpOutboundManager{outbounds: make(map[string]adapter.Outbound)}
-	for _, outbound := range outbounds {
-		manager.outbounds[outbound.Tag()] = outbound
-	}
-	return &Router{ctx: context.Background(), outbound: manager}
+// icmpDNSRouter / icmpDNSTransportManager implement only what
+// prepareMatchMetadata reaches. FakeIP returns nil unless a transport is
+// installed, which is how the "fakeip lookup failed" exit is driven below.
+type icmpDNSRouter struct {
+	adapter.DNSRouter
 }
 
-func icmpTestMetadata(network string) (adapter.InboundContext, M.Socksaddr) {
-	destination := M.SocksaddrFrom(netip.MustParseAddr("1.1.1.1"), 0)
+func (s *icmpDNSRouter) LookupReverseMapping(netip.Addr) (string, bool) { return "", false }
+
+type icmpDNSTransportManager struct {
+	adapter.DNSTransportManager
+	fakeIP adapter.FakeIPTransport
+}
+
+func (s *icmpDNSTransportManager) FakeIP() adapter.FakeIPTransport {
+	if s.fakeIP == nil {
+		return nil
+	}
+	return s.fakeIP
+}
+
+// icmpMissingFakeIPTransport claims every address and then fails to look any of
+// them up — exactly the "missing fakeip record, try enable
+// `experimental.cache_file`" error prepareMatchMetadata returns.
+type icmpMissingFakeIPTransport struct {
+	adapter.FakeIPTransport
+}
+
+func (t *icmpMissingFakeIPTransport) Store() adapter.FakeIPStore {
+	return &icmpMissingFakeIPStore{}
+}
+
+type icmpMissingFakeIPStore struct {
+	adapter.FakeIPStore
+}
+
+func (s *icmpMissingFakeIPStore) Contains(netip.Addr) bool         { return true }
+func (s *icmpMissingFakeIPStore) Lookup(netip.Addr) (string, bool) { return "", false }
+
+type icmpRouterOptions struct {
+	fakeIP adapter.FakeIPTransport
+	rules  []option.Rule
+}
+
+func icmpTestRouter(t *testing.T, options icmpRouterOptions) *Router {
+	t.Helper()
+	logger := log.NewNOPFactory().NewLogger("test")
+	defaultOutbound := &icmpL4Outbound{tag: "proxy-out"}
+	router := &Router{
+		ctx:          context.Background(),
+		logger:       logger,
+		dns:          &icmpDNSRouter{},
+		dnsTransport: &icmpDNSTransportManager{fakeIP: options.fakeIP},
+		outbound: &icmpOutboundManager{
+			defaultOutbound: defaultOutbound,
+			outbounds:       map[string]adapter.Outbound{defaultOutbound.Tag(): defaultOutbound},
+		},
+	}
+	for i, ruleOptions := range options.rules {
+		rule, err := R.NewRule(router.ctx, logger, ruleOptions, false)
+		require.NoError(t, err, "build rule[%d]", i)
+		router.rules = append(router.rules, rule)
+	}
+	return router
+}
+
+func icmpTestMetadata(network string) adapter.InboundContext {
 	return adapter.InboundContext{
+		Inbound:     "l3-in",
+		InboundType: C.TypeTun,
 		Network:     network,
 		Source:      M.SocksaddrFrom(netip.MustParseAddr("192.168.1.2"), 0),
-		Destination: destination,
-	}, destination
+		Destination: M.SocksaddrFrom(netip.MustParseAddr("1.1.1.1"), 0),
+	}
 }
 
-// TestPreMatchICMPToL4OutboundDrops pins the honest-drop contract: an ICMP flow
-// routed to an outbound that cannot carry layer 3 must come back as
-// PreMatchDrop. PreMatchContinue is not a fallback for ICMP — the TUN stack
-// answers the echo itself (sing-tun stack_gvisor_icmp.go), so a Continue here
-// means the operator sees a working ping off a tunnel that never carried the
-// packet.
+// lanRuleWithAction matches every packet from the test source, so the action is
+// what the test is actually about.
+func lanRuleWithAction(action option.RuleAction) option.Rule {
+	return option.Rule{
+		Type: C.RuleTypeDefault,
+		DefaultOptions: option.DefaultRule{
+			RawDefaultRule: option.RawDefaultRule{
+				SourceIPCIDR: badoption.Listable[string]{"192.168.1.0/24"},
+			},
+			RuleAction: action,
+		},
+	}
+}
+
+// --- exit 1: an outbound that cannot carry layer 3 --------------------------
+
 func TestPreMatchICMPToL4OutboundDrops(t *testing.T) {
-	router := icmpTestRouter(&icmpL4Outbound{tag: "proxy-out"})
-	metadata, packetDestination := icmpTestMetadata(N.NetworkICMP)
-	result := router.preMatchFlow(context.Background(), &metadata, packetDestination, nil, "proxy-out")
-	if result.Action != adapter.PreMatchDrop {
-		t.Fatal("ICMP to an L4-only outbound fell through to the ordinary pre-match path: the TUN stack will forge the echo reply and ping will lie about a tunnel that never saw the packet")
-	}
+	t.Parallel()
+	router := icmpTestRouter(t, icmpRouterOptions{})
+	result := router.PreMatch(icmpTestMetadata(N.NetworkICMP), nil)
+	require.Equal(t, adapter.PreMatchDrop, result.Action,
+		"ICMP to an L4-only outbound fell through to the ordinary pre-match path: the TUN stack will forge the echo reply and ping will lie about a tunnel that never saw the packet")
 }
 
-// TestPreMatchTCPToL4OutboundContinues is the regression guard for the drop
-// above: for TCP (and UDP) PreMatchContinue really does mean "route through the
-// ordinary connection path", and that path must stay intact for outbounds that
-// are not FlowOutbound.
 func TestPreMatchTCPToL4OutboundContinues(t *testing.T) {
-	router := icmpTestRouter(&icmpL4Outbound{tag: "proxy-out"})
-	metadata, packetDestination := icmpTestMetadata(N.NetworkTCP)
-	result := router.preMatchFlow(context.Background(), &metadata, packetDestination, nil, "proxy-out")
-	if result.Action != adapter.PreMatchContinue {
-		t.Fatal("TCP to an L4-only outbound must keep taking the ordinary connection route; the ICMP honest-drop must not leak into TCP/UDP pre-match")
-	}
+	t.Parallel()
+	router := icmpTestRouter(t, icmpRouterOptions{})
+	result := router.PreMatch(icmpTestMetadata(N.NetworkTCP), nil)
+	require.Equal(t, adapter.PreMatchContinue, result.Action,
+		"TCP to an L4-only outbound must keep taking the ordinary connection route; the ICMP honest-drop must not leak into TCP/UDP pre-match")
+}
+
+func TestPreMatchUDPToL4OutboundContinues(t *testing.T) {
+	t.Parallel()
+	router := icmpTestRouter(t, icmpRouterOptions{})
+	result := router.PreMatch(icmpTestMetadata(N.NetworkUDP), nil)
+	require.Equal(t, adapter.PreMatchContinue, result.Action,
+		"UDP to an L4-only outbound must keep taking the ordinary connection route")
+}
+
+// --- exit 2: prepareMatchMetadata failed before any rule was walked ---------
+
+// This exit arrived with the shared prepareMatchMetadata refactor (upstream
+// b911fb078): it returns before the rule walk, so it never reaches preMatchFlow
+// where the ICMP override used to live.
+func TestPreMatchICMPMetadataErrorDrops(t *testing.T) {
+	t.Parallel()
+	router := icmpTestRouter(t, icmpRouterOptions{fakeIP: &icmpMissingFakeIPTransport{}})
+	result := router.PreMatch(icmpTestMetadata(N.NetworkICMP), nil)
+	require.Equal(t, adapter.PreMatchDrop, result.Action,
+		"a fakeip record that cannot be resolved must not degrade ICMP to continue: continue is tun.ActionAccept, and Accept is a forged echo reply")
+}
+
+func TestPreMatchTCPMetadataErrorContinues(t *testing.T) {
+	t.Parallel()
+	router := icmpTestRouter(t, icmpRouterOptions{fakeIP: &icmpMissingFakeIPTransport{}})
+	result := router.PreMatch(icmpTestMetadata(N.NetworkTCP), nil)
+	require.Equal(t, adapter.PreMatchContinue, result.Action,
+		"for TCP the metadata-error exit must keep meaning `take the ordinary connection route`")
+}
+
+// --- exit 3: a rule action the pre-match walk does not handle ---------------
+
+// hijack-dns is one of the actions PreMatch's switch has no arm for, so it lands
+// in the default arm. Any future unhandled action lands there too — that is why
+// the guard is a funnel on the return value and not a per-arm override.
+func TestPreMatchICMPUnhandledRuleActionDrops(t *testing.T) {
+	t.Parallel()
+	router := icmpTestRouter(t, icmpRouterOptions{
+		rules: []option.Rule{lanRuleWithAction(option.RuleAction{Action: C.RuleActionTypeHijackDNS})},
+	})
+	result := router.PreMatch(icmpTestMetadata(N.NetworkICMP), nil)
+	require.Equal(t, adapter.PreMatchDrop, result.Action,
+		"an unhandled rule action must not degrade ICMP to continue: continue is tun.ActionAccept, and Accept is a forged echo reply")
+}
+
+func TestPreMatchTCPUnhandledRuleActionContinues(t *testing.T) {
+	t.Parallel()
+	router := icmpTestRouter(t, icmpRouterOptions{
+		rules: []option.Rule{lanRuleWithAction(option.RuleAction{Action: C.RuleActionTypeHijackDNS})},
+	})
+	result := router.PreMatch(icmpTestMetadata(N.NetworkTCP), nil)
+	require.Equal(t, adapter.PreMatchContinue, result.Action,
+		"the unhandled-action exit must stay a continue for TCP")
+}
+
+// --- exit 4: an explicit bypass ---------------------------------------------
+
+// sing-tun implements ActionBypass on the nfqueue plane only; on the TUN path it
+// falls into the same default arm as Accept (flow_dispatch.go judgeAndInstall,
+// and the ICMP forwarder's switch has no Bypass case either), i.e. into the same
+// forgery. There is no honest bypass for a packet already inside the engine's
+// TUN.
+func TestPreMatchICMPBypassDrops(t *testing.T) {
+	t.Parallel()
+	router := icmpTestRouter(t, icmpRouterOptions{
+		rules: []option.Rule{lanRuleWithAction(option.RuleAction{Action: C.RuleActionTypeBypass})},
+	})
+	result := router.PreMatch(icmpTestMetadata(N.NetworkICMP), nil)
+	require.Equal(t, adapter.PreMatchDrop, result.Action,
+		"bypass degrades to tun.ActionAccept on the TUN path, which is the forged echo reply again")
+}
+
+func TestPreMatchTCPBypassIsStillBypass(t *testing.T) {
+	t.Parallel()
+	router := icmpTestRouter(t, icmpRouterOptions{
+		rules: []option.Rule{lanRuleWithAction(option.RuleAction{Action: C.RuleActionTypeBypass})},
+	})
+	result := router.PreMatch(icmpTestMetadata(N.NetworkTCP), nil)
+	require.Equal(t, adapter.PreMatchBypass, result.Action,
+		"the ICMP honest-drop must not turn a TCP bypass rule into a drop")
+}
+
+// --- the verdicts that must pass through untouched ---------------------------
+
+// A reject rule already carries its own honest verdict; the funnel must not
+// rewrite it (a Reject sends an ICMP unreachable, which is information, not a
+// forged liveness signal).
+func TestPreMatchICMPRejectIsNotRewritten(t *testing.T) {
+	t.Parallel()
+	router := icmpTestRouter(t, icmpRouterOptions{
+		rules: []option.Rule{lanRuleWithAction(option.RuleAction{
+			Action:        C.RuleActionTypeReject,
+			RejectOptions: option.RejectActionOptions{Method: C.RuleActionRejectMethodDefault},
+		})},
+	})
+	result := router.PreMatch(icmpTestMetadata(N.NetworkICMP), nil)
+	require.Equal(t, adapter.PreMatchReject, result.Action,
+		"the ICMP funnel must only rewrite continue/bypass, never an explicit reject")
 }
 
 // lx:end l3-honest-drop

route/route.go

@@ -314,7 +314,48 @@ func (r *Router) routePacketConnection(ctx context.Context, conn N.PacketConn, m
 	return nil
 }
 
+// lx:begin l3-honest-drop
+// PreMatch funnels every verdict of the pre-match walk through one ICMP check.
+//
+// An ICMP flow has no fallback path, so PreMatchContinue is not "try the
+// ordinary connection route" the way it is for TCP and UDP: the TUN stack takes
+// the packet back and answers the echo ITSELF (sing-tun stack_gvisor_icmp.go —
+// adapter.JudgeFlow maps Continue to tun.ActionAccept, and the ICMP forwarder
+// answers Accept by rewriting Echo into EchoReply and swapping the addresses).
+// A ping routed to an outbound that cannot carry layer 3 — every proxy
+// protocol; only adapter.FlowOutbound can — would therefore return a FORGED
+// reply, and the operator would read a working ping off a tunnel that never saw
+// the packet. Dropping instead reports the truth.
+//
+// PreMatchBypass is folded into the same drop because sing-tun implements
+// bypass for the nfqueue plane only (`ActionBypass` appears nowhere in
+// flow_dispatch.go / stack_gvisor_icmp.go): on the TUN path it degrades to the
+// same Accept, i.e. to the same forgery. There is no honest bypass for an ICMP
+// packet that is already inside the engine's TUN.
+//
+// This is a funnel and not an override inside the walk on purpose: the walk has
+// several independent exits that say "continue" (the prepareMatchMetadata error
+// return, the sniff bail-outs, the un-routable `bypass`, and the default arm of
+// the rule-action switch), and an earlier version of this delta guarded only
+// the ones that pass through preMatchFlow — leaving the others as narrow paths
+// to the forged reply. Guarding the single return value cannot be outgrown by a
+// new exit.
 func (r *Router) PreMatch(metadata adapter.InboundContext, firstPacket []byte) adapter.PreMatchResult {
+	result := r.preMatch(metadata, firstPacket)
+	if metadata.Network == N.NetworkICMP {
+		switch result.Action {
+		case adapter.PreMatchContinue, adapter.PreMatchBypass:
+			return adapter.PreMatchResult{Action: adapter.PreMatchDrop}
+		}
+	}
+	return result
+}
+
+// preMatch is upstream's PreMatch body, unchanged; only the name moved, so that
+// the funnel above owns the exported entry point. An upstream change to the
+// pre-match walk applies to THIS function.
+func (r *Router) preMatch(metadata adapter.InboundContext, firstPacket []byte) adapter.PreMatchResult {
+	// lx:end l3-honest-drop
 	ctx := log.ContextWithNewID(r.ctx)
 	metadata.PreMatch = true
 	continueResult := adapter.PreMatchResult{Action: adapter.PreMatchContinue}
@@ -440,18 +481,11 @@ func applyRouteOptionsOverride(metadata *adapter.InboundContext, routeOptions *R
 
 func (r *Router) preMatchFlow(ctx context.Context, metadata *adapter.InboundContext, packetDestination M.Socksaddr, matchedRule adapter.Rule, outboundTag string) adapter.PreMatchResult {
 	continueResult := adapter.PreMatchResult{Action: adapter.PreMatchContinue}
-	// lx:begin l3-honest-drop
-	// An ICMP flow has no fallback path, so PreMatchContinue is not "try the
-	// ordinary connection route" the way it is for TCP and UDP: the TUN stack
-	// takes the packet back and answers the echo ITSELF
-	// (sing-tun stack_gvisor_icmp.go). A ping routed to an outbound that cannot
-	// carry layer 3 — every proxy protocol; only adapter.FlowOutbound can —
-	// therefore returns a FORGED reply, and the operator reads working ping off a
-	// tunnel that never saw the packet. Dropping instead reports the truth.
-	if metadata.Network == N.NetworkICMP {
-		continueResult = adapter.PreMatchResult{Action: adapter.PreMatchDrop}
-	}
-	// lx:end l3-honest-drop
+	// lx: ICMP does NOT get a local override here any more — the honest drop is
+	// applied once, to the single return value of PreMatch (see the funnel
+	// there, marker l3-honest-drop). Overriding continueResult in this function
+	// covered only the exits that reach it and left the walk's own exits
+	// forging.
 	var outbound adapter.Outbound
 	if outboundTag == "" {
 		outbound = r.outbound.Default()

shater/generate/inbound.go

@@ -174,10 +174,16 @@ func (b *builder) appendL3TunInbound(inbounds []option.Inbound, seenTag map[stri
 			// netplane installs the scoped rule/route (fwmark L3Mark -> L3Table
 			// -> L3Device) itself; that is the whole routing story.
 			AutoRoute: false,
-			// Only the gVisor stack truly FORWARDS ICMP into the routed
-			// outbound. The system stack answers echo locally — a reply forged
-			// on the router for a host it never asked — which is exactly the
-			// lie l3_tunnel exists to remove.
+			// gvisor is a CHOICE, not a necessity, and the tempting reason for
+			// it is wrong: BOTH sing-tun stacks forward ICMP through the same
+			// ForwardDispatcher first, and both forge an echo reply only for
+			// what that dispatcher declined (system: stack_system.go
+			// dispatchIPv4 -> processIPv4ICMP; gvisor: stack_gvisor_filter.go
+			// -> ICMPForwarder.HandlePacket). Do not re-derive this as "the
+			// system stack fakes ping" — D25 says so explicitly. gvisor is
+			// picked because it is already linked (with_wireguard requires
+			// with_gvisor, D23), so it costs no build tag and no new code path,
+			// and because it is the combination the integration test exercises.
 			Stack: "gvisor",
 		},
 	})