From cb055b4b1fdf75623ffc938968aef5548ee82d5f Mon Sep 17 00:00:00 2001 From: librelad Date: Wed, 27 May 2026 15:32:44 +0100 Subject: [PATCH] fix(uninstall): wipe container sub-UID app data via root helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dockerDeleteData (uninstall) and the wipe-before-restore step in restoreAppStart both did `runFileOp rm -rf $containers_dir$app_name`, which runs as $CFG_DOCKER_INSTALL_USER (dockerinstall, uid 1002 on rootless). That user owns app-template files but CANNOT remove container sub-UID dirs created by the daemon's userns mapping — postgres data at uid 232070, nextcloud html at uid 33, etc. The rm therefore silently failed with rm: cannot remove '/libreportal-containers/invidious/postgresdata': Permission denied while still reporting " successfully uninstalled" — leaving the sub-UID directory tree on disk to confuse the next install and leak storage. Fix: route the wipe through a new `app-data-remove` action in the root-owned libreportal-ownership helper. Root can rm sub-UID files unconditionally. The helper validates the app name (alphanumeric + . _ -, no traversal), refuses the WebUI's own slot (libreportal), and is idempotent when the dir is already gone. Two callers updated: - scripts/docker/app/uninstall/delete_data.sh - scripts/restore/restore_app_start.sh The helper itself ships root-owned at /usr/local/lib/libreportal/, so a fresh install or release upgrade is needed to pick up the new action. Bumped init.sh footprint_version 2 → 3 so the runtime updater prompts a root re-install on the next release. Signed-off-by: librelad --- init.sh | 2 +- scripts/docker/app/uninstall/delete_data.sh | 6 +++++- scripts/restore/restore_app_start.sh | 5 ++++- scripts/system/libreportal-ownership | 21 ++++++++++++++++++++- 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/init.sh b/init.sh index d12ce28..cc47062 100755 --- a/init.sh +++ b/init.sh @@ -130,7 +130,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/DEVELOPMENT.md. -footprint_version=2 +footprint_version=3 footprint_marker="$lp_lib_dir/.footprint_version" # Directories — three independently-relocatable roots (see scripts/source/paths.sh diff --git a/scripts/docker/app/uninstall/delete_data.sh b/scripts/docker/app/uninstall/delete_data.sh index fb822ee..36fc1c4 100755 --- a/scripts/docker/app/uninstall/delete_data.sh +++ b/scripts/docker/app/uninstall/delete_data.sh @@ -7,7 +7,11 @@ dockerDeleteData() if [[ "$app_name" == "" ]]; then isError "No app_name provided, unable to continue..." else - local result=$(runFileOp rm -rf $containers_dir$app_name) + # Runs via the root-owned helper instead of runFileOp (= dockerinstall), + # so container sub-UID dirs (postgres uid 232070, www-data uid 33, …) + # are wiped instead of left behind with a "Permission denied" error + # and a misleading "successfully uninstalled" message. + runOwnership app-data-remove "$app_name" checkSuccess "Deleting $app_name install folder" fi diff --git a/scripts/restore/restore_app_start.sh b/scripts/restore/restore_app_start.sh index 8d73b0c..5e3dcf0 100644 --- a/scripts/restore/restore_app_start.sh +++ b/scripts/restore/restore_app_start.sh @@ -56,7 +56,10 @@ restoreAppStart() echo "---- $menu_number. Wiping existing app folder" echo "" if [[ -d "$containers_dir$stored_app_name" ]]; then - runFileOp rm -rf "${containers_dir:?}$stored_app_name" + # Root-owned helper, not runFileOp — restoring over an app that left + # sub-UID data behind (postgres, www-data, …) needs to actually wipe + # those dirs before laying the snapshot down. + runOwnership app-data-remove "$stored_app_name" fi ((menu_number++)) diff --git a/scripts/system/libreportal-ownership b/scripts/system/libreportal-ownership index 861c5c8..09fd192 100644 --- a/scripts/system/libreportal-ownership +++ b/scripts/system/libreportal-ownership @@ -211,6 +211,24 @@ app_data_nobody() { [[ -d "$d/data" ]] && chown -R 65534:65534 "$d/data" } +# Wipe an entire app data tree, including container sub-UID dirs the +# manager / dockerinstall user can't reach (e.g. invidious/postgresdata uid +# 232070, nextcloud/html uid 33). Used by uninstall + restore-overwrite — +# both previously ran `rm -rf` via runFileOp (= as dockerinstall) and silently +# left sub-UID dirs behind, breaking reinstall + leaking storage. +# Idempotent: a missing dir is success (caller wants "ensure gone"). Refuses +# the WebUI's own slot (libreportal) — removing it would brick the WebUI. +app_data_remove() { + local app="${1:-}" + [[ "$app" =~ ^[A-Za-z0-9._-]+$ && "$app" != "." && "$app" != ".." ]] \ + || { echo "libreportal-ownership: invalid app name" >&2; return 1; } + [[ "$app" == "libreportal" ]] \ + && { echo "libreportal-ownership: refusing to remove the WebUI app dir" >&2; return 1; } + local d="$CONTAINERS_DIR/$app" + [[ -d "$d" ]] || return 0 + rm -rf -- "$d" +} + # 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() { @@ -234,6 +252,7 @@ case "$action" in webui) webui;; taskdir) taskdir;; app-data-nobody) app_data_nobody "${1:-}";; + app-data-remove) app_data_remove "${1:-}";; app-file) app_file "${1:-}" "${2:-}";; - *) echo "usage: libreportal-ownership {reconcile [mode]|traversal|containers-top|backups-top|db-own|app-perms|webui|taskdir|app-data-nobody |app-file }" >&2; exit 2;; + *) echo "usage: libreportal-ownership {reconcile [mode]|traversal|containers-top|backups-top|db-own|app-perms|webui|taskdir|app-data-nobody |app-data-remove |app-file }" >&2; exit 2;; esac