Files
shater/shater/generate/retired_egress_test.go
T
omarandClaude Opus 5 4869d62e02
test / go + panel tests (push) Successful in 1m39s
release / test gate (push) Successful in 1m39s
release / apk aarch64_cortex-a53 (push) Failing after 2m54s
release / apk x86_64 (push) Failing after 2m54s
release / release apk (push) Failing after 1m35s
feat(egress)!: remove byedpi — what it replaced was not weak, it was broken (D29)
The `byedpi` egress kind, the `openwrt/byedpi` package (`ciadpi`), the readiness
endpoint and the panel plate are gone. D13 is not deleted from DECISIONS.md; it
is REVERSED there, with the reason, because the reason is the whole point.

D13 adopted an external desync process on an observation: the engine's own
`tls_fragment`/`tls_record_fragment` were tried against a live ISP and did not
get through, so the method was judged too weak for anything past "just fragment
the ClientHello". The method was never tried. `common/tlsfragment` dropped a
number of labels equal to the number of DOTS in the name, and a name always has
one more label than it has dots — so the cut always landed inside the FIRST
label. `www.youtube.com` was split inside `www` and `youtube` went to the wire
in one piece, which is the word the DPI matches on. Of six blocked names exactly
one got through: `youtube.com`, the one whose first label IS the blocked word.
That defect is fixed (815011dfb, efb2177f4). With it fixed the built-in presets
do the job the external process was brought in to do, and the process is 100 KB
of binary, a second procd service, a second UCI file, a port that agreed with
our egress by hand-written comment only, a readiness prober, a five-state
service model and a panel plate — all to work around fifteen lines of ours.

So this is not "ByeDPI turned out to be bad". It is a good tool that turned out
not to be needed, and the reason we thought it was needed was ours.

A CONFIG THAT STILL SAYS `type 'byedpi'` IS THE PART THAT NEEDED WORK. Nothing
is migrated and nothing is rewritten: the kind stays unbuildable, therefore
fail-closed — no outbound, no mark, no `ip rule`, no routing table, so every
node, group and rule bound to it is blocked rather than released onto the plain
WAN. A migration to `direct` was considered and rejected: it is the only rewrite
that leaves the egress routing at all, and it would silently turn a blocked
egress into a live plain-WAN path with the router's real address — by an
upgrade, on a config nobody touched. `CurrentSchemaVersion` is therefore not
bumped either: no stored field changes meaning, and a bump would only make this
build's configs unreadable to an older daemon for no gain.

What changes is what the operator is TOLD. `model.RetiredEgressTypes` is a
closed, positive table read by BOTH `ValidateEgresses` and the generator (one
copy of the sentence, because two copies drift). It names the removal, denies
that it is a typo, says nothing is built and that the traffic is blocked rather
than leaked, names the replacement (`direct`/`interface` with `dpi 'record'`),
refuses to promise which preset defeats a given ISP, and says `apk del byedpi`.
The generic "unknown type" is still there and still says something different, on
purpose: "we took this kind away" and "you mistyped something" send an operator
to different places, and a value that was correct on the day it was written must
not be reported as a spelling mistake. The type list stays closed and positive —
`interface`, `direct`, the alias `tunnel` — and `EgressTypeKnown` does NOT admit
the retired kind: being told it was removed and having it work anyway is worse
than either alone.

`Egress.Port` goes with the kind: no surviving egress dials anything, so the
option is no longer parsed and drains out of /etc/config/shater on the next
render, the same way the deleted per-group probe_url/probe_interval did.

Tests, verified by mutation, each failing by name:
  - drop the retired branch in `ValidateEgresses` -> the retired kind is
    reported as "is not one of interface/direct" and
    TestRetiredEgressTypeIsReportedByTheValidator fails on both spellings;
  - drop it in the generator -> "unknown type \"byedpi\"" and
    TestRetiredEgressTypeIsReportedByTheGenerator fails;
  - the FAIL-OPEN mutation, which is the one that matters: let `byedpi` fall
    into the `direct` arm and be a known type -> four tests fail, including the
    two that check no outbound is emitted. A removal that quietly starts routing
    the traffic it used to block, under a reassuring message, is the failure with
    the worst consequence;
  - the panel half: empty RETIRED_EGRESS_TYPES -> two egressEdit tests fail.
Controls beside the claims: `interface`, `direct`, the `tunnel` alias and the
empty synonym must still resolve, warn about nothing and emit an outbound
(TestSupportedEgressTypesAreUntouched), and never-supported values — `proxy`,
`block`, `wireguard`, `byedpi2`, `bye dpi`, `sorcery` — must NOT draw the
removal sentence, which names a replacement for something that never existed.

CI and docs: the feed loses its fourth package everywhere the four were named —
`apk upgrade shaterd shater-core luci-app-shater`, in CLAUDE.md, both READMEs,
INSTALL.md, the release body and `shaterd`'s own diag bundle. The version
exception (byedpi carried upstream's version, ours come from the git tag) is
gone with it, so ci/version.sh and ci/sdk-build-apk.sh no longer have an
exception to remember and the "expected >=4 of OUR .apk" collect check is now 3.
INSTALL.md §5.3 gains the half a feed cannot do: dropping the package from the
feed does not take it off a router it is already on, so `apk del byedpi` is
written down, with what it removes and why it is safe.

Panel: 368 tests -> 339. Deleted with the mechanism they covered:
byedpiReady.test.ts, byedpiAge.test.ts, byedpiRefusal.test.ts (34 tests);
egressEdit.test.ts gains 5 for the retired-type sentence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
2026-07-27 17:13:50 +03:00

205 lines
9.5 KiB
Go

package generate
import (
"strings"
"testing"
"github.com/sagernet/sing-box/shater/model"
"github.com/sagernet/sing-box/shater/netplane"
)
// A RETIRED egress kind is the one configuration state a removal creates that
// nobody chose. `byedpi` — an egress that handed traffic to a separate ciadpi
// process over local SOCKS — shipped, was documented, and is configured on real
// routers right now. Deleting the code that implemented it does not delete the
// `option type 'byedpi'` sitting in somebody's /etc/config/shater.
//
// What that config must do, and what these tests pin:
//
// 1. it must keep FAILING CLOSED. No outbound, no device, no mark, no routing
// table — everything bound to it stops rather than leaving over the plain
// WAN with the router's own address. This is unchanged by the removal, and
// it is the half that must not regress while the messages are being changed;
// 2. it must be reported by the sentence that says the kind was REMOVED and
// what to put in its place — not by the generic "unknown type", which sends
// the operator hunting for a spelling mistake in a value that was correct on
// the day they typed it;
// 3. a type that was never supported must NOT get that sentence, because it
// names a replacement and there is nothing to replace.
//
// The message lives in exactly one place, model.RetiredEgressTypes, and both the
// validator and the generator read it from there. These tests assert the FACTS
// the message has to carry rather than the whole string, so rewording it stays
// possible and dropping a fact does not.
// retiredFacts are the load-bearing claims of the removal sentence. Each is
// checked as a substring; together they are what makes the message actionable
// instead of merely different.
var retiredFacts = []struct {
name string
sub string
}{
{"names the kind", `"byedpi"`},
{"says it was removed from the product", "was REMOVED from this product"},
{"denies that it is a typo", "not a misspelling"},
{"says nothing is built for it", "no outbound"},
{"says the traffic is blocked, not leaked", "BLOCKED (fail-closed)"},
{"names the replacement type", "'direct'"},
{"names the replacement preset", "'record'"},
{"refuses to promise the preset works", "is not promised here"},
{"says how to take the dead package off the router", "apk del byedpi"},
}
func assertRetiredFacts(t *testing.T, where, msg string) {
t.Helper()
for _, f := range retiredFacts {
if !strings.Contains(msg, f.sub) {
t.Errorf("%s: the removal message no longer %s (wanted %q in it).\nMessage was:\n%s",
where, f.name, f.sub, msg)
}
}
}
// TestRetiredEgressTypeIsReportedByTheValidator is the model half: an egress
// carrying a retired kind draws the removal finding, and draws it INSTEAD of the
// unrecognised-value finding.
func TestRetiredEgressTypeIsReportedByTheValidator(t *testing.T) {
// Spelled the way an operator's config and a hand edit would have it: the
// canonical lower case, and a padded mixed-case variant that ReadUCI's
// normalisation would not fold (byedpi is no alias of anything).
for _, written := range []string{"byedpi", " ByeDPI "} {
t.Run("type="+written, func(t *testing.T) {
ws := model.ValidateEgresses([]model.Egress{{Name: "bd", Type: written}})
if len(ws) != 1 {
t.Fatalf("want exactly 1 finding for a retired egress kind, got %d: %v", len(ws), ws)
}
if ws[0].Section != "egress" || ws[0].Name != "bd" {
t.Fatalf("finding = %+v, want section \"egress\" name \"bd\" — the panel groups and deep-links by these", ws[0])
}
if strings.Contains(ws[0].Message, "is not one of") {
t.Fatalf("a retired kind was reported as an unrecognised value:\n%s\nThat is the message this whole change exists to replace: it tells someone whose config was correct to go looking for a typo.", ws[0].Message)
}
assertRetiredFacts(t, "ValidateEgresses", ws[0].Message)
})
}
}
// TestRetiredEgressTypeIsReportedByTheGenerator is the engine half. The
// generator is where an apply is actually attempted, so this is the message the
// operator sees at the moment their change is refused.
func TestRetiredEgressTypeIsReportedByTheGenerator(t *testing.T) {
m := &model.Model{
Globals: plainGlobals(),
Egresses: []model.Egress{{Name: "bd", Type: "byedpi"}},
}
opts, warns, err := GenerateWithWarnings(m)
if err != nil {
t.Fatalf("Generate: %v", err)
}
got := warnMatching(warns, `egress "bd"`, "was REMOVED from this product")
if len(got) != 1 {
t.Fatalf("want exactly 1 removal warning naming the egress, got %d: %v", len(got), warns)
}
if len(warnMatching(warns, `egress "bd"`, "unknown type")) != 0 {
t.Fatalf("the retired kind ALSO drew the unknown-type warning: %v", warns)
}
assertRetiredFacts(t, "GenerateWithWarnings", got[0])
// Fact 1: still fail-closed. The sentence changed; the behaviour must not
// have. Without this the test would pass on a build that had quietly given
// `byedpi` a direct outbound and merely printed a warning about it — which is
// the failure mode with the worst consequence, since it puts the traffic the
// operator was protecting onto the plain WAN under a reassuring message.
if ob := obByTag(opts, netplane.EgressOutboundTag("bd")); ob != nil {
t.Fatalf("a retired egress kind emitted outbound type %q — it must emit NOTHING, so every binding to it is blocked", ob.Type)
}
if model.EgressTypeKnown("byedpi") {
t.Fatalf("model.EgressTypeKnown(\"byedpi\") = true — a retired kind must not be a known one; being told it was removed and having it work anyway is worse than either alone")
}
if model.EgressHasDevice(model.Egress{Name: "bd", Type: "byedpi", Interface: "eth1"}) {
t.Fatalf("a retired egress kind resolved to a device — the data plane would then install a mark, an `ip rule` and a routing table for a kind the engine builds nothing for: the exact split-brain egress the canonical type set exists to prevent")
}
for _, known := range model.KnownEgressTypes {
if known == "byedpi" {
t.Fatalf("model.KnownEgressTypes still offers %q — the panel builds its type picker from this list", known)
}
}
}
// TestRetiredEgressTypeDoesNotSwallowUnknownOnes is the CONTROL for the two
// tests above. They prove the retired kind gets its own sentence; this proves
// the instrument can still tell the other answer apart — that the removal branch
// did not become a catch-all quietly claiming every bad type was once supported.
// A removal sentence on a value that never shipped names a replacement for
// something that never existed.
func TestRetiredEgressTypeDoesNotSwallowUnknownOnes(t *testing.T) {
for _, written := range []string{"proxy", "block", "wireguard", "byedpi2", "bye dpi", "sorcery"} {
t.Run("type="+written, func(t *testing.T) {
ws := model.ValidateEgresses([]model.Egress{{Name: "u", Type: written}})
if len(ws) != 1 {
t.Fatalf("want exactly 1 finding, got %d: %v", len(ws), ws)
}
if strings.Contains(ws[0].Message, "was REMOVED from this product") {
t.Fatalf("type %q was reported as a REMOVED kind:\n%s\nIt was never one, and the message tells the operator to replace something they never had.", written, ws[0].Message)
}
if !strings.Contains(ws[0].Message, "is not one of") {
t.Fatalf("type %q drew neither the unrecognised nor the removal finding: %v", written, ws[0])
}
if _, retired := model.RetiredEgressType(written); retired {
t.Fatalf("model.RetiredEgressType(%q) claims a retired kind", written)
}
})
}
}
// TestSupportedEgressTypesAreUntouched is the other half of the control: the
// kinds that survive must keep working, silently. A removal that also broke
// `interface` or `direct` would be caught by half the suite, but not by anything
// that reads this file — and the point of a control is that it lives beside the
// claim it qualifies.
func TestSupportedEgressTypesAreUntouched(t *testing.T) {
cases := []struct {
written string
device bool
}{
{"interface", true},
{"tunnel", true}, // the accepted alias, folded to interface
{"direct", false},
{"", false}, // the documented synonym of direct
}
for _, c := range cases {
t.Run("type="+c.written, func(t *testing.T) {
eg := model.Egress{Name: "e1", Type: c.written, Interface: "eth1"}
if _, retired := model.RetiredEgressType(c.written); retired {
t.Fatalf("a SUPPORTED type was classed as retired")
}
if !model.EgressTypeKnown(c.written) {
t.Fatalf("model.EgressTypeKnown(%q) = false", c.written)
}
if got := model.EgressHasDevice(eg); got != c.device {
t.Fatalf("EgressHasDevice = %v, want %v", got, c.device)
}
if ws := model.ValidateEgresses([]model.Egress{eg}); len(ws) != 0 {
t.Fatalf("a correctly-written %q egress drew findings: %v", c.written, ws)
}
m := &model.Model{Globals: plainGlobals(), Egresses: []model.Egress{eg}}
// Through the production boundary. `tunnel` is an ALIAS resolved once,
// here, and nowhere else — the generator's switch deliberately knows only
// canonical spellings, so a Model that skipped this step is not the Model
// the daemon ever builds from. Calling it is not a workaround; leaving it
// out would be testing a path production does not take.
m.NormalizeEgressTypes()
opts, warns, err := GenerateWithWarnings(m)
if err != nil {
t.Fatalf("Generate: %v", err)
}
if len(warns) != 0 {
t.Fatalf("unexpected warnings: %v", warns)
}
if obByTag(opts, netplane.EgressOutboundTag("e1")) == nil {
t.Fatalf("no outbound was emitted for the supported type %q — everything bound to it would be blocked", c.written)
}
})
}
}