From 7e5d31d53b1a22b0b300c4c1c2a8bec571b43570 Mon Sep 17 00:00:00 2001 From: Peter van Arkel Date: Tue, 19 May 2026 20:09:35 +0200 Subject: [PATCH] (bugfix) Guard dry-run logs and recursive rsync defaults Clear the reused dry-run rsync log before each dry-run so run details only show output from the current execution. Populate new Django global configs with the existing safe rsync and exclude defaults, including archive mode and standard pseudo-filesystem exclusions. Add a host check that fails when effective rsync args do not include archive or recursive transfer, preventing real backups that only report "skipping directory .". --- src/pobsync/commands/run_scheduled.py | 1 + src/pobsync_backend/host_ops.py | 22 +++++++++++++++++++ .../tests/test_run_scheduled_config_source.py | 22 +++++++++++++++++++ src/pobsync_backend/tests/test_views.py | 13 +++++++++++ src/pobsync_backend/views.py | 3 +++ 5 files changed, 61 insertions(+) diff --git a/src/pobsync/commands/run_scheduled.py b/src/pobsync/commands/run_scheduled.py index ef68c3f..e19ecee 100644 --- a/src/pobsync/commands/run_scheduled.py +++ b/src/pobsync/commands/run_scheduled.py @@ -151,6 +151,7 @@ def run_scheduled( if dry_run: dest = f"/tmp/pobsync-dryrun/{host}/" dryrun_log = Path(f"/tmp/pobsync-dryrun/{host}/rsync.log") + dryrun_log.unlink(missing_ok=True) cmd = build_rsync_command( rsync_binary=str(rsync_binary), diff --git a/src/pobsync_backend/host_ops.py b/src/pobsync_backend/host_ops.py index f65c2c8..ff60eec 100644 --- a/src/pobsync_backend/host_ops.py +++ b/src/pobsync_backend/host_ops.py @@ -78,9 +78,31 @@ def collect_host_checks(host: HostConfig, global_config: GlobalConfig | None = N checks.append(_host_path_check("Host backup root", host_root, must_exist=True, must_be_writable=True)) for subdir in HOST_BACKUP_SUBDIRS: checks.append(_host_path_check(f"Host directory: {subdir}", host_root / subdir, must_exist=True, must_be_writable=True)) + checks.append(_rsync_recursion_check(host, global_config)) return checks +def _rsync_recursion_check(host: HostConfig, global_config: GlobalConfig) -> SelfCheck: + args = [*list(global_config.rsync_args or []), *list(global_config.rsync_extra_args or []), *list(host.rsync_extra_args or [])] + if _has_recursive_rsync_arg(args): + return SelfCheck("Host rsync recursion", "ok", "Rsync args include archive or recursive transfer.", " ".join(args)) + return SelfCheck( + "Host rsync recursion", + "failed", + "Rsync args do not include archive or recursive transfer.", + "Add --archive or --recursive before running a real backup.", + ) + + +def _has_recursive_rsync_arg(args: list[str]) -> bool: + for arg in args: + if arg in {"--archive", "--recursive"}: + return True + if arg.startswith("-") and not arg.startswith("--") and any(flag in arg for flag in ("a", "r")): + return True + return False + + def _host_path_check( name: str, path: Path, diff --git a/src/pobsync_backend/tests/test_run_scheduled_config_source.py b/src/pobsync_backend/tests/test_run_scheduled_config_source.py index 29c22ae..6e838dd 100644 --- a/src/pobsync_backend/tests/test_run_scheduled_config_source.py +++ b/src/pobsync_backend/tests/test_run_scheduled_config_source.py @@ -67,6 +67,28 @@ class RunScheduledConfigSourceTests(SimpleTestCase): self.assertEqual(result["rsync"]["exit_code"], 12) self.assertEqual(result["rsync"]["log_tail"], ["Permission denied (publickey).", "rsync error"]) + def test_dry_run_clears_previous_log_before_running(self) -> None: + def fake_run_rsync(command, log_path, timeout_seconds): + self.assertFalse(log_path.exists()) + log_path.parent.mkdir(parents=True, exist_ok=True) + log_path.write_text("current run only\n", encoding="utf-8") + return RsyncResult(exit_code=0, command=command) + + old_log = Path("/tmp/pobsync-dryrun/web-01/rsync.log") + old_log.parent.mkdir(parents=True, exist_ok=True) + old_log.write_text("old failure\n", encoding="utf-8") + + with patch("pobsync.commands.run_scheduled.run_rsync", side_effect=fake_run_rsync): + result = run_scheduled( + prefix=Path("/missing-prefix"), + host="web-01", + dry_run=True, + config_source=FakeConfigSource(), + ) + + self.assertTrue(result["ok"]) + self.assertEqual(result["rsync"]["log_tail"], ["current run only"]) + def test_successful_real_run_applies_prune_when_requested(self) -> None: with TemporaryDirectory() as tmp: prefix = Path(tmp) / "home" diff --git a/src/pobsync_backend/tests/test_views.py b/src/pobsync_backend/tests/test_views.py index 1efcbb0..c8a6dcf 100644 --- a/src/pobsync_backend/tests/test_views.py +++ b/src/pobsync_backend/tests/test_views.py @@ -331,6 +331,8 @@ class ViewTests(TestCase): self.assertEqual(response.status_code, 200) self.assertContains(response, f'value="{credential.id}" selected') + self.assertContains(response, "--archive") + self.assertContains(response, "/proc/***") def test_global_config_form_renders_static_container_backup_root_on_edit(self) -> None: self.client.force_login(self.staff_user) @@ -522,6 +524,17 @@ class ViewTests(TestCase): self.assertTrue((backup_root / host.host / "manual").is_dir()) self.assertTrue((backup_root / host.host / ".incomplete").is_dir()) + def test_host_detail_warns_when_rsync_is_not_recursive(self) -> None: + self.client.force_login(self.staff_user) + GlobalConfig.objects.create(name="default", backup_root="/backups", rsync_args=[]) + host = HostConfig.objects.create(host="web-01", address="web-01.example.test") + + response = self.client.get(reverse("host_detail", args=[host.host])) + + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Host rsync recursion") + self.assertContains(response, "Rsync args do not include archive or recursive transfer.") + def test_scan_host_known_key_action_updates_selected_credential(self) -> None: self.client.force_login(self.staff_user) credential = SshCredential.objects.create(name="default-key", key_path="/var/lib/pobsync/state/ssh-credentials/1/identity") diff --git a/src/pobsync_backend/views.py b/src/pobsync_backend/views.py index 8fdd794..3fe1cf8 100644 --- a/src/pobsync_backend/views.py +++ b/src/pobsync_backend/views.py @@ -13,6 +13,7 @@ from django.shortcuts import get_object_or_404, redirect, render from django.views.decorators.http import require_POST from pobsync.errors import PobsyncError +from pobsync.config.defaults import DEFAULT_EXCLUDES, DEFAULT_RSYNC_ARGS from .backup_runner import queue_backup_run from .forms import ( @@ -519,7 +520,9 @@ def _default_global_initial() -> dict[str, object]: "ssh_user": "root", "ssh_port": 22, "rsync_binary": "rsync", + "rsync_args": DEFAULT_RSYNC_ARGS, "default_source_root": "/", + "excludes_default": DEFAULT_EXCLUDES, "retention_daily": 14, "retention_weekly": 8, "retention_monthly": 12,