From 7acfdabbaca2c3b50e33e93ef785910574a9b23b Mon Sep 17 00:00:00 2001 From: librelad Date: Sun, 24 May 2026 17:01:05 +0100 Subject: [PATCH] refactor(de-sudo): backup subsystem data ops via runFileOp/runFileWrite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The backup engine already drops to the backup user (sudo -E -u $docker_install_user) and backupLocationOwner == $docker_install_user, which is exactly what runFileOp/runFileWrite resolve to in both modes. So convert the raw-sudo data ops (mkdir/chmod/rm/find/cat/grep/mv/chown/tee on backup repos, location configs, keys, manifests) to runFileOp/runFileWrite — creating files as the owner directly, no root chown. backup_verify creates its scratch as the backup user (runFileOp mktemp) instead of chown-after. Binary installs (kopia tar/install, borg dnf) -> runSystem. The 44 sudo -u engine drops stay (already least-privilege; the scoped sudoers will grant them). Co-Authored-By: Claude Opus 4.7 Signed-off-by: librelad --- scripts/backup/engine/borg_init.sh | 4 +- scripts/backup/engine/borg_install.sh | 2 +- scripts/backup/engine/kopia_init.sh | 4 +- scripts/backup/engine/kopia_install.sh | 4 +- scripts/backup/engine/kopia_restore.sh | 4 +- scripts/backup/engine/restic_env.sh | 2 +- scripts/backup/engine/restic_init.sh | 4 +- scripts/backup/engine/restic_restore.sh | 2 +- scripts/backup/locations/location_add.sh | 6 +-- scripts/backup/locations/location_loader.sh | 2 +- scripts/backup/locations/location_migrate.sh | 42 ++++++++++---------- scripts/backup/locations/location_paths.sh | 6 +-- scripts/backup/locations/location_remove.sh | 2 +- scripts/backup/locations/location_ssh.sh | 14 +++---- scripts/backup/manifest/manifest_collect.sh | 4 +- scripts/backup/manifest/manifest_write.sh | 8 ++-- scripts/backup/verify/backup_verify.sh | 11 ++--- 17 files changed, 61 insertions(+), 60 deletions(-) diff --git a/scripts/backup/engine/borg_init.sh b/scripts/backup/engine/borg_init.sh index 09bce80..43414e9 100644 --- a/scripts/backup/engine/borg_init.sh +++ b/scripts/backup/engine/borg_init.sh @@ -12,8 +12,8 @@ borgInitLocation() borgEnvExport "$idx" || return 1 if [[ "$(resticLocationType "$idx")" == "local" ]]; then - sudo mkdir -p "$BORG_REPO" - sudo chown -R "$docker_install_user":"$docker_install_user" "$BORG_REPO" + runFileOp mkdir -p "$BORG_REPO" + runFileOp chown -R "$docker_install_user":"$docker_install_user" "$BORG_REPO" fi if sudo -E -u "$docker_install_user" borg info "$BORG_REPO" >/dev/null 2>&1; then diff --git a/scripts/backup/engine/borg_install.sh b/scripts/backup/engine/borg_install.sh index 35c8a1e..d02d65e 100644 --- a/scripts/backup/engine/borg_install.sh +++ b/scripts/backup/engine/borg_install.sh @@ -13,7 +13,7 @@ borgInstall() if command -v apt-get >/dev/null 2>&1; then runSystem apt-get install -y borgbackup && return 0 elif command -v dnf >/dev/null 2>&1; then - sudo dnf install -y borgbackup && return 0 + runSystem dnf install -y borgbackup && return 0 elif command -v pacman >/dev/null 2>&1; then runSystem pacman -S --noconfirm borg && return 0 fi diff --git a/scripts/backup/engine/kopia_init.sh b/scripts/backup/engine/kopia_init.sh index fb7f422..249dcff 100644 --- a/scripts/backup/engine/kopia_init.sh +++ b/scripts/backup/engine/kopia_init.sh @@ -27,8 +27,8 @@ kopiaInitLocation() local) local path path=$(backupLocationResolvedPath "$idx") - sudo mkdir -p "$path" - sudo chown -R "$docker_install_user":"$docker_install_user" "$path" + runFileOp mkdir -p "$path" + runFileOp chown -R "$docker_install_user":"$docker_install_user" "$path" args=(repository create filesystem --path="$path") ;; sftp) diff --git a/scripts/backup/engine/kopia_install.sh b/scripts/backup/engine/kopia_install.sh index 0b154dd..3d98cbd 100644 --- a/scripts/backup/engine/kopia_install.sh +++ b/scripts/backup/engine/kopia_install.sh @@ -36,7 +36,7 @@ kopiaInstall() return 1 fi - sudo tar xzf "$tmp/kopia.tgz" -C "$tmp" + runSystem tar xzf "$tmp/kopia.tgz" -C "$tmp" local bin bin=$(find "$tmp" -name kopia -type f -executable | head -1) if [[ -z "$bin" ]]; then @@ -44,7 +44,7 @@ kopiaInstall() isError "Kopia binary not found in archive" return 1 fi - sudo install -m 0755 "$bin" /usr/local/bin/kopia + runSystem install -m 0755 "$bin" /usr/local/bin/kopia rm -rf "$tmp" checkSuccess "Kopia v${version} installed to /usr/local/bin/kopia" } diff --git a/scripts/backup/engine/kopia_restore.sh b/scripts/backup/engine/kopia_restore.sh index add371e..efb33b0 100644 --- a/scripts/backup/engine/kopia_restore.sh +++ b/scripts/backup/engine/kopia_restore.sh @@ -13,7 +13,7 @@ kopiaRestoreSnapshot() fi kopiaEnvExport "$idx" || return 1 - sudo mkdir -p "$target_dir" + runFileOp mkdir -p "$target_dir" isNotice "Restoring ${snapshot_id:0:12} from $(resticLocationName "$idx") → $target_dir" # Kopia's restore lays down the snapshot's source tree relative to target. @@ -22,7 +22,7 @@ kopiaRestoreSnapshot() local final_target="$target_dir" if [[ -n "$include_path" ]]; then final_target="$target_dir/${include_path#/}" - sudo mkdir -p "$final_target" + runFileOp mkdir -p "$final_target" fi sudo -E -u "$docker_install_user" kopia snapshot restore "$snapshot_id" "$final_target" local rc=$? diff --git a/scripts/backup/engine/restic_env.sh b/scripts/backup/engine/restic_env.sh index 02de227..d691e10 100644 --- a/scripts/backup/engine/restic_env.sh +++ b/scripts/backup/engine/restic_env.sh @@ -9,7 +9,7 @@ resticAllLocationIndices() compgen -v 2>/dev/null | grep -oE '^CFG_BACKUP_LOC_[0-9]+_NAME$' | grep -oE '[0-9]+' | sort -n -u return fi - sudo find "$dir" -mindepth 2 -maxdepth 2 -name location.config -type f 2>/dev/null \ + runFileOp find "$dir" -mindepth 2 -maxdepth 2 -name location.config -type f 2>/dev/null \ | awk -F/ '{print $(NF-1)}' \ | grep -E '^[0-9]+$' \ | sort -n -u diff --git a/scripts/backup/engine/restic_init.sh b/scripts/backup/engine/restic_init.sh index fe5b7e6..06029b3 100644 --- a/scripts/backup/engine/restic_init.sh +++ b/scripts/backup/engine/restic_init.sh @@ -12,8 +12,8 @@ resticInitLocation() resticEnvExport "$idx" || return 1 if [[ "$(resticLocationType "$idx")" == "local" ]]; then - sudo mkdir -p "$RESTIC_REPOSITORY" - sudo chown -R "$docker_install_user":"$docker_install_user" "$RESTIC_REPOSITORY" + runFileOp mkdir -p "$RESTIC_REPOSITORY" + runFileOp chown -R "$docker_install_user":"$docker_install_user" "$RESTIC_REPOSITORY" fi if sudo -E -u "$docker_install_user" restic snapshots --json --no-lock >/dev/null 2>&1; then diff --git a/scripts/backup/engine/restic_restore.sh b/scripts/backup/engine/restic_restore.sh index a2e31ae..f1dde74 100644 --- a/scripts/backup/engine/restic_restore.sh +++ b/scripts/backup/engine/restic_restore.sh @@ -14,7 +14,7 @@ resticRestoreSnapshot() resticEnvExport "$idx" || return 1 - sudo mkdir -p "$target_dir" + runFileOp mkdir -p "$target_dir" local args=(restore "$snapshot_id" --target "$target_dir") [[ -n "$include_path" ]] && args+=(--include "$include_path") diff --git a/scripts/backup/locations/location_add.sh b/scripts/backup/locations/location_add.sh index a85ee47..ebb1b7a 100644 --- a/scripts/backup/locations/location_add.sh +++ b/scripts/backup/locations/location_add.sh @@ -55,9 +55,9 @@ locationAdd() echo "CFG_BACKUP_LOC_${idx}_KEEP_WEEKLY=" echo "CFG_BACKUP_LOC_${idx}_KEEP_MONTHLY=" echo "CFG_BACKUP_LOC_${idx}_KEEP_YEARLY=" - } | sudo tee "$cfg_file" >/dev/null - sudo chown "$owner":"$owner" "$cfg_file" - sudo chmod 0640 "$cfg_file" + } | runFileWrite "$cfg_file" >/dev/null + runFileOp chown "$owner":"$owner" "$cfg_file" + runFileOp chmod 0640 "$cfg_file" if declare -f replacePlainPasswords >/dev/null 2>&1; then replacePlainPasswords "$cfg_file" diff --git a/scripts/backup/locations/location_loader.sh b/scripts/backup/locations/location_loader.sh index 659fb70..9e04d61 100644 --- a/scripts/backup/locations/location_loader.sh +++ b/scripts/backup/locations/location_loader.sh @@ -14,5 +14,5 @@ sourceBackupLocations() local cfg while IFS= read -r -d '' cfg; do [[ -f "$cfg" ]] && source "$cfg" - done < <(sudo find "$dir" -mindepth 2 -maxdepth 2 -name location.config -type f -print0 2>/dev/null) + done < <(runFileOp find "$dir" -mindepth 2 -maxdepth 2 -name location.config -type f -print0 2>/dev/null) } diff --git a/scripts/backup/locations/location_migrate.sh b/scripts/backup/locations/location_migrate.sh index d167255..1895217 100644 --- a/scripts/backup/locations/location_migrate.sh +++ b/scripts/backup/locations/location_migrate.sh @@ -17,7 +17,7 @@ backupLocationsMigrate() # Already migrated? Detect by presence of at least one location.config. if [[ -d "$new_dir" ]]; then local existing - existing=$(sudo find "$new_dir" -mindepth 2 -maxdepth 2 -name location.config -type f 2>/dev/null | head -1) + existing=$(runFileOp find "$new_dir" -mindepth 2 -maxdepth 2 -name location.config -type f 2>/dev/null | head -1) if [[ -n "$existing" ]]; then return 0 fi @@ -32,12 +32,12 @@ backupLocationsMigrate() local owner owner=$(backupLocationOwner) - sudo mkdir -p "$new_dir" - sudo chown "$owner":"$owner" "$new_dir" - sudo chmod 0755 "$new_dir" + runFileOp mkdir -p "$new_dir" + runFileOp chown "$owner":"$owner" "$new_dir" + runFileOp chmod 0755 "$new_dir" local indices - indices=$(sudo grep -oE '^CFG_BACKUP_LOC_[0-9]+_' "$old_file" 2>/dev/null | grep -oE '[0-9]+' | sort -nu) + indices=$(runFileOp grep -oE '^CFG_BACKUP_LOC_[0-9]+_' "$old_file" 2>/dev/null | grep -oE '[0-9]+' | sort -nu) if [[ -z "$indices" ]]; then isNotice "No locations found in legacy file — archiving without migration" fi @@ -48,14 +48,14 @@ backupLocationsMigrate() loc_dir=$(backupLocationDir "$idx") cfg_file=$(backupLocationConfig "$idx") - sudo mkdir -p "$loc_dir" - sudo chown "$owner":"$owner" "$loc_dir" - sudo chmod 0700 "$loc_dir" + runFileOp mkdir -p "$loc_dir" + runFileOp chown "$owner":"$owner" "$loc_dir" + runFileOp chmod 0700 "$loc_dir" local old_pass="/docker/configs/security/restic/loc_${idx}.pass" local pass_value="" if [[ -f "$old_pass" ]]; then - pass_value=$(sudo cat "$old_pass" | tr -d '\n\r') + pass_value=$(runFileOp cat "$old_pass" | tr -d '\n\r') fi [[ -z "$pass_value" ]] && pass_value="RANDOMIZEDPASSWORD1" @@ -63,13 +63,13 @@ backupLocationsMigrate() echo "# Backup location $idx — migrated $(date -Iseconds) from $old_file." echo "# Edit via the Locations page on /backup, or directly here." echo "CFG_BACKUP_LOC_${idx}_PASSWORD=${pass_value} # Repository Password - Used to encrypt/decrypt snapshots — back up offline!" - sudo grep "^CFG_BACKUP_LOC_${idx}_" "$old_file" | grep -v "^CFG_BACKUP_LOC_${idx}_PASSWORD=" - } | sudo tee "$cfg_file" >/dev/null - sudo chown "$owner":"$owner" "$cfg_file" - sudo chmod 0640 "$cfg_file" + runFileOp grep "^CFG_BACKUP_LOC_${idx}_" "$old_file" | grep -v "^CFG_BACKUP_LOC_${idx}_PASSWORD=" + } | runFileWrite "$cfg_file" >/dev/null + runFileOp chown "$owner":"$owner" "$cfg_file" + runFileOp chmod 0640 "$cfg_file" if [[ -f "$old_pass" ]]; then - sudo rm -f "$old_pass" + runFileOp rm -f "$old_pass" isNotice "Inlined password for location $idx into $cfg_file (old .pass removed)" elif declare -f replacePlainPasswords >/dev/null 2>&1; then replacePlainPasswords "$cfg_file" @@ -79,9 +79,9 @@ backupLocationsMigrate() local new_kopia new_kopia=$(backupLocationKopiaConfig "$idx") if [[ -f "$old_kopia" ]]; then - sudo mv "$old_kopia" "$new_kopia" - sudo chown "$owner":"$owner" "$new_kopia" - sudo chmod 0600 "$new_kopia" + runFileOp mv "$old_kopia" "$new_kopia" + runFileOp chown "$owner":"$owner" "$new_kopia" + runFileOp chmod 0600 "$new_kopia" isNotice "Moved kopia state for location $idx → $new_kopia" fi @@ -89,16 +89,16 @@ backupLocationsMigrate() local new_ssh new_ssh=$(backupLocationSshKey "$idx") if [[ -f "$old_ssh" ]]; then - sudo mv "$old_ssh" "$new_ssh" - sudo chown "$owner":"$owner" "$new_ssh" - sudo chmod 0600 "$new_ssh" + runFileOp mv "$old_ssh" "$new_ssh" + runFileOp chown "$owner":"$owner" "$new_ssh" + runFileOp chmod 0600 "$new_ssh" isNotice "Moved SSH key for location $idx → $new_ssh" fi isSuccessful "Location $idx migrated" done - sudo mv "$old_file" "${old_file}.migrated" + runFileOp mv "$old_file" "${old_file}.migrated" isSuccessful "Migration complete — old file archived as ${old_file}.migrated" # Re-source the new per-location configs so CFG vars reflect new state. diff --git a/scripts/backup/locations/location_paths.sh b/scripts/backup/locations/location_paths.sh index e176824..948c31b 100644 --- a/scripts/backup/locations/location_paths.sh +++ b/scripts/backup/locations/location_paths.sh @@ -56,9 +56,9 @@ backupLocationEnsureDir() dir=$(backupLocationDir "$idx") local owner owner=$(backupLocationOwner) - sudo mkdir -p "$dir" - sudo chown "$owner":"$owner" "$dir" - sudo chmod 0700 "$dir" + runFileOp mkdir -p "$dir" + runFileOp chown "$owner":"$owner" "$dir" + runFileOp chmod 0700 "$dir" } backupLocationResolvedPath() diff --git a/scripts/backup/locations/location_remove.sh b/scripts/backup/locations/location_remove.sh index 3b05e4c..4c6aaf1 100644 --- a/scripts/backup/locations/location_remove.sh +++ b/scripts/backup/locations/location_remove.sh @@ -20,7 +20,7 @@ locationRemove() local name_var="CFG_BACKUP_LOC_${idx}_NAME" local label="${!name_var:-Location $idx}" - sudo rm -rf "$dir" + runFileOp rm -rf "$dir" # Best-effort unset of the now-orphaned env vars so other code in this # process doesn't see stale values. diff --git a/scripts/backup/locations/location_ssh.sh b/scripts/backup/locations/location_ssh.sh index c54997b..585d82b 100644 --- a/scripts/backup/locations/location_ssh.sh +++ b/scripts/backup/locations/location_ssh.sh @@ -52,9 +52,9 @@ backupSshKeySet() return 1 fi - echo "$decoded" | sudo tee "$key_file" >/dev/null - sudo chown "$owner":"$owner" "$key_file" - sudo chmod 0600 "$key_file" + echo "$decoded" | runFileWrite "$key_file" >/dev/null + runFileOp chown "$owner":"$owner" "$key_file" + runFileOp chmod 0600 "$key_file" if ! sudo -u "$owner" ssh-keygen -y -f "$key_file" >/dev/null 2>&1; then isError "Key written but ssh-keygen can't read it — check the format (OpenSSH PEM: ed25519, rsa, ecdsa)" @@ -80,12 +80,12 @@ backupSshKeyGenerate() key_file=$(backupSshKeyFile "$idx") if [[ -f "$key_file" ]]; then - sudo rm -f "$key_file" "${key_file}.pub" + runFileOp rm -f "$key_file" "${key_file}.pub" fi sudo -u "$owner" ssh-keygen -t ed25519 -f "$key_file" -N "" -C "libreportal-loc-${idx}" -q - sudo chmod 0600 "$key_file" - [[ -f "${key_file}.pub" ]] && sudo rm -f "${key_file}.pub" # we re-derive when needed + runFileOp chmod 0600 "$key_file" + [[ -f "${key_file}.pub" ]] && runFileOp rm -f "${key_file}.pub" # we re-derive when needed isSuccessful "Generated ed25519 keypair for location $idx" isNotice "Public key (paste into the remote host's ~/.ssh/authorized_keys):" @@ -107,7 +107,7 @@ backupSshKeyDelete() local idx="$1" local key_file key_file=$(backupSshKeyFile "$idx") - [[ -f "$key_file" ]] && sudo rm -f "$key_file" "${key_file}.pub" + [[ -f "$key_file" ]] && runFileOp rm -f "$key_file" "${key_file}.pub" isSuccessful "SSH key removed for location $idx" backupSshKeyRefreshUi } diff --git a/scripts/backup/manifest/manifest_collect.sh b/scripts/backup/manifest/manifest_collect.sh index b275b1f..27103b3 100644 --- a/scripts/backup/manifest/manifest_collect.sh +++ b/scripts/backup/manifest/manifest_collect.sh @@ -41,11 +41,11 @@ manifestCollect() fi local size_bytes - size_bytes=$(sudo du -sb "$app_dir" 2>/dev/null | awk '{print $1}') + size_bytes=$(runFileOp du -sb "$app_dir" 2>/dev/null | awk '{print $1}') [[ -z "$size_bytes" ]] && size_bytes=0 local file_count - file_count=$(sudo find "$app_dir" -type f 2>/dev/null | wc -l | tr -d ' ') + file_count=$(runFileOp find "$app_dir" -type f 2>/dev/null | wc -l | tr -d ' ') local strategy="${CFG_BACKUP_STRATEGY:-auto}" declare -f backupResolveStrategy >/dev/null 2>&1 && strategy=$(backupResolveStrategy "$app_name") diff --git a/scripts/backup/manifest/manifest_write.sh b/scripts/backup/manifest/manifest_write.sh index c213c29..10ba675 100644 --- a/scripts/backup/manifest/manifest_write.sh +++ b/scripts/backup/manifest/manifest_write.sh @@ -13,9 +13,9 @@ manifestWrite() local manifest manifest=$(manifestCollect "$app_name") - echo "$manifest" | sudo tee "$manifest_path" >/dev/null - sudo chown "$docker_install_user":"$docker_install_user" "$manifest_path" - sudo chmod 0644 "$manifest_path" + echo "$manifest" | runFileWrite "$manifest_path" >/dev/null + runFileOp chown "$docker_install_user":"$docker_install_user" "$manifest_path" + runFileOp chmod 0644 "$manifest_path" local sha sha=$(echo "$manifest" | sha256sum | cut -c1-12) @@ -26,5 +26,5 @@ manifestRemove() { local app_name="$1" local manifest_path="$containers_dir$app_name/.libreportal-manifest.json" - [[ -f "$manifest_path" ]] && sudo rm -f "$manifest_path" + [[ -f "$manifest_path" ]] && runFileOp rm -f "$manifest_path" } diff --git a/scripts/backup/verify/backup_verify.sh b/scripts/backup/verify/backup_verify.sh index 1bfab55..6bd2b39 100644 --- a/scripts/backup/verify/backup_verify.sh +++ b/scripts/backup/verify/backup_verify.sh @@ -12,14 +12,15 @@ backupVerifySnapshot() fi local scratch - scratch=$(mktemp -d -t libreportal-verify-XXXXXX) - sudo chown "$docker_install_user":"$docker_install_user" "$scratch" + # Create the scratch dir AS the backup/install user so the engine (which runs + # as that user) can restore into it and we can read it back — no root chown. + scratch=$(runFileOp mktemp -d -t libreportal-verify-XXXXXX) isNotice "Verifying ${snapshot_id:0:8} via scratch restore at $scratch" if ! engineRestoreSnapshot "$idx" "$snapshot_id" "$scratch" "$containers_dir$app_name"; then isError "Verify restore FAILED for $app_name on $(resticLocationName "$idx")" - sudo rm -rf "$scratch" + runFileOp rm -rf "$scratch" return 1 fi @@ -30,9 +31,9 @@ backupVerifySnapshot() # and replaced by dumps/captures under .lp-backup) — so just sanity-check the # restore produced a non-empty tree. local restored_count - restored_count=$(sudo find "$scratch$containers_dir$app_name" -type f 2>/dev/null | wc -l) + restored_count=$(runFileOp find "$scratch$containers_dir$app_name" -type f 2>/dev/null | wc -l) - sudo rm -rf "$scratch" + runFileOp rm -rf "$scratch" if [[ "$restored_count" -lt 1 ]]; then isError "Verify FAILED for $app_name — restored snapshot is empty"