fix(app): make export/import actually work — found by a real round trip

Ran export -> uninstall -> import on trivy against the live install. It
worked end to end (1.3G app, marker file byte-identical afterwards,
container running, database status correct, tree owned by the container
user) but only after three real bugs, none of which syntax checks or
isolated tests would have caught.

Export wrote the tarball as the CONTAINER user, because tar has to read
app data holding sub-UIDs the manager cannot. That meant the container
user also had to be able to create the destination file, which fails for
any normal destination. Now tar writes to stdout and the caller's shell
creates the file: reading uses the privileges that need it, writing uses
the caller's. Import had the mirror-image bug — tar extracted as the
container user and so could not READ a manager-owned .lpapp; the caller
now opens it and tar reads stdin.

Export also failed at tar time with no hint that the destination was the
problem, so it checks the directory exists and is writable up front.

The third one was quiet and worse. The manifest is pretty-printed, so it
reads `"size_bytes": 1324973614` — with a space that a `"key":[0-9]*`
pattern does not match. Both size_bytes and storage.location came back
empty everywhere they were read, which turned "will it fit" and "does
that location still exist" into checks that always passed. That is the
failure mode preflight exists to prevent, hiding inside preflight itself.
Fixed in app_portable.sh and restore_preflight.sh.

Verified afterwards with crafted manifests: an app claiming 8 TB is now
refused on an 800 GB disk ("Needs 8192G, 806G free"), and one naming a
location this machine lacks warns and names the fallback.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
librelad 2026-08-27 11:33:52 +01:00
parent aa44e0b542
commit 1381b052ae
2 changed files with 41 additions and 9 deletions

View File

@ -57,6 +57,17 @@ appExport()
isError "'$out' already exists — refusing to overwrite it."
return 1
fi
# Check up front rather than failing at tar time: the error there is a bare
# non-zero exit with no hint that the destination was the problem.
local out_dir="${out%/*}"; [[ -z "$out_dir" ]] && out_dir="."
if [[ ! -d "$out_dir" ]]; then
isError "'$out_dir' does not exist."
return 1
fi
if [[ ! -w "$out_dir" ]]; then
isError "Cannot write to '$out_dir' — pick somewhere $(id -un) can write."
return 1
fi
isHeader "Export $app"
@ -74,15 +85,21 @@ appExport()
fi
isNotice "Writing $out"
# Run as the owner: app data holds container sub-UIDs the manager cannot
# read. --numeric-owner so those uids survive the round trip rather than
# being remapped through this machine's /etc/passwd.
if ! runFileOp tar --numeric-owner -C "${dir%/*}" -czf "$out" "$app" 2>/dev/null; then
# Split the privileges deliberately: tar READS as the container user, because
# app data holds sub-UIDs the manager cannot read — but it writes to STDOUT,
# and the destination file is created by this shell, as whoever ran the
# command. Writing directly from tar meant the container user had to be able
# to create the file too, which fails for any normal destination.
#
# --numeric-owner so container uids survive the round trip rather than being
# remapped through this machine's /etc/passwd.
if ! runFileOp tar --numeric-owner -C "${dir%/*}" -czf - "$app" > "$out" 2>/dev/null; then
isError "Export failed."
rm -f "$out"
(( was_running )) && dockerComposeUp "$app" >/dev/null 2>&1
return 1
fi
runFileOp chmod 0640 "$out" 2>/dev/null
chmod 0640 "$out" 2>/dev/null
if (( was_running )); then
isNotice "Starting $app again."
@ -156,7 +173,12 @@ appImportCheck()
continue
fi
manifest=$(appImportManifest "$f")
# Squeeze whitespace first. The manifest is PRETTY-PRINTED, so the real
# text is `"size_bytes": 1324973614` — with a space that a
# `"key":[0-9]*` pattern does not match. Both the size and the location
# silently came back empty, which turned the "will it fit" and "does
# that location exist" checks into no-ops that always passed.
manifest=$(appImportManifest "$f" | tr -d ' \n\t')
size_bytes=$(printf '%s' "$manifest" | grep -o '"size_bytes":[0-9]*' | head -1 | cut -d: -f2)
loc=$(printf '%s' "$manifest" | grep -o '"location":"[^"]*"' | head -1 | cut -d'"' -f4)
[[ "$size_bytes" =~ ^[0-9]+$ ]] || size_bytes=0
@ -264,7 +286,8 @@ appImport()
# --- the same checks the restore preflight makes -------------------------
local manifest size_bytes loc
manifest=$(appImportManifest "$file")
# Whitespace-squeezed: the manifest is pretty-printed (see appImportCheck).
manifest=$(appImportManifest "$file" | tr -d ' \n\t')
size_bytes=$(printf '%s' "$manifest" | grep -o '"size_bytes":[0-9]*' | head -1 | cut -d: -f2)
loc=$(printf '%s' "$manifest" | grep -o '"location":"[^"]*"' | head -1 | cut -d'"' -f4)
@ -300,7 +323,12 @@ appImport()
# --- unpack ---------------------------------------------------------------
isNotice "Unpacking into $dir"
runFileOp mkdir -p "${dir%/*}"
if ! runFileOp tar --numeric-owner -xzf "$file" -C "${dir%/*}" 2>/dev/null; then
# Mirror of the export split: the caller's shell opens the .lpapp (it may sit
# anywhere the user can read), and tar EXTRACTS as the container user so the
# unpacked tree lands with the right ownership. Passing the path to tar
# instead required the container user to be able to read the file, which
# fails for any file the user created themselves.
if ! runFileOp tar --numeric-owner -xzf - -C "${dir%/*}" < "$file" 2>/dev/null; then
isError "Unpack failed — removing the partial directory."
runFileOp rm -rf "$dir"
return 1

View File

@ -61,7 +61,11 @@ restorePreflightApp()
local idx="$1" app="$2" host="$3"
local manifest size_bytes size_h loc want_dir avail_kb need_kb
manifest=$(restorePreflightManifest "$idx" "$app" "$host" 2>/dev/null)
# Squeezed: the manifest is pretty-printed, so `"size_bytes": 123` carries a
# space that a `"key":[0-9]*` pattern misses — which silently emptied both
# the size and the location, turning the fit and location checks into
# no-ops that always passed.
manifest=$(restorePreflightManifest "$idx" "$app" "$host" 2>/dev/null | tr -d ' \n\t')
size_bytes=$(printf '%s' "$manifest" | grep -o '"size_bytes":[0-9]*' | head -1 | cut -d: -f2)
loc=$(printf '%s' "$manifest" | grep -o '"location":"[^"]*"' | head -1 | cut -d'"' -f4)
size_h=$(_restorePfSize "${size_bytes:-0}")