Webstar/LibrePortal
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(updater): stop a callee blanking the app name mid-update

Browse Source 
First real end-to-end auto-update on a live install failed like this:

  Automatically updating trivy (a recovery snapshot is taken first)
  Snapshotting trivy before update…
  Pulling new image(s) for …
  Update of  failed — rolling back…
  Could not roll  back automatically

The app name went empty after the snapshot. Cause: bash is dynamically
scoped, so a callee assigning an undeclared variable writes the CALLER's
local of that name — and a `while read app` loop leaves it EMPTY at EOF.
webuiBackupAppStatus's dashboard generator runs at the end of every backup
and did exactly that to updaterApplyApp's `app`.

Nothing was damaged: the pull ran against an empty name, failed before
touching the image, and the rollback was a no-op on a nonexistent app.

Fixed both ends. The generator (and three gluetun loops with the same
latent leak) now declare `local app`. updaterApplyApp/updaterRollbackApp
hold the name in `_upd_app` so they no longer depend on every callee's
hygiene, and updaterApplyAll stops leaking its own loop var.

This is exactly the untested path the roadmap flagged: "apply/revert not
yet exercised end-to-end on a live install with a pending update."

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
librelad 2026-08-11 16:37:14 +01:00
parent cdeb2d1658
commit 7fae6bc308
5 changed files with 50 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
containers/gluetun/scripts/gluetun_network.sh
View File

@ -62,6 +62,8 @@ appNetworkRegisterPorts_gluetun()
"SELECT name FROM apps WHERE status = 1 ORDER BY name;" 2>/dev/null)
local routed_lines=""
local app # local: bash is dynamically scoped, and a while-read loop
# leaves an undeclared name EMPTY in the CALLER's scope at EOF.
while IFS= read -r app; do
[[ -z "$app" || "$app" == "gluetun" ]] && continue
local app_config_file="${containers_dir}${app}/${app}.config"

2
containers/gluetun/scripts/gluetun_recreate_routed.sh
View File

@ -33,6 +33,8 @@ appGluetunRecreateRouted()
fi
local recreated=0
local app # local: bash is dynamically scoped, and a while-read loop
# leaves an undeclared name EMPTY in the CALLER's scope at EOF.
while IFS= read -r app; do
[[ -z "$app" || "$app" == "gluetun" ]] && continue
local app_config_file="${containers_dir}${app}/${app}.config"

2
containers/gluetun/scripts/gluetun_route_apps.sh
View File

@ -28,6 +28,8 @@ gluetunRouteExistingAppsPrompt()
fi
local eligible=()
local app # local: bash is dynamically scoped, and a while-read loop
# leaves an undeclared name EMPTY in the CALLER's scope at EOF.
while IFS= read -r app; do
[[ -z "$app" ]] && continue
local cfg_file="${containers_dir}${app}/${app}.config"

66
scripts/cli/commands/updater/cli_updater_commands.sh
View File

@ -158,18 +158,24 @@ updaterLastUpdateFrom()
# snapshot, docker compose for the image swap) so it shares their locking/logging.
updaterApplyApp()
{
local app="$1"
# `_upd_app`, not `app`: bash is dynamically scoped, so a callee that uses an
# undeclared `app` (a while-read loop leaves it EMPTY at EOF) reaches up and
# overwrites OUR local. That is not hypothetical — the backup dashboard
# generator, which runs at the end of the snapshot below, did exactly that,
# and the update then ran against an empty app name. The generator is fixed;
# this name makes the update immune to the next one.
local _upd_app="$1"
# "auto" when the updater's own policy enqueued this (CFG_<APP>_UPDATE_TYPE),
# "manual" when a person pressed Update. Recorded in History; changes nothing
# about how the update is applied — both take the snapshot, both can roll back.
local trigger="${2:-manual}"
local app_dir="$containers_dir/$app"
if [[ ! -d "$app_dir" ]]; then isError "App '$app' is not installed."; return 1; fi
local app_dir="$containers_dir/$_upd_app"
if [[ ! -d "$app_dir" ]]; then isError "App '$_upd_app' is not installed."; return 1; fi
if [[ "$trigger" == "auto" ]]; then
isHeader "Automatically updating $app (a recovery snapshot is taken first)"
isHeader "Automatically updating $_upd_app (a recovery snapshot is taken first)"
else
isHeader "Updating $app (a recovery snapshot is taken first)"
isHeader "Updating $_upd_app (a recovery snapshot is taken first)"
fi
# 1. DISASTER RECOVERY — snapshot before touching anything. Call the backup
@ -178,10 +184,10 @@ updaterApplyApp()
# `*)` default (a notice that exits 0), so the `if !` guard passed and the app
# was updated with NO snapshot — and rollback below was a no-op that reported
# success. backupAppStart is the real entry point and returns 0/1 honestly.
isNotice "Snapshotting $app before update…"
if ! backupAppStart "$app" >/dev/null 2>&1; then
isNotice "Pre-update snapshot did not complete cleanly — continuing is risky; aborting $app update."
updaterRecordHistory "$app" "update" "" "" "aborted-no-snapshot" "" "" "" "$trigger"
isNotice "Snapshotting $_upd_app before update…"
if ! backupAppStart "$_upd_app" >/dev/null 2>&1; then
isNotice "Pre-update snapshot did not complete cleanly — continuing is risky; aborting $_upd_app update."
updaterRecordHistory "$_upd_app" "update" "" "" "aborted-no-snapshot" "" "" "" "$trigger"
return 1
fi
@ -189,27 +195,27 @@ updaterApplyApp()
# exact build reference (repo:tag@sha256:…) even for a floating tag. Anchor is
# the <slug>-service image (updaterPrimaryImage), NOT the first line. If a
# prior rollback pinned a digest, unpin it first so we track the channel again.
local anchor; anchor="$(updaterPrimaryImage "$app" "$app_dir/docker-compose.yml")"
case "$anchor" in *@sha256:*) updaterSetAnchorRef "$app" "${anchor%%@*}"; anchor="${anchor%%@*}";; esac
local anchor; anchor="$(updaterPrimaryImage "$_upd_app" "$app_dir/docker-compose.yml")"
case "$anchor" in *@sha256:*) updaterSetAnchorRef "$_upd_app" "${anchor%%@*}"; anchor="${anchor%%@*}";; esac
local before_dig; before_dig="$(updaterRefDigest "$anchor")"
local before="$anchor${before_dig:+@$before_dig}"
# 3. Pull + recreate (uses the real, install-type-aware compose helpers).
isNotice "Pulling new image(s) for $app…"
if updaterComposePull "$app" && dockerComposeUp "$app" >/dev/null 2>&1; then
local after_ref; after_ref="$(updaterPrimaryImage "$app" "$app_dir/docker-compose.yml")"; after_ref="${after_ref%%@*}"
isNotice "Pulling new image(s) for $_upd_app…"
if updaterComposePull "$_upd_app" && dockerComposeUp "$_upd_app" >/dev/null 2>&1; then
local after_ref; after_ref="$(updaterPrimaryImage "$_upd_app" "$app_dir/docker-compose.yml")"; after_ref="${after_ref%%@*}"
local after_dig; after_dig="$(updaterRefDigest "$after_ref")"
local after="$after_ref${after_dig:+@$after_dig}"
updaterRecordHistory "$app" "update" "$before" "$after" "ok" "" "" "" "$trigger"
isSuccessful "$app updated. Rollback point retained."
updaterRecordHistory "$_upd_app" "update" "$before" "$after" "ok" "" "" "" "$trigger"
isSuccessful "$_upd_app updated. Rollback point retained."
webuiUpdaterScan >/dev/null 2>&1 || true
return 0
fi
# 4. Failure -> automatic rollback.
isNotice "Update of $app failed — rolling back to the pre-update snapshot…"
updaterRollbackApp "$app" "auto"
updaterRecordHistory "$app" "update" "$before" "" "rolled-back" "" "" "" "$trigger"
isNotice "Update of $_upd_app failed — rolling back to the pre-update snapshot…"
updaterRollbackApp "$_upd_app" "auto"
updaterRecordHistory "$_upd_app" "update" "$before" "" "rolled-back" "" "" "" "$trigger"
return 1
}
@ -221,6 +227,7 @@ updaterApplyAll()
return 0
fi
local IFS=','
local app # local: don't leak the loop var into callers
for app in $list; do
[[ -z "$app" ]] && continue
updaterApplyApp "$app" || failures=$((failures+1))
@ -232,27 +239,30 @@ updaterApplyAll()
# (called from the failure path of an apply).
updaterRollbackApp()
{
local app="$1" mode="$2"
[[ "$mode" != "auto" ]] && isHeader "Rolling $app back to its pre-update snapshot"
# `_upd_app` for the same dynamic-scoping reason as updaterApplyApp: the
# restore engine below is a deep call chain, and this function still needs
# the app's name after it returns.
local _upd_app="$1" mode="$2"
[[ "$mode" != "auto" ]] && isHeader "Rolling $_upd_app back to its pre-update snapshot"
# Re-pin the ANCHOR image to the exact build the app ran before the last
# update, so `up` below runs the OLD code — not the current channel head.
# Without this, restoring the data snapshot but recreating on the new `latest`
# image is "new code on old data", the hole a floating tag makes invisible.
# (Preserves the version sentinel; apply un-pins it again on the next update.)
local prev_ref; prev_ref="$(updaterLastUpdateFrom "$app")"
local prev_ref; prev_ref="$(updaterLastUpdateFrom "$_upd_app")"
if [[ -n "$prev_ref" && "$prev_ref" == *@sha256:* ]]; then
updaterSetAnchorRef "$app" "$prev_ref" && isNotice "Pinned $app back to its pre-update build."
updaterSetAnchorRef "$_upd_app" "$prev_ref" && isNotice "Pinned $_upd_app back to its pre-update build."
fi
# Delegate to the restore engine (latest snapshot for this app). Call the
# function directly — the old `backup app "$app" restore latest` CLI form was
# malformed (parsed as action="$app") so it silently did nothing yet exited 0.
if restoreAppStart "$app" latest "" >/dev/null 2>&1; then
dockerComposeUp "$app" >/dev/null 2>&1 || true
[[ "$mode" != "auto" ]] && updaterRecordHistory "$app" "rollback" "" "" "rolled-back"
isSuccessful "$app restored from its pre-update snapshot."
if restoreAppStart "$_upd_app" latest "" >/dev/null 2>&1; then
dockerComposeUp "$_upd_app" >/dev/null 2>&1 || true
[[ "$mode" != "auto" ]] && updaterRecordHistory "$_upd_app" "rollback" "" "" "rolled-back"
isSuccessful "$_upd_app restored from its pre-update snapshot."
return 0
fi
isError "Could not roll $app back automatically — restore manually from the Backups page."
isError "Could not roll $_upd_app back automatically — restore manually from the Backups page."
return 1
}

6
scripts/webui/data/generators/backup/webui_backup_dashboard.sh
View File

@ -58,6 +58,12 @@ webuiGenerateBackupDashboard()
local apps_json="["
first=true
# `local app` matters beyond hygiene: bash is dynamically scoped, so an
# undeclared loop variable here reaches up and overwrites the CALLER's
# variable of the same name — and a while-read loop leaves it EMPTY at EOF.
# This generator runs at the end of a backup, so it was blanking the `app`
# local of updaterApplyApp (which snapshots, then keeps using $app) mid-update.
local app
if [[ -f "$docker_dir/$db_file" ]]; then
while IFS= read -r app; do
[[ -z "$app" ]] && continue