🦄 Sync 2026-10-17 20:40:04

luci-app-homeproxy-pro/Makefile

@@ -41,7 +41,7 @@ LUCI_DEPENDS:= \
 
 PKG_NAME:=luci-app-homeproxy-pro
 PKG_VERSION:=28.10.1.14
-PKG_RELEASE:=28
+PKG_RELEASE:=29
 
 LUCI_BASENAME:=homeproxy-pro
 

luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/runtime/health.sh

@@ -328,13 +328,37 @@ hp_probe_dns() {
 # hp_probe_tcp <port>
 # 0 something accepted a connection on loopback:<port>, 1 nothing did,
 # 2 cannot probe here.
+#
+# This probe has to prove an accept WITHOUT leaving the connection hanging
+# around, because it runs inside service_started() while the reload still holds
+# the procd lock - a probe that does not return blocks every later status and
+# reload behind `flock`.
+#
+# A mixed/http/socks inbound accepts and then waits for a real proxy request,
+# so it never closes the connection.  GNU netcat keeps the socket open after
+# stdin EOF unless --close is given, and -w does NOT bound that loop: its wait
+# is spent on the connect (core_readwrite takes its select budget from -i), so
+# the bare probe blocked for as long as the operator left the router alone.
+# Measured with a real GNU netcat 0.7.1 against an inbound that accepts and
+# holds: bare and `-w 3` both hung past 8s, `-c` returned 0 in 1s.
+#
+# A nc that advertises no --close is busybox, and busybox must be invoked bare:
+# it accepts NO options at all on ImmortalWrt 25.12.2 (busybox 1.37.0), where
+# -z/-w/-c all fail with a usage error, so passing -w here would turn a working
+# probe into a warning on every reload - and into a rollback loop for anyone
+# who sets health_probe_strict.  busybox exits on stdin EOF on its own, so the
+# bare form already returns.
 hp_probe_tcp() {
 	local port="$1"
 
 	[ -n "$port" ] || return 2
 	command -v nc > "/dev/null" 2>&1 || return 2
 
-	nc 127.0.0.1 "$port" < "/dev/null" > "/dev/null" 2>&1 && return 0
+	if nc -h 2>&1 | grep -q -- '--close'; then
+		nc -c 127.0.0.1 "$port" < "/dev/null" > "/dev/null" 2>&1 && return 0
+	else
+		nc 127.0.0.1 "$port" < "/dev/null" > "/dev/null" 2>&1 && return 0
+	fi
 
 	return 1
 }

luci-app-homeproxy-pro/tests/README.md

@@ -297,7 +297,7 @@ Two further host differences are bridged so the remaining checks still run:
 | `tests/lib/luci-module.js` | The off-target LuCI module loader (`'require x as y';` rewritten into a dependency lookup), shared by the snapshot renderer and the frontend invariant tests. Its default `_()` returns a LuCI String object with `.format`, because the form code calls both halves. |
 | `tests/runtime/test_config_transaction.sh` | The `runtime/` helpers `init.d/homeproxy-pro` leans on: the known-good copy, the fallback when generation produced nothing, the rollback, and the health probe. Pure shell, runs anywhere. |
 | `tests/runtime/test_dns_snippets.sh` | The dnsmasq snippet writer's incremental behaviour (review L6). Counts `init.d/dnsmasq restart` calls through a stub and asserts that an identical second write does *not* restart (a restart flushes every client's DNS cache), while a changed resource list, routing mode, DNS port or `ipv6_support` setting does. Also pins that a mode change drops the snippet the previous mode produced, and that removing a resource list removes its snippet so the next call is stable again. Pure shell, runs anywhere. |
-| `tests/runtime/test_health_probe.sh` | The functional health probes (`runtime/health.sh`): `hp_probe_dns` / `hp_probe_tcp` answer 0 / 1 / 2 and keep "cannot probe here" (no tool, no port) distinct from "the probe failed"; `hp_run_probes` *reports* by default and only fails the gate when `health_probe_strict` is on, and never fails it for a probe that could not run; `hp_config_port_for_tag` reads the probe ports out of the running configuration in both the generator's pretty shape and the compact one. `nslookup` and `nc` are stubbed, so it is pure shell and runs anywhere. |
+| `tests/runtime/test_health_probe.sh` | The functional health probes (`runtime/health.sh`): `hp_probe_dns` / `hp_probe_tcp` answer 0 / 1 / 2 and keep "cannot probe here" (no tool, no port) distinct from "the probe failed"; `hp_run_probes` *reports* by default and only fails the gate when `health_probe_strict` is on, and never fails it for a probe that could not run; `hp_config_port_for_tag` reads the probe ports out of the running configuration in both the generator's pretty shape and the compact one. It also pins that `hp_probe_tcp` *returns* — the probes run inside `service_started()` while the reload holds the procd lock, so one that never finishes blocks every later status and reload behind `flock`, and "failures are only warnings" cannot save a loop. Two `nc` stubs model the flavours it really meets: a GNU netcat that will not answer unless passed `-c` (and that gains nothing from `-w`, whose wait is spent on the connect), and a busybox that accepts no options at all — the shape of ImmortalWrt 25.12.2, where `-z`/`-w`/`-c` are usage errors and a probe that passed them would warn on every reload. `nslookup` and `nc` are stubbed, so it is pure shell and runs anywhere. |
 | `tests/runtime/test_crontab_perms.sh` | `hp_crontab_drop()` (runtime/service.sh) edits `/etc/crontabs/root` through a temporary file, and `mv` replaces the file rather than its contents - so the new one inherited the shell's umask (0644) while procd creates the root crontab as 0600. The test drives the function against a sandboxed crontab and pins: the entry is dropped, other jobs survive, the mode is 0600 (including a file an older version had already loosened), a missing crontab is reported as a failure and is not created. Pure shell, runs anywhere. |
 | `tests/runtime/test_resource_update.sh` | The content-verification path of `scripts/update_resources.sh`: the download URL is pinned to a commit, and the file is installed only when its git blob id matches the one GitHub reports for that path and commit. Drives all four outcomes (match, mismatch, no digest from the API, no local digest) with `uclient-fetch` / `jsonfilter` / `ucode` / `uci` / `flock` stubbed and the script's absolute paths rewritten into a sandbox, asserting in each failure case that the installed list and its `.ver` are left untouched - the point of the check is that it refuses, so a driver that only walked the happy path would keep passing with the check deleted. Also covers the two post-processing steps that carry a failure silently: `china_list`'s normalisation ordering, the address-family split (both IP lists come out of one upstream file, so each has to land at its own `.txt` holding only its own family, the combined download must not be left in `RUN_DIR`, and a family with no entries has to be refused rather than installed empty), and the firewall re-render that closes the loop on the nft half. That last one is the reason a test exists at all: the route side goes live on fswatch while `homeproxy_mainland_addr_v4/v6` are rendered into `fw4_post.nft` and otherwise only a start/restart installs them, so "the list updated" and "the kernel sees it" are different events. Both directions are asserted — a successful update must re-render and call `fw4 reload`, and a failed render must leave the previous ruleset in place, must not reach `fw4`, and must not fail the update (a resource that can never refresh is worse than one briefly missing from the firewall). Pure shell, runs anywhere. |
 | `tests/runtime/test_runtime_extraction.sh` | Drives `init.d/homeproxy-pro` through a stubbed environment (fake `ip` / `nft` / `fw4` / `ucode` / `sing-box` / `uci` / `netstat` / `nslookup` / `nc`, fake procd and jsonfilter state, fixture UCI) across five scenarios and diffs the resulting command + file trace against a baseline. Two baselines exist and are captured with the SAME harness, so the diff between them is exactly the intentional change: `tests/fixtures/runtime/trace.pre-pr05.txt` (the 517-line init script before PHASE 7 moved the plumbing out — the record that the extraction was behaviour-preserving) and `tests/fixtures/runtime/trace.golden.txt` (after the health-gate fix). Scenario D is the P0 regression: a candidate whose `mixed_port` is already taken must be rejected by the health gate, the previous known-good must survive, and the reload must roll back onto it. The generated fixture configuration is pretty-printed like the generator's own `%.J` output, because the runtime reads that file (`hp_config_ports`, `hp_config_port_for_tag`) and a compact fixture would test a shape production never writes. Pure shell, no ucode needed. Regenerate with `HP_UPDATE_GOLDEN=1` (optionally `HP_GOLDEN=` / `HP_INITD=` to target another baseline or revision). |

luci-app-homeproxy-pro/tests/fixtures/runtime/trace.golden.txt

@@ -73,6 +73,7 @@ jsonfilter -e @.running
 jsonfilter -e @.exit_code
 netstat -ltnup
 nslookup -port=5333 example.com 127.0.0.1
+nc -h
 nc 127.0.0.1 5330
 chmod 600 var/run/homeproxy-pro/known-good/sing-box-c.json
 config_load homeproxy-pro
@@ -268,6 +269,7 @@ jsonfilter -e @.running
 jsonfilter -e @.exit_code
 netstat -ltnup
 nslookup -port=5333 example.com 127.0.0.1
+nc -h
 nc 127.0.0.1 5330
 jsonfilter -e @['homeproxy-pro'].instances['sing-box-s']
 jsonfilter -e @.running
@@ -528,6 +530,7 @@ jsonfilter -e @.running
 jsonfilter -e @.exit_code
 netstat -ltnup
 nslookup -port=5333 example.com 127.0.0.1
+nc -h
 nc 127.0.0.1 5330
 chmod 600 var/run/homeproxy-pro/known-good/sing-box-c.json
 config_load homeproxy-pro
@@ -991,6 +994,7 @@ jsonfilter -e @.running
 jsonfilter -e @.exit_code
 netstat -ltnup
 nslookup -port=5333 example.com 127.0.0.1
+nc -h
 nc 127.0.0.1 5330
 log Client is up again; reinstalling the intercept layer.
 log dnsmasq snippets unchanged, skipping the restart.
@@ -1160,6 +1164,7 @@ jsonfilter -e @.running
 jsonfilter -e @.exit_code
 netstat -ltnup
 nslookup -port=5333 example.com 127.0.0.1
+nc -h
 nc 127.0.0.1 5330
 chmod 600 var/run/homeproxy-pro/known-good/sing-box-c.json
 config_load homeproxy-pro
@@ -1698,6 +1703,7 @@ jsonfilter -e @.running
 jsonfilter -e @.exit_code
 netstat -ltnup
 nslookup -port=70000 example.com 127.0.0.1
+nc -h
 nc 127.0.0.1 5398
 log Warning: the DNS snippets were not installed; keeping the previous known-good configuration.
 sing-box version -n
@@ -1878,6 +1884,7 @@ jsonfilter -e @.running
 jsonfilter -e @.exit_code
 netstat -ltnup
 nslookup -port=5333 example.com 127.0.0.1
+nc -h
 nc 127.0.0.1 5330
 jsonfilter -e @['homeproxy-pro'].instances['sing-box-s']
 jsonfilter -e @.running

luci-app-homeproxy-pro/tests/runtime/test_health_probe.sh

@@ -15,6 +15,16 @@
 # gate failure.  "Cannot probe here" stays non-fatal even then - there is
 # nothing to conclude from a missing tool.
 #
+# Reporting correctly is only half of it.  service_started() runs the probes
+# while the reload still holds the procd lock, so a probe that never RETURNS
+# blocks every later status and reload behind `flock` - and no amount of
+# "failures are only warnings" helps a loop that never finishes.  hp_probe_tcp
+# therefore has to survive the two nc flavours it actually meets: one that hangs
+# unless it is told to close on stdin EOF, and one (busybox) that accepts no
+# options at all and would answer a usage error instead of a verdict.  The two
+# stubs below are shaped to fail if the probe is invoked the way that would hang
+# or warn in the field.
+#
 # The probes take explicit ports and shell out to nslookup/nc, so this runs
 # anywhere without ucode, procd or a router.
 #
@@ -107,6 +117,99 @@ expect "tcp: a refusing listener is 1" "$?" "1"
 hp_probe_tcp ""
 expect "tcp: no port means cannot-probe (2), not failure" "$?" "2"
 
+echo "== hp_probe_tcp: the probe has to RETURN, not just succeed =="
+# The probe runs inside service_started() while the reload still holds the procd
+# lock, so a probe that never returns blocks every later status and reload
+# behind `flock`.  A test cannot observe a real hang, so each stub below is
+# built to FAIL if it is invoked the way that would hang in the field:
+#
+#   * the GNU stub refuses to answer unless it is passed -c, which is what the
+#     real GNU netcat needs to leave an inbound that accepts and then holds the
+#     connection open.  It also refuses -w, because -w is spent on the connect
+#     and does not bound that loop - so "fixing" it with -w alone fails here.
+#   * the busybox stub rejects every option the way ImmortalWrt's build does,
+#     so passing -w/-z/-c costs the probe its result.
+#
+# Both record how they were called, so the test pins the invocation and not
+# just the verdict.
+BIN_GNU="$WORK/bin-gnu"
+BIN_BBB="$WORK/bin-busybox"
+mkdir -p "$BIN_GNU" "$BIN_BBB"
+NC_ARGS="$WORK/nc_args"
+
+cat > "$BIN_GNU/nc" <<'EOF'
+#!/bin/sh
+# GNU netcat 0.7.1: advertises --close/--wait, and needs -c to stop watching a
+# socket the peer is holding open.  -w is accepted but does not help.
+case "$1" in
+-h|--help)
+	echo "GNU netcat 0.7.1"
+	echo "        --close                 close on EOF from stdin"
+	echo "        --wait SEC              timeout for connects and final net reads"
+	exit 1
+	;;
+esac
+printf '%s\n' "$*" >> "$HP_PROBE_STUB_DIR/nc_args"
+case "$*" in
+-c\ *) ;;
+*) exit 2 ;;  # without -c the real nc would still be running
+esac
+[ -f "$HP_PROBE_STUB_DIR/tcp_ok" ] || exit 1
+exit 0
+EOF
+cat > "$BIN_BBB/nc" <<'EOF'
+#!/bin/sh
+# ImmortalWrt 25.12.2 (busybox 1.37.0): nc advertises no --close and accepts
+# no option at all - -z/-w/-c/-l are all usage errors.  It exits on stdin EOF
+# by itself, so the bare invocation is both correct and sufficient.
+case "$1" in
+-h|--help)
+	echo "Usage: nc [IPADDR PORT]"
+	exit 1
+	;;
+-*)
+	echo "Usage: nc [IPADDR PORT]"
+	exit 1
+	;;
+esac
+printf '%s\n' "$*" >> "$HP_PROBE_STUB_DIR/nc_args"
+[ -f "$HP_PROBE_STUB_DIR/tcp_ok" ] || exit 1
+exit 0
+EOF
+chmod +x "$BIN_GNU/nc" "$BIN_BBB/nc"
+
+SAVED_PATH="$PATH"
+
+: > "$WORK/tcp_ok"
+: > "$NC_ARGS"
+PATH="$BIN_GNU:$SAVED_PATH"
+hp_probe_tcp 5330
+expect "gnu nc: an accepting listener is 0" "$?" "0"
+expect "gnu nc: it is called with -c, so the probe can return" \
+	"$(head -1 "$NC_ARGS" | cut -d' ' -f1)" "-c"
+expect "gnu nc: the target is the mixed inbound, not the capability probe" \
+	"$(head -1 "$NC_ARGS" | awk '{print $(NF-1), $NF}')" "127.0.0.1 5330"
+
+rm -f "$WORK/tcp_ok"
+: > "$NC_ARGS"
+hp_probe_tcp 5330
+expect "gnu nc: a refusing listener is still 1" "$?" "1"
+
+: > "$WORK/tcp_ok"
+: > "$NC_ARGS"
+PATH="$BIN_BBB:$SAVED_PATH"
+hp_probe_tcp 5330
+expect "busybox nc: an accepting listener is 0" "$?" "0"
+expect "busybox nc: it is called bare, because busybox accepts no options" \
+	"$(head -1 "$NC_ARGS")" "127.0.0.1 5330"
+
+rm -f "$WORK/tcp_ok"
+hp_probe_tcp 5330
+expect "busybox nc: a refusing listener is still 1" "$?" "1"
+
+PATH="$SAVED_PATH"
+export PATH
+
 echo "== missing tooling is 'cannot probe', not 'failed' =="
 # PATH is replaced, not prepended to: the host this runs on may well have its
 # own nslookup/nc (macOS ships both), and a prepended empty directory would