A negated command only asserts in final position, and one that could never pass

[marcos-mendez]

Every negation in these suites now asserts wherever it stands. Six did not assert at all — and turning one on found a second bug in the same test.

Closes #15. Part of Keel-Linux/tracker#19, which has the organization-wide sweep.

Why a bare ! is inert

A bats test body has no assertions of its own: its verdict is the exit status of the last command it ran. Bash does not apply errexit to a negated command, so ! cmd decides the test in final position and is inert everywhere else. shellcheck grades the two apart, error: for the inert ones and note: for the rest:

shellcheck -f gcc --shell=bash tests/*.bats | grep SC2314

What was not being checked

All six are about trusting a proxy.

Where	Was not asserting
nginx.bats 19, 29	that the shipped conf.d default trusts nobody, and that it never matches geo on $remote_addr. Two of those four assertions ran.
nodebb.bats 117	nodebb_valid_proxy "2001:db8::13; }" — the value that would close the geo block and append directives of its own. The injection case. Only hello decided anything.
nodebb.bats 135, 136	that the rendered geo is on $realip_remote_addr and never on $remote_addr — the redirect loop the file's own comment warns about at length.
nodebb.bats 177, 178	that no address means no trust.

All of them are run ! now, together with the eight that were in final position and did assert: a line whose meaning depends on its position is the trap itself.

The bug this found

nodebb.bats, "proxy_conf without an address trusts nobody", asserted:

! grep -q set_real_ip_from <<< "$output"

Unanchored — against a file whose own explanatory comment names the directive:

$ nodebb_proxy_conf '' | grep -n set_real_ip_from
6:# that is not a detail. This same file sets set_real_ip_from with

So that assertion could never have held, for any input, and nobody could know, because nothing ever ran it. It is anchored now:

run ! grep -q '^set_real_ip_from' <<< "$conf"

which is the property that was meant, and the spelling the positive case at line 167 already used (^set_real_ip_from 2001:db8::13;$). The rendered file is correct — with no address it emits no set_real_ip_from directive. Only the assertion was wrong, so nothing shipped broken.

A second shape of the same defect

run replaces $output, $status and $lines. Two tests here do run nodebb_proxy_conf … and then grep <<< "$output" several times; once the first of those becomes a run !, the next one greps the refutation's empty output and passes for a new reason. Both tests now keep the rendered file in a local first:

run nodebb_proxy_conf ""
[ "$status" -eq 0 ]
# every `run` below replaces $output, so keep the rendered file first
local conf=$output

That was caught by an audit over every converted site, not by the suite — the vacuous pass is green by construction. It is written up in the docs/traps.md entry that goes with this sweep.

What else was verified

The shipped overlay/etc/nginx/conf.d/nodebb-proxy.conf and the rendered output were both read directly, and nodebb_valid_proxy was run against all five of its cases by hand, before the conversion. Everything except the assertion above is correct.

Test plan

• The validator's five cases and both rendered files verified by hand, before the conversion.

• shellcheck -f gcc --shell=bash --severity=error --include=SC2314 tests/*.bats reports nothing.

• Every converted site audited for run clobbering $output/$status/$lines; the two real cases fixed.

• bats tests/*.bats green, 134 tests.

• tests/coverage.sh, threshold 95, unchanged:

  100.00  31/31    zz-project-packages
  100.00  43/43    nodebb.sh
   96.97  32/33    40nodebb
  100.00  137/137  boot-test-lib.sh
  100.00  54/54    keel-archive-check
  

• Tests ship nothing, so package / changelog passes by reporting that nothing that ships changed. No changelog entry and no version bump.

[marcos-mendez]

Reviewed: every converted refutation goes red when its property is broken; emitting set_real_ip_from with no trusted address fails the anchored check. None reads the output of the run it replaced. No changes needed; merged main so the renamed appliance check runs on this head.