Fix expire phase failing on snapshots without owner write bit

rsync_tmbackup.sh

@@ -118,6 +118,12 @@ fn_expire_backup() {
         fi
 
         fn_log_info "Expiring $1"
+        # Sanitize permissions before deletion: snapshots may contain files/dirs
+        # without owner write bit (e.g. ISPConfig .php-fcgi-starter at 0500,
+        # web1/ at 0550) that would make `rm -rf` fail. Only safe on local dest.
+        if [ -z "$SSH_DEST_FOLDER_PREFIX" ]; then
+                fn_fix_permissions "$1"
+        fi
         fn_rm_dir "$1"
 }
 
@@ -299,38 +305,29 @@ fn_df_t() {
         fn_run_cmd "df -T '${1}'"
 }
 
-# Function to set permissions on the last used directory
+# Function to set permissions on a backup directory tree.
+# Covers any dir or file lacking owner write (u+w) — this includes the legacy
+# narrow cases (0111, 000) plus 0500/0550/0700/0710 typical of ISPConfig,
+# /root/, /etc/ssl/private/, etc.
 fn_fix_permissions() {
     local base_dir="$1"
+    local sudo_cmd=""
+
+    if [ "$SUDO" = true ]; then
+        sudo_cmd="sudo"
+    fi
 
     fn_log_info "Fixing permissions in directory: $base_dir"
 
-    if [ "$SUDO" = true ]; then
-        if ! find "$base_dir" -type d -perm 0111 -exec sudo chmod 700 {} \;; then
-            fn_log_error "Failed to set permissions on some directories"
-            exit 1
-        fi
-        if ! find "$base_dir" -type d -perm 000 -exec sudo chmod 700 {} \;; then
-            fn_log_error "Failed to set permissions on some directories"
-            exit 1
-        fi
-        if ! find "$base_dir" -type f -perm 000 -exec sudo chmod 600 {} \;; then
-            fn_log_error "Failed to set permissions on some files"
-            exit 1
-        fi
-    else
-        if ! find "$base_dir" -type d -perm 0111 -exec chmod 700 {} \;; then
-            fn_log_error "Failed to set permissions on some directories"
-            exit 1
-        fi
-        if ! find "$base_dir" -type d -perm 000 -exec chmod 700 {} \;; then
-            fn_log_error "Failed to set permissions on some directories"
-            exit 1
-        fi
-        if ! find "$base_dir" -type f -perm 000 -exec chmod 600 {} \;; then
-            fn_log_error "Failed to set permissions on some files"
-            exit 1
-        fi
+    # Any dir without u+w → grant u+rwX so we can traverse and remove
+    if ! find "$base_dir" -type d ! -perm -u=w -exec $sudo_cmd chmod u+rwX {} +; then
+        fn_log_error "Failed to set permissions on some directories"
+        exit 1
+    fi
+    # Any file without u+w → grant u+rw
+    if ! find "$base_dir" -type f ! -perm -u=w -exec $sudo_cmd chmod u+rw {} +; then
+        fn_log_error "Failed to set permissions on some files"
+        exit 1
     fi
 
     fn_log_info "Permissions fixed successfully"

tests/06_expire_perms.bats

@@ -0,0 +1,72 @@
+#!/usr/bin/env bats
+#
+# Regression tests for expire phase when snapshots contain files/dirs
+# without owner write bit (e.g. ISPConfig .php-fcgi-starter at mode 0500,
+# web1/ directories at 0550). Before the fix, `rm -rf` in fn_expire_backup
+# would fail with "Permission denied" on these snapshots.
+#
+
+load 'test_helper'
+
+setup() {
+    setup_test_environment
+    create_sample_files "$TEST_SOURCE" 2
+}
+
+teardown() {
+    # Restore perms so teardown's rm -rf works even if a test failed mid-way
+    if [ -n "$TEST_TEMP_DIR" ] && [ -d "$TEST_TEMP_DIR" ]; then
+        chmod -R u+rwX "$TEST_TEMP_DIR" 2>/dev/null || true
+    fi
+    teardown_test_environment
+}
+
+@test "expire removes snapshot containing 0500 file (ISPConfig .php-fcgi-starter case)" {
+    # Backup 1 (will become the one to expire)
+    run "$SCRIPT_PATH" -m 1 "$TEST_SOURCE" "$TEST_DEST"
+    assert_success
+
+    local oldest
+    oldest=$(find "$TEST_DEST" -maxdepth 1 -type d -name "????-??-??-??????" | head -1)
+    [ -n "$oldest" ]
+
+    # Inject the problematic perms inside the first snapshot
+    mkdir -p "$oldest/var/www/php-fcgi-scripts/web1"
+    echo '#!/bin/bash' > "$oldest/var/www/php-fcgi-scripts/web1/.php-fcgi-starter"
+    chmod 0500 "$oldest/var/www/php-fcgi-scripts/web1/.php-fcgi-starter"
+    chmod 0550 "$oldest/var/www/php-fcgi-scripts/web1"
+
+    sleep 1
+
+    # Backup 2 with -m 1 must expire backup 1 — this is where the bug fires
+    echo "trigger" >> "$TEST_SOURCE/file_1.txt"
+    run "$SCRIPT_PATH" -m 1 "$TEST_SOURCE" "$TEST_DEST"
+    assert_success
+
+    assert_dir_not_exists "$oldest"
+
+    local count
+    count=$(count_backups "$TEST_DEST")
+    [ "$count" -eq 1 ]
+}
+
+@test "expire removes snapshot with 0400 file and 0700 parent dir" {
+    run "$SCRIPT_PATH" -m 1 "$TEST_SOURCE" "$TEST_DEST"
+    assert_success
+
+    local oldest
+    oldest=$(find "$TEST_DEST" -maxdepth 1 -type d -name "????-??-??-??????" | head -1)
+
+    mkdir -p "$oldest/etc/ssl/private"
+    echo 'fake-key' > "$oldest/etc/ssl/private/server.key"
+    chmod 0400 "$oldest/etc/ssl/private/server.key"
+    chmod 0700 "$oldest/etc/ssl/private"
+
+    sleep 1
+
+    echo "trigger" >> "$TEST_SOURCE/file_1.txt"
+    run "$SCRIPT_PATH" -m 1 "$TEST_SOURCE" "$TEST_DEST"
+    assert_success
+
+    assert_dir_not_exists "$oldest"
+}