π΄ DO-NOT-MERGE-YET: airvpn killswitch β close the fail-open, stop root-sourcing the site config, fix the apply/doc order - #355
Open
ZacxDev wants to merge 4 commits into
Open
Conversation
β¦onfig, and fix the apply/doc order π΄ DO NOT MERGE until the `bar` skill's mandatory re-test protocol has been run on a physically reachable host. See the PR body. This is the killswitch half of the public-IP scrub, split out of #350 because `scripts/airvpn-updown` is a LIVE fail-closed killswitch on the workbench's uplink and a change to it is a runtime change, not a docs change. The lighthouse IP moves out of tracked source (this repo is PUBLIC) into a root-owned 0600 /etc/airvpn-updown.env, and the four audit findings against the first version of that move are fixed: A-2 FAIL-OPEN (π΄). A malformed value -- a partially-written env file is enough -- was interpolated into the nft ruleset, making it a SYNTAX ERROR. `up` does `nft add table` + `nft flush table` BEFORE loading, so re-arming with a bad value DELETED a working killswitch, installed nothing, and exited 0. The blanket fallback carried the same value, so it failed identically. Now: * the value is validated (`valid_ip`) and an unparseable one is UNSET, loudly; * `arm_failclosed` carries NO operator-supplied text at all -- the fallback for "the primary did not parse" must not contain what broke the parse. A-4 ROOT-SOURCED CONFIG (π΄). The file was `.`-sourced as root with no owner/mode check, so a stray `LAN_SUBNET=`/`IFACE=`/`NEBULA_USER=` line rewrote the killswitch's own constants and any other shell in it simply ran. Now the file is PARSED, not sourced: owner must be root or self, no group/other write, and only `NEBULA_LIGHTHOUSE` is read. A-3 APPLY GAP (π‘). `apply-airvpn-host.sh` installed the new helper but never created, chmod'd or checked the env file, so a fresh-host apply shipped a silently degraded killswitch. It now provisions it -- BEFORE installing the helper that reads it -- and refuses rather than proceeding, mirroring how the wg conf is handled at step 1. `claude/skills/bar/airvpn.md` had the same ordering defect (the requirement was a trailing sentence AFTER the install command) and is renumbered into dependency order. Also fixed while testing this: `log()` was defined BELOW its first callers, so every rejection reason above was `log: command not found`. The guards failed safe and said nothing -- which is most of their value gone. New: `airvpn-updown check-env`, a read-only action that prints the resolved value without touching a rule or route, and `scripts/tests/test_airvpn_updown_env.py` (42 tests) built on it. 10/10 mutants killed, each by the test that owns it. Deletes the `scripts/airvpn-updown` PENDING_SCRUB entry in the same commit, which is what the ratchet in #350 requires. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
F1. `test_airvpn_updown_env.py` used the real lighthouse IP's first three octets as the "partially-written file" fixture -- byte-identical to the prefix of the literal this PR deletes. Merging is what publishes it, and it narrows the address from "somewhere in Hetzner" to a /24. Neither gate can catch this: a 3-octet prefix is not an address, so `public_ip_scan.is_reportable()` is False by construction. TEST-NET-3 now, with a comment saying every literal in this file must be TEST-NET. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
β¦ake the protocol safe to follow
Round 2 of the adversarial audit. The core was confirmed sound (A-4 proven closed
by side effect, A-3 ordering real, the log() fix real); these are the findings
that were not.
F3 π΄ THE A-2 GUARD WAS SILENT FOR THE SHAPE A-2 IS NAMED FOR.
`done < "$path"` with a plain `while IFS= read -r line` DROPS a final line that
has no trailing newline -- and a file whose write was cut short is exactly a
file with no final newline. `NEBULA_LIGHTHOUSE=<ip>` with no `\n` resolved to
UNSET with NO log line, falling into the "key simply absent, no noise" branch,
while the PR body claimed "a partially-written env file was enough". Mutant X4
survived all 44 tests. Fixed with `|| [[ -n "$line" ]]`, plus coverage that
kills X4. CRLF shares the root cause and was MEASURED to resolve correctly
(the rtrim already eats `\r`); both tails are now pinned.
F4 π΄ THE LOAD IS NOW ONE ATOMIC TRANSACTION.
`nft add table` + `nft flush table` + `nft -f -` is three transactions, the
first two of which destroy the working killswitch before the third is known to
parse. Validating the env value shut one door into that state, not the only
one: the fallback still interpolates `$NEBULA_USER`, and
`id -un nebula-mesh 2>/dev/null || echo nebula-mesh` emits the name EVEN WHEN
THE ACCOUNT DOES NOT EXIST, so nft must resolve it via getpwnam -- on a fresh
host, or after a rename, BOTH rulesets fail and the uplink is left flushed and
empty at exit 0. Now one `nft -f` carrying `table {}` + `delete table` + the
definition, so a failed parse changes nothing. Mutant Y6 survived all 44 tests.
G6 the arm line no longer lies. It was unconditional and printed `armed` even
when the primary load AND the fallback had failed -- the line carrying this
PR's new `lighthouse=` signal was the one that could not be trusted. It now
ASKS THE KERNEL (`nft list table`) and reports ARMED(primary) /
ARMED(fallback) / NOT-ARMED. Mutant Y5 survived.
G1 `valid_ip` accepted values the ruleset cannot express: `::`, `:::::`,
`0::0::0`, real IPv6, and leading-zero octets. The consumers are `ip daddr`
(v4 family) and `ip route replace <v> via <v4 gw>`, so every one of those
passed validation and then failed the nft LOAD -> flush -> fallback, which has
no `meta mark` accept, so wg's own packets are dropped and THE TUNNEL DIES.
The previous `test_a_valid_ipv6_resolves` explicitly blessed that; it is
reversed. Leading zeros are octal to getent and decimal to nft, so they are
rejected rather than normalised, via `10#` rather than by a bash arithmetic
crash.
G4 a FIFO at $SITE_ENV blocked FOREVER -- at top level, i.e. inside wg-quick's
PostUp as root, with the interface UP and the killswitch NOT yet armed. An
indefinite fail-OPEN window and a NEW failure mode. Gated on `-f`.
G3 `check-env` could not distinguish absent from unreadable, so running the one
diagnostic the protocol tells you to trust WITHOUT sudo on a correct
0600 root:root host reported plain UNSET. It now reports `state=`.
G9 two guards were SPELLED, the trap their own docstrings claimed to avoid:
- the read-only check forbade a token list; mutants X2 (`echo x >/tmp/canary`)
and X3 (`touch`) both walked past it. Replaced by three complementary
properties -- no writes into a sandbox (behavioural), no command outside a
read-only set (behavioural, via a narrowed PATH), and no output redirection
to a path (syntactic, exhaustive over the redirection class). X2 survived my
FIRST replacement too; the syntactic property is what kills it.
- the fallback check grepped for the literal `NEBULA_LIGHTHOUSE`; mutant Y1
(alias it outside the function, interpolate the alias inside) restored the
hazard with the token gone. Now the RENDERED ruleset text is asserted, via a
fully-stubbed harness -- nft/ip/wg/logger are stubs, nothing real is touched.
F2 + G8 + G7 the operator protocol would have blackholed the host, and lives in
the durable artifact now, not just the PR body.
- It never said the tunnel must ALREADY BE UP. Re-arming with it down yields a
chain with no `meta mark` and no endpoint accept, ending in `drop`.
- It omitted the install step, so `check-env` would have hit the OLD deployed
script and printed usage.
- It never observed the /32 route. `ip route get <lighthouse>` is added as the
ONLY step that sees A-1; the off-LAN ssh check cannot substitute, because
with a healthy tunnel the traffic arrives either way.
- `claude/skills/bar/airvpn.md` re-test protocol + Debug/bail are rewritten,
and six tests pin them so the durable doc cannot drift back.
Also rewrites the fallback comment: it reached the right conclusion through the
exact firewall-vs-routing conflation this PR criticises elsewhere. The audit's
own finding is stronger -- `meta skuid` PRECEDES the lighthouse accept, so that
accept was DOMINATED and always a no-op. Only the /32 route ever mattered.
83 tests (was 44). 44 red at base 2c768d0, 83 green at HEAD. 24/24 mutants
killed, including all eight the audit named as survivors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
β¦e two unvalidated addresses Round 3 of the audit. Rebased onto current main now that #350 has merged, so the stacked-parent hazard is gone and branch CI shows the truth. P1 (π΄-equivalent) -- STEP 11 DID NOT DO WHAT THE PROTOCOL SAID. `ssh zach@10.42.0.30` is the WORKBENCH'S OWN nebula address, and step 1 puts the operator on the LAN. Followed literally it is a SELF-SSH; from another LAN host it is a same-LAN hop. Neither exercises the nebula direct-punch path that the section's own opening line says a LAN-only test cannot reach -- the path that locked the host out in #118. The step carrying the entire "off-LAN" claim never said which host to run it FROM, so it passed unconditionally. It now names the source host, carries a still-on-the-LAN guard, defines PASS / FAIL / INVALID, and says the protocol CANNOT BE COMPLETED without an off-LAN host rather than letting the self-ssh be substituted. π‘1 -- ARMED(none), a FOURTH state my own F4 made reachable. Before the atomic load, "both loads failed" implied the table was already flushed, so NOT-ARMED was the only outcome. After F4 the PREVIOUS table survives, so `ks_present` is true with no ruleset of ours installed -- reported as `ARMED(none)` while the line above it said "uplink is NOT filtered". The surviving table was built for a different endpoint/fwmark, so it can read as armed with a dead tunnel. Now `STALE(previous)`, with both contradictory lines fixed, four states pinned pairwise distinct, and the skill's "three states" list corrected to four. π‘3 (M12) -- the FALLBACK path's atomicity was untested. The atomicity test drove the harness with no failure injected, so it only ever saw the primary path; open-coding the old add+flush+load in `arm_failclosed` survived all 83 tests. Both callers are pinned now, every rendered load must be self-contained, and a reachability control proves the fallback really produced a second load. π‘4 -- `$EP` and `$GW` reached the ruleset with NO validation, while `valid_ip` -- hardened for exactly this -- was applied only to the lighthouse. A gateway-less default route yields the literal `eth0` from the awk; a peer with no endpoint makes wg print `(none)`; an IPv6 endpoint survives the sed. Each failed the PRIMARY load, dropping into the blanket fallback, which has no `meta mark` accept -- the state that kills the tunnel. nft also tries to DNS-RESOLVE these tokens, as root, inside wg-quick's PostUp. Both producers validate now. π‘2 / π‘5 -- two comments that were false. "A failure here changes NOTHING" is true of the helper and false of the composed path (four lines later the fallback replaces what survived). And the `\r` comment still carried the theory I retracted last round -- measured false, rtrim strips it -- corrected in the test but not in the script. π’ M5 -- the `10#` radix pin was UNREACHABLE: the leading-zero check fired first, so deleting `10#` changed nothing and the comment credited it anyway. Rather than ship an untestable guard with corrected prose, the two checks are REORDERED: radix-pinned range check first, leading-zero second. Identical verdict for every input, and `10#` is now load-bearing -- a mutant deleting it dies on "value too great for base". π’ P3 / P4 / P5 -- the precondition tested EXISTENCE not up-ness (an interface left by a half-failed `wg-quick up` printed TUNNEL-UP); the install step never said to confirm the branch, which is how the deployed helper was stale in the first place; and `ARMED(fallback)` was called merely "degraded" with no instruction to bail. Folds in the auditor's REAL X11 and X15, which my reconstruction had missed: the apply-script fixture now asserts exact bytes including the trailing newline (`.strip()` erased the byte F3 is about), and the prereq-ordering test anchors on the π΄ block's own text rather than a substring Procedure step 1 also contains. Also hardens the FIFO fixture that leaked 30 blocked processes into the operator's session, the oldest for 77 minutes: the cleanup opens the write end non-blocking and unlinks, on the green path too. 94 tests (was 85). 12 red at base 4fa6f4f, 94 green at HEAD. 32/32 mutants killed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ZacxDev
force-pushed
the
airvpn-killswitch-env
branch
from
August 6, 2026 19:29
51c799c to
ed17f4f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
π΄ DO NOT MERGE β this needs YOU, on a host you can physically reach
scripts/airvpn-updownis a LIVE fail-closed killswitch on the workbench's uplink. Everyclaim below is from unit tests,
bash -n,nft -cand reading the code. Thebarskill'smandatory re-test protocol has NOT been run and I cannot run it.
The protocol lives in
claude/skills/bar/airvpn.md(11 steps, pinned by nine tests). Read itthere, not here.
π΄ Round 3 fixed the step that the entire banner exists for. Step 11 said
ssh zach@10.42.0.30β the workbench's own nebula address β while step 1 puts you on theLAN. Followed literally that is a self-ssh; from another LAN host it is a same-LAN hop.
Neither touches the nebula direct-punch path that the section's opening line says a LAN-only
test cannot reach, and that locked the host out in #118. The step carrying the whole "off-LAN"
claim never said which host to run it from, so it passed unconditionally. It now names the
source host, carries a still-on-the-LAN guard, defines PASS / FAIL / INVALID, and says the
protocol cannot be completed without an off-LAN host rather than letting the self-ssh stand
in for one.
Now based on
main#350 has merged, so this is rebased onto current
mainand retargeted tomain. Thestacked-parent hazard is gone, and branch CI now shows the truth rather than the pre-existing
breakage #356/#357 fixed. It still deletes the
scripts/airvpn-updownPENDING_SCRUBentry#350 added β required, because that ratchet fails in both directions.
Round 3 β what the delta audit found
The audit confirmed a lot: F1's scrub verified across 17 encodings over all 702 tracked files;
F3 over 19 byte-level fixtures including every CR shape; F4's atomicity confirmed, and the
missing-table footgun confirmed real and confirmed guarded (a bare
delete tableon a missingtable aborts the file with rc=1;
table X {}+delete+ definition gives rc=0); G1, G6, G4,G3 correct and distinct; X2/X3/Y1 all killed at their own assertions. These are the findings
that were not sound.
π΄ P1 β step 11 (above)
π‘ 1 β
ARMED(none): a FOURTH state my own F4 made reachableBefore the atomic load, "both loads failed" implied the table had already been flushed, so
NOT-ARMEDwas the only reachable outcome. After F4 the previous table survives, soks_presentis true withKS_RULESET=noneβ and the arm line readARMED(none)while theline immediately above it said
FALLBACK β¦ uplink is NOT filtered, which is now false in thatcorner. Worse than meaningless: the surviving ruleset was built for a different endpoint and
fwmark, so it can read as armed while the tunnel is dead. No test covered it.
Now
STALE(previous), loud, with both contradictory lines fixed. The four states are pinnedpairwise distinct across every corner of (load outcome Γ kernel answer), and the skill's
"three states" list is now a four-row table with a bail column.
π‘ 3 β M12: the fallback path's atomicity was untested
test_the_load_is_ONE_atomic_transactiondrove the harness with no failure injected, so itonly ever inspected the primary path. Open-coding the old
add+flush+loadinsidearm_failclosedsurvived all 83 tests β the "second caller open-codes the old sequence"shape, de-atomising precisely the path that runs when things are already going wrong. Both
callers are pinned now, every rendered load must be self-contained, and a reachability
control proves the fallback actually produced a second load.
π‘ 4 β
$EPand$GWreached the ruleset with no validationvalid_ipβ hardened for exactly this β was applied to the lighthouse only. Measured withnft -c: a gateway-less default route (default dev eth0) makes the awk yield the literaleth0; a peer with no endpoint makeswg showprint(none); an IPv6 endpoint survives thesed. Each fails the primary load β blanket fallback β nometa markaccept β thetunnel dies. nft also tries to DNS-resolve these tokens, as root, inside wg-quick's
PostUp. Both producers validate now, with a positive control that valid values still land
(a validator that rejects everything is not a validator).
π‘ 2 and π‘ 5 β two comments that were false
"A failure here changes NOTHING" is true of the helper and false of the composed path:
four lines later
arm_failclosedreplaces whatever survived with the blanket fallback. And the\rcomment still carried the theory I retracted last round β measured false, the rtrimstrips it β corrected in the test docstring but not in the script.
π’ M5 β the
10#radix pin was unreachable, and the comment credited it anywayThe leading-zero check fired first, so deleting
10#made zero behavioural difference.Rather than ship an untestable guard with corrected prose, the two checks are reordered:
radix-pinned range check first, leading-zero second. Identical verdict for every input, and
10#is now load-bearing β a mutant deleting it dies onvalue too great for base.π’ P3 / P4 / P5
The precondition tested existence, not up-ness (
ip link show airvpnsucceeds for aninterface left by a half-failed
wg-quick up, and prints TUNNEL-UP) β it now requires the linkUP and a non-zero wg handshake. The install step never said to confirm the branch,
which is exactly how the deployed helper was found stale. And
ARMED(fallback)was calledmerely "degraded" with no instruction to bail β step 6 now says BAIL NOW for both
ARMED(fallback)andSTALE(previous), so you do not walk into step 10'scurland get aconfusing failure instead of a diagnosed one.
The auditor's real X11 and X15, which my reconstruction had missed
I had guessed these from their IDs and said so. Both were the G11-shaped defect and both are
now folded in:
printf 'NEBULA_LIGHTHOUSE=%s\n'β%ssurvived because the fixture asserted.strip()-ed content, erasing the very byte F3 is about. Now exact bytes.doc.index("/etc/airvpn-updown.env")is satisfied by Procedure step 1's own command. Nowanchored on the π΄ block's own text, plus an assertion that it still precedes
## Procedures.The 30 leaked processes
Thank you for killing those. That was my FIFO fixture:
subprocess.runkills its direct childon timeout, but a shell blocked in
open()on a writer-less FIFO can outlive it, and the X11gmutant (which removes the
-fgate) makes that reachable. The fixture now opens the write endnon-blocking and unlinks β on the green path too, because a cleanup that only fires on
failure is a cleanup that is never exercised.
π΄ A-1, restated (unchanged, and still REASONED not measured)
meta skuidis a firewall ACCEPT; the droppedip route replace <lighthouse> via <gw>is aROUTING bypass, and the first does not restore the second. With
suppress_prefixlength 0, onlya more-specific main-table route beats the tunnel default, so with the
/32gone the lighthouseroutes into the tunnel. Your audit strengthened this:
meta skuidprecedes thelighthouse
accept, so that accept was dominated β a no-op β in both rulesets. Only the/32ever mattered.Scope: WORKBENCH-ONLY. REASONED, NOT MEASURED β I have not observed a routing table with
the
/32absent. Step 7 (ip route get <lighthouse>) is how you measure it.What is measured
94 tests (was 85). Red/green: 12 failed / 82 passed at
4fa6f4f(10 distinct testfunctions), 94 passed at HEAD. Both tiers agree.
π΄ Four of this round's guards are INVARIANT guards, green at base, and I am labelling them
rather than counting them as regression coverage β their value is mutation-proof, not
base-red:
β¦load_is_ONE_atomic_transaction(both callers)β¦LEADING_ZERO_octet_is_REJECTEDMutation battery β 32 mutants, 32 killed, each at its own assertion. Two survived my first
attempt and are worth naming, because both were my own harness being weaker than it looked:
EPis only computed inside theif [[ -n "$GW" && -n "$PHYS" ]]branch β with the harness's default no-opip, my endpointtest never reached
endpoint_ipat all. It needed a real default route to become reachable,and now carries a control asserting the gateway landed.
A or Bβ the mutant only brokeA. Nowand.Authoritative gate, re-baselined against current
main(60e6d9d) β no longer attributinganything to a pre-existing red gate, since main is green:
origin/main(60e6d9d)RESULT: PASSRESULT: PASSNode tier: 1024/1024, PASS.
bash -nclean on both scripts. The local (non-sandbox) tiershows 26 failures on both
mainand this branch β identical set, environment-dependent,unchanged by this PR.
Correction accepted: my
test_browser_agent"ceiling crossing" diagnosis was wrong. Yourmeasurement (171.04s on main, 171.99s on the merged tree, 481 passed both) does not reproduce
it; the 291.68s run accompanied two real failures from the stale base, so the wall time was
inflated by an assertion timeout, not a budget crossing. Wall-time decomposition was the right
instrument and I drew the wrong conclusion from it.
What is NOT verified
nft -c's semantic pass (unprivileged: syntax only).uppath against a real kernel. The harness stubs every external command;nothing here arms, disarms, routes or loads a ruleset.
derivation β not the packet-level behaviour of the chain. Nine behaviour mutants survived
your earlier audit (inverting the uplink guard, never calling
arm_failclosed, dropping the/32route) and this round does not close them. That is the gap the operator protocol existsto cover, which is why P1 mattered.
π€ Generated with Claude Code