main
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  omarandClaude Opus 5
|
8c0ea55054 |
feat(dns,fetch): resolvers and list fetches follow the uplink; the node cache survives the reboot it exists for
The carrier behind this router's SIM refuses TCP/443 to 9.9.9.9 and 1.1.1.1 while
carrying everything else — measured with a positive control (ya.ru:443 and
77.88.8.8:53 connect, every sim-bypass node connects, those two are refused). The
configured resolvers go out DIRECT, not through the tunnel, so on that uplink DNS
resolved nothing: the vless server names did not resolve, the hop in front of
awgout never came up, and the whole chain died with it. One pair of global scalars
cannot be right for two uplinks; the object that knows which uplink is live is the
profile.
* config profile gains resolver_default, resolver_fallback and fetch_detour
beside endpoint_resolver. Empty = inherit, PER FIELD.
* globals.fetch_detour replaces `const filterFetchDetour = tagDirect`. Behind a
carrier whitelist `direct` is not the safe path, it is the path where the
source is refused forever and the list never loads.
* A subscription's fetch_via becomes an OVERRIDE, which gives it a third state.
ReadUCI used to parse an absent option as the literal "direct", so "chose
clear-text" and "never touched this row" were the same value. migrate2to3
performs the reinterpretation ONCE, in the open. Schema 2 -> 3.
* An unusable override falls back (resolvers to globals, fetch_detour to direct)
and says so at critical, naming profile, field, value and what is in force.
The panel was displaying globals while the engine used the profile's value; the
owner caught it. The field now keeps the STORED value with a separate line naming
what is in force, and the rule that answers "what is in force" moved to the daemon
(GET /api/config/effective) so it stops existing in two languages.
Cold start, by owner's requirement: rule-sets are read from the cache when the
source is unreachable instead of being dropped, and the subscription cache reader
is fixed. Its first fix was wrong and only Linux said so — mtime ties to the digit
because the kernel caches the stamp per tick, and this board has no RTC, so the
ordering can invert across a reboot. Replaced by a generation counter in the file.
Woke and closed a LAN-dark defect: wgdedup read only the deprecated, always-empty
DownloadDetour, never HTTPClient.Detour, so fetch_detour=node:<awg> made a node
used, the dedup pass did not know, merged it away, and left the rule-set pointing
at a tag box.Start could not resolve. Reproduced through a real box.New.
Also: ValidateProfiles had no caller; "applied from the cache" graded critical
though the list is in force; the auth matrix never walked /api/log or
/api/rules/reachability; the CLI and daemon disagreed about where a subscription
is fetched.
NOT fixed, stated rather than implied: the R5 preflight still probes direct, so a
list never yet fetched cannot bootstrap over the detour alone; the router's own
DNS on the SIM stays dead (dnscrypt-proxy bootstraps via blocked addresses).
Gate: bash scripts/run-tests.sh green, 7/7, privileged tests really ran.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
omarandClaude Opus 5
|
625942834b |
fix(gate): [5/7] hid the runner's exit code exactly when it explained everything
The line naming a nonzero `go test` status was printed only when every privileged test had produced a verdict — on the reasoning that a named FAILED already explains the status. The case that actually happens is the opposite one: the run dies at package level, so it names no test, so the loop above prints MISSING for all of them, and the one line pointing at the real cause was the one suppressed. A reader then goes hunting for three vanished tests instead of at the build error above. To be exact about what was and was not broken, because the framing matters: the exit status was never SWALLOWED. priv_bad is set by the MISSING branch, so FAILED is set and the gate fails either way — this was a diagnosis bug, not a correctness one. What changes is whether the log says why. Verified on the branch a green run never reaches, by driving the edited block with all four (priv_rc, priv_bad) combinations: the new message appears only for (1,1), the old one only for (1,0), and priv_bad/FAILED come out 1 in both. The full gate is green with the change in, which covers the (0,0) path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
4bf4ad8aa2 |
test(engine): the observatory tests were racing their own engine's loop
CI's `[4/7] go test -race -shuffle=on` failed the v0.2.23 gate on
TestRefreshObservatoryForcesOnePass ("force flag survived the forced pass").
The product is NOT at fault, and this was established rather than assumed.
WHAT ACTUALLY BROKE. ConfigureObservatory starts the ticker goroutine and its
first tick fires immediately — by contract, so an applied config gets its first
verdicts in seconds — and a plan change additionally nudges the loop into a
pass on purpose. That tick advances the cursor and consumes the force flag.
Two tests then read exactly those fields straight after a Configure, i.e. read
values another goroutine is entitled to rewrite in the same instant. Four
assertions, all racy:
observatory_test.go:78 identical-plan reconfigure reset the cursor to 2
observatory_test.go:86 changed-plan reconfigure kept the cursor at 2
observatory_test.go:176 after refresh: cursor=2 force=false
observatory_test.go:186 force flag survived the forced pass
The last one is the busy guard: with the loop's first tick still in flight the
test's hand-driven observatoryTickOnce is a silent no-op, so nothing clears the
flag it just raised.
NOT a cross-test dependency, and not a leaked goroutine — the direction was
measured, not guessed. Each test reproduces ALONE in the CI container at
`-count=3000`: 16/3000 and 7/3000, with all four messages. The earlier
`-count=80` in isolation was simply too few iterations; a loaded `-shuffle=on`
package run widens the window, which is why CI saw it and a laptop did not.
THE FIX is isolation, not a weakened assertion. detachObservatoryLoop stops the
goroutine and leaves a PLACEHOLDER stop channel behind, so the reconfigures
these tests make still run the whole state machine — plan rebuild, cursor
policy, nudge — with no second writer (ConfigureObservatory starts a loop only
when e.obs.stop is nil; e.obs.nudge is left nil and every send to it has a
default). quiesceObservatoryLoop, which four chain tests already used for the
same reason, is now that plus a cursor rewind.
Mutation-checked: with detachObservatoryLoop neutered the flake returns at
18/3000 and 6/3000 with the same four messages; restored, 20 consecutive
`-race -count=1 -shuffle=on` runs of the package are clean, as is the full
`scripts/run-tests.sh`.
TWO TESTS GAINED THE ABILITY TO FAIL. TestObservatoryTickStoppedEngine and
TestObservatoryTicksDuringManualRun assert `cursor != 0` after a hand-driven
tick — which the loop's own first pass had already satisfied for them, so they
held whether or not the tick under test did anything. The second one is the
worse case: it exists to forbid the tick deferring to a manual run, and the
busy guard could make the tick do nothing while its assertion still passed.
Both now quiesce first.
TWO NEW TESTS, for the contract the flake kept stumbling into without ever
asserting it — a refresh raised while a tick is in flight:
- TestRefreshDuringInFlightTickRunsAFullForcedPass parks the loop's first
pass inside a stub probe, so "in flight" is a fact rather than a hope,
raises force there, and requires a second full pass over jobs the polite
freshness gate would skip. TWO independent wakeups carry the request across
— the buffered nudge and the tick's deferred re-nudge — and that is
measured: disabling EITHER leaves the test green, disabling BOTH makes it
fail with "force is still raised" and 2 attempts instead of 4. So it
asserts the observable contract, not a mechanism, and says so.
- TestForcedPassChainsItsBatchesWithoutWaitingForTheTick pins what
observatoryTickOnce's defer claims and nothing held: a forced pass chains
its batches instead of spending a 10s tick each. THREE batches, because two
prove nothing — the loop's unconditional first tick pays for one and the
refresh's still-unconsumed nudge pays for the second, so a two-batch plan
finishes even with the chaining removed. Measured that way round first;
at three, removing the defer leaves 48 of 54 targets undialled.
obsSelectorFixture/obsWideSelectorFixture exist because obsFixture's urltest
members are SelfChecked and the observatory does not dial them at all — a stub
waiting on that plan would hang, not fail.
No product file is touched: shater/engine/observatory.go is byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
be1cdbfc63 |
feat(netplane): an explicitly named private subnet is routed, not silently swallowed
Measured on the production router: a `config ruleset` of type=ipcidr holding
10.10.10.0/24, a rule pointing it at node:awghome, config_applied=true,
tunnel_rules=1, engine_running=true, ZERO warnings — and from a LAN client,
100% packet loss and no TCP. The rule was accepted, applied, reported healthy,
and could not fire.
The cause is one line of ordering. `ip daddr { 10.0.0.0/8, 172.16.0.0/12,
192.168.0.0/16, 127.0.0.0/8, 169.254.0.0/16, ... } accept` sits ABOVE every
divert line in the prerouting chain, so the packet is accepted and handed to
plain routing before the engine — which holds the rule — ever sees it. That
default is right and stays: LAN-to-LAN, the router's own services and every
local plane must not be dragged through a tunnel, and a catch-all rule must
never quietly acquire them. What was wrong is that naming a subnet OUTRIGHT
could not override it, and that nothing said so.
So the divert for NAMED private destinations is emitted one line higher, and
"named" is deliberately narrow (netplane/coverage.go, privateRoutedPlan):
- the CIDR must be an ENTRY of an INLINE type=ipcidr rule-set — the only
destination list this stage can read;
- it must be CONTAINED in 10/8, 172.16/12 or 192.168/16. A prefix that merely
overlaps one (0.0.0.0/0, 10.0.0.0/7) is a catch-all that happens to include
private space, and does not acquire it;
- the referencing rule must be enabled and target node:/group:/chain:/egress:
or block. `direct` is not an override: it asks for what the bypass already
does, and diverting into the engine to reach the same verdict would be
strictly worse, because the engine's direct outbound follows the DEFAULT
route and LAN-to-LAN could be pushed out the WAN;
- it must not overlap a network this router itself carries;
- 127/8, 169.254/16, 224/4 and 255.255.255.255 are never taken.
THE SELF-AMPUTATION GUARD DISTINGUISHES A LAN FROM AN UPLINK, and that
distinction is the difference between a safety device and an obstacle. A
collision with one of our OWN networks (any zone that is not a WAN zone, plus
any interface whose zone is unknown) is refused by name — diverting it takes
the LAN away from the LAN and the operator finds out over the console. A
collision with an UPLINK subnet routes and discloses: ISPs hand out RFC1918
WANs routinely — this router's own gateway is 10.0.0.1 — and on a /8 uplink
every private subnet on earth "collides", so refusing there would disable the
feature on precisely the routers that want it, for a reason that would read as
a bug. Nothing of ours lives on the uplink subnet: `fib daddr type local`
already accepts the router's own addresses above these lines, and every divert
line is scoped to LAN ingress, so router-originated traffic never meets them.
PING IS HOW ANYONE CHECKS A ROUTE, and a TPROXY divert carries TCP and UDP
only — the kernel needs a socket and ICMP has not got one. Stopping there would
rebuild this same defect one protocol down: TCP succeeds, ping reports 100%
loss, and the operator concludes the route is broken. So with l3_tunnel on, the
L3 mark is stamped on ICMP bound for these destinations (again above the
bypass, which is the only reason it was not already happening) and the existing
`ip rule` delivers it into the engine's TUN, where the SAME route rules pick
the outbound and a WireGuard/AmneziaWG one carries it. The forward chain's
fail-closed drop excludes that mark, because unlike the tproxy legs the LAN-to-TUN
leg really does traverse forward and the `oifname "shater-l3*"` accept that
would rescue it sits four steps lower. With l3_tunnel OFF nothing is emitted,
nothing is claimed, and the rule is told so by name.
THE DOUBT ALWAYS FALLS BACK TO THE BYPASS. Failing to route a named subnet
costs a feature and shows up the moment it is tested; routing one we should not
have touched can take the router's own management network into a tunnel that
may not even be up. So an unreadable list, an inventory we could not enumerate,
and an address family we cannot check the router's own addresses in (IPv6 —
`ubus call network.interface dump` reports IPv4 only) all resolve to "leave it
on the bypass", and every one of them says so. Seven distinct sentences now
exist where there was silence: refused-for-our-own-network, refused-for-no-
inventory, reserved space, catch-all-does-not-acquire, IPv6-not-checkable,
uplink-overlap-disclosed, and ping-does-not-reach-with-l3_tunnel-off. The
eighth is the blind spot itself: an address list this plan never reads
(url/file type=ipcidr, or geoip whose category is not an ISO country code)
might contain private destinations, and that is disclosed unconditionally —
"warn on suspicion" is not available, because suspicion would mean reading the
list. It is graded `warning` rather than critical through a named marker in
apply/warnings.go: it describes a maybe, and a red that means "probably fine"
is how the next red stops being read.
generate.ruleSetTypeIsIPCIDR now delegates to netplane.IsIPCIDRRulesetType.
Two packages asking the same question of the same field must not each carry
their own list of spellings.
VERIFIED
- `bash scripts/run-tests.sh` green in full ("OK: the shipped tag set, on
linux, passes every test we own", exit 0), with the three privileged
^TestIntegration tests RAN by name.
- Every new test mutation-checked: 17 reverts, each failing the test that
covers it, by name.
- BOTH CONTROLS. Without an explicit naming, private space is still bypassed
(TestPrivateDestinationBypassIsStillTheDefault) and a catch-all still does
not take it; with it, the divert appears above the bypass. A test green in
both states would prove nothing.
- BYTE-FOR-BYTE. Two goldens, plain and L3, captured from a git worktree at
the PARENT commit — not from this code, which would only prove
self-consistency. A config that names no private subnet renders the
identical text, so the applier's idempotence check still sees no work.
- REAL NFTABLES. The rendered plane (both the tproxy and the ICMP/L3 shapes)
loads with `nft -f` on nftables 1.0.9 and the kernel holds the lines as
written; the instrument was shown able to REJECT a deliberately broken copy
of the same file.
NOT VERIFIED
- Nothing here has been run on the testbed or the router. Whether the packet
that now reaches the engine actually comes out of awghome is the owner's
acceptance test, not this commit's claim.
- Whether a named IPv6 ULA could be handled safely was not investigated
beyond establishing that the inventory cannot check it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
bbb493ea91 |
fix(ci): the package-count assertion lives in two scripts and only one was updated
D29 removed byedpi, so the feed carries three packages. sdk-build-apk.sh was changed to >=3; build-feed-apk.sh still demanded >=4 and killed both arch lanes of v0.2.22 with `expected >=4 .apk … found 3`. Nothing was published from that run. The comment now says the count is duplicated, because reading one script was what made this look done. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
4869d62e02 |
feat(egress)!: remove byedpi — what it replaced was not weak, it was broken (D29)
The `byedpi` egress kind, the `openwrt/byedpi` package (`ciadpi`), the readiness endpoint and the panel plate are gone. D13 is not deleted from DECISIONS.md; it is REVERSED there, with the reason, because the reason is the whole point. D13 adopted an external desync process on an observation: the engine's own `tls_fragment`/`tls_record_fragment` were tried against a live ISP and did not get through, so the method was judged too weak for anything past "just fragment the ClientHello". The method was never tried. `common/tlsfragment` dropped a number of labels equal to the number of DOTS in the name, and a name always has one more label than it has dots — so the cut always landed inside the FIRST label. `www.youtube.com` was split inside `www` and `youtube` went to the wire in one piece, which is the word the DPI matches on. Of six blocked names exactly one got through: `youtube.com`, the one whose first label IS the blocked word. That defect is fixed ( |
||
|
omarandClaude Opus 5
|
efb2177f43 |
fix(tlsfragment): one cut, in the label a blocklist keys on — and a budget for it
Follow-up to
|
||
|
omarandClaude Opus 5
|
815011dfb0 |
fix(tlsfragment): the SNI was cut in exactly one label — always the first one
`splits[:len(splits)-strings.Count(serverName.ServerName, ".")]` is identically
`splits[:1]`: labels are always one more than dots, so the subtraction cancels
for EVERY name in existence. One label was ever cut, and it was the leftmost
one. On the provider measured from this router — which blocks by the name in
the handshake, proved by the same address answering for SNI www.google.com and
going silent for www.youtube.com — that is the whole observed table:
youtube.com cut inside "youtube" -> 301
m.youtube.com cut inside "m" -> blocked
tv.youtube.com cut inside "tv" -> blocked
www.youtube.com cut inside "www" -> blocked
music/studio.* cut inside the label in front -> blocked
The one name that worked is the one whose first label IS the blocked word. The
count subtracted must be the labels of the PUBLIC SUFFIX, not the dots of the
whole name: "com" is one, "co.uk" and "com.br" and "pp.ru" are two.
Second half of the same defect, and the reason the table above shows a cut
"inside m" at all: the offset was `rand.Intn(len(label))`, whose 0 is the
label's own boundary — the label goes out whole in the next segment, which is
not a cut, it is a segment boundary that happens to touch a label. For a
one-byte label 0 is the ONLY value it can take. Offsets are now drawn from
[1, len-1], so a cut always leaves a non-empty piece of the label on both
sides, and a label too short to have an interior offset carries no cut instead
of a fake one. That also closes the 1-in-7 hole in the case that WAS working:
youtube.com drew offset 0 once every seven connections and handed the name over
intact.
Two panics went with it, both reachable from the LAN, because route/conn.go
wraps the outbound with this and the ClientHello it fragments is the client's:
an empty label (SNI ".youtube.com" or the perfectly ordinary FQDN
"youtube.com.", where the suffix list declines to answer and the trailing empty
label survives) reached rand.Intn(0) — "panic: invalid argument to Intn", the
daemon and with it the router's proxying. And a plan with no cuts at all would
have indexed b[:splitIndexes[0]] on an empty slice; Write now writes the
ClientHello unchanged in that case, which is the only honest thing to do for a
name of one byte.
The classification is closed and errs toward MORE cutting: narrowing the label
set needs proof (a public suffix that really is a tail of the name), widening
needs none, so a trailing dot, an unmanaged TLD, a name that IS a public suffix
("com", "co.uk", "localhost") and an IP literal all keep every label rather
than fall silently into "cut nothing". When no label is long enough to cut, the
name itself is cut once — a matcher looking for the whole FQDN still fails
across that split.
Dropped with it: `splits[0] == "..."`, unreachable since strings.Split on "."
cannot produce a token containing a dot. And the plan now runs over the FIRST
entry of the server_name_list (MyServerName.Length) instead of everything left
in the extension, so a second entry cannot be fed to the public suffix list as
if it were part of the name.
Tests (cutplan_test.go, package-internal so the plan itself is visible) are
verified by mutation five ways: the old dot arithmetic, the old rand.Intn
offset, the removed empty-label guard, the removed empty-plan guard, and a
one-byte corruption of a segment. Each fails by name and with the numbers. The
controls: youtube.com — the case that already worked — must still be severed;
the reassembled segments must be byte-identical to the ClientHello in all three
modes (tls_fragment, tls_record_fragment, both), with the record framing
re-parsed rather than assumed; and Write must report len(b).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
0a34e64c2c |
fix(panel): byedpi off the status poll, and disabled stops speaking for two situations
GET /api/status no longer carries the readiness report — the daemon dropped it with the cache behind it, after one probe was measured at 6.4 s on 16 enabled instances behind a black hole while the panel polled that endpoint every 5 s from every open tab and read the field NOWHERE. The Status type, the mock fixture and every comment describing a cache, a background refresh or a 20 s staleness rule now say what the daemon does: one endpoint, and it connects when a human asks. `disabled` covers two situations with opposite next actions: no instance is enabled — how the package ships — and an instance that IS written and looks enabled while /etc/init.d/byedpi refuses it (`port 'auto'`, `port '99999'`, `enabled ' 1'`, `enabled 'TRUE'` — all four measured on the 25.12.1 testbed against validate_data). The editor's fixed sentence said "that is how the package ships" about a section the operator had typed themselves. The daemon keeps its `problems` list off the wire, so `detail` is the ONLY carrier: the refusal now shows that sentence verbatim plus a tail that says only what is true of both — the consequence, never the fix. byedpiRefusal moves to byedpiReady.ts beside the gate it explains, and its table now EXCLUDES `disabled` from the type, so re-adding a fixed sentence for it does not compile. `?mock&byedpi=rejected` reaches the second case in a browser; `?mock&byedpi=noanswer` reaches "nothing has been measured", which is now only a failed fetch — the fabricated cold-cache body is gone. Also: two comments about `config_applied` that the daemon's pointer+omitempty change made false — the removed "positively phrased so a naive client falls the alarming way" rationale, and "absent means a daemon too old", which now also means the offline `shaterd status` stub. Tests (byedpiRefusal.test.ts, +10) verified by mutation both ways: a fixed "that is how it ships" and a fixed "your typo" each fail, and the control asserts the factory state still reads as the factory state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
1708159ecf |
fix(apply,shaterd): the offline stub alarmed about an apply nobody attempted
`config_applied: false` means "/etc/config/shater was read and REFUSED — what is running is the PREVIOUS configuration, your edit is not in effect", and the panel draws a critical band saying exactly that. The field was a plain bool, so that alarm was the ZERO VALUE OF THE TYPE — and `shaterd status`'s offline stub, built by a process that never applied anything, over a data plane that may have been installed and enforcing for weeks, published it by simply never mentioning the field. It is the config_readable defect returning in a new field, with the one difference that decides the fix: config_readable can be MEASURED by the stub and now is, while this one cannot be measured at all without a daemon. So the field says nothing when nobody measured it. ConfigApplied becomes a *bool with omitempty; the live Applier.Status() assigns a verdict on BOTH arms, so an absent key can only come from something that is not a live status. That is the same closed-set-plus-unknown shape `plane`, `traffic` and `daemon_answered` already have, and the one panel/src/appliedConfig.ts already implements (=== true / === false / else unknown). The Go doc claiming absence should read as false is gone: it contradicted the only consumer, and the consumer was right. The four fields around it (apply_error, apply_error_stage, apply_attempts, apply_failed_since_unix) stay plain: they are qualified by config_applied the way enabled/kill_switch/panel_port are qualified by config_readable, and their zero values point at "nothing was refused" — the quiet side, not the alarm. Also: the stub shipped `warnings: null` on its happy path while apply.Status documents Warnings as always non-nil so a consumer can map over it unconditionally. Three states, distinguishable ON THE WIRE through one `shaterd status`, with the control that would catch the opposite break (a build that omitted the key for a real refusal, deleting the alarm from the product). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
9d7f0dc92f |
fix(panel): the byedpi report had a five-second timer and no reader, and its parser could forge "listening"
Two defects found by looking at both sides of the byedpi readiness check at once.
1. GET /api/status carried the whole readiness report from a cache that a poll
refreshed in the background once the copy passed byedpiRefreshAfter = 3 s.
The panel shell polls that endpoint every 5 s, so EVERY poll started a
refresh: a PATH lookup, a read of /etc/config/byedpi, and one connect per
enabled instance, forever, per open tab, hidden ones included. The design
note rejected a background ticker because "a closed panel costs nothing" —
true, and silent about the open one it had become.
Measured, one enabled instance, twelve polls five seconds apart:
before 12 connects, 13 ciadpi PATH lookups per minute per tab
after 0 connects, 12 PATH lookups (one per poll, for byedpi_installed)
And nothing read it: `grep -rn '\.byedpi\b' panel/src` finds no consumer —
the readiness plate, the per-egress cross-check and the egress-type gate all
come from GET /api/byedpi. So the field is gone from the status response, and
with its only cached reader gone the cache went too, together with the
background goroutine, the staleness rules, the negative-age contract and
Server.Close's duty to wait for a probe. GET /api/byedpi still connects, on
the goroutine of the request that asked.
2. readByeDPIInstances claimed to mirror /etc/init.d/byedpi "exactly" and did
not. The init script validates each section with
'enabled:bool:0' 'port:port:1080' and refuses to start one whose validation
failed. Go read the port with strconv.Atoi and, on failure, KEPT the 1080
default — so `option port 'auto'` on an enabled instance became "an enabled
instance on 1080", and anything else accepting there produced state
"listening": the one state that unlocks the byedpi egress type, handed out
for a proxy that does not exist. `port '99999'` produced the second half:
"unknown" with a sentence asserting a connection attempt that never happened.
The same shape lived in `enabled`: strings.ToLower+TrimSpace read ' 1' and
'TRUE' as on, while the router starts neither (measured — the first is
refused by validation, the second normalises to an empty value so
`[ "$enabled" -eq 1 ]` never fires).
The parse is now a closed positive list, and its expectations were MEASURED
on the 25.12.1 testbed against /sbin/validate_data with the init script's own
spec rather than inferred from libvalidate's source:
enabled: absent/"" -> off; exactly 1|on|true|yes|enabled -> starts;
exactly 0|off|false|no|disabled -> off; anything else -> does not
start, and is REPORTED by section, option and value.
port: absent/"" -> 1080; plain decimal digits 1..65535 -> that port;
anything else -> NO port is assumed, the section is not counted as
a listener and nothing is dialled for it.
Deliberately narrower than libvalidate's `port` (which also takes a sign,
leading whitespace and, through an overflow, twenty digits): narrow declines
to call a working instance a listener and prints why, wide hands out a green
apply onto a port nothing is on.
Two further sentences that asserted actions that never happened, found while
fixing the above and not reported by the review: instances past
byedpiMaxInstances were never dialled yet fell into the "the connection attempt
neither succeeded nor was refused" clause, and that clause listed their ports
alongside genuinely inconclusive ones. "Not dialled" is now its own tally with
its own sentence, and each sentence names only the ports its own claim covers.
Every test here was checked by mutation, and each carries its control:
byedpi_initparity_test.go proves the instrument BOTH accepts a valid section
(state listening, against a real socket, in a world where every connect is
accepted) AND refuses every value the init script would not start, dialling
nothing for them; byedpi_pollcost_test.go measures the poll cost with a meter
shown counting a real probe in the same test, and keeps the probe-cost control
(16 black-holed ports = 6.4 s) that explains why it is off the poll path.
Gate: bash scripts/run-tests.sh green, including -race; ok shater/panel by name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
c407771cf2 |
fix(panel): four cards, one log row and a status the daemon knew and nobody saw
The board is not one row per name. carryForward supersedes by (name, KIND) and appends carried rows LAST, so a chain `x` and a node `x` both live on it — and `new Map(results.map(r => [r.group, r]))` kept the last. The chain card showed the node's milliseconds, exit address and verdict as its own end-to-end measurement, unmarked. Attribution is now by kind (targetResult.ts), with kind:'' and a missing kind as ordered last resorts. A connection routed to the engine's `block` outbound was drawn as plain mono text, indistinguishable from `nl-reality-1` — on the page where a DNS row about the same host gets a crit rail and a BLOCK mark. It is the kill-switch's own Final and a legitimate rule target, so the connection log now carries the same outcome axis the DNS log has: killed / carried / no exit recorded, a crit rail and a mark that survives the width where the exit column is dropped. Insights.tsx held a raw NUL at byte 36359 — a template separator written as the byte instead of the escape. `file` called the source binary and ripgrep, git grep and every tree-wide search skipped it in silence. It is the escape now, and the whole of panel/src is free of control bytes. Three contract texts had drifted from the daemon: the searched-field list did not mention `error` (fixed on the Go side, and there were two copies), the connection hint named neither `proto` nor the chain hops, and rowMatches folded case with toLowerCase() — Unicode-aware, where the daemon folds ASCII only, so a needle could find rows in the panel that the router would never return. And the four status fields the daemon started publishing: config_applied, apply_error, apply_error_stage, apply_attempts, apply_failed_since_unix. A refused configuration retried on a widening interval while `engine_running` was true, the hash was the OLD config's and every warning described the OLD config. engine_running is TRUE there and is not contradicted — the band says WHICH configuration is running, and the hash row, the traffic default and the findings list each say they are about that older one. Absent is not false: a daemon without the field is `unknown` and raises nothing, because there is no evidence its hash is stale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
26e1d38924 |
test(bridge): make the fragment sweep test assert the property it names
TestBridgeFragmentSweepIsPerCall claimed its probe used "an EXISTING key, not a new one: the sweep must still run". It did not: the stale datagram carried IPv4 id 61 and the probe id 62, and fragKey includes the identification, so the probe opened a NEW key — the one arrangement in which the sweep runs even when it runs only on new keys. Moving r.sweep(now) inside the `entry == nil` branch left the test green. The probe is now the SECOND fragment of a datagram whose first fragment is already cached, with the two entries opened half a fragTimeout apart so the stale one is past its deadline and the live one is not (deadlines are set at creation and never refreshed). Two assertions before the probe pin the setup: the stale entry must still be there, and the live key must already exist — if a later edit breaks either, the test says so instead of quietly proving nothing. The released bytes are checked too, which is the half of the timeout this test is about (the correctness half is already caught by TestBridgeFragmentTimeout). Same sweep of TestBridgeFragmentMalformed, which had the same shape of hole: a FIRST fragment carries MF=1 and can never complete a datagram, so `got != nil` is unreachable whether the packet was refused or accepted, and "truncated header" asserted only that. Every subtest now asserts on the cache, and a case for the classic overread — a header claiming TotalLength 276 in a 28-byte buffer — is added; its control is the aligned subtest already at the bottom. Mutations (linux, -race): sweep moved into the new-key branch fails SweepIsPerCall by name; clamping TotalLength to the buffer instead of refusing fails the new malformed subtest — and, as predicted, leaves its `got != nil` assertion silent. Control: moving the sweep after the entry lookup while keeping it unconditional keeps every test green, so the test discriminates "per call", not "the line moved". No production code changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
42d84ac74c |
fix(model): a failed backup may stop a config write only when the filesystem is the reason
backupBeforeChange was added with "any failure aborts the write", justified by "the uci commit that follows writes the same filesystem, so whatever stops one stops the other". That holds for a full or read-only /overlay and for nothing else — and the existence probe is a stat, which also returns ENOTDIR (something dropped a file where /etc/shater should be), EACCES, ELOOP. In that state PUT /api/config answered 500, `sub update` exited non-zero and the profile watcher stopped saving, PERMANENTLY: none of those causes clears itself. A convenience added this wave must not be able to take the product away. Two changes, both about not inferring what can be measured: - The probe is not evidence. stat(dest) answers "is this transition already captured?"; when it cannot answer, the copy is now ATTEMPTED and the attempt is the measurement. Only "the filesystem will not take bytes" short-circuits it. - The failure is classified. filesystemRefusesWrites is a positive, CLOSED list — ENOSPC, EROFS, EDQUOT, EIO — each a condition under which the uci commit would fail too, so aborting only changes which error the operator reads and ours names the cause. Everything else is about the backup's PATH and falls to the recoverable side: the config is saved, and the missing undo is NAMED through reportBackupProblem (same shape as subCacheLogf; model cannot import logsink, which imports model) rather than skipped in silence. TestWriteAbortsWhenTheBackupCannotBeWritten used a FILE where the backup directory should be — that is ENOTDIR, the exact case that must no longer veto — so it now injects ENOSPC at the copy, and the ENOTDIR case moved to TestBackupPathFailureDoesNotVetoTheWrite. statBackup/writeBackupFile are seams because the two deciding failures are the two a temp directory cannot produce. Mutation-checked (linux, -race), each with the other half green: restoring "any failure aborts" fails only the two carry-on tests; "nothing aborts" fails only the two abort tests; restoring the old stat handling fails only the test that pins "attempt the copy"; dropping ENOSPC from the list or adding ENOTDIR to it fails the classifier test and the end-to-end tests that depend on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
a05ad21b39 |
fix(apply,shaterd): four states the daemon was in and could not say
1. A SWITCHED-OFF SUBSCRIPTION CAN STILL GO OUT ON THE PLAIN WAN (blocker). Three places had to agree about `enabled=0` and did not: UpdateSubscription resolves by name and never reads it; cmdSubUpdate reads it only when no name was given; warnings.go skipped disabled subscriptions entirely on the stated premise that one "is never fetched". The premise was the false one, and the per-row Fetch-now button added this wave posts exactly the named request. Kept the behaviour, dropped the premise. Enabled means "include in the automatic refresh" everywhere else in the system — MergeSubCaches loads a disabled subscription's cached nodes unconditionally and they route traffic — and a refusal here is worked around by enable/fetch/disable, which enrols the sub in the 6-hourly sweep and is strictly worse. The automatic paths still honour it (the nameless sweep, and shater-cron's own `en = 1` check). The named path now says so on stderr and in the daemon log, and the leak finding fires for disabled subscriptions with the WHEN clause corrected — "every scheduled refresh" is false of a subscription no schedule touches. 2. refreshBootArmor DISARMED THE NEXT BOOT FROM A CONFIG THE DAEMON REFUSES. Every call site is gated on readErr == nil and nothing else; ParseUCIExport drops unknown options silently, so a config written by a newer build reads clean, and with the divert set emptied by the parse RenderHoldNft returns "" and the armor was REMOVED — with no log line at all, unlike the disarm one branch above it. Measured: with the new gate removed, the armor really is deleted. Now gated on the schema, and both removal paths are announced. 3. THE FIRST-BOOT DEADLOCK IS NAMED. Every subscription pulled through the tunnel, the tunnel built from nodes only a fetch supplies, the caches gone: the fetch waits for the tunnel and the tunnel waits for the fetch, forever, with the LAN dark. The CLI refusal goes to /dev/null (shater-cron) and the daemon line to a syslog `log_syslog='0'` switches off. It is now a critical finding in /api/status, which survives both, with the state named and two escapes — the free one first, the costly one priced. 4. A REJECTED CONFIGURATION WAS INVISIBLE, AND THE ENGINE CHURNED. Measured on the stand: with a config the engine cannot accept on disk, cron retries every 60s and every attempt is a full engine swap, while status showed engine_running=true, the OLD hash, the OLD warnings, and `grep -ci` for the broken element returned 0. Invisible by construction: everything published about a config is published by a SUCCESSFUL apply, and engineDownCause is gated on the engine being down — here it is up. Status gains config_applied / apply_error / apply_error_stage / apply_attempts / apply_failed_since_unix, and a critical finding that says the running configuration is a DIFFERENT one and names the reason. Reconcile paces an identical retry (three free attempts, then doubling to a 15m cap); any change to the configuration cancels the wait, and POST /api/apply is deliberately not paced. The post-swap abort is deliberately NOT recorded — it is already loud and its retry costs no swap. Also, from review-by-seams: - The netplane channel was graded critical wholesale over three distinguishable states. `udp '0'` + closed is the kill switch doing what it was told and may be exactly what was asked for; the leak and the total cut-off are not. The first is now `warning` (not `info`: attentionFindings drops info, and the blast radius is wider than the switch's name). Default stays critical, the exception is a closed list, and netplaneprotoseverity_test.go pins it against the REAL renderer so a rewording fails by name instead of drifting. - devicefilter_severity_test.go carried a FOURTH unlinked copy of DEVICE-FILTER-NOT-APPLIED and compared it with itself — the same shape as the noGatewayFinding fixture this wave removed. apply's copies are one constant now, and the real coupling is a test that runs generate and grades what comes back. Mutation: renaming the tag in generate fails it by name; the two old fixture tests survive that untouched, which is the whole point. - The history-write failure was logged ABOVE the deduplication gate its own call site documents eight lines below. At one cron reconcile a minute a standing cause (full /overlay, an entry over the 128 KiB ceiling) wrote 1440 identical lines a day, and under log_persist=1 that many appends to flash — the exact wear the history ring's own dedup exists to prevent. Now gated on the message changing, cleared by a success. The Warning is still returned every time; only the log had a repetition problem. Every fix mutation-checked with the failure text recorded, and every one has a control showing the instrument can still give the opposite answer: an enabled subscription still fetches and keeps the scheduled wording; a legitimate disarm still happens and is still logged; a healthy box raises no rejected state; an ordinary netplane finding is still critical; a DIFFERENT history failure still prints. One mutation (the history-dedup latch) SURVIVED its first test — the counter matched the success path's Info line too — and the test was fixed. shater/apply is green. shater/cmd/shaterd was green when run 20 minutes ago and now fails to BUILD on shater/panel/byedpi.go, a neighbour's in-flight refactor; the full gate run for the same reason cannot be completed on this tree right now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
43bb8913ea |
fix(diag): the bundle printed a DNS account id, and scrubbed the wrong file
Two holes, both in the direction the verb cannot afford: `shaterd diag` produces
the one text block a person SENDS somewhere.
1. resolver.address was on the allow-list, printed verbatim. For a DoH resolver
that field is a URL, and generate/dns.go's parseDoHAddress keeps and USES
u.Path — which is exactly where NextDNS, AdGuard and Control D carry the
account identifier. Whoever holds it reads and rewrites this household's DNS,
so it is a credential. The panel had always read it that way (DNS.tsx's
resolverAddr shows u.host and flags the rest); the disagreement was resolved
in favour of the side whose output goes to a stranger. Now: scheme and host
survive, userinfo/path/query/fragment do not, and a BARE address
("1.1.1.1", "dns.adguard.com:853") is still printed in full because it is
host and port and it is what the fault is read from.
The fix could not be "delete the key from the list": TestDiagMasking-
IsClosedOverTheWholeModel asserted the allow-listed fields come out
UNMASKED, so it actively pinned the leak. The transform lives in a second
closed table (diagMaskedForm), and the sweep now compares the masked render
against the raw one line by line, expecting either the plain mask or exactly
what that table declares.
2. The second layer collected its literals from `uci export shater` alone, and
that file does not hold this router's credentials. model/render.go never
writes a FromSub node; the several hundred subscription nodes live in
/etc/shater/subs/*.json, which keep.d/shater-core describes in its own words
as carrying "every node's credentials". The reachable path is not
hypothetical: parse/sharelink.go quotes a rejected node's USERINFO into its
error, generate/outbound.go warns it, apply/warnings.go logs it, and the last
32 KiB of that log is section six of the bundle — with LogToFile on by
default. diagSubCacheSecrets now reads those files by the same closed
positive-list rule (unknown JSON key => collected, so a field added to
model.Node tomorrow is covered), and a file it cannot read is NAMED in the
bundle instead of silently reducing the scrub.
Fixing the first half exposed the second: the log carried the userinfo, not
the whole URI, so a literal scrub of the URI walked past it. diagSecretParts
expands every refused value into its userinfo, username, password, query
values (encoded and decoded) and path. Not the fragment — in a share link
that is the node's display name, which is on the printable side.
The banner no longer says secrets are masked "throughout". It says what is
masked, and then names what is still in there: values under 8 characters (masked
in the config, not scrubbed elsewhere), list/ruleset URLs, and the limits of a
literal scrub.
Mutation-checked, each with the control that the instrument SEES the planted
secret in the unfixed output:
resolver.address back on the allow-list -> resolver test fails on the id
diagMaskAddress made the identity function -> transform test names the field
sub-cache literals withheld from the scrub -> log-scrub test fails
diagSecretParts reduced to the whole value -> log-scrub test fails
sub-cache safe list turned into a blocklist -> closure test fails
unreadable cache file swallowed -> honesty test fails
nil collector seam read as "nothing to do" -> honesty test fails
transform applied per section, not per key -> new-field test fails
one section given an open default -> sweep fails, by name
an allow-listed value over-masked -> sweep fails, by name
masked lines dropped entirely -> sweep's vacuity guard fires
scripts/run-tests.sh green (all 7 steps, -race included).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
89571bcdb1 |
fix(netplane,generate): a network with option udp '0' had its UDP dropped in silence
Two defects of the same family: a state the plane produces and nobody names. 1. The divert is written per PROTOCOL, the fail-closed drop per INTERFACE. A tproxy inbound with `option udp '0'` puts its device in the drop scope (nftDivertRefs does not look at the flags, and must not: the drop is the backstop for ESP/GRE/SCTP too) while emitting no UDP TPROXY line for it. With kill_switch=closed every outbound UDP packet from that network is dropped; with kill_switch=open the same packets leave the WAN in the clear. TCP works, DNS works (dnsmasq answers it past the fib-local bypass), so it presents as "some sites do not load", not as a firewall. Verified by rendering: no second LAN is required, the shipped one-inbound shape does it. The drop is NOT narrowed to match the divert. Doing so would turn `option udp '0'` — which is how you kill QUIC so the engine can route by SNI — into "UDP now bypasses the proxy", i.e. it would convert a QUIC-blocking config into a QUIC-leaking one, and it would open a per-protocol hole in the kill switch through a knob whose name says nothing about leaking. The plane already takes the other decision one field over: with ipv6 off no v6 divert is emitted and closed mode drops v6 anyway, deliberately and in writing. So the state stays and is named instead, in three shapes (protocol dropped / protocol leaked / both flags off), each naming the network, the option, the kill-switch state and the concrete traffic that dies. coverage.go could not have caught this: it skips covered[i.Device], and the device IS covered. The new check is derived from the model alone and so runs outside that file's Interfaces() gate. 2. networkList's open `default:` sent (tcp=0, udp=0) to "" — which the engine reads as BOTH — so an inbound the plane feeds nothing acquired a listener for everything. The four cases are now named and closed, "neither" is a second return value rather than a synonym for "both", and a tproxy inbound that carries no protocol is refused with a warning that also names the netplane half: switching both flags off does not remove the network from the plane, it removes the way out of it, so closed mode cuts that network off completely. generate_test.go: the three linux fixtures that built a tproxy inbound with model.Inbound's zero-value flags now spell TCP/UDP out. UCI defaults both to true; only a Go-built model gets false, and only that fixture relied on it. Gate green (bash scripts/run-tests.sh, exit 0). Every new test mutation-checked in both directions: suppressing the warning fails 6 tests by name, and making it fire unconditionally fails the controls. Rendered ruleset text is byte-identical for all nine shapes dumped before/after — the only diff is added warning lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
6ced96fafa |
fix(panel): a switched-off subscription is not "never fetched", and an inline list is not "empty"
Two things the panel asserted that the code does not do.
1. THE OFF SWITCH IS NOT A GATE ON FETCHING. The row badge for a disabled
subscription with `fetch_via=proxy` and no detour was drawn quiet and said
"Nothing is disclosed yet — this subscription is switched off … so it is never
fetched". False in all three places that could have contradicted it:
Applier.UpdateSubscription resolves a subscription BY NAME and has never read
Enabled; `shaterd sub update` consults Enabled only when no name is given; and
this panel's own per-row "Fetch now" — new in this wave, previously buried in the
collapsed Options panel — is disabled on `busy || fetching` and nothing else. One
click sent the router's real address to the feed host under a badge saying
nothing was disclosed.
The two halves of the old condition are not alike, so they stopped being one
state. NO URL is real and refused at the bottom (subscribe/fetch.go rejects an
empty URL before it builds a request) — that branch keeps its quiet badge. OFF is
amber, and its sentence says what the switch actually does: it stops the
scheduled refresh, and the button on the row asks for a fetch whatever the switch
says.
The daemon reached the same conclusion from its side in this wave — the fetch is
deliberately allowed and logged, and its finding now varies on Enabled — so the
badge's own summary over that finding varies the same way. "On every scheduled
refresh" printed over a switched-off row is the same lie inverted: it sends the
reader hunting a cron job that is not running.
2. AN INLINE LIST HAS NO ENTRY COUNT, AND "NOT PUBLISHED" IS NOT "EMPTY".
engine.go fills RuleSetStat.RuleCount from (*rule.RemoteRuleSet).RuleCount(), and
LocalRuleSet.RuleCount does not exist in the tree at all, so an inline list always
arrives with rule_count 0. The chip called a working parental-control list
"empty — nothing matches". It reads "size unknown" now: unlit, never green and
never the amber that says something is wrong. A mixed group is counted as a floor
("1,284+") instead of presenting a partial sum as the whole.
BOTH INSTRUMENTS WERE HOLDING THE LIE UP. subFetch.test.ts pinned the sentence
verbatim, and deviceLists.test.ts fixed `{remote:false, rule_count:3}` — a record
no router can produce, so its green light was wired to nothing. The mock carried
the same impossible state on three local rule-sets and fabricated a count on
update. All of them now match what the daemon sends.
Verified in ?mock (new `?subleak=paused|pausedapplied|nourl`), 390 and 1280, both
themes. Each fix reverted in turn with the failure text; controls both ways — a
remote list that really is empty still says "empty", and the two leaking states
are still told apart from the one that is genuinely quiet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
42536675e2 |
fix(panel): read the age, the scope and the hop — three fields the panel was ignoring
The daemon changed under the panel in three places, and in each one the panel kept drawing a screen that was right only by accident. byedpi readiness carries `age_seconds` now, because GET /api/status stopped probing: sixteen enabled instances on ports that neither accept nor refuse cost 6.41 s per poll, measured, and the Apply page polls up to 27 times a minute. The report is served from a cache and every sentence in it is present tense, so the panel stamps it. Negative is not an age — zero is the common answer (a loopback connect finishes in microseconds) — so "not a measurement" is its own reading, and a daemon too old to send the field is a third one: the reading is real, its age is not reported. The cold first poll after a start says "not measured yet" rather than "not determined": nobody has looked is a normal state of a router that booted ten seconds ago, and it calls for a different sentence than an instrument that looked and failed. Both keep the unlit lamp and both keep the egress type locked. A test run no longer wipes the board, so a card can show a reading from twenty minutes ago beside one from a second ago. Which is which comes from `scope`, not from comparing timestamps — the router has no RTC and a computed "n minutes ago" would be fiction. A carried row says "earlier run" and is drawn as a qualifier; an empty scope is "cannot attribute", never "everything is carried", because a real run always covers at least one target. A chain blocked at a hop was kept red by matching a fragment of the daemon's error sentence — the last place prose decided anything here. It arrives as `blocked_by` now, so the match is gone and the row names the hop. The mock carried the old contract: it emptied the board on every run while a comment claimed the daemon did too. It carries forward now, by (name, kind), capped at 64, and `?mock&board=carried` lands on a finished board holding both kinds of row. `?mock&byedpi=cold` and `?mock&byedpiage=N` reach the two states the freshness rendering exists for. Verified in ?mock at 390 and 1280, both themes, no horizontal scroll. Each of the three fixes was reverted in turn and the tests named the failure; the controls run the other way too — a helper that marked every row carried, or reddened every row, fails just as loudly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
a4ea5dba44 |
fix(tests): the last two packages that wrote to the router's own /etc/shater
|
||
|
omarandClaude Opus 5
|
ea34e744bd |
fix(panel): the status poll stops dialling — a readiness cache that carries its age and has an owner
GET /api/status called byedpiProbe on every request. On a healthy loopback that
is nothing, but the probe's cost lives in exactly the state it was written to
report honestly: a port that neither accepts nor refuses burns the full
byedpiDialTimeout, and byedpiMaxInstances of them burn 6.4 s. Measured, on this
tree:
1 enabled instance, live listener 0.45 ms
1 enabled instance, refused 0.33 ms
16 enabled, refused 3.6 ms
1 enabled, black-holed 400 ms
16 enabled, black-holed 6.41 s
The panel shell polls /api/status every 5 s on every page and the Apply page
adds its own 4 s poll, so the pathological state hung the panel for seconds at a
time precisely while an operator was trying to find out what was wrong. A check
that gets slow exactly when it matters is worse than one that is always slow.
The poll now reads a cache (byedpiReadiness.cached), refreshed asynchronously off
the same path: 15 ns per call, primed, and 20 back-to-back polls against 16
black-holed ports cost less than one probe. A background ticker was rejected —
it would dial on a router whose panel nobody has open — and so was blocking the
first poll to fill a cold cache, since that is the same 6.4 s hang, just rarer.
The cache is not allowed to lie:
- every served report carries age_seconds. Detail is written in the present
tense, and a present-tense sentence about a measurement taken some seconds
ago is a claim nobody checked;
- a report older than byedpiCacheMaxAge is NOT SERVED. It is replaced by an
explicit unknown with a negative age, so a panel that ignores the age fails
to an unlit lamp rather than to a stale "listening" unlocking an egress type
onto a port nothing is on;
- GET /api/byedpi still really connects. A re-check button answered from a copy
is a button that does nothing.
And it has an OWNER. The refresh runs a goroutine that dials; left as a package
variable it belonged to nobody, could not be awaited, and — as the race detector
showed — went on reading byedpiConfigPath / byedpiInstalled / byedpiDial after
whatever started it believed it was finished. The cache is now per-Server, with
stop() that forbids further refreshes and does not return while one is dialling,
called from Server.Close. The daemon already defers that Close, so the probe
cannot outlive the server.
byedpiDial became a seam alongside byedpiInstalled and byedpiConfigPath: the
timeout branch is the expensive one and the one a real loopback cannot be
provoked into, so without it neither the cost nor its removal could be shown.
Ten mutations, each killed by a named test: the probe back on the request path;
the cached copy claiming age 0; an over-age reading quoted anyway; a cold cache
returning a blank instead of an explicit unknown; a refresh that is not
single-flight; a late older probe overwriting a newer one; /api/byedpi answering
from the cache; the unmeasured report claiming the binary is absent; stop() not
waiting; Server.Close not stopping. The last one survived its first test — which
asserted a poll straight after Close did not dial, and passed with the stop
removed entirely because the reading was fresh and no poll was due — so the test
now advances the clock to make it due.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
e5dfd74b61 |
fix(engine): testing one node stops wiping the group board; the blocked hop becomes a field
Two separate honesty defects in the shared test board, both surfaced while
closing the byedpi readiness fix.
1. startTestRun replaced the results slice outright, so pressing Test on one
NODE blanked every group and chain card on the Targets screen, and testing a
group blanked the nodes. Nothing on screen explained it, because nothing had
happened to those targets — the daemon had thrown their readings away.
Earlier results are now carried forward for every target the new run does not
itself re-measure. The alternative, one board wiped per run, is simpler and
has no staleness question at all; it was rejected because it destroys
information the daemon still has. These are the OBSERVATORY's numbers, taken
by a prober that never stopped, and a group's reading does not become false
because somebody tested a node afterwards.
The staleness question it does raise was already answered: every result
carries tested_unix, the instant the OBSERVATION was taken, and GroupTestStatus
publishes this run's scope — so a carried row is identifiable as carried
without comparing timestamps, and drawn with its age. The board is capped at
groupTestCarryMax, evicting the oldest first; that cap is the only way a row
can leave without a newer one taking its place, and it is documented as such.
done/total still describe this run's targets only.
2. A chain whose exit was never dialled, because an earlier hop was probed and
did not answer, shipped that fact as prose only: source="" (correct — nothing
measured the exit) plus a sentence naming the hop. A client reading source
strictly filed it under "nobody looked", which is the wrong colour, so the
panel kept the row loud by matching a fragment of our error message — the
last place it read our prose to decide anything.
GroupTestResult now carries blocked_by: the 1-based hop index, 0 everywhere
else. It does NOT set source; nothing measured this target's own path, and
stamping an instrument on a measurement that never happened is exactly the lie
source was added to prevent. blocked_by>0 beside source="" is the complete
statement. Field and sentence are produced together in chainBlockedResult so
they cannot come to disagree.
Tests (grouptest_board_test.go), each verified by mutation:
- carrying forward is asserted WITH its control, that a run does replace the
rows it covers — "nothing disappeared" alone is also satisfied by a board
that stopped updating;
- target identity is (name, kind), so a group and a node of one name do not
evict each other, with the empty-kind wildcard pinned both ways;
- the cap drops the oldest end;
- blocked_by carries the hop, keeps source empty, and every other result
carries 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
17843be5ad |
fix(tests): running the suite deleted the router's own state — four packages did it
shater/stats/store_test.go ended with
_ = os.Remove(statsFilePath())
and statsFilePath() is not a test path. It is THE product path: /etc/shater/stats.db
on every host where that directory exists, which is the testbed and the router. So
`go test ./shater/...` deleted the accumulated query and connection log of whatever
machine ran it. The test passed. It had always passed — damage done by a test is a
side effect, not a wrong answer, and no instrument in this tree could see one.
A filesystem sweep (the new scripts/check-test-fs-isolation.sh: seed a router-shaped
canary tree in a container, run the whole gated suite, diff) found it was not alone.
Six packages, by measurement, not by reading:
shater/stats DELETED /etc/shater/stats.db (the line above; also
TestComboBackendSwitchSequence opened and pruned the
live DB, which the delete had been hiding)
shater/logsink DELETED /etc/shater/shaterd.log and /var/log/shaterd.log —
New()/Reconfigure() purge BOTH product locations when
the file toggle is off, so Config.Path (which every test
here already set) never protected them. The daemon's own
log, the one an operator reads after an outage.
shater/apply DELETED /var/run/shater.active — the ONE token hotplug and cron
check before touching the data plane. Clearing it on a
live router makes both stand down on a box that is up.
holdstate_test.go's `t.Cleanup(os.Remove(ActiveFlag))`
was not a cleanup; it was the delete.
shater/panel REWROTE /etc/shater/stats.db — stats.NewStore("sqlite") from
TestStatsEndpointsAcrossBackends resolves the product
path too.
shater/model CREATED /etc/shater/config.pre-v{0,1,2}.bak, config.pre-unreadable.bak
shater/generate REWROTE /etc/shater/cache.db
The last two are NOT fixed here — another agent is working in those trees. Both are
one TestMain away: model already has liveConfigPath/configBackupDir as vars, and
generate already has cacheFilePersistent; what leaks is product code (backupBeforeChange,
the engine's cache_file) called from tests that do not redirect them.
THE FIX is the seam generate/cache.go and generate/ruleset.go already use — the path
becomes a package-level var that only tests assign — plus, in each case, a test that
still pins the SHIPPED value, because an isolation that leaves the real decision
untested has only moved the defect:
stats: statsDirPersistent/statsFilePersistent/statsFileFallback + the exported
SetPathsForTest (exported because shater/panel needs it from outside).
New TestStatsFilePathPrefersPersistentDir covers both branches.
logsink: PersistPath/TmpfsPath + a TestMain, since the hazard is in New(), which
every test calls. New TestLogPathsAreTheShippedOnes.
apply: ActiveFlag + the existing TestMain. New TestActiveFlagIsTheShippedPath,
which also records WHY /var/run: tmpfs, so a reboot clears it.
TestNewStoreSelection got stronger rather than weaker. Its "sqlite" case used to
accept "sqlite" OR "memory" because the real path might not open on this host — an
expected value that depended on the machine. At a private path there is no excuse:
a writable directory MUST report "sqlite", and a new control at an unopenable path
MUST report "memory" (the honest "persistence is not active" signal) without a crash.
TWO GUARDS, because one of them cannot see half of it:
shater/testguard/fsisolation_test.go — parses every _test.go under shater/ and
fails BY NAME when a filesystem-mutating call gets a path that is not PROVABLY
temp-rooted. Positive and closed: what it cannot prove is a failure, not a
default, which is the only rule that catches a path built by a function call.
It follows local vars, closures, filepath.Join/Sprintf/+, helper parameters via
their call sites, helper return values, and the save/override/restore idiom.
Four waivers, each keyed on file+function+callee, each with the reason printed on
every run, each a struct field traced by hand; a waiver that stops matching fails
the test as STALE. Runs inside [2/7] and [4/7] — no new gate step, no new minute.
Blind spot, stated: damage done by PRODUCT code a test merely calls (which is
exactly logsink, model and generate above).
scripts/check-test-fs-isolation.sh — the dynamic half, for that blind spot. It
refuses to run outside a container unless told twice, because its method is to
let the damage happen and then look, and it seeds/unseeds only what was missing.
Verified:
- mutation, task 1: statsFilePath forced to the fallback -> the new path test
fails ("with ... present = .../fallback-stats.db, want the persistent ...");
newPersistent forced to memory -> "Backend = \"memory\", want \"sqlite\"";
the fallback made to report "sqlite" -> "Backend = \"sqlite\", want \"memory\"".
Green again after each revert.
- mutation, the guard: the original os.Remove(statsFilePath()) put back -> named
at store_test.go:154 with "the path comes out of statsFilePath(), which this
check cannot follow"; a planted test writing "/etc/config/network" -> named as
a literal path; the walk pointed at one package -> its own <150-file control
fires ("reading a blank page"); a waiver matching nothing -> STALE WAIVER.
- control, the sweep: with a planted violator it reports DELETED /etc/shater/stats.db
and MODIFIED /etc/config/network; without it, those are gone and only the two
foreign packages remain. Its bisect named shater/stats.TestComboBackendSwitchSequence
on its own.
- counts, declared vs executed (go test -list against top-level verdicts):
stats 112/112, panel 121/121, apply 122/122, logsink 26/26, testguard 1/1,
0 skips, 0 failures.
- scripts/run-tests.sh: [1/7][2/7][3/7][5/7][6/7][7/7] green. [4/7] -race fails on
four TestByeDPI* in shater/panel — a data race between byedpi.go's background
probe and byedpi_test.go's forceByeDPIBinary cleanup, in another agent's
uncommitted work (shater/panel/byedpi_cache_test.go is untracked). Proven not
ours: a pristine HEAD tree carrying ONLY this commit's files passes -race over
all 35 packages, and the same run with -skip ^TestByeDPI is green on the live
tree too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
dac2f85c84 |
fix(insights): a failed DNS lookup stops being the healthiest row in the log
The DNS log drew every row from `action`, which answers WHICH WAY the lookup
went — so a query that left through a detour and then timed out came back as an
accent-coloured `proxy` tag, blocked=false, nothing else said. The row that
describes the exact moment the tunnel broke was the most reassuring line on the
page. `actionTag()` also fell open (`return 'pass'`), so every value the panel
did not recognise — including every value a future daemon might add — rendered
green.
The daemon now carries the outcome as its own axis (LogEntry.Status/Error,
|
||
|
omarandClaude Opus 5
|
e065786a32 |
feat(panel): unlock byedpi on a listener, test one node, and stop two badges arguing
Three things the panel was saying that it had not established.
BYEDPI. The egress type unlocked on `status.byedpi_installed`, which is
LookPath("ciadpi") — "is the package installed", while the operator is asking
"will traffic sent here go anywhere". They come apart on the SHIPPED config: the
packaged /etc/config/byedpi is inert, so installing the package unlocked the
type, the egress went on 127.0.0.1:1080, the apply was green and nobody was
listening. The gate is now `byedpi.state === 'listening'` and nothing else
(byedpiReady.ts, the only place that decides it). GET /api/byedpi also carries
the per-egress port cross-check, so a mismatch is drawn on the row that has it,
naming both ports, in crit — the state where every other signal reads healthy.
`unknown` is neither answer. It keeps the type locked (a control that opens on
nothing established is the same defect wearing a new word) and it is never
painted as a refusal: dashed border, unlit lamp, "not determined", plus a
Re-check button so a dropped request is not a dead end.
ONE NODE. A freshly pasted node had no instrument — the group test reads the
observatory's board and the observatory only probes what the rules route
through, so the first question anyone asks answered "not routed by any enabled
rule". Every node row now has Test, over the same singleton run and the same
GET poll the Targets page uses.
The reading is classified on `source`, not on prose: measured-and-failed is red,
`source:""` is an unlit lamp and the faintest text on the row, because a
negative result that cannot be told from a check that never ran answers nothing.
One escalation survives, documented and narrow: a chain whose exit was never
reached because a hop it runs through WAS probed and failed. Targets keeps its
exact previous appearance while its instrument changes underneath.
The three refusals stay three facts — 404 the node is not in the saved config,
503 the config could not be read (an unknown, never a verdict about the node),
400 no name — with three tones and three sentences.
SUBSCRIPTION FETCH ROUTE. The panel's draft predicate drew amber "proxy · no
route" while the daemon now grades the same fact critical on the same row, so a
saved leaking subscription wore both, at two severities, about one thing.
subFetch.ts reconciles them: where the daemon has spoken it outranks the
prediction, in BOTH directions — including the dangerous one, where the
predicate is content ("via group:auto") and the daemon reports the leak anyway.
The prediction still speaks for a draft nothing has applied yet, and a
subscription that is switched off is not accused of a disclosure the daemon
deliberately does not report for it.
Fixtures reach every state: ?byedpi=<five states>|mismatch, ?nodetest=ok|dead|
unmeasured|400|404|503, ?subleak=draft|applied|divergent. The group-test fixture
also stopped being kinder than the daemon — engine.startTestRun replaces the
whole board, so refreshing one target really does blank the others.
42 tests, 8 mutations each killed by name, and both controls: the gate is shown
to open on `listening` and to stay shut on the other four, and "not checked" is
shown to be drawn differently from "did not answer" — the assertions fail if
either pair is ever drawn alike. Verified in the browser at 390 and 1280, both
themes, no horizontal scroll.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
46a2d4aaad |
test(model): pin the wire contract the panel's PUT actually sends
handleConfigPut decodes model.Model with DisallowUnknownFields, so this is the layer the missing fields bit at: not "the attachment is ignored" but "the whole save is rejected with json: unknown field \"Blocklists\"", losing every unrelated edit batched into the same PUT. Mutating the JSON name reproduces that message exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
67290ec2b6 |
feat(devices): attach named block/allow lists to a device, without a dangling tag
The panel half already ships Device.Blocklists/Device.Allowlists and PUT /api/config decodes with DisallowUnknownFields, so until now the first attachment rejected the WHOLE save with `json: unknown field "Blocklists"`, losing every other edit in it. This is the engine half. A device now references `config blocklist` / `config allowlist` sections by name (UCI: `list blocklist` / `list allowlist`, since `block`/`allow` already mean the typed domains), which brings geosite categories and url-sourced lists to parental control for free. Two things here are constructions, not checks. The tag a device's rule references comes from the accumulator that emitted the rule-set, never from the list name. Rule-set tags resolve at engine START (RuleSetItem.Start), so a name-derived tag passes box.New and fails box.Start — and because both configs share one cache_file path, every apply on a live engine takes the close-old-then-start-new branch, so the old box is already gone when the new one refuses. That is no engine, a closed kill switch and a dark LAN, from one mistyped list name. A reference that yields no tag emits no rule at all; the emptiness is warned, tagged DEVICE-FILTER-NOT-APPLIED so the panel grades it critical rather than guessing from prose. Materialisation is a single memoised point shared by both consumers. Devices are built before the network-wide filter, so materialising a shared list twice would hand dedupeRuleSetTags an already-claimed tag — which it DROPS, silently switching the network-wide filter off for that list. The mutation test for this reproduces exactly that: DNS-FILTER-NOT-APPLIED, filtering nothing. Order is the feature: typed allow, typed block, attached allow, attached block, then the network filter. Otherwise a parent who types youtube.com into a child's Block loses to whatever an attached geosite category permits, and the panel draws a "Blocked" chip over a rule that does nothing. Typed and attached matchers stay SEPARATE rules — rule_set AND-gates over the domain matchers, so merging them would mean "the domain AND the list". Attaching a list is itself the switch for that device: Enabled=0 means "does not participate in the network-wide filter", not "dead", so the list still loads and filters here — and generate says so instead of leaving it to be discovered. A blocked name's reply comes from the LIST (Blocklist.Response), so tier 4 is up to two rules; the typed tier keeps NXDOMAIN, having no owning object to say otherwise. Purely additive: an old config has neither list, parses to nil, and the generated engine config is byte-for-byte what it was. No schema bump. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
9339e8e70d |
fix(generate): the cache fallback test ran or skipped depending on its neighbours
TestCacheFallsBackWhenDirMissing guarded itself with
if fi, err := os.Stat(cacheDirPersistent); err == nil && fi.IsDir() {
t.Skipf("%s exists on this machine; ...")
}
i.e. its subject was the machine it happened to run on. The gate caught it as an
UNDECLARED SKIP in one container run and not in the next, with no change to the
code — and BOTH outcomes were green. Only the undeclared-skip check saw it at
all; every other instrument here reports `ok shater/generate` either way.
An order-dependent test proves nothing on the runs where it does run either,
because nobody can tell afterwards which runs those were.
The three cache paths become vars (production never assigns them, same seam
generate/ruleset.go already uses for listsDirOverride) and the test points them
at a temp tree. It now covers BOTH branches with no skip: an absent dir must
choose tmpfs, and — the control — a present one must choose the persistent
path. Without that second half the test is satisfied by a cacheFilePath that
returns the fallback unconditionally, which is exactly the regression the
persistent branch exists to prevent (a cache that never survives a reboot, so a
reboot before the WAN is up fails to start the engine and takes the LAN with it).
Verified:
- both halves killed by mutation (force persistent -> the first assertion
fails; force fallback -> the control fails), green again after revert;
- 5 x `go test -shuffle=on ./shater/generate/`: 474 verdicts and 3 skips
every time, TestCacheFallsBackWhenDirMissing PASS on all five, never SKIP;
- shater/generate: 385 declared func Test*, 385 top-level verdicts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
a67f51c22c |
docs(panel): the q= field list did not mention error, which the filter searches
shater/stats/filter.go:156 searches ConnLogEntry.Error along with the six fields the doc names, so `q=timeout` works and the contract said it did not. Verified against the predicate, not against a report. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
56c9ea56e6 |
fix(gate): [4/7] reported a failure that did not exist — 664 s of pure sleep
The -race step failed with
FAIL shater/netplane 600.019s
panic: test timed out after 10m0s
running tests: TestApplyIfaceSysctlsCoversRuleDivertedIface
over code that was neither hung nor wrong. Measured (golang:1.26, 32 cores):
shater/netplane is 1.971 s without -race and 663.762 s with it. A 337x factor
is not "-race is slower".
Nine of netplane's test files intercept nft/ip/ubus/uci/sysctl by re-exec'ing
the test binary as a no-op helper — the standard os/exec trick. Under -race
that child is ThreadSanitizer-instrumented, and TSan's atexit_sleep_ms DEFAULTS
TO 1000: every -race process sleeps a flat second before exiting, on no CPU.
~660 intercepted commands, one second each. The per-test times said so out
loud — 12.17 / 13.18 / 14.17 / 129.62 s — they were counting, not measuring.
Isolated, five runs each, of a `func main() {}` with nothing in it:
built plain 0.0014 s/run
built with -race 1.010 s/run
built with -race, sleep disabled 0.008 s/run
So the children now run with GORACE=atexit_sleep_ms=0, set once in a package
TestMain rather than in each of the nine fakes (they all build the child env as
append(os.Environ(), ...), so one assignment covers the ones written later too).
TSan reads GORACE at process init, long before TestMain, so the detector of the
test process itself is untouched; only the children see it, and they do nothing
but write a canned string and exit. Proven, not assumed: a deliberate data race
in netplane is still reported under -race with this in place.
shater/netplane 663.762 s -> 10.625 s (203 === RUN and 128 top-level
verdicts on both sides)
shater/devices 28.412 s -> 0.358 s (same disease, same cure)
gate [4/7] end to end: was a 600 s timeout, now 56 s
WHAT THE GATE ITSELF WAS MISSING. A deadline and a failed assertion both exit
non-zero, and this script printed the same "FAILED [race]: go test exited 1"
for both — so the reader could not tell "the product is wrong" from "nobody
knows yet". [2/7]/[4/7] now name a timeout as a TIMED OUT, list the tests that
were still running, print only the goroutine dump instead of a quarter megabyte
of PASS lines, and spell out the two opposite fixes (a block, or slowness that
must be MEASURED first). Verified both ways: a sleeping test reads TIMED OUT, a
t.Fatal still reads FAILED.
The deadline stays at go test's own 10m, now written down with the measurement
beside it, and stays there as the hang detector — the slowest package under
-race is 18.9 s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
cae5655dbe |
fix(panel): the hazard band said DNS leaves and that nothing leaves
Measured on the stand, real LAN clients in netns behind veth, counters in a
separate nft table: in the state this band predicts, 0 packets left the WAN
across the whole run, against 27 in the control that differs only by one added
catch-all rule. Except for exactly 2 — both plaintext UDP/53. So the band's
detail ("nothing reaches the internet") was wrong by those two packets, and its
own DNS step, which calls that lookup the one thing that still leaves, was
right. One word: nothing ELSE reaches the internet.
The DNS step was also behind reality. It named only the lookups devices send to
the ROUTER, but the shipped dns_intercept='1' pulls a query aimed at a resolver
the device picked for itself into the engine too, answers it there, and it
leaves in the same clear UDP/53 — measured both ways, each producing its own
plaintext packet on the WAN. Encrypted DNS is not the way out either: :853 out
of the LAN measured connects=0, because the plan rejects it. The generator's own
critical warning (generate/dns.go) has said all of this for as long as it has
existed; only the panel had fallen behind it.
"takes ... and drops it" is untouched, and measured: the engine accepts on the
local tproxy socket in ~100 us even for an unreachable address and then closes,
so the client gets an immediate ECONNRESET rather than a hang. "Blocks" and
"ignores" would both be less accurate. Nothing is added about ping: the stand's
ICMP probe was 100% loss in BOTH states, so it proved nothing either way.
The test is the point. The two halves live fifteen lines apart and each reads
fine alone, so a wording fix does not survive the next editor. The new test
checks the INVARIANT instead: the band is flattened to clauses and no clause may
claim that nothing leaves while another names something that does. Its detector
is proved on a fabricated band first (a prior that cannot fire measures
nothing), and it asserts the no-resolver band really does contain a clause
admitting the leak, so the check cannot pass by finding neither half.
Mutations, all caught: detail back to "nothing reaches" -> the invariant fails
and prints both clauses verbatim; DNS step back to the router-only wording ->
the resolver test fails; DNS step stops admitting the leak -> two tests fail.
Control: with one resolver configured the DNS step is absent and no clause
claims anything leaves; emitting the step unconditionally fails that control.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
fbcf211d19 |
test(stats): pin that the log filter does NOT search status
matchLog's field list is documented as positive and closed, and the new `status` is deliberately outside it for the same reason `outbound_kind` is: it is a fixed vocabulary word, so q=failed would silently match every failed row while the operator was looking for text. The test row now carries a Status, so the assertion is not vacuous — a filter block IS an answer, which is also the Status/Error invariant this row models. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
419eaf7bbe |
docs(porting): the schema number on line 146 was v0.1's, read as v0.2's
PART A is the frozen v0.1 survey, so `CurrentSchemaVersion=1` was archaeology
that happened to be correct about the branch it describes — and directly
contradicted the live schema subsection thirty lines below, which says
`shaterd migrate` writes 2. Anyone skimming the file map for "what is the schema
version" got 1. Say whose number it is, name v0.2's (2, steps {0->1, 1->2}), and
name what migrate1to2 did, since that is what the reader is usually after.
Also documents the `shaterd migrate` reporting contract in PART B: the closed
classification, the two non-syslog channels a failure reaches the operator on
with globals.log_syslog=0, and why 30_shater-core still exits 0 after one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
a794fbe374 |
fix(stats): a failed DNS lookup no longer reaches the log as a healthy row
dnstrack.QueryEvent carries Failed and Error; stats.LogEntry carried neither.
A SERVFAIL, a timeout, a loopback or a rejected-cached lookup was therefore
written into the query log with action "pass" — or, when the resolver that
timed out had a detour, with the flow-coloured "proxy" — blocked=false, and
nothing anywhere saying no answer was produced. The daemon already knew, one
event at a time: TotalStats.Failed is counted from that very fact in the same
function. The row threw it away, so the aggregate said "N failed" while every
row said everything was fine.
LogEntry gains two fields:
Status — closed vocabulary, "answered" | "failed" | "" (NOT RECORDED), same
discipline as OutboundKind/RuleKind. It is a separate axis rather than a
fourth Action value because Action says WHICH PATH the lookup took: a query
that went out through a detour and then timed out is action=proxy AND
status=failed, and folding the two would erase the one fact that says
whether the tunnel is what broke. It is also what an old panel would have
silently mapped back onto "pass" through its own open fallback.
Error — the producer's own cause text, verbatim, meaningful only when
Status=="failed". No grading is invented on top: three of the four causes
are fixed literals ("loopback", "rejected (cached)", "rejected") and the
fourth is the transport's err.Error(), which cannot be classified without
guessing. "failed" with an empty Error is honest and reachable — the lookup
failed and the cause was not recorded. What IS derivable stays derivable:
Rcode separates "no response at all" (-1) from "the server refused".
queryStatus is a closed POSITIVE list over the sources a producer emits; an
unlisted or zero Source falls to "" (not recorded), never to "answered". The
aggregate is untouched: blocked/failed are computed once in handleEvent and the
row is labelled from those same two values, so the counter and the row can
never disagree and nothing is counted twice.
Cost: LogEntry 152 -> 184 B on 64-bit (+6.4 KB at the default 200-row ring).
Status is a package constant, so its body costs nothing; Error is interned in
its OWN table (maxErrKeys=128, clamped to 160 B) rather than the rule table,
because the transport's error text embeds the queried name and a flood of
distinct causes would otherwise keep clearing the routing-text table.
Tests: every assertion mutation-checked, and the control is three-state — the
same instrument separates answered from blocked from failed, with the
aggregate pinned to identical totals across the change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
735aa5428f |
fix(shater-core): name which of the four ways shaterd migrate ended
Both call sites swallowed the result. /etc/uci-defaults/30_shater-core ran `shaterd migrate >/dev/null 2>&1` — stdout, stderr AND the exit status gone, so a refusal was indistinguishable from a success on the one screen the operator who caused it was reading. /etc/init.d/shater logged, but with a single sentence that described only one of the outcomes: "routing rules that still carry the removed dst_domain/dst_ip options stay DISABLED until this succeeds. Free space on /overlay and re-run". On a DOWNGRADE every clause of that is false — nothing is disabled, /overlay is not the problem, and re-running never helps, because the fix is to put the newer package back. A confident wrong diagnosis costs more than no diagnosis. The outcome is now classified with a CLOSED positive list — ok / downgrade / unreadable / failed — and the last rung is the point of it: an unrecognised failure says it is unrecognised and quotes the binary verbatim instead of being reported as one of the causes we can name. `downgrade` is recognised by the substring "newer than this build", which both model.migrateWith's refusal and model.ErrSchemaTooNew contain; that seam is a contract and is now pinned. log_syslog=0 is honoured, not worked around. It is a statement about the syslog stream, not a request to be left uninformed, so failures go to two channels that are not syslog: the script's own stderr (the operator's terminal on a hand-typed restart; the package manager's output inside `apk add`), and /etc/shater/migrate-failed on flash — written on failure, REMOVED on the first success, so its absence is the honest all-clear. syslog gets the same line when log_syslog allows it. A migration that SUCCEEDED stays routine. uci-defaults still exits 0, deliberately: a uci-defaults script that does not is kept and re-run at every boot, and this one re-runs a detached enable+restart of shater/shater-cron plus a firewall reload — one recoverable failure would become permanent boot-time churn, to carry a status nothing reads. The retry that matters already exists in start_service, which runs the migration every start. Found by mutation while writing the tests: reverting start_service's call site left every other test green, because they all call shater_migrate directly. The reporter would have been perfect and unreachable. TestInitScriptStartServiceUses- TheReporter closes that. Verified: sh -n and busybox ash -n on the target (ImmortalWrt 25.12.1 r37978), the classifier exercised there under busybox ash against the real uci; six mutations rolled back one at a time, each caught by name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
d1f43dbbe6 |
fix(shaterd): diag printed constant.Version instead of the version it was handed
renderDiag took a version through its seam and then ignored it, reading constant.Version directly — so the one field the dead-daemon test could have pinned was the one field it could not see change. The bundle now prints what it was given, and the test asserts the value and not just the heading. Also names the cost the schema gate adds: model.readDiskState's own comment says "this runs once per write", and it now also runs once per apply, i.e. once a minute from shater-cron — one `uci export shater` fork and two parses of a few kilobytes. It reads the DISK rather than m.Globals.SchemaVersion deliberately: m need not have come from disk (rollbackTo hands in an in-memory snapshot), and the question is about the file this build would have to live with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
67c4829f03 |
fix(apply): name the fetch_via=proxy that is not going through anything
`fetch_via=proxy` with an empty `fetch_detour` is not "proxy, details to
follow". Applier.HTTPClient hands "" to resolveVia, which passes it through
(it is not a `chain:` selector), engine.ViaToTag maps "" to the tag `direct`,
and the feed is dialled through the box's direct outbound — over the ordinary
WAN, with the router's real address, merely from inside the daemon process
rather than from the CLI. Nothing fails. The subscription provider, the party
`fetch_via=proxy` is chosen to hide from, sees that address on every
scheduled refresh.
The picker exists and defaults to Direct, so the state is not "unconfigured";
it is "configured, and silently equal to direct". The message opens on that.
Critical, by this file's own rule at the top — protection the operator
CONFIGURED is not in effect — and by consistency: criticalMarkers already
grades the identical disclosure critical when generate says it about DNS
("in the clear", "your provider sees", "leaves over the plain WAN with your
real IP address").
A detour that names nothing is a SEPARATE finding at `warning`, because it
has the opposite consequence: resolveVia or the engine refuse by name and
UpdateSubscription returns the error rather than falling back, so nothing is
disclosed — what breaks is the refresh, loudly. One sentence for both would
send the operator to fix the wrong thing. A bare name that is really an
egress or a chain gets its own text giving the spelling that resolves, rather
than a false "nothing answers to that name".
Filed under section `subscription` + the sub's own name, which the panel
already routes to that row (Nodes.tsx entityFindings/findingsByName) and to
Overview. The severity is part of that binding, not just the volume: `info`
is filtered out of entity routing on purpose, so it would never reach the
row — recorded at the constant.
Also completes the FetchDetour contract in model.go, which listed neither
`chain:X` — the form apply.resolveVia has a dedicated branch for — nor what
"" actually does.
Verified: 9 mutations, each reverting one part, each caught by a named test;
controls show the same instrument silent for a resolved detour, for
fetch_via=direct, and for a subscription that is disabled or has no URL.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
b43f673ad2 |
fix(apply,shaterd): a downgraded build may not RUN a config it can only half-read
model.WriteUCI already refuses to write a config whose schema is newer than the
build, so a downgrade can no longer eat the file. What was still open was
RUNNING one. ParseUCIExport ignores options it does not recognise — silently —
so a v3 config read by a v2 build yields a Model with the v3 settings simply
absent. The engine starts perfectly happily and routes traffic by a policy
nobody wrote. Nothing said so: cmdRun never called Migrate(), /etc/init.d/shater
calls it, logs one daemon.err line on failure and starts us anyway, and that log
defaults to a tmpfs file globals.log_syslog='0' can switch off entirely.
REFUSE OR START — and why refuse. Both sides, weighed by "the default falls to
the recoverable side":
* REFUSE. With kill_switch=closed the fail-closed plane goes up and LAN->WAN
forwarding stops. Loud, immediate, impossible to miss. SSH, LuCI and the
panel stay reachable, nothing on disk changes, and reinstalling the build
the router ran ten minutes ago puts everything back exactly as it was. The
damage is an outage the operator caused themselves and can undo.
* START ANYWAY. Traffic the missing rules were meant to tunnel leaves through
the plain WAN with the router's real address on it, and nothing announces
it. That is not recoverable in the sense that matters — the disclosure has
already happened. It is the same choice `sub update` made when it was given
FAIL over a silent direct fetch.
So: refuse. But the daemon does NOT exit and does not crash-loop — a refusal
nobody can see would be the third bad option. It stays up, keeps serving the
panel and the control socket, and says why in three places:
1. apply.schemaDowngradeGate refuses every apply (step 0 of applyLocked), with
the engine-swap failure policy of step 2: a previous engine that IS running
a config this build understood is left alone; with no engine, holdLocked
installs the fail-closed plane — and honours kill_switch=open, which is the
operator's documented fail-open choice and may not be quietly overridden.
This is in applyLocked and not only in cmdRun on purpose: cron reconciles
once a minute, so a gate that only ran at startup would be bypassed sixty
seconds later.
2. Status carries the PAIR: schema_version (disk) and schema_supported
(model.CurrentSchemaVersion). Either alone is unreadable — the panel
already showed the disk version, and "v3" next to a build that understands
v2 looks entirely normal. The difference IS the fault. schema_supported is
a compile-time constant and is therefore set even on the offline stub, i.e.
on the daemon most likely not to be answering. A critical warning naming
the downgrade is computed at READ time, because in this state no apply can
succeed and "the warnings of the last successful apply" would be empty.
3. cmdRun consults model.Migrate() before reading the config (so a bare
`shaterd run` gets the gate too) and classifies the outcome with
CheckConfigWritable: ErrSchemaTooNew is the downgrade, anything else is an
ordinary migration failure and is NOT reported as one.
Only ErrSchemaTooNew blocks. ErrUnmigratedConfig — schema-v1 dst_domain/dst_ip
leftovers — must not: the init script documents starting anyway with those rules
disabled, and turning that into a blackout would be a regression.
Also in this pass, reported by the coordinator: standing_state_test.go's
noGatewayFinding claimed to be quoted from netplane "so the test breaks if that
warning is ever reworded". It cannot — the string never leaves this package and
netplane.noGatewayWarning is never called — and the claim was already false when
it was read: netplane's text has since gained "over IPv4" and an IPv6 clause
while every test here stayed green. A fixture that advertises a guarantee it
does not provide is worse than one that advertises nothing. The comment now says
what it is, and netplanechannel_test.go pins the two couplings that are real:
the severity comes from the CHANNEL (warningFromText(t, "interface",
SeverityCritical), no classify pass), so no rewording can demote it — with the
control that the same texts on the generate channel are NOT critical — while
Section/Name DO come from the `kind "name": ` prefix, asserted in both
directions. A genuine text link is one exported helper in netplane away and is
left to whoever owns that file.
Verified: 13 seeded mutations. Twelve killed by named assertions; the
thirteenth SURVIVED — the guard in schemaWriteVerdict could not be seen, because
on a build host model.CheckConfigWritable answers nil for everything, so the
test reported success whether the guard was there or not. The checker is now
injected and both directions of that guard are killed. Every schema assertion is
walked over all three relations (disk newer / equal / older), so nothing here
passes by always answering the same way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
da7411e29a |
feat(panel): attach named block/allow lists to a device, and show whether they loaded
A device could only carry hand-typed domains. It can now also reference the
`config blocklist` / `config allowlist` sections by name, which brings geosite
categories and url lists to parental control for free (Device.Blocklists /
Device.Allowlists — the Go half is landing separately).
The composition problem was the order. The engine decides a name in five steps —
allow typed, block typed, allow attached, block attached, network filter — so the
typed lane and the attached lane of ONE control are two steps apart, with the
other control's lane in between. Two controls therefore cannot show the order by
position. The card draws it instead: a five-stop rail, lit per step where this
device actually has something, and the same step number stamped on each lane
inside the two pickers.
ListPicker is a new component rather than a generalised SrcPicker: that one is
welded to useSrcOptions(), to CIDR validation, and to an empty state reading
"everyone · all LAN clients", which on a block list means the opposite of the
truth. It reuses SrcPicker.css and its whole interaction language.
Honesty, in three places it would otherwise have lied:
- a list chip reports what /api/ruleset/status says, not that someone attached
it. Never fetched reads "not loaded" in crit, an empty one "empty", one the
engine has not mentioned "load unknown" — dim, never green. A name the config
no longer has reads "no such list".
- attaching a list is itself the switch for that device, so a list with
Enabled=0 is NOT drawn as dead, and the DNS page's "configured but inactive"
is replaced by a sentence naming the devices still running it. A row for such
a list now reads "N devices only" instead of "off"/"inactive".
- an attached allow list is terminal, so it lifts the network blocklists off
everything it covers. Said in the picker at the moment of choosing and again
on the card.
cleanDomain demanded /^[a-z0-9.-]+$/, so a colon could not be typed and the
engine's own full: / suffix: / keyword: vocabulary was unreachable from the
panel. parseDomainEntry accepts them from a closed positive list and refuses, by
name, the three shapes the engine silently discards: an unknown `word:` prefix, a
marker with no value, and an IP. The keyword case gets its own message — an empty
keyword is strings.Contains(host, "") and would take the device off the internet.
The logic lives in src/deviceLists.ts with tests, since `node --test` cannot load
a .tsx. Each test was mutation-checked, and the load reading is shown giving both
a positive and a negative result.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
73bd02dd71 |
feat(byedpi,nodetest): check the listener, not the file; and let one node be tested
A2 — the panel unlocked the byedpi egress type on LookPath("ciadpi"), which
answers "is the package installed" while the operator is asking "will traffic
sent here go anywhere". Those come apart on the SHIPPED configuration: the
packaged /etc/config/byedpi is inert (enabled='0'), and the port is coordinated
between the two packages by comment only — nothing in the daemon had ever read
that file. Result: type unlocked, egress on 127.0.0.1:1080, apply green, nobody
listening.
shater/panel/byedpi.go now decides on three separate facts (binary, enabled
instances + their ports read from the conffile, a TCP connect to each) and
reports a CLOSED state: unknown | not_installed | disabled | not_listening |
listening. Only "listening" may gate the egress type. GET /api/byedpi adds the
per-egress port reconciliation, so a mismatch is NAMED with both numbers instead
of going quiet. Nothing overclaims: the check is a connect, not a SOCKS5
handshake, and every sentence says so. A connect that is neither accepted nor
refused is "unknown", never "no".
C4 — a just-added node had no instrument: the group test reads the observatory's
board, and the observatory only probes what the rules route through, so the one
question a fresh node exists to ask ("is it alive?") answered "no rule routes
through it". POST /api/groups/test now takes {"kind":"node"} and runs the SAME
instrument — same singleton, same runner, same result type, same status poll,
same exit_ip through the target's own outbound with the same refusal to answer
from `direct`. The only addition is one fallback: a node the observatory does not
cover is measured once, here, through probeOneInto (the observatory's own
dialler), recorded under its own tag alone. A node whose base tag is a plan STORE
ALIAS — the egress-bound-group case — is NOT dialled: the board already holds its
egress-path number, and a bare-WAN measurement filed there would be the same
poisoning one layer down.
Results gained kind (group|chain|node|"") and source (observatory|on-demand|""),
so "nobody measured this" is distinguishable from "measured and dead".
Also: PUT /api/config maps model.ErrSchemaTooNew to 409 beside ErrUnmigratedConfig.
A downgrade refusal is the guard working, fixed by the operator, not by us; 500
sent the reader to the daemon log.
Every new test was verified by mutation (14 mutations, each killed by name), and
each detector has a control: the byedpi probe is shown seeing a real loopback
listener AND its absence with nothing else changed, and the node test is shown
telling a live node from a dead one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
ffe4d8726f |
feat(apply,shaterd): a configuration history on disk, and a support bundle that outlives the daemon
Two holes from the operations audit, and I can confirm both of its readings.
CONFIGURATION HISTORY. Applier.lastGood and Applier.snapshot are fields in
this process. A daemon restart or a reboot loses both, and Rollback with no
snapshot goes to rollbackEngineAndPlane, which re-reads the CURRENT
/etc/config/shater — that is, it re-asserts the config that broke. With
confirm_timeout at 0 by the owner's choice there is no auto-rollback either,
so "what did the working config look like?" had no answer at all once the
daemon had restarted. Nothing on this router kept one.
Every successful apply now files RenderUCIExport(m) — the existing pure
function, not a second serializer — into /etc/shater/history/<unix>-<version>.uci.
* DEDUPLICATED against the newest entry. shater-cron reconciles once a
minute and every reconcile runs applyLocked to completion, change or no
change, so a file per apply would be ~1440 identical writes a day onto
overlay flash and would fill the ring with twenty copies of one config
twenty minutes after the last real edit. One file is now one change.
* 20 files / 512 KiB total / 128 KiB per entry, hard ceilings, not defaults.
The shipped /etc/config/shater is 12.6 KB of which 459 bytes are actual
configuration; a loaded one renders to a few KiB up to low tens of KiB, so
twenty entries normally cost 50-200 KiB and the byte cap binds only for
inline entry lists. Against what this product already grants itself on the
same overlay — 4 MiB of compiled lists, an 8 MiB rule-set cache, a stats.db
defaulting to 64 MB — 512 KiB is a rounding error. An entry over the
per-entry ceiling is REFUSED rather than allowed to evict the whole ring,
and the refusal is reported.
* 0700 dir / 0600 files. Checked, not assumed: the Makefile installs
/etc/config/shater with INSTALL_CONF, i.e. 0600 root:root, and these files
carry the same node credentials and subscription URLs.
* A failure NEVER fails the apply, and is never swallowed: it becomes a
Warning folded into the set Status publishes (gather + append + finalize,
the seam abortAfterSwap already uses), so the panel says the history has
stopped instead of the directory quietly going stale.
* NOT kept across sysupgrade. The audit's premise that /etc/shater is in
keep.d is wrong — keep.d/shater-core lists four specific paths, not the
directory. Excluding it follows model.backupBeforeChange's existing
precedent for config.pre-v*.bak: the archive is held in RAM across the
flash and routinely ends up in cloud storage, and this is a local undo for
changes made on THIS box.
`shaterd diag`. The only thing this product could hand over was
GET /api/log?range=, served by the daemon — so in a crash loop the one channel
that does not need ssh dies with the process. `shaterd diag` prints version,
our packages from `apk list -I`, status, `nft list table inet shater`,
`ip rule`, the log tail and the configuration, as one block, collected entirely
by the short-lived process.
* It works with a DEAD daemon, which is the case it exists for. The status
section falls back to the same offline stub `shaterd status` prints and
LEADS with the fact that no daemon answered, so an empty-looking section
can never read as a healthy one. No section is ever silently absent: a
missing nft/ip/apk produces "NOT COLLECTED: <reason>", and `uci export`
failing falls back to the raw file and says so.
* Masking is a POSITIVE, CLOSED list of the fields that may be PRINTED
(diagSafeUCI), keyed by section type. Everything it does not name is
masked — an unknown option, an unknown section, and every field added to
model.Model after this build. That is the direction the open `default:`
lesson demands: the recoverable side is "hidden", not "shown".
TestDiagMaskingIsClosedOverTheWholeModel proves it by reflection over every
string the model can render, with the control that the same instrument sees
those values in the unmasked text.
* A second layer scrubs the refused literals from the WHOLE document, because
masking the config alone would only move the leak: the daemon prints a
subscription URL into its own log on a fetch failure.
* node.uri keeps its scheme and nothing else — "is this node vless or
wireguard" is most of the diagnosis and a protocol name is not a secret.
Verified: 14 seeded mutations, every one killed by a named assertion (dedupe
removed, prune removed, ceiling removed, 0600->0644, 0700->0755, failure
swallowed, warning not folded, version not sanitized; allow-list defaulting to
ALLOWED, uri scheme dropped, scrub removed, failed sections made absent, stub
banner removed, masking removed). Every check is paired with its control — the
ring tests assert the newest entry is present and correct, so "the old one is
gone" cannot be satisfied by a ring that silently stopped writing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
7f84ea9496 |
feat(panel): the logs answer why it went there, where it went out, and where the search stopped
Four fixes on one path — the one a person actually walks when a site does not open: Insights -> DNS log -> Connections. Three of them were fields the daemon already put on the wire and the panel dropped on the floor. 1. Connections shows the routing record. ConnLogEntry gains rule_kind/rule/chain, with the discipline stats.go wrote them under: "" is NOT RECORDED and can never be drawn as "no rule matched". Three states, three readings, and a CONTROL test that fails if any two of them render alike. The outbound path is printed rule-named-tag first, dialling-outbound last (the wire order is the reverse). 2. The DNS log says where the lookup left. outbound_kind is a closed four: detour (tag named) / default (the resolver names no detour -> the query went out the plain WAN, past the tunnel; marked amber) / local (cache, optimistic answer, filter block: nothing egressed) / "" (not recorded). An unrecognised value falls to "not recorded", the recoverable side, not to one of the answers. 3. Both logs take q=. The daemon filters inside the store on the same walk as the cursor, so limit counts MATCHING rows. The searched fields are named under the box, because a POSITIVE CLOSED list is also a statement about what is NOT searched: no ports, no rule_kind, no outbound_kind — q=default matching every default-egress row would be a trap wearing a filter costume. logRoute mirrors filter.go exactly so the ?mock backend finds and misses what hardware does. 4. A TRUNCATED page is not the end of the log. A filtered walk is budgeted (MaxFilterScan); a page that ended on that budget is short for a reason that has nothing to do with how much data exists. X-Stats-Log-Truncated is now read and the state is NAMED — an amber "Scan stopped" plate, the empty text saying "not the end of the log" instead of "nothing found", the count line refusing to say "all loaded", and the daemon resume cursor behind a button. The cursor matters twice: a truncated page can have ZERO rows, so there is no row seq to page from, and the live tail now advances on rows EXAMINED rather than rows matched — a filtered after= poll that matched nothing used to rescan the same window every tick forever. Also: .fp-select gets max-width:100% + min-width:0. A <select> shrink-wraps to its widest option and, as a flex item, refuses to shrink below it: the geo provider label measured 501px in a 375px viewport and gave the PAGE a horizontal scrollbar (scrollWidth 559 vs clientWidth 375, measured). Settings.css and Networks.css each carried a narrow copy of this fix; the component is the right place. Verified on an isolated harness with no page-local CSS: bare select overflows a 320px row at 438px, adding the class alone brings it to 320/320. Tests: 34 new, every one mutation-verified — unrecorded folded into default / into local, rowMatches returning true unconditionally, outbound_kind added to the searched fields, historyExhausted ignoring truncated, logEndNote drawing both situations with one sentence, logCountLabel saying "all loaded" on an incomplete scan. Each revert reproduced its own failure text. Both search directions are covered (finds / does not find), which is what catches a filter that matches everything. Browser-checked at 390 and 1280 in both themes, no horizontal scroll; the six-click resume walk from "scan stopped" to "Nothing in the log matches" was exercised live in ?mock. NOT verified: no hardware or VM run — the truncated state was exercised against the mock backend, whose scan budget is 60 rows where the daemon uses 20000. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
fd8b424d5a |
fix(netplane): a missing IPv6 gateway is not an outage, and it never was one for IPv4
Measured on the production BPI-R3: egress `ewan` was carrying the entire household's traffic (chain default, plane full, verdict tunnel, 15 hours up) while the panel showed, at CRITICAL, "this egress CANNOT REACH ANYTHING outside its own subnet — every node, group and rule bound to it will fail to connect". table 8208 held `default via 10.0.0.1 dev eth1`; the IPv4 half was perfect. eth1 holds one address, fe80::.../64, and the ISP publishes no IPv6, so `ip -6 route show default` is empty router-wide. The -6 pass found no nexthop and one family-agnostic text declared the whole egress dead. Two defects in one line. A per-family fact was stated as an absolute, and the absence of an optional ISP feature was graded as an outage — in the loudest register this codebase has, on a channel apply grades critical wholesale. Red that stands for fifteen hours over a healthy router is not a warning. IPv4 stays loud and unchanged in substance: an uplink with no IPv4 nexthop carries nothing. It now scopes its consequence to IPv4 and says outright that it is not describing IPv6. IPv6 splits on one piece of evidence — does the device hold a global IPv6 address? If it does not, IPv6 is simply not provisioned on this link: nothing is broken, nothing leaks (the v6 mark keeps its own table and its unreachable floor, so it cannot fall through to main), and there is nothing the operator can do because the missing thing is upstream. Silent. If it does, IPv6 is configured and the nexthop is missing anyway — a real fault, still critical, now scoped to IPv6. Silence requires positive evidence: a failed address read makes us louder, never quieter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
a9ec36e053 |
fix(netplane): a second LAN's inbound options meant nothing, and rule counters were structurally blind
Three things the divert plane got wrong once more than one tproxy inbound exists, and one it got wrong all along. Per-rule diverts read `tcp`/`udp`/`tproxy_port` off the FIRST enabled tproxy inbound and applied them to every device the plan touches. With one LAN — the shipped shape — first and owner are the same section and nothing showed. With two, `option udp '0'` on the second inbound was ignored (UDP diverted anyway, into another section's listener), `option udp '1'` was ignored the other way (no per-rule UDP line at all, so the rule's counter never ticked for UDP and Insights showed a rule that appeared never to match), and `option tproxy_port` pointed at the wrong listener. Same for the dns_intercept :53 lines, which sit above the fib-local bypass. Each ingress device now resolves to the inbound that OWNS it; a device no inbound claims still falls back to the primary, because that is the only listener its traffic can reach. Verified byte-identical output for every single-inbound shape against the pre-change renderer. Rule counters: a counter exists only for a rule the plane emitted a divert line for, and it only emits them from SOURCE selectors — so a rule written by domain or ruleset never appears in RuleTraffic at all, and absence there could not be told apart from "carried nothing". It cannot be measured: which rule a packet matches is decided inside the engine after the divert, where nftables cannot see it. So no counter is invented. Instead the plane says which rules it can measure (RuleMeasures) and what the numbers it does have actually mean — an upper bound, not the rule's traffic — and the two are pinned to the rendered ruleset in both directions. Counters are now declared BY the emitting line, so a rule whose fragments were all dropped no longer leaves a counter attached to nothing, reading a confident, permanent, false 0 B. untunnelable_egress could resolve, pass validation and still mark nothing when the plan has no LAN ingress device — while apply's note, gated on the same binding succeeding, told the operator that IPsec/GRE/SCTP now leave through it. The gate is right (the marking rule has no safe unscoped form), the silence was not; bound-but-inert is now named. stats/panel do NOT consult RuleMeasures yet — wiring it is a change outside this package, and the gap is still visible to an operator today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
d40eeede0c |
fix(model): refuse a config newer than the build, and never write the version back down
A downgrade ate the configuration silently, and permanently. Migrate() refuses a newer schema, but nothing on the write paths calls it: the daemon starts regardless, ParseUCIExport reads the options it knows and drops the rest, and WriteUCI replaces the WHOLE package. So an older build rewrote /etc/config/shater with only what it understood. The second half is what made it unrecoverable — schema_version round-tripped through the Model, so the rewritten file claimed the OLDER version, and a newer build put back afterwards saw cur == CurrentSchemaVersion and migrated nothing. Nobody had to be at the keyboard for any of it: the profile watcher looks every 25 s and `sub update` runs from cron every 6 h, and both persist through WriteUCI. The mechanism for refusing already existed and already worked in the other direction (ErrUnmigratedConfig + CheckConfigWritable); this is its second caller, not new machinery. - guardSchemaDowngrade refuses the write and the panel's pre-flight when the config on disk is newer than this build, naming both versions and the way back (put the newer package on again — the config is untouched). ErrSchemaTooNew so a caller can answer 409 instead of 500. - withDiskSchema takes schema_version from the DISK, never from the caller. A PUT body that omits it sends 0, and a rendered 0 is an OMITTED option: the version would have vanished and the next `shaterd migrate` would replay every step. A body claiming 99 would have locked the box out of its own panel. - backupBeforeChange copies the live config to /etc/shater/config.pre-v<schema>.bak before the first migration and before the first write — once per schema version, write-then-rename. A failed backup aborts: the `uci commit` that follows writes to the same filesystem, so refusing costs nothing that was not already lost, and best-effort-and-carry-on is the silent skip we keep paying for. - The reverse direction is fenced by a test: an unmigrated v1 config still refuses a rule-changing write as ErrUnmigratedConfig, still allows one that leaves the rules alone, and still keeps its `list dst_domain` and its v1 stamp. The shipped /etc/config/shater now says what "conffile" actually buys — values across a package upgrade, not comments across the first write, which happens without an operator — and the annotated file is installed a second time as /usr/share/shater/config.sample, where nothing rewrites it. INSTALL.md gains the downgrade procedure. Measured on the testbed VM (ImmortalWrt 25.12.1 r37978, apk-tools 3.0.5) against the real apk-v0.2.9/v0.2.10 feeds in an isolated --root sandbox: `apk upgrade <named>` does not downgrade at all; `apk add <pkg>=<ver>` does, and leaves a pin in world that a later upgrade obeys; `apk upgrade -a` downgrades too but took four unrelated packages with it. Tests in shater/model/schemadowngrade_test.go; every assertion checked by mutation (8 mutations, each killed a named test) and every refusal paired with a control that accepts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
7c93019e81 |
fix(panel): draw the settings that decide whether traffic leaks
Five things the panel knew and did not say, each one a state where the screen read healthier than the router was. Rule.Kill was typed, round-tripped and drawn nowhere. `open` sends a rule's traffic out direct — around the kill-switch, with the real address — when its target cannot be built, and such a rule looked exactly like one that fails closed. It now has an editor beside Target and an amber mark on the row; the fail-closed default draws nothing, so the two states are not priced alike. An unreadable value is its own state: it blocks, like the daemon, and the picker re-surfaces it verbatim rather than rewriting a value it never showed. Alert channels were write-once for Type/Token/ChatID/URL/Events, so fixing a typo meant deleting the channel and going back to BotFather for a token you already owned. Add and edit are now one form. The token box starts empty and the caption says what empty means — keep, never clear — because the panel refuses to show the secret and a save may only clear a field the editor could show. Same rule covers a type switch: the other kind's settings stay stored and unused. The add-rule form pre-filled Target=direct. An untouched form is a rule with no matchers, i.e. the default route, so one press put the whole LAN on the plain WAN. `block` would only have swapped the leak for an outage; the recoverable default here is no default, so the form refuses and asks. The empty state said "all traffic follows the default route" without naming it. On a fresh install that route is `block` — the LAN has no internet — and this is the page the kill-switch alarm sends people to. Both it and the lead now name the route in force. The interception board was computed from the config alone and lit `lan` green over a stopped engine. Green now needs the engine up AND the full plane; a hold plane blocks rather than carries, and unknown is an unlit socket. Also: four rungs of the untunnelable copy claimed traceroute works. It prints `* * *` and no hops on every setting — the wording is now apply/warnings.go's own udpTracerouteFacts, said once. Tests: killPolicy / alertEdit / defaultRoute / intercept, 38 cases, each mutation-checked (16 mutants, all caught). Browser-verified at 390 and 1280, no horizontal overflow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
bc7ea359c8 |
feat(panel): the settings that only /etc/config/shater could reach
Five groups of real daemon settings had no control in the panel, so the
only way to change them was to edit the config over SSH. Each one is now
editable where it belongs, and each editor is built so it cannot lose a
field it declines to display.
DNS list refresh intervals. Blocklist.UpdateInterval was hard-coded to
"24h" in two places and shown nowhere, while the row displayed the
interval the ENGINE reported — a readout dressed as a knob.
Allowlist.UpdateInterval did not exist in the panel at all. It matters
because an allowlist is how a blocklist false positive gets corrected: one
pinned to a day delivers the fix up to a day after the site broke.
Geo data. GeoProvider and the four URL fields are consumed for real
(generate.SetGeoProvider, /api/ruleset/categories) and the panel USES the
data they pick, while offering no way to choose it. New Settings group
with the closed five-provider list, the custom {category} templates and
the two category indexes. Only `custom` reads the templates, so only
`custom` renders them; an unknown provider is preserved and marked rather
than silently rewritten to auto on page load.
Subscription filters. Include/Exclude/FilterProto/FilterCountry/Dedup,
Format, ExpireAlertDays and the three device-identity headers are now
editable — the same five filters a group already offered over its members,
applied one step earlier. 376 nodes can become the four Dutch ones without
SSH. ExpireAlertDays keeps its three states (blank = the 3-day default,
"off" = -1) instead of being flattened.
Edit-after-create. Blocklists, allowlists and resolvers could be
configured only at creation; a typo in a URL meant delete and rebuild, and
deleting a resolver clears whichever global slot it filled. Every one now
has a row editor. `file` and `geosite` sources are offered when a list
already IS one, so opening a list the panel cannot create never becomes a
way to destroy it.
Stale local type copies. DNS.tsx and Settings.tsx carried local
Blocklist/Allowlist/Globals extensions whose comments claimed api.ts did
not type those fields; api.ts had typed them for a long time. Deleted —
the note was an invitation to declare the next field twice. (Egress.Target
was already gone.)
Along the way, three defects the work surfaced:
* parseDomains cut comments per TOKEN, so pasting "# ads and trackers"
contributed ads, and, trackers as three real blocked domains. Cut per
line now.
* FetchVia=proxy with no FetchDetour resolves to the tag `direct`
(engine.ViaToTag), so the feed is pulled over the plain WAN and the
provider logs the router real address — the one thing `proxy` is
chosen to hide. The row said "via proxy" for it. It now says
"proxy - no route" and the editor carries an amber explanation. The
picker also gained chains, which apply.resolveVia supports for real
and the picker excluded with a comment that misdescribed the contract.
* Adding a subscription only saved it. apply does not fetch, and
shater-cron is inert unless globals.enabled=1 AND the service is live,
so on a router not yet switched on nothing would ever fill it — and
the only Update button sat at the bottom of a collapsed panel. Adding
now fetches, reported separately from the save, and every row carries
Fetch now. A row with no nodes says what to press.
Nodes also gained the forward link nothing had: a node is not something a
routing rule can point at, and no page said so.
The merges live in subEdit.ts / dnsListEdit.ts / geoProvider.ts because
`node --test` cannot mount JSX. The rebuilt shapes return Complete<T>, so
a field added to api.ts fails the build in the function that has to decide
about it; the subscription merge extends instead, because five of its
fields are provider-reported state no control can show.
Verified: npm run build green; 173 tests pass; 18 mutations each killed a
named test and a probe field added to Allowlist broke the build inside
nextAllowlist; zero Cyrillic in panel/src; Chromium at 390 and 1280 with
no horizontal overflow (the detector caught a real 559px select spill at
390 before the fix).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
447cd8cf7d |
fix(panel): a switched-off service is not a fault, and say so before the apply that is
The shipped config is `enabled '0'` + `kill_switch 'closed'` with nothing
applied. Every lamp in the panel was derived from what is INSTALLED and none
from whether anything was MEANT to be, so a package that installed exactly as
designed showed a crit master lamp ("Engine down"), a crit kill-switch module
("NOT IN EFFECT") and three crit pips on Apply — at a person who had not done
anything yet. Red that fires on a correct installation is red nobody reads by
the time something is actually wrong.
serviceIntent() is the missing question, and every readout that used to answer
from the installed state now asks it first: off ⇒ unlit socket and a word that
says why; on ⇒ every alarm exactly as before. A positive `off` only — an
unreadable configuration stays `unknown` and keeps its crit, because that is
the state where the LAN really is cut off.
applyRisk() is the other half. Applying an empty config with the service on
and the kill-switch closed sets route.final = block, and the tproxy divert for
the shipped `lan` inbound is installed — so every TCP connection and UDP flow
from the LAN is handed to the engine and dropped. The panel read that state
perfectly once it existed and said nothing before, with confirm_timeout at 0,
so the most dangerous apply this router does ran with no auto-rollback. The
band names the outcome, the missing rollback and the fix, and does not block
the apply.
Insights had a short-circuit for this exact job that never fired: it was gated
on logging being off, and the shipped backend is memory. Ten sections drew ten
well-mannered "nothing yet" states and not one named the switch.
Every test is mutation-checked, and each one is paired with the control that
proves the instrument can still produce the alarm.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
|
||
|
omarandClaude Opus 5
|
65db309e3c |
fix(sub): fetch_via=proxy went out on the plain WAN from cron and at boot
`fetch_via=proxy` on a subscription means "pull this feed through the tunnel", and it is set for exactly one reason: the provider is blocked, or the owner does not want the provider (and every hop to it) learning the router's real address. The panel honoured it — Applier.UpdateSubscription resolves fetch_detour against the running engine — so testing it once from the browser showed it working. The CLI verb did not: it logged one daemon.warn line and fetched DIRECT. /etc/init.d/shater-cron calls exactly that verb, so every scheduled refresh and the fetch-at-boot went out unproxied, and the only trace was a syslog line in a log globals.log_syslog='0' switches off. The CLI cannot do this fetch itself — only one process may own the engine — so it now DELEGATES: a new control-socket verb `sub update <name>` runs the very same Applier.UpdateSubscription the panel's Refresh button calls. One implementation, so the two paths cannot drift again. With no daemon to ask, the subscription FAILS (exit 1) instead of falling back. The refusal is recoverable — shater-cron does not stamp the item, so it retries after RETRY_SECS and the already-cached nodes keep working — where a silent direct fetch is not: the disclosure has already happened. Direct subscriptions are untouched and still need no daemon at all. Order is load-bearing: the direct pass and its UCI write run first, then the delegated ones, because the daemon re-reads UCI and writes back userinfo counters a later write from this process would silently drop. Also in this file, reported by the LuCI agent: the offline stub of `shaterd status` published config_readable=false after a SUCCESSFUL read, telling every consumer to disbelieve three values it had just read correctly (LuCI worked around it by reading the field only when a daemon answered), and swallowed a FAILED read with no trace — the inverted lie apply.Status() was fixed for, in the one situation that matters most: a full /overlay where "not enabled" tells the owner they switched it off themselves while the fail-closed plane holds the LAN shut. Both halves now mirror the live path exactly. Tests are mutation-verified in both directions, with an instrument that gives a positive reading for BOTH "went through the tunnel" and "went direct" — a live origin server and a live control socket in every case, so neither zero is an artifact of the other endpoint being absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
d63f1d896d |
fix(bridge): reassemble return-path IP fragments — the bridge dropped them
sing-tun's classifyReturn answers `returnPass` for any IP fragment, so the l3 return path never judges one. On the WireGuard endpoint a passed packet still reaches the endpoint's own tun stack; the bridge has no second consumer — both deliverReturn and the batch read loops offer a packet to each attached return path and then drop whatever nobody claimed. A fragmented answer coming back through a bridge outbound was therefore lost outright, 100% of the time. Fragments do arrive: the return direction is fragmented by the LOCAL kernel (conntrack defragments at PREROUTING for the NAT lookup, the output path re-fragments to the bridge TUN's 1500-byte MTU honouring IPCB frag_max_size). packet.go's fixReturnChecksum already recognises a fragment and declines to touch it — the path was known to carry them. frag_reassembly.go is a deliberate sibling of transport/wireguard/ frag_reassembly.go: same algorithm, same ceilings (64 datagrams, 1 MiB, 5 s, non-refreshed deadline, partial overlap poisons the key), so collapsing the two into one shared package later is mechanical. They are not shared today only because the seam that would host the shared type — transport/wireguard/port.go and its test suite — is owned by other work in flight. Windows is deliberately untouched: there a fragment never reaches deliver() at all, because classifyInbound needs a transport header to decide ours/not-ours and WinDivert reinjects the rest into the host stack. Different function, different defect, platform we do not ship. protocol/tailscale gets a comment, not a fix: the one ReturnPackets call that package makes carries BuildUnreachable replies, which are synthesised whole and can never be fragments, and the real tunnel return path is upstream tstun.Wrapper.Write, ahead of every seam this tree owns. Verified: 23 tests, all 14 seeded mutations killed (including "seam removed" on both the portable and the Linux batch loop), -race clean on linux/amd64 in docker and on windows/amd64. The darwin seam is compile- and vet-checked only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
55dea4e729 |
feat(stats): the DNS log says where it went out, and both logs can be searched
C1. handleEvent had QueryEvent.Outbound in its hand, used it only to compute action(), and dropped it. The query log could name the resolver that answered and not the channel that resolver's own packets took — the one fact an anti-leak `detour` on a resolver exists to control. LogEntry now carries Outbound + OutboundKind, on the ConnLogEntry.RuleKind discipline: "" is reserved for NOT RECORDED, so the three states that all have an empty tag stay distinct — "detour" (tag recorded), "default" (the resolver names none, so its packets take the plain WAN), "local" (cache/optimistic/ filter block: nothing egressed at all). An unrecognised Source falls to unrecorded, the recoverable side. Rows from older builds decode to unrecorded and are therefore still distinguishable from a recorded no-detour row. No rule name is attached, and that is deliberate: the DNS path has strictly less to work with than the connection path did. A DNS *rule* picks a SERVER, not an outbound, and the event carries no rule identity at all — only the transport's tag. Inventing one would be a forgery. Cost, measured: LogEntry 120 -> 152 B (+32 B/row, two string headers on aarch64). +6.4 KB at the default ring of 200, +160 KB at 5000. Tag bodies go through the existing intern table (maxRuleKeys=512, shared with the rule text). C2. /api/stats/log and /api/stats/conns take q=<substring>, applied INSIDE the store on the same walk as the seq cursor. It has to be there: Limit is applied by the store, so post-filtering a returned page would hand back 3 rows of a 50-row page and call it a page. Substring, not regex — nothing a client can type costs more than a linear scan. Pagination stays honest. A filtered walk must examine rows it will not return, so it is bounded (MaxFilterScan=20000) — and a page that stopped on that bound is short for a reason that has nothing to do with how much data exists. That is reported: LogPage.Truncated + ScanCursor, surfaced as X-Stats-Log-Truncated and X-Stats-Log-Cursor. Unfiltered requests are untouched: no budget, never truncated, same walk as before. The logRing seam now returns LogPage/ConnPage instead of (rows, pending) so the truncation state cannot be dropped on the floor between the ring and the API. Tests: all mutation-verified (7 reverts, each reproduced with its message), including the copying-variant control for the intern table — strings.Clone passes an equality check and fails the identity check the test actually makes. Filter coverage is both-directions (finds / does not find) on both backends, with mem-vs-bolt parity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
2dfd7caf2b |
fix(apply): the untunnelable note may not describe an egress the plane never bound
untunnelablePolicyWarnings opened its egress branch on `Globals.UntunnelableEgress != ""` alone. netplane refuses far more than a typo: UntunnelableEgressBinding fails CLOSED for any name that does not resolve to an interface/tunnel egress WITH a device — nothing is marked in prerouting, no forward-chain accept is rendered, addEgressRouting installs no rule and no table, and the `untunnelable` policy decides everything by itself. The note nevertheless opened with "...now leave through egress "x": the kernel routes them out that interface", about a carrier that does not exist; it even printed `(device )` once the device was interpolated. The tail hedged the case thirty lines later, and the first sentence is what gets read. The branch is now gated on netplane's OWN verdict, called rather than re-derived (apply imports netplane, so unlike model.ValidateUntunnelableEgress there is no copy to keep in lockstep). That needs the whole model, so collectWarnings/gatherWarnings/untunnelablePolicyWarnings take *model.Model instead of model.Globals. When the option is set and unbound, the note now LEADS with that fact and then gives the ordinary policy text, because that is exactly what the router is doing. The bound branch drops "a name that matches no interface/tunnel egress" from its failure list — that case can no longer arrive there — and names the device it resolved to. Two further claims found while checking the rest of the file against the code: - the `icmp` rung promised "IPTV and VPN passthrough work only toward addresses your rules route directly". Multicast crosses this router under NO setting (the stream is WAN-side inbound; a client's outbound multicast is UDP, which untunnelableFilter structurally cannot match), and the other three rungs all say so. One true clause was carrying one false one — the same sentence the `block` note records having removed for being false. - "\"icmp\" drops it, excepting only ping/echo" understated a leak. `icmp` is the one rung that walks the destination plan and it ACCEPTS raw ESP/AH/GRE toward provably-direct destinations, so the operator was told it was contained while it left with the router's real address. Ratcheted by untunnelable_egress_honesty_test.go, each assertion with a control: the bound and unbound halves are walked in one pass, and the IPTV and `icmp` checks fail if the matrix ever stops producing the notes they read. The traceroute matrix grew a third egress value (set-and-bound, set-and-unbound) so the bound branch keeps being walked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
6f84d0ca7b |
fix(luci): read daemon_answered, and stop reading a config nobody could read
The dashboard told a live daemon from a dead one by `plane: ""` — a side effect of the offline stub being a zero value, not a promise anyone made. `shaterd status` now states it: daemon_answered, true on the live branch and false on the stub. The detector reads the field first and keeps the plane test only as the fallback for the non-atomic update window (new luci-app-shater, old shaterd). When the two disagree the field wins; a stub carrying a plane word must still read as "no daemon answered". Both lists are positive and closed. A daemon_answered that is not exactly true/false is not a verdict and falls through; a plane word this build does not know lands in unknown. Nothing lights green or amber on a guess, and the launcher button is still never disabled. config_readable was already on the wire and nothing here read it. With it false, enabled/kill_switch/panel_port are zero values: "inert (disabled)" and a green "closed (fail-closed)" were being rendered out of placeholders, in the one situation — a full /overlay, an interrupted commit — where the fail-closed plane has the LAN cut off and the owner is told they did it to themselves. Those rows now say "not known", a Configuration row carries the daemon's own reason and its don't-switch-anything-off warning, and an absent nft table is no longer softened to amber by an `enabled` nobody could read. The field is consulted ONLY when a daemon answered: the offline stub reads UCI directly and never sets ConfigReadable, so its false is a zero value while its enabled/panel_port ARE real reads. Taking it at face value would put "could not be read" on screen for a readable file. Same reasoning drops plane, traffic and hash on the stub branch — the contract calls them placeholders. panel_port is CONFIGURED, not bound: shaterd logs a panel bind failure and carries on, and SHATER_PANEL_ADDR can switch the server off while the port is still reported. Nothing measures a listener, so the hint, the tooltip and the new Panel port row say the port is configured rather than checked, and its lamp stays unlit even on a healthy router. tests/status-readout.test.js grows the new cases and now runs under gate step [7/7]. Mutation-checked four ways against copies: dropping the daemon_answered branches fails 7 assertions by name, dropping the plane fallback 5, reading config_readable without the daemon gate 5, and rendering the placeholders as readings 3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS |
||
|
omarandClaude Opus 5
|
e38108a7c4 |
test(gate): install iproute2 in the docker lane — without ip every slot is free
This change was already in the working tree when this session started; it is
committed here because it is load-bearing and an uncommitted load-bearing file
is a trap.
netplane.L3SlotFor asks the kernel through `ip link show` and reclaims through
`ip link del`. golang:1.26 ships no iproute2, so in the docker re-exec lane
every slot read as FREE, TestIntegrationL3StaleSlotIsReclaimed stood itself
down rather than pass while proving the opposite of what it claims, and [5/7]
then failed the gate — correctly, since this environment HAS root and
/dev/net/tun and the capability guard is therefore not what skipped it.
Installing it is also what made the concurrent-namespace defect visible at all
(see
|
||
|
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_01BHw89tdWddzhjUc4bAH4tS |
||
|
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. |