Fix the proxy trust test and the conffile prompt that published a broken layer #1

Merged

marcos-mendez merged 3 commits from fix/proxy-trust into main

2026-09-26 15:28:11 -03:00

marcos-mendez commented

2026-09-26 15:17:52 -03:00

(Migrated from github.com)

Two fixes found on the live forum on 2026-09-26, carried into the sources,
plus the tests that keep them from coming back.

1. The trusted proxy was tested against the wrong address

/etc/nginx/conf.d/nodebb-proxy.conf, written at first boot by
firstboot.d/40nodebb, declared geo $nodebb_trusted_proxy, which reads
$remote_addr. The same file sets set_real_ip_from with
real_ip_header X-Forwarded-For, and the realip module runs in the post read
phase, before any geo or map variable is evaluated. By the time the trust
test ran, $remote_addr already held the address the proxy had put in
X-Forwarded-For: the visitor, not the proxy. The geo block never matched,
$nodebb_scheme stayed http, and every request to port 80 was answered
with a 307 to https, which the TLS terminating proxy sent straight back.

Fixed by matching on $realip_remote_addr, which keeps the address the
connection really came from. The rendered file and the shipped default both
carry a comment explaining the trap.

Reproduced and verified on a second container (forum2) assembled from the
layer of 2026-09-26, from the public services VM, over IPv6:

request	before	after
trusted source, `X-Forwarded-For` present, `X-Forwarded-Proto: https`	307 loop	200, `<title>Home \| NodeBB</title>`
trusted source, no `X-Forwarded-For`	200	200
untrusted source, both headers	307	307
trusted source, no proxy headers	307	307

The second row is why this went unnoticed: without X-Forwarded-For the
realip module does nothing and the old geo block happened to work.

2. The build published a half configured layer

The overlay ships /etc/confconsole/services.txt, a conffile of confconsole,
so the explicit upgrade of inithooks and confconsole added in f9ee052 met a
dpkg conffile prompt. Conf scripts run with stdin closed, dpkg read end of
file at the prompt and left confconsole install ok unpacked, apt exited 100
and root.patched failed.

The build failing was not the problem. bt-layer packs and exports whatever
root.patched left behind even when make fails (... | tee -a "$log" || true),
and because the target never finished, fab never removed the build overlays.
The layer published on 2026-09-26 (sha256 40e3f4da) therefore carries
/usr/local/bin/systemctl and /usr/local/bin/service, the build time
chroot shims, which core and nodejs-nginx do not have. At the first boot
10regen-sshkeys looped on them forever.

Fixed here by keeping the overlay's conffile non interactively and by
checking the dpkg state as well as the version, since a half configured
package reports the new version either way. The bt-layer half is a
buildtasks change and is not in this pull request.

Tests

tests/hook.bats, 15 tests: the hook runs for real against scratch
directories with PATH stubs for systemctl, nginx, redis-cli, chown
and runuser and a fake ./nodebb. Setup call, config.json patch, proxy
conf rendering, nginx -t, service calls, asset build switch, both
idempotence paths, bad and empty APP_TRUSTED_PROXY, domain fallback,
missing password with and without a terminal.
tests/nginx.bats, 12 tests: the shipped conf.d default and the rendered
file agree on the geo variable and the map, the site file and the shared
proxy include.
tests/nodebb.bats, 26 tests: the library, including an assertion that the
geo block uses $realip_remote_addr and that neither geo $remote_addr
nor the old geo $nodebb_trusted_proxy is present.
tests/coverage.sh now runs the whole suite and measures the hook as well
as the library.

Coverage (decision 0003, 95 percent on project authored code)

kcov line coverage (threshold 95 percent):
 100.00  43/43  nodebb.sh
  96.97  32/33  40nodebb

The one uncovered line is inside the dialog loop, which needs a terminal; the
test that drives it through script(1) covers the rest of the branch.

Test plan

bats tests/ on the workstation: 53 tests, all passing
tests/coverage.sh: both files above 95 percent
sh -n and shellcheck on the changed shell
first boot of forum2 from the layer, driven by the inithooks chain
alone: 00declarative read /etc/keel/instance.yaml, wrote
/etc/inithooks.conf, 20 hooks completed, nodebb, nginx and redis
active, keel diff reports the four known drifts
the four HTTP cases above, from the public services VM over IPv6
a clean layer rebuild with this recipe, which needs write access to the
build host's product tree

Two fixes found on the live forum on 2026-09-26, carried into the sources, plus the tests that keep them from coming back. ## 1. The trusted proxy was tested against the wrong address `/etc/nginx/conf.d/nodebb-proxy.conf`, written at first boot by `firstboot.d/40nodebb`, declared `geo $nodebb_trusted_proxy`, which reads `$remote_addr`. The same file sets `set_real_ip_from` with `real_ip_header X-Forwarded-For`, and the realip module runs in the post read phase, before any geo or map variable is evaluated. By the time the trust test ran, `$remote_addr` already held the address the proxy had put in `X-Forwarded-For`: the visitor, not the proxy. The geo block never matched, `$nodebb_scheme` stayed `http`, and every request to port 80 was answered with a 307 to https, which the TLS terminating proxy sent straight back. Fixed by matching on `$realip_remote_addr`, which keeps the address the connection really came from. The rendered file and the shipped default both carry a comment explaining the trap. Reproduced and verified on a second container (`forum2`) assembled from the layer of 2026-09-26, from the public services VM, over IPv6: | request | before | after | | --- | --- | --- | | trusted source, `X-Forwarded-For` present, `X-Forwarded-Proto: https` | 307 loop | 200, `<title>Home \| NodeBB</title>` | | trusted source, no `X-Forwarded-For` | 200 | 200 | | untrusted source, both headers | 307 | 307 | | trusted source, no proxy headers | 307 | 307 | The second row is why this went unnoticed: without `X-Forwarded-For` the realip module does nothing and the old geo block happened to work. ## 2. The build published a half configured layer The overlay ships `/etc/confconsole/services.txt`, a conffile of confconsole, so the explicit upgrade of inithooks and confconsole added in f9ee052 met a dpkg conffile prompt. Conf scripts run with stdin closed, dpkg read end of file at the prompt and left confconsole `install ok unpacked`, apt exited 100 and `root.patched` failed. The build failing was not the problem. `bt-layer` packs and exports whatever `root.patched` left behind even when make fails (`... | tee -a "$log" || true`), and because the target never finished, fab never removed the build overlays. The layer published on 2026-09-26 (sha256 `40e3f4da`) therefore carries `/usr/local/bin/systemctl` and `/usr/local/bin/service`, the build time chroot shims, which `core` and `nodejs-nginx` do not have. At the first boot `10regen-sshkeys` looped on them forever. Fixed here by keeping the overlay's conffile non interactively and by checking the dpkg state as well as the version, since a half configured package reports the new version either way. The `bt-layer` half is a buildtasks change and is not in this pull request. ## Tests - `tests/hook.bats`, 15 tests: the hook runs for real against scratch directories with PATH stubs for `systemctl`, `nginx`, `redis-cli`, `chown` and `runuser` and a fake `./nodebb`. Setup call, `config.json` patch, proxy conf rendering, `nginx -t`, service calls, asset build switch, both idempotence paths, bad and empty `APP_TRUSTED_PROXY`, domain fallback, missing password with and without a terminal. - `tests/nginx.bats`, 12 tests: the shipped conf.d default and the rendered file agree on the geo variable and the map, the site file and the shared proxy include. - `tests/nodebb.bats`, 26 tests: the library, including an assertion that the geo block uses `$realip_remote_addr` and that neither `geo $remote_addr` nor the old `geo $nodebb_trusted_proxy` is present. - `tests/coverage.sh` now runs the whole suite and measures the hook as well as the library. ## Coverage (decision 0003, 95 percent on project authored code) ``` kcov line coverage (threshold 95 percent): 100.00 43/43 nodebb.sh 96.97 32/33 40nodebb ``` The one uncovered line is inside the dialog loop, which needs a terminal; the test that drives it through `script(1)` covers the rest of the branch. ## Test plan - [x] `bats tests/` on the workstation: 53 tests, all passing - [x] `tests/coverage.sh`: both files above 95 percent - [x] `sh -n` and shellcheck on the changed shell - [x] first boot of `forum2` from the layer, driven by the inithooks chain alone: `00declarative` read `/etc/keel/instance.yaml`, wrote `/etc/inithooks.conf`, 20 hooks completed, nodebb, nginx and redis active, `keel diff` reports the four known drifts - [x] the four HTTP cases above, from the public services VM over IPv6 - [ ] a clean layer rebuild with this recipe, which needs write access to the build host's product tree

marcos-mendez commented

2026-09-26 15:28:09 -03:00

(Migrated from github.com)

shell / coverage passed in 2m2s: 100.00 percent on lib/nodebb.sh (43/43) and 96.97 percent on firstboot.d/40nodebb (32/33), both above the 95 percent bar of decision 0003.

tests / build-and-boot cannot run and is not being waited for. It targets the self-hosted runner keel-lxc-1, which is online and idle, but the organization's Default runner group has allows_public_repositories: false and this repository is public, so the job stays queued until GitHub cancels it after 24 hours. Two other things would stop it even then: that runner has no fab or buildtasks (docs/releases-host.md section 7), and the recipe change here needs a layer rebuild on the build host. The appliance was instead validated by hand on the build host, in a second container assembled from the layer of 2026-09-26; the results are in the description and in docs/forum-build-2026-09-26.md.

`shell / coverage` passed in 2m2s: 100.00 percent on lib/nodebb.sh (43/43) and 96.97 percent on firstboot.d/40nodebb (32/33), both above the 95 percent bar of decision 0003. `tests / build-and-boot` cannot run and is not being waited for. It targets the self-hosted runner `keel-lxc-1`, which is online and idle, but the organization's Default runner group has `allows_public_repositories: false` and this repository is public, so the job stays queued until GitHub cancels it after 24 hours. Two other things would stop it even then: that runner has no fab or buildtasks (docs/releases-host.md section 7), and the recipe change here needs a layer rebuild on the build host. The appliance was instead validated by hand on the build host, in a second container assembled from the layer of 2026-09-26; the results are in the description and in docs/forum-build-2026-09-26.md.