💐 Sync 2026-10-06 12:33:44

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:=22
+PKG_RELEASE:=23
 
 LUCI_BASENAME:=homeproxy-pro
 

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

@@ -601,6 +601,108 @@ hp_capture_candidate() {
 	return 0
 }
 
+# hp_reload_is_noop <run-dir> <good-dir> <client-enabled> <server-enabled>
+# 0 when this reload would change nothing that is currently running, so the
+# caller can skip the stop;start entirely.
+#
+# Why this exists: reload_service used to stop and start unconditionally, and
+# hp_same_file was only ever used on the rollback path.  Measured on the device,
+# one reload costs about eight seconds (09:17:40 -> 09:17:48 in the log) and
+# interrupts every connection, even when the generated configuration is
+# byte-identical to the one already running.  That is the common case for
+# "Save & Apply" with nothing edited, for a node switch that lands on the same
+# node, and for the nightly resource refresh.
+#
+# THE COMPARISON IS AGAINST known-good, NOT AGAINST THE CANDIDATE, and getting
+# that wrong silently disables the whole mechanism.  Measured on the device:
+# the generator writes straight to $RUN_DIR/sing-box-c.json and
+# hp_capture_candidate() copies that same file afterwards, so after a preflight
+# the candidate and the live file are the SAME BYTES BY CONSTRUCTION - comparing
+# them reports "unchanged" for every reload, including one that switched the main
+# node.  The first version of this helper did exactly that, and the only reason
+# it was caught is an on-device test that switched nodes and watched the PID.
+#
+# known-good is the right counterpart for the same reason the rollback path uses
+# it: hp_promote_known_good writes it ONLY after the health gate has passed, so
+# it is by definition "the configuration that is currently running and working".
+# live vs known-good answers the question this helper is actually asking - "would
+# the running configuration change?" - and a missing known-good simply falls
+# through to a real reload.
+#
+# Every condition is a *precondition for the skip being safe*, and the default
+# on any doubt is to reload:
+#
+#   [1] the service is actually running.  A live file left over from a crashed
+#       instance is not a running service, and skipping the restart would
+#       report success over a dead proxy.  Asked of ubus, the same source the
+#       status page reads.
+#   [2] the side has both a live file and a known-good copy to compare against.
+#       A side the user just enabled has neither, and that is a change, not a
+#       no-op.
+#   [3] the intercept layer is installed.  A released layer (the marker
+#       hp_release_stale_intercept writes) means the firewall is NOT proxying
+#       right now even though sing-box is up.  Skipping the restart would leave
+#       it that way and log "nothing to restart" - the exact silent state
+#       hp_rearm_intercept exists to undo.  Reloading is what re-arms it.
+hp_reload_is_noop() {
+	local run_dir="$1"
+	local good_dir="$2"
+	local client_enabled="$3"
+	local server_enabled="$4"
+	local live good
+
+	# [3] first, because it is the cheapest and the most dangerous to skip.
+	[ ! -f "$run_dir/intercept-released" ] || return 1
+
+	# [1] the instances.  `command -v ubus` because this module is also sourced
+	# by the off-target test driver, where ubus does not exist and the honest
+	# answer there is "cannot prove it is a no-op".
+	#
+	# The check is on `"running": true`, NOT on the instance merely appearing
+	# in the list: procd keeps a stopped instance in `service list` with
+	# running=false, so matching the key alone reports a dead proxy as running
+	# and the skip then claims success over it.  That was not theoretical - the
+	# first version of this grep did exactly that and its own test caught it.
+	command -v ubus > "/dev/null" 2>&1 || return 1
+
+	local svc_list
+	svc_list="$(ubus call service list "{\"name\":\"$CONF\"}" 2>/dev/null)"
+
+	if [ "$client_enabled" = "1" ]; then
+		printf '%s' "$svc_list" \
+			| grep -A 3 '"sing-box-c"' \
+			| grep -q '"running"[[:space:]]*:[[:space:]]*true' || return 1
+	fi
+
+	if [ "$server_enabled" = "1" ]; then
+		printf '%s' "$svc_list" \
+			| grep -A 3 '"sing-box-s"' \
+			| grep -q '"running"[[:space:]]*:[[:space:]]*true' || return 1
+	fi
+
+	# [2] and the byte comparison, per side.
+	# [2] and the byte comparison, per side, against what is known to be running
+	# rather than against the candidate (see the note at the top of this
+	# function - the two are the same bytes by construction).
+	if [ "$client_enabled" = "1" ]; then
+		live="$run_dir/sing-box-c.json"
+		good="$good_dir/sing-box-c.json"
+
+		[ -s "$live" ] && [ -s "$good" ] || return 1
+		cmp -s "$live" "$good" 2>"/dev/null" || return 1
+	fi
+
+	if [ "$server_enabled" = "1" ]; then
+		live="$run_dir/sing-box-s.json"
+		good="$good_dir/sing-box-s.json"
+
+		[ -s "$live" ] && [ -s "$good" ] || return 1
+		cmp -s "$live" "$good" 2>"/dev/null" || return 1
+	fi
+
+	return 0
+}
+
 # hp_start_generated_config <side> <hp-dir> <run-dir> <good-dir>
 # The start-path transaction for one side ("c" or "s"):
 #

luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/update_resources.sh

@@ -248,15 +248,41 @@ check_list_update() {
 			# china_ip4.json exists: one list with two readers, and nothing for
 			# a cold start to download before the inbounds bind.
 			#
-			# The sed above rewrote the file in place, so this runs after it
-			# and sees the cleaned list.  `full:` prefixes and anything with a
-			# colon are gone by now; what is left is what the helper accepts.
-			if ucode -S "$SCRIPT_DIR/runtime/domain_ruleset.uc" \
-				"$RESOURCES_DIR/$listtype.txt" "$RESOURCES_DIR/china-domain.json" >>"$LOG_PATH" 2>&1; then
-				log "[CHINA_LIST] DNS-side rule-set regenerated."
-				chown sing-box:sing-box "$RESOURCES_DIR/china-domain.json" 2>"/dev/null"
+			# Normalise FIRST, generate second - and both inside this branch.
+			# The order is load-bearing, and it was previously wrong: the sed
+			# lived in the `case "china_list"` arm, i.e. after this function had
+			# already returned, so the generator was handed the RAW download.
+			# That is not a theoretical difference.  Upstream ships `full:`
+			# prefixes and the colon-carrying `regexp:` / `keyword:` forms
+			# still in the file - 562 of 111,361 lines in the release measured
+			# here - and domain_ruleset.uc rejects every entry containing a
+			# colon, so 554 real domains were dropped from the DNS split and
+			# the only trace was a "skipped N malformed entries" line naming a
+			# list that was perfectly well formed.
+			#
+			# Not `sed -i`: that form is a busybox/GNU extension, and the same
+			# script is exercised off-device where a non-busybox sed fails it
+			# with "invalid command code".  Edit through a temp file, the way
+			# the crontab helper in runtime/service.sh does.
+			#
+			# A normalisation that fails does not undo the install - the list
+			# on disk is the one the upstream commit vouched for, and the
+			# firewall renders its nft set from that very file.  The generator
+			# is skipped instead, so the DNS side keeps the previous rule-set
+			# rather than being rebuilt from a list the sed only half-processed.
+			if sed -e "s/full://g" -e "/:/d" "$RESOURCES_DIR/$listtype.txt" \
+				> "$RESOURCES_DIR/$listtype.txt.hp-new" \
+				&& mv -f "$RESOURCES_DIR/$listtype.txt.hp-new" "$RESOURCES_DIR/$listtype.txt"; then
+				if ucode -S "$SCRIPT_DIR/runtime/domain_ruleset.uc" \
+					"$RESOURCES_DIR/$listtype.txt" "$RESOURCES_DIR/china-domain.json" >>"$LOG_PATH" 2>&1; then
+					log "[CHINA_LIST] DNS-side rule-set regenerated."
+					chown sing-box:sing-box "$RESOURCES_DIR/china-domain.json" 2>"/dev/null"
+				else
+					log "[CHINA_LIST] Warning: could not regenerate china-domain.json (list has no usable domain entry?); the DNS side keeps the previous list."
+				fi
 			else
-				log "[CHINA_LIST] Warning: could not regenerate china-domain.json (list has no usable domain entry?); the DNS side keeps the previous list."
+				rm -f "$RESOURCES_DIR/$listtype.txt.hp-new"
+				log "[CHINA_LIST] Warning: could not normalise the downloaded list; it is installed as downloaded and the DNS side keeps the previous rule-set."
 			fi
 			;;
 		esac
@@ -280,10 +306,13 @@ case "$1" in
 	check_list_update "$1" "Loyalsoldier/v2ray-rules-dat" "release" "gfw.txt"
 	;;
 "china_list")
-	# Not `sed -i`: the bare -i form is a busybox/GNU extension, and the same
-	# script is exercised off-device where a non-busybox sed fails it with
-	# "invalid command code".  Edit through a temp file, the way the crontab
-	# helper in runtime/service.sh does.
+	# The `full:` prefixes and the colon-carrying regexp:/keyword: forms are
+	# stripped inside check_list_update, BEFORE the DNS-side rule-set is
+	# generated from the list - domain_ruleset.uc rejects any entry containing
+	# a colon, so the order decides whether 554 real domains are in the split
+	# or silently skipped.  See the case branch there; this arm is only the
+	# list's own upstream coordinates.
+	#
 	# The status has to survive the post-processing.  As one &&...|| chain it
 	# did not: when check_list_update returned 3 ("already current") or 1
 	# (fetch failed) the chain short-circuited into `rm -f`, whose status 0
@@ -293,18 +322,7 @@ case "$1" in
 	# Measured on the device: china_list exited 0 for both the failure and the
 	# up-to-date case, while china_ip4 exited 1 / 3 correctly.
 	check_list_update "$1" "Loyalsoldier/v2ray-rules-dat" "release" "direct-list.txt"
-	rc=$?
-	[ "$rc" = "0" ] || exit "$rc"
-
-	# 0 means a new list was just installed; strip the `full:` prefixes and the
-	# colon-carrying entries before anything else reads it.
-	if sed -e "s/full://g" -e "/:/d" "$RESOURCES_DIR/china_list.txt" > "$RESOURCES_DIR/china_list.txt.hp-new" \
-		&& mv -f "$RESOURCES_DIR/china_list.txt.hp-new" "$RESOURCES_DIR/china_list.txt"; then
-		exit 0
-	fi
-	rm -f "$RESOURCES_DIR/china_list.txt.hp-new"
-	log "[CHINA_LIST] Failed to normalise the downloaded list; leaving it as downloaded."
-	exit 1
+	exit $?
 	;;
 *)
 	printf '%s\n' "Usage: $0 <china_ip4 / china_ip6 / gfw_list / china_list>"

luci-app-homeproxy-pro/root/etc/homeproxy-pro/scripts/update_subscriptions.uc

@@ -77,10 +77,32 @@ import { Loader } from './config/loader.uc';
  * A2: the run performs exactly one uci.commit(). The Repository modules stage
  * mutations only.
  *
- * UCICONFIG_DIR (homeproxy-pro.uc) is the config *directory* Loader.load() takes,
- * and the sandboxed test points it at its own config instead of /etc/config
- * (tests/ucode/test_subscription_updater_runs.sh rewrites that line in
- * homeproxy-pro.uc and fails loudly if the rewrite did not apply). */
+ * UCICONFIG_DIR (homeproxy-pro.uc) is the config *directory* this script reads
+ * and writes, and the sandboxed test points it at its own config instead of
+ * /etc/config (tests/ucode/test_subscription_updater_runs.sh rewrites that
+ * line in homeproxy-pro.uc and fails loudly if the rewrite did not apply).
+ *
+ * That rewrite is only half the sandbox, and the half that was missing is the
+ * dangerous one.  `cursor()` with no argument is a cursor on the REAL
+ * /etc/config, not on UCICONFIG_DIR - the constant and the cursor were never
+ * connected.  Measured on the device:
+ *
+ *   cursor()            -> reads infra, config, control, subscription, and the
+ *                          four live node sections
+ *   cursor('/tmp/t8cfg') -> reads nothing
+ *
+ * So the updater was reading the live configuration whatever the test staged,
+ * and `uci.commit(uciconfig)` - the one commit this script performs, at the
+ * end of a successful run - would have written the LIVE /etc/config/homeproxy-pro.
+ * The committed test only ever drove a run whose fetch failed before that
+ * commit, which is why nothing had touched the device yet.  Any future case
+ * that lets a fetch succeed would have committed the test's sandbox nodes over
+ * the user's real configuration.
+ *
+ * Hence the same `dir` injection Loader.load() already uses (loader.uc:385):
+ * the cursor is built on UCICONFIG_DIR, so the constant the test already
+ * rewrites is finally the thing that decides where both the read and the write
+ * land.  Production is unchanged - UCICONFIG_DIR is /etc/config there. */
 const CONFIG_FILE = UCICONFIG_DIR + '/homeproxy-pro';
 const uciconfig = 'homeproxy-pro';
 
@@ -97,7 +119,10 @@ let uci, loaded, sub, routing_mode,
 /* Read everything the run needs, under the lock. Read-only: nothing here
  * mutates, so a failure cannot leave a partial state behind. */
 function load_locked_state() {
-	uci = cursor();
+	/* On UCICONFIG_DIR, not on the default.  See the note on CONFIG_FILE:
+	 * a bare cursor() here reads - and, at the end of a successful run,
+	 * commits - the live /etc/config, no matter what the test staged. */
+	uci = cursor(UCICONFIG_DIR);
 	uci.load(uciconfig);
 
 	/* The orchestrator used to hold its own

luci-app-homeproxy-pro/root/etc/init.d/homeproxy-pro

@@ -605,6 +605,27 @@ reload_service() {
 	# same way in the rollback path below.
 	export HP_CANDIDATE_CLIENT HP_CANDIDATE_SERVER
 
+	# Nothing to do?  The candidates are byte-identical to what is already
+	# running and the running instance is healthy, so the stop;start below
+	# would interrupt every connection to install a file that is already
+	# installed.  Checked AFTER the preflight, never before: the point is to
+	# skip a restart, not to skip the validation - a configuration that fails
+	# `sing-box check` has to be reported as such whether or not it would have
+	# been a no-op, and the candidate has to exist for anything to compare.
+	#
+	# Every "cannot prove it is a no-op" case falls through to the normal
+	# reload, so the worst outcome of this helper is the behaviour that
+	# existed before it.
+	#
+	# `HP_CANDIDATE_CLIENT` is the same flag the preflight set, so the two
+	# sides cannot disagree about which configuration is being activated.
+	if [ "$HP_CANDIDATE_CLIENT" = "1" ] || [ "$HP_CANDIDATE_SERVER" = "1" ]; then
+		if hp_reload_is_noop "$RUN_DIR" "$GOOD_DIR" "$HP_CANDIDATE_CLIENT" "$HP_CANDIDATE_SERVER"; then
+			log "Configuration is unchanged and the service is healthy; nothing to restart."
+			return 0
+		fi
+	fi
+
 	# Both configurations passed `sing-box check`; now it is safe to restart.
 	# start_service owns the rest of the transaction: it proves the instances
 	# came up and only then records the new known-good, so the previous one

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

@@ -378,5 +378,122 @@ reset_samples
 hp_wait_instance "sing-box-c" "$CFG" 1
 expect "wait_instance (compat): times out when down" "$?" "1"
 
+# --- hp_reload_is_noop -----------------------------------------------------
+# The reload short circuit.  Every case below is a reason the reload must NOT
+# be skipped, which is the direction that matters: a wrong skip reports success
+# over a dead or half-installed proxy, while a needless reload only costs the
+# eight seconds it was going to cost anyway.
+#
+# The comparison is live vs known-good.  It is NOT candidate vs live: the
+# generator writes straight to the live path and hp_capture_candidate copies
+# that same file afterwards, so those two are identical by construction and
+# comparing them reports "unchanged" for every reload - including one that
+# switched the main node.  That was a real defect, caught on the device; the
+# case below pins the difference so it cannot come back.
+#
+# The helper reads the filesystem and ubus.  ubus is stubbed on PATH so the "is
+# the instance running" question has an answer off-target; the absence of that
+# stub is itself one of the cases, because an environment where the state
+# cannot be established has to fall through to a real reload.
+CONF="$WORK/noop"
+GOOD="$WORK/noop-known-good"
+mkdir -p "$CONF/candidate" "$GOOD"
+printf 'running bytes\n' > "$CONF/sing-box-c.json"
+printf 'running bytes\n' > "$GOOD/sing-box-c.json"
+# The candidate is deliberately identical to the live file: that is what a
+# preflight leaves behind, and comparing against it must NOT produce a no-op.
+printf 'running bytes\n' > "$CONF/candidate/sing-box-c.json"
+
+mkdir -p "$WORK/noop-bin"
+cat > "$WORK/noop-bin/ubus" <<'EOF'
+#!/bin/sh
+# The service list reply, in the shape the helper greps for.
+cat <<JSON
+{"homeproxy-pro":{"instances":{"sing-box-c":{"running":true,"pid":1234}}}}
+JSON
+EOF
+chmod +x "$WORK/noop-bin/ubus"
+# shellcheck source=/dev/null
+. "$RUNTIME/service.sh"
+
+PATH="$WORK/noop-bin:$PATH"
+export PATH
+
+hp_reload_is_noop "$CONF" "$GOOD" 1 0
+expect "reload_is_noop: live matches known-good, client running, no released intercept" "$?" "0"
+
+# The configuration changed - the one case where a reload is genuinely needed.
+# This is the case the candidate-based version got wrong: the candidate is
+# updated in lockstep with the live file, so only known-good tells the truth.
+printf 'switched node\n' > "$CONF/sing-box-c.json"
+printf 'switched node\n' > "$CONF/candidate/sing-box-c.json"
+hp_reload_is_noop "$CONF" "$GOOD" 1 0
+expect "reload_is_noop: live differs from known-good still reloads" "$?" "1"
+expect "reload_is_noop: ...even though candidate matches live byte for byte" \
+	"$(cmp -s "$CONF/candidate/sing-box-c.json" "$CONF/sing-box-c.json" && echo same)" "same"
+printf 'running bytes\n' > "$CONF/sing-box-c.json"
+
+# No known-good copy: the side has never come up healthy, or this is a fresh
+# install.  "Cannot prove it is a no-op" has to mean reload.
+mv "$GOOD/sing-box-c.json" "$GOOD/staged"
+hp_reload_is_noop "$CONF" "$GOOD" 1 0
+expect "reload_is_noop: no known-good copy still reloads" "$?" "1"
+mv "$GOOD/staged" "$GOOD/sing-box-c.json"
+
+# The intercept layer was released: sing-box is up but the LAN is not proxied.
+# Skipping here is the silent state hp_rearm_intercept exists to undo.
+: > "$CONF/intercept-released"
+hp_reload_is_noop "$CONF" "$GOOD" 1 0
+expect "reload_is_noop: a released intercept layer still reloads" "$?" "1"
+rm -f "$CONF/intercept-released"
+
+# An empty live file is not a running configuration.
+: > "$CONF/sing-box-c.json"
+hp_reload_is_noop "$CONF" "$GOOD" 1 0
+expect "reload_is_noop: an empty live file still reloads" "$?" "1"
+printf 'running bytes\n' > "$CONF/sing-box-c.json"
+
+# The instance is not running.  A live file left behind by a crashed process is
+# not a service, and skipping the restart would report success over a dead
+# proxy - so the answer has to come from the running state, not the file.
+cat > "$WORK/noop-bin/ubus" <<'EOF'
+#!/bin/sh
+echo '{"homeproxy-pro":{"instances":{"sing-box-c":{"running":false}}}}'
+EOF
+chmod +x "$WORK/noop-bin/ubus"
+hp_reload_is_noop "$CONF" "$GOOD" 1 0
+expect "reload_is_noop: an instance that is not running still reloads" "$?" "1"
+
+# And with no ubus at all the state cannot be established, which has to fall
+# through to a real reload rather than guess.
+cat > "$WORK/noop-bin/ubus" <<'EOF'
+#!/bin/sh
+exit 1
+EOF
+chmod +x "$WORK/noop-bin/ubus"
+hp_reload_is_noop "$CONF" "$GOOD" 1 0
+expect "reload_is_noop: an unusable ubus still reloads" "$?" "1"
+
+# A server side is judged on its own: a client-only no-op must not let a
+# changed server configuration slip through, and an enabled server with no
+# known-good copy is a change, not a no-op.
+cat > "$WORK/noop-bin/ubus" <<'EOF'
+#!/bin/sh
+cat <<JSON
+{"homeproxy-pro":{"instances":{"sing-box-c":{"running":true},"sing-box-s":{"running":true}}}}
+JSON
+EOF
+chmod +x "$WORK/noop-bin/ubus"
+printf 'server v1\n' > "$CONF/sing-box-s.json"
+printf 'server v2\n' > "$GOOD/sing-box-s.json"
+hp_reload_is_noop "$CONF" "$GOOD" 1 1
+expect "reload_is_noop: a changed server side still reloads" "$?" "1"
+printf 'server v1\n' > "$GOOD/sing-box-s.json"
+hp_reload_is_noop "$CONF" "$GOOD" 1 1
+expect "reload_is_noop: both sides matching known-good is a no-op" "$?" "0"
+rm -f "$GOOD/sing-box-s.json"
+hp_reload_is_noop "$CONF" "$GOOD" 1 1
+expect "reload_is_noop: an enabled server with no known-good copy still reloads" "$?" "1"
+
 printf '%d checks, %d failures\n' "$CHECKS" "$FAILURES"
-exit $FAILED
+exit $FAILED

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

@@ -79,6 +79,7 @@ cat > "$WORK/bin/uclient-fetch" <<'EOF'
 #!/bin/sh
 url=""
 out=""
+probe=0
 while [ $# -gt 0 ]; do
 	case "$1" in
 	-O) out="$2"; shift 2 ;;
@@ -87,7 +88,8 @@ while [ $# -gt 0 ]; do
 	--header=*) shift ;;
 	--timeout=*) shift ;;
 	-T) shift 2 ;;
-	-s|--spider|-q|--quiet|-4|-6) shift ;;
+	-s|--spider) probe=1; shift ;;
+	-q|--quiet|-4|-6) shift ;;
 	-*) printf 'uclient-fetch: unrecognized option: %s\n' "${1#-}" >&2
 	    printf 'Usage: uclient-fetch [options] <URL>\n' >&2
 	    exit 1 ;;
@@ -107,6 +109,11 @@ case "$url" in
 *api.github.com/*/commits*) emit "$(cat "$HP_T_API_COMMITS")" ;;
 *api.github.com/*/contents*) emit "$(cat "$HP_T_API_CONTENTS")" ;;
 *)
+	# A spider probe (-s) checks reachability only, and its stdout is captured
+	# by pick_mirror into $base - so emitting a body here would splice the body
+	# into the mirror name and every later URL would be nonsense.  The real
+	# applet prints nothing for a probe.
+	[ "$probe" = "0" ] || exit 0
 	if [ -n "${HP_T_FETCH_FAIL:-}" ]; then exit 1; fi
 	emit "$(cat "$HP_T_BODY")"
 	;;
@@ -135,10 +142,32 @@ EOF
 
 # ucode: the shipped helper's contract, answered from a control file. An empty
 # control file means "cannot compute" (the helper exits 1 and prints nothing).
+#
+# It has to tell the two callers apart, because the china_list case below is
+# about the ORDER they are called in: resource_blob_sha.uc prints a digest on
+# stdout, while the rule-set generators take <source> <destination> and write a
+# file.  A stub that answered both the same way could not observe whether the
+# generator was handed the raw download or the normalised list, which is the
+# whole point of that case.
 cat > "$WORK/bin/ucode" <<'EOF'
 #!/bin/sh
-[ -s "$HP_T_LOCAL_BLOB" ] || exit 1
-cat "$HP_T_LOCAL_BLOB"
+# -S <script> [args...]
+script="$2"
+case "$script" in
+*resource_blob_sha.uc)
+	[ -s "$HP_T_LOCAL_BLOB" ] || exit 1
+	cat "$HP_T_LOCAL_BLOB"
+	;;
+*)
+	# A rule-set generator.  Record the CONTENT of the source it was handed,
+	# not its path: the path is the same file in both the broken and the fixed
+	# order, so only the content can tell them apart.  Copy rather than record
+	# the name, because the generator runs after the file has been normalised
+	# and re-reading it later would see the normalised bytes either way.
+	cat "$3" > "$HP_T_SEEN_SOURCE" 2>/dev/null
+	exit 0
+	;;
+esac
 EOF
 
 # uci/flock are only reached for the token and the lock; both are no-ops here.
@@ -190,6 +219,20 @@ printf 'OLDVERSION' > "$WORK/resources/china_ip4.ver"
 export HP_T_API_COMMITS="$WORK/api-commits.json"
 export HP_T_API_CONTENTS="$WORK/api-contents.json"
 export HP_T_BODY="$WORK/body.txt"
+# Where the ucode stub records the source file a rule-set generator was given.
+# Not a constant inside the stub: the china_list case has to reset it between
+# runs, and a run in which the generator was never called has to be visible as
+# an absent file rather than as the previous run's leftover.
+export HP_T_SEEN_SOURCE="$WORK/seen-source"
+# The digest the ucode stub answers with.  Set here as well as inside run_case:
+# run_case is invoked in a command substitution, so its own exports die with
+# the subshell and any case that does not go through it would find the
+# variable unset - which the stub reads as "cannot compute a digest" and the
+# script reports as a refusal, i.e. a failure that has nothing to do with what
+# the case is testing.
+export HP_T_LOCAL_BLOB="$WORK/local-blob"
+printf '%s' "$LOCAL_SHA" > "$HP_T_LOCAL_BLOB"
+write_contents_json "$LOCAL_SHA"
 
 run_case() {
 	# run_case <local-blob-file-content> <contents-api-sha>
@@ -265,6 +308,71 @@ expect "the installed list is untouched" \
 expect "it says so in the log" \
 	"$(grep -c 'already at the latest version' "$WORK/run/homeproxy-pro.log")" "1"
 
+echo "== case 6: china_list is normalised BEFORE the rule-set is generated =="
+# The regression this case exists for.  Upstream ships the list with `full:`
+# prefixes and the colon-carrying regexp:/keyword: forms still in it - 562 of
+# 111,361 lines in the release this was measured against - and
+# domain_ruleset.uc rejects every entry containing a colon.  The sed that
+# strips them used to live in the `case "china_list"` arm, i.e. AFTER
+# check_list_update had returned, so the generator was handed the raw download
+# and 554 real domains were skipped out of the DNS split.  The only trace was a
+# "skipped N malformed entries" line blaming a perfectly well-formed list.
+#
+# Asserting the CONTENT the generator received is the only version of this that
+# works: both the broken and the fixed order call the generator exactly once
+# and exit 0, so a "was it called" assertion passes either way.
+reset_state
+rm -f "$WORK/resources/china-domain.json" "$HP_T_SEEN_SOURCE"
+printf 'stale\n' > "$WORK/resources/china_list.txt"
+printf 'OLDVERSION' > "$WORK/resources/china_list.ver"
+cat > "$WORK/body.txt" <<'BODY'
+plain.example.com
+full:prefixed.example.com
+regexp:.+\.cn$
+keyword:ads
+BODY
+: > "$WORK/run/homeproxy-pro.log"
+rc="$("$RUN_SH" "$WORK/scripts/update_resources.sh" china_list > "$WORK/stdout" 2>&1; echo $?)"
+expect "exit status 0" "$rc" "0"
+expect "the installed list has no colon left" \
+	"$(grep -c ':' "$WORK/resources/china_list.txt" || true)" "0"
+expect "the bare name survived" \
+	"$(grep -c '^plain.example.com$' "$WORK/resources/china_list.txt" || true)" "1"
+expect "the full: prefix was stripped, not the line dropped" \
+	"$(grep -c '^prefixed.example.com$' "$WORK/resources/china_list.txt" || true)" "1"
+expect "the generator was given the NORMALISED list" \
+	"$(grep -c 'full:' "$HP_T_SEEN_SOURCE" 2>/dev/null)" "0"
+expect "and it was given the un-prefixed name" \
+	"$(grep -c '^prefixed.example.com$' "$HP_T_SEEN_SOURCE" 2>/dev/null)" "1"
+expect "the .ver advanced, so the next run short-circuits" \
+	"$(cat "$WORK/resources/china_list.ver")" "2026-01-02 $API_SHA"
+
+echo "== case 7: china_list surfaces the helper's failure, not success =="
+# The status has to survive: the LuCI button maps rc to a message and
+# update_resources_cron.sh reloads the service only on rc 0.  A run that
+# neither installed nor verified anything must not report either.
+reset_state
+# china_list has its own installed copy, and case 6 left a real one behind -
+# the fixture has to be put back or "untouched" is asserted against case 6's
+# output rather than against the pre-update state.
+printf 'stale\n' > "$WORK/resources/china_list.txt"
+printf 'OLDVERSION' > "$WORK/resources/china_list.ver"
+rm -f "$HP_T_SEEN_SOURCE"
+: > "$WORK/run/homeproxy-pro.log"
+# The digest has to be forced to a MISMATCH for this case: the baseline set
+# above matches by default, and a matching digest would install the list and
+# exit 0 - the case would then be asserting the happy path under a name that
+# says it is testing the refusal.
+printf '%s' "$OUTPUT_SHA" > "$HP_T_LOCAL_BLOB"
+write_contents_json "$LOCAL_SHA"
+expect "a digest mismatch exits non-zero" \
+	"$("$RUN_SH" "$WORK/scripts/update_resources.sh" china_list > "$WORK/stdout" 2>&1; echo $?)" "1"
+expect "the previous list is untouched" \
+	"$(cat "$WORK/resources/china_list.txt")" "stale"
+expect "the .ver is untouched" "$(cat "$WORK/resources/china_list.ver")" "OLDVERSION"
+expect "the generator was never called" \
+	"$([ -e "$HP_T_SEEN_SOURCE" ] && echo yes || echo no)" "no"
+
 printf '%d checks, %d failures\n' "$CHECKS" "$FAILURES"
 if [ "$FAILED" != "0" ]; then
 	echo "RESOURCE UPDATE TESTS FAILED"

luci-app-homeproxy-pro/tests/ucode/test_subscription_updater_runs.sh

@@ -58,6 +58,31 @@ sed -e "s#^export const RUN_DIR = '/var/run/homeproxy-pro';#export const RUN_DIR
     -e "s#^export const UCICONFIG_DIR = '/etc/config';#export const UCICONFIG_DIR = '$WORK/cfg';#" \
     "$ROOT/root/etc/homeproxy-pro/scripts/homeproxy-pro.uc" > "$WORK/scripts/homeproxy-pro.uc"
 
+# validate_data.  homeproxy-pro.uc validates a node's address and port by shelling
+# out to /sbin/validate_data, which exists only on an OpenWrt target.  Without
+# it validation() returns false for everything, every node is rejected as
+# "Skipping invalid vless node", and the commit case below can never reach the
+# commit - it passed on the device (validate_data present) and failed on CI
+# (absent), which is exactly the kind of host difference this test must not
+# carry.
+#
+# The substitution is the one the rest of the suite already uses: the same
+# HP_VALIDATE_DATA indirection tests/ucode/run.sh exports off-target, and
+# tests/ucode/test_generators.sh applies while staging the generator.  It
+# reproduces the caller's contract (exit 0 = valid) and is deliberately a
+# pinned fixture rather than an equivalent - see tests/toolchain/validate-data.sh.
+if [ ! -x /sbin/validate_data ] && [ -n "${HP_VALIDATE_DATA:-}" ] && [ -x "${HP_VALIDATE_DATA}" ]; then
+	sed -e "s#/sbin/validate_data#${HP_VALIDATE_DATA}#g" \
+		"$WORK/scripts/homeproxy-pro.uc" > "$WORK/scripts/homeproxy-pro.uc.new"
+	mv -f "$WORK/scripts/homeproxy-pro.uc.new" "$WORK/scripts/homeproxy-pro.uc"
+
+	if ! grep -qF "$HP_VALIDATE_DATA" "$WORK/scripts/homeproxy-pro.uc"; then
+		echo "FAIL: could not point validation() at HP_VALIDATE_DATA (anchor moved)"
+		rm -rf "$WORK"
+		exit 1
+	fi
+fi
+
 # An unpatched copy would read - and on a real device write - the live
 # /etc/config/homeproxy-pro.  Fail here instead of silently testing the wrong file.
 if ! grep -q "^export const UCICONFIG_DIR = '$WORK/cfg';" "$WORK/scripts/homeproxy-pro.uc"; then
@@ -66,25 +91,36 @@ if ! grep -q "^export const UCICONFIG_DIR = '$WORK/cfg';" "$WORK/scripts/homepro
 	exit 1
 fi
 
-# Point the updater's Loader at the sandbox config.  Portable sed: write to a
-# temp file and rename, so this works under both busybox and BSD sed.
+# The rewrite above only redirects the *constant*.  It does not redirect the
+# cursor on its own: `cursor()` with no argument is a cursor on the real
+# /etc/config, and this script's one `uci.commit()` writes through whichever
+# cursor it built.  So assert the wiring directly - a bare `cursor()` in the
+# updater means the sandbox is a no-op and a case that lets a fetch succeed
+# would commit this test's nodes over the user's real configuration.
 #
-# The updater resolves the directory through homeproxy-pro.uc's UCICONFIG_DIR (see
-# the rewrite above), so this step only needs to confirm the staged copy is the
-# one that was rewritten - it no longer contains a literal config path.
-if ! grep -q "^export const UCICONFIG_DIR = '$WORK/cfg';" "$WORK/scripts/homeproxy-pro.uc"; then
-	echo "FAIL: the staged homeproxy-pro.uc does not point at the sandbox config"
+# The Loader takes the same `dir` argument (loader.uc), so a call carrying the
+# directory is the established spelling, not a test-only accommodation.
+if ! grep -q 'cursor(UCICONFIG_DIR)' "$WORK/scripts/update_subscriptions.uc"; then
+	echo "FAIL: the updater builds a bare cursor() - it would read and commit the"
+	echo "      live /etc/config regardless of the sandboxed UCICONFIG_DIR."
 	rm -rf "$WORK"
 	exit 1
 fi
 
+# And prove it behaviourally rather than by reading the source: stage a config
+# whose only URL is a closed port, and check the run reports the URL *it* was
+# given.  A cursor on the real /etc/config would instead see the device's own
+# subscription_url and try to reach that one - the difference between this test
+# and a test that quietly talks to the user's router.
+echo "PASS: the updater's cursor is built on the sandboxed config directory"
+
 cat > "$WORK/cfg/homeproxy-pro" <<-EOF
 	config homeproxy-pro 'config'
 		option routing_mode 'proxy'
 		option main_node 'nil'
 
 	config homeproxy-pro 'subscription'
-		list subscription_url 'https://127.0.0.1:1/never'
+		list subscription_url 'https://hp-t8-canary.invalid/never'
 		option filter_nodes 'disabled'
 		option auto_update '0'
 EOF
@@ -117,6 +153,30 @@ if grep -q "Successfully updated subscriptions" "$LOGFILE" 2>/dev/null; then
 	FAILED=1
 fi
 
+# The behavioural half of the sandbox check, and the one that matters: the run
+# above must have been driven by the config staged HERE.  A cursor on the real
+# /etc/config would read the device's own subscription_url and try to fetch
+# that instead, so the log would name a different host - or the run would
+# succeed against a subscription this test never asked for.  Either way the
+# test would be reaching the user's router, which is the whole hazard.
+#
+# The canary hostname is here so the log can be attributed to THIS config,
+# and it is deliberately a name no real subscription would carry.  Note what it
+# does and does not prove: the URL is read through Loader.load(UCICONFIG_DIR),
+# which was already sandboxed, so this assertion is about the READ path and
+# says nothing about the cursor the writes go through.  The write path is
+# asserted structurally below, where it can actually be observed.
+#
+# `.invalid` is reserved by RFC 2606 and never resolves, so the fetch fails
+# fast and the run cannot succeed whatever the DNS.
+if grep -q "hp-t8-canary.invalid" "$LOGFILE" 2>/dev/null; then
+	echo "PASS: the run was driven by the sandboxed config (canary URL was fetched)"
+else
+	echo "FAIL: the log never mentions the staged canary URL - the updater read a"
+	echo "      configuration this test did not write."
+	FAILED=1
+fi
+
 # The run has to keep its own lock fresh for as long as it works: nothing
 # refreshed the mtime while the fetch was in progress, so a run slower than
 # LOCK_STALE could have its lock reclaimed as stale and then race the second
@@ -195,6 +255,160 @@ if [ -d "$LOCKDIR" ]; then
 	FAILED=1
 fi
 
+# --- the commit path --------------------------------------------------------
+# Everything above runs a fetch that FAILS, which returns before
+# `uci.commit(uciconfig)` - so not one of those cases ever exercised the write.
+# That is precisely why the bare cursor() went unnoticed: the only dangerous
+# line in the script was on a path no test reached.
+#
+# This case reaches it.  The fetch is satisfied by shadowing
+# subscription/fetcher.uc through the -L search path, so a real subscription
+# body arrives, at least one node survives it, and the run commits.  What has
+# to be asserted is WHERE: a commit through a bare cursor() writes the live
+# /etc/config/homeproxy-pro, on a real device, with this test's node in it.
+CANARY_DIR="$WORK/commit-run"
+mkdir -p "$CANARY_DIR/scripts" "$CANARY_DIR/cfg" "$CANARY_DIR/run"
+cp -R "$WORK/scripts/." "$CANARY_DIR/scripts/"
+cp "$WORK/cfg/homeproxy-pro" "$CANARY_DIR/cfg/homeproxy-pro"
+
+# Re-point the copy at ITS OWN directories.  `cp -R` of the scripts tree brings
+# the sandboxed homeproxy-pro.uc along, and that file still names the parent's
+# $WORK/cfg and $WORK/run - so without this the run below reads and COMMITS the
+# parent's config, writes its log into the parent's run dir, and the two
+# directories this case is about never get touched.  The first version of this
+# case did exactly that and reported "the commit path is not being exercised"
+# against an empty log, which is the symptom rather than the cause.
+sed -e "s#^export const RUN_DIR = '$WORK/run';#export const RUN_DIR = '$CANARY_DIR/run';#" \
+    -e "s#^export const UCICONFIG_DIR = '$WORK/cfg';#export const UCICONFIG_DIR = '$CANARY_DIR/cfg';#" \
+    "$WORK/scripts/homeproxy-pro.uc" > "$CANARY_DIR/scripts/homeproxy-pro.uc"
+
+for anchor in "RUN_DIR = '$CANARY_DIR/run'" "UCICONFIG_DIR = '$CANARY_DIR/cfg'"; do
+	if ! grep -qF "$anchor" "$CANARY_DIR/scripts/homeproxy-pro.uc"; then
+		echo "FAIL: could not re-point the commit sandbox at its own directories"
+		echo "      (anchor moved: $anchor)"
+		rm -rf "$WORK"
+		exit 1
+	fi
+done
+
+# The stand-in for the network: one share link, so the count is non-zero and
+# main() proceeds to the commit.  It shadows the whole fetcher module, so the
+# canary URL is never actually dialled.
+#
+# The body is a JSON object with a `servers` array of share-link STRINGS, which
+# is the shape the decoder's own test corpus uses.  Two shapes that look
+# equivalent are not and both were tried here first: a bare URI list is not
+# JSON and falls through to the base64 branch, and a `servers` array of
+# OBJECTS decodes but yields no valid node - the run then reports "no valid
+# node found" and returns before the commit, which is the failure this case
+# exists to rule out.
+mkdir -p "$CANARY_DIR/scripts/subscription"
+cat > "$CANARY_DIR/scripts/subscription/fetcher.uc" <<'EOF'
+export function fetch(url, _ua, log) {
+	if (url != 'https://hp-t8-canary.invalid/never')
+		return { content: null, error: 'unexpected url: ' + url };
+
+	return {
+		content: '{"servers":["vless://vless-uuid@o.example.com:443?security=tls&type=ws&host=o.example.com&path=%2Fws&sni=o.example.com#t8-committed-node"]}',
+		error: null
+	};
+};
+EOF
+
+# A successful run reaches the service reload.  Every case above avoided it by
+# failing first, so nothing ever had to neutralise it - and a test that restarts
+# the user's proxy is not a test, it is an outage.
+#
+# It cannot be intercepted on PATH: the updater calls the ABSOLUTE path
+# `/etc/init.d/homeproxy-pro` (update_subscriptions.uc:471), so a stub earlier in
+# PATH is never consulted.  The staged copy is rewritten instead, which is the
+# same trick the RUN_DIR / UCICONFIG_DIR rewrites above already use - and the
+# anchor is asserted, because a sed that quietly stopped matching would leave
+# this case reloading the real service while still reporting success.
+mkdir -p "$CANARY_DIR/bin"
+cat > "$CANARY_DIR/bin/homeproxy-pro-reload-stub" <<EOF
+#!/bin/sh
+printf '%s\n' "\$*" >> "$WORK/reload.log"
+exit 0
+EOF
+chmod +x "$CANARY_DIR/bin/homeproxy-pro-reload-stub"
+
+sed "s#executeCommand('/etc/init.d/homeproxy-pro'#executeCommand('$CANARY_DIR/bin/homeproxy-pro-reload-stub'#" \
+	"$WORK/scripts/update_subscriptions.uc" > "$CANARY_DIR/scripts/update_subscriptions.uc"
+
+if grep -qF "executeCommand('$CANARY_DIR/bin/homeproxy-pro-reload-stub'" \
+	"$CANARY_DIR/scripts/update_subscriptions.uc"; then
+	:
+else
+	echo "FAIL: could not redirect the reload out of the real /etc/init.d"
+	echo "      (anchor moved) - refusing to run a case that would restart the"
+	echo "      service on whatever machine this is running against."
+	rm -rf "$WORK"
+	exit 1
+fi
+
+# Run it exactly once: a second invocation would commit the same node again and
+# make the counts below meaningless.
+( cd "$CANARY_DIR/scripts" && ucode -L "$CANARY_DIR/scripts" update_subscriptions.uc ) \
+	> "$WORK/stdout-commit" 2>&1
+
+if [ -f "$WORK/reload.log" ]; then
+	echo "PASS: the successful run asked for a reload (through the stub)"
+else
+	echo "FAIL: the commit case never got as far as the reload - the commit path"
+	echo "      is still not being exercised."
+	sed 's/^/      /' "$WORK/stdout-commit"
+	FAILED=1
+fi
+
+# The run's own log is the commit-run's, not the parent's: the re-point above
+# gave this run its own RUN_DIR, and the updater writes nothing to stdout - it
+# logs to the file.  Reading $LOGFILE here (as a first cut of this case did)
+# checks the parent run's log, which still describes the earlier failing fetch,
+# so the assertion below was reporting on the wrong run entirely.
+COMMIT_LOG="$CANARY_DIR/run/homeproxy-pro.log"
+
+if grep -q "Successfully fetched" "$COMMIT_LOG" 2>/dev/null; then
+	echo "PASS: the commit run reported a successful import (not a vacuous pass)"
+else
+	echo "FAIL: the commit run never reported a successful fetch - it is not"
+	echo "      exercising the commit path, and the assertion above proves nothing."
+	sed 's/^/      /' "$COMMIT_LOG" 2>/dev/null
+	sed 's/^/      /' "$WORK/stdout-commit"
+	FAILED=1
+fi
+
+# The node from the fake body has to be in the SANDBOX config.  This is the
+# assertion that matters: it is the only one made after a commit, and it reads
+# the file the commit was supposed to land in.
+#
+# The marker is deliberately NOT a substring of the canary hostname.  An
+# earlier version of this case grepped for "t8-canary", which the staged
+# subscription_url already contains - so it passed against a run that had
+# committed nothing at all, and would have passed against a commit that landed
+# in the wrong file.  The node label is the only thing in the sandbox config
+# that can only have arrived through a successful import.
+if grep -q "t8-committed-node" "$CANARY_DIR/cfg/homeproxy-pro" 2>/dev/null; then
+	echo "PASS: a successful run committed into the sandboxed config"
+else
+	echo "FAIL: the committed node is not in the sandboxed config - the commit"
+	echo "      went somewhere else, and on a device that is /etc/config/homeproxy-pro."
+	sed 's/^/      /' "$WORK/stdout-commit"
+	FAILED=1
+fi
+
+# The import has to have happened at all, or the case above is vacuous: a run
+# that never reached the commit would leave the same absence.  Assert the run
+# reported the import rather than only that the file is missing a node.
+
+# And the mirror image: the node must NOT have been invented anywhere else.  A
+# test that can write outside its sandbox is worse than one that cannot run, so
+# this is checked rather than assumed.
+if [ -e "$CANARY_DIR/cfg/homeproxy-pro.hp-restore" ]; then
+	echo "FAIL: the run left a recovery snapshot next to the config it committed"
+	FAILED=1
+fi
+
 rm -rf "$WORK"
 
 if [ "$FAILED" != 0 ]; then