diff --git a/Makefile b/Makefile index 794772f..bf1669f 100644 --- a/Makefile +++ b/Makefile @@ -25,6 +25,7 @@ UNIT_BATS := test/init-detection.bats test/verify-init-quoting.bats test/ver test/verify-bump-knobs-unit.bats test/verify-replay-unit.bats \ test/verify-podman-userns-unit.bats test/verify-build-hash-unit.bats \ test/verify-policy-ceilings-unit.bats test/verify-home-guard-unit.bats \ + test/verify-learn-collapse-unit.bats \ test/verify-drift-render-unit.bats test/verify-awk-data-unit.bats ACCEPT_BATS := test/acceptance.bats test/acceptance-bump.bats test/verify-run-default.bats SECURITY_BATS := $(wildcard test/verify-security-*.bats) diff --git a/bin/sluice b/bin/sluice index f2b9ff2..bb0e30f 100755 --- a/bin/sluice +++ b/bin/sluice @@ -4194,7 +4194,10 @@ _learn_review() { local parents p subs ya; parents="$(printf '%s\n' "$hosts" | while read -r h; do [ -n "$h" ] && parent_of "$h"; done | sort | uniq -c | awk '$1>=2{print $2}')" for p in $parents; do _collapsible "$p" || continue # never offer a wildcard equal to a public suffix (foo.github.io -> .github.io) - subs="$(printf '%s\n' "$hosts" | while read -r h; do [ "$(parent_of "$h")" = "$p" ] && echo "$h"; done)" + # `if`, not `&&`: a failed test on the LAST line leaves the while loop - and so the whole command + # substitution - at status 1, which this plain assignment propagates into errexit. That killed the + # review mid-screen, with no error text, whenever the last blocked host wasn't a child of $p. + subs="$(printf '%s\n' "$hosts" | while read -r h; do if [ "$(parent_of "$h")" = "$p" ]; then echo "$h"; fi; done)" # shellcheck disable=SC2086 printf ' %s subdomains of %s: %s\n' "$(printf '%s\n' "$subs" | awk 'END{print NR}')" "$p" "$(printf '%s ' $subs)" printf ' collapse to .%s (matches all its subdomains)? [y/N] ' "$p" diff --git a/src/80-learn.sh b/src/80-learn.sh index 74fc132..ff3d2f0 100644 --- a/src/80-learn.sh +++ b/src/80-learn.sh @@ -271,7 +271,10 @@ _learn_review() { local parents p subs ya; parents="$(printf '%s\n' "$hosts" | while read -r h; do [ -n "$h" ] && parent_of "$h"; done | sort | uniq -c | awk '$1>=2{print $2}')" for p in $parents; do _collapsible "$p" || continue # never offer a wildcard equal to a public suffix (foo.github.io -> .github.io) - subs="$(printf '%s\n' "$hosts" | while read -r h; do [ "$(parent_of "$h")" = "$p" ] && echo "$h"; done)" + # `if`, not `&&`: a failed test on the LAST line leaves the while loop - and so the whole command + # substitution - at status 1, which this plain assignment propagates into errexit. That killed the + # review mid-screen, with no error text, whenever the last blocked host wasn't a child of $p. + subs="$(printf '%s\n' "$hosts" | while read -r h; do if [ "$(parent_of "$h")" = "$p" ]; then echo "$h"; fi; done)" # shellcheck disable=SC2086 printf ' %s subdomains of %s: %s\n' "$(printf '%s\n' "$subs" | awk 'END{print NR}')" "$p" "$(printf '%s ' $subs)" printf ' collapse to .%s (matches all its subdomains)? [y/N] ' "$p" diff --git a/test/verify-learn-collapse-unit.bats b/test/verify-learn-collapse-unit.bats new file mode 100644 index 0000000..6fe167d --- /dev/null +++ b/test/verify-learn-collapse-unit.bats @@ -0,0 +1,92 @@ +#!/usr/bin/env bats +# _learn_review's wildcard-collapse offer (unit; no engine). The offer loop builds `subs` from a command +# substitution whose last pipeline stage is a `while` loop. When the final iteration's test fails, the +# `&&` short-circuits, the loop exits 1, pipefail carries it out of the substitution, and the plain +# assignment propagates it - so `set -e` kills `sluice learn` mid-screen with NO error text: the user +# sees the header and a blank line, exit 1. Fires whenever a collapse is offered and the last blocked +# host is not a child of that parent. +# +# Harness notes, all three load-bearing: +# 1. The function is sed-extracted from the BUILT bin/sluice (the verify-hostbudget-unit pattern), so +# the loop under test is the real shipped text, not a copy retyped here. +# 2. Two mechanical rewrites, neither touching the loop: the interactive `read` is pointed at stdin +# instead of /dev/tty, and the "both streams non-tty" early return is forced off. Without the +# second, bats' piped stdout takes that return and the loop never executes - a vacuous pass. +# 3. The function is called BARE, exactly as src/80-learn.sh:342 calls it. Calling it as +# `_learn_review ... || echo rc=$?` disarms errexit for the whole function body, and every +# assertion here would pass against the unfixed code. +load test_helper/common + +setup() { + SRC="$(cd "$BATS_TEST_DIRNAME/.." && pwd)/src" + SLUICE_BIN="${SRC%/src}/bin/sluice" + EXTRACT="$BATS_TEST_TMPDIR/learn_review.sh" + sed -n '/^_learn_review()/,/^}/p' "$SLUICE_BIN" \ + | sed -e 's# "$EXTRACT" + # Guard the extraction itself: a sed that silently matched nothing would make every test below + # pass for the wrong reason. + grep -q 'subs="\$(' "$EXTRACT" + [ "$(grep -c '/dev/tty' "$EXTRACT")" = 0 ] +} + +# $1 = rows ("\t\t"), $2 = answers fed to the prompts. +_review() { + local t="$BATS_TEST_TMPDIR/drive.sh" + { + echo 'set -euo pipefail' + echo ". '$SRC/00-prelude.sh'" + echo ". '$SRC/10-egress-helpers.sh'" + echo ". '$EXTRACT'" + echo 'learn_apply() { echo "APPLY-REACHED"; return 0; }' + echo 'reload_allowlist() { return 0; }' + printf '_learn_review "%s" "blocked"\n' "$1" # bare - see harness note 3 + echo 'echo "SURVIVED"' + } > "$t" + printf '%s\n' "$2" | bash "$t" 2>&1 +} + +# bytes-desc, the order cmd_learn feeds in; the trailing row is NOT a child of example.com. +_rows_trailing_stranger() { + local TAB; TAB="$(printf '\t')" + printf 'a.example.com%s1%s300\nb.example.com%s1%s200\nz.other.test%s1%s100' "$TAB" "$TAB" "$TAB" "$TAB" "$TAB" "$TAB" +} + +# POSITIVE CONTROL. Proves the harness reaches the collapse offer at all. Without this, the regression +# below could pass because the function early-returned rather than because the bug is fixed. +@test "learn-collapse: the harness reaches the collapse offer (positive control)" { + run _review "$(_rows_trailing_stranger)" "$(printf 'n\ns\ns\ns')" + assert_output --partial "collapse to .example.com" +} + +# THE REGRESSION. Unfixed, this prints the header and dies: exit 1, no prompt, no error text. +@test "learn-collapse: a trailing non-child host does not abort the review" { + run _review "$(_rows_trailing_stranger)" "$(printf 'n\ns\ns\ns')" + assert_success + assert_output --partial "collapse to .example.com" + assert_output --partial "SURVIVED" +} + +# The mirror case, which always worked - pinned so a fix cannot regress it the other way. +@test "learn-collapse: a trailing child host still offers the collapse" { + local TAB; TAB="$(printf '\t')" + run _review "$(printf 'z.other.test%s1%s300\na.example.com%s1%s200\nb.example.com%s1%s100' "$TAB" "$TAB" "$TAB" "$TAB" "$TAB" "$TAB")" "$(printf 's\nn\ns\ns')" + assert_success + assert_output --partial "collapse to .example.com" + assert_output --partial "SURVIVED" +} + +# subs feeds both the offer's count and the `handled` set - a truncated list would under-report which +# hosts a collapse covers, and silently leave one blocked after the user accepted. +@test "learn-collapse: the offer counts both subdomains, not one" { + run _review "$(_rows_trailing_stranger)" "$(printf 'n\ns\ns\ns')" + assert_success + assert_output --partial "2 subdomains of example.com" +} + +@test "learn-collapse: accepting the collapse consumes both subdomains" { + run _review "$(_rows_trailing_stranger)" "$(printf 'y\ns')" + assert_success + assert_output --partial "collapse to .example.com" + assert_output --partial "SURVIVED" +}