Clean up failed backup publication safely
This commit is contained in:
1 parent
4c4b51270c
commit
3d267afaee
4 files changed
+100
-4
No files matched your search
@@ -24,7 +24,10 @@ verified production backup, so production deployment remains blocked.
|
||||
SYSTEM, or local Administrators; other and deny entries fail closed.
|
||||
It writes into a private staging directory, checks the
|
||||
stream size and SHA-256, verifies the archive and manifest, then publishes
|
||||
the complete backup directory without overwriting an existing ID.
|
||||
the complete backup directory without overwriting an existing ID. It checks
|
||||
the published ACL before acknowledging; on failure it removes only the same
|
||||
verified staging directory, and refuses cleanup if its identity or contents
|
||||
have changed.
|
||||
- Only after Windows confirms the stored archive identity does the VPS proceed
|
||||
with image cutover. If backup transport or verification fails, it attempts
|
||||
to restart the previous services and records a recovery journal if restart
|
||||
|
||||
@@ -30,6 +30,10 @@ remains blocked by the backup gate.
|
||||
NTFS/path/ACL owner and allowed entries, and the local ignore rule before
|
||||
SSH; it requires 100 GiB plus 1 MiB free. These code changes remain
|
||||
unverified on Windows and the VPS.
|
||||
- Receiver cleanup after a post-publication ACL failure is identity-bound to
|
||||
the owned staging directory and permits only the expected archive files. A
|
||||
replaced, reparse, or unexpected target is left for manual review without an
|
||||
ACK; this path is covered by local fixtures and remains pending Windows rerun.
|
||||
- Synthetic tests cover archive/protocol integrity, helper download rules,
|
||||
artifact/run provenance, receiver publication, release schema, and rollback
|
||||
refusal. Windows ACL, NTFS, PowerShell, SSH alias, Gitea helper against the
|
||||
|
||||
@@ -8,6 +8,7 @@ import json
|
||||
import os
|
||||
import secrets
|
||||
import shutil
|
||||
import stat
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
@@ -34,6 +35,28 @@ def _reparse(path: Path) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def _directory_identity(path: Path) -> tuple[int, int]:
|
||||
info = path.lstat()
|
||||
if _reparse(path) or not stat.S_ISDIR(info.st_mode) or not info.st_ino:
|
||||
raise ValueError("backup publication directory is not a stable real directory")
|
||||
return info.st_dev, info.st_ino
|
||||
|
||||
|
||||
def _remove_owned_directory(path: Path, identity: tuple[int, int]) -> None:
|
||||
try:
|
||||
current = _directory_identity(path)
|
||||
except FileNotFoundError:
|
||||
return
|
||||
if current != identity:
|
||||
raise ValueError(f"refusing to remove a replaced backup directory: {path}")
|
||||
for child in path.iterdir():
|
||||
child_info = child.lstat()
|
||||
if (child.name not in {"backup.tar.gz", "backup.tar.gz.sha256"}
|
||||
or _reparse(child) or not stat.S_ISREG(child_info.st_mode)):
|
||||
raise ValueError(f"refusing to remove unexpected backup content: {child}")
|
||||
shutil.rmtree(path)
|
||||
|
||||
|
||||
def check_private_acl(path: Path) -> None:
|
||||
command = r"""
|
||||
$ErrorActionPreference = 'Stop'
|
||||
@@ -171,7 +194,11 @@ def receive_backup(reader, writer, *, repository: Path, request: dict, expected_
|
||||
|
||||
root, final = check_repository_target(repository, header["backup_id"])
|
||||
staging = Path(tempfile.mkdtemp(prefix=".staging-", dir=root))
|
||||
staging_identity = None
|
||||
published_identity = None
|
||||
ack_started = False
|
||||
try:
|
||||
staging_identity = _directory_identity(staging)
|
||||
check_private_acl(staging)
|
||||
archive_path = staging / "backup.tar.gz"
|
||||
with archive_path.open("xb") as archive_file:
|
||||
@@ -194,17 +221,30 @@ def receive_backup(reader, writer, *, repository: Path, request: dict, expected_
|
||||
os.fsync(stream.fileno())
|
||||
os.chmod(sidecar, 0o600)
|
||||
check_private_acl(sidecar)
|
||||
if final.exists() or _reparse(final):
|
||||
raise ValueError("backup ID already exists or resolves through a reparse point")
|
||||
os.rename(staging, final)
|
||||
published_identity = staging_identity
|
||||
if _directory_identity(final) != published_identity:
|
||||
raise ValueError("published backup directory identity changed during rename")
|
||||
for path in (final, final / "backup.tar.gz", final / "backup.tar.gz.sha256"):
|
||||
check_private_acl(path)
|
||||
ack_started = True
|
||||
write_ack(
|
||||
writer, session_nonce=header["nonce"], backup_id=header["backup_id"], size=size,
|
||||
archive_sha256=archive_sha256, manifest_sha256=header["manifest_sha256"], verified=True,
|
||||
)
|
||||
return {"backup_id": header["backup_id"], "archive_sha256": archive_sha256, "size": size}
|
||||
except Exception:
|
||||
if staging.exists():
|
||||
shutil.rmtree(staging)
|
||||
except Exception as error:
|
||||
cleanup_path = final if published_identity is not None and not ack_started else staging
|
||||
cleanup_identity = published_identity if cleanup_path == final else staging_identity
|
||||
if cleanup_identity is not None:
|
||||
try:
|
||||
_remove_owned_directory(cleanup_path, cleanup_identity)
|
||||
except Exception as cleanup_error:
|
||||
raise ValueError(
|
||||
f"backup failed and its output could not be safely removed; inspect {cleanup_path}"
|
||||
) from cleanup_error
|
||||
raise
|
||||
|
||||
|
||||
|
||||
@@ -112,6 +112,55 @@ class ExternalReceiverTests(unittest.TestCase):
|
||||
self.assertEqual(list(root.iterdir()), [])
|
||||
self.assertEqual(ack.getvalue(), b"")
|
||||
|
||||
def test_post_publish_acl_failure_removes_owned_final_without_ack(self):
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
root = Path(directory)
|
||||
final = root / "backup-20261031T120000Z"
|
||||
stream, _manifest_sha, _size, _digest = self.source_stream()
|
||||
ack = io.BytesIO()
|
||||
|
||||
def fail_published_archive(path):
|
||||
if path == final / "backup.tar.gz":
|
||||
raise ValueError("post-publish ACL failed")
|
||||
|
||||
with patch.object(receive_external, "check_repository_target", return_value=(root, final)), \
|
||||
patch.object(receive_external, "check_private_acl", side_effect=fail_published_archive):
|
||||
with self.assertRaisesRegex(ValueError, "post-publish ACL failed"):
|
||||
receive_external.receive_backup(stream, ack, repository=root, request=self.request)
|
||||
self.assertFalse(final.exists())
|
||||
self.assertEqual(list(root.iterdir()), [])
|
||||
self.assertEqual(ack.getvalue(), b"")
|
||||
|
||||
def test_cleanup_refuses_a_replaced_or_reparse_directory(self):
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
root = Path(directory)
|
||||
owned = root / "owned"
|
||||
final = root / "final"
|
||||
owned.mkdir()
|
||||
identity = receive_external._directory_identity(owned)
|
||||
owned.rename(root / "moved-owned")
|
||||
final.mkdir()
|
||||
with self.assertRaisesRegex(ValueError, "replaced backup directory"):
|
||||
receive_external._remove_owned_directory(final, identity)
|
||||
self.assertTrue(final.is_dir())
|
||||
self.assertTrue((root / "moved-owned").is_dir())
|
||||
if os.name != "nt":
|
||||
link = root / "link"
|
||||
link.symlink_to(final, target_is_directory=True)
|
||||
with self.assertRaisesRegex(ValueError, "real directory"):
|
||||
receive_external._remove_owned_directory(link, identity)
|
||||
self.assertTrue(final.is_dir())
|
||||
|
||||
def test_cleanup_refuses_unexpected_directory_contents(self):
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
path = Path(directory) / "owned"
|
||||
path.mkdir()
|
||||
identity = receive_external._directory_identity(path)
|
||||
(path / "foreign-data").write_text("keep", encoding="utf-8")
|
||||
with self.assertRaisesRegex(ValueError, "unexpected backup content"):
|
||||
receive_external._remove_owned_directory(path, identity)
|
||||
self.assertEqual((path / "foreign-data").read_text(encoding="utf-8"), "keep")
|
||||
|
||||
def test_target_requires_exact_local_git_exclude_rule(self):
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
repository = Path(directory)
|
||||
|
||||
Reference in new issue
Block a user