Pre-beta review: data safety in storage, catalog and backup runs

Storage and repository:
- Listing skipped every name containing ".part-", directories too, so the
  exports of a user like "john.part-time" were invisible to prune on S3,
  SFTP, FTP and SMB and their data was deleted. Only real temporary uploads
  ("<name>.part-<8 hex>") are skipped now.
- prune refuses to run while a recorded per-user export's manifest is
  missing, instead of freeing that export's data.
- forget() and untrash() copy first, switch the catalog, and delete the
  originals last, so a crash can no longer leave the catalog pointing at a
  missing snapshot or database dump.
- Writes are checked against the expected size, so a short upload is an
  error instead of a truncated pack; raw restores check rename() and touch().
- A catalog write refuses to overwrite a generation another process just
  wrote, and the catalog lock lasts 300 s, so opening a large repository
  inside it no longer lets the lease run out.

Backup runs and scheduling:
- A file that keeps changing is read again at most three times; then the
  last read is kept and reported, instead of restarting forever and
  blocking cleanup.
- Unreadable files and folders are recorded, logged, shown by occ
  backup:run, stored in the snapshot and notified to admins, instead of
  being skipped silently.
- After a failed scheduled backup, at most three attempts a day, an hour
  apart, instead of a new run and notification every five minutes.
- The job no longer spins on a run whose lease another process holds, and
  the weekly cleanup catches up after a Sunday spent backing up.
- Notifications for the per-user export retention events.

lib/Backend/LocalBackend.php

@@ -33,11 +33,16 @@ final class LocalBackend implements IBackend {
 			if ($out === false) {
 				throw new BackendException('Cannot write ' . $path);
 			}
+			$expected = StorageBackend::remaining($stream);
 			if (stream_copy_to_stream($stream, $out) === false || !fflush($out)) {
 				throw new BackendException('Write failed for ' . $path);
 			}
 			fclose($out);
 			$out = false;
+			clearstatcache(true, $tmp);
+			if ($expected !== null && filesize($tmp) !== $expected) {
+				throw new BackendException('Incomplete write for ' . $path);
+			}
 			if (!rename($tmp, $target)) {
 				throw new BackendException('Cannot finalise ' . $path);
 			}
@@ -94,7 +99,7 @@ final class LocalBackend implements IBackend {
 		$result = [];
 		$it = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($base, \FilesystemIterator::SKIP_DOTS));
 		foreach ($it as $file) {
-			if ($file->isFile() && !str_contains($file->getFilename(), '.part-')) {
+			if ($file->isFile() && !StorageBackend::isTemporary($file->getFilename())) {
 				$result[] = ltrim(substr($file->getPathname(), strlen($this->root)), '/');
 			}
 		}

lib/Backend/StorageBackend.php

@@ -30,7 +30,13 @@ final class StorageBackend implements IBackend {
 		}
 	}
 
+	/** A temporary upload of put(): "<name>.part-<8 hex>". */
+	public static function isTemporary(string $name): bool {
+		return preg_match('/\.part-[0-9a-f]{8}$/', $name) === 1;
+	}
+
 	public function put(string $path, $stream): void {
+		$expected = self::remaining($stream);
 		$target = $this->abs($path);
 		$this->mkdirs(dirname($target));
 		$writeTo = $this->atomicPut ? $target : $target . '.part-' . bin2hex(random_bytes(4));
@@ -54,6 +60,10 @@ final class StorageBackend implements IBackend {
 			if (is_resource($stream)) {
 				fclose($stream);
 			}
+			// Some adapters report success after a short write or a failed flush: check the size.
+			if ($expected !== null && $this->storage->filesize($writeTo) !== $expected) {
+				throw new BackendException('Incomplete write for ' . $path);
+			}
 			if ($writeTo !== $target && !$this->storage->rename($writeTo, $target)) {
 				throw new BackendException('Cannot finalise ' . $path);
 			}
@@ -132,19 +142,35 @@ final class StorageBackend implements IBackend {
 			return;
 		}
 		while (($name = readdir($dh)) !== false) {
-			if ($name === '.' || $name === '..' || str_contains($name, '.part-')) {
+			if ($name === '.' || $name === '..') {
 				continue;
 			}
 			$child = $dir . '/' . $name;
 			if ($this->storage->is_dir($child)) {
 				$this->listInto($child, $result);
-			} else {
+			} elseif (!self::isTemporary($name)) {
 				$result[] = $child;
 			}
 		}
 		closedir($dh);
 	}
 
+	/** Bytes left to read in a seekable $stream, or null when it cannot tell. @param resource $stream */
+	public static function remaining(mixed $stream): ?int {
+		if (!is_resource($stream)) {
+			return null;
+		}
+		if (!(stream_get_meta_data($stream)['seekable'] ?? false)) {
+			return null;
+		}
+		$stat = fstat($stream);
+		$pos = ftell($stream);
+		if ($stat === false || $pos === false) {
+			return null;
+		}
+		return max(0, $stat['size'] - $pos);
+	}
+
 	private function mkdirs(string $dir): void {
 		if ($dir === '' || $dir === '.' || $this->storage->is_dir($dir)) {
 			return;

lib/BackgroundJob/BackupJob.php

@@ -38,13 +38,19 @@ final class BackupJob extends TimedJob {
 		$this->setTimeSensitivity(IJob::TIME_SENSITIVE);
 	}
 
+	private const MAX_DAILY_ATTEMPTS = 3;
+
 	protected function run($argument): void {
 		$budget = PHP_SAPI === 'cli' ? 240.0 : 20.0;
 		$deadline = microtime(true) + $budget;
 		$this->startScheduled();
 		foreach ($this->runs->findRunning(null, BackupService::KIND_FULL) as $run) {
 			while (microtime(true) < $deadline && $run->getStatus() === 'running') {
+				$before = $run->getUpdatedAt();
 				$run = $this->backups->step($run, min($deadline, microtime(true) + $budget));
+				if ($run->getUpdatedAt() === $before && $run->getStatus() === 'running') {
+					break; // another process holds this run's lease: leave it to that process
+				}
 			}
 		}
 		$this->failStaleRestores();
@@ -67,10 +73,15 @@ final class BackupJob extends TimedJob {
 		}
 	}
 
-	/** Once a week (after Sunday's backup), apply the retention policy to non-append-only locations. */
+	/**
+	 * Once a week, apply the retention policy to non-append-only locations: from Sunday on, at the
+	 * first moment no backup is running, so a long Sunday backup only delays it instead of skipping
+	 * the week.
+	 */
 	private function weeklyPrune(): void {
 		$lastWeek = $this->appConfig->getValueString(Application::APP_ID, 'last_prune_week', '');
-		if (date('N') !== '7' || $lastWeek === date('o-W') || $this->runs->findRunning(null, BackupService::KIND_FULL) !== []) {
+		$due = date('N') === '7' ? date('o-W') : date('o-W', (int)strtotime('last sunday'));
+		if (strcmp($lastWeek, $due) >= 0 || $this->runs->findRunning(null, BackupService::KIND_FULL) !== []) {
 			return;
 		}
 		foreach ($this->targets->list() as $t) {
@@ -83,7 +94,7 @@ final class BackupJob extends TimedJob {
 				$this->logger->warning('NG Backup: cleanup of ' . $t->getName() . ' failed: ' . $e->getMessage());
 			}
 		}
-		$this->appConfig->setValueString(Application::APP_ID, 'last_prune_week', date('o-W'));
+		$this->appConfig->setValueString(Application::APP_ID, 'last_prune_week', $due);
 	}
 
 	/** Daily schedule: appconfig ng_backup/schedule_time "HH:MM" (server time); empty = no schedule. */
@@ -92,7 +103,7 @@ final class BackupJob extends TimedJob {
 		if (!preg_match('/^([01]\d|2[0-3]):[0-5]\d$/', $at)) {
 			return;
 		}
-		$todayAt = strtotime(date('Y-m-d') . ' ' . $at);
+		$todayAt = (int)strtotime(date('Y-m-d') . ' ' . $at);
 		if (time() < $todayAt) {
 			return;
 		}
@@ -101,6 +112,12 @@ final class BackupJob extends TimedJob {
 			if ($this->runs->findRunning($t->getId(), BackupService::KIND_FULL) !== [] || ($last !== null && $last->getStartedAt() >= $todayAt)) {
 				continue;
 			}
+			// After a failure: retry at most MAX_DAILY_ATTEMPTS times a day, an hour apart, instead
+			// of starting (and notifying about) a new full run every five minutes all day.
+			$today = $this->runs->startedSince($t->getId(), BackupService::KIND_FULL, $todayAt);
+			if (count($today) >= self::MAX_DAILY_ATTEMPTS || ($today !== [] && time() - (int)$today[0]->getFinishedAt() < 3600)) {
+				continue;
+			}
 			try {
 				$this->backups->start($t, 'scheduled');
 			} catch (\Throwable $e) {

lib/Command/RunBackup.php

@@ -75,6 +75,13 @@ HELP);
 			});
 			if ($run->getStatus() === Run::DONE) {
 				$output->writeln(sprintf('  done in %.0fs', microtime(true) - $start));
+				$f = json_decode($run->getStats() ?? '{}', true)['files'] ?? [];
+				if (($f['unreadable'] ?? 0) > 0) {
+					$output->writeln(sprintf('<comment>  warning: %d unreadable files or folders are missing from this backup (see the server log)</comment>', $f['unreadable']));
+				}
+				if (($f['changing'] ?? 0) > 0) {
+					$output->writeln(sprintf('<comment>  warning: %d files kept changing and were saved as read (see the server log)</comment>', $f['changing']));
+				}
 			} else {
 				$output->writeln('<error>  ' . $run->getStatus() . ': ' . $run->getError() . '</error>');
 				$failed++;

lib/Db/RunMapper.php

@@ -43,6 +43,17 @@ final class RunMapper extends QBMapper {
 		return $this->findEntities($qb);
 	}
 
+	/** @return list<Run> runs of $kind started at or after $since, newest first */
+	public function startedSince(int $targetId, string $kind, int $since): array {
+		$qb = $this->db->getQueryBuilder();
+		$qb->select('*')->from($this->getTableName())
+			->where($qb->expr()->eq('target_id', $qb->createNamedParameter($targetId, IQueryBuilder::PARAM_INT)))
+			->andWhere($qb->expr()->eq('kind', $qb->createNamedParameter($kind)))
+			->andWhere($qb->expr()->gte('started_at', $qb->createNamedParameter($since, IQueryBuilder::PARAM_INT)))
+			->orderBy('id', 'DESC');
+		return $this->findEntities($qb);
+	}
+
 	public function lastFinished(int $targetId, string $kind): ?Run {
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('*')->from($this->getTableName())

lib/Job/BackupRun.php

@@ -49,9 +49,11 @@ final class BackupRun {
 	public static function step(Repository $repo, array $state, float $deadline): array {
 		$state['steps']++;
 		if ($state['phase'] === 'files') {
-			$walker = new TreeWalker($state['roots'], $state['exclude']);
-			$parent = new ParentCursor($repo, $state['parent'], $state['parentPos']);
 			$stats = $state['stats'];
+			$walker = new TreeWalker($state['roots'], $state['exclude'], function (string $logical) use (&$state, &$stats): void {
+				self::recordUnreadable($state, $stats, $logical);
+			});
+			$parent = new ParentCursor($repo, $state['parent'], $state['parentPos']);
 			$lines = [];
 			$finishedWalk = true;
 
@@ -68,15 +70,20 @@ final class BackupRun {
 					$stats['reused']++;
 				} else {
 					$cur = $state['cur'];
-					if ($cur === null || $cur['path'] !== $logical || $cur['version'] !== $version) {
-						if ($cur !== null && $cur['path'] === $logical) {
-							$stats['retried'] = ($stats['retried'] ?? 0) + 1; // changed since the previous step
+					if ($cur !== null && $cur['path'] === $logical && $cur['version'] !== $version) {
+						// Changed since the previous step: start over, unless it keeps changing.
+						if (self::countRetry($state, $stats, $logical)) {
+							$cur['version'] = $version;
+						} else {
+							$cur = null;
 						}
+					}
+					if ($cur === null || $cur['path'] !== $logical) {
 						$cur = ['path' => $logical, 'offset' => 0, 'blobs' => [], 'version' => $version];
 					}
-					$fh = fopen($path, 'rb');
+					$fh = @fopen($path, 'rb');
 					if ($fh === false) {
-						$stats['unreadable'] = ($stats['unreadable'] ?? 0) + 1;
+						self::recordUnreadable($state, $stats, $logical);
 						$state['cur'] = null;
 						continue;
 					}
@@ -91,12 +98,17 @@ final class BackupRun {
 						}
 					}
 					fclose($fh);
-					if (Repository::fileVersion($path) !== $cur['version']) {
-						// Changed while reading: read it again from the start (now or next step).
-						$stats['retried'] = ($stats['retried'] ?? 0) + 1;
-						$state['cur'] = null;
-						$finishedWalk = false;
-						break;
+					$now = Repository::fileVersion($path);
+					if ($now !== $cur['version']) {
+						if (!self::countRetry($state, $stats, $logical)) {
+							// Changed while reading: read it again from the start (now or next step).
+							$state['cur'] = null;
+							$finishedWalk = false;
+							break;
+						}
+						// Keeps changing (a live database, VM image or log): keep what was read,
+						// like any copy of a file in use, and report it instead of retrying forever.
+						$cur['version'] = $now;
 					}
 					if (!$complete) {
 						$state['cur'] = $cur;
@@ -104,6 +116,7 @@ final class BackupRun {
 						break;
 					}
 					$blobs = $cur['blobs'];
+					$size = $cur['offset'];
 					$state['cur'] = null;
 					$stats['read']++;
 				}
@@ -135,7 +148,8 @@ final class BackupRun {
 			$segments = $state['segments'];
 			$repo->putSnapshot($state['snapshot'],
 				['time' => $state['started'], 'parent' => $state['parent'], 'label' => $state['label'],
-					'roots' => array_keys($state['roots']), 'stats' => $state['stats']] + $state['meta'],
+					'roots' => array_keys($state['roots']), 'stats' => $state['stats'],
+					'warnings' => self::warnings($state)] + $state['meta'],
 				(function () use ($repo, $segments) {
 					foreach ($segments as $blobs) {
 						yield from $repo->readLines($blobs);
@@ -146,6 +160,52 @@ final class BackupRun {
 		return $state;
 	}
 
+	/**
+	 * A file or directory that could not be read: it is missing from this snapshot. Remembered (first
+	 * 1000, deduplicated across steps) so the run can report it instead of silently succeeding.
+	 */
+	private static function recordUnreadable(array &$state, array &$stats, string $logical): void {
+		if (isset($state['unreadable'][$logical])) {
+			return;
+		}
+		if (count($state['unreadable'] ?? []) < 1000) {
+			$state['unreadable'][$logical] = true;
+		}
+		$stats['unreadable'] = ($stats['unreadable'] ?? 0) + 1;
+	}
+
+	/** @return array{unreadable: list<string>, changing: list<string>} examples for the report (first 50 each) */
+	public static function warnings(array $state): array {
+		return [
+			'unreadable' => array_slice(array_map('strval', array_keys($state['unreadable'] ?? [])), 0, 50),
+			'changing' => array_slice(array_map('strval', array_values($state['changing'] ?? [])), 0, 50),
+		];
+	}
+
+	/** Retries of one file that changes while it is read, before its last read is kept anyway. */
+	public const MAX_RETRIES = 3;
+
+	/**
+	 * Count a retry of $logical. Returns true once it has changed MAX_RETRIES times: the caller then
+	 * keeps the data it read, and the file is listed in $state['changing'] (first 50) for the report.
+	 */
+	private static function countRetry(array &$state, array &$stats, string $logical): bool {
+		$stats['retried'] = ($stats['retried'] ?? 0) + 1;
+		$n = (($state['retryPath'] ?? null) === $logical ? (int)($state['retryCount'] ?? 0) : 0) + 1;
+		$state['retryPath'] = $logical;
+		$state['retryCount'] = $n;
+		if ($n < self::MAX_RETRIES) {
+			return false;
+		}
+		if ($n === self::MAX_RETRIES) {
+			$stats['changing'] = ($stats['changing'] ?? 0) + 1;
+			if (count($state['changing'] ?? []) < 50) {
+				$state['changing'][] = $logical;
+			}
+		}
+		return true;
+	}
+
 	private static function readFull($fh, int $length): string {
 		$buffer = '';
 		while (strlen($buffer) < $length && !feof($fh)) {

lib/Job/TreeWalker.php

@@ -23,6 +23,8 @@ final class TreeWalker {
 	public function __construct(
 		private array $roots,
 		private array $exclude = [],
+		/** Called with the logical path of a directory that cannot be listed. */
+		private ?\Closure $onUnreadable = null,
 	) {
 	}
 
@@ -57,6 +59,9 @@ final class TreeWalker {
 		}
 		$names = @scandir($abs);
 		if ($names === false) {
+			if ($this->onUnreadable !== null) {
+				($this->onUnreadable)($logical === '' ? '/' : $logical);
+			}
 			return;
 		}
 		$names = array_values(array_diff($names, ['.', '..']));

lib/Notification/Notifier.php

@@ -42,6 +42,10 @@ final class Notifier implements INotifier {
 					: $l->t('Backup to %s failed', [$p['target']]));
 				$notification->setParsedMessage((string)($p['error'] ?? ''));
 				break;
+			case 'run_warnings':
+				$notification->setParsedSubject($l->t('Backup to %1$s finished with warnings: %2$s unreadable, %3$s changed while being read', [$p['target'], $p['unreadable'], $p['changing']]));
+				$notification->setParsedMessage($l->t('Unreadable files and folders are missing from this backup (check their permissions). Files that kept changing were saved as they were while being read. Examples: %s', [$p['examples'] ?? '']));
+				break;
 			case 'quota_warning':
 				$notification->setParsedSubject($l->t('Backup location %1$s is at %2$s%% of its limit (%3$s of %4$s)', [$p['target'], $p['percent'], $p['used'], $p['limit']]));
 				$notification->setParsedMessage($l->t('Backups stop when the limit is reached. Raise the limit, or let the retention policy remove older backups.'));
@@ -62,6 +66,8 @@ final class Notifier implements INotifier {
 					'catalog_trusted' => $l->t('%1$s accepted an older state of backup location %2$s (catalog generation %3$s)', [$who, $p['target'] ?? '', $p['generation'] ?? '']),
 					'slot_added' => $l->t('%1$s added backup passphrase slot %2$s (%3$s); download and confirm the new recovery kit', [$who, $p['slot'] ?? '', $p['label'] ?? '']),
 					'slot_replaced' => $l->t('%1$s set a new passphrase for backup key slot %2$s (%3$s); download and confirm the new recovery kit', [$who, $p['slot'] ?? '', $p['label'] ?? '']),
+					'user_export_retention_changed' => $l->t('%1$s changed how many exports per user are kept to %2$s', [$who, $p['keep'] ?? '']),
+					'user_exports_forgotten' => $l->t('%1$s removed %2$s per-user exports from %3$s', [$who, $p['count'] ?? '', $p['target'] ?? '']),
 					'slot_removed' => $l->t('%1$s removed backup passphrase slot %2$s; download and confirm the new recovery kit', [$who, $p['slot'] ?? '']),
 					default => $l->t('NG Backup security event'),
 				});

lib/Repository/Catalog.php

@@ -88,6 +88,11 @@ final class Catalog {
 	/** Write the next generation after the in-memory state was changed. */
 	public function write(): void {
 		$next = $this->gen + 1;
+		// Two writers starting from the same generation would otherwise overwrite each other
+		// silently and break the chain for whichever server anchored the lost write.
+		if ($this->backend->exists($this->path($next))) {
+			throw new RepositoryException("Catalog generation $next was written by another process at the same time; try again");
+		}
 		$plain = json_encode(['repository' => $this->repositoryId, 'gen' => $next, 'time' => gmdate('c'), 'prev' => $this->hash,
 			'snapshots' => array_keys($this->snapshots), 'trash' => array_keys($this->trash),
 			'userExports' => array_keys($this->userExports)], JSON_THROW_ON_ERROR);

lib/Repository/Repository.php

@@ -236,8 +236,13 @@ final class Repository {
 				unlink($tmp);
 				throw new RepositoryException('Size mismatch restoring ' . $entry['p']);
 			}
-			rename($tmp, $dest);
-			touch($dest, $entry['m']);
+			if (!rename($tmp, $dest)) {
+				@unlink($tmp);
+				throw new RepositoryException('Cannot put the restored file in place: ' . $entry['p']);
+			}
+			if (!touch($dest, $entry['m'])) {
+				throw new RepositoryException('Cannot set the modification time of ' . $entry['p']);
+			}
 			$count++;
 		}
 		return $count;
@@ -375,28 +380,39 @@ final class Repository {
 			// must be decrypted and re-encrypted under the new path, not moved as raw bytes.
 			$dbInTrash = 'trash/' . $meta['db'];
 			$this->putObject($dbInTrash, $this->getObject($meta['db']));
-			$this->backend->delete($meta['db']);
 		}
-		// The snapshot object is bound to its id (not its path), so it can be moved as raw bytes.
-		$this->backend->move('snapshots/' . $snapshotId, 'trash/snapshots/' . $snapshotId);
+		// Copy first, switch the catalog, delete the originals last: a crash at any point leaves
+		// either the old state or the new one plus unreferenced leftovers, never a catalog entry
+		// pointing at a missing object. The snapshot object is bound to its id (not its path),
+		// so it can be copied as raw bytes.
+		$this->backend->put('trash/snapshots/' . $snapshotId, $this->backend->get('snapshots/' . $snapshotId));
 		$this->putObject('trash/info/' . $snapshotId, json_encode(['forgottenAt' => time(), 'by' => $by, 'db' => $meta['db'] ?? null,
 			'dbInTrash' => $dbInTrash, 'time' => $meta['time'] ?? null, 'label' => $meta['label'] ?? ''], JSON_THROW_ON_ERROR));
 		unset($this->catalog->snapshots[$snapshotId]);
 		$this->catalog->trash[$snapshotId] = true;
 		$this->catalog->write();
+		if ($dbInTrash !== null) {
+			$this->backend->delete($meta['db']);
+		}
+		$this->backend->delete('snapshots/' . $snapshotId);
 	}
 
 	/** Undo forget(): move a snapshot back from the trash. */
 	public function untrash(string $snapshotId): void {
 		$info = $this->trashInfo($snapshotId);
-		if ($info['dbInTrash'] !== null && $this->backend->exists($info['dbInTrash'])) {
+		$restoreDb = $info['dbInTrash'] !== null && $this->backend->exists($info['dbInTrash']);
+		if ($restoreDb) {
 			$this->putObject($info['db'], $this->getObject($info['dbInTrash']));
-			$this->backend->delete($info['dbInTrash']);
 		}
-		$this->backend->move('trash/snapshots/' . $snapshotId, 'snapshots/' . $snapshotId);
+		// Same order as forget(): copy, switch the catalog, then delete the trash copies.
+		$this->backend->put('snapshots/' . $snapshotId, $this->backend->get('trash/snapshots/' . $snapshotId));
 		$this->catalog->snapshots[$snapshotId] = true;
 		unset($this->catalog->trash[$snapshotId]);
 		$this->catalog->write();
+		if ($restoreDb) {
+			$this->backend->delete($info['dbInTrash']);
+		}
+		$this->backend->delete('trash/snapshots/' . $snapshotId);
 		$this->backend->delete('trash/info/' . $snapshotId);
 	}
 
@@ -530,7 +546,10 @@ final class Repository {
 				}
 			}
 		}
-		foreach ($this->backend->list('users') as $path) {
+		// listUserExports() throws when a recorded export's manifest is missing: then stop, since
+		// freeing its blobs would turn a tampering or storage alarm into irreversible deletion.
+		$exports = array_unique(array_merge($this->listUserExports(), $this->backend->list('users')));
+		foreach ($exports as $path) {
 			foreach (json_decode($this->getObject($path), true, 512, JSON_THROW_ON_ERROR)['entries'] as $e) {
 				foreach ($e['b'] ?? [] as $b) {
 					$used[$b] = true;

lib/Service/AlertService.php

@@ -54,6 +54,13 @@ final class AlertService {
 			['kind' => $run->getKind(), 'target' => $targetName, 'error' => mb_strimwidth((string)$run->getError(), 0, 300, '…')]);
 	}
 
+	/** A backup that finished but is missing unreadable files, or holds files that changed while read. */
+	public function runWarnings(Run $run, string $targetName, int $unreadable, int $changing, array $examples): void {
+		$this->notifyAdmins('run_warnings', 'run-' . $run->getId(), ['target' => $targetName,
+			'unreadable' => (string)$unreadable, 'changing' => (string)$changing,
+			'examples' => mb_strimwidth(implode(', ', array_merge($examples['unreadable'], $examples['changing'])), 0, 300, '…')]);
+	}
+
 	/**
 	 * Remember how much is stored on a location (for the settings page) and warn administrators
 	 * once a day when it is at 80% of its limit or more.

lib/Service/BackupService.php

@@ -14,6 +14,7 @@ use OCA\NgBackup\Db\Snapshot;
 use OCA\NgBackup\Db\SnapshotMapper;
 use OCA\NgBackup\Db\Target;
 use OCA\NgBackup\Job\BackupRun;
+use OCA\NgBackup\Repository\RepositoryException;
 use OCP\App\IAppManager;
 use OCP\IConfig;
 use OCP\IDBConnection;
@@ -151,16 +152,25 @@ final class BackupService {
 					// Catalog write under the catalog lock, from a repository opened inside it: a per-user
 					// export or restore may have written the catalog since this step opened $repo.
 					try {
-						$this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 30,
+						$this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 300,
 							fn () => $this->targets->repository($target)->recordSnapshot((string)$state['files']['snapshot']));
 						$state['files']['phase'] = 'done';
-					} catch (LockedException) {
-						// Another catalog write is in progress: the next step retries this commit.
+					} catch (LockedException|RepositoryException) {
+						// Another catalog write is in progress (or just raced us to the next
+						// generation): the next step retries this commit on the fresh catalog.
 					}
 				}
 				if ($state['files']['phase'] === 'done') {
 					$this->recordSnapshot($run, $state['files']);
 					$this->alerts->storedBytes($target, $repo->storedBytes());
+					$stats = $state['files']['stats'];
+					if (($stats['unreadable'] ?? 0) > 0 || ($stats['changing'] ?? 0) > 0) {
+						$warnings = BackupRun::warnings($state['files']);
+						$this->logger->warning(sprintf('NG Backup run %d to %s: %d unreadable (missing from the snapshot): %s; %d changed while being read: %s',
+							$run->getId(), $target->getName(), $stats['unreadable'] ?? 0, implode(', ', $warnings['unreadable']),
+							$stats['changing'] ?? 0, implode(', ', $warnings['changing'])));
+						$this->alerts->runWarnings($run, $target->getName(), (int)($stats['unreadable'] ?? 0), (int)($stats['changing'] ?? 0), $warnings);
+					}
 					$run->setStatus(Run::DONE);
 					$run->setPhase('done');
 					$run->setFinishedAt(time());

lib/Service/UserBackupService.php

@@ -58,7 +58,7 @@ final class UserBackupService {
 
 	private function withCatalogLock(Target $target, callable $fn): mixed {
 		try {
-			return $this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 30, $fn);
+			return $this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 300, $fn);
 		} catch (LockedException) {
 			throw new \RuntimeException('Another catalog write for ' . $target->getName() . ' is in progress; try again');
 		}

lib/Service/UserRestoreService.php

@@ -129,7 +129,7 @@ final class UserRestoreService {
 
 	private function withCatalogLock(Target $target, callable $fn): mixed {
 		try {
-			return $this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 30, $fn);
+			return $this->leases->with(PruneService::catalogLockKey($target), LeaseService::EXCLUSIVE, 300, $fn);
 		} catch (LockedException) {
 			throw new \RuntimeException('Another catalog write for ' . $target->getName() . ' is in progress; try again');
 		}

psalm-baseline.xml

@@ -224,14 +224,9 @@
     <MissingParamType>
       <code><![CDATA[$fh]]></code>
     </MissingParamType>
-    <PossiblyNullIterator>
-      <code><![CDATA[$segments]]></code>
-    </PossiblyNullIterator>
     <PossiblyUndefinedArrayOffset>
       <code><![CDATA[$mtime]]></code>
-      <code><![CDATA[$state['cur']]]></code>
       <code><![CDATA[$state['phase']]]></code>
-      <code><![CDATA[$state['segments']]]></code>
       <code><![CDATA[$state['stats']]]></code>
     </PossiblyUndefinedArrayOffset>
   </file>

tests/Unit/Repository/CrashSafetyTest.php

@@ -0,0 +1,106 @@
+<?php
+
+declare(strict_types=1);
+
+// SPDX-FileCopyrightText: 2026 Dillard Blom
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+namespace OCA\NgBackup\Tests\Unit\Repository;
+
+use OCA\NgBackup\Backend\LocalBackend;
+use OCA\NgBackup\Crypto\KeyRing;
+use OCA\NgBackup\Job\BackupRun;
+use OCA\NgBackup\Repository\Repository;
+use OCA\NgBackup\Repository\RepositoryException;
+use OCA\NgBackup\Tests\Unit\TempDirTrait;
+use PHPUnit\Framework\TestCase;
+
+/** Data-safety cases found in the pre-beta review. */
+class CrashSafetyTest extends TestCase {
+	use TempDirTrait;
+
+	public function testListingKeepsNamesThatOnlyLookLikeTemporaryFiles(): void {
+		$root = $this->tempDir();
+		$backend = new LocalBackend($root);
+		mkdir("$root/users/john.part-time", 0700, true);
+		file_put_contents("$root/users/john.part-time/20261006T120000Z-aaaa0001", 'x');
+		file_put_contents("$root/users/upload.part-deadbeef", 'partial');
+		$this->assertSame(['users/john.part-time/20261006T120000Z-aaaa0001'], $backend->list('users'));
+	}
+
+	public function testPruneStopsWhenARecordedExportManifestIsMissing(): void {
+		$root = $this->tempDir();
+		$backend = new LocalBackend($root);
+		$keys = KeyRing::generate();
+		$repo = Repository::initWithKey($backend, $keys, $keys->wrap('pw'));
+		$stats = [];
+		$blob = $repo->storeData(random_bytes(1000), $stats);
+		$repo->flushPacks();
+		$path = 'users/alice/20261006T120000Z-aaaa0001';
+		$repo->putObject($path, json_encode(['uid' => 'alice', 'entries' => ['f.txt' => ['t' => 'f', 'b' => [$blob], 's' => 1000]]]));
+		$repo->recordUserExport($path);
+		unlink("$root/$path");
+
+		$packsBefore = $backend->list('packs');
+		try {
+			Repository::openWithKey($backend, $keys)->prune();
+			$this->fail('prune must refuse while a recorded export is missing');
+		} catch (RepositoryException $e) {
+			$this->assertStringContainsString('missing on the location', $e->getMessage());
+		}
+		$this->assertSame($packsBefore, $backend->list('packs'), 'no pack was deleted');
+	}
+
+	public function testForgetAndUntrashKeepTheSnapshotRestorable(): void {
+		$src = $this->tempDir();
+		file_put_contents("$src/a.txt", 'hello');
+		$root = $this->tempDir();
+		$backend = new LocalBackend($root);
+		$keys = KeyRing::generate();
+		Repository::initWithKey($backend, $keys, $keys->wrap('pw'));
+		$state = BackupRun::start($src);
+		while ($state['phase'] !== 'commit') {
+			$state = BackupRun::step(Repository::openWithKey($backend, $keys), $state, microtime(true) + 5);
+		}
+		Repository::openWithKey($backend, $keys)->recordSnapshot($state['snapshot']);
+		$id = $state['snapshot'];
+
+		Repository::openWithKey($backend, $keys)->forget($id, 'test');
+		$repo = Repository::openWithKey($backend, $keys);
+		$this->assertSame([], $repo->listSnapshots());
+		$this->assertArrayHasKey($id, $repo->trash());
+		$this->assertFalse($backend->exists('snapshots/' . $id), 'original removed after the catalog switch');
+
+		$repo->untrash($id);
+		$repo = Repository::openWithKey($backend, $keys);
+		$this->assertSame([$id], $repo->listSnapshots());
+		$this->assertSame([], $repo->trash());
+		$target = $this->tempDir();
+		$repo->restore($id, $target);
+		$this->assertSame('hello', file_get_contents("$target/a.txt"));
+	}
+
+	public function testFileThatKeepsChangingDoesNotStallTheRun(): void {
+		$src = $this->tempDir();
+		file_put_contents("$src/live.db", random_bytes(3 * Repository::BLOB_SIZE));
+		$root = $this->tempDir();
+		$backend = new LocalBackend($root);
+		$keys = KeyRing::generate();
+		Repository::initWithKey($backend, $keys, $keys->wrap('pw'));
+
+		$state = BackupRun::start($src);
+		$n = 0;
+		for ($i = 0; $i < 50 && $state['phase'] !== 'commit'; $i++) {
+			$state = BackupRun::step(Repository::openWithKey($backend, $keys), $state, microtime(true) + 0.001);
+			// Rewrite the file after every step: it never stays the same between two steps.
+			$fh = fopen("$src/live.db", 'r+b');
+			fwrite($fh, random_bytes(16));
+			fclose($fh);
+			touch("$src/live.db", time() + ++$n);
+			clearstatcache();
+		}
+		$this->assertSame('commit', $state['phase'], 'the run finishes instead of retrying forever');
+		$this->assertSame(1, $state['stats']['changing'] ?? 0);
+		$this->assertSame(['live.db'], BackupRun::warnings($state)['changing']);
+	}
+}