From 7b32dc2e292d25ceb0a7c756272813cdc636fb96 Mon Sep 17 00:00:00 2001 From: librelad Date: Sat, 23 May 2026 16:39:56 +0100 Subject: [PATCH] fix(backup): clean snapshot-id capture + accept --latest on restore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found while testing live backups end-to-end: - Engine backup adapters logged to stdout, so the caller's $() snapshot-id capture was polluted with log text — verify-after-backup then failed with 'no matching ID' on every run. Route their log lines to stderr so stdout is only the id (restic/borg/kopia). - 'libreportal app restore --latest' (as the help advertises) and the bare 'restore ' both failed: --latest was passed to restic verbatim and unset args arrive as the literal 'empty'. Normalise both to 'latest'. Co-Authored-By: Claude Opus 4.7 Signed-off-by: librelad --- scripts/backup/engine/borg_backup.sh | 7 ++++--- scripts/backup/engine/kopia_backup.sh | 8 ++++---- scripts/backup/engine/restic_backup.sh | 10 ++++++---- scripts/cli/commands/app/cli_app_restore.sh | 8 +++++++- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/scripts/backup/engine/borg_backup.sh b/scripts/backup/engine/borg_backup.sh index d0282be..c63334f 100644 --- a/scripts/backup/engine/borg_backup.sh +++ b/scripts/backup/engine/borg_backup.sh @@ -29,7 +29,8 @@ borgBackupAppToLocation() local loc_name loc_name=$(resticLocationName "$idx") - isNotice "Snapshotting $app_name → $loc_name (archive: $archive)" + # Logs to stderr; stdout is only the archive name for the caller's $(). + isNotice "Snapshotting $app_name → $loc_name (archive: $archive)" >&2 sudo -E -u "$docker_install_user" borg create \ --comment "$comment" \ @@ -40,10 +41,10 @@ borgBackupAppToLocation() local rc=$? if [[ $rc -eq 0 ]]; then - isSuccessful "Backup created in $loc_name: $archive" + isSuccessful "Backup created in $loc_name: $archive" >&2 echo "$archive" else - isError "Backup to $loc_name failed for $app_name" + isError "Backup to $loc_name failed for $app_name" >&2 fi borgEnvUnset diff --git a/scripts/backup/engine/kopia_backup.sh b/scripts/backup/engine/kopia_backup.sh index 8555839..9fd46b8 100644 --- a/scripts/backup/engine/kopia_backup.sh +++ b/scripts/backup/engine/kopia_backup.sh @@ -20,7 +20,7 @@ kopiaBackupAppToLocation() local loc_name loc_name=$(resticLocationName "$idx") - isNotice "Snapshotting $app_name → $loc_name (kopia)" + isNotice "Snapshotting $app_name → $loc_name (kopia)" >&2 # Kopia has no per-run --exclude; it reads .kopiaignore from the source # tree. On the live path write the raw DB data dirs (made relative to the @@ -50,11 +50,11 @@ kopiaBackupAppToLocation() snapshot_id=$(echo "$output" | grep -oE '"id":\s*"[^"]+"' | head -1 | cut -d'"' -f4) if [[ $rc -eq 0 ]]; then - isSuccessful "Backup created in $loc_name: ${snapshot_id:0:12}" + isSuccessful "Backup created in $loc_name: ${snapshot_id:0:12}" >&2 echo "$snapshot_id" else - isError "Kopia backup to $loc_name failed for $app_name" - echo "$output" | tail -10 + isError "Kopia backup to $loc_name failed for $app_name" >&2 + echo "$output" | tail -10 >&2 fi kopiaEnvUnset diff --git a/scripts/backup/engine/restic_backup.sh b/scripts/backup/engine/restic_backup.sh index 422223f..248bfeb 100644 --- a/scripts/backup/engine/restic_backup.sh +++ b/scripts/backup/engine/restic_backup.sh @@ -32,7 +32,9 @@ resticBackupAppToLocation() local loc_name loc_name=$(resticLocationName "$idx") - isNotice "Snapshotting $app_name → $loc_name" + # Logs go to stderr so this function's stdout is ONLY the snapshot id — + # the caller captures it with $() and feeds it to verify/retention. + isNotice "Snapshotting $app_name → $loc_name" >&2 local output output=$(sudo -E -u "$docker_install_user" restic backup \ --host "$host_tag" \ @@ -47,11 +49,11 @@ resticBackupAppToLocation() snapshot_id=$(echo "$output" | grep -o '"snapshot_id":"[^"]*"' | tail -1 | cut -d'"' -f4) if [[ $rc -eq 0 ]]; then - isSuccessful "Backup created in $loc_name: ${snapshot_id:0:8}" + isSuccessful "Backup created in $loc_name: ${snapshot_id:0:8}" >&2 echo "$snapshot_id" else - isError "Backup to $loc_name failed for $app_name" - echo "$output" | tail -10 + isError "Backup to $loc_name failed for $app_name" >&2 + echo "$output" | tail -10 >&2 fi resticEnvUnset diff --git a/scripts/cli/commands/app/cli_app_restore.sh b/scripts/cli/commands/app/cli_app_restore.sh index c4f5d6d..ba7a964 100755 --- a/scripts/cli/commands/app/cli_app_restore.sh +++ b/scripts/cli/commands/app/cli_app_restore.sh @@ -9,7 +9,7 @@ cliAppRestore() { local app_name="$1" - local snapshot="${2:-latest}" + local snapshot="$2" local repo="$3" if [[ -z "$app_name" ]]; then @@ -18,5 +18,11 @@ cliAppRestore() { return 1 fi + # Normalise the snapshot selector: unset args arrive as the literal + # "empty" from the CLI wrapper, and the help advertises --latest, but + # restorePickSnapshot expects the bare token "latest". + [[ -z "$snapshot" || "$snapshot" == "empty" || "$snapshot" == "--latest" ]] && snapshot="latest" + [[ -z "$repo" || "$repo" == "empty" ]] && repo="" + restoreAppStart "$app_name" "$snapshot" "$repo" }