fix(nebula): pin the interpreter fix tier-independently, and stop reporting an exec fault as a config fault - #1420
Conversation
…orting an exec fault as a config fault Round-1 audit of #1407 left two findings. Both are here. FINDING 1 — the fix it shipped had no regression guard, and its only coverage was on the tier that is structurally blind to a revert. MEASURED: revert #1407's `run_check` to a direct `"$CHECK"` exec and the dev-host tier still reports 30 passed, because the dev host HAS /usr/bin/env. Only `nix build .#checks.x86_64-linux.pytests` goes red. So `scripts/gate.sh --tier both` — the command most sessions actually run — went fully green on the exact defect #1407 existed to remove. An agent "simplifying" run_check back to what #1272 wrote would have seen green and shipped it. `test_the_verifier_is_never_execed_via_its_own_shebang` is a source assertion, so it fails wherever the suite runs. Pinned as a PAIR — the helper must exist AND no direct call site may reappear — because presence alone passes if someone adds a second, direct site beside the helper, and absence alone passes if the helper is deleted outright. `test_the_verifiers_exit_codes_are_a_closed_set` pins its load-bearing precondition: the {0,1,2} set is asserted on BOTH sides, so the verifier growing an `exit 3` fails rather than being silently reclassified as an exec fault. FINDING 2 — the misdiagnosing abort was unchanged. check-nebula-relays.sh only ever exits 0, 1 or 2. Every other code fell into the same `*)` arm as a genuine rc 2 and was reported as ABORT: the verifier could not read the current config (rc=126); fix that first asserting a fact about $CFG that is false in every one of those cases — and that is precisely how the /usr/bin/env fault presented, which is why it read as a config problem instead of an exec one. Split into `2)` (a real verifier refusal — unchanged message) and `*)` (it did not run). One `verifier_answered` classifier, used at both sites rather than open-coded twice. 🔴 The consequential site is the POST-REBUILD verify, not the preflight. There a non-verifier rc reaches `die` AFTER `nixos-rebuild test` has activated, so the EXIT trap rolls back a change that WORKED and prints the PERSISTED paragraphs for what was really a signal or exec fault. VERIFICATION Red -> green, and every guard watched failing: whole suite at HEAD 38 passed new guards vs pre-#1407 nix/system 4 of 6 fail (the shebang guard correctly passes — #1407 is now in main — and the rc-2 pin passes both sides by design, being a guard against WIDENING, not a regression test) Mutation sweep, PYTHONDONTWRITEBYTECODE=1, every mutation applied-verified (the count is asserted before substituting, so an unapplied edit cannot be scored as a caught mutant): CONTROL unmutated 8 passed revert $BASH -> direct exec killed by the shebang guard verifier grows `exit 3` killed by the closed-set test widen verifier_answered to 126 killed by post-rebuild[126] + closed-set drop site-B classification killed by post-rebuild[126,137] revert the preflight split killed by preflight[126,127,137] reword the die message killed by all 5 behavioural cases 🔴 The post-rebuild test exists BECAUSE of that sweep. Widening verifier_answered was first caught by the STRUCTURAL test alone: every behavioural case aborted at the preflight, so verifier_answered — used only at the post-rebuild site — never executed. A guard that is never reached is not a guard. `apply_beside_sequenced_verifier` (answer 1, then fail) is the only way to reach that site. Four mutants added to scripts/tests/mutants-nebula-relay.sh so this is re-derivable rather than believed. ⚠ The stub verifier goes through `write_exec`, NOT a hand-written shebang. A `#!/usr/bin/env bash` stub execs on the dev host and ENOENTs in the sandbox, which would make the test fabricate rc 126 from its own stub rather than from the code under test — circular, and passing for the wrong reason on one tier. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NNxEE4ktaHsXxNsxeWXWC5 Claude-Session-Id: 4b14058c-f3f5-4988-ae42-90910f7dbe43
…tring overclaiming
Two corrections found by running the repo's own battery rather than my ad-hoc
sweep, which was weaker.
1. M-FH-1 reported 🔴 WRONG-KILLER: the right test failed, but not with the
message the mutant declared. The assertion had been reworded after the
battery entry was written and the two drifted. My own sweep scored this
mutant KILLED — it only checks WHICH test failed, not that the test's own
assertion is what fired, which is exactly the hole the battery's header
describes ("a mutant can die to a DIFFERENT test's error and be scored as
covered while its own assertion is unreachable"). The phrase is now asserted
to exist in the test source before being written into the battery.
2. test_apply_declares_every_tool_it_execs' docstring claimed the relationship
its name implies while the body inspects one side: a hard-coded list ⊆
declared. It cannot catch a tool the script execs but does not declare, which
is the failure the preflight exists to prevent.
MEASURED: `head`, `id` and `rm` are exec'd and undeclared right now, and the
test is green. ($BASH is a fourth omission but a CORRECT one — the
interpreter is already running, so it needs no `command -v`.)
The body is unchanged: closing it means deriving the exec'd set from the
source, which is real work and out of scope here. The docstring now says what
the body does rather than what the name suggests, because a guard that reads
as coverage while providing none is worse than no guard — it stops anyone
looking.
Full battery, clean /tmp: pass=26 fail=0, all three controls green.
⚠ An earlier full run reported CONTROL-KILL as WRONG-KILLER. Not a defect in
this branch: it passes in isolation on origin/main AND here. A prior battery run
was killed at a 10-minute tool cap before its cleanup trap ran, leaving 29
/tmp/nebula-relay-pre.* files — the residue class the battery's own header
documents as making later runs read stale artifacts as live defects. Clearing
them made the full run green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NNxEE4ktaHsXxNsxeWXWC5
Claude-Session-Id: 4b14058c-f3f5-4988-ae42-90910f7dbe43
…once anything was written
Three 🟡 findings, all in prose this PR itself added. Fix-round prose is the
likeliest next finding, and it was.
FINDING 1 (payload) — `die_verifier_did_not_run` ended with an unconditional
"this script is idempotent and has changed nothing it cannot repeat".
It is reached from THREE sites: the preflight (nothing written), the post-`test`
verify, and the post-`switch` verify. At the last two that sentence directly
contradicts the trap paragraph printed two lines BELOW it, which says PERSISTED
/ restoring the file is NOT enough.
Following it loses the change SILENTLY. Measured end state after that failure:
$CFG rolled back, but the RUNNING unit still advertises the relay. A re-run's
preflight asks the running unit, gets rc 0, prints "ALREADY SATISFIED --
nothing to do" and exits 0 — a green all-clear over a config file that no
longer contains the change, which the next `nixos-rebuild switch` by anyone
quietly removes. Pre-PR this site did NOT invite a re-run; the invitation was
new, and it was mine.
Now conditional on `${PATCHED:-0}`. That spelling and not `$PATCHED`: the trap
flags are declared AFTER the preflight, so a bare reference aborts under
`set -u` at the first call site.
Both branches are pinned — an untested branch is what these rounds keep
finding. Mutation-checked: forcing the conditional to the pre-fix branch fails
test_a_POST_REBUILD_...[126] and [137] (M-FH-6).
FINDING 2 (scaffolding) — the paired guard's negative half was a source regex,
and it pinned a SPELLING.
MEASURED by the audit: `"${CHECK}" "$RELAY"`, `$CHECK "$RELAY"`, a line-split
call and others all evade it while reintroducing the exact /usr/bin/env
dependency — leaving the dev-host suite at 38 passed.
Replaced by `test_the_verifier_runs_with_its_shebang_BROKEN`: it runs the whole
happy path against a verifier whose shebang points at a nonexistent interpreter
AND whose exec bit is cleared. Invoked through `$BASH` the file is merely READ,
so none of that matters; ANY direct exec, however spelled, dies. It is
spelling-independent AND tier-independent — it manufactures the
missing-interpreter condition instead of waiting for a sandbox that happens to
lack /usr/bin/env.
Verified against the exact evasion: the braces mutant is MISSED by the regex and
KILLED by the behavioural guard (M-FH-5). The structural test is kept as the
cheap half, with a docstring that now says it is spelling-bound rather than
claiming the pair is sufficient.
FINDING 3 (payload) — the 126 line documented the PRE-FIX failure mode.
"found, but not executable -- or ITS INTERPRETER is missing" is unreachable
under `"$BASH" "$CHECK"`: bash READS the file, so the exec bit is never
consulted (measured: `bash <readable, mode 644, exits 7>` -> 7), and the
interpreter is by definition already running. What actually yields 126 now is
$CHECK being a directory or unreadable — neither mentioned. A reader would
`chmod +x` and re-run to identical failure. Corrected, along with the
parametrize label that repeated it.
ALSO — a false claim in this PR's own comment. It said the post-rebuild site
prints the PERSISTED / "profile may have moved" paragraphs. It does not:
PERSISTED and SWITCH_ATTEMPTED are both 0 there, so it prints ACTIVATED, NOT
PERSISTED — which test_verifier_failure_after_a_good_test_says_activated_not_
persisted already pins. Those paragraphs belong to the post-switch site.
VERIFICATION
suite 39 passed
battery (clean /tmp) pass=27 fail=0, three controls green
M-FH-5 braces evasion killed by the behavioural guard (regex misses it)
M-FH-6 advice made killed by post-rebuild[126] and [137]
unconditional
⚠ NOT FIXED, recorded as open: no test reaches the post-SWITCH verify (site 3)
— `apply_beside_sequenced_verifier` is two-stage by construction. Finding 1's
fix is exercised at site 2, where PATCHED=1 selects the same branch, so the
conditional is covered; the SITE is not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NNxEE4ktaHsXxNsxeWXWC5
Claude-Session-Id: 4b14058c-f3f5-4988-ae42-90910f7dbe43
12df41d to
acc988e
Compare
… allowlist
The sandbox tier went red on `test_no_test_writes_a_usr_bin_env_shebang_at_
runtime`, flagging two lines this branch added:
test_nebula_relay_apply.py:1022 assert original.startswith("#!"), ...
test_nebula_relay_apply.py:1023 chk.write_text("#!/nonexistent/interpreter\n" ...)
The guard is RIGHT to flag them — it scans for a test writing its own shebang,
and line 1023 does exactly that. It is also the correct call to allowlist rather
than reword, because this is a genuine exception the list did not yet describe.
The existing entries cover two shapes: (a) an absolute, sandbox-present
interpreter, and (b) a shebang ASSERTED rather than written. This is a third:
an INTENTIONALLY unresolvable interpreter, written onto a COPY of the verifier,
with the test then asserting the run SUCCEEDS anyway — proving
apply-nebula-relay.sh reads it through "$BASH" instead of exec'ing it.
That is the guard's own hazard inverted into a control. The scan exists because
an unresolvable interpreter breaks a stub SILENTLY; here it is the independent
variable, and if it ever started mattering the test fails loudly. Nothing execs
the stub, so an unresolvable path cannot hide anything.
Both entries name a path fragment rather than a shebang literal, so this file's
own test_this_guards_source_does_not_match_itself stays green — the same
constraint the existing entries are written under.
Accounting is two-way (an entry matching no offender FAILS), so these pins
cannot become rubber stamps: 9 passed with both matching.
⚠ Worth stating plainly: this guard caught a defect class in MY test on its
first sandbox run, in a branch whose subject is that same class. The dev-host
tier did not flag it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NNxEE4ktaHsXxNsxeWXWC5
Claude-Session-Id: 4b14058c-f3f5-4988-ae42-90910f7dbe43
… battery that read stale bytecode
ROUND 2 confirmed all three round-1 findings genuinely fixed (re-derived
independently). It returned one SHOULD-FIX and four NITs. All addressed here.
SHOULD-FIX (scaffolding) — a docstring my OWN fix falsified.
test_the_verifier_is_never_execed_via_its_own_shebang still said "with the fix
reverted every behavioural test in this file still passes -- measured: 30
passed. Only the sandbox tier goes red." Adding the behavioural guard made the
dev-host tier catch that revert too. RE-MEASURED at this tree with
`run_check() { "$CHECK" "$@"; }`: 2 failed, 37 passed on the dev host.
That sentence was the entire stated justification for keeping the structural
half — and it was written in the commit whose subject was "stop a guard's
docstring overclaiming". It now records what the structural half actually earns
(fails fast, names the intended shape) instead of reading like the load-bearing
guard, which it is not.
NIT, but a real payload hole — `${PATCHED:-0}` read an INHERITED env var.
The trap flags were declared below the preflight, so the only way to read
PATCHED early was the `:-` fallback — which silently accepts a value from the
caller's environment. This script's header documents `sudo env "PATH=$PATH"
bash ...`, which preserves it. Reproduced: an exported PATCHED=1 made the
preflight print "DO NOT simply re-run" when nothing had been written.
Flags moved above their first read; the fallback is gone. Regression test added
and watched RED on the pre-fix shape.
NIT — the trap-branch comment claimed "PERSISTED / the profile may have moved"
at the post-switch site. Those are mutually exclusive branches and the second is
unreachable there: `set -e` plus PERSISTED=1 immediately after a successful
switch means PERSISTED is always 1 by that verify. Corrected.
NIT — M-FH-5 had been inserted between M-FH-2's explanatory comment and M-FH-2
itself, so the comment read as the wrong mutant's preamble. Re-seated.
NIT — acc988e's message quoted `pass=27` for a tree that prints 28. The 27 came
from a run predating M-FH-6, i.e. the sweep predated the mutant guarding that
commit's own change. Recorded rather than silently corrected.
TWO HARNESS DEFECTS FOUND AND FIXED
1. The battery READ a stale bytecode cache. Its header claims
PYTHONDONTWRITEBYTECODE=1 "throughout", which stops it WRITING one but not
reading one an ordinary pytest run left behind. MEASURED: an edit adding
explicit assertion messages was invisible for two consecutive runs, both
reporting WRONG-KILLER while the phrase was demonstrably in the output of the
same command run by hand. `run_tests` now purges __pycache__ first; verified
by deliberately recreating the stale cache.
2. Several assertions passed `combined` as their own message, so under a mutant
the expected phrase appeared nowhere in the output and the battery scored a
correct kill as WRONG-KILLER. Every mutant's expected phrase was then
cross-checked against its killer's body.
🔴 NOT CERTIFIED, AND SAY SO: I have not obtained a single full-green battery
run. Three consecutive full runs failed a DIFFERENT pair each time (M-FA-3+
M-FH-4; M-FH-4+M-FH-6; M-FI-2+M-FH-1), always exactly two, while every mutant
passes repeatedly in isolation. One reported M-FH-1 as SURVIVED — a FALSE
SURVIVED, the dangerous direction — for a mutation measured by hand as failing
two tests. Box load was 64-90 throughout, from other sessions. The per-mutant
results are the trustworthy evidence; the full-run aggregate on a loaded box is
not, and two earlier `fail=0` runs should be read as luck rather than proof.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NNxEE4ktaHsXxNsxeWXWC5
Claude-Session-Id: 4b14058c-f3f5-4988-ae42-90910f7dbe43
Round 2 complete — and one thing NOT certifiedRound 2 confirmed all three round-1 findings genuinely fixed (the auditor re-derived each independently rather than trusting the diff). It returned one SHOULD-FIX and four NITs; all are addressed in The SHOULD-FIX was a docstring my own fix falsified. One NIT was a real payload hole: Two harness defects found and fixed along the way
🔴 Not certified — read before mergingI have not obtained a single full-green battery run. Three consecutive full runs failed a different pair each time, always exactly two:
Every one of those passes repeatedly in isolation, including Box load was 64–90 throughout, from other sessions. Conclusion: the per-mutant results are trustworthy; the full-run aggregate on a loaded box is not. Two earlier The gate itself is unaffected: both sandbox tiers were green at Recorded as open, not fixed
|
…), its headline guard proven re
…broken script, and a battery that cannot be certified (#1434) * docs(handoff): NEW doc, genuinely new effort: no existing handoff references #1407 or #1420, and the Claude-Session-Id: 4b14058c-f3f5-4988-ae42-90910f7dbe43 * docs(handoff): PR #1420 was gated on the merged tree (both tiers PASS), its headline guard proven re * docs(handoff): the re-gate of post-merge origin/main @ b79ccfb came back PASS on BOTH tiers, closin * fix(handoff): redact the client subdomain literal — #1434 was unmergeable from its first commit MEASURED on the merged tree (origin/main 8a9ebba + this branch), running the doc-reading gates directly: FAILED scripts/tests/test_no_client_hostnames.py::test_no_client_subdomain_literal_is_committed 1 failed, 692 passed in 278.03s Four occurrences of a real client subdomain in a doc tracked by a PUBLIC repo. TWO OF THEM (lines 136, 142) came from this branch's FIRST commit c578351, so this PR has been red since it was opened and nobody had gated it — a docs-only change reads as gate-exempt and is not: test_doc_path_rot, test_no_client_hostnames, test_no_public_ips, test_no_captured_text and test_no_captured_markup all read tracked files. Redacted in place rather than allowlisted. Allowlisting would disarm a gate over a genuine leak — the finding itself (rank 2: the literal is still reachable in git history at 6d488a1, and all four content gates are blind to history) is unchanged and still open. The meaning of each sentence is preserved; only the literal is gone. Not fixed by handoff_doc.py because its Gotchas/Open-investigations sections APPEND: a merge cannot remove text from them, and three of the four occurrences were in already-committed prose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Jq8nmf8ZHZkJ56enFiaM1 Claude-Session-Id: 9d71983b-623b-4cfb-b261-7263073b277e --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes the two 🟡 findings from #1407's round-1 audit.
Finding 1 — the fix shipped without a regression guard, and its only coverage was on the tier that is blind to a revert
MEASURED: revert #1407's
run_checkto a direct"$CHECK"exec and the dev-host tier still reports 30 passed — the dev host has/usr/bin/env. Onlynix build .#checks.x86_64-linux.pytestsgoes red. Soscripts/gate.sh --tier both, the command most sessions actually run, went fully green on the exact defect #1407 existed to remove. An agent "simplifying"run_checkback to what #1272 wrote would have seen green and shipped it.Two source-level guards, which fail wherever the suite runs:
test_the_verifier_is_never_execed_via_its_own_shebang— pinned as a pair: the helper must exist and no direct call site may reappear. Presence alone passes if someone adds a second, direct site beside the helper; absence alone passes if the helper is deleted outright.test_the_verifiers_exit_codes_are_a_closed_set— pins finding 2's load-bearing precondition on both sides, so the verifier growing anexit 3fails loudly instead of being silently reclassified as an exec fault.Finding 2 — the misdiagnosing abort was unchanged
check-nebula-relays.shonly ever exits 0, 1 or 2. Every other code fell into the same*)arm as a genuine rc 2:— asserting a fact about
$CFGthat is false in every one of those cases, and precisely how the/usr/bin/envfault presented, which is why it read as a config problem rather than an exec one.Split into
2)(a real refusal — message unchanged) and*)(it did not run), via oneverifier_answeredclassifier used at both sites rather than open-coded twice.🔴 The consequential site is the post-rebuild verify, not the preflight. There a non-verifier rc reaches
dieafternixos-rebuild testhas activated, so the EXIT trap rolls back a change that worked and prints the PERSISTED paragraphs for what was really a signal or exec fault.Verification
Both tiers, merged tree (branch is off
origin/main@0477e52a, fast-forward):nix build …#checks…pytestscollected=21135 passed=21133 failed=0nix build …#checks…nodeteststests=1449 pass=1449 fail=0collectedrose 21127 → 21135, exactly +8 = the test cases added. Nothing silently skipped.Mutation battery (
scripts/tests/mutants-nebula-relay.sh), clean/tmp— pass=26 fail=0, all three controls green, four new mutants each killed by the named test with its own message:M-FH-1revert$BASH→ direct execM-FH-2verifier growsexit 3M-FH-3widenverifier_answeredto accept 126M-FH-4drop site-B classificationTwo things the work itself caught, recorded because they generalise
🔴 The repo's battery caught a defect my own ad-hoc sweep scored as PASSING. M-FH-1 died to the right test but not to the right assertion — I had reworded the message after writing the mutant entry. My sweep only checked which test failed; the battery also checks the test's own assertion fired. That is the exact "green for the wrong reason" hole its header documents, hit while writing a guard against a different instance of the same class. The phrase is now asserted present in the test source before being written into the battery.
🔴 The post-rebuild test exists because the sweep found a hole in my tests. Widening
verifier_answeredwas first caught by the structural test alone: every behavioural case aborted at the preflight, soverifier_answered— used only at the post-rebuild site — never executed. A guard that is never reached is not a guard.apply_beside_sequenced_verifier(answer 1, then fail) is the only way to reach that site.⚠ The stub verifier goes through
write_exec, not a hand-written shebang. A#!/usr/bin/env bashstub execs on the dev host and ENOENTs in the sandbox — the test would have fabricated rc 126 from its own stub rather than from the code under test: circular, and green for the wrong reason on one tier.Also corrected — audit finding 5, docstring only
test_apply_declares_every_tool_it_execsclaimed the relationship its name implies while its body inspects one side (a hard-coded list ⊆ declared). MEASURED:head,idandrmare exec'd and undeclared right now, and it is green. ($BASHis a fourth omission but a correct one — the interpreter is already running.)Body unchanged. Closing it means deriving the exec'd set from the source, which is real work and out of scope here. The docstring now says what the body does rather than what the name suggests, because a guard that reads as coverage while providing none is worse than none — it stops anyone looking. Recorded as open, not fixed.
Not a finding, recorded for the next reader
An earlier full battery run reported
CONTROL-KILLas WRONG-KILLER. Not a defect in this branch — it passes in isolation onorigin/mainand here. A prior run was killed at a 10-minute tool cap before its cleanup trap fired, leaving 29/tmp/nebula-relay-pre.*files, the residue class the battery's own header documents as making later runs read stale artifacts as live defects. Clearing them made the full run green.