Found by the full-stack VM harness, which 3524 unit tests and a 7-image
container matrix had all missed. On a real Ubuntu 24.04 install the log read
"Opening HTTP/HTTPS (80, 443) via nftables" and not one line of
bootstrap_firewall appeared: it returned at its "a firewall is already active"
guard, so the box got its two ports opened and no default-deny policy at all.
The headline feature of that work was a no-op on exactly the population it was
written for, and it looked green the whole way.
The guard asked `[ "$(firewall_detect)" != "none" ]`. firewall_detect answers a
different question — "which backend do I open a port THROUGH" — and returns
`nftables` whenever the nft binary exists and can list a ruleset, including an
empty one. That is true of every modern Linux, so the guard reported every box
as already protected. Verified directly against a stock-image fixture: same
inputs, `firewall_detect` -> nftables, inbound-default-deny -> unprotected.
firewall_inbound_default_deny() asks what the bootstrap actually cares about:
does this host already deny inbound by default? Presence of rules is
deliberately NOT the test -- Docker installs plenty of nft and iptables chains
and protects nothing. A default-deny policy on the input hook is: firewalld
running, ufw active *with* "deny (incoming)", an nft base chain hooked to input
with policy drop or reject, or an iptables INPUT policy that is not ACCEPT.
Unknown answers 1 (unprotected), because the caller's response is to offer
protection and every caller re-checks before acting.
firewall_detect is left alone -- configure_firewall legitimately wants a
backend name for a binary that exists -- and both functions now carry a comment
saying which question they answer, since that is what was conflated.
Six cases pin the behaviour, five of them shapes the old guard got wrong: an
empty nft ruleset must run the bootstrap, Docker's chains must not read as
protection, an active-but-allow-inbound ufw must be fixed rather than
respected, a real nft policy drop must be left alone, and the two predicates
must disagree on a stock image -- that last one asserted at the predicate level
so the distinction survives a refactor of either function. make_stub_stdout is
new because an exit-code stub cannot express "the tool ran fine and reported
nothing", which is the whole stock-image case.
Green on all seven CI images (265/265/265/275/275/275/263, 0 failed).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Critical install/update/uninstall hotfixes from the hardening plan
(docs/plans/INSTALLER_UPDATER_UNINSTALLER_HARDENING_PLAN.md §4):
- Firewall automation (G1): new sourceable scripts/lib/firewall.sh detects
firewalld/ufw/nftables/iptables and opens 80/443; install.sh runs
configure_firewall, update.sh re-ensures it idempotently. Exactly what we
open is recorded in /etc/serverkit/install-state.json (scripts/lib/state.sh)
so uninstall undoes only our rules. Fixes the #1 "installed but unreachable"
failure on fresh RHEL-family boxes.
- Accept Python 3.11 OR 3.12 (G2): locate_python now iterates
python3.12/3.11/3 and provision_python installs the distro package
(python3.11 on Debian 12, 3.12 on Ubuntu/Fedora, 3.11/3.12 on RHEL) before
ever falling back to a source compile.
- Use $PYTHON_BIN for secret generation instead of a bare python3.
- Distro-first Node 20 (replacing unconditional NodeSource): install the
distro nodejs+npm, fall back to NodeSource only when it's missing or < 18,
and validate node>=18 before continuing.
- Unify uninstall (G4): new scripts/lib/uninstall.sh is the single canonical
teardown routine shared by uninstall.sh and `serverkit uninstall` (they had
drifted — one ran `down -v`, one didn't). Default preserves user data
(volumes kept, DB snapshotted to backups); --purge/--delete-volumes removes
volumes+DB+data dirs; --keep-data preserves /etc + /var/lib + backups;
--yes/--dry-run added; honors SERVERKIT_DIR. Removes the apt lock-wait
drop-in on uninstall.
Tests: new scripts/test/test_install.sh (11 cases, source-able install.sh)
covers python/node detection, firewall dry-run + state, and the uninstall
data-handling matrix. Existing test_update.sh still green. shellcheck clean
(error + warning) on all new/changed shell.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
