fix(updater): retain two verified backup generations

scripts/update/tests/host_storage_integration.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-"""Real migration -> adoption -> two updates, including a backup restore, on synthetic data.
+"""Real migration -> adoption -> three updates, retention and restore on synthetic data.
 
 Requires local storage test images and registry:2; never contacts a FEDEO instance.
 Release HTTP/health responses are fixtures; Compose, registry, SDK, PostgreSQL,
@@ -101,7 +101,8 @@ def run():
             docker('push', image['repository'] + ':fixture')
         # Copy health, networking and SDK settings from the original fixture, but start the new image's migration script.
         updater.releases = Releases(config, images)
-        for version in ('3.4.0', '3.5.0'):
+        backups = []
+        for version in ('3.4.0', '3.5.0', '3.6.0'):
             http.version = version
             checked = updater.execute('check', version)
             assert checked['plan']['storageProvider'] == 'seaweedfs'
@@ -112,6 +113,15 @@ def run():
             old_matrix = migration.inspect(retained)
             assert not old_matrix['State']['Running'] and old_matrix['HostConfig']['RestartPolicy']['Name'] == 'no'
             backup = Path(result['backup'])
+            backups.append(backup)
+            retention = result['backupRetention']
+            assert retention['status'] == 'complete', retention
+            assert len(retention['retained']) == min(len(backups), 2)
+            if len(backups) == 3:
+                assert not backups[0].exists()
+                assert u.read_json(backups[0].parent / 'backup-pruned.json')['status'] == 'removed'
+                assert backups[1].is_dir() and backups[2].is_dir()
+                assert (root / '.updates/storage-migration/backup/complete.json').is_file()
             with tarfile.open(backup / 'files.tar') as archive:
                 assert archive.extractfile('matrix/postgres/retained.txt').read() == b'retired Matrix'
                 assert 'storage/config/s3.json' in archive.getnames()
@@ -136,7 +146,8 @@ def run():
             migration.helper('restore-verify', endpoint='http://' + restore_name + ':9000')
         finally:
             docker('rm', '-f', restore_name)
-        print(json.dumps({'result': 'passed', 'restoredUpdaterBackup': str(backup), 'evidence': str(root)}), flush=True)
+        print(json.dumps({'result': 'passed', 'retention': 'two_generations_verified',
+                          'restoredUpdaterBackup': str(backup), 'evidence': str(root)}), flush=True)
     except BaseException:
         print('Failure evidence: ' + str(root), flush=True)
         for path in sorted((root / '.updates').glob('*/commands.log')):

scripts/update/tests/test_backup_retention.py

@@ -0,0 +1,190 @@
+import json
+from pathlib import Path
+import shutil
+import tempfile
+import unittest
+from unittest.mock import patch
+
+from test_updater import u
+
+
+class RetentionRunner:
+    def __init__(self, log, owner):
+        self.log, self.owner = log, owner
+
+    def run(self, command):
+        if command[:2] == ['docker', 'ps']:
+            return 'container' if self.owner.mounts else ''
+        if command[:2] == ['docker', 'inspect']:
+            return json.dumps([{'Mounts': self.owner.mounts}])
+        raise AssertionError(command)
+
+
+class RetentionTests(unittest.TestCase):
+    def setUp(self):
+        self.temp = tempfile.TemporaryDirectory()
+        self.addCleanup(self.temp.cleanup)
+        self.root = Path(self.temp.name)
+        (self.root / 'docker-compose.yml').write_text('unchanged compose')
+        self.mounts = []
+        self.updater = u.Updater(self.root, runner_factory=lambda log: RetentionRunner(log, self))
+        self.jobs = [self.backup(i) for i in range(1, 5)]
+        latest = u.read_json(self.jobs[-1] / 'status.json')
+        u.write_json(self.updater.home / 'installed.json', {
+            'manifest': {'version': latest['version'], 'commit': latest['revision']},
+            'composeSha256': u.file_sha(self.root / 'docker-compose.yml')})
+        u.write_json(self.updater.home / 'status.json', latest)
+
+    def backup(self, index, *, name=None, status='succeeded', command='apply'):
+        job = self.updater.home / (name or f'202601{index:02d}-120000-{index:08x}')
+        directory = job / 'backup'
+        directory.mkdir(parents=True)
+        files = ['database.dump', 'globals.sql', 'files.tar', 'files.list', 'containers.json',
+                 'config/.env', 'config/docker-compose.yml']
+        for name in files:
+            path = directory / name
+            path.parent.mkdir(exist_ok=True)
+            path.write_text(f'{index}: {name}')
+        u.write_json(directory / 'complete.json', {
+            'database': 'database.dump', 'files': 'files.tar', 'storageProvider': 'minio',
+            'sha256': {name: u.file_sha(directory / name) for name in files}})
+        u.write_json(job / 'status.json', {
+            'command': command, 'status': status, 'phase': 'complete', 'job': str(job),
+            'backup': str(directory), 'maintenance': False, 'recoveryRequired': False,
+            'version': f'3.{index}.0', 'revision': str(index) * 40,
+            'startedAt': f'2026-01-{index:02d}T12:00:00+00:00'})
+        (job / 'commands.log').write_text('audit log stays')
+        return job
+
+    def test_keep_two_and_preserve_failed_migration_and_audit_records(self):
+        special = [self.backup(5, status='failed'), self.backup(6, name='storage-migration'),
+                   self.backup(7, name='foreign'), self.backup(8, command='check')]
+        report = self.updater.prune_backups()
+        self.assertEqual(report['status'], 'complete')
+        self.assertEqual(len(report['removed']), 2)
+        self.assertEqual(len(report['retained']), 2)
+        for job in self.jobs[:2]:
+            self.assertFalse((job / 'backup').exists())
+            self.assertEqual(u.read_json(job / 'backup-pruned.json')['status'], 'removed')
+            self.assertTrue((job / 'commands.log').is_file())
+        for job in self.jobs[2:] + special:
+            self.assertTrue((job / 'backup/complete.json').is_file())
+        self.assertEqual(self.updater.prune_backups()['removed'], [])
+
+    def test_dry_run_checks_but_does_not_delete_or_write_markers(self):
+        result = self.updater.prune_backups(dry_run=True)
+        self.assertEqual(len(result['planned']), 2)
+        self.assertFalse((self.updater.home / 'backup-retention.json').exists())
+        for job in self.jobs:
+            self.assertTrue((job / 'backup').is_dir())
+            self.assertFalse((job / 'backup-pruned.json').exists())
+
+    def test_corrupt_either_retained_generation_prevents_all_deletions(self):
+        for job in self.jobs[2:]:
+            with self.subTest(job=job.name):
+                path = job / 'backup/database.dump'
+                original = path.read_bytes()
+                path.write_bytes(b'corrupt')
+                with self.assertRaisesRegex(u.UpdateError, 'Prüfsumme'):
+                    self.updater.prune_backups()
+                self.assertTrue(all((p / 'backup').exists() for p in self.jobs))
+                path.write_bytes(original)
+
+    def test_incomplete_retained_backup_prevents_deletion(self):
+        (self.jobs[-1] / 'backup/globals.sql').unlink()
+        with self.assertRaisesRegex(u.UpdateError, 'fehlt'):
+            self.updater.prune_backups()
+        self.assertTrue((self.jobs[0] / 'backup').exists())
+
+    def test_seaweedfs_requires_image_and_provider_snapshot(self):
+        path = self.jobs[-1] / 'backup/complete.json'
+        manifest = u.read_json(path)
+        manifest['storageProvider'] = 'seaweedfs'
+        u.write_json(path, manifest)
+        with self.assertRaisesRegex(u.UpdateError, 'Unvollständiges'):
+            self.updater.prune_backups()
+        self.assertTrue((self.jobs[0] / 'backup').exists())
+
+    def test_unknown_files_and_links_are_preserved(self):
+        (self.jobs[0] / 'backup/foreign.txt').write_text('do not delete')
+        outside = self.root / 'outside'
+        outside.write_text('keep')
+        (self.jobs[1] / 'backup/files.tar').unlink()
+        (self.jobs[1] / 'backup/files.tar').symlink_to(outside)
+        result = self.updater.prune_backups()
+        self.assertEqual(len(result['skipped']), 2)
+        self.assertEqual(result['removed'], [])
+        self.assertEqual(outside.read_text(), 'keep')
+
+    def test_symlinked_backup_directory_is_not_followed(self):
+        backup = self.jobs[0] / 'backup'
+        outside = self.root / 'outside'
+        backup.rename(outside)
+        backup.symlink_to(outside, target_is_directory=True)
+        result = self.updater.prune_backups()
+        self.assertTrue(outside.is_dir())
+        self.assertTrue(backup.is_symlink())
+        self.assertEqual(len(result['skipped']), 1)
+
+    def test_manifest_path_escape_is_rejected(self):
+        path = self.jobs[-1] / 'backup/complete.json'
+        manifest = u.read_json(path)
+        manifest['sha256']['../outside'] = 'a' * 64
+        u.write_json(path, manifest)
+        with self.assertRaisesRegex(u.UpdateError, 'Ungültiger Pfad'):
+            self.updater.prune_backups()
+        self.assertTrue((self.jobs[0] / 'backup').exists())
+
+    def test_container_mount_protects_backup_but_metrics_root_mount_does_not(self):
+        self.mounts = [{'Source': '/', 'RW': False},
+                       {'Source': str(self.jobs[0] / 'backup/files.tar'), 'RW': False}]
+        result = self.updater.prune_backups()
+        self.assertEqual(len(result['skipped']), 1)
+        self.assertTrue((self.jobs[0] / 'backup').exists())
+        self.assertFalse((self.jobs[1] / 'backup').exists())
+
+    def test_same_device_bind_mount_is_protected(self):
+        read_text = Path.read_text
+        mount = self.jobs[0] / 'backup/config'
+        def read(path, *args, **kwargs):
+            if path == Path('/proc/self/mountinfo'):
+                return f'1 2 3:4 / {mount} rw - ext4 /dev/test rw\n'
+            return read_text(path, *args, **kwargs)
+        with patch.object(Path, 'read_text', read):
+            result = self.updater.prune_backups()
+        self.assertEqual(len(result['skipped']), 1)
+        self.assertTrue((self.jobs[0] / 'backup').exists())
+
+    def test_partial_deletion_can_resume_without_touching_retained_backups(self):
+        original = shutil.rmtree
+        def fail(path, *args, **kwargs):
+            if path == self.jobs[0] / 'backup':
+                (path / 'complete.json').unlink()
+                (path / 'database.dump').unlink()
+                raise OSError('simulated disk error')
+            return original(path, *args, **kwargs)
+        with patch.object(u.shutil, 'rmtree', fail):
+            result = self.updater.prune_backups()
+        self.assertEqual(len(result['skipped']), 1)
+        self.assertEqual(u.read_json(self.jobs[0] / 'backup-pruned.json')['status'], 'deleting')
+        result = self.updater.prune_backups()
+        self.assertEqual(len(result['removed']), 1)
+        self.assertTrue(all((p / 'backup').exists() for p in self.jobs[2:]))
+
+    def test_recovery_running_and_lock_block_cleanup(self):
+        for state in ({'recoveryRequired': True}, {'maintenance': True}, {'status': 'running'},
+                      {'phase': 'storage_migration'}):
+            with self.subTest(state=state):
+                u.write_json(self.updater.home / 'status.json', state)
+                with self.assertRaises(u.UpdateError):
+                    self.updater.prune_backups()
+        u.write_json(self.updater.home / 'status.json', {'status': 'failed', 'recoveryRequired': False})
+        with self.updater.lock(), self.assertRaises(u.UpdateError):
+            self.updater.prune_backups()
+        self.assertEqual(len(self.updater.prune_backups()['removed']), 2)
+
+    def test_changed_installation_blocks_cleanup(self):
+        (self.root / 'docker-compose.yml').write_text('manually restored')
+        with self.assertRaisesRegex(u.UpdateError, 'installierten Stand'):
+            self.updater.prune_backups()
+        self.assertTrue(all((p / 'backup').exists() for p in self.jobs))

scripts/update/tests/test_updater.py

@@ -646,6 +646,39 @@ class UpdateTests(unittest.TestCase):
         self.assertFalse((self.root / '.updates/installed.json').exists())
         self.assertEqual(self.runner.calls[-1][-3:], ['backend', 'frontend', 'traefik'])
 
+    def test_backup_retention_runs_only_after_successful_public_verification(self):
+        def prune():
+            self.assertEqual(self.updater.status()['status'], 'succeeded')
+            self.assertTrue(any(url.endswith('/api/health') for url in self.http.requests))
+            return {'retained': [], 'removed': [], 'skipped': []}
+        with patch.object(self.updater, '_prune_backups', side_effect=prune) as cleanup:
+            self.run_update()
+            cleanup.assert_called_once()
+
+    def test_failed_update_never_prunes_backups(self):
+        self.http.bad_health = True
+        with patch.object(self.updater, '_prune_backups') as cleanup:
+            with self.assertRaises(u.UpdateError):
+                self.run_update()
+            cleanup.assert_not_called()
+
+    def test_retention_error_does_not_fail_update_or_stop_the_new_application(self):
+        with patch.object(self.updater, '_prune_backups', side_effect=OSError('disk error')):
+            result = self.run_update()
+        self.assertEqual(result['status'], 'succeeded')
+        self.assertEqual(result['backupRetention']['status'], 'warning')
+        self.assertEqual(u.read_json(self.updater.home / 'backup-retention.json')['status'], 'warning')
+        self.assertFalse(result['recoveryRequired'])
+        self.assertEqual(sum('stop' in args for args in self.runner.calls), 1)
+
+    def test_interrupt_during_retention_preserves_completed_update(self):
+        with patch.object(self.updater, '_prune_backups', side_effect=KeyboardInterrupt):
+            with self.assertRaises(KeyboardInterrupt):
+                self.run_update()
+        self.assertEqual(self.updater.status()['status'], 'succeeded')
+        self.assertFalse(self.updater.status()['recoveryRequired'])
+        self.assertEqual(sum('stop' in args for args in self.runner.calls), 1)
+
     def test_resolve_requires_a_working_manually_recovered_installation(self):
         self.runner.fail = lambda args: 'migrate' in args
         with self.assertRaises(u.UpdateError):