diff --git a/src/pobsync_backend/retention.py b/src/pobsync_backend/retention.py index c658db3..bc7fd3b 100644 --- a/src/pobsync_backend/retention.py +++ b/src/pobsync_backend/retention.py @@ -103,6 +103,7 @@ def run_sql_retention_apply( raise ConfigError(f"Refusing to delete unsupported snapshot kind: {snap_kind!r}") path = _snapshot_delete_path(path=Path(snap_path), dirname=dirname) + _validate_snapshot_delete_path(host=host, kind=snap_kind, path=path, dirname=dirname) reason = str(item.get("reason") or "outside retention policy") if not path.exists(): actions.append(f"skip missing {snap_kind}/{dirname}") @@ -339,14 +340,55 @@ def _validate_incomplete_delete_path(*, host: str, path: Path, dirname: str) -> raise ConfigError(f"Refusing to delete incomplete snapshot outside host backup root: {path}") +def _validate_snapshot_delete_path(*, host: str, kind: str, path: Path, dirname: str) -> None: + if kind not in {SnapshotRecord.Kind.SCHEDULED, SnapshotRecord.Kind.MANUAL}: + raise ConfigError(f"Refusing to delete unsupported snapshot kind: {kind!r}") + path_parts = path.parts + if path.name != dirname or kind not in path_parts or host not in path_parts: + raise ConfigError(f"Refusing to delete unexpected snapshot path: {path}") + kind_index = path_parts.index(kind) + if kind_index == 0 or path_parts[kind_index - 1] != host: + raise ConfigError(f"Refusing to delete snapshot outside host backup root: {path}") + + def _remove_snapshot_tree(path: Path) -> None: - _make_directories_user_writable(path) - shutil.rmtree(path) + _make_snapshot_tree_user_removable(path) + shutil.rmtree(path, onexc=_retry_remove_with_user_permissions) -def _make_directories_user_writable(path: Path) -> None: - for directory in [path, *[child for child in path.rglob("*") if child.is_dir() and not child.is_symlink()]]: - mode = directory.stat().st_mode - if mode & stat.S_IWUSR: +def _make_snapshot_tree_user_removable(path: Path) -> None: + stack = [path] + while stack: + directory = stack.pop() + if directory.is_symlink(): continue - directory.chmod(mode | stat.S_IWUSR) + _make_path_user_removable(directory) + try: + children = list(directory.iterdir()) + except OSError: + continue + for child in children: + if child.is_dir() and not child.is_symlink(): + stack.append(child) + + +def _retry_remove_with_user_permissions(function: Any, path: str, excinfo: BaseException) -> None: + failed_path = Path(path) + _make_path_user_removable(failed_path) + function(path) + + +def _make_path_user_removable(path: Path) -> None: + try: + mode = path.stat().st_mode + except OSError: + return + wanted = stat.S_IRUSR | stat.S_IWUSR + if path.is_dir() and not path.is_symlink(): + wanted |= stat.S_IXUSR + if mode & wanted == wanted: + return + try: + path.chmod(mode | wanted) + except OSError: + return diff --git a/src/pobsync_backend/tests/test_sql_retention.py b/src/pobsync_backend/tests/test_sql_retention.py index 9918353..bfb84ca 100644 --- a/src/pobsync_backend/tests/test_sql_retention.py +++ b/src/pobsync_backend/tests/test_sql_retention.py @@ -154,6 +154,68 @@ class SqlRetentionTests(TestCase): ], ) + def test_apply_deletes_snapshot_with_non_traversable_nested_directory(self) -> None: + with TemporaryDirectory() as tmp: + prefix = Path(tmp) / "home" + host = HostConfig.objects.create( + host="web-01", + address="web-01.example.test", + retention_daily=0, + retention_weekly=0, + retention_monthly=0, + retention_yearly=0, + ) + old_dir = Path(tmp) / "backups" / host.host / "scheduled" / "20260518-021500Z__OLD" + restricted_dir = old_dir / "data" / "var" / "lib" / "snapd" / "void" + restricted_dir.mkdir(parents=True) + restricted_dir.joinpath("state").write_text("preserved permissions\n") + restricted_dir.chmod(0) + new_dir = Path(tmp) / "backups" / host.host / "scheduled" / "20260519-021500Z__NEW" + new_dir.mkdir(parents=True) + old = self._snapshot(host, old_dir.name, path=str(old_dir)) + self._snapshot(host, new_dir.name, path=str(new_dir)) + + result = run_sql_retention_apply( + prefix=prefix, + host=host.host, + kind="scheduled", + protect_bases=False, + yes=True, + max_delete=1, + acquire_lock=False, + ) + + self.assertFalse(old_dir.exists()) + self.assertFalse(SnapshotRecord.objects.filter(pk=old.pk).exists()) + self.assertEqual(result["deleted"][0]["dirname"], old.dirname) + + def test_apply_rejects_scheduled_snapshot_path_outside_host_kind_directory(self) -> None: + host = HostConfig.objects.create( + host="web-01", + address="web-01.example.test", + retention_daily=0, + retention_weekly=0, + retention_monthly=0, + retention_yearly=0, + ) + self._snapshot( + host, + "20260518-021500Z__OLD", + path="/backups/web-01/manual/20260518-021500Z__OLD", + ) + self._snapshot(host, "20260519-021500Z__NEW") + + with self.assertRaisesRegex(ConfigError, "unexpected snapshot path"): + run_sql_retention_apply( + prefix=Path("/tmp/pobsync-test"), + host=host.host, + kind="scheduled", + protect_bases=False, + yes=True, + max_delete=1, + acquire_lock=False, + ) + def test_apply_respects_max_delete(self) -> None: host = HostConfig.objects.create( host="web-01", @@ -213,6 +275,36 @@ class SqlRetentionTests(TestCase): self.assertEqual(purged.action, PurgedSnapshot.Action.INCOMPLETE_CLEANUP) self.assertEqual(purged.reason, "manual incomplete cleanup") + def test_incomplete_cleanup_deletes_non_traversable_nested_directory(self) -> None: + with TemporaryDirectory() as tmp: + prefix = Path(tmp) / "home" + host = HostConfig.objects.create(host="web-01", address="web-01.example.test") + incomplete_dir = Path(tmp) / "backups" / host.host / ".incomplete" / "20260519-031500Z__BROKEN01" + restricted_dir = incomplete_dir / "data" / "var" / "lib" / "snapd" / "void" + restricted_dir.mkdir(parents=True) + restricted_dir.joinpath("state").write_text("interrupted\n") + restricted_dir.chmod(0) + record = SnapshotRecord.objects.create( + host=host, + kind=SnapshotRecord.Kind.INCOMPLETE, + dirname=incomplete_dir.name, + path=str(incomplete_dir), + status="failed", + started_at=datetime(2026, 5, 19, 3, 15, tzinfo=timezone.utc), + ) + + result = run_incomplete_cleanup( + prefix=prefix, + host=host.host, + yes=True, + max_delete=1, + acquire_lock=False, + ) + + self.assertFalse(incomplete_dir.exists()) + self.assertFalse(SnapshotRecord.objects.filter(pk=record.pk).exists()) + self.assertEqual(result["deleted"][0]["dirname"], incomplete_dir.name) + def test_incomplete_cleanup_respects_max_delete(self) -> None: host = HostConfig.objects.create(host="web-01", address="web-01.example.test") SnapshotRecord.objects.create(