From eb7d4c262d91665f7fe69dad51899d8028e1e73c Mon Sep 17 00:00:00 2001
From: Andy Lemin <andlemin@amazon.com>
Date: Sun, 27 Sep 2026 17:15:24 +1000
Subject: [PATCH] pfui: upgrade in place, depend on redis, and summarise each
 scan

Both server installers now put the service back as they found it. rcctl check
records whether it was running, the daemon is stopped before anything is
replaced, and started again only if it had been running. install(1) renames
into place, so replacing the binary under a running daemon succeeds while that
daemon keeps the old inode: an upgrade reported success and changed nothing.
The Rust installer stops only once its build has succeeded, so a failed build
leaves the old daemon serving, and a failed restart is reported as such,
because the firewall was whitelisting a moment before. A daemon the operator
had stopped stays stopped, and only then is the enable and start advice
printed. Two passes in the OpenBSD job cover both cases, with the pids proving
the restart and the config proving it was not replaced.

The sync loop logs one line per cycle that did something: the table's size,
what expired, and what was added to or removed from the table and the file.
A cycle that changed nothing says nothing, so INFO stays quiet in a steady
state and every line means work was done.

Both rc.d scripts start Redis before the daemon when REDIS_HOST is local. The
daemon whitelists without it, but nothing expires until it appears. A failure
is not fatal, since a non-zero rc_pre would abort a start the daemon can
manage, and a REDIS_HOST elsewhere is left alone. The daemon cannot do this
itself: starting a service needs proc exec, which its pledge excludes under
CTL: IOCTL.

Redis connect, read and write are bounded. The redis crate blocks indefinitely
on all three by default, so a stalled server held a worker for good, and enough
of those retire the pool and stop the firewall whitelisting with nothing in the
log. Each pass now asserts that every acknowledged message also finished, the
acknowledgement preceding the store writes, which is how an unbounded store
call shows up.

update_dns_blocklist.sh fetches HaGeZi's pro and tif domain lists, the two the
README recommends, and converts them to Unbound's local-zone format. Only
plausible domains are written: the files carry a header, and the wildcard
variants carry '*.' prefixes, either of which would take the whole include
file with it. The guards against a short or collapsed list are unchanged. The
installer keeps an existing copy rather than replacing it, since the sources,
the allowlist hook and the thresholds are meant to be edited.

Both workflows drop their pull_request trigger and gain a concurrency group.
With a PR open they ran everything twice, and a force-push left the previous
commit's jobs running; GitHub shows push-triggered runs on the PR anyway. The
OpenBSD job also times compiling and running the tests separately, because the
test crates pull in dev-dependencies the daemon build does not and one budget
for both could not tell a slow compile from a hung test.
---
 .github/workflows/ci.yml                      |   5 +-
 .github/workflows/openbsd.yml                 |  12 +-
 README.md                                     |   9 +-
 client-unbound/tests/test_blocklist_update.py |  37 ++++-
 client-unbound/tools/update_dns_blocklist.sh  |  70 +++++----
 install-client-unbound.sh                     |  12 +-
 install-server-python.sh                      |  38 ++++-
 install-server-rust.sh                        |  38 ++++-
 server-python/rc.d/pfui_firewall              |  15 ++
 server-rust/rc.d/pfui_firewall                |  15 ++
 server-rust/src/backends.rs                   |  15 +-
 server-rust/src/sync.rs                       | 135 ++++++++++++++++--
 tests/openbsd/run.sh                          | 109 ++++++++++++--
 13 files changed, 426 insertions(+), 84 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index e8985bd..fa645c5 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -2,7 +2,10 @@ name: tests
 
 on:
   push:
-  pull_request:
+
+concurrency:
+  group: tests-${{ github.ref }}
+  cancel-in-progress: true
 
 jobs:
   python:
diff --git a/.github/workflows/openbsd.yml b/.github/workflows/openbsd.yml
index 98d1a3c..be8c3cf 100644
--- a/.github/workflows/openbsd.yml
+++ b/.github/workflows/openbsd.yml
@@ -5,14 +5,6 @@ name: openbsd
 # The daemon's ioctl, unveil and pledge code compiles on no other platform.
 
 on:
-  pull_request:
-    paths:
-      - 'server-rust/**'
-      - 'client-unbound/**'
-      - 'protocol/**'
-      - 'install-*.sh'
-      - 'tests/openbsd/**'
-      - '.github/workflows/openbsd.yml'
   push:
     paths:
       - 'server-rust/**'
@@ -35,6 +27,10 @@ on:
         description: 'Unbound to build: latest, master, or a release-* tag'
         default: 'latest'
 
+concurrency:
+  group: openbsd-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   end-to-end:
     name: OpenBSD ${{ matrix.release }}
diff --git a/README.md b/README.md
index 3d756cb..b729e39 100644
--- a/README.md
+++ b/README.md
@@ -280,9 +280,12 @@ Edit _unbound user's crontab to; import domain lists / execute 'unbound-control
 NB; If you want to keep the `unbound-control` cron commands under the _adblock user, you will need to add _adblock to _unbound group.
 
 NB; `client-unbound/tools/update_dns_blocklist.sh` is an alternative to
-unbound-adblock that ships with PFUI. It still fetches StevenBlack's hosts-format
-lists, so it converts with a hosts parser; pointing it at HaGeZi means reading the
-domain-only format instead.
+unbound-adblock that ships with PFUI. It fetches the same two HaGeZi lists,
+`pro` and `tif`, converts them to Unbound's local-zone format and publishes one
+file for `include:`. It refuses to publish a list that is short or has shrunk
+sharply, so a failed download cannot quietly turn the filter off. An existing
+copy is never replaced by the installer, since the sources and thresholds are
+meant to be edited.
 
 References;\
 HaGeZi's lists and what each tier blocks: https://github.com/hagezi/dns-blocklists \
diff --git a/client-unbound/tests/test_blocklist_update.py b/client-unbound/tests/test_blocklist_update.py
index 17970fd..77f6e51 100644
--- a/client-unbound/tests/test_blocklist_update.py
+++ b/client-unbound/tests/test_blocklist_update.py
@@ -37,8 +37,11 @@
   esac
 fi
 lines=${CURL_LINES:-1200}
-awk -v n="$lines" 'BEGIN { for (i = 0; i < n; i++) print "0.0.0.0 bad" i ".example.com" }' \
-  > "$dest"
+{
+  echo "# Title: stub list"
+  echo "!"
+  awk -v n="$lines" 'BEGIN { for (i = 0; i < n; i++) print "bad" i ".example.com" }'
+} > "$dest"
 """
 
 RECORDER_STUB = r"""#!/usr/bin/env bash
@@ -113,16 +116,16 @@ def test_norestart_publishes_without_restarting(harness):
 
 
 def test_a_failed_source_falls_back_to_the_mirror(harness):
-    """The StevenBlack lists have a second URL; using it is not a failure."""
+    """Each list has a second URL; using it is not a failure."""
     result = harness.run(CURL_FAIL="raw.githubusercontent.com")
     assert result.returncode == 0, result.stderr.decode()
-    assert "sbc.io" in harness.calls()
+    assert "jsdelivr" in harness.calls()
     assert harness.lines()
 
 
 def test_a_source_that_cannot_be_downloaded_at_all_aborts(harness):
     """No previous copy to fall back on, so there is nothing to publish."""
-    result = harness.run(CURL_FAIL="hosts")
+    result = harness.run(CURL_FAIL="onlydomains")
     assert result.returncode != 0
     assert harness.lines() is None, "published a list built from a failed download"
     assert "rcctl" not in harness.calls(), "restarted the resolver anyway"
@@ -134,7 +137,7 @@ def test_a_source_that_fails_today_is_reused_from_yesterday(harness):
     assert harness.run("norestart").returncode == 0
     before = harness.lines()
 
-    result = harness.run("norestart", CURL_FAIL="pgl.yoyo.org")
+    result = harness.run("norestart", CURL_FAIL="tif-onlydomains")
     assert result.returncode == 0, result.stderr.decode()
     assert harness.lines() == before, "reusing a cached source lost domains"
 
@@ -148,7 +151,7 @@ def test_a_previously_published_list_survives_a_failed_run(harness):
         if stale.name != "dns_blocklist":
             stale.unlink()
 
-    result = harness.run(CURL_FAIL="hosts")
+    result = harness.run(CURL_FAIL="onlydomains")
     assert result.returncode != 0
     assert harness.lines() == before, "a failed run replaced a working blocklist"
     assert "rcctl" not in harness.calls(), "restarted the resolver anyway"
@@ -194,3 +197,23 @@ def test_no_staging_files_are_left_behind(harness):
     harness.run("norestart", CURL_LINES=3)  # Refused
     leftovers = [p.name for p in harness.etc.iterdir() if ".new." in p.name]
     assert leftovers == [], f"left {leftovers} in place"
+
+
+def test_only_plausible_domains_reach_unbound(harness):
+    """HaGeZi's files carry a header, and the wildcard variants carry '*.'
+    prefixes. A line Unbound cannot parse as a local-zone takes the whole
+    include file with it, so anything that is not a domain is dropped."""
+    extra = "\n".join(
+        [
+            'echo "*.wildcard.example"',
+            'echo "0.0.0.0 hosts-format.example"',
+            'echo "not a domain at all"',
+        ]
+    )
+    stub = harness.etc.parent / "bin" / "curl"
+    stub.write_text(stub.read_text().replace("} > \"$dest\"", extra + "\n} > \"$dest\""))
+    assert harness.run("norestart").returncode == 0
+    published = "\n".join(harness.lines())
+    assert "bad1.example.com" in published
+    for rejected in ("wildcard.example", "0.0.0.0 hosts-format", "not a domain"):
+        assert rejected not in published, f"{rejected!r} reached the include file"
diff --git a/client-unbound/tools/update_dns_blocklist.sh b/client-unbound/tools/update_dns_blocklist.sh
index 73fb634..328b85a 100755
--- a/client-unbound/tools/update_dns_blocklist.sh
+++ b/client-unbound/tools/update_dns_blocklist.sh
@@ -1,6 +1,8 @@
 #!/usr/bin/env bash
 
-# Example DNS BlockList script to download common bad domains from some example well known sources
+# Example DNS BlockList script: HaGeZi's pro and tif domain lists, converted to
+# Unbound's local-zone format and published as one file for 'include:'.
+# https://github.com/hagezi/dns-blocklists
 
 set -u
 
@@ -52,10 +54,12 @@ require() {
   [ -s "$1" ] || die "$2 is unavailable and no previous copy exists ($1)"
 }
 
-# hosts_to_unbound <raw> <unbound-format>
-hosts_to_unbound() {
-  grep '^0\.0\.0\.0' "$1" \
-    | awk '{print "local-zone: \""$2"\" redirect\nlocal-data: \""$2" A 0.0.0.0\""}' \
+# domains_to_unbound <domain list> <unbound-format>
+# One domain per line, with comments and a header. A line Unbound cannot parse as
+# a local-zone takes the whole include file with it, so only domains are written.
+domains_to_unbound() {
+  grep -E '^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)+$' "$1" \
+    | awk '{print "local-zone: \""$1"\" redirect\nlocal-data: \""$1" A 0.0.0.0\""}' \
     > "$2"
 }
 
@@ -63,41 +67,33 @@ lines() { wc -l < "$1" | tr -d '[:space:]'; }
 
 log info "Updating DNS Domain Filter Lists"
 
-# https://github.com/StevenBlack/hosts
-SB_BASE=https://raw.githubusercontent.com/StevenBlack/hosts/master/alternates
-# Fallback is plain HTTP and unauthenticated: content is not verified
-SB_MIRROR=http://sbc.io/hosts/alternates
-
-log info "Downloading StevenBlack Bad Domains - Unified hosts (adware + malware) + fakenews + gambling"
-fetch "${ETC}/stephenblack_adware_malware_fakenews_gambling-raw" \
-  "${SB_BASE}/fakenews-gambling/hosts" \
-  "${SB_MIRROR}/fakenews-gambling/hosts"
-require "${ETC}/stephenblack_adware_malware_fakenews_gambling-raw" \
-  "StevenBlack fakenews-gambling list"
-
-log info "Downloading StevenBlack Bad Domains - Unified hosts (adware + malware) + fakenews + gambling + social"
-fetch "${ETC}/stephenblack_adware_malware_fakenews_gambling_social-raw" \
-  "${SB_BASE}/fakenews-gambling-social/hosts" \
-  "${SB_MIRROR}/fakenews-gambling-social/hosts"
-require "${ETC}/stephenblack_adware_malware_fakenews_gambling_social-raw" \
-  "StevenBlack fakenews-gambling-social list"
-
-log info "Converting StevenBlack Bad Domains from RAW format to Unbound config format"
-hosts_to_unbound "${ETC}/stephenblack_adware_malware_fakenews_gambling-raw" \
-  "${ETC}/stephenblack_adware_malware_fakenews_gambling-unbound"
-hosts_to_unbound "${ETC}/stephenblack_adware_malware_fakenews_gambling_social-raw" \
-  "${ETC}/stephenblack_adware_malware_fakenews_gambling_social-unbound"
-
-echo
-log info "Downloading YoYo AdServers Bad Domains"
-fetch "${ETC}/yoyo_adservers-unbound" \
-  "https://pgl.yoyo.org/adservers/serverlist.php?hostformat=unbound&showintro=0&mimetype=plaintext"
-require "${ETC}/yoyo_adservers-unbound" "YoYo AdServers list"
+# https://github.com/hagezi/dns-blocklists
+# The -onlydomains variants only: wildcard/*.txt carries '*.' prefixes and
+# adblock/ carries ABP filter syntax, neither of which Unbound loads.
+HG_BASE=https://raw.githubusercontent.com/hagezi/dns-blocklists/main/wildcard
+# Same content, used only if the first fails
+HG_MIRROR=https://cdn.jsdelivr.net/gh/hagezi/dns-blocklists@latest/wildcard
+
+# pro: ads, tracking and telemetry. tif: malware, phishing and scams.
+for list in pro tif; do
+  log info "Downloading HaGeZi ${list} domains"
+  fetch "${ETC}/hagezi_${list}-raw" \
+    "${HG_BASE}/${list}-onlydomains.txt" \
+    "${HG_MIRROR}/${list}-onlydomains.txt"
+  require "${ETC}/hagezi_${list}-raw" "HaGeZi ${list} list"
+done
+
+log info "Converting HaGeZi domains to Unbound config format"
+for list in pro tif; do
+  domains_to_unbound "${ETC}/hagezi_${list}-raw" "${ETC}/hagezi_${list}-unbound"
+  [ -s "${ETC}/hagezi_${list}-unbound" ] \
+    || die "the HaGeZi ${list} list produced no usable domains"
+done
 
 echo
 log info "Merging all Bad Domains"
-cat "${ETC}/stephenblack_adware_malware_fakenews_gambling_social-unbound" \
-    "${ETC}/yoyo_adservers-unbound" > "${ETC}/dns_blocklist_all" \
+cat "${ETC}/hagezi_pro-unbound" "${ETC}/hagezi_tif-unbound" \
+    > "${ETC}/dns_blocklist_all" \
   || die "cannot write ${ETC}/dns_blocklist_all"
 
 echo
diff --git a/install-client-unbound.sh b/install-client-unbound.sh
index d05a950..c683fd6 100755
--- a/install-client-unbound.sh
+++ b/install-client-unbound.sh
@@ -381,8 +381,16 @@ if [[ "$OS" = "OpenBSD" ]]; then
   [ -f "${TARGET}/update_root_hints.sh" ] && mv "${TARGET}/update_root_hints.sh" "${TARGET}/update_root_hints.sh.${HOUR}"
   # root-owned: these run from cron with the privilege to write /var/unbound and restart the service
   install -m 755 -o root -g wheel "${DIR}"/client-unbound/tools/update_root_hints.sh ${TARGET}/update_root_hints.sh
-  [ -f "${TARGET}/update_dns_blocklist.sh" ] && mv "${TARGET}/update_dns_blocklist.sh" "${TARGET}/update_dns_blocklist.sh.${HOUR}"
-  install -m 755 -o root -g wheel "${DIR}"/client-unbound/tools/update_dns_blocklist.sh ${TARGET}/update_dns_blocklist.sh
+  # The sources, allowlist hook and thresholds are meant to be edited, so an
+  # existing copy is kept
+  if [ -f "${TARGET}/update_dns_blocklist.sh" ]; then
+    echo "PFUIDNS: Keeping the existing ${TARGET}/update_dns_blocklist.sh"
+    echo "PFUIDNS: (the shipped version is ${DIR}/client-unbound/tools/update_dns_blocklist.sh)"
+  else
+    install -m 755 -o root -g wheel "${DIR}"/client-unbound/tools/update_dns_blocklist.sh \
+      ${TARGET}/update_dns_blocklist.sh || die "cannot install update_dns_blocklist.sh"
+    echo "PFUIDNS: Installed ${TARGET}/update_dns_blocklist.sh (HaGeZi pro + tif)"
+  fi
   echo "New scripts: ${TARGET}/update_root_hints.sh, ${TARGET}/update_dns_blocklist.sh"
 
   # The resolver's own config carries an operator's whole ruleset, so an
diff --git a/install-server-python.sh b/install-server-python.sh
index cbeace0..63f96e5 100755
--- a/install-server-python.sh
+++ b/install-server-python.sh
@@ -26,6 +26,32 @@ add_pkg() {
   done
 }
 
+# install(1) renames into place, so a binary replaced under a running daemon
+# leaves that daemon on the old inode.
+SERVICE_WAS_RUNNING=no
+
+stop_for_upgrade() {
+  rcctl check pfui_firewall >/dev/null 2>&1 || return 0
+  SERVICE_WAS_RUNNING=yes
+  echo "PFUIFW: Stopping pfui_firewall to upgrade it"
+  rcctl stop pfui_firewall >/dev/null 2>&1 \
+    || die "cannot stop pfui_firewall; stop it by hand and re-run"
+}
+
+# Only what was running is started. A failure is loud: the firewall was
+# whitelisting a moment ago.
+start_after_upgrade() {
+  [ "${SERVICE_WAS_RUNNING}" = yes ] || return 0
+  echo "PFUIFW: Starting pfui_firewall"
+  if rcctl start pfui_firewall; then
+    return 0
+  fi
+  err=1
+  echo "PFUIFW: FAILED to start pfui_firewall, which was running before this" >&2
+  echo "        upgrade, so this firewall is not whitelisting now. Its reason:" >&2
+  echo "        tail /var/log/daemon | grep pfui_firewall" >&2
+}
+
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 HOUR=$(date +%d-%b-%H_%M)
 
@@ -118,6 +144,8 @@ if [[ "$OS" = "OpenBSD" ]]; then
   python3 -c "import lz4.frame, yaml, redis, service" \
     || die "dependencies are not importable by $(command -v python3)"
 
+  stop_for_upgrade
+
   echo "PFUIFW: Installing PFUI Firewall Service (will backup any existing pfui_firewall configuration)"
   install -m 755 -o root -g wheel "${DIR}"/server-python/pfui_firewall.py /usr/local/sbin/pfui_firewall \
     || die "cannot install the daemon"
@@ -186,14 +214,20 @@ if [[ "$OS" = "OpenBSD" ]]; then
   echo "PFUIFW: NOTE point pf.conf 'persist file' paths at ${PERSIST_DIR}/ipv{4,6}_domains (see /etc/pf-pfui-example.conf)"
 fi
 
+start_after_upgrade
+
 if [[ $err != 0 ]]; then
   echo "PFUIFW: All Completed, but with some errors. Please investigate."
   exit 1
 else
   echo "PFUIFW: All Completed successfully."
 fi
-echo "PFUIFW: Enable service 'rcctl enable pfui_firewall'"
-echo "PFUIFW: Start service 'rcctl start pfui_firewall'"
+if [ "${SERVICE_WAS_RUNNING}" = yes ]; then
+  echo "PFUIFW: pfui_firewall is running the version just installed"
+else
+  echo "PFUIFW: Enable service 'rcctl enable pfui_firewall'"
+  echo "PFUIFW: Start service 'rcctl start pfui_firewall'"
+fi
 echo
 echo "PFUIFW: If PFUI_Unbound runs on THIS host, uncomment SOCKET_UNIX in"
 echo "        /etc/pfui_firewall.yml and add '- SOCKET: /var/run/pfui/pfui_firewall.sock'"
diff --git a/install-server-rust.sh b/install-server-rust.sh
index fb2a97e..0a55490 100755
--- a/install-server-rust.sh
+++ b/install-server-rust.sh
@@ -29,6 +29,32 @@ add_pkg() {
   done
 }
 
+# install(1) renames into place, so a binary replaced under a running daemon
+# leaves that daemon on the old inode.
+SERVICE_WAS_RUNNING=no
+
+stop_for_upgrade() {
+  rcctl check pfui_firewall >/dev/null 2>&1 || return 0
+  SERVICE_WAS_RUNNING=yes
+  echo "PFUIFW: Stopping pfui_firewall to upgrade it"
+  rcctl stop pfui_firewall >/dev/null 2>&1 \
+    || die "cannot stop pfui_firewall; stop it by hand and re-run"
+}
+
+# Only what was running is started. A failure is loud: the firewall was
+# whitelisting a moment ago.
+start_after_upgrade() {
+  [ "${SERVICE_WAS_RUNNING}" = yes ] || return 0
+  echo "PFUIFW: Starting pfui_firewall"
+  if rcctl start pfui_firewall; then
+    return 0
+  fi
+  err=1
+  echo "PFUIFW: FAILED to start pfui_firewall, which was running before this" >&2
+  echo "        upgrade, so this firewall is not whitelisting now. Its reason:" >&2
+  echo "        tail /var/log/daemon | grep pfui_firewall" >&2
+}
+
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 HOUR=$(date +%d-%b-%H_%M)
 
@@ -131,6 +157,8 @@ fi
 # re-applies this on every start, because MAKEDEV resets it on release upgrades
 chgrp _pfui_firewall /dev/pf && chmod 660 /dev/pf || die "cannot set /dev/pf ownership"
 
+stop_for_upgrade
+
 echo "PFUIFW: Installing PFUI Firewall daemon"
 # The binary is the whole install: no module tree, no wire module. Overwrites
 # the Python daemon at the same path when present, deliberately, for drop-in.
@@ -199,14 +227,20 @@ if grep -q 'persist file "/var/spool/pfui' /etc/pf.conf 2>/dev/null; then
   echo "        Until then a PF reload restores whatever the old files hold."
 fi
 
+start_after_upgrade
+
 if [[ $err != 0 ]]; then
   echo "PFUIFW: All Completed, but with some errors. Please investigate."
   exit 1
 else
   echo "PFUIFW: All Completed successfully."
 fi
-echo "PFUIFW: Enable service 'rcctl enable pfui_firewall'"
-echo "PFUIFW: Start service 'rcctl start pfui_firewall'"
+if [ "${SERVICE_WAS_RUNNING}" = yes ]; then
+  echo "PFUIFW: pfui_firewall is running the version just installed"
+else
+  echo "PFUIFW: Enable service 'rcctl enable pfui_firewall'"
+  echo "PFUIFW: Start service 'rcctl start pfui_firewall'"
+fi
 echo
 echo "PFUIFW: If PFUI_Unbound runs on THIS host, uncomment SOCKET_UNIX in"
 echo "        /etc/pfui_firewall.yml and add '- SOCKET: /var/run/pfui/pfui_firewall.sock'"
diff --git a/server-python/rc.d/pfui_firewall b/server-python/rc.d/pfui_firewall
index 79445b5..fc5a15e 100755
--- a/server-python/rc.d/pfui_firewall
+++ b/server-python/rc.d/pfui_firewall
@@ -26,6 +26,21 @@ rc_pre() {
 		install -d -o _pfui_firewall -g _pfui_firewall -m 755 /var/run/pfui
 	fi
 	chgrp _pfui_firewall /dev/pf && chmod 660 /dev/pf
+
+	# The daemon whitelists without Redis, but nothing expires until it appears.
+	# Never fatal: a non-zero rc_pre would abort a start the daemon can manage.
+	# Only a local REDIS_HOST is ours to start.
+	redis_host=$(sed -n 's/^[[:space:]]*REDIS_HOST:[[:space:]]*\([^[:space:]#]*\).*/\1/p' \
+	    /etc/pfui_firewall.yml 2>/dev/null | tr -d "\"'" | tail -1)
+	case "${redis_host:-127.0.0.1}" in
+	127.*|::1|localhost)
+		if ! rcctl check redis >/dev/null 2>&1; then
+			echo "pfui_firewall: starting redis first"
+			rcctl start redis >/dev/null 2>&1 || \
+			    echo "pfui_firewall: redis did not start; nothing will expire until it does"
+		fi
+		;;
+	esac
 }
 
 # The daemon takes its verb positionally, so ${daemon} ${daemon_flags} cannot be
diff --git a/server-rust/rc.d/pfui_firewall b/server-rust/rc.d/pfui_firewall
index a074a3a..f32363b 100644
--- a/server-rust/rc.d/pfui_firewall
+++ b/server-rust/rc.d/pfui_firewall
@@ -31,6 +31,21 @@ rc_pre() {
 		install -d -o _pfui_firewall -g _pfui_firewall -m 755 /var/run/pfui
 	fi
 	chgrp _pfui_firewall /dev/pf && chmod 660 /dev/pf
+
+	# The daemon whitelists without Redis, but nothing expires until it appears.
+	# Never fatal: a non-zero rc_pre would abort a start the daemon can manage.
+	# Only a local REDIS_HOST is ours to start.
+	redis_host=$(sed -n 's/^[[:space:]]*REDIS_HOST:[[:space:]]*\([^[:space:]#]*\).*/\1/p' \
+	    /etc/pfui_firewall.yml 2>/dev/null | tr -d "\"'" | tail -1)
+	case "${redis_host:-127.0.0.1}" in
+	127.*|::1|localhost)
+		if ! rcctl check redis >/dev/null 2>&1; then
+			echo "pfui_firewall: starting redis first"
+			rcctl start redis >/dev/null 2>&1 || \
+			    echo "pfui_firewall: redis did not start; nothing will expire until it does"
+		fi
+		;;
+	esac
 }
 
 # Must stay: rc.subr's default rc_reload sends SIGHUP, which the daemon
diff --git a/server-rust/src/backends.rs b/server-rust/src/backends.rs
index b757d52..b2d5511 100644
--- a/server-rust/src/backends.rs
+++ b/server-rust/src/backends.rs
@@ -7,6 +7,7 @@ use std::cell::RefCell;
 use std::net::IpAddr;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
+use std::time::Duration;
 
 use crate::config::Config;
 use crate::logger::Logger;
@@ -21,6 +22,11 @@ pub struct RealBackends {
     pf_dev: SharedDev,
 }
 
+/// Connect, read and write block indefinitely by default, and a worker stuck in
+/// one is a worker the pool has lost. Generous, because the store runs after the
+/// acknowledgement.
+const REDIS_TIMEOUT: Duration = Duration::from_secs(5);
+
 thread_local! {
     // One connection per thread, opened lazily and dropped on any error so
     // the next use reconnects
@@ -71,7 +77,14 @@ impl RealBackends {
         CONN.with(|slot| {
             let mut slot = slot.borrow_mut();
             if slot.is_none() {
-                *slot = Some(self.client.get_connection().map_err(|e| e.to_string())?);
+                let conn = self
+                    .client
+                    .get_connection_with_timeout(REDIS_TIMEOUT)
+                    .map_err(|e| e.to_string())?;
+                conn.set_read_timeout(Some(REDIS_TIMEOUT))
+                    .and_then(|()| conn.set_write_timeout(Some(REDIS_TIMEOUT)))
+                    .map_err(|e| e.to_string())?;
+                *slot = Some(conn);
             }
             match f(slot.as_mut().expect("connection present")) {
                 Ok(v) => Ok(v),
diff --git a/server-rust/src/sync.rs b/server-rust/src/sync.rs
index 5f1f2fd..93e0ca3 100644
--- a/server-rust/src/sync.rs
+++ b/server-rust/src/sync.rs
@@ -32,8 +32,28 @@ pub trait SyncOps: Send + Sync {
     fn table_push(&self, table: &str, ips: &[String]);
 }
 
+/// What one cycle did, for the summary line.
+#[derive(Default)]
+struct Worked {
+    /// Entries in the PF table after the cycle read it
+    size: usize,
+    expired: usize,
+    table_added: usize,
+    table_removed: usize,
+    file_added: usize,
+    file_removed: usize,
+}
+
+impl Worked {
+    /// A steady state logs nothing, so every summary line means work was done.
+    fn changed(&self) -> bool {
+        self.expired | self.table_added | self.table_removed | self.file_added | self.file_removed
+            != 0
+    }
+}
+
 /// Expire IPs whose metadata says they are past their TTL or cache expiry.
-fn scan_redis_db(ops: &dyn SyncOps, table: &str, log: &Logger) {
+fn scan_redis_db(ops: &dyn SyncOps, table: &str, log: &Logger) -> usize {
     let expired = match ops.expired_entries(table) {
         Ok(e) => e,
         Err(e) => {
@@ -41,11 +61,11 @@ fn scan_redis_db(ops: &dyn SyncOps, table: &str, log: &Logger) {
             log.error(&format!(
                 "Failed to scan Redis for {table}, retrying next scan: {e}"
             ));
-            return;
+            return 0;
         }
     };
     if expired.is_empty() {
-        return;
+        return 0;
     }
     let ips: Vec<String> = expired.iter().map(|e| e.ip.clone()).collect();
     if log.verbose {
@@ -65,11 +85,14 @@ fn scan_redis_db(ops: &dyn SyncOps, table: &str, log: &Logger) {
     }
     if let Err(e) = ops.db_pop(table, &ips) {
         log.error(&format!("Failed to delete {ips:?} from Redis: {e}"));
+        return 0;
     }
+    ips.len()
 }
 
 /// Remove orphaned IPs (no Redis key) from the PF table, add missing ones.
-fn sync_pf_table(ops: &dyn SyncOps, table: &str, log: &Logger) {
+/// Returns the table's size, then what was added and removed.
+fn sync_pf_table(ops: &dyn SyncOps, table: &str, log: &Logger) -> (usize, usize, usize) {
     // PF table before Redis, and never diff against a partial read
     let t_ips = match ops.table_show(table) {
         Ok(ips) => ips,
@@ -77,16 +100,17 @@ fn sync_pf_table(ops: &dyn SyncOps, table: &str, log: &Logger) {
             log.error(&format!(
                 "Failed to read PF table {table}, skipping this sync: {e}"
             ));
-            return;
+            return (0, 0, 0);
         }
     };
+    let size = t_ips.len();
     let db_ips = match ops.db_ips(table) {
         Ok(ips) => ips,
         Err(e) => {
             log.error(&format!(
                 "Failed to read Redis for {table}, skipping this sync: {e}"
             ));
-            return;
+            return (size, 0, 0);
         }
     };
     let t_set: BTreeSet<&str> = t_ips.iter().map(String::as_str).collect();
@@ -100,11 +124,13 @@ fn sync_pf_table(ops: &dyn SyncOps, table: &str, log: &Logger) {
     if !add.is_empty() {
         ops.table_push(table, &add);
     }
+    (size, add.len(), del.len())
 }
 
 /// Remove orphaned IPs from the persist file, add missing ones. Set-based, so
 /// file_push's duplicate appends collapse here.
-fn sync_pf_file(ops: &dyn SyncOps, table: &str, file: &Path, log: &Logger) {
+/// Returns what was added and removed.
+fn sync_pf_file(ops: &dyn SyncOps, table: &str, file: &Path, log: &Logger) -> (usize, usize) {
     // File before Redis, for the reason given on sync_pf_table
     let content = match std::fs::read_to_string(file) {
         Ok(c) => c,
@@ -113,7 +139,7 @@ fn sync_pf_file(ops: &dyn SyncOps, table: &str, file: &Path, log: &Logger) {
                 "Failed to read {}, skipping this sync: {e}",
                 file.display()
             ));
-            return;
+            return (0, 0);
         }
     };
     let db_ips = match ops.db_ips(table) {
@@ -122,7 +148,7 @@ fn sync_pf_file(ops: &dyn SyncOps, table: &str, file: &Path, log: &Logger) {
             log.error(&format!(
                 "Failed to read Redis for {table}, skipping this sync: {e}"
             ));
-            return;
+            return (0, 0);
         }
     };
     let f_set: BTreeSet<&str> = content
@@ -150,6 +176,7 @@ fn sync_pf_file(ops: &dyn SyncOps, table: &str, file: &Path, log: &Logger) {
             ));
         }
     }
+    (add.len(), del.len())
 }
 
 /// One full cycle for one address family's table and file.
@@ -157,11 +184,27 @@ pub fn run_cycle(ops: &dyn SyncOps, table: &str, file: &Path, log: &Logger) {
     if log.verbose {
         log.info(&format!("Scan for expiring {table} IPs"));
     }
-    scan_redis_db(ops, table, log);
+    let mut worked = Worked {
+        expired: scan_redis_db(ops, table, log),
+        ..Default::default()
+    };
     // Each reconciliation reads its own Redis snapshot, after the store it
     // may delete from
-    sync_pf_table(ops, table, log);
-    sync_pf_file(ops, table, file, log);
+    let (size, table_added, table_removed) = sync_pf_table(ops, table, log);
+    (worked.size, worked.table_added, worked.table_removed) = (size, table_added, table_removed);
+    (worked.file_added, worked.file_removed) = sync_pf_file(ops, table, file, log);
+
+    if worked.changed() {
+        log.info(&format!(
+            "{table}: {} addresses, {} expired, table +{}/-{}, file +{}/-{}",
+            worked.size,
+            worked.expired,
+            worked.table_added,
+            worked.table_removed,
+            worked.file_added,
+            worked.file_removed
+        ));
+    }
 }
 
 /// Run cycles every `scan_period` seconds until `term`, sleeping in one-second
@@ -410,4 +453,72 @@ mod tests {
         term.store(true, Ordering::Relaxed);
         handle.join().unwrap();
     }
+
+    #[test]
+    fn a_cycle_that_changed_nothing_logs_nothing() {
+        // INFO stays quiet in a steady state
+        let quiet = Worked {
+            size: 142,
+            ..Default::default()
+        };
+        assert!(!quiet.changed());
+        for worked in [
+            Worked {
+                expired: 1,
+                ..Default::default()
+            },
+            Worked {
+                table_added: 1,
+                ..Default::default()
+            },
+            Worked {
+                table_removed: 1,
+                ..Default::default()
+            },
+            Worked {
+                file_added: 1,
+                ..Default::default()
+            },
+            Worked {
+                file_removed: 1,
+                ..Default::default()
+            },
+        ] {
+            assert!(worked.changed());
+        }
+    }
+
+    #[test]
+    fn the_summary_counts_every_store_the_cycle_touched() {
+        let ops = FakeOps {
+            expired: vec![Expired {
+                ip: "1.1.1.1".into(),
+                qname: "one.one.one.one.".into(),
+            }],
+            db: vec!["1.0.0.1".into(), "8.8.8.8".into()],
+            table: vec!["1.0.0.1".into(), "9.9.9.9".into()],
+            ..Default::default()
+        };
+        let dir = tempfile::tempdir().unwrap();
+        let file = dir.path().join("ipv4_domains");
+        std::fs::write(&file, "1.0.0.1\n203.0.113.9\n").unwrap();
+        run_cycle(&ops, "t4", &file, &log());
+
+        // 8.8.8.8 was missing from the table, 9.9.9.9 orphaned in it
+        assert_eq!(
+            *ops.pushed.lock().unwrap(),
+            vec![vec!["8.8.8.8".to_string()]]
+        );
+        assert_eq!(
+            *ops.popped.lock().unwrap(),
+            vec![vec!["9.9.9.9".to_string()]]
+        );
+        let published: Vec<String> = std::fs::read_to_string(&file)
+            .unwrap()
+            .lines()
+            .map(str::to_string)
+            .collect();
+        assert!(published.contains(&"8.8.8.8".to_string()));
+        assert!(!published.contains(&"203.0.113.9".to_string()));
+    }
 }
diff --git a/tests/openbsd/run.sh b/tests/openbsd/run.sh
index a046920..dcd1275 100755
--- a/tests/openbsd/run.sh
+++ b/tests/openbsd/run.sh
@@ -100,6 +100,7 @@ since_mark() { sed -n "/$MARK/,\$p" $DAEMON_LOG; }
 logged() { since_mark | grep -q "$1"; }
 
 redis_up() { [ "$(redis-cli ping 2>/dev/null)" = PONG ]; }
+redis_down() { ! redis_up; }
 
 in_table() { pfctl -t "$1" -T show 2>/dev/null | tr -d ' ' | grep -qx "$2"; }
 not_in_table() { ! in_table "$@"; }
@@ -334,6 +335,13 @@ run_pass() {
 	resolve $NAME >/dev/null
 	retry 10 "the Redis record did not flip to kind cache" redis_kind_is $TABLE4 "$ip" cache
 
+	# The ack precedes the store writes, so an ack with no total is a worker
+	# stuck in one
+	acks=$(since_mark | grep -c "latency microsecs to ack=" || true)
+	totals=$(since_mark | grep -c "latency microsecs total=" || true)
+	[ "$acks" = "$totals" ] \
+		|| fail "$acks messages acknowledged but $totals finished; a worker is stuck in a store"
+
 	# The sync loop must have read the table by now
 	sleep 6
 	logged "Scan for expiring $TABLE4" || fail "the sync loop did not run"
@@ -361,6 +369,9 @@ expiry_pass() {
 	retry 30 "$CRAFTED_IP is still in $PERSIST4" not_in_persist $PERSIST4 $CRAFTED_IP
 	logged "TTL expired from $TABLE4: $CRAFTED_IP (expiry.pfui-e2e.)" \
 		|| fail "the expiry was not logged with its domain"
+	logged "$TABLE4: .* expired, table " \
+		|| fail "the cycle that expired an address logged no summary"
+	since_mark | grep "$TABLE4: .* expired, table " | tail -1
 	rcctl stop pfui_firewall >/dev/null 2>&1 || fail "pfui_firewall did not stop"
 	[ ! -e $SOCK ] || fail "$SOCK was left behind after stop"
 	note "ok"
@@ -385,6 +396,73 @@ reload_pass() {
 	note "ok"
 }
 
+upgrade_pass() {
+	step "pass upgrade: the installer restarts a running daemon and keeps its config"
+	clear_state
+	write_daemon_config tcp IOCTL
+	mark upgrade
+	start_daemon tcp
+	send_crafted 127.0.0.1:10001 before.pfui-e2e. $CRAFTED_IP 3600 \
+		|| fail "the daemon was not serving before the upgrade"
+	before=$(pgrep -f /usr/local/sbin/pfui_firewall | sort | tr '\n' ' ')
+	cp /etc/pfui_firewall.yml /tmp/pfui-config-before-upgrade.yml
+
+	{ rc=0; bash "$ROOT/install-server-rust.sh" || rc=$?; echo "$rc" > /tmp/rc.upgrade; } 2>&1 \
+		| tee /tmp/install-upgrade.log
+	[ "$(cat /tmp/rc.upgrade)" = 0 ] || fail "the installer failed on an upgrade"
+	grep -q "Stopping pfui_firewall to upgrade it" /tmp/install-upgrade.log \
+		|| fail "the installer did not stop the running daemon"
+	grep -q "running the version just installed" /tmp/install-upgrade.log \
+		|| fail "the installer did not report the daemon restarted"
+
+	retry 20 "pfui_firewall is not running after the upgrade" rcctl check pfui_firewall
+	after=$(pgrep -f /usr/local/sbin/pfui_firewall | sort | tr '\n' ' ')
+	[ -n "$after" ] || fail "no pfui_firewall process after the upgrade"
+	[ "$before" != "$after" ] || fail "the daemon was never restarted (pids $before)"
+	cmp -s /tmp/pfui-config-before-upgrade.yml /etc/pfui_firewall.yml \
+		|| fail "the upgrade replaced /etc/pfui_firewall.yml"
+	send_crafted 127.0.0.1:10001 after.pfui-e2e. $CRAFTED_IP 3600 \
+		|| fail "the daemon is not serving after the upgrade"
+	note "ok"
+}
+
+# A daemon the operator had stopped must stay stopped.
+no_start_pass() {
+	step "pass upgrade: a stopped daemon is left stopped"
+	clear_state
+	write_daemon_config tcp IOCTL
+	if rcctl check pfui_firewall >/dev/null 2>&1; then
+		fail "the daemon is running before this pass"
+	fi
+	{ rc=0; bash "$ROOT/install-server-rust.sh" || rc=$?; echo "$rc" > /tmp/rc.nostart; } 2>&1 \
+		| tee /tmp/install-nostart.log
+	[ "$(cat /tmp/rc.nostart)" = 0 ] || fail "the installer failed on a stopped host"
+	! grep -q "Stopping pfui_firewall" /tmp/install-nostart.log \
+		|| fail "the installer stopped a daemon that was not running"
+	! rcctl check pfui_firewall >/dev/null 2>&1 \
+		|| fail "the installer started a daemon the operator had stopped"
+	grep -q "Start service" /tmp/install-nostart.log \
+		|| fail "the installer did not advise how to start it"
+	note "ok"
+}
+
+# rc_pre starts Redis when REDIS_HOST is local.
+redis_dependency_pass() {
+	step "pass redis: starting the daemon starts a stopped local Redis"
+	clear_state
+	write_daemon_config tcp IOCTL
+	mark redis-dep
+	rcctl stop redis >/dev/null 2>&1 || fail "cannot stop redis"
+	retry 10 "redis is still answering after a stop" redis_down
+	rcctl start pfui_firewall || fail "pfui_firewall did not start"
+	retry 20 "rc.d did not start redis" redis_up
+	retry 20 "pfui_firewall is not running" rcctl check pfui_firewall
+	send_crafted 127.0.0.1:10001 redis.pfui-e2e. $CRAFTED_IP 3600 \
+		|| fail "the daemon is not serving"
+	retry 10 "$CRAFTED_IP was not recorded in Redis" in_redis $TABLE4 $CRAFTED_IP
+	note "ok"
+}
+
 fail_loud_pass() {
 	step "pass fail-loud: a table missing from the ruleset refuses to start under IOCTL"
 	clear_state
@@ -408,12 +486,18 @@ rust_tests() {
 	cd "$ROOT/server-rust"
 	export CARGO_HOME=$BUILD_ROOT/cargo
 	export CARGO_TARGET_DIR=$BUILD_ROOT/target
-	# Streamed and bounded, so a hang is named rather than silent
-	{ rc=0; timeout 900 cargo test --release --locked || rc=$?; echo "$rc" > /tmp/rc.cargo; } 2>&1 \
-		| grep -vE '^\s+(Compiling|Downloaded|Downloading)' | tee /tmp/cargo-test.log
-	[ "$(cat /tmp/rc.cargo)" = 0 ] || fail "cargo test failed (exit $(cat /tmp/rc.cargo); 124 is the timeout)"
+	# Timed separately: the test crates pull in dev-dependencies the daemon build
+	# does not, and compiling them dominates this step in a VM
+	{ rc=0; timeout 1800 cargo test --release --locked --no-run || rc=$?
+	  echo "$rc" > /tmp/rc.build; } 2>&1 | tee /tmp/cargo-build.log
+	[ "$(cat /tmp/rc.build)" = 0 ] \
+		|| fail "compiling the tests failed (exit $(cat /tmp/rc.build); 124 is the timeout)"
+	{ rc=0; timeout 600 cargo test --release --locked || rc=$?
+	  echo "$rc" > /tmp/rc.cargo; } 2>&1 | tee /tmp/cargo-test.log
+	[ "$(cat /tmp/rc.cargo)" = 0 ] \
+		|| fail "cargo test failed (exit $(cat /tmp/rc.cargo); 124 is the timeout)"
 	{ rc=0; PFUI_TEST_TABLE4=$TABLE4 PFUI_TEST_TABLE6=$TABLE6 \
-		timeout 300 cargo test --release --locked --test pf_ioctl_live -- --ignored --test-threads=1 \
+		timeout 600 cargo test --release --locked --test pf_ioctl_live -- --ignored --test-threads=1 \
 		|| rc=$?; echo "$rc" > /tmp/rc.live; } 2>&1 | tee /tmp/cargo-live.log
 	[ "$(cat /tmp/rc.live)" = 0 ] || fail "the live ioctl suite failed (exit $(cat /tmp/rc.live))"
 	pfctl -t $TABLE4 -T flush >/dev/null 2>&1 || true
@@ -427,9 +511,13 @@ latency_summary() {
 	grep -h "latency microsecs" $DAEMON_LOG | tail -8 || true
 	grep -h "Query Unblocked" $DAEMON_LOG | tail -4 || true
 	grep -h "latency microsecs" $DAEMON_LOG | awk '
-		/to ack=/ { sub(/.*to ack=/, ""); ack = $1 + 0; have = 1; next }
-		/total=/  { if (have) { sub(/.*total=/, ""); if (ack > $1 + 0) bad++; n++; have = 0 } }
-		END { printf "    pairs=%d out_of_order=%d\n", n, bad; exit (bad > 0) }' \
+		/to ack=/ { sub(/.*to ack=/, ""); ack[++a] = $1 + 0; next }
+		/total=/  { sub(/.*total=/, ""); tot[++t] = $1 + 0 }
+		END {
+			for (i = 1; i <= a && i <= t; i++) if (ack[i] > tot[i]) bad++
+			printf "    acks=%d totals=%d out_of_order=%d\n", a, t, bad
+			exit (bad > 0)
+		}' \
 		|| fail "an acknowledgement was logged after its total"
 }
 
@@ -472,8 +560,11 @@ for ctl in IOCTL PFCTL; do
 done
 expiry_pass
 reload_pass
+upgrade_pass
+no_start_pass
+redis_dependency_pass
 fail_loud_pass
 latency_summary
 stop_all
 
-step "complete: $PASSES transport/control passes, expiry, reload and fail-loud all passed"
+step "complete: $PASSES transport/control passes, plus expiry, reload, upgrade and fail-loud"