Skip to content

fix(nebula): drop the dead :443 lighthouse fallback, and mark the script that added it superseded - #1361

Open
ZacxDev wants to merge 6 commits into
mainfrom
fix/nebula-drop-dead-443
Open

fix(nebula): drop the dead :443 lighthouse fallback, and mark the script that added it superseded#1361
ZacxDev wants to merge 6 commits into
mainfrom
fix/nebula-drop-dead-443

Conversation

@ZacxDev

@ZacxDev ZacxDev commented Sep 7, 2026

Copy link
Copy Markdown
Member

Removes a nebula fallback that cannot ever work, and marks the script that added it superseded.

The defect

apply-nebula-443.sh appended <lighthouse>:443 to the prod lighthouse's staticHostMap as a fallback for networks blocking UDP 4242. Measured — it is inert:

fact source
the prod nebula lighthouse listens on 4242 only clusters/production/apps/nebula/lighthouse/lighthouse-config.yamllisten.port
443/udp on that host is WireGuard k0s/host-firewall/relay-firewall.sh443/udp (wireguard)
no 443→4242 redirect exists grepped across homelab-infra

Nebula handshakes a socket that does not speak nebula and gets nothing back.

The harm is not the wasted handshakes — it is that the entry reads as a fallback. In the one situation it was written for, there is none, while the config asserts otherwise. That matters now: the operator is about to be off-LAN for months and nebula is the only remote path to the workbench. Tailscale is the real answer to blocked-UDP (clawgate #497); this just stops the config claiming a safety net it does not have.

Shape

Follows PR #1272 (rank 1 of the same effort): --check mode that is read-only and rootless and reads the running unit's config; a host guard on the mesh address because workbench and portable both answer nixos; temp-patch → nix-instantiate --parse → unique backup → move → rebuild, with a trap that restores on any later failure and distinguishes "file restored, system already switched"; idempotent.

🔴 The lighthouse IP is not written into this public repo. The script derives it from the :4242 entry beside the :443 one and refuses unless both name the same host — which is also a correctness check, since a :443 beside a different host is not this entry.

sed, not awk — 3-arg match() is a gawk extension. sed reports no substitution count, so the edit is verified from the result: exactly one changed line, line count unchanged, no :443 surviving.

Verified

Substitution, on a fixture: 1 line changed · :443 gone · trailing comment preserved · line count unchanged · output parses as Nix · second pass is a no-op.
🔴 Negative control: a "10.42.0.1" = [ "a:4242" "b:4242" ]; row — two addresses, neither :443 — is left byte-identical. That is the row a looser pattern would have clobbered.

--check on both hosts, because a checker that cannot go red is not a checker:

workbench (no :443)        -> PASS  rc 0
portable  (dead entry live)-> FAIL  rc 1   naming the address

Not verified

The apply path has not been run. /etc/nixos/configuration.nix on the portable is root-only, so unlike on the workbench the patch anchor could not be checked against the real file. The script therefore derives its target at runtime and aborts rather than guesses if the shape is not what apply-nebula-443.sh leaves behind — including an explicit refusal when a :443 string is present in some other form.

To land it: sudo env "PATH=$PATH" bash nix/system/apply-nebula-drop-443.sh on the portable.

Refs clawgate #497 (rank 2) · claim nebula-pre-departure-hardening-2 · follows #1272.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TcDJ4ccN8savidfXbHPoqh

ZacxDev and others added 2 commits September 7, 2026 14:17
…ipt that added it superseded

apply-nebula-443.sh appended a second address for the prod lighthouse,
`<ip>:443`, as a fallback for networks that block UDP 4242. It cannot ever work:

  * the prod nebula lighthouse listens on 4242 ONLY
    (clusters/production/apps/nebula/lighthouse/lighthouse-config.yaml)
  * 443/udp on that host is WIREGUARD (k0s/host-firewall/relay-firewall.sh)
  * no 443->4242 redirect exists anywhere in homelab-infra

So nebula handshakes a socket that does not speak nebula and gets nothing back.

The harm is not the wasted handshakes. It is that the entry READS AS A FALLBACK:
in the one situation it was written for — a hotel or cafe blocking UDP 4242 —
there is no fallback at all, while the config asserts there is one. That matters
now because the operator is about to be off-LAN for months and nebula is the only
remote path to the workbench. Tailscale is the real answer to blocked-UDP
(clawgate #497); this only stops the config claiming a safety net it does not have.

apply-nebula-drop-443.sh removes it, in the shape PR #1272 established:

  * `--check` mode is READ-ONLY and needs no root — it reads the config the
    RUNNING unit loaded, so a stale edit that was never switched cannot read as
    satisfied. rc 0 gone / 1 present / 2 cannot tell.
  * host guard on the MESH ADDRESS, not `hostname`: the workbench and the
    portable both answer `nixos`, so hostname cannot discriminate. Defaults to
    the portable (10.42.0.100), where apply-nebula-443.sh ran.
  * the lighthouse IP is NOT written into this PUBLIC repo. The script derives it
    from the `:4242` entry beside the `:443` one and refuses unless both name the
    same host — which is a correctness check too, since a `:443` beside a
    different host is not the entry this removes.
  * patch a temp copy -> `nix-instantiate --parse` -> uniquely-named backup ->
    move into place -> rebuild, with a trap that restores on ANY later failure
    and says explicitly when the FILE was restored but the system was already
    switched. Idempotent: already-gone exits 0 without touching the file.
  * sed, not awk: 3-arg `match()` is a gawk extension and this must not depend on
    which awk the host ships. sed reports no substitution count, so the edit is
    verified from the result instead — exactly one changed line, line count
    unchanged, and no `:443` string surviving.

VERIFIED before committing:

  substitution, on a fixture — 1 line changed, `:443` gone, trailing comment
  preserved, line count unchanged, output parses as Nix, and a second pass is a
  no-op. 🔴 Negative control: a `"10.42.0.1" = [ "a:4242" "b:4242" ];` row — two
  addresses, neither of them :443 — is left byte-identical, which is the row a
  looser pattern would have clobbered.

  `--check`, on BOTH hosts, because a checker that cannot go red is not a
  checker: workbench (no :443) -> PASS rc 0; portable (dead entry live) -> FAIL
  rc 1 naming the address.

NOT verified: the apply path itself has not been run — /etc/nixos/configuration.nix
on the portable is root-only, so the patch anchor could not be checked against the
real file the way it was on the workbench. The script therefore derives its target
at runtime and ABORTS rather than guessing if the shape is not the one
apply-nebula-443.sh leaves behind, including an explicit refusal when a `:443`
string is present in some other form.

Refs clawgate #497 (rank 2), claim nebula-pre-departure-hardening-2.
Follows PR #1272, which did rank 1.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcDJ4ccN8savidfXbHPoqh
Claude-Session-Id: 5e2d201d-d6e6-481d-85e9-0792db43cebf
…refuse unknown args, and verify against the RUNNING process

Round 1 of the adversarial audit found no 🔴 and verified the PR's factual
premise (lighthouse 4242-only, 443/udp is WireGuard, no redirect, no IP leak).
Everything below is a fix for what it did find.

#1/#2 — the `:443` searches were FILE-WIDE, so one unrelated quoted `:443`
anywhere in configuration.nix (an nginx `proxyPass = "https://up:443"` is the
obvious one) aborted the run with "a ':443' string survived the patch", blaming
a substitution that had worked — and made the documented "re-running is a no-op"
false for the same reason. It failed closed, so never dangerous; it was
UNUSABLE, and the portable's configuration.nix has never been read. Both
searches now go through `hostmap_443()`, scoped to staticHostMap-shaped lines.

#3 — `MODE="${1:-apply}"` was compared only against `--check`, so `--dry-run`,
`--help` and `-c` all ran the full patch + `nixos-rebuild switch`. For a root
script that rebuilds the OS the exploratory invocations were the destructive
ones. Now an explicit `case` with `exit 64` on anything unrecognised.

#4 — `--check`'s rc 2 ("cannot tell") was collapsed into the failure branch, so
an unreadable verifier rolled the change back while asserting ":443 is still
advertised" — a claim the check never made — and offered a rollback that
RE-ADDS the dead entry. Now three outcomes, with one retry on rc 2 and an
honest message that says it is evidence of neither success nor failure.

#5 — `--check` read `systemctl cat`, i.e. the unit file on disk AFTER a switch,
not what the running process was started with; a deferred restart therefore read
as PASS while the dead address was still live. Now reads
`/proc/<MainPID>/cmdline`, which is what the header always claimed.

#6 — `--check` had no positive control and degraded silently to "always PASS":
of five plausible YAML shapes carrying a live `:443`, four passed. It now
requires the extracted block to contain at least one `:4242`, else rc 2.

#7 — `SWITCHED=1` was set only after `nixos-rebuild switch` returned 0, but
switch-to-configuration restarts units and THEN activates, so a partial switch
made the trap print "the system was never switched" for exactly the case where
something was. Set before the call.

#8 — added a dry-build of the CURRENT, unpatched config in preflight. This is
the real stranding vector and it is not the :443 removal: a switch applies every
pending edit in configuration.nix, and the rollback restores only this script's
token. The header now says so explicitly.

#9 — added a post-switch reachability probe. Deliberately ADVISORY: ICMP to a
lighthouse mesh address is measured-flaky on this fleet (10.42.0.2 answered from
the workbench after one restart and not another, while discovery worked
throughout), so a failed ping must not roll back a good change.

#10 — CLAUDE.md named apply-nebula-443.sh as the shape to copy for staged
system changes, and this PR makes its first line "DO NOT RUN". Repointed at
apply-nebula-drop-443.sh with the properties worth copying named.

#11 — apply-nebula-443.sh now REFUSES to run behind an explicit override env
var. 14 handoff docs still name it; a banner comment does not stop an execution.

#12 — added scripts/tests/test_nebula_drop_443.py (14 cases), following the
precedent of test_freeze_instrumentation.py. Findings #1, #2 and #3 would each
have been caught by it.

Nits: `diff` and `wc` added to the tool preflight (both load-bearing — without
`diff` the changed-line count is 0 and the script dies with a WRONG diagnosis);
`nix-instantiate --parse` now runs on $CFG first as a control so a pre-existing
syntax error is not misattributed to the patch; mode 100755 per the repo's own
convention.

🔴 ONE FIX CAME FROM WRITING THE TEST, not from the audit: the sed matched a
`:4242`/`:443` pair regardless of whether the two addresses named the SAME host
— only the shell guard's ORDERING made that safe. The expression now carries a
`\2` backreference and discriminates on its own.

VERIFIED: 14/14 pytest cases pass. The fixed script was then driven end-to-end
with shimmed id/ip/systemctl/nixos-rebuild against synthetic configs — #1 pair +
unrelated nginx :443 -> rc 0, pair removed, nginx line kept; #2 already-gone +
unrelated :443 -> rc 0 no-op; #3 `--dry-run` -> rc 64 with the config untouched;
negative control, mismatched hosts -> rc 1 refused. The success path exercised
the new /proc-reading --check tier against a real process.

Refs clawgate #497, audit round 1 on PR #1361.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcDJ4ccN8savidfXbHPoqh
Claude-Session-Id: 5e2d201d-d6e6-481d-85e9-0792db43cebf
@ZacxDev

ZacxDev commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Round 1 audit — fixes pushed as c5bd8b86

Round 1 found no 🔴 and independently verified the PR's factual premise (lighthouse listens 4242-only; 443/udp is WireGuard; no 443→4242 redirect; no IP committed). All 12 🟡 and the nits are addressed.

One fix did not come from the audit — it came from writing the test: the sed matched a :4242/:443 pair regardless of whether the two addresses named the same host. Only the shell guard's ordering made that safe. The expression now carries a \2 backreference and discriminates on its own.

Verified: 14/14 new pytest cases; then the fixed script driven end-to-end with shimmed id/ip/systemctl/nixos-rebuild — pair + unrelated nginx :443 → rc 0 with the nginx line kept; already-gone + unrelated :443 → rc 0 no-op; --dry-run → rc 64 with the config untouched; mismatched hosts → rc 1 refused. The success path exercised the new /proc-reading --check tier against a real process.

1. #1/#2 both `:443` searches now go through `hostmap_443()`, scoped to staticHostMap-shaped lines instead of the whole file; the "re-running is a no-op" claim in the header is restored to true.
2. #3 argument handling is an explicit `case` with `exit 64` on anything unrecognised; `--dry-run` no longer runs the destructive path.
3. #4 post-switch verification distinguishes rc 0/1/2, retries once on rc 2, and no longer asserts ":443 still advertised" when the verifier could not read anything.
4. #5 `--check` reads `/proc/<MainPID>/cmdline` instead of `systemctl cat`, so a deferred restart cannot read as PASS.
5. #6 `--check` requires the extracted static_host_map block to contain at least one `:4242`, else rc 2 — a positive control against silent always-PASS.
6. #7 `SWITCHED=1` is set BEFORE `nixos-rebuild switch`, so a partial switch no longer prints "the system was never switched".
7. #8 preflight dry-builds the CURRENT unpatched config, and the header states that a switch applies every pending edit in configuration.nix.
8. #9 a post-switch reachability probe was added, deliberately advisory because lighthouse ICMP is measured-flaky on this fleet.
9. #10 CLAUDE.md no longer names the superseded script as the exemplar; it points at apply-nebula-drop-443.sh.
10. #11 apply-nebula-443.sh now refuses to run behind an explicit override env var, rather than only carrying a banner comment.
11. #12 scripts/tests/test_nebula_drop_443.py added (14 cases), following test_freeze_instrumentation.py; findings #1, #2 and #3 would each have been caught by it.
12. nits: `diff` and `wc` added to the tool preflight; `nix-instantiate --parse` runs on $CFG first as a control; mode 100755.
13. NOT from the audit — the sed now uses a `\2` backreference so it requires both addresses to name the same host.

Still not verified, unchanged from the PR body: the apply path has never run against the portable's real /etc/nixos/configuration.nix, which is root-only and has never been read. The script derives its target at runtime and aborts rather than guesses.

ZacxDev and others added 2 commits September 7, 2026 15:28
…nder set -e

F1 (🔴). `check_one; check_rc=$?` is unreachable past the call: `check_one`
re-enables `-e` before returning and a bare simple command is subject to
errexit, so a non-zero return exits the shell BEFORE the assignment. The whole
three-outcome block, which was round 1's headline fix, was dead for rc 1 and
rc 2. Measured: stub rc 1 and rc 2 both exited with the next line never reached.

The consequence was worse than an inert fix. A transient post-switch read
failure (unit still restarting, MainPID momentarily 0) exits the shell, the EXIT
trap restores the backup -- RE-ADDING the dead :443 entry to a file the switch
had already corrected -- and prints only "ROLLED BACK … run nixos-rebuild switch
to return it", which if followed re-applies the dead address to the running
system. Fixed at BOTH call sites (`check_rc=0; check_one || check_rc=$?`); the
audit named one, the retry a few lines below had the identical shape.

F2 (🟡). Round 1 narrowed BOTH `:443` searches to fix the nginx false-abort, but
they answer different questions. `hostmap_443` asks "did a :443 survive MY
patch?" and must be scoped; the no-op branch asks "might I be failing to see a
:443?" and must be WIDE, because its job is to refuse rather than guess. Sharing
the scoped one made it miss a hand-wrapped multi-line list and print
`ALREADY GONE … Nothing to do` with the entry still present -- a
refuse-rather-than-guess guard failing OPEN into a success-shaped message. Split
into `hostmap_443` (scoped) and `any_443_addr` (wide, still narrow enough to
ignore an nginx `host:443` URL).

F3 (🟡). Two overclaims retracted. (a) `dry-build` EVALUATES -- per man 8
nixos-rebuild it "shows what store paths would be built … but otherwise does
nothing" -- so it cannot make every later failure attributable to this change; a
derivation that fails to COMPILE passes it and fails during the real switch. The
control now claims evaluation only, and says so in the output and the header.
(b) `nixos-rebuild` reads /etc/nixos regardless of $CFG, so with NEBULA_CFG
pointed elsewhere the control spoke about a file the script was not patching.
Skipped there rather than asserted wrongly.

F5 (🟢). The control moved below the no-op checks, so a run with nothing to do
no longer pays a full evaluation, and no longer dies on an unrelated broken
pending edit while having no work to do. The header's "re-running is a no-op"
is true again.

F6 (🟢). A missing `ping` returns 127 with stderr swallowed, rendering
identically to "the lighthouse did not answer" -- which the comment would have
blamed on fleet ICMP flakiness. Now distinguished explicitly. Not preflighted as
a hard requirement: the probe is advisory and its absence must not block.

F7 (🟢). "14 handoff docs name this script" was wrong -- re-measured with GNU
grep: 9. The argument survives; the number did not. (The repo-wide figure is
dropped rather than corrected: I did not validate it.)

Also from the audit's non-findings, both being claims: the SWITCHED rationale
said switch-to-configuration "restarts units and THEN activates" (real order is
stop -> activate -> start; the conclusion holds), and CLAUDE.md said nix/system
holds "nine of these" when it holds 17.

F4 (🟢, scaffolding). The scoping guard was SPELLED -- it asserted the string
`hostmap_443()` was present, so a mutant widening the helper's BODY back to a
whole-file grep, reintroducing round-1 #1 verbatim, left it green. Now
behavioural: the helpers' real regexes are extracted from the script and run
against fixtures. Also killed the loosened `:443$` mutant. And the module
docstring named two tests that did not exist -- corrected, and now pinned by
`test_docstring_names_only_real_tests`.

RESOLVED, an open question from the round-2 report: the target machine's
/etc/nixos is NEITHER a flake NOR a git repo (checked on the host). So
`nixos-rebuild switch` reads configuration.nix directly and the uncommitted-edit
concern does not apply.

VERIFIED: 19/19 pytest. Then driven end-to-end with shims -- F1: the retry runs,
the honest "COULD NOT VERIFY" message is reached, no false ":443 still
advertised" claim, and the rollback still happens (correct for an unverified
state). F2: a multi-line entry is refused, no "ALREADY GONE", the entry is left
intact. Round 1's four cases all still pass.

Refs clawgate #497, audit round 2 on PR #1361.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcDJ4ccN8savidfXbHPoqh
Claude-Session-Id: 5e2d201d-d6e6-481d-85e9-0792db43cebf
…erclaim F3 retracted

The script's own header was corrected in 59d8dd7; the doc a human reads FIRST
was not, so the code said 'this only evaluates' while CLAUDE.md said 'a later
failure is attributable'. Also adds the errexit rule to what is worth copying,
since F1 was exactly that mistake in the exemplar itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcDJ4ccN8savidfXbHPoqh
Claude-Session-Id: 5e2d201d-d6e6-481d-85e9-0792db43cebf
@ZacxDev

ZacxDev commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Round 2 audit — fixes pushed as 59d8dd79 + ec19192d

Round 2 found a 🔴 I introduced in round 1: check_one; check_rc=$? is dead code under set -e. The bare call exits the shell on any non-zero return, so the entire three-outcome verification — round 1's headline fix — was unreachable for rc 1 and rc 2. Confirmed independently before fixing: stub rc 1 and rc 2 both exited with the next line never reached.

Worse than inert: a transient post-switch read failure rolled back a change that had applied, re-adding the dead :443 entry, and printed advice that would re-apply it to the running system. Fixed at both call sites — the audit named one; the retry a few lines below had the identical shape.

Resolved, an open question from the round-2 report: the target machine's /etc/nixos is neither a flake nor a git repo (checked on the host), so nixos-rebuild switch reads configuration.nix directly and the uncommitted-edit concern does not apply.

Verified: 19/19 pytest, then driven end-to-end with shims — F1: retry runs, honest COULD NOT VERIFY reached, no false ":443 still advertised", rollback still occurs (correct for an unverified state). F2: multi-line entry refused, no ALREADY GONE, entry intact. Round 1's four cases still pass.

1. F1 both `check_one` call sites now use `check_rc=0; check_one || check_rc=$?`, so the rc 0/1/2 block and the 5s retry are reachable.
2. F2 the two `:443` searches are split: `hostmap_443` (scoped, post-patch survival) and `any_443_addr` (wide, refuse-rather-than-guess); a hand-wrapped multi-line entry is now refused instead of reported ALREADY GONE.
3. F3a the preflight control claims EVALUATION only, in its output and in the header, because `dry-build` does not build.
4. F3b the control is skipped when NEBULA_CFG is not /etc/nixos/configuration.nix, rather than asserting about a file nixos-rebuild does not read.
5. F5 the control moved below the no-op checks, so a run with nothing to do pays no evaluation and cannot die on an unrelated pending edit.
6. F6 a missing `ping` is reported distinctly from a failed probe, rather than being attributed to fleet ICMP flakiness.
7. F7 the handoff-doc count corrected from 14 to 9; the unvalidated repo-wide figure dropped rather than corrected.
8. F4 the scoping guard is now behavioural — the helpers' real regexes are extracted from the script and run against fixtures — and the module docstring's test names are pinned by a test.
9. Non-findings from the round-2 report, both being claims: the SWITCHED comment's activation order (stop -> activate -> start), and CLAUDE.md's "nine of these" -> 17.
10. CLAUDE.md's exemplar sentence, which still carried the dry-build overclaim after the script's own header was corrected; the errexit rule was added to what is worth copying.

Still not verified, unchanged: the apply path has never run against the portable's real /etc/nixos/configuration.nix (root-only, never read).

…the repo's gate red

R3-1 (🔴). The two fixtures I added last round used `1.2.3.4` — globally
routable, in no documentation range — and `scripts/tests/test_no_public_ips.py`
fails on it. Merging would have left `main`'s pytest tier red for everyone, and
per CLAUDE.md nothing blocks a merge here. Now `203.0.113.9` (TEST-NET-3), which
the rest of the file already used.

🔴 THE VERIFICATION CLAIM IS THE REAL LESSON. Last round's message said
"VERIFIED: 19/19 pytest". That was TRUE and structurally blind: it named one
file, and the failure was in another. A green run covers only what it ran. This
round runs the adjacent set — 187 passed across test_nebula_drop_443,
test_no_public_ips, test_no_client_hostnames, test_nix_read_paths,
test_doc_path_rot.

R3-2 + R3-3 (🟡, one regex from both sides). `any_443_addr` matched `"<ip>:443"`
anywhere, so it (a) fired on an unrelated WireGuard peer `endpoint` or nginx
upstream — aborting a good run and telling the operator to delete a line that
legitimately exists, on a fleet where 443/udp genuinely IS WireGuard — and (b)
MISSED a DNS name or IPv6 literal in the staticHostMap, i.e. it was NARROWER
than the scoped helper on the axis it was widened for, failing OPEN into
`ALREADY GONE`. Both directions wrong at once, because it discriminated on what
the address LOOKS like.

It now scopes by CONTEXT: a brace-matched `staticHostMap = { … }` block, plus a
bare-quoted-address-alone-on-its-line rule and a one-line-entry rule. None of
the three can match a WireGuard `endpoint =` or an nginx `proxyPass`.

🔴 AND THE FIRST VERSION OF THAT FIX RE-OPENED THE HOLE. Block-matching ALONE
returns an empty block for a file whose `staticHostMap = {` cannot be located,
and an empty block rendered as "no :443 anywhere" — F2 again, one round later.
Caught by re-running the round-2 regression drive after the rewrite, not by
reading it. Hence the union, and a positive control that refuses when the file
mentions staticHostMap but the block will not brace-match.

R3-4 (🟡). The errexit rule I added to CLAUDE.md — the bullet whose entire job
is to be copied verbatim — dropped the `rc=0` pre-init, so under `set -u` it
crashes on the SUCCESS path. It traded F1's bug for its mirror image. Now states
the whole form and says why each half is load-bearing.

R3-5 (🟡). "nixos-rebuild never reads $CFG" is true of the ENTRYPOINT only: a
$CFG that is `imports`ed by configuration.nix — the usual reason to put a nebula
block in its own module, so the likeliest reason to set NEBULA_CFG at all — IS
evaluated transitively. The control was skipped exactly there, printing a false
reason. It now always runs, and a $CFG outside the entrypoint gets an explicit
warning that the switch may evaluate a different configuration than the one
being patched.

R3-6 (🟢, scaffolding). `test_check_pattern_does_not_match_a_longer_port` said
"pins the boundary behaviourally" and then asserted against a literal retyped in
the test — so it exercised grep, not the script. It now lifts the pattern from
the script, like `_sed_expr` does.

The helper guards are now behavioural in the strongest available sense: the
shipped function definitions are EXTRACTED from the script and executed, rather
than their patterns being re-typed. Two bugs in that harness were found by
running it — the helper names appear in the comment ABOVE the definitions (so a
naive `.index()` yielded an EMPTY fragment and every test died with "command not
found"), and `any_443_addr` became multi-line (so capturing one line truncated
it mid-function). Both now assert loudly instead of failing obscurely.

VERIFIED: 187 passed across the five files above. Both regression drives still
green — round-1's four cases and round-2's F1/F2 cases, the latter of which is
what caught the re-opened hole.

Refs clawgate #497, audit round 3 on PR #1361.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcDJ4ccN8savidfXbHPoqh
Claude-Session-Id: 5e2d201d-d6e6-481d-85e9-0792db43cebf
@ZacxDev

ZacxDev commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Round 3 audit — fixes pushed as 82054827

Round 3 found a 🔴 in my own test fixtures: they used a globally routable IP literal (in no documentation range), and the repo's own test_no_public_ips.py fails on it. Merging would have left main's pytest tier red. Reproduced before fixing — 1 failed at head, 15 passed at the round-2 base — and now 203.0.113.9 (TEST-NET-3), which the rest of the file already used.

🔴 The real lesson is the verification claim. Round 2's message said "VERIFIED: 19/19 pytest" — true, and structurally blind: it named one file, and the failure was in another. A green run covers only what it ran. This round runs the adjacent set: 187 passed across test_nebula_drop_443, test_no_public_ips, test_no_client_hostnames, test_nix_read_paths, test_doc_path_rot.

🔴 And my first fix for R3-2/R3-3 re-opened F2. Block-matching alone returns an empty block when staticHostMap = { cannot be located, and an empty block rendered as "no :443 anywhere" — the round-2 finding again, one round later. Caught by re-running the round-2 regression drive after the rewrite, not by reading it. The shipped version is a union of three scopes plus a positive control that refuses when the file mentions staticHostMap but the block will not brace-match.

Footnote worth recording: posting this comment was blocked twice by the repo's own pre-commit guard for quoting the offending literal while describing the finding. The guard is right, and it is the same class of check that caught R3-1.

1. R3-1 both fixtures use TEST-NET-3; test_no_public_ips passes, and the adjacent gate set (187 tests) is run rather than a single file.
2. R3-2 any_443_addr no longer matches a WireGuard peer `endpoint =` or an nginx `proxyPass`; it scopes by context, not by address shape.
3. R3-3 any_443_addr now catches DNS-name and IPv6 entries inside the staticHostMap, so it is no longer narrower than the scoped helper it was split from.
4. The R3-2/R3-3 fix carries a positive control: it refuses when $CFG mentions staticHostMap but the block cannot be brace-matched, rather than reporting a clean file it did not read.
5. R3-4 the CLAUDE.md errexit rule now states the whole form `rc=0; check_one || rc=$?` and says why each half is load-bearing under set -e and set -u respectively.
6. R3-5 the preflight control always runs; the false "nixos-rebuild never reads $CFG" skip reason is gone, replaced by an explicit warning when $CFG is not the entrypoint.
7. R3-6 test_check_pattern_does_not_match_a_longer_port lifts the pattern from the script instead of retyping it.
8. The helper guards execute the SHIPPED function definitions, extracted from the script, rather than re-typed patterns; two extraction bugs found by running it now assert loudly instead of failing obscurely.

Still not verified, unchanged across all three rounds: the apply path has never run against the portable's real /etc/nixos/configuration.nix (root-only, never read). Both regression drives — round-1's four cases and round-2's F1/F2 — remain green.

…it a fourth time

Round 4 returned six findings, four of them in one place: `any_443_addr`, the
"is a :443 present in some OTHER shape?" detector. Its history across this PR:

  r1 #1  whole-file grep      -> aborted on an unrelated :443; script UNUSABLE
  r2 F2  scoped to one-liners -> missed a wrapped list; ALREADY GONE, entry live
  r3 R3-2/3 address-shape widen -> fired on a WireGuard `endpoint` AND still
                                   missed DNS/IPv6 — wrong in both directions
  r4 F1  block-relative line numbers printed as FILE line numbers, pointing the
         operator at the wrong line of a root-owned file to hand-edit
  r4 F2  defeated by a comment mentioning staticHostMap, and by a second nebula
         network; the positive control added for exactly this saw neither,
         because it tests for an EMPTY extraction and not a WRONG one
  r4 F3  the over-fire returned in a narrower shape: a multiline list of
         "host:443" strings anywhere in the file aborts a clean run

Four fixes, four new failure modes, each fix moving the failure rather than
removing it. The question it tries to answer has an AUTHORITATIVE answer that
does not involve parsing Nix at all: `--check` reads the config the RUNNING
nebula process was started with. Every static approximation of that was wrong in
both directions at once.

So the detector is DELETED. When the one-line anchor does not match, the script
now states plainly what it did and did not look at, and points at `--check` and
at a `grep -n ':443'` the operator can run. It does not guess, does not
over-fire, has no line-number to get wrong, and cannot fail open into a
success-shaped message because it no longer claims the entry is gone.

Net: -20 executable lines, and F1, F2, F3 and F7 have no surface left.

Also fixed:

F4 (a REGRESSION I introduced in round 3). Lifting the `--check` pattern from
the script removed the shape assertion that was the only thing able to see a
`:443$` mutant — the three retained cases all behave identically under it. The
discriminating case is a trailing-comment line, which the round-2 sweep had
NAMED but never exercised. Restored, and mutation-verified: the mutant now dies.
🔴 My first attempt to verify that was itself a false green — the `sed` mutation
silently did not match, so "SURVIVED" meant "the instrument never ran". Redone
with the mutation asserted present before the suite runs.

F6. `test_verify_is_errexit_safe` checked for the absence of the bare call but
never for the `check_rc=0` pre-init — so stripping it left the suite green,
while under `set -u` that crashes on the SUCCESS path, leaving OK=0 so the trap
rolls back a correctly-applied change. Mutation-verified.

F5/F8 are moot: the vacuous block-extractor test and the fragile extraction
harness both covered code that no longer exists.

VERIFIED: 182 passed across the five adjacent test files (not one file — that
blindness is what shipped round 3's 🔴). Both regression drives green; drive 2's
F2 case was rewritten to the new contract and asserts the script states its
scope, points at --check, and does NOT claim the entry is gone.

Refs clawgate #497, audit round 4 on PR #1361.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TcDJ4ccN8savidfXbHPoqh
Claude-Session-Id: 5e2d201d-d6e6-481d-85e9-0792db43cebf
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