Address code review: source allowlist function from script in tests, fix find exclusions for BACKUP_COMPLETE, simplify log message
This commit is contained in:
@@ -476,9 +476,7 @@ _create_archive_impl() {
|
|||||||
accept=1
|
accept=1
|
||||||
elif [[ "$mode" == "HOME" && "$tar_rc" -eq 1 && "$has_fatal_diag" -eq 0 ]]; then
|
elif [[ "$mode" == "HOME" && "$tar_rc" -eq 1 && "$has_fatal_diag" -eq 0 ]]; then
|
||||||
accept=1
|
accept=1
|
||||||
log "NOTE: $archive_name completed with nonfatal warnings (live files changed" \
|
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)."
|
||||||
"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)")
|
ARCHIVE_WARNINGS+=("$archive_name: nonfatal warnings -- live files changed during backup (normal on an active desktop)")
|
||||||
fi
|
fi
|
||||||
|
|
||||||
@@ -859,7 +857,7 @@ CHECKSUM_FILE="$BACKUP_DIR/SHA256SUMS.txt"
|
|||||||
echo "(/run/media/Second_Drive) and are reconstructable/internal-backup data."
|
echo "(/run/media/Second_Drive) and are reconstructable/internal-backup data."
|
||||||
echo ""
|
echo ""
|
||||||
echo "Artifact listing:"
|
echo "Artifact listing:"
|
||||||
find "$BACKUP_DIR" -mindepth 1 -maxdepth 2 -type f ! -name 'INCOMPLETE' | sort
|
find "$BACKUP_DIR" -mindepth 1 -maxdepth 2 -type f ! -name 'INCOMPLETE' ! -name 'BACKUP_COMPLETE' | sort
|
||||||
} > "$MANIFEST_FILE"
|
} > "$MANIFEST_FILE"
|
||||||
|
|
||||||
# ── Generate checksums for all backup artifacts ─────────────────
|
# ── Generate checksums for all backup artifacts ─────────────────
|
||||||
@@ -869,7 +867,7 @@ log "Generating SHA-256 checksums …"
|
|||||||
cd "$BACKUP_DIR"
|
cd "$BACKUP_DIR"
|
||||||
while IFS= read -r -d '' file; do
|
while IFS= read -r -d '' file; do
|
||||||
sha256sum "$file"
|
sha256sum "$file"
|
||||||
done < <(find . -mindepth 1 -maxdepth 2 -type f ! -name 'SHA256SUMS.txt' ! -name 'INCOMPLETE' -print0 | sort -z)
|
done < <(find . -mindepth 1 -maxdepth 2 -type f ! -name 'SHA256SUMS.txt' ! -name 'INCOMPLETE' ! -name 'BACKUP_COMPLETE' -print0 | sort -z)
|
||||||
) > "$CHECKSUM_FILE"
|
) > "$CHECKSUM_FILE"
|
||||||
|
|
||||||
log "Manifest written to $MANIFEST_FILE"
|
log "Manifest written to $MANIFEST_FILE"
|
||||||
|
|||||||
@@ -336,43 +336,38 @@ class ManualBackupWorkflowTests(unittest.TestCase):
|
|||||||
# ── Behavioral tests ──────────────────────────────────────────────────────
|
# ── Behavioral tests ──────────────────────────────────────────────────────
|
||||||
|
|
||||||
def test_allowlist_accepts_file_changed_message(self):
|
def test_allowlist_accepts_file_changed_message(self):
|
||||||
"""Behavioral: _is_home_warning_allowlisted returns true for the exact
|
"""Behavioral: _is_home_warning_allowlisted (from the actual backup script)
|
||||||
GNU tar 'file changed as we read it' message (LC_ALL=C)."""
|
returns true for the exact GNU tar 'file changed as we read it' message (LC_ALL=C)."""
|
||||||
script = r"""
|
script = f"""#!/usr/bin/env bash
|
||||||
_is_home_warning_allowlisted() {
|
# Load the production function directly from the backup script to avoid logic drift
|
||||||
local msg="$1"
|
eval "$(awk '/^_is_home_warning_allowlisted[(][)]/,/^[}}]/' {BACKUP_SCRIPT})"
|
||||||
case "$msg" in
|
|
||||||
*"file changed as we read it"*) return 0 ;;
|
|
||||||
*"file removed before we read it"*) return 0 ;;
|
|
||||||
*) return 1 ;;
|
|
||||||
esac
|
|
||||||
}
|
|
||||||
msgs=(
|
msgs=(
|
||||||
"tar: /home/user/firefox.db: file changed as we read it"
|
"tar: /home/user/firefox.db: file changed as we read it"
|
||||||
"tar: /home/user/.local/share/app: file removed before we read it"
|
"tar: /home/user/.local/share/app: file removed before we read it"
|
||||||
)
|
)
|
||||||
for msg in "${msgs[@]}"; do
|
for msg in "${{msgs[@]}}"; do
|
||||||
_is_home_warning_allowlisted "$msg" || { echo "BLOCKED: $msg"; exit 1; }
|
_is_home_warning_allowlisted "$msg" || {{ echo "BLOCKED: $msg"; exit 1; }}
|
||||||
echo "ALLOWED: $msg"
|
echo "ALLOWED: $msg"
|
||||||
done
|
done
|
||||||
"""
|
"""
|
||||||
result = subprocess.run(["bash", "-c", script], capture_output=True, text=True)
|
with tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) as f:
|
||||||
self.assertEqual(result.returncode, 0, f"stderr: {result.stderr}")
|
f.write(script)
|
||||||
self.assertIn("ALLOWED", result.stdout)
|
script_path = f.name
|
||||||
self.assertNotIn("BLOCKED", result.stdout)
|
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):
|
def test_allowlist_rejects_fatal_diagnostics(self):
|
||||||
"""Behavioral: _is_home_warning_allowlisted returns false for permission
|
"""Behavioral: _is_home_warning_allowlisted (from the actual backup script)
|
||||||
errors, I/O errors, write errors, and other fatal conditions."""
|
returns false for permission errors, I/O errors, write errors, and other
|
||||||
script = r"""
|
fatal conditions."""
|
||||||
_is_home_warning_allowlisted() {
|
script = f"""#!/usr/bin/env bash
|
||||||
local msg="$1"
|
# Load the production function directly from the backup script to avoid logic drift
|
||||||
case "$msg" in
|
eval "$(awk '/^_is_home_warning_allowlisted[(][)]/,/^[}}]/' {BACKUP_SCRIPT})"
|
||||||
*"file changed as we read it"*) return 0 ;;
|
|
||||||
*"file removed before we read it"*) return 0 ;;
|
|
||||||
*) return 1 ;;
|
|
||||||
esac
|
|
||||||
}
|
|
||||||
msgs=(
|
msgs=(
|
||||||
"tar: /home/user/file: Permission denied"
|
"tar: /home/user/file: Permission denied"
|
||||||
"tar: /dev/sdb: Cannot read: Input/output error"
|
"tar: /dev/sdb: Cannot read: Input/output error"
|
||||||
@@ -380,15 +375,21 @@ msgs=(
|
|||||||
"Write error"
|
"Write error"
|
||||||
"tar: /home/user: Cannot stat: No such file or directory"
|
"tar: /home/user: Cannot stat: No such file or directory"
|
||||||
)
|
)
|
||||||
for msg in "${msgs[@]}"; do
|
for msg in "${{msgs[@]}}"; do
|
||||||
_is_home_warning_allowlisted "$msg" && { echo "ALLOWED_BUT_SHOULD_NOT: $msg"; exit 1; }
|
_is_home_warning_allowlisted "$msg" && {{ echo "ALLOWED_BUT_SHOULD_NOT: $msg"; exit 1; }}
|
||||||
echo "CORRECTLY_BLOCKED: $msg"
|
echo "CORRECTLY_BLOCKED: $msg"
|
||||||
done
|
done
|
||||||
"""
|
"""
|
||||||
result = subprocess.run(["bash", "-c", script], capture_output=True, text=True)
|
with tempfile.NamedTemporaryFile(mode="w", suffix=".sh", delete=False) as f:
|
||||||
self.assertEqual(result.returncode, 0, f"stderr: {result.stderr}")
|
f.write(script)
|
||||||
self.assertIn("CORRECTLY_BLOCKED", result.stdout)
|
script_path = f.name
|
||||||
self.assertNotIn("ALLOWED_BUT_SHOULD_NOT", result.stdout)
|
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):
|
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
|
"""Behavioral: _create_archive_impl in HOME mode accepts tar exit 1 when
|
||||||
|
|||||||
Reference in New Issue
Block a user