diff --git a/docs/superpowers/contracts/gate-reads-jail-args-2026-08-22.json b/docs/superpowers/contracts/gate-reads-jail-args-2026-08-22.json new file mode 100644 index 00000000..9db60919 --- /dev/null +++ b/docs/superpowers/contracts/gate-reads-jail-args-2026-08-22.json @@ -0,0 +1,22 @@ +{ + "title": "Make the coverage gate read jail_args and refuse a suite that dies partway", + "objective": "Close the two review-found defects that let checks pass more easily than they should: is_covered() ignored the jail_args marker every other consumer reads, and bash's command hash cache made two 'tool is missing' assertions test the opposite of their names.", + "acceptance_criteria": [ + "is_covered() reads the jail_args file beside a suite's run_all.sh, the same marker ci_mirror.sh and shell-tests.yml consume, so a suite declares its own mounts in exactly one place.", + "A suite with no jail_args still gets the historical default binds, so nothing that passed before starts failing for lack of a marker.", + "A jail_args that is present but empty is a hard error, never a silent skip.", + "is_covered() passes --fail-on-case-error, so a suite that aborts partway fails the gate rather than being graded on the lines it reached before dying.", + "NEGATIVE CONTROL: stripping the binds a suite needs must make the gate exit non-zero. Before this change both the working and the broken configuration exited 0.", + "dot_resolver_install.sh, which is already off the allowlist, still clears the real gate afterwards -- the campaign's proof that 100% is reachable must survive the stricter check.", + "Every 'tool is missing' assertion genuinely hides the tool: removal clears bash's hash table via a shared _t_unstub helper.", + "No measured coverage figure moves as a result of either fix, and every existing assertion still passes.", + "shellcheck clean with no suppressions; all files within the 250-line cap." + ], + "out_of_scope": [ + "test_cco_state.sh's removal of beeper.orig: get_real_binary probes with [[ -f ]], which the hash cache does not affect.", + "test_aw_autostart.sh's removal of aw-qt: the assertion runs in a subshell via $status_script, so the parent shell's cache is irrelevant.", + "Arming check_shell_coverage.sh in any hook; that waits for allowlist-zero.", + "The kcov under-report, which is tracked separately in docs/shell-split-verification.md." + ], + "verifier": "bash meta/scripts/check_shell_coverage.sh on dot_resolver_install.sh must exit 0; replacing that directory's jail_args with `--bind /etc` alone must make the same command exit non-zero, and restoring it must return exit 0 with an empty git diff --stat. Plus the features/lib runner under --fail-on-case-error reporting zero case failures, and all four features libs re-measured at their pre-change figures." +} diff --git a/docs/superpowers/evidence/gate-reads-jail-args-2026-08-22.json b/docs/superpowers/evidence/gate-reads-jail-args-2026-08-22.json new file mode 100644 index 00000000..a09c101e --- /dev/null +++ b/docs/superpowers/evidence/gate-reads-jail-args-2026-08-22.json @@ -0,0 +1,50 @@ +{ + "intent": "Close two defects found in review that would each have made the campaign's guarantees hollow: is_covered() ignored the jail_args marker (so 44 features/lib libs could never reach 100% through the gate), and two suites unstubbed a tool with a bare `rm`, which bash's command hash cache makes a no-op.", + "scope": [ + "meta/scripts/lib/shell_coverage_bar.sh: is_covered() reads jail_args and passes --fail-on-case-error", + "linux_configuration/scripts/single_use/features/lib/tests/features_harness.sh: new _t_unstub helper", + ".../tests/test_dwm_config.sh and test_rpi_nc_ca.sh: switched to _t_unstub", + "Non-goal: test_cco_state.sh's `rm` of beeper.orig -- get_real_binary probes with `[[ -f ]]`, which the hash cache does not affect, so that test is sound" + ], + "changes": [ + "GATE DEFECT: is_covered() hardcoded `--bind /etc --bind /usr/local/bin --bind /var/lib` and never read the jail_args file that ci_mirror.sh and shell-tests.yml both consume. The features/lib suite needs /usr/local/share and /etc/pki and ABORTS under set -e without them -- yet the coverage number still printed, because kcov had already recorded whatever the dead run reached. The gate would have graded a lib on a suite that never finished.", + "is_covered() now reads the marker, falls back to the historical defaults when a suite has none, fails closed on a present-but-empty marker, and passes --fail-on-case-error so a suite that dies partway fails the gate instead of being scored on its stump.", + "HASH-CACHE DEFECT: the _t_unstub helper written for the transcribe harness was needed in features_harness.sh too. Two assertions were vacuous -- test_dwm_config.sh's 'verify still reports when dwm is absent' and test_rpi_nc_ca.sh's uname removal after the macOS branch, where a cached uname would have left every later branch running under a faked Darwin.", + "This was found by review rather than by a failing test, which is the point: both defects made checks PASS more easily, so nothing went red." + ], + "verification": [ + { + "command": "bash meta/scripts/check_shell_coverage.sh .../dot_resolver_install.sh", + "result": "pass", + "evidence": "exit 0 -- the lib that is already OFF the allowlist still clears the real gate now that the gate reads the marker and enforces case failure." + }, + { + "command": "NEGATIVE CONTROL: replace jail_args with `--bind /etc` alone, re-run the gate, restore", + "result": "pass", + "evidence": "gate exit=1 with the binds stripped, exit=0 after restoring. Before this commit BOTH cases exited 0 -- the gate could not fail on an aborting suite. git diff --stat on jail_args is empty afterwards." + }, + { + "command": "re-measure all four features libs after the _t_unstub fix", + "result": "pass", + "evidence": "dot_resolver_install.sh 39/39 = 100.00% (unchanged -- its off-allowlist clearance is sound with real absence enforced), dwm_config.sh 47.95%, rpi_nc_ca.sh 98.63%, aw_autostart.sh 82.93%. No figure moved." + }, + { + "command": "the features/lib runner under --fail-on-case-error after the change", + "result": "pass", + "evidence": "0 case failures -- every assertion still passes with the tools genuinely absent rather than cached." + }, + { + "command": "grep -rn 'rm -f \"$TEST_TMPDIR/bin/' --include='test_*.sh' linux_configuration/", + "result": "pass", + "evidence": "Four hits audited. Two fixed; test_aw_autostart.sh's runs in a subshell via $status_script so the parent's cache is irrelevant; test_cco_state.sh probes with [[ -f ]], not command -v." + } + ], + "risks": [ + "is_covered() now runs the full jailed suite per lib with --fail-on-case-error, so a directory whose suite is slow makes the gate slower per file. Hook mode was measured at 0.42s/file before; a jailed suite is seconds, which is why an --all sweep belongs in CI rather than a commit hook.", + "A suite that legitimately has no jail_args gets the historical default binds. If such a suite later needs a mount it will abort and now correctly fail the gate -- loudly, which is the intended direction, but it is a new way for the gate to go red." + ], + "rollback": [ + "git revert the commit; is_covered() returns to hardcoded binds with no case-failure check, and the two suites return to vacuous absence assertions.", + "After rollback, re-run the negative control: it should exit 0 with the binds stripped, confirming the fail-open behaviour is back." + ] +} diff --git a/linux_configuration/scripts/single_use/features/lib/tests/features_harness.sh b/linux_configuration/scripts/single_use/features/lib/tests/features_harness.sh index a15439c3..78c7ed80 100755 --- a/linux_configuration/scripts/single_use/features/lib/tests/features_harness.sh +++ b/linux_configuration/scripts/single_use/features/lib/tests/features_harness.sh @@ -86,6 +86,16 @@ _t_stub() { chmod +x "$TEST_TMPDIR/bin/$1" } +# Remove a stub AND forget it. bash caches executable locations in a hash +# table, so `command -v foo` keeps succeeding after foo is deleted until the +# table is cleared -- silently turning every "tool is missing" test into a +# "tool is present" test that asserts the opposite of its own name. Measured: +# without the `hash -r` the removed stub is STILL FOUND. +_t_unstub() { + rm -f "$TEST_TMPDIR/bin/$1" + hash -r +} + _t_called() { if [[ -f "$TEST_TMPDIR/calls.log" ]] && grep -q "$1" "$TEST_TMPDIR/calls.log"; then _t_pass "$2" diff --git a/linux_configuration/scripts/single_use/features/lib/tests/test_dwm_config.sh b/linux_configuration/scripts/single_use/features/lib/tests/test_dwm_config.sh index cc823175..09b66d19 100755 --- a/linux_configuration/scripts/single_use/features/lib/tests/test_dwm_config.sh +++ b/linux_configuration/scripts/single_use/features/lib/tests/test_dwm_config.sh @@ -176,7 +176,7 @@ _t_has "$verify_out" 'dwm version:' "verify reports the dwm version" _t_has "$verify_out" 'xsession registered' "verify confirms the xsession is registered" # --- verify: tolerates a missing dwm binary --------------------------------- -rm -f "$TEST_TMPDIR/bin/dwm" +_t_unstub dwm missing_out="$(verify 2>&1)" || true _t_has "$missing_out" 'dwm version:' "verify still reports when dwm is absent (dwm -v exits 1)" diff --git a/linux_configuration/scripts/single_use/features/lib/tests/test_rpi_nc_ca.sh b/linux_configuration/scripts/single_use/features/lib/tests/test_rpi_nc_ca.sh index 2c6e2b03..cdfc3ce6 100755 --- a/linux_configuration/scripts/single_use/features/lib/tests/test_rpi_nc_ca.sh +++ b/linux_configuration/scripts/single_use/features/lib/tests/test_rpi_nc_ca.sh @@ -141,7 +141,7 @@ chmod +x "$TEST_TMPDIR/bin/uname" mac_out="$(phase_install_ca 2>&1)" _t_has "$mac_out" 'Detected macOS' "the macOS branch is selected when uname reports Darwin" _t_has "$mac_out" 'system keychain' "the macOS branch reports the keychain install" -rm -f "$TEST_TMPDIR/bin/uname" +_t_unstub uname # --- phase_install_ca: certutil reports the CA is ALREADY installed --------- # `certutil -L` emitting a line containing "Nextcloud" takes the else branch. diff --git a/meta/scripts/lib/shell_coverage_bar.sh b/meta/scripts/lib/shell_coverage_bar.sh index fea33d66..7f8c1fea 100755 --- a/meta/scripts/lib/shell_coverage_bar.sh +++ b/meta/scripts/lib/shell_coverage_bar.sh @@ -35,10 +35,38 @@ is_covered() { command -v kcov >/dev/null 2>&1 || return 0 command -v unshare >/dev/null 2>&1 || return 0 + # A suite declares the mounts and seeds it needs in a `jail_args` file + # beside its run_all.sh -- the same marker ci_mirror.sh and + # shell-tests.yml read. Hardcoding flags here was a latent gate failure: + # the features/lib suite needs /usr/local/share and /etc/pki, and without + # them it ABORTS partway under `set -e`. The coverage number still printed + # (kcov had already recorded the lines the dead run reached), so the gate + # would have passed a lib whose suite never finished. + local jail_args=() + local args_file + args_file="$(dirname "$suite")/jail_args" + if [[ -f "$args_file" ]]; then + local arg + while IFS= read -r arg; do + jail_args+=("$arg") + done < <(grep -v '^#\|^$' "$args_file" || true) + if [[ ${#jail_args[@]} -eq 0 ]]; then + echo "Error: jail_args is present but empty: $args_file" >&2 + return 1 + fi + else + # No marker: the suite runs without needing anything mounted, so give + # it the historical defaults rather than nothing. + jail_args=(--bind /etc --bind /usr/local/bin --bind /var/lib) + fi + + # --fail-on-case-error: a suite that dies partway must fail the gate, not + # be graded on however many lines it managed to reach first. "$REPO_ROOT/meta/scripts/shell_coverage_jail.sh" \ --subject "$suite" \ - --bind /etc --bind /usr/local/bin --bind /var/lib \ + "${jail_args[@]}" \ --measure "$(basename "$path")" \ --min "$COVERAGE_BAR" \ + --fail-on-case-error \ -- "" >/dev/null 2>&1 }