test: adopt upstream bats-assert and cover the apache hardening - #51
Merged
zebby76 merged 2 commits intoSep 3, 2026
Merged
Conversation
test/lib held 857 lines copied from bats-assert at some point, with no version and no provenance, and the copy had drifted: it parses the short options only. An unrecognised long option falls through to the argument branch, so refute_output --partial "<address>" asserted that the output does not contain the literal string "--partial", which is true of everything. Written against an apache image that does emit the signature, it passed. Nothing else in the suites used a long form, so this was the only assertion affected, but the shape of the failure is the point: a silently tautological assertion is worse than an absent one. The libraries now come from upstream at pinned tags -- bats-support 0.3.0, bats-assert 2.1.0 -- resolved through BATS_LIB_PATH. `make -C test deps` clones them into test/lib, which is untracked; the CI job installs the same tags through bats-core/bats-action and hands the path to the suites, since the action exposes it as a step output rather than exporting it. The upstream API differs from the copy, so all 60 call sites move: assert_output -l "X" assert_line "X" assert_output -l -r "X" assert_line --regexp "X" assert_output -l 0 "X" assert_line -n 0 "X" refute_output -l "X" refute_line "X" refute_output -p "X" refute_output --partial "X" test/helpers/containers.bash was the easy one to miss -- it asserts too, and `assert_output -l "healthy"` under the upstream helper compares the output to the string "-l". The image suites caught it immediately, which is the argument for the change in one line. The deps target compares the checked-out tag rather than the directory, so it is idempotent when current and re-fetches after a version bump here. It uses FORCE rather than .PHONY: make skips the implicit rule search for a phony target, so a pattern rule declared phony never fires. Verified on images built from this branch: nginx 17/17, apache 17/17, cli 4/4, and the demo suites still parse (19, 12, 6 and 1 tests).
… variants The dotfile assertions were guarded to the nginx variant because apache had not been hardened yet. It has now, and both images answer 404 for the same requests, so the guard goes and the three assertions cover the pair. Three more cover what was specific to apache: a browsable document root, TRACE, and the server signature in the error page footer. Pointed at the published 8.5.9 apache image, which carries none of the fixes: 12 dotfiles are not served not ok 13 file inside a dot directory is not served not ok 14 /.well-known keeps its normal handling ok 15 the document root is not browsable not ok 16 TRACE is refused not ok 17 error pages carry no server signature not ok Five failures against the published image, seventeen passes against one built from this branch, on both variants -- apache skipping the three that describe the nginx monitoring port, which it does not have.
zebby76
added a commit
that referenced
this pull request
Sep 3, 2026
…#54) * test: replace the vendored assertion helpers with upstream bats-assert test/lib held 857 lines copied from bats-assert at some point, with no version and no provenance, and the copy had drifted: it parses the short options only. An unrecognised long option falls through to the argument branch, so refute_output --partial "<address>" asserted that the output does not contain the literal string "--partial", which is true of everything. Written against an apache image that does emit the signature, it passed. Nothing else in the suites used a long form, so this was the only assertion affected, but the shape of the failure is the point: a silently tautological assertion is worse than an absent one. The libraries now come from upstream at pinned tags -- bats-support 0.3.0, bats-assert 2.1.0 -- resolved through BATS_LIB_PATH. `make -C test deps` clones them into test/lib, which is untracked; the CI job installs the same tags through bats-core/bats-action and hands the path to the suites, since the action exposes it as a step output rather than exporting it. The upstream API differs from the copy, so all 60 call sites move: assert_output -l "X" assert_line "X" assert_output -l -r "X" assert_line --regexp "X" assert_output -l 0 "X" assert_line -n 0 "X" refute_output -l "X" refute_line "X" refute_output -p "X" refute_output --partial "X" test/helpers/containers.bash was the easy one to miss -- it asserts too, and `assert_output -l "healthy"` under the upstream helper compares the output to the string "-l". The image suites caught it immediately, which is the argument for the change in one line. The deps target compares the checked-out tag rather than the directory, so it is idempotent when current and re-fetches after a version bump here. It uses FORCE rather than .PHONY: make skips the implicit rule search for a phony target, so a pattern rule declared phony never fires. Verified on images built from this branch: nginx 17/17, apache 17/17, cli 4/4, and the demo suites still parse (19, 12, 6 and 1 tests). * test: cover the apache hardening and assert the dotfile rules on both variants The dotfile assertions were guarded to the nginx variant because apache had not been hardened yet. It has now, and both images answer 404 for the same requests, so the guard goes and the three assertions cover the pair. Three more cover what was specific to apache: a browsable document root, TRACE, and the server signature in the error page footer. Pointed at the published 8.5.9 apache image, which carries none of the fixes: 12 dotfiles are not served not ok 13 file inside a dot directory is not served not ok 14 /.well-known keeps its normal handling ok 15 the document root is not browsable not ok 16 TRACE is refused not ok 17 error pages carry no server signature not ok Five failures against the published image, seventeen passes against one built from this branch, on both variants -- apache skipping the three that describe the nginx monitoring port, which it does not have.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits: the library swap, then the assertions it makes possible.
Why the vendored helpers had to go
test/libheld 857 lines copied from bats-assert at some point, with no version and noprovenance — and the copy had drifted. It parses the short options only, and an unrecognised
long option falls through to the argument branch. So this, written while covering the apache
hardening:
refute_output --partial "<address>"asserted that the output does not contain the literal string
--partial— true of everything. Itpassed against an apache image that does emit the signature.
Nothing else in the suites used a long form, so that was the only assertion affected. The shape
of the failure is the point: a silently tautological assertion is worse than an absent one.
What replaces them
Upstream at pinned tags — bats-support 0.3.0, bats-assert 2.1.0 — resolved through
BATS_LIB_PATH:make -C test depsclones them intotest/lib, now untracked;bats-core/bats-actioninstalls the same tags and the path is handed to the suites,since the action exposes it as a step output rather than exporting it.
The upstream API differs from the copy, so all 60 call sites move:
assert_output -l "X"assert_line "X"assert_output -l -r "X"assert_line --regexp "X"assert_output -l 0 "X"assert_line -n 0 "X"refute_output -l "X"refute_line "X"refute_output -p "X"refute_output --partial "X"test/helpers/containers.bashwas the easy one to miss — it asserts too, andassert_output -l "healthy"under the upstream helper compares the output to the string-l.The image suites caught it on the next run, which is the argument for this change in one line.
The
depstarget compares the checked-out tag rather than the directory, so it is idempotent whencurrent and re-fetches after a version bump. It uses
FORCErather than.PHONY: make skips theimplicit rule search for a phony target, so a pattern rule declared phony never fires.
The assertions
The dotfile assertions were guarded to nginx because apache had not been hardened yet. #49 landed,
both images answer 404 for the same requests, so the guard goes and they cover the pair. Three
more cover what was specific to apache — a browsable document root, TRACE, and the server
signature in the error page footer.
Against the published 8.5.9 apache image, which carries none of the fixes:
Verified
AGENTS.mdgains the two test tiers,make -C test deps, and a correction: the suites no longerassume floating
:<variant>tags, andsnapshot-<variant>is the same tag on both branches, so arebuild is needed after switching.