bbb493ea9198f56d960acfc8138f2d615b401895
3151
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  omarandClaude Opus 5
|
06c04c157d |
fix(gate): a unit test in one package was deleting another package's TUN
`go test` runs package binaries CONCURRENTLY and every one of them shares the
host's network namespace. netplane.L3SlotFor is destructive by design — it
DELETES a candidate slot it finds occupied rather than waiting for it — and
netplane.removeL3Devices deletes both slots unconditionally. Two test binaries
reached those for real:
shater/engine l3slot_test.go calls l3RetargetForNext for its return value
shater/apply Applier.Teardown -> netplane.TeardownRouting -> removeL3Devices
Measured with an `ip` shim on PATH inside the gate container: apply.test issued
9 `ip link del shater-l3a` + 9 `ip link del shater-l3b` per run, engine.test one
per l3slot test — into the namespace where shater/generate's privileged tests
were holding a live TUN. From the other side that is
post-start inbound/tun[l3-in]: starting TUN interface: find tun interface: Link not found
no [shater-l3a shater-l3b] device exists after a successful Start
i.e. an intermittently red [2/7]/[4/7] in a package that did nothing wrong,
while [5/7] — which runs only `^TestIntegration`, so neither binary reaches the
slot code — passed the very same test seconds later. It only became visible when
iproute2 was installed into the gate container: without `ip` every slot read as
free and no deletion was ever issued.
Not a product defect. shaterd is one process with one engine; the running
generation's slot is excluded before anything is deleted, and nothing else on
the router calls L3SlotFor.
The kernel is faked rather than the CHOICE: making the engine's tests stub the
slot answer would delete the only place the ENGINE checks that the running
generation's slot is excluded, which is the invariant the production outage
violated. netplane.L3StubKernelForTest points the two kernel operations at an
in-memory set; engine and apply install it from TestMain (forget-proof, unlike a
per-test helper whose omission fails in a different package on some runs only).
netplane's TestL3StubKernelTakesTheSlotChoiceOffTheKernel is the control, in
both directions: stubbed, nothing reaches the exec seam; restored, the same call
does.
Mutation: with the engine TestMain reverted, the generate binary's
TestIntegrationL3* failed 8 of 8 runs beside a loop of the engine binary; with
it, 0 of 8. With L3StubKernelForTest degraded to a no-op, the control fails
naming the three escaped `ip` calls.
Also: the DoH3 ownership test's control now retries.
requireInstrumentFindsPackedQuery packed a query into a pooled buffer, released
it and demanded the scan find it — but under -race sync.Pool.Put drops one
object in four on purpose, so the control failed 18 of 60 measured runs and took
the whole -race pass down with it. Its sibling control in the same file already
retried for exactly this reason. The claim is existential ("this instrument CAN
find a released buffer"), so one success out of 32 proves it and nothing is
diluted; 0 of 60 after. What it does not buy is stated in the code: the VERDICT
is still a 3-in-4 detector under -race, which is the safe direction, and the
non-race pass runs the same test as a certainty.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
3654acf7fb |
fix(egress): tunnel was a device to the router and an unknown type to the engine
An egress type was read by two halves that never call each other. netplane's EgressDevice accepted `tunnel`, so addEgressRouting gave it a mark, an `ip rule`, a routing table with an unreachable floor and a prerouting mark bypass, and `untunnelable_egress` (D26) carried ESP/AH/GRE/IGMP/SCTP out of it by kernel routing with the engine nowhere in the path. generate's outbound switch had never heard of `tunnel`: default arm, no outbound, so every node, group and rule bound to the same egress was fail-closed. One name, two answers. Refusing `tunnel` would have broken the half that works to match the half that does not — D26's kernel egress is shipped and verified, and the generator's refusal is already loud and fail-closed. `tunnel` is not a distinct kind either: the data plane treats it identically to `interface` in every line that mentions it, and the panel's own `interface` label already reads "out a specific WAN or tunnel". So it is an ALIAS, and it is folded to `interface` ONCE, at the config boundary (Model.NormalizeEgressTypes, called by ParseUCIExport/ReadUCI). Teaching the generator a second string would have left two strings for the next consumer to forget; after the fold there is one. - model: CanonicalEgressType / EgressTypeKnown / KnownEgressTypes — a closed, positive registry, plus NormalizeEgressTypes on the load path. An unrecognised type is left as written, never defaulted: substituting `direct` for a typo would send traffic somewhere nobody asked for. - model: ValidateEgresses now NAMES an unknown type at validate time. Until now the only notice was a generator warning raised while building an engine config, which said nothing about the data plane — and the two disagreed anyway. - netplane: EgressDevice and the prerouting mgmt-bypass consult the registry instead of carrying their own copies of the rule. The bypass now keys off EgressDevice, so a device-kind egress with no interface no longer gets an accept for a mark addEgressRouting never installs. - panel: the egress editor cleared Interface/Port/DPI for every type it had no branch for — including types it renders no field for — so opening an egress it labels "(unknown)", changing only the NAME and saving deleted its `interface`. On a `tunnel` egress that silently unbound untunnelable_egress and dropped the ESP/GRE carrier back to policy. A save may now only clear a field the editor was in a position to show. - panel: the unknown-type hint said "This engine builds no outbound for that type", which was false for the one unknown type anybody had — the data plane was building it a routing table at that moment. It now names both halves and states what saving does. Tests: TestEgressTypeMeansTheSameInBothHalves runs one table of written types through the real boundary and then asks netplane AND generate, requiring one verdict (external test package: generate imports netplane, so nothing inside netplane can import generate). Mutation-checked both ways — dropping the fold fails on `tunnel`; restoring the old EgressDevice string test reproduces the historical split with "generate emitted outbound egress-probe = false ... want true". Panel: egressEdit.test.ts, mutation-checked by restoring the unconditional clear (Interface undefined, want 'wg0'). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
3a9b3f523d |
fix(l3): a test that is not about the TUN must not open one
The l3_tunnel default flip (
|
||
|
omarandClaude Opus 5
|
201aa7c168 |
fix(panel): traceroute never printed a hop — stop saying it works
Five untunnelable notes told the operator that a plain `traceroute` works, "still follows your rules", or that the hops it prints are the tunnel's path. Measured on the production router: it prints `* * *` and nothing else, under every rung of the ladder — `direct` included — with the L3 ingress on or off. There is no mechanism that could print a hop. The UDP probe is diverted by tproxy and delivered LOCALLY to the engine's socket; local delivery is not forwarding, so the TTL is never decremented and no router on the path is provoked into a time-exceeded. The engine opens its own connection with a fresh TTL, and an ICMP error raised against that has no way back to the client's datagram. `traceroute -I` and Windows `tracert` are ICMP echo and do work — that half of the text was true and is kept. One shared udpTracerouteFacts now carries the symptom, the cause and the way out, so the panel cannot fork the claim; netplane/untunnelable.go states the same fact in the same terms. Second correction in the same notes: the outbounds that carry an echo are not just WireGuard/AmneziaWG. generate/route.go's l3Target is exhaustive by adapter registration — a wireguard/AWG node AND the direct outbound behind `direct` or an interface egress. In the commonest configuration here that is most of the address space, and those pings answer out of the ordinary uplink with its real address. The old text let an operator conclude either "tunnelled" or "dropped"; it was neither. traceroute_honesty_test.go is the ratchet: an exhaustive matrix over policy x kill switch x L3 x egress, asserting the retired sentences never return and that any note mentioning a trace carries the shared facts verbatim — with a control that fails if the matrix stopped mentioning tracing at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
78bb6a1be8 |
fix(netplane): a read that fails, a floor nobody checked, a flow that predates the plane
Four defects, all of the same family: something the plane relies on stops being true and nothing says so. 1. One failed `uci -q export firewall` opened a hole AND switched off the alarm for it. nftZoneDevices answered nil on a read failure — the same answer as an empty zone — so a rule with `src: zone:lan` produced no divert line, no fail-closed drop and no accept_local; and uncoveredNetworkWarnings, whose job is to report exactly that, ran the same command, got the same nil and stayed silent. The read now carries its error: renderNft refuses under a closed kill-switch (same contract as an unusable device name) and warns under an open one, and the coverage check names the blindness itself. 2. RoutingPresent did not check the fail-closed floor its Apply twin installs. addEgressRouting/addL3Routing install three things per binding; the presence checks knew two. A floor that failed to install once was never retried, and the table fell through to `main` the first time its device went down. The checklist test grows clause (e) so the next mark cannot repeat it. 3. A flow established before the divert plane existed bypassed it for life: confirmed by conntrack while nothing diverted it, offloaded to fw4's flowtable, steered by netdev-ingress ahead of our prerouting hook and refreshed by its own packets. On the divert going from ABSENT to PRESENT — not on every apply — the TCP/UDP entries of flows forwarded from the divert devices' subnets are dropped, so they re-derive their path. Not a flush: the router's own addresses and LAN-to-LAN are excluded, so SSH, LuCI and the panel survive. Measured on the stand: 3 client flows cut, the live SSH session and the router's own connections untouched; `conntrack` CLI confirmed absent there, which is why this is ctnetlink. 4. The untunnelable text claimed Linux/macOS traceroute "still prints hops". It prints none, under any policy: the UDP probe is delivered locally by tproxy, local delivery does not decrement TTL, and no router raises time-exceeded. `traceroute -I` is what works. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
ea3a4c518e |
test(generate): the L3 ingress is the default now — say so in the fixtures, not in 51 rewrites
The l3_tunnel default flip (
|
||
|
omarandClaude Opus 5
|
0f69880150 |
test(gate): a skipped test is a test that did not run — name it, or fail
Three holes, one shape: work that reads as coverage and is not. 1. shater/apply's TestApplyInstallsHoldWhenEngineFailsToStart — the only end-to-end test between "the engine died" and "the LAN forwards to the WAN in the clear" — asserted nothing. It broke the engine by pointing a rule-set at /nonexistent/nope.srs and stood itself down with t.Skip when that failed to break anything; it stopped breaking anything once LocalRuleSet.reloadFile began treating an unreadable file as empty. Measured in golang:1.26: the skip fired unconditionally and the package still printed `ok shater/apply`. It now injects the failure at the engineApply seam — the branch under test is applyLocked's, and a particular cause that stops causing retires the test silently — and COUNTS the seam calls, so applyLocked ceasing to go through it fails by name instead of quietly asserting something else. Everything else stays real: the model, generate, the kill-switch decision, netplane.RenderHoldNft, the latch, Status. New companion TestEngineApplyReallyFailsWithoutStarting is the control that the real engine.Apply can fail with the engine left stopped, so the simulated state is one this fork can be in. Mutation-checked both ways: drop the holdLocked call from applyLocked and the test fails with "0 holding planes were installed, want 1"; bypass the seam and it fails with "the engine-swap seam ran 0 times, want exactly 1". 2. warnings_test.go had two of the same genre. The len(genWarnings)==0 t.Skip is now a t.Fatal — an unloadable blocklist must always warn, and a generate that stops saying so is the W7 regression, not a reason to stand down. TestStatusWarningsAlwaysNonNil pins readConfig itself: its "zero warnings" assertion was true on a build host only because the config read failed SILENTLY, so once that failure started publishing a critical warning the same line meant two different things in two environments. 3. The gate could not see any of it. It now runs the suites with -v and matches every `--- SKIP` against SKIP_DECLARED; an undeclared skip fails BY NAME, a declared one prints its reason on every run. check_skips proves its own instrument first (no `=== RUN` line => the check was reading a blank page), and it also reports on a suite that failed elsewhere, so a red tree cannot become a hiding place. -v costs no test time (38/25/24 s plain vs 38/24/24 s, warm) — only output, which is filtered on a green run. Also closes the same hole one language over: [6/7] requires every non-Go test file in the tree to be claimed by a named runner, and [7/7] runs the ones this gate owns with a verdict by name. openwrt/luci-app-shater/tests/ status-readout.test.js — 24 assertions over the one screen an operator reaches while the LAN is cut off — was executed by nothing at all, and [1/7] could not report it because `go list` is its instrument. The non-Go suites run on the HOST before the docker re-exec, so the local loop really executes them rather than printing "did not run" every time; where there is no node at all they are named and the notice replaces the closing banner. Controls, all run and reverted: a planted t.Skip is caught and named; a planted failing .test.js is caught and named; an unclaimed test file is caught and named. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
164b703a7d |
feat(l3): ping travels the tunnel by default, and every LAN zone can reach it
l3_tunnel was opt-in, and "off" had no honest win left in it. Off, a LAN ping is decided by `untunnelable` alone and every rung is a drop (block) or a disclosure (icmp/direct send the echo out of the WAN with the client's real address). "Ping works" was never the state where ping was tunnelled — it was the state where ping was leaking. On, an L3-capable outbound carries the echo and one that is not drops it honestly: adapter.JudgeFlow returns ActionDrop for an ICMP flow whose outbound is not a tun.Port, so no reply is forged. The price is a standing TUN + gVisor netstack, ~2 MB RSS, and it is stated where the option is. The switch stays. It is a real answer on a 32/64 MB device and when bisecting whether the L3 ingress is what broke a box — but it is now a WARNED answer: ValidateGlobals says what the off state does to ping and names the policy that takes over. Two combinations also changed meaning and are now reported: untunnelable=icmp is no longer "block plus working ping" (the prerouting L3 mark claims every ICMP packet before the forward chain the echo accept lives in, and a LAN host's ICMP errors are marked in with them and dropped in the TUN), and the existing =direct report gains a sibling rather than standing alone. The fw4 seeding was the second half of the same problem. The divert set spans every LAN inbound and every iface:/zone: rule source, but 30_shater-core seeded a forwarding into shater_l3 for `lan` only — so on a multi-zone router ICMP from the other zones is marked, routed, accepted by `inet shater`, and dropped by fw4's zone policy with nothing in any log. Every zone gets a forwarding now, guarded by a scan of the actual src/dest pairs so a re-run adds nothing. Every zone including an uplink, because guessing which zones hold clients is wrong somewhere and a superfluous entry authorises nothing: accept_to_shater_l3 is `oifname "shater-l3*" accept`, and the only thing that routes a packet into that device is our own fwmark rule. scripts/testbed-lao.sh builds the second LAN zone this needs to be visible at all. It is not installed by the package — that is the whole opt-in mechanism. Verified on local_openwrt (ImmortalWrt 25.12.1 r37978): three runs of the seeder leave exactly one forwarding per zone (lan/wan/lao) and no existing section altered; deleting the lao forwarding removes `jump accept_to_shater_l3` from chain forward_lao and re-seeding restores it; with the idempotency guard disabled two runs produce nine forwardings instead of three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
de6fa8ebf4 |
fix(doh3): Close is not an ownership handoff — stop pooling the query buffer
Review found the hole and it is real. My previous fix gave the pooled buffer to the transport and released it when the transport closed the body, on the grounds that "http3.Transport closes the request body on every path, hence the Once". That sentence is true about how many times the body is closed and says nothing about when — the failure mode this project keeps writing down. Verified against the pinned quic-go: on every error path RoundTripOpt (http3/transport.go:167-173) closes the body the moment doRequest returns, and doRequest (http3/client.go:338-341) waits only on the request-CANCELLATION watchdog — close(reqDone); <-done — never on the goroutine writing the body. Nothing in quic-go joins that goroutine. So Close is not a handoff point, and the sync.Once stopped a double Release while doing nothing about a read after one. One correction to the review's severity, since it changes what we tell people: on the failure path the bytes do not reach the resolver. Every ReadResponse error branch (http3/stream.go:325, :336, :343, :363) calls str.CancelWrite BEFORE RoundTripOpt closes the body, so what the writer reads out of the recycled buffer is thrown at a cancelled stream. The disclosure primitive is the success path only; the failure path is a read of somebody else's memory, which is undefined behaviour and a -race finding, and not shippable either. Fixed by not sharing at all: Pack() into memory the body owns. The alternative — a lock around Read and Close — would also be correct and was rejected because it keeps a released-but-referenced object alive, and that is now twice in one day that an assumption about quic-go's internal lifetimes has been wrong. The cost is negative, measured rather than assumed: Pack is 87 ns/op at 64 B and 1 alloc against 108 ns/op at 64 B and 1 alloc for the pooled version, because buf.NewSize allocates the Buffer struct itself — the same 64 bytes — and then adds Get/Put on top. The pool was never saving an allocation here. The failure path cannot be caught on the wire, so the new test pins the cause: a query tagged with a random needle, an exchange that fails (server never answers; context already cancelled), then the pool drained on the goroutine RoundTripOpt ran on, demanding the needle is not there. Mutations run without -race: restoring pooledRequestBody fails both subtests 5/5, and blunting the scan trips its control. -race is a separate pass, green at -count=3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
b91fba1295 |
fix(l3): a covered last fragment must complete; say what the timeout really does
Two review findings on the fragment reassembler. 1. A whole datagram could vanish. entry.total was assigned before addRange was asked, so a last fragment (MF=0) whose range was already covered by MF=1 fragments answered fragInsertDuplicate and returned nil — while the entry was already complete(). Nothing re-examined it, because every later fragment is a duplicate too, so it died at its deadline with all its bytes present. A duplicate now falls through to the completion check: it contributes no bytes (held bytes still win) but it does contribute the total length. This is what the documented first-wins policy always implied; the code just did not do it. The sender needed is non-conforming, so the old behaviour was safe rather than exploitable — but it contradicted the comment three screens up, and that comment is the next reader's only defence. Also closed positively: a last fragment declaring an end BELOW the bytes already held now poisons the datagram instead of quietly never completing. 2. The 5 s timeout was not a memory ceiling and the comment said it was. sweep ran only when a NEW key was created, so once fragmented traffic stopped, up to fragMaxEntries entries stayed resident indefinitely. Both halves are fixed, and the honest one is the comment. sweep now runs on EVERY fragment — an O(64) scan on a path that is already the rare one — which releases residue as soon as any fragment arrives instead of waiting for an unrelated new datagram. That still does not cover total silence, so fragTimeout now documents the guarantee the code actually keeps: bounded by fragMaxEntries/fragMaxTotalBytes at all times, released on the next fragment, NOT "freed within 5 s". No timer, deliberately: it would need a goroutine with a lifecycle tied to something returnDeviceWrapper has no teardown hook for, and a goroutine that must be stopped and might not be is a failure this project has already paid for — to reclaim at most ~1.1 MiB that only exists after fragmented traffic has already happened. What bounds growth is the byte and entry ceiling; this timeout's job is correctness, and for that a check driven by the arriving fragment is exact. The now-unreachable per-key deadline check is removed rather than left as dead defence in depth. 16 mutations, all red. M15 (duplicate returns early again) reds only the buggy case while the control and the poison case stay green, so the test is shown able to see both an assembled datagram and a lost one. M17 (sweep back inside the new-key branch) reds the new test while both old timeout subtests stay green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
df078c3205 |
fix(model): the write rollback may not swallow its own failure
The rollback added for the "failed import commits the deletion" defect went
through migrate.go's staged(), which drops the revert's error on the floor
(`_ = u.Revert("shater")`). That is defensible where staged() lives — a
migration that cannot revert leaves a half-migrated config, wrong but visible —
and it is not defensible here, because the delta this path stages STARTS WITH A
DELETE OF THE WHOLE PACKAGE. A revert that silently does not take leaves that
delete in /tmp/.uci, the caller is told only "import failed" and believes
nothing happened, and the next `uci commit shater` from any process publishes
an EMPTY /etc/config/shater. The guard reintroduced the exact loss it was
added to prevent.
writeUCIWith now uses its own revertStagedWrite, which reports both failures.
migrate.go's staged() is untouched: changing its signature to suit this caller
would rewrite a contract three migration paths depend on, for a hazard those
paths do not have.
The wrapped error names the CONSEQUENCE and the one command that clears it
("a staged DELETE ... will publish it ... run `uci revert shater` NOW"), not
just the fact — "revert failed" tells an operator nothing about what it costs.
ErrStagedWriteStuck makes it machine-detectable, so a caller can tell "your
change did not happen" from "your change did not happen and this router is one
unrelated `uci commit` away from an empty config".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
d0471b2418 |
build(shater-core): ship the keep.d entry, or the node inventory dies at the next flash
files/ is not installed wholesale — every path in Package/shater-core/install is explicit — so the keep.d file added alongside it would never have reached a router. sysupgrade's "keep settings" walks /lib/upgrade/keep.d/*, and without this entry /etc/shater/subs does not survive a flash: the restored box has its rules and its groups and no nodes for them to point at, and the only repair is `sub update`, which needs the internet the tunnel was going to provide. /etc/config/shater needs no entry — it is a package conffile and sysupgrade already keeps it that way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
3314927bef |
fix(panel): stop the readouts claiming things the daemon never said
Nine places where the panel asserted more than it could know. Each was
checked against the daemon before being changed, and the two that a test
can reach are pinned by tests proven with a mutation.
MULTICAST IPTV WAS AN INSTRUCTION, AND IT WAS WRONG. The `direct` rung
said "Ping, multicast IPTV, and connecting to a VPN ... all work", so
someone who wanted IPTV read it and moved to the most open setting on the
ladder — the one that also lets a client's ESP/GRE past the proxy — and
still had no IPTV. The stream is UDP; every rule the policy emits carries
`meta l4proto != { tcp, udp }`, and the fail-closed forward chain accepts
only the RFC1918/link-local daddr sets, with no 224.0.0.0/4 among them.
The daemon says so itself in the note drawn a few pixels below. IPTV is
now stated once, and it says it does not work.
THE `block` COST LINE WAS UNCONDITIONAL, and three settings contradict
it: an open kill-switch (no drops are emitted at all), Globals.L3Tunnel
(ICMP is marked into the engine's TUN before the forward chain) and
Globals.UntunnelableEgress (ESP/AH/GRE/SCTP are routed out a named
device). The last two were not in the panel's `Globals` type, so the page
could not have been honest about them even in principle; they were added
rather than papered over with a vaguer sentence, and the copy is now
derived from all three.
THE KILL-SWITCH WAS READ WITH `=== 'closed'`. The daemon decides with
!EqualFold(TrimSpace(v), "open") and `Status.kill_switch` is the raw UCI
string, so `'Closed'`, `' closed '` and `''` — all of which BLOCK on the
router — drew OPEN, amber, "Nothing is meant to be blocked", and through
protectionState downgraded a plane-less router from crit to amber. One
normaliser now, `planeState.killSwitchClosed`, used by all five callers
that had their own spelling of it.
AN UNREADABLE CONFIG IS NOT "TURNED OFF". `enabled`, `kill_switch` and
`panel_port` are sourced from the config and are placeholders when it
could not be read (new `config_readable`). That happens on a full
/overlay or an interrupted `uci commit` — exactly when the fail-closed
plane has the LAN cut off on purpose — and the daemon publishes
plane:"hold" with enabled:false. Checking `!enabled` first rendered
"Turned off", amber, no alarm, and pointed at a Settings page backed by
the same unreadable file. The check now comes first, carries the daemon's
"do not turn anything off to fix it", and the kill-switch readout refuses
to name a policy it could not read instead of printing ARMED from "".
Also: the holding plane promises "no client TRAFFIC reaches the WAN", not
"nothing" — DNS to the router still goes to the ISP in the clear, by
design, so the daemon can recover; the stats backend is bbolt, not SQLite,
and reclaims space by rebuilding the file, not by a VACUUM that does not
exist (and skips it when the disk cannot fit the copy); the lock screen
sent people to System → shater when the menu entry is admin/services/shater,
which is the one instruction the product gives to someone who has just
lost access; and the panel port is configured, not confirmed — a failed
listen is only a log line.
RULESET.FORMAT WAS DESTROYED BY RENAMING A LIST. The edit form rebuilt
the object from its own controls and has no control for `Format`, so the
value could only be restored over SSH. It decides how a `file` list is
parsed and stops a `url` .srs being read as text; without it the list
matches nothing, the rule stops firing, and the traffic falls silently
through to the next rule. Carried now for the two sources the generator
consults it for. The same class of loss is made loud elsewhere: the two
other rebuild sites return `Complete<T>`, so adding a field to `Inbound`
or `DNSRule` fails the build in the function that has to decide.
Egress.Target is deleted: it is not in the Go model, so the "which egress
points at this node" branches could never fire, and had anything ever put
a string on it PUT would have rejected the whole write under
DisallowUnknownFields.
One layout fix on the way past: at 390px the policy plate's grid column
was sized by the select's longest option, so the sentence beside it was
clipped mid-word — which is how a line about what leaks loses its second
half.
Verified: npm run build + tsc clean; 57 tests pass; mutation-checked by
restoring the old comparison, the old check order and the old rebuild in
turn, each time watching the matching tests fail with the exact inverted
reading; browser-checked at 390 and 1280 against the mock, which now
reproduces `?ks=Closed` and `?cfg=unreadable` verbatim instead of
normalising them out of existence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
6801146240 |
feat(core): back up the product state, and let the watchdog see a crash loop
Two things the box could not survive, both silent. BACKUPS CARRIED NOTHING. No shater package put a single entry in /lib/upgrade/keep.d, so "keep settings" and LuCI Backup took /etc/config/shater (a conffile) and nothing else. Everything the product knows besides UCI lives in /etc/shater: the entire node inventory (subs/*.json, hundreds of nodes on the live router), the boot-armor arm token, the compiled blocklists. Restored onto a new router the config looked complete and had no nodes to route to — and the repair, `sub update`, needs the internet the tunnel was supposed to provide. keep.d/shater-core keeps subs/, boot.nft, lists/ and alert-state.json, and names what it refuses and why: stats.db is history bounded only by stats_disk_limit_mb (0 = unlimited) and the archive is built in RAM; cache.db is sing-box's cache and a stale one is worse than none; shaterd.log is a log carrying the query history of the box it came from. THE WATCHDOG COULD NOT SEE A CRASH LOOP. /etc/init.d/shater respawns every 5s, forever; shater-cron escalated only after five consecutive ticks where `pidof` found nothing. A daemon dying seconds into startup is back before the next 60s sample, so the counter reset every time — while the fail-closed plane held the LAN shut and the panel, served by that daemon, never came up. The tick's sleep is now spent sampling the daemon's identity (via its pidfile, not `pidof`, which also matches the CLI verbs this loop runs) every 5s. A tick in which 3 different daemons lived is churn; two such ticks in a row is the verdict. A legitimate bounce replaces the daemon once and is announced twice over (RESTART_FLAG up, ACTIVE_FLAG down), either of which discards the tick. The action is the one the operator already chose: kill_switch=open stops the stack, exactly as the dead-daemon path does; kill_switch=closed — and an absent or unrecognised value, which is the documented default — reports at daemon.crit and leaves the decision to the person, naming the command that opens the LAN. Also drops the ruleset loop from shater_run_due. `shaterd ruleset update` has never existed; it exited 0, so the loop stamped every url rule-set as freshly updated and fired a reconcile for work that never happened. Now that it exits non-zero the same loop would emit ~288 syslog lines a day per rule-set instead. The comment says who does own the refresh, and where the gap that is left is. Verified: sh -n and busybox `ash -n`; the pure detector driven with synthetic sample streams under busybox ash (13 cases); shater_sample_pid against a real /proc with a live process named shaterd as the positive control; and the whole chain end to end against a real 2s-lifetime crash loop. Each threshold and each veto is pinned by a mutation that makes the gate fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
2f8c692c39 |
fix(l3): the TUN is a reclaimable slot — one fixed name made every apply fatal
On the production router every configuration change with l3_tunnel=1 killed the engine and held the LAN down, three times in a row: 19:10:33 reconcile failed: start inbound/tun[l3-in]: open tun: TUNSETIFF: device or resource busy 19:14:02 start instance failed and could not restore previous config; engine stopped 19:14:38 reconcile failed: TUNSETIFF: device or resource busy A new generation had to open the device the outgoing one still held. That alone is a failed apply; what made it an outage is that the recovery path rebuilds the PREVIOUS config, which named the same device — so the rescue failed for exactly the reason it was needed. A recovery path must not depend on the resource whose contention it is recovering from. The device is now one of two slots, chosen by the ENGINE at box-build time, on a copy of the options taken AFTER the hash — so the stored config stays canonical and a no-op reconcile is still a no-op. It cannot be chosen in generate: generate runs every minute and its output is what Apply hashes, so an alternating name there would rebuild the engine once a minute forever. Rotation alone was NOT enough, and that was measured, not reasoned: the two-slot build survived five applies of five kinds and then failed on 4 of 10 back-to-back changes with the original outage in full, because a retired generation keeps its TUN until its budgeted Close finishes. So an occupied non-current slot is now DELETED rather than waited for — the running generation's slot is excluded first and never touched, every other slot belongs to a box that is carrying nothing. No bounded wait: waiting on an asynchronous kernel teardown is the race this design removes. The firewall never learns which slot is live — our accepts and the fw4 zone match `shater-l3*`, verified to validate AND load on ImmortalWrt 25.12.1 / nftables 1.1.6, so the ruleset is byte-identical across a swap. Routers seeded by a pre-slot build are migrated in place, or fw4 would silently resume dropping the forward. A2: turning the feature off left the device, the ip rule and table 8200 behind — addL3Routing returned early instead of tearing down, and nothing else owns that device. The disabled branch and TeardownRouting now remove all three. Two smaller lies found while proving this, both measured: `ip -6 route flush` does not take a non-unicast route, so the fail-closed floor survived and the next add answered `File exists` — reported as a CRITICAL "this table has no floor, traffic can leave over the plain WAN" on every apply, about a floor that was right there; and teardown left it behind. Fixed both. Verified on local_openwrt (ImmortalWrt 25.12.1, kernel 6.12.94 — the router's revision) before and after, with binaries built from the same tree: the pre-fix binary reproduces the outage and the leftovers; the fixed one survives all five apply kinds and 12 back-to-back changes and leaves nothing behind. Ten reverted mutations, each shown failing. See D28. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omar
|
c788425cad |
feat(stats): the connection log now says which rule sent it there
The tracker has carried the matched route rule and the outbound chain since upstream (common/trafficcontrol/tracker.go Rule/Chain); nothing in shater/ ever read them, so "why did this connection go out that exit" was unanswerable from the log and cost hours per report. ConnLogEntry gains RuleKind/Rule/Chain. Rule is the engine rule text, not the model rule name: nothing survives generation that ties an emitted option.Rule back to the /etc/config/shater rule it came from, and a guessed name would be worse than none. RuleKind keeps the two empty cases apart — "default" is a recorded fact (nothing matched, took route.Final), "" means not recorded at all, which is what an old persisted row decodes to. Both fields are interned, so the ring pays 56 B/row of headers instead of a private copy of text that is identical across every connection one rule matched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS @ |
||
|
omarandClaude Opus 5
|
0144282f5e |
fix(apply): a finding that is still true may not erase itself
Three ways this package published calm over a router that was not doing what
its config said. All three are the inverted failure: not an error raised when
things are fine, but silence when they are not.
1. Critical policy-routing findings were erased by the next no-op reconcile.
applyDataPlaneLocked set routeWarnings only on the full path; applyLocked
published the set unconditionally, so a minute later the fast path replaced
it with one that no longer contained the finding. Neither surviving finding
("this egress CANNOT REACH ANYTHING outside its own subnet", "table could
not be given a fail-closed floor") makes RoutingPresent false, so nothing
brought it back: zero findings, plane full, green, over an egress carrying
nothing. The comment on the gate claimed the previous set stood; it did not.
planeOutcome now distinguishes "nothing was found" from "nothing was
checked" (routeMeasured, written only by measuredRouting), and applyLocked
carries the last MEASUREMENT forward across the fast path. A re-measurement
still retires a finding, so this is not a latch.
2. An unreadable configuration was published as enabled=false. The panel tests
!enabled before plane and renders "Turned off", amber, no alarm, "turn it on
in Settings" — over a LAN the boot armor had cut off, pointing at a settings
page backed by the same unreadable file. Status now carries config_readable
and config_error, plus a critical finding in section "config".
3. The reason the engine failed to start existed nowhere. holdLocked logged it
and called no publisher, and Warnings carries the last SUCCESSFUL apply — so
plane="hold" with an empty findings list was a normal state of the product.
The cause is recorded and published at read time while the engine is down,
so it self-clears when the engine comes up; the boot-time arm is a warning,
a real failure is critical.
Each fix is mutation-checked, and the route-warning test carries its control:
it sees a live finding, sees it survive the fast path, and sees a re-measured
clean state retire it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
b642e5d8fe |
fix(egress): an interface egress with no interface was bound to br-lan
generate/outbound.go resolved the bind device with netplane.IfaceDevice,
whose empty-name fallback is "br-lan" — correct for an INBOUND with no
network, a black hole for an egress. netplane.EgressDevice returns "" for
the same egress on purpose (it calls br-lan "catastrophic here"), so
addEgressRouting installed no `ip rule` and no routing table for that
egress's mark, and the prerouting marking and the forward-chain accept
skipped it too.
The outbound was therefore emitted with SO_BINDTODEVICE=br-lan and a
routing mark nothing routed: every node, group and rule bound to that
egress dialled public addresses out of the LAN bridge. Not a leak — the
bind pins the socket to the LAN — but a total, silent black hole, with the
panel showing a configured, applied egress and no findings at all. The
`if dev == "" { dev = eg.Interface }` line that stood there read as a
guard against exactly this and could never execute: IfaceDevice never
returns "".
- generate now calls netplane.EgressDevice — the data plane's own
resolution — so a bind can no longer name a device the routing was never
installed for, and ` eth1 ` binds what the netplane routes. A device-less
egress emits NO outbound and is reported; every reference to it then
resolves through egressDetourOrBlock to tagBlock, so the traffic is
blocked rather than sent out over the plain WAN.
- model.ValidateEgresses reports the same egress on the config channel
(netplane's own skip is silent), built on model.EgressHasDevice — the
model-side twin of EgressDevice, which ValidateUntunnelableEgress now
shares so the two model resolutions cannot drift either.
- TestEgressDeviceResolutionParity runs one table through
netplane.EgressDevice and model.EgressHasDevice and requires one verdict,
the same treatment TestUntunnelableEgressResolutionLockstep gave the
earlier validator/data-plane divergence.
Also: the UntunnelableEgress comment claimed "the panel says which, at
apply time, from whether the device is point-to-point". It does not. The
operator-facing text states both possibilities and declines to claim
either, there is no UI for the option, and isPointToPoint is consulted
only to warn that a gateway-less device can reach nothing. Said so, so the
next implementer does not read a described feature as a built one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
35e4900769 |
fix(shaterd): three success reports for work that was not done
`shaterd status` fabricated a status when the daemon was unreachable and exited 0. The stub is the same struct, printed by the same marshaller, so the only thing that distinguished it was `plane` being "" — a value a live Applier.Status() cannot emit. luci-app-shater was forced to key its "daemon down" verdict off exactly that side effect, and filling `plane` in the stub for any reason would have silently turned "dead" into "fine" on that page. Both branches now carry an explicit "daemon_answered" boolean, and the offline branch exits 1. The field is ADDITIVE and spliced in, not re-marshalled: every existing key keeps its name, value and position (including plane:"" — still emitted deliberately so dashboard.js keeps working until it moves onto the new field), and a newer daemon's unknown fields are relayed untouched. model.writeUCIWith committed the staged package DELETION when the import that was supposed to refill it failed: /etc/config/shater came out empty, the caller saw only "WriteUCI: import: ...", the next ReadUCI reported Enabled=false and the next reconcile tore the plane down. Both error paths now revert through migrate.go's staged() instead — the same idiom, for the same reason. `shaterd ruleset update` printed a note and exited 0. shater-cron runs it with output discarded and, on a zero exit, stamps the ruleset as freshly updated and sets changed=1, so every source=url ruleset was permanently "just updated" by a verb that fetched nothing. notImpl now exits 1 (not 2 — a caller must be able to tell an unimplemented verb from an unknown one). pidfilePath becomes a var so the daemon-answered / daemon-absent split is testable without writing to the real /var/run, mirroring ctlPath. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
d3e33294c1 |
fix(l3): reassemble return-path IP fragments — classifyReturn drops them
sing-tun's forwardReturn.classifyReturn refuses to judge a fragment (flow_parse.go sets `fragment` for IPv4 MF/offset and for an IPv6 fragment extension header; flow_dispatch.go:703 answers returnPass), so a fragmented answer coming back through a WireGuard/AmneziaWG endpoint falls through to the endpoint's own tun stack instead of the l3 return path, and the LAN client never sees it. Measured on the live router: `ping -c3 -s 1400` through an AWG tunnel with MTU 1280 is 100% loss while the WAN capture shows 3 x (1312 + 208) in both directions — the far host answers, the peer fragments the answer to fit the tunnel, the fragments die in classifyReturn. `-s 56` is 3/3 and PMTUD with DF works end to end, so only the fragmented return is broken. sing-tun is pinned upstream with no `replace`, but the fix does not need to live there: every decrypted packet passes returnDeviceWrapper.Write before it is offered to ReturnPackets. Reassemble there and classifyReturn gets a whole datagram. Hard ceilings, because this runs on a 128-256 MB router: 64 concurrent datagrams, 1 MiB of held bytes, 64 disjoint ranges per datagram, 65535 bytes per datagram, 5 s to complete (timer starts at the first fragment and is never refreshed). Over any ceiling evicts oldest-first. Overlap policy: a range contained in one already held is a duplicate and is ignored (first-wins, deterministic) because benign networks do retransmit; any PARTIAL overlap poisons the datagram until its deadline. No conforming fragmenter emits one, and every historical hole in this area comes from a reassembler that tried to resolve the conflict. The MTU of shater-l3 is untouched (65535 on purpose) and sing-tun is untouched. 14 mutations run against the tests; each turns at least one test red, including the two that first survived (a stale-head reuse the sweep was covering for, and a fast-path copy). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
32aac89139 |
docs: stop the docs promising a safety net that ships disarmed
Every install recipe walked the reader through `shaterd apply` + `shaterd confirm` as if commit-confirm were armed. It is not: DefaultGlobals() never seeds ConfirmTimeout, the shipped config carries confirm_timeout '0', and ArmRollback returns at once on a non-positive timeout. A reader following the README believed an apply that cut their SSH would undo itself. It would not. README/README.en/INSTALL now arm it in the recipe and say what 0 means; the apply-flow diagram gained the edge it always took on a stock box. The boot armor was documented nowhere at all (`grep -rli armor --include=*.md` returned zero) while shipping enabled and blocking LAN->WAN on every boot. INSTALL 4 now says what it is, why SSH/LuCI stay up on purpose, every condition under which it refuses to arm, and how to switch it off. Also removed or corrected, each checked against the code, not inherited: * MASQUE/CONNECT-IP is advertised in both READMEs and absent from parse, generate and model -- registry names it among the types deliberately left unregistered. Dropped, with the fork-vs-product distinction spelled out. The inverse too: Hysteria2/TUIC/XHTTP were tagged [T1] while shipped under with_quic/with_xhttp; ShadowTLS is generate+registry only, no parser. * `direct (flow-offload on)` -- no offload/flowtable/flow_offloading anywhere in openwrt/, shater/ or panel/src. The product does not do this. * shater-core deps were two releases stale in two places, one of which vouched for a config.buildinfo check that never covered kmod-tun. Ruling narrowed to what was actually checked. * PORTING's "Full schema" -- the shipped config points at it -- was missing l3_tunnel and untunnelable_egress (UCI is their only path; the panel does not show them) and the blocklist/allowlist/device/alert sections, while listing a `config preset` that ReadUCI has no branch for. * ARCHITECTURE had no L3 ingress and no kernel egress at all, though both are [MVP] and one creates an fw4 zone in the user's firewall config. New 3a. * nftset-for-routing in the DNS diagram: that is the v0.1 mechanism, gone in v0.2. * CONTEXT described a pre-Phase-1 repo and a 24.10.3 testbed. The testbed is ImmortalWrt 25.12.1 r37978-cd0a06bfd3fd (read off the box), which is not a detail: .apk does not install on 24.10 at all. * The gate existed and no .md mentioned it. README/README.en/CONTEXT now do. * release.yml's header still described publishing as either/or after the rolling pointer became unconditional. Comment only. * Shipped /etc/config/shater: schema_version '1' against CurrentSchemaVersion=2; a pointer to a dns_filter line that was not in the globals block (added, '0'); and `option sniff '1'` on the inbound -- an option the model deliberately does not have, which the first panel save would have silently washed out. * lx-changelog pointed at a D25 heading that does not exist. * ROADMAP 2b and 5 were done and unmarked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
9e6dda22b3 |
fix(http3,doh3): stop releasing what is still being read
Two suspicions, both put to a test rather than to a reading. Both were real, and
neither was the leak the suspicion named — both are objects released while still
in use.
roundTripHTTP3Race ran both racers on one cancellable context and cancelled it
before returning the WINNER. quic-go and net/http reset a request's stream when
its context dies, so the caller got a response whose body stopped mid-read:
H3_REQUEST_CANCELLED (local) (read 2687 of 65536 bytes). That path is taken
whenever there is no cached HTTP/3 connection and the request is replayable —
the first request to every host, and every one after an idle close. Each racer
now has a context of its own; losers are cancelled where everything used to be,
and the winner's cancel travels with its body.
DoH3's Exchange packed the query into a POOLED buffer and released it the moment
RoundTrip returned. But http3 writes the request body on a goroutine of its own
and returns as soon as the response HEADERS arrive — the body is still being
read. With the window held open the query on the wire diverges from the query we
packed at exactly offset 8192, quic-go's copy-buffer size: everything past that
was the next pool user's memory, sent to the resolver. Not a slowdown — a data
race and a small memory-disclosure primitive. The buffer now goes back when the
transport closes the body, which http3 does on every path, and can do twice.
Both files diverge from upstream again, hours after
|
||
|
omarandClaude Opus 5
|
fde4bed571 |
fix(luci): stop calling the daemon dead when only the engine is
`running` changed meaning on 2026-07-26 (
|
||
|
omarandClaude Opus 5
|
cb26936ebf |
fix(wgdedup): merge identical WireGuard copies instead of blocking one
A rule pointing at node:awgout, which was already the first hop of the
default-route chain, took the house off the internet for two minutes.
The pass saw one private key materialised twice, kept the copy that
sorted first alphabetically, and fail-closed everything that routed
through the other one — which happened to be the default route for all
traffic.
The mechanism was right and the framing was wrong. The physical limit is
one DEVICE per key, not one mention per key. Two copies that build the
same device — same key, same peers, same address/MTU/AWG parameters and
the same dialer — are one device written down twice, and there is nothing
for them to fight over. Those are now MERGED: one survives and every
reference to the others is rewritten to it, silently. That makes the
shape the owner wanted expressible: one chain using awgout as an
intermediate hop and another using it as a terminal, both entering over
the same egress, coexisting on one device.
Identity is the marshalled options blob rather than a hand-picked field
list, so a field added to WireGuardEndpointOptions or DialerOptions later
reads as "different" instead of being silently merged.
Only a real incompatibility — different detour, different peers,
different device parameters — is still two devices, and then:
- the survivor is chosen by WEIGHT, not by tag order: reachability from
route.Final (the default route) dominates, breadth of use breaks
ties, tag order only settles a true tie;
- the warning names the consequence. "Everything that routed through X
is fail-closed" is equally true of a stray test rule and of the whole
house's default route, and that is what the operator read it as. It
now says which of the three it is, measured on the finished config:
the default route is dead, or it survives via another path, or it
never touched the lost copy.
A merge must not rename away the subscription fetch detour: that
reference lives in the model and is resolved against the running box, so
this pass cannot rewrite it. Such tags win the survivor slot outright,
which costs nothing since every copy in a class is the same device.
Tests: identical copies coexist on one device; a real incompatibility
keeps the default-route copy even when it sorts last and says so; the
warning does not announce an outage when the default route survives
through a group, and does announce one when it dead-ends behind a
surviving exit; no duplication at all is a no-op. All seven mutations
(merge off, weight off, member-dedup off, pin off, detour-following off,
consequence collapsed, plus a positive control) fail the suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
8db29b6267 |
fix(apply): say when shaterd apply armed no safety net
`shaterd apply` exists for one reason: snapshot the last-good, apply, and arm
an automatic rollback so a change that costs you access to the router undoes
itself. It answered `{"changed":false}` and not one word about that.
On the live router (2026-07-26) that was a trap. The operator edited UCI, ran
`uci commit`, the `config.change` reload trigger had already restarted the
daemon, and the fresh daemon applied the new config on startup. By the time
`apply` ran there was nothing left to apply — and the last-good it snapshotted
as the ROLLBACK TARGET was the newly applied config itself. The watcher was
armed onto the very configuration it was meant to protect against: firing it
would have restored exactly what was already loaded. No safety net, no word
said, house offline.
The verb now answers the question it exists to answer, in a closed vocabulary:
rollback_armed true ONLY when a window was armed AND its target differs
from what is running. An armed watcher pointing at the
running config is not a net and is not reported as one.
reason applied | already-applied | nothing-to-apply | disabled |
commit-confirm-off | config-unreadable | apply-failed
message the same thing in the operator's words, never empty.
The two "nothing moved" cases are told apart where they CAN be: an
/etc/config/shater mtime later than this daemon's start, with the running
config already matching it, can only mean a reconcile beat this command to it
(reason=already-applied). Where they cannot — the `uci commit` reload trigger
is stop+start, so it moves the daemon's start past the edit — the text says
so instead of reading as success: no net, harmless if you changed nothing,
unprotected if you did, and shaterd cannot tell which.
Two silent holes surface as a side effect, both previously reported as plain
success: `confirm_timeout=0` (the SHIPPED DEFAULT in
openwrt/shater-core/files/etc/config/shater) makes ArmRollback a no-op, and a
failed post-apply ReadUCI skips the arming entirely.
Arming behaviour is byte-for-byte unchanged — this only makes its absence
visible. A real safeguard for the already-applied case is separate work.
Tests are mutation-verified three ways: reverting classifyApply to the old
{changed,error} fails 11 tests; blinding the mtime discriminator fails exactly
the discriminating one (and falls back to the honest ambiguous text); making
sameConfig always report "different" fails every invariant that forbids
claiming a net over an identical target.
NOT verified on hardware: local_openwrt was held by another agent, so the
control-socket round trip and the real mtime/daemon-start comparison have not
been exercised on a router.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
564033cd10 |
fix(apply): chain: as a subscription fetch detour resolved to a name nothing answers to
`fetch_detour=chain:<X>` never worked. engine.ViaToTag maps "chain:X" to the bare tag "X", but the generator materialises a chain as one wrapper per hop — chain-<X>-h1..chain-<X>-hN — and routes into the LAST one. The lookup missed and the update failed with "unknown outbound tag". It failed CLOSED, so the feed was never pulled over the plain WAN by this path. But the miss had a sharp edge: when a node or group happened to share the chain's name, the lookup HIT it, and the subscription was fetched through a completely different outbound with nothing said. Applier.HTTPClient now resolves chain: before the engine sees it, against the tags the RUNNING box actually holds (outbounds unioned with endpoints — a WG hop is an endpoint and Outbounds() does not list those), mirroring the generator: the highest-indexed chain-<X>-h<i> wrapper is the entry, and a chain that flattens to one hop IS that hop. Every other via form is passed through untouched. The case the generator cannot serve is named rather than papered over: chains are built lazily, only for a chain some enabled rule/egress/DNS detour targets, and a fetch detour is not one of those references — so a chain nothing else points at has no outbounds at all. That, and every other miss, is an explicit refusal wrapping engine.ErrOutboundUnknown (the panel already maps it to 400). Never a fall back to direct: that would put the feed and the owner's real address on the plain WAN, which is the thing fetch_via=proxy is set to avoid. Tests are mutation-checked. Pre-fix behaviour resolves "work"/"solo" and kills every chain case; first-hop-instead-of-last, member-copies-count-as-hops, dropped pass-through, and a silent direct fallback each kill their own test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
add90b5b2f |
fix(panel): the DNS footnote was a grid item nobody placed
`.dns-filter-note` under the endpoint-resolver readout is a DIRECT child of `.dns-filter-card`, so it is a grid item. With no explicit span it auto-placed into column 1 — the toggle's `auto` track — and sized that track to its own max-content: 237px at 390px, 322px at 1280px. That left the `1fr` copy column with 0px, so "Network-wide ad & tracker blocking" laid out one word per line and spilled 2px past the viewport, scrolling the whole page sideways on a phone. On desktop the same cause parked the 52px toggle in a 322px column, 270px away from the copy it labels. Measured at 390px: documentElement.scrollWidth 377 vs clientWidth 375. With `grid-column: 1 / -1` on the footnote: 375/375, and the track list goes from `237px 0px` to `52px 185px`. Cancelling just that one declaration in the live DOM puts 377/375 and `237px 0px` straight back, so nothing else contributes. Verified with playwright over 320/360/375/390/414/430/480/560/640/720/768/ 1024/1280/1440: zero horizontal overflow at every width, with every rule editor open, all three master toggles flipped, every source tab, and every resolver type. No `overflow-x: hidden` anywhere — the page does not scroll sideways because nothing overflows, not because the symptom is hidden. Focus rings and prefers-reduced-motion re-checked and unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
a571bd0e1a |
docs(claude): model is the executor's call, skills are mandatory, standards that earned their place
The old file pinned every subagent to fable — which broke the moment that quota ran out mid-session — and spent half its length on panel scaffolding that has been done for weeks. It said nothing about the test gate, the testbed, or the hardware router, so none of that reached a subagent unless it was retyped by hand into the brief. What is new is not advice, it is the list of things whose absence cost a day each: a test must be mutation-checked or it is decoration; an instrument with no control proves nothing; a subagent must be told it may refute the orchestrator, because the best results this project has had arrived exactly that way; a formally-true sentence that reads as "it works" is still a lie. Skills are now a table mapping this project's areas to the skills that cover them, with the rule that they are invoked BEFORE the work rather than after something failed to run, and that every brief must name them — a subagent cannot see this conversation and will not guess they exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
1267d20fb8 |
docs: drop the L3 handoff note — it is merged, and it said to
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tSapk-v0.2.19-aarch64_cortex-a53 apk-v0.2.19-x86_64 v0.2.19 |
||
|
omarandClaude Opus 5
|
35f697ed08 |
docs(openwrt): say why mtu_fix is inert instead of claiming an MTU we no longer set
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
d0fb6befb1 |
fix(l3): the l3-in MTU is not a tunnel budget — 1420 was a forgery generator
shater-l3 was created at 1420, the WireGuard payload budget, copied one layer too far out. It bought nothing: what actually goes into the tunnel is sized by sing-tun's forwardToPort against Port.PortMTU(), which already fragments to the outbound MTU without DF and answers a well-formed `fragmentation needed` quoting it with DF. All 1420 did was make the KERNEL split every packet above 1392 bytes of payload on its way into the device -- and a fragment is the one thing sing-tun will not judge. Dispatch returns on parsed.fragment before calling JudgeFlow, the fragments reach the gVisor stack, it reassembles them, and the ICMP forwarder's installFlow demands an unspecified port address that a WireGuard endpoint never has. So it declined and answered the echo itself. `ping -s 1392` honest, `ping -s 1393` a lie, and only for the outbounds the feature exists for. 65535 rather than merely "large": no IP datagram can exceed it, so the kernel cannot fragment at this device for any packet ever. Anything smaller leaves a band open and re-opens the class. It is also sing-box's own default TUN MTU on Linux. Memory was measured, not argued. Three paired runs of the integration test under -test.memprofilerate=1 allocate 5.41/5.48/5.47 MB at 65535 against 5.76/5.46/5.70 MB at 1420, and a -diff_base profile puts every difference in netlink interface enumeration. Nothing in the read path scales with the MTU: gVisor reads through fdbased.BufConfig, which sing-tun pins to one 65535-byte view regardless. I predicted a ~1.8 MB saving from GSO switching off above 49152 and was wrong -- protocol/tun turns GSO back on at StartStateStart whenever a FlowOutbound exists, so the GRO scaffolding is there at both values. The corrected reasoning is in the constant's comment so the next reader does not redo the mistake. The integration test now reads the MTU back off the real kernel device, which is the assertion the value exists for: a kernel that clamped it would restore the forgery without changing a generated byte. D25's KNOWN HOLE block is replaced with what is genuinely left. Chiefly: a big non-DF ping does not start WORKING, it starts failing HONESTLY -- classifyReturn declines fragments on the way back too, so the packet really leaves, the far host really answers, and the reply is not NAT'd home. And a client that fragments on the wire itself is still uncovered; that is the nft carve-out's job, with a warning that conntrack defrag may reassemble in prerouting and leave such a rule unable to match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
81c96019b5 |
fix(panel): let a routing rule say ICMP, instead of calling one broken
The Proto picker was a closed list of the two transports and the ten sniffed L7 labels, and anything else drew "<value> — never matches". The engine now routes ICMP by rule (Rule.Proto accepts icmp, icmpv4, icmpv6), so a working ping rule was rendered as a dead one and could not be created here at all — the operator had to hand-edit /etc/config/shater and then watch the panel call the result broken. Adds a third group, "Layer 3". All three spellings are offered: they are not synonyms — icmpv4/icmpv6 pin the rule's ip_version — so hiding the narrowing would both strand a capability outside the UI and silently widen such a rule the first time someone edited it here. The doc comment no longer claims the list IS generate/route.go's sniffedProtocols; only the middle group is. ICMP goes to the emitted rule's `network`, never to `protocol`, which is the whole reason it never matched as a sniffed label. An unknown value is still kept and offered as written, but the never-matches flag is now judged on the lower-cased value, the way the engine judges it — a hand-written `ICMP` is a live rule, not an inert one. Verified: npm run build clean (tsc --noEmit + vite build); an icmp rule added through the panel renders as a plain "PROTO icmp" chip; no horizontal overflow at 360px. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
baed8ff8f2 |
fix(model): fwmark_base 0x7f routes the engine's own traffic into its own TUN
The panel offers fwmark_base and table_base as free hex fields under "Advanced" and nothing has ever checked them. What makes that more than a footgun is that the derived values are invisible from the number typed: the L3 mark is base+0x80, so 0x7f lands it exactly on 0xff — the loop-guard mark the engine stamps on its OWN traffic — and `ip rule fwmark 0xff lookup 8200` then captures everything the engine sends and routes it into the engine's TUN. The router loses the internet the moment l3_tunnel is switched on, for a reason nothing on screen connects to a collapsed section. fwmark_base 0xff had produced the same failure since long before the L3 offset existed. table_base is worse and got the same treatment: its derived values can land on the kernel's own table ids, and teardown does `ip route flush table <n>`. It is count-sensitive (egress #i uses base+0x10+i), so the check takes the egresses rather than living in ValidateGlobals. Written as "derive every value this layout produces, then look for duplicates and reserved ids" rather than as a blacklist, so a future offset is covered by construction. The layout constants are duplicated from netplane (the import only runs one way) and pinned by netplane's TestMarkLayoutConstantsLockstep. Warn-only, like every check in this file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
f80fb4dd1b |
fix(netplane): give every mark-driven table a floor, and check the L3 pair
Two halves of the same omission.
1. A fwmark lookup that finds an empty table does not fail — it falls
through to main. Every mark-driven table now gets an `unreachable
default` at the maximum metric: it loses to any real default route while
one exists, it has no device so the kernel never garbage-collects it, and
it turns "lookup failed, try main" into "lookup succeeded: unreachable".
The fallthrough stops depending on somebody reading a warning at the
moment an interface goes down. Deliberately not gated on the kill-switch:
that switch decides whether traffic may escape the tunnel, while an egress
binding is a statement about WHICH UPLINK, and silently substituting a
different one is not what "fail open" was meant to permit.
RoutingPresent's "does this table have a default route" test is tightened
in the same breath, or the floor would answer it and turn the safety net
into a blindfold.
2. RoutingPresent had never heard of addL3Routing. This is the same defect
its own comment describes as already caught twice ("a presence check must
cover everything its Apply counterpart installs"), committed a third time
— and its trigger needs no interface to go down: editing a node URI
restarts the engine, the kernel destroys shater-l3 and takes `default dev
shater-l3 table 8200` with it, the rendered nft text is unchanged, so the
fast-path skipped ApplyRouting forever and LAN ping stayed dead until
someone restarted the daemon.
TestRoutingPresentSeesL3Table, TestEgressTableGetsFailClosedFloor and
TestEveryStampedMarkIsRoutedAndVerified all fail on the code they replace.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
b71b793681 |
fix(netplane): a mark says where a packet was sent, not where it went
The forward chain let untunnelable-egress traffic past the kill-switch on the strength of its fwmark alone. `ip rule fwmark X lookup N` does not deliver the packet to table N, it delivers the LOOKUP there — and a lookup that finds nothing falls through to main. So when the egress interface goes down and the kernel garbage-collects its default route, every non-TCP/UDP packet from the LAN is still stamped, still accepted here (above the fail-closed drop), and leaves out the plain WAN with the router's real address. Nothing we render changes, so no apply runs and nothing notices. Ordinary egress traffic never had this hole: the engine binds those sockets to the device, and a dead device fails the socket. The untunnelable-egress path is made of nothing but a mark, so the accept now carries the second opinion instead — `meta mark X oifname "dev"`, strictly narrower than either half, true only when the routing did what the mark asked. The comment being replaced argued correctly that oifname ALONE would be too loose, then drew from that the conclusion that oifname should be dropped rather than added. Same conjunction in the holding plane, where it is theory (that plane stamps nothing) but where a bare mark accept has no business sitting. Also folds the egress device resolution into one EgressDevice(), because the binding and model.ValidateUntunnelableEgress had already drifted: the validator trimmed the interface name and the binding did not, so `option interface ' '` gave a panel saying "the option is ignored" over a data plane that was marking packets for a table nobody built. TestUntunnelableEgressAcceptIsBoundToItsDevice and TestUntunnelableEgressResolutionLockstep fail on the code they replace. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
61c87ad1d9 |
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 |
||
|
omar
|
4dee508e12 |
fix(route): let a rule say "icmp", and say when saying it is a lie
`icmp` fell through ruleMatchers' proto switch into RawDefaultRule.Protocol — the SNIFFED-L7 field, compared against what the sniffers labelled a connection. Nothing ever labels a flow "icmp" (PreMatch skips the sniff action for an ICMP flow outright), so the rule was structurally valid and permanently dead. That made the whole L3 ingress unusable on a real config: with no way to write "ICMP goes here", every ping fell to the catch-all, which resolves to the chain's last hop — a group of VLESS nodes that cannot carry layer 3 at all. icmp is a NETWORK. NetworkItem.Match is a map lookup over metadata.Network, and adapter.JudgeFlow sets that to N.NetworkICMP for BOTH ICMPv4 and ICMPv6 (one case covers both protocol numbers), so there is exactly one network value and it covers both families. `icmpv4`/`icmpv6` narrow that same network with an ip_version item instead of inventing a second one: metadata.IPVersion comes from the destination address, and an ICMPv6 packet always has an IPv6 destination — no false positives, no false negatives. An ICMP rule that cannot fire is not a dead setting: ICMP has no fall-through, so route.preMatchFlow DROPS it. Four ways to get that silently are now reported: l3_tunnel off (nothing enters the engine at all), icmpv6 with ipv6 off (neither the nft mark nor the TUN address exists), a port matcher next to it (JudgeFlow zeroes both ports), and a target that cannot carry layer 3 — decidable from the model, because the capability is fixed by the outbound TYPE: only wireguard/AWG endpoints and the direct outbound behind direct/interface egresses declare N.NetworkICMP. A mixed group gets its own text (the answer follows group.Now()), `block` gets none (dropping the ping IS the policy), and an unresolved target gets none either (ruleKillFallback already said the louder thing). Wording stays clear of shater/apply's criticalMarkers on purpose: a failed ping is fail-CLOSED, and a cosmetic alarm is how the real one stops being read. |
||
|
omarandClaude Opus 5
|
76da5134ef |
test(gate): the two tests that need a kernel may not skip in silence
The L3 branch adds TestIntegrationL3TunInboundStarts and TestIntegrationL3EgressICMPIsAFlow — the only tests that prove the engine really opens shater-l3 and that the egress outbound really is a FlowOutbound. Both need root plus /dev/net/tun, both guard themselves with t.Skip, and the gate could not see either: `go test` prints `ok <pkg>` whether a test ran or skipped, so [2/5]'s per-package `ok` check is satisfied and the gate closes by claiming it "passes every test we own". That is this script's own founding failure (115 of 116 test files never running while CI stayed green) one level down, and it would have shipped invisibly. Two halves. Where the capability CAN be granted, grant it. From a non-linux host the gate re-execs into a container; that container now gets --cap-add NET_ADMIN and --device /dev/net/tun, probed rather than assumed, so a plain `scripts/run-tests.sh` on a dev box actually exercises the kernel path instead of quietly stepping over it. Where it cannot, say so where it cannot be missed. The act_runner is an LXC guest whose kernel has no tun module at all (checked on 10.10.10.211: `modprobe tun` -> "Module tun not found", /dev/net does not exist, act_runner runs job containers with privileged:false and no container.options), so the device cannot be handed down without reconfiguring the Proxmox host. New step [5/5] therefore DISCOVERS every ^TestIntegration under the fork's trees — no hand-kept list, so a privileged test written next month joins on the day it is named — runs them with -v, and demands a verdict for each BY NAME: RAN, or FAILED/MISSING (fatal), or SKIPPED while the environment could have run it (fatal, because the capability guard cannot be what skipped it), or skipped for a reason this box genuinely has — which replaces the closing banner, so the last line of the gate can never claim coverage it does not have. SHATER_REQUIRE_PRIVILEGED=1 makes that last case fatal for runs that can. The discovery call carries -ldflags for the same reason every other call does: `go test -list` links each test binary, and without -checklinkname=0 every package pulling common/badtls fails to link. The first cut of this step omitted it, swallowed the error, and printed "none declared" — a check against silent skipping that was itself silently skipping. Its exit status is now inspected and an empty list is only ever reported after a successful enumeration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omar
|
4c630c9a13 |
docs: handoff note for the L3 branch
Transient, to be deleted when omp/work merges. Everything meant to outlive the merge is already in D25/D26 and the lx changelog; this file is the part that is only useful while the branch is still a branch — the verification commands, the testbed recipe, what was proven on hardware and what was not, and the six files that will conflict on rebase. |
||
|
omar
|
d8dbefcd07 |
docs: record the AWG site-to-site path as declined, not impossible
D26's "no port-like selector" line disposes of NAT-based forwarding and nothing else, and read alone it says "impossible" — which is false and would be re-derived at the cost of another research pass. The endpoint is protocol-blind in both directions, so ESP could ride it untouched with the client's own source address and no NAT whatsoever. That was declined for two reasons worth naming: lx-owned code in the forward hot path, and a server-side AllowedIPs prerequisite that turns a router option into a deployment contract. |
||
|
omar
|
974208fc05 |
docs: record the kernel egress, and retract the reason D25 gave for the ceiling
D26 writes down where the engine's boundary actually is, because the intuitive answer is wrong and someone will look for it again: the WG/AWG forward path never consults gVisor in either direction, so the limit is sing-tun's ForwardDispatcher — its parser and its port-shaped NAT — and the kernel egress was chosen because it clears that limit without a line of new hot-path code, not because userspace "cannot". Tailscale documents the same boundary for their userspace mode and is quoted as corroboration, with the caveat that ours sits at the dispatcher rather than the stack. D25 said two things that do not survive checking, and both are corrected in place rather than left for the next reader to trip over. It blamed the netstack for the ICMP-echo ceiling; that was the dispatcher. And it called `stack: gvisor` mandatory because the system stack fakes ping — the system stack runs the very same dispatcher first and only forges an echo for packets the dispatcher declined, so gvisor is a deliberate choice (already linked via with_wireguard, and the combination the integration test exercises), not a necessity. The operator note says what the option buys and refuses to call an egress a tunnel on its own say-so: with a WireGuard device it is one, with a second WAN the destination sees that uplink's address. It also says what the option does not fix — multicast IPTV stays broken — and that IPsec through NAT-T is ordinary UDP that never needed any of this. |
||
|
omar
|
2eb71e8244 |
feat(netplane,model): hand the protocols the engine will not dispatch to the kernel
ESP, AH, GRE, IGMP and SCTP cannot enter the engine, and the reason is not the
one that looks obvious. A WireGuard or AmneziaWG endpoint forwards straight past
its gVisor stack — WritePackets reads the IP version and the destination address
and hands the raw bytes to the device, and the return path offers every
decrypted packet back before the stack sees it. WireGuard would carry ESP today
if anything handed it one. What refuses is sing-tun's ForwardDispatcher: its
parser recognises TCP, UDP and ICMP echo, and its NAT wants a port-shaped
selector that ESP, AH and GRE do not have. The retracted rationale is corrected
where it was written down, not quietly dropped.
So these protocols go to the kernel instead. untunnelable_egress names an
interface or tunnel egress; prerouting stamps that egress's OWN mark on
everything that is not TCP or UDP, and addEgressRouting has already bound that
mark to a table whose default route leaves via the device. Every protocol works
because nothing in the path has to understand any of them. No new mark, no new
table, no new code in the hot path.
Whether that is a tunnel depends on the device, and nothing here claims
otherwise: a WireGuard interface is one, a second WAN is a different uplink
whose real address the far end sees.
The wide `!= { tcp, udp }` filter is safe here and stays banned for the L3
ingress, for the same reason stated in both places: there the receiver is a
dispatcher that knows four protocols, here it is the kernel. ICMP is claimed by
the L3 ingress first when both are on. The local plane keeps its exclusions —
router-addressed traffic, private destinations, ICMPv6 ND/RA — and with IPv6 off
the marking is scoped to v4, because addEgressRouting installs no v6 rule then
and a marked v6 packet would fall into the main table.
An interface egress with an empty `interface` no longer resolves: IfaceDevice
defaults to br-lan, so it passed the binding while addEgressRouting skipped it —
mark set, no rule, straight past a closed kill switch and out the default WAN.
|
||
|
omar
|
668cccbf24 |
test(generate): the L3 device name is a singleton, so wait for the kernel to take it back
Both gated tests stand an engine up on shater-l3. Run together, the second met `TUNSETIFF: device or resource busy` and failed for a reason that had nothing to do with what it asserts — the first had closed its box and yielded while unregister_netdevice was still catching up. Each passed alone, which is the shape of a fixture bug that gets rediscovered rather than fixed. The poll that already guarded the first test is now a shared helper both call. It stays a poll rather than a sleep for the reason it always was: the removal is usually immediate and a fixed wait would be either flaky or slow. |
||
|
omar
|
4ea4585402 |
test(generate): pin that ping through an interface egress is real, and byedpi's is not
An interface egress is a direct outbound carrying BindInterface and a routing mark, and direct builds its ICMP port from the very same dialer control — so ping routed at that egress leaves through that device, marked, like every other packet bound to it. Nothing said so. Both halves of that sentence are one `common.Cast[*dialer.DefaultDialer]` away from being false: if the dialer ever stops being a DefaultDialer, icmpPort is nil, PreMatchFlow declines, and ping through the egress degrades to a drop without a single generated byte changing. The gated test asserts the live outbound, not the config, because that is where the cast happens. The failure the codegen half guards is worse than a broken ping: losing BindInterface or the mark does not stop the echo, it sends it out the main table over the plain WAN with the real address, which is the one thing an egress exists to prevent. byedpi is a SOCKS outbound and cannot be a tun.Port, so ICMP aimed at it is dropped. That is the honest end of l3-honest-drop and it is pinned too, because the alternative the TUN stack offers is a forged reply. |
||
|
omar
|
dc6d102473 |
docs: put a number on the second netstack, and say what it does not bound
Measured on a throwaway harness in a container: peak RSS of a process that brought the engine up went from ~26 MB to ~28 MB with l3_tunnel on, three paired runs. It is x86_64, idle, with an empty ICMP NAT table, so it stays listed as unverified for the router — an indicative figure is more useful than silence only if it says loudly what it is not. |
||
|
omar
|
683afc0a47 |
docs: record how ping got through the tunnel, and where it stops
D25 writes down the reasoning that is expensive to reconstruct: why a TUN rather than TPROXY, why the interface is its own with auto_route off, why gvisor is mandatory rather than preferred, and why the ceiling is ICMP echo — a boundary in sing-tun's flow parser and gVisor's protocol set, not an unfinished edge of ours. It also records what carries layer 3 and what does not, that masque could and does not, and the two things still unproven: the live-router path end to end, and what a second gVisor NIC costs in memory on the hardware. D17 gains one line: its claim that TPROXY cannot carry ICMP is still true, and is no longer the end of the story. |
||
|
omar
|
2c3e20512e |
feat(openwrt): let fw4 know the L3 tunnel device before it exists
Both nft tables run and a drop in either one wins, so our forward accept for shater-l3 decides nothing on its own: fw4 sees a device in no zone and drops the forward, and the feature fails with exactly the symptom it was built to fix — ping does not work, and nothing says why. The zone names the device directly rather than a network. fw4 resolves a zone's networks through netifd, and a proto-none interface for a device the daemon creates is never up and contributes nothing, so list network would compile to an empty device set. list device compiles to a plain iifname/oifname match that is valid before the TUN exists and starts matching the moment shaterd creates it, with no firewall reload at enable time. It is seeded unconditionally, not gated on l3_tunnel: uci-defaults run once, and a zone naming an absent device is inert. Gating it would mean the option could be switched on and never take effect. The sections are named so a re-run is a no-op instead of a second zone, and kmod-tun joins DEPENDS because /dev/net/tun is not on a stock image. |
||
|
omar
|
51b2f04672 |
feat(netplane,generate): carry LAN ping through the tunnel, on a TUN of its own
Kernel TPROXY needs a socket to hand a packet to, so it moves TCP and UDP and nothing else. Everything else reached the forward chain and met the untunnelable policy, whose best answer was "let it out with your real address" and whose default was "drop it" — so on a stock install ping simply did not work, and the setting that fixed it did so by leaking. The engine has been able to do better for a while: sing-tun's ForwardDispatcher does real ICMP forwarding with NAT on the echo id, and a WireGuard or AmneziaWG endpoint is a tun.Port that carries the packet for real. What was missing was a way in, because nothing on the router could hand it an IP packet. l3_tunnel (opt-in, off by default) adds one: the generator emits an "l3-in" TUN inbound and prerouting fwmarks LAN ICMP into it. The interface is its own and auto_route is off, so the main routing table is never touched and the fwmark plus addL3Routing's ip rule are the only entrance — the TPROXY plane is byte for byte what it was. gvisor is not a preference: the system stack forges echo replies locally, which is the very thing this is meant to end. Only icmp and ipv6-icmp are ever marked, and only after the local plane is out of the way — the router itself, private destinations, and ICMPv6 ND/RA, which mean nothing off-link and take v6 down if one neighbour probe is tunnelled. ESP, AH, GRE, IGMP and SCTP are deliberately left alone: sing-tun's parser and gVisor's stack know no such protocol, so marking them would black-hole the traffic while looking like a feature. They stay with the untunnelable policy, which also keeps its say over what happens if the ip rule fails to install. Ping and Windows tracert now cross the tunnel; IPv6 traceroute shows only the destination, because the return path recognises TimeExceeded for v4 alone. |
||
|
omar
|
f190c8251e |
feat(lx): stop answering ping on behalf of a tunnel that never saw it
PreMatchContinue is not "fall back to the ordinary route" the way it is for TCP and UDP. An ICMP flow has no ordinary route: the TUN stack takes the packet back and answers the echo itself, swapping the addresses and writing a reply (sing-tun stack_gvisor_icmp.go). So a ping routed to any outbound that cannot carry layer 3 — every proxy protocol; only adapter.FlowOutbound can — came back successful, and the operator read a working tunnel off a packet that was never sent. That is worse than the packet loss it replaced. Loss is a fault the operator can see and chase; a forged reply is a fault that reports itself as health, and it reports it on the one tool anyone reaches for first. preMatchFlow now overrides continueResult once, at the top, for N.NetworkICMP. One hunk covers every exit that used to fall through — no such outbound, a group whose selection is gone, an outbound whose Network() omits icmp, an outbound that is not a FlowOutbound — and keeps the diff to three lines against a function upstream will keep editing. JudgeFlow carries the same verdict in its !isPort branch, because FlowOutbound and tun.Port are separate interfaces and drift between them must not reopen the forgery. TCP and UDP are untouched, and the test pins that as hard as it pins the drop. |
||
|
omarandClaude Opus 5
|
1945404eaa |
fix(armor): a reboot is not someone switching the product off
The boot armor never armed on the router it shipped to. procd runs the
K-links on the way down with the action `shutdown`, and stop_service
classified actions with an OPEN default:
case $action in restart|reload) keep;; *) DISARM;; esac
`shutdown` matched nobody, fell into `*`, and deleted the arm token. The
mechanism erased itself at exactly the transition it exists for, so every
boot found nothing to load. Measured on the live router, one minute apart
across a reboot:
13:28 /etc/shater/boot.nft present
---- reboot
18s at_S22: NO_TABLE armor_file=NO_FILE
It did not fail every time, which is worse than failing always: on the way
down `rm` from this script raced a `SaveBootArmor` driven by the ifdown
hotplug storm, and whichever landed second won. Two reboots on the same box
an hour apart gave opposite outcomes.
Both lists are now positive and CLOSED. Only `stop` disarms; only
`restart`/`reload` hand off. An action nobody thought of changes nothing,
so the default now fails toward a boot that arms when it need not have --
recoverable in the second before the daemon applies, and still gated by
shater-armor's four state refusals. The old default failed toward the
plaintext window the feature was built to close.
Also closed, found while proving the above:
* Every restart left the LAN in the clear for 80-90ms. The exit path was
`Teardown(); armOnExit()`, and TeardownNft DELETES the table -- two nft
transactions with no `inet shater` between them, leaving fw4's
`lan -> wan ACCEPT` as the only policy. Every restart, every LuCI Save
& Apply. TeardownExiting arms first under the apply lock and skips the
delete iff a plane actually went in; RenderHoldNft is one `nft -f` that
REPLACES the table, so the kernel never observes its absence.
35k-sample instrument: 7 and 6 no-table hits before, 0 across three
runs after.
* SaveBootArmor fsynced the payload but not the directory, so a power cut
could lose the rename that publishes it -- a boot with no armor and no
error anywhere.
`stop` now also reads rc.d state, so a package transaction that stops the
service is not mistaken for a person switching it off. This one does not
reproduce on apk (it runs no pre-upgrade script and never calls prerm on an
upgrade; verified with apk adbdump and 245k samples across a real reinstall)
-- it is one returning opkg lane away from being live, and the removal case
is now stated rather than implicit.
Both new tests are mutation-checked: reverting the predicate fails naming
`shutdown`; reverting the teardown fails with `did [arm delete], want [arm]`.
initscript_test.go sources the SHIPPED shell and calls the real predicates
with every action procd uses -- a comment claiming `shutdown` was handled is
what shipped last time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
apk-v0.2.18-aarch64_cortex-a53
apk-v0.2.18-x86_64
v0.2.18
|