From 8fad6c6a4d68ba9a5ca3692f413999fcda315bf4 Mon Sep 17 00:00:00 2001 From: librelad Date: Mon, 24 Aug 2026 20:39:35 +0100 Subject: [PATCH] fix(restore): take the source path from the snapshot, not local config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restore built its restic include filter from THIS host's containers root: engineRestoreSnapshot "$idx" "$id" "/" "$containers_dir$app" restic reproduces a snapshot's absolute paths, so that only works when both sides agree byte-for-byte. LibrePortal has shipped configurable roots for a while, so restoring a snapshot taken on a host installed with --containers-dir=/mnt/ssd/apps onto a default host matched no include path and restored NOTHING — with no error, because an include filter that matches nothing is not a failure. Storage locations turn that from a rare cross-host case into an ordinary one. storageSnapshotSourcePath asks the repository where the app actually lived. storageRestoreAppTo restores in place when that agrees with where the app belongs here, and stages-then-moves when it does not — which is also what makes "restore this app onto a different disk" possible at all. Both restore_app_start.sh and resticRestoreAppLatest go through it, and both fall back to the old behaviour when a snapshot does not report its paths, so older snapshots restore exactly as before. The move into place runs as root (app-adopt) for the same reason app-move does: a restored tree carries container sub-UIDs the manager cannot recreate. Staging is constrained to the restore/migrate area and the destination is validated against the root-owned registry, so neither end is taken on trust from the caller. The manifest now records where an app lived — location name, path and fs uuid. The name is what travels, since a path means nothing on the other host; the rest is for diagnostics and for answering "is this the same disk?" during a migrate. Co-Authored-By: Claude Opus 5 --- scripts/backup/engine/restic_restore.sh | 10 +- scripts/backup/manifest/manifest_collect.sh | 13 +++ scripts/restore/restore_app_start.sh | 10 +- scripts/source/files/arrays/files_storage.sh | 1 + .../source/files/arrays/function_manifest.sh | 6 ++ scripts/storage/storage_restore_path.sh | 96 +++++++++++++++++++ scripts/system/libreportal-ownership | 52 +++++++++- 7 files changed, 183 insertions(+), 5 deletions(-) create mode 100644 scripts/storage/storage_restore_path.sh diff --git a/scripts/backup/engine/restic_restore.sh b/scripts/backup/engine/restic_restore.sh index 254b459..7e057b0 100644 --- a/scripts/backup/engine/restic_restore.sh +++ b/scripts/backup/engine/restic_restore.sh @@ -118,7 +118,15 @@ resticRestoreAppLatest() return 1 fi - local include_path="$(appDir "$app_name")" + # Prefer the path the SNAPSHOT records over this host's layout: they differ + # whenever the snapshot came from a host with a different --containers-dir, + # or from a different storage location, and an include filter that matches + # nothing restores nothing without saying so. + local include_path="" + if declare -f storageSnapshotSourcePath >/dev/null 2>&1; then + include_path=$(storageSnapshotSourcePath "$idx" "$snapshot_id" "$app_name" 2>/dev/null) || include_path="" + fi + [[ -z "$include_path" ]] && include_path="$(appDir "$app_name")" resticRestoreSnapshot "$idx" "$snapshot_id" "$target_dir" "$include_path" } diff --git a/scripts/backup/manifest/manifest_collect.sh b/scripts/backup/manifest/manifest_collect.sh index 7f094f1..266e79f 100644 --- a/scripts/backup/manifest/manifest_collect.sh +++ b/scripts/backup/manifest/manifest_collect.sh @@ -47,6 +47,18 @@ manifestCollect() local file_count file_count=$(runFileOp find "$app_dir" -type f 2>/dev/null | wc -l | tr -d ' ') + # Where this app lived, so a restore or migrate onto a host with a + # different disk layout can resolve it instead of guessing. The NAME is what + # travels (paths mean nothing on the other host); the path and fs uuid are + # recorded for diagnostics and for the "is this the same disk?" question. + local storage_json='{}' + if declare -f storageLocationName >/dev/null 2>&1; then + local _root="${app_dir%/*}" _locname _fsuuid + _locname=$(storageLocationName "$_root" 2>/dev/null) || _locname="default" + _fsuuid=$(findmnt -no UUID --target "$_root" 2>/dev/null | tail -1) + storage_json="{\"location\":\"${_locname}\",\"path\":\"${app_dir}\",\"fs_uuid\":\"${_fsuuid}\"}" + fi + local strategy="${CFG_BACKUP_STRATEGY:-auto}" declare -f backupResolveStrategy >/dev/null 2>&1 && strategy=$(backupResolveStrategy "$app_name") @@ -75,6 +87,7 @@ manifestCollect() "size_bytes": $size_bytes, "file_count": $file_count, "strategy": "$strategy", + "storage": $storage_json, "databases": $databases_json } EOF diff --git a/scripts/restore/restore_app_start.sh b/scripts/restore/restore_app_start.sh index ee7d408..1fa2735 100644 --- a/scripts/restore/restore_app_start.sh +++ b/scripts/restore/restore_app_start.sh @@ -72,9 +72,13 @@ restoreAppStart() echo "" echo "---- $menu_number. Restoring snapshot ${chosen_id:0:8}" echo "" - local include_path="$(appDir "$stored_app_name")" - engineRestoreSnapshot "$chosen_idx" "$chosen_id" "/" "$include_path" - if [[ $? -ne 0 ]]; then + # Take the source path from the SNAPSHOT, not from local config. Building the + # include filter from this host's containers root only works when both sides + # agree byte-for-byte, so a snapshot taken on a host with a different + # --containers-dir (or on a different storage location) matched nothing and + # restored silently. storageRestoreAppTo restores in place when the paths + # agree and stages-then-moves when they do not. + if ! storageRestoreAppTo "$chosen_idx" "$chosen_id" "$stored_app_name"; then isError "Restore failed — leaving app in stopped state" return 1 fi diff --git a/scripts/source/files/arrays/files_storage.sh b/scripts/source/files/arrays/files_storage.sh index 541a751..c0f75bf 100644 --- a/scripts/source/files/arrays/files_storage.sh +++ b/scripts/source/files/arrays/files_storage.sh @@ -9,6 +9,7 @@ storage_scripts=( "storage/storage_disks.sh" "storage/storage_locations.sh" "storage/storage_move.sh" + "storage/storage_restore_path.sh" "storage/storage_scan.sh" ) diff --git a/scripts/source/files/arrays/function_manifest.sh b/scripts/source/files/arrays/function_manifest.sh index de37146..7df9cae 100644 --- a/scripts/source/files/arrays/function_manifest.sh +++ b/scripts/source/files/arrays/function_manifest.sh @@ -1029,9 +1029,11 @@ declare -gA LP_FN_MAP=( [_storageProbeDir]="storage/storage_checks.sh" [_storageRefreshWebui]="storage/storage_locations.sh" [storageRemove]="storage/storage_locations.sh" + [storageRestoreAppTo]="storage/storage_restore_path.sh" [storageScan]="storage/storage_scan.sh" [storageScanCandidates]="storage/storage_scan.sh" [_storageSkipTarget]="storage/storage_scan.sh" + [storageSnapshotSourcePath]="storage/storage_restore_path.sh" [storageSyncAllAppComments]="storage/storage_app_config.sh" [storageSyncAppComment]="storage/storage_app_config.sh" [_storageWriteLocationConfig]="storage/storage_locations.sh" @@ -2249,9 +2251,11 @@ declare -gA LP_FN_ROOT=( [_storageProbeDir]="scripts" [_storageRefreshWebui]="scripts" [storageRemove]="scripts" + [storageRestoreAppTo]="scripts" [storageScan]="scripts" [storageScanCandidates]="scripts" [_storageSkipTarget]="scripts" + [storageSnapshotSourcePath]="scripts" [storageSyncAllAppComments]="scripts" [storageSyncAppComment]="scripts" [_storageWriteLocationConfig]="scripts" @@ -3506,9 +3510,11 @@ _storageOptionList() { unset -f _storageOptionList; __lpAutoload "${install_scri _storageProbeDir() { unset -f _storageProbeDir; __lpAutoload "${install_scripts_dir}storage/storage_checks.sh"; _storageProbeDir "$@"; } _storageRefreshWebui() { unset -f _storageRefreshWebui; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; _storageRefreshWebui "$@"; } storageRemove() { unset -f storageRemove; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; storageRemove "$@"; } +storageRestoreAppTo() { unset -f storageRestoreAppTo; __lpAutoload "${install_scripts_dir}storage/storage_restore_path.sh"; storageRestoreAppTo "$@"; } storageScan() { unset -f storageScan; __lpAutoload "${install_scripts_dir}storage/storage_scan.sh"; storageScan "$@"; } storageScanCandidates() { unset -f storageScanCandidates; __lpAutoload "${install_scripts_dir}storage/storage_scan.sh"; storageScanCandidates "$@"; } _storageSkipTarget() { unset -f _storageSkipTarget; __lpAutoload "${install_scripts_dir}storage/storage_scan.sh"; _storageSkipTarget "$@"; } +storageSnapshotSourcePath() { unset -f storageSnapshotSourcePath; __lpAutoload "${install_scripts_dir}storage/storage_restore_path.sh"; storageSnapshotSourcePath "$@"; } storageSyncAllAppComments() { unset -f storageSyncAllAppComments; __lpAutoload "${install_scripts_dir}storage/storage_app_config.sh"; storageSyncAllAppComments "$@"; } storageSyncAppComment() { unset -f storageSyncAppComment; __lpAutoload "${install_scripts_dir}storage/storage_app_config.sh"; storageSyncAppComment "$@"; } _storageWriteLocationConfig() { unset -f _storageWriteLocationConfig; __lpAutoload "${install_scripts_dir}storage/storage_locations.sh"; _storageWriteLocationConfig "$@"; } diff --git a/scripts/storage/storage_restore_path.sh b/scripts/storage/storage_restore_path.sh new file mode 100644 index 0000000..a902570 --- /dev/null +++ b/scripts/storage/storage_restore_path.sh @@ -0,0 +1,96 @@ +#!/bin/bash + +# Resolving where a snapshot's data actually lives, and where it should land. +# +# The bug this exists to fix predates storage locations. Restore did: +# +# engineRestoreSnapshot "$idx" "$id" "/" "$containers_dir$app" +# +# — restore to / with an include filter built from the LOCAL containers root. +# restic reproduces a snapshot's absolute paths, so that only works when source +# and destination paths are byte-identical. LibrePortal has shipped three +# configurable roots for a while, so migrating from a host installed with +# --containers-dir=/mnt/ssd/apps onto a default host matched no include path and +# restored NOTHING, silently. Storage locations make that ordinary rather than +# rare. +# +# The fix is to take the source path from the SNAPSHOT rather than from local +# config, and to stage-and-move whenever it differs from where the app belongs +# here. + +# The path an app occupied in a snapshot. restic records the backed-up paths on +# the snapshot itself, so ask the repository rather than guessing. +storageSnapshotSourcePath() +{ + local idx="$1" snapshot_id="$2" app="$3" + local json path + + json=$(engineSnapshotsJson "$idx" "$snapshot_id" 2>/dev/null) || return 1 + # "paths":["/libreportal-containers/bookstack"] + path=$(printf '%s' "$json" \ + | grep -o '"paths":\[[^]]*\]' \ + | head -1 \ + | grep -o '"/[^"]*"' \ + | tr -d '"' \ + | grep -E "/${app}(/|$)" \ + | head -1) + [[ -n "$path" ]] || return 1 + printf '%s' "${path%/}" +} + +# Restore an app from a snapshot to wherever it belongs on THIS host. +# +# Same path on both sides -> restore in place, exactly as before. +# Different -> restore into staging, then move the tree into position. The move +# goes through the root helper because the restored tree carries container +# sub-UIDs the manager cannot handle. +storageRestoreAppTo() +{ + local idx="$1" snapshot_id="$2" app="$3" + + local dest + if ! dest=$(appDir "$app"); then + isError "Cannot restore $app — its storage location is not mounted." + return 1 + fi + + local src + if ! src=$(storageSnapshotSourcePath "$idx" "$snapshot_id" "$app"); then + # Older snapshot, or an engine that does not report paths: fall back to + # the historical behaviour rather than refusing. + isNotice "Snapshot does not report its source path — restoring in place." + engineRestoreSnapshot "$idx" "$snapshot_id" "/" "$dest" + return $? + fi + + if [[ "${src%/}" == "${dest%/}" ]]; then + engineRestoreSnapshot "$idx" "$snapshot_id" "/" "$src" + return $? + fi + + isNotice "This snapshot was taken at '$src'; restoring to '$dest'." + + local stage="${restore_dir%/}/relocate-$app.$$" + runInstallOp mkdir -p "$stage" + if ! engineRestoreSnapshot "$idx" "$snapshot_id" "$stage" "$src"; then + isError "Restore into staging failed." + runInstallOp rm -rf "$stage" + return 1 + fi + + # restic reproduces the full absolute path beneath --target. + local staged="$stage/${src#/}" + if [[ ! -d "$staged" ]]; then + isError "Restored tree not found at '$staged' — leaving staging in place for inspection." + return 1 + fi + + if ! runOwnership app-adopt "$app" "$staged" "${dest%/*}"; then + isError "Could not move the restored tree into '$dest' — it is still at '$staged'." + return 1 + fi + + runInstallOp rm -rf "$stage" + isSuccessful "Restored $app to $dest" + return 0 +} diff --git a/scripts/system/libreportal-ownership b/scripts/system/libreportal-ownership index 0f5164a..d259978 100644 --- a/scripts/system/libreportal-ownership +++ b/scripts/system/libreportal-ownership @@ -322,6 +322,55 @@ app_move() { return 0 } +# Adopt a restored tree into place: move to /. +# +# Separate from app_move because the source is a staging directory under the +# system tree, not a live app dir — but the same reasoning applies: the tree +# carries container sub-UIDs the manager cannot recreate, and the destination +# root is validated against the ROOT-OWNED registry rather than trusted from +# the caller. +app_adopt() { + local app="${1:-}" staged="${2:-}" dest_root="${3:-}" + [[ "$app" =~ ^[A-Za-z0-9._-]+$ && "$app" != "." && "$app" != ".." ]] \ + || { echo "libreportal-ownership: invalid app name" >&2; return 1; } + [[ -n "$staged" && "$staged" == /* && -d "$staged" ]] \ + || { echo "libreportal-ownership: staged tree must be an existing absolute path" >&2; return 1; } + [[ "$staged" == *..* ]] \ + && { echo "libreportal-ownership: invalid staged path" >&2; return 1; } + # Staging must live under the system tree — never an arbitrary location. + [[ "$staged" == "$RESTORE_DIR"/* || "$staged" == "$MIGRATE_DIR"/* ]] \ + || { echo "libreportal-ownership: staged tree must be under the restore/migrate area" >&2; return 1; } + [[ -n "$dest_root" && "$dest_root" == /* ]] \ + || { echo "libreportal-ownership: destination must be an absolute path" >&2; return 1; } + dest_root="${dest_root%/}" + + local ok=0 + [[ "$dest_root" == "$CONTAINERS_DIR" ]] && ok=1 + if (( ! ok )) && [[ -r "$STORAGE_REGISTRY" ]]; then + local _id _path _rest + while IFS=$'\t' read -r _id _path _rest || [[ -n "$_id" ]]; do + [[ -z "$_path" || "$_id" == \#* ]] && continue + [[ "${_path%/}" == "$dest_root" ]] && { ok=1; break; } + done < "$STORAGE_REGISTRY" + fi + (( ok )) || { echo "libreportal-ownership: '$dest_root' is not a registered storage location" >&2; return 1; } + + local dst="$dest_root/$app" + rm -rf -- "$dst" + mkdir -p -- "$dest_root" + + local s_dev d_dev + s_dev=$(stat -c '%d' -- "$staged" 2>/dev/null) + d_dev=$(stat -c '%d' -- "$dest_root" 2>/dev/null) + if [[ -n "$s_dev" && "$s_dev" == "$d_dev" ]]; then + mv -- "$staged" "$dst" || return 1 + else + cp -a --reflink=auto -- "$staged" "$dst" || { rm -rf -- "$dst"; return 1; } + rm -rf -- "$staged" + fi + return 0 +} + # Chown one LibrePortal-managed file under an app dir to the container owner. # relpath is validated: no traversal, no absolute path, safe charset only. app_file() { @@ -349,5 +398,6 @@ case "$action" in app-data-remove) app_data_remove "${1:-}";; app-file) app_file "${1:-}" "${2:-}";; app-move) app_move "${1:-}" "${2:-}";; - *) echo "usage: libreportal-ownership {reconcile [mode]|traversal|containers-top|backups-top|db-own|app-perms|webui|webui-bind|taskdir|app-data-nobody |app-data-remove |app-file |app-move }" >&2; exit 2;; + app-adopt) app_adopt "${1:-}" "${2:-}" "${3:-}";; + *) echo "usage: libreportal-ownership {reconcile [mode]|traversal|containers-top|backups-top|db-own|app-perms|webui|webui-bind|taskdir|app-data-nobody |app-data-remove |app-file |app-move |app-adopt }" >&2; exit 2;; esac