From 536b5b8e10f1a04ba9333138e87731acf2bdf753 Mon Sep 17 00:00:00 2001 From: librelad Date: Thu, 27 Aug 2026 12:20:59 +0100 Subject: [PATCH] restore: stop the first-run bulk restore from silently dropping most of the host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An end-to-end run restored 4 of 13 apps and reported "First-run restore complete — 4 apps restored" as a success. Two truncations, and fixing the first had hidden the second: * the CLI dispatcher calls handlers with no arguments, so "$@"/shift inside one operate on an empty list. Fixed earlier with LP_CLI_ARGS. * LP_CLI_ARGS was built from start.sh's "$@" — but the root wrapper invokes start.sh with exactly nine hardcoded positional slots. So the array could never hold more than nine entries, and `${LP_CLI_ARGS[@]:5}` yielded at most four app names. The wrapper now forwards the real argv after those nine slots (they stay untouched: every dispatcher reads them, and unset ones must keep arriving as the literal "empty"), and start.sh reads it back as "${@:10}". Verified: a preflight given six apps checks six, where five was the previous ceiling. footprint_version 7 -> 8, since the wrapper is root-owned and baked at install. Two further fixes so a truncation cannot pass as success again: * restoreFirstRunBulk with no app list is now a whole-host restore — it discovers the host's apps and re-applies the preflight. The installer's report runs in its own process, so without this an app the user was told would be skipped got restored anyway. init.sh now passes no list, so a whole-host restore builds nothing that can be truncated. * it counts what actually landed and returns non-zero naming the failures, instead of reporting the length of the list it was handed. Co-Authored-By: Claude Opus 5 --- init.sh | 17 ++++--- .../commands/restore/cli_restore_commands.sh | 2 +- .../commands/restore/cli_restore_header.sh | 2 +- scripts/restore/restore_first_run.sh | 45 +++++++++++++++++-- start.sh | 17 +++++-- 5 files changed, 67 insertions(+), 16 deletions(-) diff --git a/init.sh b/init.sh index f93f0d1..7a93152 100755 --- a/init.sh +++ b/init.sh @@ -134,7 +134,7 @@ command_symlink="/usr/local/bin/libreportal" # `update apply` runs as the manager and CANNOT rewrite root-owned files, so a bump # tells the updater the new release needs a root re-install (which re-bakes them). # Recorded at install in $lp_lib_dir/.footprint_version. See docs/contributing/development.md. -footprint_version=7 +footprint_version=8 footprint_marker="$lp_lib_dir/.footprint_version" # Directories — three independently-relocatable roots (see scripts/source/paths.sh @@ -480,10 +480,6 @@ initRestoreFromBackup() return 0 fi - local -a app_list=() - local a - while IFS= read -r a; do [[ -n "$a" ]] && app_list+=("$a"); done <<< "$apps" - # Preflight before anything is written: each app's manifest is read out of # its own snapshot and checked against this machine, so an app this version # no longer ships, or one that will not fit, is skipped rather than restored @@ -501,7 +497,10 @@ initRestoreFromBackup() esac isNotice "Restoring — this takes a while." - "${as_manager[@]}" libreportal restore first-run bulk "$idx" "$source_host" "${app_list[@]}" + # No app list, deliberately: bulk discovers the host's apps and re-applies + # the preflight itself. Passing the list here is what let a 13-app restore + # arrive as four and still report success. + "${as_manager[@]}" libreportal restore first-run bulk "$idx" "$source_host" # The wizard would ask for identity, domains and apps that the backup has # already answered, so mark it done rather than asking twice. @@ -1947,7 +1946,11 @@ if [[ $command1 == "reset" ]]; then elif [ -f "$script_dir/start.sh" ]; then chmod 0755 "$script_dir"/* cd "$script_dir" - ./start.sh "$command1" "$command2" "$command3" "$command4" "$command5" "$command6" "$command7" "$command8" "$command9" + # The nine slots stay exactly as they were — every dispatcher reads them, + # and unset ones must keep arriving as the literal "empty". The REAL argv + # rides after them so commands that take an open-ended list of app names + # are not silently cut to four; start.sh reads it back as "${@:10}". + ./start.sh "$command1" "$command2" "$command3" "$command4" "$command5" "$command6" "$command7" "$command8" "$command9" "$@" else clone_and_install fi diff --git a/scripts/cli/commands/restore/cli_restore_commands.sh b/scripts/cli/commands/restore/cli_restore_commands.sh index 289543c..fc417c2 100755 --- a/scripts/cli/commands/restore/cli_restore_commands.sh +++ b/scripts/cli/commands/restore/cli_restore_commands.sh @@ -100,7 +100,7 @@ cliHandleRestoreCommands() restoreFirstRunDiscover "$name" ;; bulk) - [[ -z "$name" || -z "$extra" ]] && { isNotice "Usage: restore first-run bulk [app2 ...]"; return; } + [[ -z "$name" || -z "$extra" ]] && { isNotice "Usage: restore first-run bulk [app1 app2 ...] (no apps = whole host, preflight-filtered)"; return; } local loc_idx="$name" local source_host="$extra" # Same fix as `restore preflight`: `shift 4; "$@"` here was diff --git a/scripts/cli/commands/restore/cli_restore_header.sh b/scripts/cli/commands/restore/cli_restore_header.sh index f87b43f..ff98666 100755 --- a/scripts/cli/commands/restore/cli_restore_header.sh +++ b/scripts/cli/commands/restore/cli_restore_header.sh @@ -34,6 +34,6 @@ cliShowRestoreHelp() echo "restore first-run discover [loc_idx]" echo " Discover LibrePortal backups in a location (fresh-install flow)." echo "" - echo "restore first-run bulk [app2 ...]" + echo "restore first-run bulk [app1 app2 ...]" echo " Bulk-restore multiple apps on a fresh install." } diff --git a/scripts/restore/restore_first_run.sh b/scripts/restore/restore_first_run.sh index 485fd55..bf0c603 100644 --- a/scripts/restore/restore_first_run.sh +++ b/scripts/restore/restore_first_run.sh @@ -19,23 +19,62 @@ restoreFirstRunDiscover() return $rc } +# Restore a host's apps onto this machine. +# +# With no app list this is a WHOLE-HOST restore: the apps are discovered from +# the repository and filtered through the preflight. Both halves matter. +# +# Discovery, because an explicit list has to survive the CLI wrapper's fixed +# positional slots to get here — a 13-app restore arrived as four, restored +# those, and reported success. The wrapper now forwards the real argv, but a +# whole-host restore that never builds a list cannot be truncated at all. +# +# The preflight, because the installer prints its report in a separate process, +# so the decision it made there is gone by the time this runs. Without +# re-applying it, an app the user was told would be skipped — one this version +# no longer ships, or one too big for the disk — gets restored anyway. restoreFirstRunBulk() { local idx="$1" local source_host="$2" shift 2 local apps_to_restore=("$@") + local -i preflighted=0 if [[ ${#apps_to_restore[@]} -eq 0 ]]; then - isError "No apps specified for bulk first-run restore" + restorePreflightReport "$idx" "$source_host" >/dev/null 2>&1 + apps_to_restore=("${RESTORE_PREFLIGHT_OK[@]}") + preflighted=1 + fi + + if [[ ${#apps_to_restore[@]} -eq 0 ]]; then + isError "No apps to restore for '$source_host' in this repository" return 1 fi isHeader "First-run bulk restore from $(resticLocationName "$idx") (host=$source_host)" + (( preflighted )) && isNotice "Restoring ${#apps_to_restore[@]} apps the preflight approved." + # Count what actually landed. A per-app failure must not be reported as a + # complete restore — that is how "4 apps restored" read as success when + # nine had gone missing. + local app + local -i ok=0 bad=0 + local -a failed=() for app in "${apps_to_restore[@]}"; do - restoreAppStart "$app" "latest" "$idx" "$source_host" + if restoreAppStart "$app" "latest" "$idx" "$source_host"; then + ok=$(( ok + 1 )) + else + bad=$(( bad + 1 )); failed+=("$app") + fi done - isSuccessful "First-run restore complete — ${#apps_to_restore[@]} apps restored" + if (( bad > 0 )); then + isError "First-run restore finished with failures — $ok of ${#apps_to_restore[@]} restored" + isNotice "Failed: ${failed[*]}" + return 1 + fi + + isSuccessful "First-run restore complete — $ok apps restored" + return 0 } diff --git a/start.sh b/start.sh index 9602f38..1d6a42d 100755 --- a/start.sh +++ b/start.sh @@ -9,10 +9,19 @@ initial_command5="$5" initial_command6="$6" initial_command7="$7" # The full argument vector, for commands that take an open-ended list (restore -# preflight / first-run bulk take any number of app names). The CLI dispatcher -# calls its handlers with NO arguments, so "$@" and `shift` inside a handler -# operate on an empty list — which silently truncated those lists to nothing. -declare -a LP_CLI_ARGS=("$@") +# preflight / first-run bulk take any number of app names). Two separate +# truncations conspired here, and fixing only the first hid the second: +# +# * the CLI dispatcher calls its handlers with NO arguments, so "$@" and +# `shift` inside a handler operate on an empty list +# * the root wrapper invokes this script with exactly nine positional slots, +# so reading "$@" here caps any list at nine — which for `first-run bulk` +# meant four app names. A 13-app restore quietly restored four and then +# reported "First-run restore complete — 4 apps restored" as a success. +# +# So the wrapper now appends the real argv after those nine slots, and this is +# where it is read back. Offsets below are into the true argv, unchanged. +declare -a LP_CLI_ARGS=("${@:10}") displayLibrePortalLogo() {