fix: bound package hooks so apk cannot roll the upgrade back

The router upgrade died at the installer's 120s ceiling while apk was
running tachyon-1.2.66.pre-upgrade, so apk rolled the transaction back
and the old build stayed installed.

prerm reached INIT_PATH status through rc.common, which waits on
flock -w 1000. A stuck start or retry_start_on_wan_up holding that lock
stalls the hook for up to a quarter of an hour. Every init call in the
hooks is now bounded, the status probe moved after the flock waiters are
killed, and the Makefile hook bounds the old version's cleanup from the
outside since that code cannot be trusted to bound itself.

The installer also called a rollback a success: a rebuilt tag keeps its
version, so both the presence and version checks still passed. It now
compares the commit SHA baked into the installed constants module and
fails loudly when it did not move. Package installs get 300s, since a
kill mid-transaction is what caused the rollback.
This commit is contained in:
Dushnilin
2026-08-09 18:23:16 +03:00
parent ba69e3d240
commit f770316345
5 changed files with 273 additions and 17 deletions
+51 -5
View File
@@ -1732,10 +1732,12 @@ pkg_install_name() {
return 0
fi
# 300s: named packages carry postinst hooks too (sing-box, kmods pulling deps
# from the network), and a kill mid-transaction is rolled back on apk.
if [ "$PKG_IS_APK" -eq 1 ]; then
apk_with_lock_retry "apk" 120 apk add "$pkg_name"
apk_with_lock_retry "apk" 300 apk add "$pkg_name"
else
run_logged_timeout "opkg" 120 opkg install "$pkg_name"
run_logged_timeout "opkg" 300 opkg install "$pkg_name"
fi
local rc=$?
@@ -1748,14 +1750,37 @@ pkg_install_name() {
return 0
}
# Commit SHA baked into the currently installed backend, read straight from the
# installed constants module. Empty when nothing is installed or the placeholder
# was never substituted. This is what tells a rebuild of the same tag apart from
# the build already on disk — the version string cannot.
installed_backend_sha() {
_sha_file="/usr/lib/tachyon/core/constants.uc"
[ -r "$_sha_file" ] || return 0
# "unknown" is what the Makefile substitutes when git is unavailable at build
# time, and the placeholder survives an unprocessed tree; neither identifies a
# build, so both must read as "no SHA" rather than as a SHA that never moves.
sed -n 's/.*TACHYON_COMMIT_SHA", *"\([^"]*\)".*/\1/p' "$_sha_file" 2>/dev/null |
head -1 | grep -v -e COMPILED -e '^unknown$' || true
}
pkg_install_files() {
if [ "$DRY_RUN" -eq 1 ]; then
msg "[dry-run] would install downloaded package file(s): $*"
return 0
fi
# Recorded before the transaction so a rolled-back rebuild of the same tag is
# detectable: the version stays identical, only the SHA moves.
local sha_before
sha_before="$(installed_backend_sha)"
if [ "$PKG_IS_APK" -eq 1 ]; then
apk_with_lock_retry "apk" 120 apk add --allow-untrusted "$@"
# 300s, not 120s: the package hooks stop and restart the service, and on a
# loaded router that legitimately outlives two minutes. Being killed here
# makes apk roll the whole transaction back, which silently leaves the old
# build installed — the exact failure this ceiling used to cause.
apk_with_lock_retry "apk" 300 apk add --allow-untrusted "$@"
local rc=$?
if [ $rc -ne 0 ]; then
# apk may report warnings as errors even though the package installed successfully
@@ -1768,6 +1793,14 @@ pkg_install_files() {
pkg_expected_ver="$(basename "$pkg_file" | sed 's/^[^_]*_//; s/\.apk$//')"
pkg_actual_ver="$(apk info "$pkg_name" 2>/dev/null | head -1 | sed "s/${pkg_name}-//")"
if [ "$pkg_actual_ver" = "$pkg_expected_ver" ]; then
# A matching version proves nothing for a rebuilt tag: it also
# matches when apk rolled back to the identical old version. If
# the backend SHA did not move either, nothing was written.
if [ "$pkg_name" = "tachyon" ] && [ -n "$sha_before" ] &&
[ "$(installed_backend_sha)" = "$sha_before" ]; then
log_line "FAIL apk install failed (exit $rc); backend still at $sha_before, transaction was rolled back"
return $rc
fi
warn "Package manager reported non-critical errors during installation (package is installed OK)"
return 0
fi
@@ -1779,7 +1812,8 @@ pkg_install_files() {
# --force-reinstall: a release tag can be rebuilt, so the .ipk version may
# equal the installed one. Without it opkg treats that as "already
# installed" and the rebuild is never written.
run_logged_timeout "opkg" 120 opkg install --force-overwrite --force-downgrade --force-reinstall "$@"
# 300s for the same reason as the apk branch: the hooks restart the service.
run_logged_timeout "opkg" 300 opkg install --force-overwrite --force-downgrade --force-reinstall "$@"
local rc=$?
if [ $rc -ne 0 ]; then
log_line "FAIL opkg install failed (exit $rc)"
@@ -2400,8 +2434,20 @@ install_backend_package() {
cfg_hash_before="${_sha_out%% *}"
fi
local backend_sha_before
backend_sha_before="$(installed_backend_sha)"
if ! pkg_install_files "$TACHYON_BACKEND_FILE"; then
if pkg_is_installed "tachyon"; then
# "tachyon is installed" is not evidence of success on a rebuilt tag: apk
# rolls the transaction back to the identical old version, so both the
# package name and its version still check out. Only a moved commit SHA
# proves new files landed. Without a SHA to compare (fresh install, or an
# unstamped build) fall back to the presence check.
local backend_sha_after
backend_sha_after="$(installed_backend_sha)"
if [ -n "$backend_sha_before" ] && [ "$backend_sha_before" = "$backend_sha_after" ]; then
fail "tachyon installation failed: still running build $backend_sha_before, the package manager rolled the upgrade back (see $LOG_FILE)"
elif pkg_is_installed "tachyon"; then
warn "Package manager reported errors during installation, but tachyon is installed — continuing"
else
fail "tachyon installation failed"
+19 -2
View File
@@ -44,9 +44,26 @@ endef
define Package/tachyon/prerm
#!/usr/bin/ucode
if (getenv("IPKG_INSTROOT") == null || getenv("IPKG_INSTROOT") == "")
system("/usr/bin/tachyon package_prerm >/dev/null 2>&1");
// apk runs this as <pkg>-<version>.pre-upgrade and rolls the whole transaction
// back if the hook does not finish, which leaves the old build installed. The
// cleanup invoked below belongs to the OLD version, so it cannot be trusted to
// bound itself — the hook bounds it from the outside. 90s sits far below any
// package-manager or installer patience and far above a healthy cleanup.
// BusyBox timeout wanted `-t SECONDS` on older builds and rejects it on newer
// ones; the spelling is probed against /bin/true because probing with the real
// command would run the cleanup twice whenever the first form is unsupported.
if (getenv("IPKG_INSTROOT") == null || getenv("IPKG_INSTROOT") == "") {
let prefix = "";
if (system("timeout 1 /bin/true >/dev/null 2>&1") == 0)
prefix = "timeout 90 ";
else if (system("timeout -t 1 /bin/true >/dev/null 2>&1") == 0)
prefix = "timeout -t 90 ";
system(prefix + "/usr/bin/tachyon package_prerm >/dev/null 2>&1");
}
// Whatever the cleanup managed to do, the removal itself must proceed: a failing
// prerm aborts the upgrade on both package managers.
exit(0);
endef
+71 -10
View File
@@ -23,6 +23,16 @@ const SING_BOX_CRONET = env("TACHYON_SING_BOX_CRONET", "/usr/lib/libcronet.so");
const SING_BOX_MANAGED_MARKER = env("SB_MANAGED_SERVICE_MARKER", "Tachyon managed sing-box service for binary variants");
const PACKAGE_UPGRADE_STATE = env("TACHYON_PACKAGE_UPGRADE_STATE", "/tmp/tachyon-package-was-running");
const PACKAGE_TEST_MODE = env("TACHYON_PACKAGE_TEST_MODE", "") != "";
// apk aborts and rolls the whole transaction back if a hook outlives its
// patience, and the installer kills it even sooner. Anything that touches
// /etc/init.d/tachyon goes through rc.common, which waits on `flock -w 1000` —
// a stuck start or retry_start_on_wan_up holding that lock would stall the hook
// for a quarter of an hour. Every such call is bounded instead.
const HOOK_COMMAND_TIMEOUT = env("TACHYON_HOOK_COMMAND_TIMEOUT", "20");
// Starting the service legitimately takes longer than probing or stopping it —
// sing-box comes up, nft rules load, subscriptions may be re-read — so it gets
// its own, larger budget. Still bounded: postinst must end.
const HOOK_START_TIMEOUT = env("TACHYON_HOOK_START_TIMEOUT", "60");
function shell_quote(value) {
return "'" + replace(as_string(value), /'/g, "'\\''") + "'";
@@ -49,6 +59,47 @@ function command_success_from_args(args) {
return normalize_status(system(command_from_args(args) + " >/dev/null 2>&1")) == 0;
}
// Which spelling of `timeout` this box understands, resolved once. BusyBox
// required `-t SECONDS` on older builds and rejects it on newer ones. Probing is
// done against /bin/true rather than the real command: an unsupported form would
// otherwise mean running the cleanup twice.
let timeout_prefix_cache = null;
function timeout_prefix() {
if (timeout_prefix_cache != null)
return timeout_prefix_cache;
if (normalize_status(system("timeout 1 /bin/true >/dev/null 2>&1")) == 0)
timeout_prefix_cache = [ "timeout" ];
else if (normalize_status(system("timeout -t 1 /bin/true >/dev/null 2>&1")) == 0)
timeout_prefix_cache = [ "timeout", "-t" ];
else
timeout_prefix_cache = []; // no usable timeout binary
return timeout_prefix_cache;
}
// Same as command_success_from_args, but the command cannot outlive `seconds`.
// A timed-out command reports failure (timeout exits 124), which every caller
// here already treats as "could not do it, carry on" — the alternative is an
// upgrade the package manager rolls back.
function bounded_command_success_from_args(args, seconds) {
seconds = as_string(seconds) != "" ? as_string(seconds) : HOOK_COMMAND_TIMEOUT;
let prefix = timeout_prefix();
if (length(prefix) == 0)
return command_success_from_args(args);
let bounded = [];
for (let part in prefix)
push(bounded, part);
push(bounded, seconds);
for (let arg in args)
push(bounded, arg);
return command_success_from_args(bounded);
}
function path_exists(path) {
return fs.stat(as_string(path)) != null;
}
@@ -97,9 +148,9 @@ function restore_dnsmasq_if_needed() {
if (dont_touch_dhcp_enabled())
return;
command_success_from_args([ BIN_PATH, "restore_dnsmasq" ]);
bounded_command_success_from_args([ BIN_PATH, "restore_dnsmasq" ]);
if (path_exists(DNS_APPLY_UC))
command_success_from_args([ "ucode", DNS_APPLY_UC, "failsafe-restore" ]);
bounded_command_success_from_args([ "ucode", DNS_APPLY_UC, "failsafe-restore" ]);
}
function remove_managed_sing_box() {
@@ -107,15 +158,19 @@ function remove_managed_sing_box() {
if (data == null || index(data, SING_BOX_MANAGED_MARKER) < 0)
return;
command_success_from_args([ SING_BOX_INIT, "stop" ]);
command_success_from_args([ SING_BOX_INIT, "disable" ]);
bounded_command_success_from_args([ SING_BOX_INIT, "stop" ]);
bounded_command_success_from_args([ SING_BOX_INIT, "disable" ]);
unlink_if_exists(SING_BOX_INIT);
unlink_if_exists(SING_BOX_BIN);
unlink_if_exists(SING_BOX_CRONET);
}
function remember_upgrade_state(action) {
if (as_string(action) != "upgrade" || !command_success_from_args([ INIT_PATH, "status" ])) {
// INIT_PATH status goes through rc.common's flock, so it is bounded like every
// other init call here. A timeout is indistinguishable from "not running", and
// that is the safe reading: the worst case is not restarting a service that was
// wedged anyway, versus stalling the upgrade until apk rolls it back.
if (as_string(action) != "upgrade" || !bounded_command_success_from_args([ INIT_PATH, "status" ])) {
unlink_if_exists(PACKAGE_UPGRADE_STATE);
return;
}
@@ -126,16 +181,20 @@ function prerm_cleanup(action) {
if (env("IPKG_INSTROOT", "") != "")
return true;
remember_upgrade_state(action);
if (!PACKAGE_TEST_MODE) {
// Kill the WAN hotplug monitor and all init.d/tachyon/flock processes FIRST
// to prevent INIT_PATH stop from blocking indefinitely on rc.common flock -w 1000.
// This has to happen before the status probe below, or that probe becomes the
// very thing that waits on the lock those processes hold.
system("ps 2>/dev/null | grep -E '99-tachyon-wan|flock 1000|init[.]d/tachyon' | awk '{print $1}' | while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
system("ps 2>/dev/null | grep -F '/etc/init.d/tachyon' | awk '{print $1}' | while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
}
remember_upgrade_state(action);
if (!PACKAGE_TEST_MODE) {
// Stop the service using BIN_PATH directly (bypasses rc.common flock)
command_success_from_args([BIN_PATH, "stop"]);
command_success_from_args([INIT_PATH, "stop"]);
bounded_command_success_from_args([BIN_PATH, "stop"]);
bounded_command_success_from_args([INIT_PATH, "stop"]);
// Kill any remaining tachyon processes
system("ps 2>/dev/null | grep -F '/usr/bin/tachyon' | awk '{print $1}' | while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
@@ -155,8 +214,10 @@ function postinst_restore() {
system("ps 2>/dev/null | grep -E '99-tachyon-wan|flock 1000' | awk '{print $1}' | while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
system("ps 2>/dev/null | grep -F '/etc/init.d/tachyon' | awk '{print $1}' | while read _pid; do kill -9 \"$_pid\" 2>/dev/null; done; true");
}
// Start via INIT_PATH.
command_success_from_args([INIT_PATH, "start"]);
// Start via INIT_PATH. Bounded: a start that hangs must not take the package
// transaction down with it — the service can be started by hand afterwards,
// a rolled-back upgrade leaves the user on the old build with no recourse.
bounded_command_success_from_args([INIT_PATH, "start"], HOOK_START_TIMEOUT);
unlink_if_exists(PACKAGE_UPGRADE_STATE);
return true;
}
+81
View File
@@ -0,0 +1,81 @@
#!/usr/bin/env bash
# A rebuilt release keeps its version string, so "the package is installed and the
# version matches" stays true even when the package manager rolled the upgrade
# back — which is how a failed install used to end with a green success banner.
# The only thing that moves is the commit SHA baked into the installed constants
# module, so these checks pin how the installer reads it.
set -eo pipefail
ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
INSTALLER="$ROOT_DIR/install.sh"
WORK_DIR="$(mktemp -d)"
trap 'rm -rf "$WORK_DIR"' EXIT
fail() {
printf 'FAIL: %s\n' "$1" >&2
exit 1
}
assert_eq() {
local expected="$1" actual="$2" label="$3"
[ "$actual" = "$expected" ] || fail "$label: expected '$expected', got '$actual'"
}
[ -r "$INSTALLER" ] || fail "install.sh must exist"
# The function is lifted out of the installer rather than sourced: install.sh runs
# main() at the bottom, so sourcing it would attempt a real installation. Only the
# hardcoded path is redirected, so the extraction logic under test is the shipped
# one, character for character.
sed -n '/^installed_backend_sha() {/,/^}/p' "$INSTALLER" >"$WORK_DIR/sha.sh"
[ -s "$WORK_DIR/sha.sh" ] ||
fail "install.sh must define installed_backend_sha for rollback detection"
grep -Fq '/usr/lib/tachyon/core/constants.uc' "$WORK_DIR/sha.sh" ||
fail "installed_backend_sha must read the installed constants module"
sed 's#/usr/lib/tachyon/core/constants.uc#'"$WORK_DIR"'/constants.uc#' \
"$WORK_DIR/sha.sh" >"$WORK_DIR/sha-under-test.sh"
read_sha() {
local constants="$1"
rm -f "$WORK_DIR/constants.uc"
[ "$constants" = "" ] || cp "$constants" "$WORK_DIR/constants.uc"
(
# shellcheck disable=SC1090
. "$WORK_DIR/sha-under-test.sh"
installed_backend_sha
)
}
printf ' c.TACHYON_COMMIT_SHA = env("TACHYON_COMMIT_SHA", "ba69e3d2");\n' \
>"$WORK_DIR/stamped.uc"
assert_eq "ba69e3d2" "$(read_sha "$WORK_DIR/stamped.uc")" "a stamped build must report its SHA"
# An unprocessed tree keeps the literal placeholder; treating it as a SHA would
# make every install look like a rollback.
printf ' c.TACHYON_COMMIT_SHA = env("TACHYON_COMMIT_SHA", "__COMPILED_COMMIT_SHA__");\n' \
>"$WORK_DIR/placeholder.uc"
assert_eq "" "$(read_sha "$WORK_DIR/placeholder.uc")" "the build placeholder must not read as a SHA"
# The Makefile substitutes "unknown" when git is unavailable at build time. It
# never changes between builds, so it must not be compared as a SHA either.
printf ' c.TACHYON_COMMIT_SHA = env("TACHYON_COMMIT_SHA", "unknown");\n' \
>"$WORK_DIR/unknown.uc"
assert_eq "" "$(read_sha "$WORK_DIR/unknown.uc")" "an unstamped build must not read as a SHA"
assert_eq "" "$(read_sha "")" "a missing constants module must yield no SHA"
# A rolled-back rebuild is only detectable by comparing SHAs, so the backend
# install must refuse to accept a mere presence check when the SHA did not move.
grep -Fq 'rolled the upgrade back' "$INSTALLER" ||
fail "install.sh must fail loudly when the backend SHA did not move after install"
# 120s was below the time the package hooks legitimately need to stop and restart
# the service; being killed there makes apk roll the transaction back.
for pm in apk opkg; do
if grep -nE "(apk_with_lock_retry|run_logged_timeout) \"$pm\" 120 $pm (add|install)" "$INSTALLER" >/dev/null; then
fail "$pm package install must not keep the 120s ceiling that rolled upgrades back"
fi
done
printf 'PASS: installer rollback detection\n'
+51
View File
@@ -47,6 +47,21 @@ if grep -n -E 'grep -q "105 tachyon"|sed -i "/105 tachyon|tachyon_dont_touch_dhc
fi
grep -Fq '#!/usr/bin/ucode' "$TACHYON_MAKEFILE" ||
fail "tachyon Makefile package hooks must use ucode entrypoints"
# The prerm hook lives inside a Makefile define, so ucode_syntax_lint.sh never
# sees it. A syntax error there only surfaces on a router mid-upgrade.
sed -n '/define Package\/tachyon\/prerm/,/^endef/p' "$TACHYON_MAKEFILE" |
sed '1d;$d' >"$WORK_DIR/prerm-hook.uc"
[ -s "$WORK_DIR/prerm-hook.uc" ] ||
fail "tachyon Makefile must define a prerm hook"
ucode -c -o /dev/null "$WORK_DIR/prerm-hook.uc" 2>"$WORK_DIR/prerm-hook.err" ||
fail "tachyon Makefile prerm hook must compile: $(cat "$WORK_DIR/prerm-hook.err")"
# apk runs prerm as pre-upgrade and rolls the whole transaction back if it does
# not finish, so the cleanup call — which executes the OLD installed build — must
# be bounded from the outside.
grep -Fq 'timeout' "$WORK_DIR/prerm-hook.uc" ||
fail "tachyon Makefile prerm must bound package_prerm so a stuck cleanup cannot roll the upgrade back"
grep -Fq '/usr/bin/tachyon package_prerm' "$TACHYON_MAKEFILE" ||
fail "tachyon Makefile prerm must delegate cleanup to package_prerm"
grep -Fq '/usr/bin/tachyon package_postinst' "$TACHYON_MAKEFILE" ||
@@ -190,4 +205,40 @@ TACHYON_RT_TABLES="$WORK_DIR/rt_tables_upgrade" \
[ ! -e "$TACHYON_PACKAGE_UPGRADE_STATE" ] ||
fail "package pre-upgrade must not mark an already stopped service"
# Every init.d call goes through rc.common's `flock -w 1000`, so a wedged start
# or retry_start_on_wan_up holding that lock used to stall pre-upgrade for a
# quarter of an hour — long enough for apk (or the installer) to kill the hook and
# roll the new build back. The hook must give up on its own instead.
cat >"$WORK_DIR/hanging-init" <<'SH'
#!/usr/bin/env bash
sleep 600
SH
chmod 0755 "$WORK_DIR/hanging-init"
: >"$WORK_DIR/rt_tables_hang"
hang_start="$(date +%s)"
TACHYON_HOOK_COMMAND_TIMEOUT=2 \
TACHYON_INIT="$WORK_DIR/hanging-init" \
TACHYON_BIN="$WORK_DIR/hanging-init" \
TACHYON_DNS_APPLY_UC="$WORK_DIR/missing-dns-apply.uc" \
TACHYON_SING_BOX_INIT="$WORK_DIR/missing-sing-box-init" \
TACHYON_UCI_STATE_FILE="$WORK_DIR/dont-touch.state" \
TACHYON_RT_TABLES="$WORK_DIR/rt_tables_hang" \
ucode -L "$TACHYON_LIB" "$PACKAGE_UC" prerm upgrade
hang_elapsed="$(( $(date +%s) - hang_start ))"
[ "$hang_elapsed" -lt 30 ] ||
fail "package pre-upgrade must abandon a blocked init call, took ${hang_elapsed}s"
# Same for the restart in postinst: a start that never returns must not hold the
# package transaction open.
printf '1\n' >"$TACHYON_PACKAGE_UPGRADE_STATE"
hang_start="$(date +%s)"
TACHYON_HOOK_START_TIMEOUT=2 \
TACHYON_INIT="$WORK_DIR/hanging-init" \
ucode -L "$TACHYON_LIB" "$PACKAGE_UC" postinst
hang_elapsed="$(( $(date +%s) - hang_start ))"
[ "$hang_elapsed" -lt 30 ] ||
fail "package postinst must abandon a blocked service start, took ${hang_elapsed}s"
[ ! -e "$TACHYON_PACKAGE_UPGRADE_STATE" ] ||
fail "package postinst must clear the upgrade state even when the start times out"
printf 'package lifecycle checks passed\n'