fix(classify): cross-address-family mapping verdict (closes #14) — v0.1.3

CHANGELOG.md

@@ -4,6 +4,24 @@ All notable changes to this project are documented here. Format follows [Keep a
 
 ## [Unreleased]
 
+## [0.1.3] — 2026-04-26
+
+### Fixed
+
+- Cross-address-family probe sets no longer produce a wrong ADM/symmetric verdict. Previously, a probe set mixing IPv6-resolved hostname servers (e.g., `stun.l.google.com`) with IPv4-literal servers (e.g., a self-hosted coturn) compared mapped endpoints across address families and reported ADM because the endpoints differed by construction (each family observes its own NAT). The classifier now groups successes by address family, classifies each group independently, and combines under the rule "Unknown is absence of information, not disagreement": matching verdicts win, two confident verdicts that differ produce Unknown, a confident verdict beats Unknown from the other group. Closes #14.
+
+### Added
+
+- New warning value `mixed_address_family_probes` in `warnings[]`. Emitted whenever successful probes span both IPv4 and IPv6 address families. Additive to the JSON schema.
+
+### Migration note
+
+JSON consumers checking `nat_type == "ADM"` for cross-family probe sets will see `"Unknown"` on the same input under v0.1.3 — the previous verdict was incorrect. Forecast-checking consumers (`webrtc_forecast.direct_p2p`) are mostly unaffected: the dominant cross-family disagreement case stays exit 1 (Unknown → 1, was ADM → 1). The verdict-flip case (genuinely agreed EIM across families, previously ADM, now EIM → exit 0) is the bug being fixed.
+
+### Note on tag versioning
+
+v0.1.3 is the first 3-segment-semver tag after the v0.1.2.x patches; v0.1.2.1 and v0.1.2.2 were 4-segment tags incompatible with Go module versioning (proxy silently substituted a pseudo-version). See those releases' notes. From v0.1.3 onward, tags are 3-segment semver only.
+
 ## [0.1.2.2] — 2026-04-26
 
 ### Fixed
@@ -85,7 +103,8 @@ Initial release. See [`docs/design.md`](docs/design.md) for scope and architectu
 - Go 1.25+
 - [`github.com/pion/stun/v3`](https://github.com/pion/stun)
 
-[Unreleased]: https://github.com/1mb-dev/natcheck/compare/v0.1.2.2...HEAD
+[Unreleased]: https://github.com/1mb-dev/natcheck/compare/v0.1.3...HEAD
+[0.1.3]: https://github.com/1mb-dev/natcheck/releases/tag/v0.1.3
 [0.1.2.2]: https://github.com/1mb-dev/natcheck/releases/tag/v0.1.2.2
 [0.1.2.1]: https://github.com/1mb-dev/natcheck/releases/tag/v0.1.2.1
 [0.1.2]: https://github.com/1mb-dev/natcheck/releases/tag/v0.1.2

docs/design.md

@@ -1,6 +1,6 @@
 # natcheck — Architecture
 
-> Status: v0.1.2 shipped (RFC 5780 §4.4 filtering classification). v0.2 design contract is the addendum at end of document.
+> Status: v0.1.3 shipped (RFC 5780 §4.4 filtering classification + cross-family mapping fix). v0.2 design contract is the addendum at end of document.
 > Last updated: 2026-04-26
 
 Product framing and technical spec. v0.1 spec lives in this document as shipped; v0.2 design is the addendum at the end. Working notes and per-phase plans live in `todos/` (dev-internal, not tracked in git).
@@ -331,8 +331,10 @@ v0.2 is a milestone reached via four bisectable patches, not a single release. E
 | Patch | Status | Adds |
 |---|---|---|
 | **v0.1.2** | shipped (PRs #7–#11, tag `v0.1.2`) | RFC 5780 §4.4 filtering classification (capability-driven via `--server` advertising `OTHER-ADDRESS`); JSON `filtering` object; classifier upgrade emitting reserved `possible`; coturn reference config asset + setup doc; `internal/stunserver` foundation package |
-| **v0.1.3** | planned | Hairpinning detection via two local sockets, parallel with mapping probes; JSON `hairpinning` field |
-| **v0.1.4** | planned | `natcheck server` subcommand: stateless RFC 5780 §3 STUN responder. Promoted from in-process responder via shared `internal/stunserver/` package |
+| **v0.1.2.x** | shipped (tags `v0.1.2.1`, `v0.1.2.2`) | coturn config + setup doc fixes so v0.1.2's filtering classification works on more provider topologies; `scripts/validate-coturn.sh` provisioner. No code or schema delta. (4-segment tags incompatible with Go module versioning — see those releases' notes; lesson recorded.) |
+| **v0.1.3** | shipped (tag `v0.1.3`) | Cross-address-family mapping classification fix (#14): per-family grouping + combine. New `mixed_address_family_probes` warning. Hairpinning shifts to v0.1.4. |
+| **v0.1.4** | planned | Hairpinning detection via two local sockets, parallel with mapping probes; JSON `hairpinning` field |
+| **v0.1.5** | planned | `natcheck server` subcommand: stateless RFC 5780 §3 STUN responder. Promoted from in-process responder via shared `internal/stunserver/` package |
 | **v0.2.0** | planned | Version bump; README + site copy reconciliation; CHANGELOG with v0.1 → v0.2 JSON migration section |
 
 v0.2.0 is the line where downstream consumers can rely on the new schema fields.

internal/classify/classify.go

@@ -126,6 +126,7 @@ const (
 	WarnInsufficientProbes              = "insufficient_probes"
 	WarnFilteringBehaviorNotTested      = "filtering_behavior_not_tested"
 	WarnFilteringSkippedNoChangeRequest = "filtering_skipped_no_change_request"
+	WarnMixedAddressFamilyProbes        = "mixed_address_family_probes"
 )
 
 // cgnatPrefix is RFC 6598 shared address space.
@@ -152,6 +153,14 @@ func Classify(results []probe.Result, filtering *probe.FilteringResult) Verdict
 // classifyMapping derives mapping behavior, public endpoint, CGNAT, and the
 // initial warning set. Forecast is computed by decideForecast after filtering
 // data is folded in.
+//
+// Address-family handling: probes are grouped by Mapped.Addr().Is4() and
+// classified per-group, then combined. Each family observes its own NAT
+// (the v4 NAT and v6 router are independent), so cross-family equality
+// comparison would always disagree by construction. The combine rule treats
+// Unknown as absence of information, not disagreement: a confident verdict
+// from one family wins over Unknown from the other; two confident verdicts
+// must match or the combined verdict is Unknown.
 func classifyMapping(results []probe.Result) Verdict {
 	v := Verdict{Warnings: []string{WarnFilteringBehaviorNotTested}}
 
@@ -162,24 +171,126 @@ func classifyMapping(results []probe.Result) Verdict {
 		}
 	}
 
-	switch len(successes) {
-	case 0:
+	if len(successes) == 0 {
 		v.Type = Blocked
 		v.Warnings = append(v.Warnings, WarnAllProbesFailed)
 		return v
+	}
+
+	var v4, v6 []probe.Result
+	for _, r := range successes {
+		if r.Mapped.Addr().Is4() {
+			v4 = append(v4, r)
+		} else {
+			v6 = append(v6, r)
+		}
+	}
+	multiFamily := len(v4) > 0 && len(v6) > 0
+
+	var v4g, v6g *Verdict
+	if len(v4) > 0 {
+		g := classifyGroup(v4)
+		v4g = &g
+	}
+	if len(v6) > 0 {
+		g := classifyGroup(v6)
+		v6g = &g
+	}
 
+	combined := combineGroups(v4g, v6g)
+	v.Type = combined.Type
+	v.LegacyName = combined.LegacyName
+	v.PublicEndpoint = successes[0].Mapped // top-level endpoint = first success, regardless of family
+	v.CGNAT = combined.CGNAT
+
+	// Warning propagation:
+	//   - WarnInsufficientProbes / WarnCGNATDetected propagate independently
+	//     from any group (they describe facts true of that group).
+	//   - WarnADMOrStricter propagates only when the COMBINED verdict is ADM
+	//     (suppressed under disagreement-Unknown — premise no longer holds).
+	//   - WarnMixedAddressFamilyProbes fires whenever both families are present.
+	if multiFamily {
+		v.Warnings = appendUnique(v.Warnings, WarnMixedAddressFamilyProbes)
+	}
+	for _, g := range []*Verdict{v4g, v6g} {
+		if g == nil {
+			continue
+		}
+		for _, w := range g.Warnings {
+			if w == WarnADMOrStricter && v.Type != AddressDependentMapping {
+				continue
+			}
+			v.Warnings = appendUnique(v.Warnings, w)
+		}
+	}
+
+	return v
+}
+
+// combineGroups reconciles per-family verdicts. "Unknown is absence of
+// information, not disagreement" — a confident verdict beats Unknown; two
+// confident verdicts must match or the combined verdict is Unknown.
+//
+// CGNAT is OR'd across groups (the fact applies if true for any family;
+// decideForecast's CGNAT precedence handles the forecast accordingly).
+func combineGroups(v4g, v6g *Verdict) Verdict {
+	switch {
+	case v4g != nil && v6g != nil:
+		// Both families present.
+		out := Verdict{CGNAT: v4g.CGNAT || v6g.CGNAT}
+		switch {
+		case v4g.Type == Unknown && v6g.Type == Unknown:
+			out.Type = Unknown
+		case v4g.Type == Unknown:
+			out.Type = v6g.Type
+			out.LegacyName = v6g.LegacyName
+		case v6g.Type == Unknown:
+			out.Type = v4g.Type
+			out.LegacyName = v4g.LegacyName
+		case v4g.Type == v6g.Type:
+			out.Type = v4g.Type
+			out.LegacyName = v4g.LegacyName
+		default:
+			out.Type = Unknown
+		}
+		return out
+	case v4g != nil:
+		return *v4g
+	case v6g != nil:
+		return *v6g
+	default:
+		// Unreachable: caller ensures at least one group is non-nil.
+		return Verdict{Type: Unknown}
+	}
+}
+
+// appendUnique appends w to ws only if not already present. Preserves order.
+func appendUnique(ws []string, w string) []string {
+	for _, x := range ws {
+		if x == w {
+			return ws
+		}
+	}
+	return append(ws, w)
+}
+
+// classifyGroup runs the case-1/case-many mapping classification for a
+// non-empty set of successful probes. CGNAT detection is applied. Warnings
+// returned do NOT include WarnFilteringBehaviorNotTested — that's the
+// caller's concern at the combined-verdict level.
+func classifyGroup(group []probe.Result) Verdict {
+	v := Verdict{}
+	switch len(group) {
 	case 1:
 		v.Type = Unknown
-		v.PublicEndpoint = successes[0].Mapped
+		v.PublicEndpoint = group[0].Mapped
 		v.Warnings = append(v.Warnings, WarnInsufficientProbes)
 		applyCGNAT(&v)
-		return v
-
 	default:
-		v.PublicEndpoint = successes[0].Mapped
+		v.PublicEndpoint = group[0].Mapped
 		allSame := true
-		for _, r := range successes[1:] {
-			if r.Mapped != successes[0].Mapped {
+		for _, r := range group[1:] {
+			if r.Mapped != group[0].Mapped {
 				allSame = false
 				break
 			}
@@ -193,8 +304,8 @@ func classifyMapping(results []probe.Result) Verdict {
 			v.Warnings = append(v.Warnings, WarnADMOrStricter)
 		}
 		applyCGNAT(&v)
-		return v
 	}
+	return v
 }
 
 // applyFiltering folds *probe.FilteringResult into the verdict. Updates

internal/classify/classify_test.go

@@ -431,6 +431,158 @@ func TestClassify_FilteringMatrix(t *testing.T) {
 			if tc.wantWarnAbsent != "" && hasWarning(got.Warnings, tc.wantWarnAbsent) {
 				t.Errorf("Warnings unexpectedly contains %q; got %v", tc.wantWarnAbsent, got.Warnings)
 			}
+			// Filtering matrix uses two same-family probes; the cross-family
+			// warning must never leak in.
+			if hasWarning(got.Warnings, WarnMixedAddressFamilyProbes) {
+				t.Errorf("Warnings unexpectedly contains %q; got %v", WarnMixedAddressFamilyProbes, got.Warnings)
+			}
+		})
+	}
+}
+
+// TestClassify_MixedAddressFamily covers the per-family grouping + combine
+// path added for #14. Each family observes its own NAT, so cross-family
+// equality comparison would always disagree by construction; the classifier
+// groups by Mapped.Addr().Is4(), classifies each group, and combines under
+// the rule "Unknown is absence of information, not disagreement."
+func TestClassify_MixedAddressFamily(t *testing.T) {
+	v4ok := func(host, mapped string) probe.Result {
+		return probe.Result{
+			Server: probe.Server{Host: host, Port: 3478},
+			Mapped: netip.MustParseAddrPort(mapped),
+			RTT:    10 * time.Millisecond,
+		}
+	}
+	v6ok := func(host, mapped string) probe.Result {
+		return probe.Result{
+			Server: probe.Server{Host: host, Port: 3478},
+			Mapped: netip.MustParseAddrPort(mapped),
+			RTT:    10 * time.Millisecond,
+		}
+	}
+
+	cases := []struct {
+		name           string
+		results        []probe.Result
+		wantType       NATType
+		wantLegacy     string
+		wantCGNAT      bool
+		wantP2P        string
+		wantWarnHas    []string
+		wantWarnAbsent []string
+	}{
+		{
+			name: "mixed_v4eim_v6eim_agreed",
+			results: []probe.Result{
+				v4ok("a", "203.0.113.45:51820"),
+				v4ok("b", "203.0.113.45:51820"),
+				v6ok("c", "[2001:db8::1]:51820"),
+				v6ok("d", "[2001:db8::1]:51820"),
+			},
+			wantType:       EndpointIndependentMapping,
+			wantLegacy:     "cone",
+			wantP2P:        "likely",
+			wantWarnHas:    []string{WarnMixedAddressFamilyProbes, WarnFilteringBehaviorNotTested},
+			wantWarnAbsent: []string{WarnADMOrStricter, WarnInsufficientProbes, WarnAllProbesFailed},
+		},
+		{
+			name: "mixed_v4eim_v6adm_disagreement",
+			results: []probe.Result{
+				v4ok("a", "203.0.113.45:51820"),
+				v4ok("b", "203.0.113.45:51820"),
+				v6ok("c", "[2001:db8::1]:51820"),
+				v6ok("d", "[2001:db8::2]:51821"),
+			},
+			wantType:       Unknown,
+			wantP2P:        "unknown",
+			wantWarnHas:    []string{WarnMixedAddressFamilyProbes},
+			wantWarnAbsent: []string{WarnADMOrStricter}, // suppressed: combined verdict isn't ADM
+		},
+		{
+			name: "mixed_v4eim_v6singleton_v4_wins",
+			results: []probe.Result{
+				v4ok("a", "203.0.113.45:51820"),
+				v4ok("b", "203.0.113.45:51820"),
+				v6ok("c", "[2001:db8::1]:51820"),
+			},
+			wantType:    EndpointIndependentMapping,
+			wantLegacy:  "cone",
+			wantP2P:     "likely",
+			wantWarnHas: []string{WarnMixedAddressFamilyProbes, WarnInsufficientProbes},
+		},
+		{
+			name: "mixed_v4cgnat_eim_v6eim_cgnat_precedence",
+			results: []probe.Result{
+				v4ok("a", "100.64.1.5:51820"),
+				v4ok("b", "100.64.1.5:51820"),
+				v6ok("c", "[2001:db8::1]:51820"),
+				v6ok("d", "[2001:db8::1]:51820"),
+			},
+			wantType:    EndpointIndependentMapping,
+			wantLegacy:  "cone",
+			wantCGNAT:   true,
+			wantP2P:     "unknown", // CGNAT precedence overrides EIM
+			wantWarnHas: []string{WarnMixedAddressFamilyProbes, WarnCGNATDetected},
+		},
+		{
+			name: "mixed_singleton_each_combined_unknown",
+			results: []probe.Result{
+				v4ok("a", "203.0.113.45:51820"),
+				v6ok("b", "[2001:db8::1]:51820"),
+			},
+			wantType:    Unknown,
+			wantP2P:     "unknown",
+			wantWarnHas: []string{WarnMixedAddressFamilyProbes, WarnInsufficientProbes},
+		},
+		{
+			name: "single_family_v6_only_no_mixed_warning",
+			results: []probe.Result{
+				v6ok("a", "[2001:db8::1]:51820"),
+				v6ok("b", "[2001:db8::1]:51820"),
+			},
+			wantType:       EndpointIndependentMapping,
+			wantLegacy:     "cone",
+			wantP2P:        "likely",
+			wantWarnAbsent: []string{WarnMixedAddressFamilyProbes, WarnADMOrStricter, WarnInsufficientProbes},
+		},
+		{
+			name: "single_family_v4_unchanged_path",
+			results: []probe.Result{
+				v4ok("a", "203.0.113.45:51820"),
+				v4ok("b", "203.0.113.45:51820"),
+			},
+			wantType:       EndpointIndependentMapping,
+			wantLegacy:     "cone",
+			wantP2P:        "likely",
+			wantWarnAbsent: []string{WarnMixedAddressFamilyProbes, WarnADMOrStricter, WarnInsufficientProbes},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := Classify(tc.results, nil)
+			if got.Type != tc.wantType {
+				t.Errorf("Type = %v, want %v", got.Type, tc.wantType)
+			}
+			if got.LegacyName != tc.wantLegacy {
+				t.Errorf("LegacyName = %q, want %q", got.LegacyName, tc.wantLegacy)
+			}
+			if got.CGNAT != tc.wantCGNAT {
+				t.Errorf("CGNAT = %v, want %v", got.CGNAT, tc.wantCGNAT)
+			}
+			if got.Forecast.DirectP2P != tc.wantP2P {
+				t.Errorf("DirectP2P = %q, want %q", got.Forecast.DirectP2P, tc.wantP2P)
+			}
+			for _, want := range tc.wantWarnHas {
+				if !hasWarning(got.Warnings, want) {
+					t.Errorf("Warnings missing %q; got %v", want, got.Warnings)
+				}
+			}
+			for _, absent := range tc.wantWarnAbsent {
+				if hasWarning(got.Warnings, absent) {
+					t.Errorf("Warnings unexpectedly contains %q; got %v", absent, got.Warnings)
+				}
+			}
 		})
 	}
 }