Skip to content

πŸ”΄ 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
mainfrom
airvpn-killswitch-env
Open

πŸ”΄ DO-NOT-MERGE-YET: airvpn killswitch β€” close the fail-open, stop root-sourcing the site config, fix the apply/doc order#355
ZacxDev wants to merge 4 commits into
mainfrom
airvpn-killswitch-env

Conversation

@ZacxDev

@ZacxDev ZacxDev commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

πŸ”΄ DO NOT MERGE β€” this needs YOU, on a host you can physically reach

scripts/airvpn-updown is a LIVE fail-closed killswitch on the workbench's uplink. Every
claim below is from unit tests, bash -n, nft -c and reading the code. The bar skill's
mandatory 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 it
there, 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 the
LAN. 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 main and retargeted to main. The
stacked-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-updown PENDING_SCRUB entry
#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 table on a missing
table 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 reachable

Before the atomic load, "both loads failed" implied the table had already been flushed, so
NOT-ARMED was the only reachable outcome. After F4 the previous table survives, so
ks_present is true with KS_RULESET=none β€” and the arm line read ARMED(none) while the
line immediately above it said FALLBACK … uplink is NOT filtered, which is now false in that
corner. 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 pinned
pairwise 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_transaction drove the harness with no failure injected, so it
only ever inspected the primary path. Open-coding the old add+flush+load inside
arm_failclosed survived 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 β€” $EP and $GW reached the ruleset with no validation

valid_ip β€” hardened for exactly this β€” was applied to the lighthouse only. Measured with
nft -c: a gateway-less default route (default dev eth0) makes the awk yield the literal
eth0; a peer with no endpoint makes wg show print (none); an IPv6 endpoint survives the
sed. Each fails the primary load β†’ blanket fallback β†’ no meta mark accept β†’ the
tunnel 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_failclosed replaces whatever survived with the blanket fallback. And the
\r comment still carried the theory I retracted last round β€” measured false, the rtrim
strips it β€” corrected in the test docstring but not in the script.

🟒 M5 β€” the 10# radix pin was unreachable, and the comment credited it anyway

The 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 on value too great for base.

🟒 P3 / P4 / P5

The precondition tested existence, not up-ness (ip link show airvpn succeeds for an
interface left by a half-failed wg-quick up, and prints TUNNEL-UP) β€” it now requires the link
UP 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 called
merely "degraded" with no instruction to bail β€” step 6 now says BAIL NOW for both
ARMED(fallback) and STALE(previous), so you do not walk into step 10's curl and get a
confusing 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:

  • X11 β€” printf 'NEBULA_LIGHTHOUSE=%s\n' β†’ %s survived because the fixture asserted
    .strip()-ed content, erasing the very byte F3 is about. Now exact bytes.
  • X15 β€” moving the whole prerequisite block to the end of the skill file survived because
    doc.index("/etc/airvpn-updown.env") is satisfied by Procedure step 1's own command. Now
    anchored 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.run kills its direct child
on timeout, but a shell blocked in open() on a writer-less FIFO can outlive it, and the X11g
mutant (which removes the -f gate) makes that reachable. The fixture now opens the write end
non-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 skuid is a firewall ACCEPT; the dropped ip route replace <lighthouse> via <gw> is a
ROUTING bypass, and the first does not restore the second. With suppress_prefixlength 0, only
a more-specific main-table route beats the tunnel default, so with the /32 gone the lighthouse
routes into the tunnel. Your audit strengthened this: meta skuid precedes the
lighthouse accept, so that accept was dominated β€” a no-op β€” in both rulesets. Only the
/32 ever mattered.

Scope: WORKBENCH-ONLY. REASONED, NOT MEASURED β€” I have not observed a routing table with
the /32 absent. 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 test
functions), 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:

guard why it is green at base proven by
…load_is_ONE_atomic_transaction (both callers) the fix was in the TEST; both callers were already atomic M12
apply fixture asserting exact bytes the apply script already wrote the newline X11
prereq-ordering anchored on the πŸ”΄ block the block was already in the right place X15
…LEADING_ZERO_octet_is_REJECTED the verdict is unchanged; only the check ORDER moved M5

Mutation 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:

  • G4b (delete the endpoint validation) survived because EP is only computed inside the
    if [[ -n "$GW" && -n "$PHYS" ]] branch β€” with the harness's default no-op ip, my endpoint
    test never reached endpoint_ip at all. It needed a real default route to become reachable,
    and now carries a control asserting the gateway landed.
  • P1m (change the off-LAN step's source host back) survived an assertion written as
    A or B β€” the mutant only broke A. Now and.
group mutants result
original battery M1–M10 10 10 killed
audit-named survivors X2/X3/X4/X11g/X15g/Y5/Y6/Z1/Z2/Z3/Z4/Z6 12 12 killed
the auditor's real X11, X15 2 2 killed
new: M12, M5, S1, G4a, G4b, P1m, P3m, P4m, P5m 9 9 killed

Authoritative gate, re-baselined against current main (60e6d9d) β€” no longer attributing
anything to a pre-existing red gate, since main is green:

origin/main (60e6d9d) this branch
collected 6311 6405 (+94)
passed 6310 6404 (+94)
skipped 1 1
failed 0 0
verdict RESULT: PASS RESULT: PASS

Node tier: 1024/1024, PASS. bash -n clean on both scripts. The local (non-sandbox) tier
shows 26 failures on both main and this branch β€” identical set, environment-dependent,
unchanged by this PR.

Correction accepted: my test_browser_agent "ceiling crossing" diagnosis was wrong. Your
measurement (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

  • The re-test protocol. Not run β€” the whole of the banner at the top.
  • nft -c's semantic pass (unprivileged: syntax only).
  • The real up path against a real kernel. The harness stubs every external command;
    nothing here arms, disarms, routes or loads a ruleset.
  • The A-1 routing claim, as stated above.
  • πŸ”΄ G10 still stands: the suite covers env resolution, ruleset RENDERING and arm-state
    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
    /32 route) and this round does not close them. That is the gap the operator protocol exists
    to cover, which is why P1 mattered.

πŸ€– Generated with Claude Code

ZacxDev and others added 4 commits August 6, 2026 14:05
…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
ZacxDev force-pushed the airvpn-killswitch-env branch from 51c799c to ed17f4f Compare August 6, 2026 19:29
@ZacxDev
ZacxDev changed the base branch from scrub-public-ips to main August 6, 2026 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant