Merge pull request #332 from naturallaw777/copilot/fix-manual-backup-failure

Fix home.tar exit-1 failure on active desktop; harden full backup workflow

app/sovran_systemsos_web/scripts/sovran-hub-backup.sh

@@ -32,6 +32,8 @@ LND_STOPPED=0
 LND_UNITS_TO_RESTART=()
 
 ARCHIVE_FILES=()
+ARCHIVE_WARNINGS=()
+PARTIAL_FILES=()
 DB_DUMP_FILES=()
 MANIFEST_EXCLUDES=()
 LND_BACKUP_NOTES=()
@@ -58,6 +60,17 @@ cleanup() {
   local rc=$?
   local restart_failed=0
 
+  # Remove any partial archive files or temporary diagnostic files
+  if [[ "${#PARTIAL_FILES[@]}" -gt 0 ]]; then
+    local partial
+    for partial in "${PARTIAL_FILES[@]}"; do
+      [[ -f "$partial" ]] && rm -f "$partial" || true
+    done
+  fi
+
+  # Release the concurrency lock file descriptor if it was opened
+  [[ -n "${LOCK_FD:-}" ]] && exec {LOCK_FD}>&- 2>/dev/null || true
+
   if [[ "$LND_STOPPED" -eq 1 ]]; then
     log "Restarting previously active LND-related services…"
     for (( idx=${#LND_UNITS_TO_RESTART[@]}-1 ; idx>=0 ; idx-- )); do
@@ -86,6 +99,11 @@ cleanup() {
     log "ERROR: Backup terminated unexpectedly (exit code $rc)."
     set_status "FAILED"
   fi
+
+  # Mark the backup directory as incomplete so failed runs are identifiable
+  if [[ -n "${BACKUP_DIR:-}" && -d "${BACKUP_DIR:-}" && ! -f "${BACKUP_DIR:-}/BACKUP_COMPLETE" ]]; then
+    touch "${BACKUP_DIR}/INCOMPLETE" 2>/dev/null || true
+  fi
 }
 
 trap cleanup EXIT
@@ -257,6 +275,17 @@ set_status "RUNNING"
 log "=== Sovran_SystemsOS External Hub Backup ==="
 log "Starting backup process…"
 
+# ── Acquire exclusive run lock ────────────────────────────────────
+# Prevents two simultaneous backup runs (e.g. from double-click or
+# stale RUNNING status after a Hub restart).
+
+LOCK_FILE="/var/lock/sovran-hub-backup.lock"
+# Note: exec {LOCK_FD}>>file requires bash 4.1+ (NixOS provides bash 5.x).
+exec {LOCK_FD}>>"$LOCK_FILE" 2>/dev/null || \
+  fail "Cannot open lock file: $LOCK_FILE. Ensure /var/lock is writable."
+flock --nonblock "$LOCK_FD" 2>/dev/null || \
+  fail "Another backup is already running. Wait for it to complete or check $BACKUP_STATUS."
+
 require_cmd tar
 require_cmd sha256sum
 require_cmd findmnt
@@ -272,6 +301,8 @@ require_cmd hostname
 require_cmd date
 require_cmd python3
 require_cmd runuser
+require_cmd mktemp
+require_cmd flock
 
 # ── Detect system role ───────────────────────────────────────────
 
@@ -324,7 +355,15 @@ MANIFEST_EXCLUDES+=(
   "/var/lib/electrs (excluded from manual backup)"
   "/var/lib/*/log and /var/lib/*/logs"
   "/var/lib/*/cache and /var/lib/*/tmp"
-  "/home/*/.cache and /home/*/.local/share/Trash"
+  "/home/*/.cache (system and application disk caches)"
+  "/home/*/.local/share/Trash and /home/*/Trash (trash directories)"
+  "/home/*/.mozilla/firefox/*/cache2 and */startupCache (Firefox volatile cache — profile data is kept)"
+  "/home/*/.config/google-chrome/*/Cache (Chrome disk cache — profile data is kept)"
+  "/home/*/.config/chromium/*/Cache (Chromium disk cache — profile data is kept)"
+  "/home/*/.config/BraveSoftware/Brave-Browser/*/Cache (Brave disk cache — profile data is kept)"
+  "/home/*/.local/share/baloo (KDE file indexer — rebuilt automatically)"
+  "/home/*/.thumbnails (thumbnail cache — rebuilt automatically)"
+  "/home/*/.xsession-errors and .xsession-errors.old (X session error logs)"
 )
 
 if [[ "$ROLE" == "desktop" || "$LND_AVAILABLE" -eq 1 ]]; then
@@ -375,28 +414,117 @@ TIMESTAMP="$(date '+%Y%m%d_%H%M%S')"
 BACKUP_DIR="${TARGET}/Sovran_SystemsOS_Backup/${TIMESTAMP}"
 DB_DUMP_DIR="$BACKUP_DIR/database-dumps"
 mkdir -p "$BACKUP_DIR" "$DB_DUMP_DIR"
+# Write an INCOMPLETE marker immediately; it is removed only on successful completion.
+# Failed runs keep this marker so they are easily identifiable and not confused with
+# complete backups during restore selection.
+touch "$BACKUP_DIR/INCOMPLETE"
 log "Backup destination: $BACKUP_DIR"
 
 create_tar_archive() {
-  local archive_name="$1"
+  _create_archive_impl STRICT "$@"
-  shift
+}
+
+# ── Helper: classify a GNU tar (LC_ALL=C) diagnostic line ────────
+# Returns 0 (true) if the message is an allowlisted transient condition
+# that is safe to ignore for /home (live desktop file changes).
+# Only two verified GNU tar messages qualify; all others are fatal.
+_is_home_warning_allowlisted() {
+  local msg="$1"
+  case "$msg" in
+    *"file changed as we read it"*)   return 0 ;;
+    *"file removed before we read it"*) return 0 ;;
+    *) return 1 ;;
+  esac
+}
+
+# ── Internal archive builder ──────────────────────────────────────
+# mode: STRICT — any tar error fails.
+#       HOME   — tar exit 1 is accepted when every diagnostic is an
+#                allowlisted transient condition (live file changes).
+_create_archive_impl() {
+  local mode="$1"; shift
+  local archive_name="$1"; shift
   local archive_path="$BACKUP_DIR/$archive_name"
+  local partial_path="${archive_path}.partial"
+  local diag_tmp
+  # mktemp creates files with mode 0600 (owner-only) by default, so tar
+  # diagnostics (which may include file paths) are not readable by other users.
+  diag_tmp="$(mktemp /tmp/sovran-tar-diag.XXXXXX)"
+  PARTIAL_FILES+=("$partial_path" "$diag_tmp")
 
   log "Creating $archive_name …"
-  tar \
+
+  local tar_rc=0
+  LC_ALL=C tar \
     --create \
-    --file "$archive_path" \
+    --file "$partial_path" \
     --numeric-owner \
     --acls \
     --xattrs \
     --sparse \
     --one-file-system \
-    "$@"
+    "$@" 2>"$diag_tmp" || tar_rc=$?
 
+  # Log every tar diagnostic to the backup log so it appears in the Hub UI
+  local has_fatal_diag=0
+  if [[ -s "$diag_tmp" ]]; then
+    while IFS= read -r diag_line; do
+      [[ -n "$diag_line" ]] || continue
+      log "tar: $diag_line"
+      if [[ "$mode" == "HOME" ]] && ! _is_home_warning_allowlisted "$diag_line"; then
+        has_fatal_diag=1
+      fi
+    done < "$diag_tmp"
+  fi
+
+  local accept=0
+  if [[ "$tar_rc" -eq 0 ]]; then
+    accept=1
+  elif [[ "$mode" == "HOME" && "$tar_rc" -eq 1 && "$has_fatal_diag" -eq 0 ]]; then
+    accept=1
+    log "NOTE: $archive_name completed with nonfatal warnings (live files changed during backup -- this is normal on an active desktop and does not affect the safety of your backup)."
+    ARCHIVE_WARNINGS+=("$archive_name: nonfatal warnings -- live files changed during backup (normal on an active desktop)")
+  fi
+
+  if [[ "$accept" -eq 0 ]]; then
+    rm -f "$partial_path" "$diag_tmp"
+    if [[ "$tar_rc" -gt 1 ]]; then
+      fail "tar exited with fatal code $tar_rc while creating $archive_name."
+    elif [[ "$mode" == "HOME" ]]; then
+      fail "tar exited with code $tar_rc and unrecognized diagnostics while creating $archive_name."
+    else
+      fail "tar exited with code $tar_rc while creating $archive_name."
+    fi
+  fi
+
+  # Verify the partial archive is non-empty and readable (spot-check first entry)
+  if [[ ! -s "$partial_path" ]]; then
+    rm -f "$partial_path" "$diag_tmp"
+    fail "Archive $archive_name is empty after creation."
+  fi
+
+  # Fast readability check: read only the first tar entry header (~512 bytes).
+  # head -1 closes the pipe after one line, sending SIGPIPE to tar which then
+  # exits early — so this is O(1) regardless of archive size.
+  local spot_entry
+  spot_entry="$(LC_ALL=C tar --list --file "$partial_path" 2>/dev/null | head -1 || true)"
+  if [[ -z "$spot_entry" ]]; then
+    rm -f "$partial_path" "$diag_tmp"
+    fail "Archive $archive_name failed readability check."
+  fi
+
+  # Atomic publish: rename partial to final path only after acceptance
+  mv "$partial_path" "$archive_path"
+  rm -f "$diag_tmp"
   ARCHIVE_FILES+=("$archive_name")
   log "Created archive: $archive_name"
 }
 
+# ── Home archive: tolerates allowlisted live-file warnings ───────
+create_home_tar_archive() {
+  _create_archive_impl HOME "$@"
+}
+
 export_postgresql_dumps() {
   if ! command -v pg_dump >/dev/null 2>&1 || ! has_unit "postgresql.service"; then
     log "PostgreSQL tools/service not available — skipping PostgreSQL exports."
@@ -605,11 +733,24 @@ log "Stage 2 complete."
 log ""
 log "── Stage 3/5: Home directory (/home) ───────────────────────"
 if [[ -d /home ]]; then
-  create_tar_archive "home.tar" \
+  create_home_tar_archive "home.tar" \
     -C / \
     --exclude='home/*/.cache' \
     --exclude='home/*/.local/share/Trash' \
     --exclude='home/*/Trash' \
+    --exclude='home/*/.mozilla/firefox/*/cache2' \
+    --exclude='home/*/.mozilla/firefox/*/startupCache' \
+    --exclude='home/*/.mozilla/firefox/*/thumbnails' \
+    --exclude='home/*/.config/google-chrome/*/Cache' \
+    --exclude='home/*/.config/google-chrome/*/Code Cache' \
+    --exclude='home/*/.config/chromium/*/Cache' \
+    --exclude='home/*/.config/chromium/*/Code Cache' \
+    --exclude='home/*/.config/BraveSoftware/Brave-Browser/*/Cache' \
+    --exclude='home/*/.config/BraveSoftware/Brave-Browser/*/Code Cache' \
+    --exclude='home/*/.local/share/baloo' \
+    --exclude='home/*/.thumbnails' \
+    --exclude='home/*/.xsession-errors' \
+    --exclude='home/*/.xsession-errors.old' \
     home
   log "Stage 3 complete."
 else
@@ -708,13 +849,29 @@ CHECKSUM_FILE="$BACKUP_DIR/SHA256SUMS.txt"
   echo "- PostgreSQL DB dump: sudo -u postgres pg_restore --create --clean --if-exists -d postgres database-dumps/postgresql_<db>.dump"
   echo "- MariaDB DB dump: mariadb <db_name> < database-dumps/mariadb_<db>.sql"
   echo "- LND SCB: keep lnd-static-channel-backup.scb with wallet seed for channel recovery procedures"
+  echo "- Note: when restoring /var/lib, exclude raw DB directories (var/lib/postgresql, var/lib/mysql)"
+  echo "  and restore from native dumps instead for PostgreSQL and MariaDB"
+  echo ""
+  echo "Nonfatal warnings:"
+  if [[ "${#ARCHIVE_WARNINGS[@]}" -eq 0 ]]; then
+    echo "- none"
+  else
+    for warning in "${ARCHIVE_WARNINGS[@]}"; do
+      echo "- $warning"
+    done
+  fi
   echo ""
   echo "Important note: Bitcoin blockchain and Electrs index data are intentionally excluded"
   echo "from manual external backup because they already live on the internal second drive"
   echo "(/run/media/Second_Drive) and are reconstructable/internal-backup data."
   echo ""
   echo "Artifact listing:"
-  find "$BACKUP_DIR" -mindepth 1 -maxdepth 2 -type f | sort
+  # INCOMPLETE exists at this point (created at backup start); BACKUP_COMPLETE does not
+  # exist yet (written after checksums). Excluding both marker files here is intentional:
+  # INCOMPLETE is excluded so it doesn't appear as a data artifact, and BACKUP_COMPLETE
+  # is excluded defensively for consistency should the ordering ever change.
+  find "$BACKUP_DIR" -mindepth 1 -maxdepth 2 -type f \
+    ! -name 'INCOMPLETE' ! -name 'BACKUP_COMPLETE' -print0 | sort -z | tr '\0' '\n'
 } > "$MANIFEST_FILE"
 
 # ── Generate checksums for all backup artifacts ─────────────────
@@ -724,7 +881,7 @@ log "Generating SHA-256 checksums …"
   cd "$BACKUP_DIR"
   while IFS= read -r -d '' file; do
     sha256sum "$file"
-  done < <(find . -mindepth 1 -maxdepth 2 -type f ! -name 'SHA256SUMS.txt' -print0 | sort -z)
+  done < <(find . -mindepth 1 -maxdepth 2 -type f ! -name 'SHA256SUMS.txt' ! -name 'INCOMPLETE' ! -name 'BACKUP_COMPLETE' -print0 | sort -z)
 ) > "$CHECKSUM_FILE"
 
 log "Manifest written to $MANIFEST_FILE"
@@ -733,8 +890,21 @@ log "Checksums written to $CHECKSUM_FILE"
 # ── Done ─────────────────────────────────────────────────────────
 
 log ""
+if [[ "${#ARCHIVE_WARNINGS[@]}" -gt 0 ]]; then
+  log "Backup completed with nonfatal warnings:"
+  for warning in "${ARCHIVE_WARNINGS[@]}"; do
+    log "  WARNING: $warning"
+  done
+  log "Your important data is backed up. The warnings above indicate files that changed"
+  log "during backup, which is normal on an active desktop and does not affect your backup."
+  log ""
+fi
 log "All Finished! Your data is now backed up to a third location."
 log "Please eject the drive safely before removing it from your Sovran Pro."
 
+# Remove incomplete marker and write completion marker only after all work succeeds
+rm -f "$BACKUP_DIR/INCOMPLETE"
+echo "$(date -u '+%Y-%m-%dT%H:%M:%SZ')" > "$BACKUP_DIR/BACKUP_COMPLETE"
+
 BACKUP_COMPLETE=1
 set_status "SUCCESS"

app/tests/test_manual_backup_workflow.py

@@ -1,4 +1,9 @@
 import asyncio
+import os
+import re
+import subprocess
+import sys
+import tempfile
 import unittest
 from pathlib import Path
 
@@ -10,6 +15,25 @@ SUPPORT_JS = REPO_ROOT / "app" / "sovran_systemsos_web" / "static" / "js" / "sup
 NIX_HUB_FILE = REPO_ROOT / "modules" / "core" / "sovran-hub.nix"
 
 
+def _extract_bash_function(script_path: Path, func_name: str) -> str:
+    """Extract a bash function definition from a shell script using Python regex.
+
+    The pattern matches from 'funcname() {' to the first line whose only content
+    is '}' (the function closing brace, at column 0). This works correctly for
+    functions whose control structures (case/if/while/for) use indented braces,
+    because only the function's own closing brace sits at column 0. It would not
+    correctly extract a function that contains a nested function definition.
+
+    Returns the complete function definition, safe to inline into a test bash
+    script without eval or awk."""
+    source = script_path.read_text()
+    pattern = r"^" + re.escape(func_name) + r"\(\) \{.*?^}"
+    match = re.search(pattern, source, re.MULTILINE | re.DOTALL)
+    if match is None:
+        raise ValueError(f"Function '{func_name}' not found in {script_path}")
+    return match.group(0)
+
+
 class ManualBackupWorkflowTests(unittest.TestCase):
     def test_backup_script_uses_tar_archives_with_checksums_and_exclusions(self):
         source = BACKUP_SCRIPT.read_text()
@@ -138,6 +162,527 @@ class ManualBackupWorkflowTests(unittest.TestCase):
         self.assertEqual(rc, 127)
         self.assertIn("bash: command not found", stderr_out)
 
+    # ── Tests for home tar exit-1 tolerance (production fix) ──────────────────
+
+    def test_backup_script_has_home_tar_archive_function(self):
+        """The script must define create_home_tar_archive for /home only,
+        separate from the strict create_tar_archive used for other sources."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            "create_home_tar_archive",
+            source,
+            "backup script must define create_home_tar_archive for /home",
+        )
+        self.assertIn(
+            "_is_home_warning_allowlisted",
+            source,
+            "backup script must define _is_home_warning_allowlisted helper",
+        )
+        self.assertIn(
+            "ARCHIVE_WARNINGS",
+            source,
+            "backup script must track nonfatal warnings in ARCHIVE_WARNINGS",
+        )
+
+    def test_backup_script_allowlist_covers_verified_gnu_tar_messages(self):
+        """The allowlist must contain the exact GNU tar (LC_ALL=C) strings for
+        the two permitted transient conditions."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            "file changed as we read it",
+            source,
+            "allowlist must include exact GNU tar 'file changed as we read it' message",
+        )
+        self.assertIn(
+            "file removed before we read it",
+            source,
+            "allowlist must include exact GNU tar 'file removed before we read it' message",
+        )
+
+    def test_backup_script_stage3_uses_home_tar_function(self):
+        """Stage 3 must call create_home_tar_archive (not create_tar_archive)
+        so the /home archive tolerates live file changes on an active desktop."""
+        source = BACKUP_SCRIPT.read_text()
+        # The home archive call must use the tolerant function
+        self.assertIn(
+            'create_home_tar_archive "home.tar"',
+            source,
+            "Stage 3 must use create_home_tar_archive for home.tar",
+        )
+
+    def test_backup_script_strict_function_still_used_for_etc(self):
+        """create_tar_archive (strict) must still be used for /etc/nixos,
+        secrets, var-lib-lnd-clean, and var-lib — only /home is tolerant."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            'create_tar_archive "etc-nixos.tar"',
+            source,
+            "etc-nixos must use strict create_tar_archive",
+        )
+        self.assertIn(
+            'create_tar_archive "var-lib.tar"',
+            source,
+            "var-lib must use strict create_tar_archive",
+        )
+
+    # ── Tests for partial-file atomic publish ─────────────────────────────────
+
+    def test_backup_script_uses_partial_path_and_atomic_rename(self):
+        """Archives must be written to a .partial path and atomically renamed
+        to the final name only after acceptance, so a failed run never leaves a
+        partial archive that appears complete."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            ".partial",
+            source,
+            "backup script must write to a .partial path before renaming",
+        )
+        self.assertIn(
+            'mv "$partial_path" "$archive_path"',
+            source,
+            "backup script must atomically rename partial to final archive path",
+        )
+        self.assertIn(
+            "PARTIAL_FILES",
+            source,
+            "backup script must track partial files for cleanup",
+        )
+
+    # ── Tests for INCOMPLETE / BACKUP_COMPLETE markers ────────────────────────
+
+    def test_backup_script_writes_incomplete_marker(self):
+        """A backup directory must be marked INCOMPLETE immediately after
+        creation, so interrupted or failed runs are identifiable."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            'touch "$BACKUP_DIR/INCOMPLETE"',
+            source,
+            "backup script must create INCOMPLETE marker after mkdir",
+        )
+
+    def test_backup_script_writes_backup_complete_marker(self):
+        """A BACKUP_COMPLETE file must be written only after all work succeeds,
+        so restore tools can distinguish complete from incomplete backups."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            'BACKUP_COMPLETE"',
+            source,
+            "backup script must write BACKUP_COMPLETE file on success",
+        )
+        self.assertIn(
+            'rm -f "$BACKUP_DIR/INCOMPLETE"',
+            source,
+            "backup script must remove INCOMPLETE marker on success",
+        )
+
+    # ── Tests for concurrency lock ─────────────────────────────────────────────
+
+    def test_backup_script_uses_flock_concurrency_lock(self):
+        """The script must acquire an exclusive flock before starting work to
+        prevent two simultaneous backup runs."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            "flock",
+            source,
+            "backup script must use flock for concurrency protection",
+        )
+        self.assertIn(
+            "LOCK_FILE",
+            source,
+            "backup script must define a LOCK_FILE for the flock",
+        )
+
+    # ── Tests for expanded home exclusions ────────────────────────────────────
+
+    def test_backup_script_home_excludes_browser_caches(self):
+        """The home archive must exclude browser disk caches (volatile data)
+        while keeping profile data, bookmarks, and user documents."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            "--exclude='home/*/.mozilla/firefox/*/cache2'",
+            source,
+            "backup script must exclude Firefox cache2 directory",
+        )
+        self.assertIn(
+            "--exclude='home/*/.config/google-chrome/*/Cache'",
+            source,
+            "backup script must exclude Chrome Cache directory",
+        )
+        self.assertIn(
+            "--exclude='home/*/.config/chromium/*/Cache'",
+            source,
+            "backup script must exclude Chromium Cache directory",
+        )
+        self.assertIn(
+            "--exclude='home/*/.config/BraveSoftware/Brave-Browser/*/Cache'",
+            source,
+            "backup script must exclude Brave Cache directory",
+        )
+
+    def test_backup_script_home_excludes_other_volatile_caches(self):
+        """The home archive must exclude baloo, thumbnails, and X session
+        error logs — all volatile/reconstructable content."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            "--exclude='home/*/.local/share/baloo'",
+            source,
+        )
+        self.assertIn(
+            "--exclude='home/*/.thumbnails'",
+            source,
+        )
+        self.assertIn(
+            "--exclude='home/*/.xsession-errors'",
+            source,
+        )
+
+    # ── Tests for require_cmd additions ───────────────────────────────────────
+
+    def test_backup_script_requires_mktemp_and_flock(self):
+        """The script must explicitly check for mktemp and flock via require_cmd
+        so that missing tools fail early with a clear error."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            "require_cmd mktemp",
+            source,
+            "backup script must require mktemp",
+        )
+        self.assertIn(
+            "require_cmd flock",
+            source,
+            "backup script must require flock",
+        )
+
+    # ── Behavioral tests ──────────────────────────────────────────────────────
+
+    def test_allowlist_accepts_file_changed_message(self):
+        """Behavioral: _is_home_warning_allowlisted (from the actual backup script)
+        returns true for the exact GNU tar 'file changed as we read it' message (LC_ALL=C).
+        The function definition is extracted from the production script via Python regex
+        to ensure the test exercises the real allowlist without eval-of-awk risks."""
+        func_def = _extract_bash_function(BACKUP_SCRIPT, "_is_home_warning_allowlisted")
+        script = (
+            "#!/usr/bin/env bash\n"
+            + func_def + "\n"
+            + 'msgs=(\n'
+            + '  "tar: /home/user/firefox.db: file changed as we read it"\n'
+            + '  "tar: /home/user/.local/share/app: file removed before we read it"\n'
+            + ")\n"
+            + 'for msg in "${msgs[@]}"; do\n'
+            + '  _is_home_warning_allowlisted "$msg" || { echo "BLOCKED: $msg"; exit 1; }\n'
+            + '  echo "ALLOWED: $msg"\n'
+            + "done\n"
+        )
+        with tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) as f:
+            f.write(script)
+            script_path = f.name
+        try:
+            result = subprocess.run(["bash", script_path], capture_output=True, text=True)
+            self.assertEqual(result.returncode, 0, f"stderr: {result.stderr}")
+            self.assertIn("ALLOWED", result.stdout)
+            self.assertNotIn("BLOCKED", result.stdout)
+        finally:
+            os.unlink(script_path)
+
+    def test_allowlist_rejects_fatal_diagnostics(self):
+        """Behavioral: _is_home_warning_allowlisted (from the actual backup script)
+        returns false for permission errors, I/O errors, write errors, and other
+        fatal conditions."""
+        func_def = _extract_bash_function(BACKUP_SCRIPT, "_is_home_warning_allowlisted")
+        script = (
+            "#!/usr/bin/env bash\n"
+            + func_def + "\n"
+            + 'msgs=(\n'
+            + '  "tar: /home/user/file: Permission denied"\n'
+            + '  "tar: /dev/sdb: Cannot read: Input/output error"\n'
+            + '  "tar: Error is not recoverable: exiting now"\n'
+            + '  "Write error"\n'
+            + '  "tar: /home/user: Cannot stat: No such file or directory"\n'
+            + ")\n"
+            + 'for msg in "${msgs[@]}"; do\n'
+            + '  _is_home_warning_allowlisted "$msg" && { echo "ALLOWED_BUT_SHOULD_NOT: $msg"; exit 1; }\n'
+            + '  echo "CORRECTLY_BLOCKED: $msg"\n'
+            + "done\n"
+        )
+        with tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) as f:
+            f.write(script)
+            script_path = f.name
+        try:
+            result = subprocess.run(["bash", script_path], capture_output=True, text=True)
+            self.assertEqual(result.returncode, 0, f"stderr: {result.stderr}")
+            self.assertIn("CORRECTLY_BLOCKED", result.stdout)
+            self.assertNotIn("ALLOWED_BUT_SHOULD_NOT", result.stdout)
+        finally:
+            os.unlink(script_path)
+
+    def test_home_tar_exits_1_with_only_allowlisted_warnings_is_accepted(self):
+        """Behavioral: _create_archive_impl in HOME mode accepts tar exit 1 when
+        every diagnostic is allowlisted and publishes the final archive."""
+        with tempfile.TemporaryDirectory() as tmpdir:
+            src = os.path.join(tmpdir, "home", "user1")
+            dest = os.path.join(tmpdir, "backup")
+            os.makedirs(src, exist_ok=True)
+            os.makedirs(dest, exist_ok=True)
+            with open(os.path.join(src, "testfile.txt"), "w") as f:
+                f.write("test content\n")
+
+            # Emulate the allowlist + acceptance logic in HOME mode
+            script = f"""#!/usr/bin/env bash
+set -euo pipefail
+BACKUP_DIR="{dest}"
+ARCHIVE_FILES=()
+ARCHIVE_WARNINGS=()
+PARTIAL_FILES=()
+
+log() {{ echo "$*"; }}
+fail() {{ echo "FAIL: $*" >&2; exit 1; }}
+
+_is_home_warning_allowlisted() {{
+  local msg="$1"
+  case "$msg" in
+    *"file changed as we read it"*)   return 0 ;;
+    *"file removed before we read it"*) return 0 ;;
+    *) return 1 ;;
+  esac
+}}
+
+_create_archive_impl() {{
+  local mode="$1"; shift
+  local archive_name="$1"; shift
+  local archive_path="$BACKUP_DIR/$archive_name"
+  local partial_path="${{archive_path}}.partial"
+  local diag_tmp
+  diag_tmp="$(mktemp /tmp/sovran-tar-diag.XXXXXX)"
+  PARTIAL_FILES+=("$partial_path" "$diag_tmp")
+
+  local tar_rc=0
+  LC_ALL=C tar --create --file "$partial_path" \
+    --numeric-owner --sparse --one-file-system \
+    "$@" 2>"$diag_tmp" || tar_rc=$?
+
+  # Inject a simulated allowlisted warning to match production scenario
+  echo "tar: {src}/testfile.txt: file changed as we read it" >> "$diag_tmp"
+  tar_rc=1  # simulate exit 1
+
+  local has_fatal_diag=0
+  if [[ -s "$diag_tmp" ]]; then
+    while IFS= read -r diag_line; do
+      [[ -n "$diag_line" ]] || continue
+      if [[ "$mode" == "HOME" ]] && ! _is_home_warning_allowlisted "$diag_line"; then
+        has_fatal_diag=1
+      fi
+    done < "$diag_tmp"
+  fi
+
+  local accept=0
+  if [[ "$tar_rc" -eq 0 ]]; then
+    accept=1
+  elif [[ "$mode" == "HOME" && "$tar_rc" -eq 1 && "$has_fatal_diag" -eq 0 ]]; then
+    accept=1
+    ARCHIVE_WARNINGS+=("$archive_name: nonfatal warnings")
+  fi
+
+  [[ "$accept" -eq 1 ]] || {{ rm -f "$partial_path" "$diag_tmp"; fail "Rejected"; }}
+  [[ -s "$partial_path" ]] || {{ rm -f "$partial_path" "$diag_tmp"; fail "Empty"; }}
+
+  mv "$partial_path" "$archive_path"
+  rm -f "$diag_tmp"
+  ARCHIVE_FILES+=("$archive_name")
+  echo "CREATED: $archive_name"
+}}
+
+create_home_tar_archive() {{ _create_archive_impl HOME "$@"; }}
+
+create_home_tar_archive "home.tar" -C "{tmpdir}" home
+echo "WARNINGS: ${{#ARCHIVE_WARNINGS[@]}}"
+[[ -f "{dest}/home.tar" ]] && echo "FILE_EXISTS" || echo "FILE_MISSING"
+[[ ! -f "{dest}/home.tar.partial" ]] && echo "PARTIAL_CLEANED" || echo "PARTIAL_LEFT"
+"""
+            script_file = os.path.join(tmpdir, "test.sh")
+            with open(script_file, "w") as sf:
+                sf.write(script)
+            result = subprocess.run(["bash", script_file], capture_output=True, text=True)
+            self.assertEqual(result.returncode, 0, f"stdout: {result.stdout}\nstderr: {result.stderr}")
+            self.assertIn("CREATED: home.tar", result.stdout)
+            self.assertIn("FILE_EXISTS", result.stdout)
+            self.assertIn("PARTIAL_CLEANED", result.stdout)
+
+    def test_home_tar_exits_1_with_fatal_diagnostic_fails(self):
+        """Behavioral: _create_archive_impl in HOME mode must fail when tar exit 1
+        includes a non-allowlisted diagnostic (e.g. permission denied)."""
+        with tempfile.TemporaryDirectory() as tmpdir:
+            dest = os.path.join(tmpdir, "backup")
+            os.makedirs(dest, exist_ok=True)
+
+            script = f"""#!/usr/bin/env bash
+set -euo pipefail
+BACKUP_DIR="{dest}"
+ARCHIVE_FILES=()
+ARCHIVE_WARNINGS=()
+PARTIAL_FILES=()
+
+fail() {{ echo "CORRECTLY_FAILED: $*"; exit 1; }}
+log() {{ echo "$*"; }}
+
+_is_home_warning_allowlisted() {{
+  local msg="$1"
+  case "$msg" in
+    *"file changed as we read it"*)   return 0 ;;
+    *"file removed before we read it"*) return 0 ;;
+    *) return 1 ;;
+  esac
+}}
+
+# Simulate tar exit 1 with a fatal permission-denied diagnostic
+archive_name="home.tar"
+partial_path="$BACKUP_DIR/$archive_name.partial"
+diag_tmp=$(mktemp)
+echo "tar: /home/user/locked: Permission denied" > "$diag_tmp"
+
+tar_rc=1
+has_fatal_diag=0
+while IFS= read -r diag_line; do
+  [[ -n "$diag_line" ]] || continue
+  if ! _is_home_warning_allowlisted "$diag_line"; then
+    has_fatal_diag=1
+  fi
+done < "$diag_tmp"
+rm -f "$diag_tmp"
+
+if [[ "$tar_rc" -eq 1 && "$has_fatal_diag" -eq 0 ]]; then
+  echo "INCORRECTLY_ACCEPTED"
+  exit 0
+else
+  fail "tar exit 1 with Permission denied"
+fi
+"""
+            script_file = os.path.join(tmpdir, "test.sh")
+            with open(script_file, "w") as sf:
+                sf.write(script)
+            result = subprocess.run(["bash", script_file], capture_output=True, text=True)
+            self.assertEqual(result.returncode, 1, f"Should have failed: {result.stdout}")
+            self.assertIn("CORRECTLY_FAILED", result.stdout)
+
+    def test_home_tar_exits_2_always_fails(self):
+        """Behavioral: tar exit code 2 (fatal) must always fail even in HOME mode."""
+        with tempfile.TemporaryDirectory() as tmpdir:
+            dest = os.path.join(tmpdir, "backup")
+            os.makedirs(dest, exist_ok=True)
+
+            script = f"""#!/usr/bin/env bash
+BACKUP_DIR="{dest}"
+ARCHIVE_WARNINGS=()
+
+fail() {{ echo "CORRECTLY_FAILED: $*"; exit 1; }}
+log() {{ echo "$*"; }}
+
+_is_home_warning_allowlisted() {{
+  case "$1" in
+    *"file changed as we read it"*)   return 0 ;;
+    *"file removed before we read it"*) return 0 ;;
+    *) return 1 ;;
+  esac
+}}
+
+tar_rc=2
+has_fatal_diag=0
+
+accept=0
+if [[ "$tar_rc" -eq 0 ]]; then
+  accept=1
+elif [[ "HOME" == "HOME" && "$tar_rc" -eq 1 && "$has_fatal_diag" -eq 0 ]]; then
+  accept=1
+fi
+
+if [[ "$accept" -eq 0 ]]; then
+  if [[ "$tar_rc" -gt 1 ]]; then
+    fail "tar exited with fatal code $tar_rc"
+  fi
+fi
+echo "SHOULD_NOT_REACH_HERE"
+"""
+            script_file = os.path.join(tmpdir, "test.sh")
+            with open(script_file, "w") as sf:
+                sf.write(script)
+            result = subprocess.run(["bash", script_file], capture_output=True, text=True)
+            self.assertEqual(result.returncode, 1, f"Should have failed: {result.stdout}")
+            self.assertIn("CORRECTLY_FAILED", result.stdout)
+            self.assertNotIn("SHOULD_NOT_REACH_HERE", result.stdout)
+
+    def test_partial_file_cleanup_on_failure(self):
+        """Behavioral: a .partial file must be removed when the archive creation
+        fails, so no incomplete archive is left on the destination."""
+        with tempfile.TemporaryDirectory() as tmpdir:
+            dest = os.path.join(tmpdir, "backup")
+            os.makedirs(dest, exist_ok=True)
+
+            partial_path = os.path.join(dest, "home.tar.partial")
+
+            script = f"""#!/usr/bin/env bash
+BACKUP_DIR="{dest}"
+PARTIAL_FILES=()
+
+fail() {{
+  # Simulate cleanup removing partial files
+  for p in "${{PARTIAL_FILES[@]}}"; do
+    [[ -f "$p" ]] && rm -f "$p"
+  done
+  echo "FAILED: $*"
+  exit 1
+}}
+log() {{ :; }}
+
+partial_path="$BACKUP_DIR/home.tar.partial"
+diag_tmp=$(mktemp)
+PARTIAL_FILES+=("$partial_path" "$diag_tmp")
+
+# Simulate partial archive being written
+touch "$partial_path"
+
+# Simulate a fatal error
+fail "tar exited with code 2"
+"""
+            script_file = os.path.join(tmpdir, "test.sh")
+            with open(script_file, "w") as sf:
+                sf.write(script)
+            result = subprocess.run(["bash", script_file], capture_output=True, text=True)
+            self.assertEqual(result.returncode, 1, f"Should have failed: {result.stdout}")
+            self.assertFalse(
+                os.path.exists(partial_path),
+                "Partial file must be removed on failure",
+            )
+            self.assertIn("FAILED", result.stdout)
+
+    def test_archive_warnings_recorded_in_manifest(self):
+        """Source text: ARCHIVE_WARNINGS must be written into BACKUP_MANIFEST.txt
+        so the user can see which archives had nonfatal live-file warnings."""
+        source = BACKUP_SCRIPT.read_text()
+        self.assertIn(
+            "ARCHIVE_WARNINGS[@]",
+            source,
+            "backup script must iterate ARCHIVE_WARNINGS in manifest generation",
+        )
+        self.assertIn(
+            "Nonfatal warnings:",
+            source,
+            "manifest must include a 'Nonfatal warnings:' section",
+        )
+
+    def test_backup_script_logs_warnings_before_success_message(self):
+        """Source text: when ARCHIVE_WARNINGS is non-empty, the script must log
+        the warnings before the 'All Finished!' success message."""
+        source = BACKUP_SCRIPT.read_text()
+        # Both constructs must be present
+        self.assertIn(
+            "${#ARCHIVE_WARNINGS[@]}",
+            source,
+            "backup script must check ARCHIVE_WARNINGS count before success message",
+        )
+        self.assertIn(
+            "All Finished!",
+            source,
+        )
+
 
 if __name__ == "__main__":
     unittest.main()