Six negations about trusting a proxy assert nothing, including the nginx injection case #15

New issue

Closed

opened 2026-09-28 05:28:51 -03:00 by marcos-mendez · 0 comments

marcos-mendez commented

2026-09-28 05:28:51 -03:00

(Migrated from github.com)

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 on its own line decides the test when it is the final command of the body and is inert everywhere else.

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

Six negations here are inert, and every one of them is about trusting a proxy.

tests/nginx.bats, the shipped conf.d default

@test "the shipped conf.d default trusts nobody" {
    grep -q '^    default 0;$' "$DEFAULT_CONF"
    ! grep -q ' 1;$' "$DEFAULT_CONF"              # line 19 — inert
    ! grep -q '^set_real_ip_from' "$DEFAULT_CONF" # line 20 — last, asserts
}

@test "the shipped conf.d default never matches geo on remote_addr" {
    ! grep -q '^geo \$remote_addr' "$DEFAULT_CONF"          # line 29 — inert
    ! grep -q '^geo \$nodebb_trusted_proxy' "$DEFAULT_CONF" # line 30 — last, asserts
}

Two of the four assertions ran.

tests/nodebb.bats, the validator and the renderer

Line 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.
Lines 135 and 136, that the rendered geo is on $realip_remote_addr and never on $remote_addr. That is the redirect loop the file's own comment warns about at length, and its guard did not run.
Lines 177 and 178, that no address means no trust.

What to do

Write every negation run ! cmd, with bats_require_minimum_version 1.5.0, the ones in final position included, so that no line's meaning depends on its position.

Watch for one thing while converting: run replaces $output, and several tests here do run nodebb_proxy_conf … and then grep <<< "$output" more than once. A second run ! reading <<< "$output" reads the first refutation's empty output and passes for a new reason. The rendered file has to be kept in a local first.

Test plan

shellcheck --severity=error --include=SC2314 over tests/*.bats reports nothing.
bats tests/*.bats green with the six inert assertions live.
tests/coverage.sh at or above the committed threshold of 95.
Tests ship nothing, so package / changelog passes by reporting that nothing that ships changed.

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` on its own line decides the test when it is the final command of the body and is **inert everywhere else**. shellcheck -f gcc --shell=bash tests/*.bats | grep SC2314 Six negations here are inert, and every one of them is about trusting a proxy. ## tests/nginx.bats, the shipped conf.d default @test "the shipped conf.d default trusts nobody" { grep -q '^ default 0;$' "$DEFAULT_CONF" ! grep -q ' 1;$' "$DEFAULT_CONF" # line 19 — inert ! grep -q '^set_real_ip_from' "$DEFAULT_CONF" # line 20 — last, asserts } @test "the shipped conf.d default never matches geo on remote_addr" { ! grep -q '^geo \$remote_addr' "$DEFAULT_CONF" # line 29 — inert ! grep -q '^geo \$nodebb_trusted_proxy' "$DEFAULT_CONF" # line 30 — last, asserts } Two of the four assertions ran. ## tests/nodebb.bats, the validator and the renderer - Line 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. - Lines 135 and 136, that the rendered `geo` is on `$realip_remote_addr` and never on `$remote_addr`. That is the redirect loop the file's own comment warns about at length, and its guard did not run. - Lines 177 and 178, that no address means no trust. ## What to do Write every negation `run ! cmd`, with `bats_require_minimum_version 1.5.0`, the ones in final position included, so that no line's meaning depends on its position. Watch for one thing while converting: `run` replaces `$output`, and several tests here do `run nodebb_proxy_conf …` and then grep `<<< "$output"` more than once. A second `run !` reading `<<< "$output"` reads the first refutation's empty output and passes for a new reason. The rendered file has to be kept in a local first. ## Test plan - `shellcheck --severity=error --include=SC2314` over `tests/*.bats` reports nothing. - `bats tests/*.bats` green with the six inert assertions live. - `tests/coverage.sh` at or above the committed threshold of 95. - Tests ship nothing, so `package / changelog` passes by reporting that nothing that ships changed.