Merge pull request #2: Pre-beta review

Pre-beta review: data safety, safer commands, known limitations

PLAN.md

@@ -359,6 +359,24 @@ minimale volledige restore (`occ backup:restore:full` op een verse installatie)
     Backlog: (a) de originele weergavenaam herstellen (het label uit de export is niet getest als
     weergavenaam); (b) de interne `migrated-`-naam hernoemen (vereist directe databasewijziging, niet voor de beta).
 
+**Backlog uit de pre-beta review (2026-10-07)** — geen belemmering voor 0.9.0-beta of 1.0.0, staat
+in README "Known limitations":
+1. Geheugen bij grote instanties (>1 TB, miljoenen bestanden): blob-index, prune-set, browse van een
+   snapshot in de web-UI, `trashExtras` bij restore en het manifest van een user-export staan volledig
+   in het geheugen; streaming/paginering nodig.
+2. Twee servers op één locatie: catalogus kan breken (generatie overschreven, of anchor >200 generaties
+   achter). Docs-waarschuwing staat er; mogelijk later een echte put-if-absent per backend.
+3. Sleutelrotatie: een slot verwijderen trekt geen toegang in (master-key blijft gelijk).
+4. Rollback vóór DR niet detecteerbaar: referentie-generatie en -hash in de kit opnemen of tonen.
+5. Kit-velden buiten `wrapped_key` (o.a. `delete_delay_days`) zijn niet geauthenticeerd; nu alleen getoond.
+6. Repository-id in de AD van objecten (formaatwijziging).
+7. Wachtwoorden van locaties via `--option-file` of omgevingsvariabele i.p.v. de commandoregel.
+8. Tijdelijke restore-bestanden niet via de Files-API verwijderen (komen nu in de prullenbak).
+9. DB-restore in foreign-key-volgorde (cascades).
+10. Packs zonder index (index-upload mislukt) opruimen.
+11. `backup:restore:full` in onderhoudsmodus en hervatbaar maken; nu: opnieuw draaien met `--force`.
+12. Config/rename op FTP/SMB-servers die geen rename over een bestaand bestand toestaan.
+
 Doorgeschoven naar **v1.3.0** (fase 3, niet meer fase 2): in-place volledige restore
 (eigen restore-modus i.p.v. onderhoudsmodus) en zelfbediening voor gebruikers (eigen bestanden
 terugzetten vanuit de Files-app).

README.md

@@ -7,8 +7,9 @@ installations and hosted Nextcloud. (Choosing SMB as a storage backend is the on
 see "Storage backend experiences" below — that requirement comes from files_external, not from
 ng_backup, and applies to any Nextcloud app that writes to an SMB share.)
 
-**Status: pre-release. Not yet tested on production.** See `PLAN.md` for the design and `docs/HOWTO.md` for a
-step-by-step guide from setup to a tested restore.
+**Status: beta (0.9.0).** Tested thoroughly, usable alongside your current backup, but not yet a
+drop-in replacement for it. See "Known limitations" below, `PLAN.md` for the design and
+`docs/HOWTO.md` for a step-by-step guide from setup to a tested restore.
 
 License: AGPL-3.0-or-later.
 
@@ -147,6 +148,29 @@ Contributions welcome via PR — add what you ran into with your own provider.
   mid-run behaves like any other local-path failure (the run resumes). See PLAN.md 3.5 for why NFS
   can't be driven from PHP directly.
 
+## Known limitations
+
+On the backlog; none of these block the beta or 1.0.0, but know them before you rely on it:
+
+- **Very large instances** (well over 1 TB, millions of files): the blob index, cleanup, browsing a
+  snapshot in the web interface and per-user export manifests are held in memory. Expect high
+  memory use there; reports from such instances are very welcome.
+- **One location, one server.** Never let two Nextcloud servers write to the same location (for
+  example a disaster recovery test server pointed at the production location): they can break each
+  other's catalog, and the location is then refused as possibly tampered with. Test a disaster
+  recovery against a copy of the location, or only read from it.
+- **Removing a passphrase slot does not revoke access.** The master key stays the same, so someone
+  with an old recovery kit and that slot's passphrase can still decrypt the location if they can
+  read it. Key rotation is not available yet.
+- **Rollback before a disaster recovery is not detected.** A fresh server has no record of the
+  newest catalog generation, so a location rolled back to an older state looks valid there.
+- Location passwords passed with `-o password=...` are visible in the process list and shell
+  history while the command runs.
+- Temporary files of a restore into a user's files end up in that user's trash.
+- Restoring the database does not order tables by foreign keys; apps with cascading foreign keys
+  could lose rows (Nextcloud core does not use them).
+- A pack whose index upload failed stays on the location until removed by hand.
+
 ## Commands
 
 | Command | Purpose |

docs/HOWTO.md

@@ -14,7 +14,8 @@ occ backup:key:confirm -u <your-admin-uid> --code=<code shown> --confirm="Yes, I
 
 - The passphrase protects the backup. Keep it somewhere other than this server.
 - Optional: extra passphrases for other people with `occ backup:key:slot:add --label="..."`.
-  Any one passphrase is enough to restore.
+  Any one passphrase is enough to restore. Removing a slot later does not revoke access for someone
+  who kept an old kit and that passphrase (see "Known limitations" in the README).
 - The recovery kit (`/var/tmp/ng_backup-kit.json` above) is also required for disaster recovery. The web
   user writes it there; then move it off the server as root, next to the passphrase, and delete the
   copy in `/var/tmp`. Without the kit and a passphrase, the backup cannot be read.
@@ -102,7 +103,9 @@ occ backup:user:restore offsite users/<uid>/<exportId> --mode=as-backup
   use, add `--install-user-migration` to approve that explicitly. The command stops after the
   installation; run it once more.
 - `replace` first makes a safety export of the current account, then recreates the user under the
-  same id. `as-backup` restores into `<uid>-bak` and leaves the original untouched.
+  same id. When the account exists, the command asks first; in scripts, add `--force`. Shares to
+  others and data of apps without a user_migration migrator are not restored, so prefer
+  `as-backup` when in doubt: it restores into `<uid>-bak` and leaves the original untouched.
 - A restore onto a server where the user does not exist works. Calendars and address books get an
   internal name with the prefix `migrated-` (for example `migrated-personal`). That prefix comes from
   user_migration and cannot be turned off. The user can change the display name of these calendars and
@@ -115,7 +118,8 @@ occ backup:user:restore offsite users/<uid>/<exportId> --mode=as-backup
 ## 7. Disaster recovery: restore the whole server
 
 Use a fresh Nextcloud installation of the same major version, with NG Backup installed and
-the same location available:
+the same location available. For a test, use a **copy** of the location: two servers writing to one
+location break each other's catalog.
 
 ```
 occ backup:key:import /path/ng_backup-kit.json --passphrase-file=/path/passphrase
@@ -124,6 +128,9 @@ occ backup:list offsite                              # the snapshots must be lis
 occ backup:restore:full offsite <snapshot>
 ```
 
+The command refuses on an installation that already has more than one user, and asks before it
+starts; `--force` skips both (also needed to rerun it after it stopped halfway).
+
 Afterwards, check that users, files and the database are complete, and run a backup of the restored
 server. The restored installation keeps its own app configuration and secret.
 

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,39 @@ 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 local $stream (php://temp, php://memory or a plain file), or null:
+	 * stream wrappers of remote storages do not report a reliable size.
+	 */
+	public static function remaining(mixed $stream): ?int {
+		if (!is_resource($stream)) {
+			return null;
+		}
+		$meta = stream_get_meta_data($stream);
+		if (!($meta['seekable'] ?? false) || !in_array($meta['wrapper_type'] ?? '', ['PHP', 'plainfile'], true)) {
+			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') {
-				$run = $this->backups->step($run, min($deadline, microtime(true) + $budget));
+				$next = $this->backups->step($run, min($deadline, microtime(true) + $budget));
+				if ($next === $run) {
+					break; // step() hands back the same object when another process holds the run's lease
+				}
+				$run = $next;
 			}
 		}
 		$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/KeyImport.php

@@ -76,6 +76,9 @@ HELP);
 			return 1;
 		}
 		$output->writeln('Backup key imported, fingerprint <info>' . $this->keys->fingerprint() . '</info>.');
+		// Read from the kit file, which is not authenticated beyond the wrapped key: show it so a
+		// wrong or altered value stands out.
+		$output->writeln('Deletion delay for forgotten snapshots: <info>' . $this->keys->deleteDelayDays() . '</info> days (from the kit).');
 		$output->writeln('Next: connect the backup location with the same details as before (<info>occ backup:target:add</info>), then <info>occ backup:list</info> and <info>occ backup:restore:full</info>.');
 		return 0;
 	}

lib/Command/KeyKit.php

@@ -39,11 +39,24 @@ final class KeyKit extends Command {
 			$output->write($json, false, OutputInterface::OUTPUT_RAW);
 			return 0;
 		}
-		if (file_put_contents($file, $json) === false) {
+		// Created owner-only from the start (no window where it is world-readable), never over an
+		// existing file or symlink someone else may have placed at a predictable path.
+		$old = umask(0077);
+		try {
+			$fh = @fopen($file, 'xb');
+		} finally {
+			umask($old);
+		}
+		if ($fh === false) {
+			$output->writeln("<error>Cannot create $file (it must not exist yet)</error>");
+			return 1;
+		}
+		$ok = fwrite($fh, $json) === strlen($json);
+		if (!fclose($fh) || !$ok) {
+			@unlink($file);
 			$output->writeln("<error>Cannot write $file</error>");
 			return 1;
 		}
-		@chmod($file, 0600);
 		$output->writeln("Recovery kit written to <info>$file</info>. Copy it off this server, then run <info>occ backup:key:confirm</info>.");
 		return 0;
 	}

lib/Command/RestoreFull.php

@@ -9,7 +9,11 @@ namespace OCA\NgBackup\Command;
 
 use OCA\NgBackup\Service\RestoreFullService;
 use OCA\NgBackup\Service\TargetService;
+use OCP\IUserManager;
 use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Helper\QuestionHelper;
+use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Question\ConfirmationQuestion;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -18,6 +22,7 @@ final class RestoreFull extends Command {
 	public function __construct(
 		private TargetService $targets,
 		private RestoreFullService $restoreFull,
+		private IUserManager $userManager,
 	) {
 		parent::__construct();
 	}
@@ -27,6 +32,7 @@ final class RestoreFull extends Command {
 			->setDescription('Disaster recovery: restore the database, data directory and config on a fresh installation')
 			->addArgument('target', InputArgument::REQUIRED, 'Location name or id')
 			->addArgument('snapshot', InputArgument::REQUIRED, 'Snapshot id (see backup:list)')
+			->addOption('force', null, InputOption::VALUE_NONE, 'Also on an installation that already has more than one user, and without asking')
 			->setHelp(<<<'HELP'
 For a FRESH Nextcloud installation only, same major version as the backup, right after
 occ maintenance:install, occ backup:key:import and occ backup:target:add -- before anyone else
@@ -50,12 +56,29 @@ HELP);
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output): int {
+		$force = (bool)$input->getOption('force');
+		// A whole-server restore replaces the database and data directory of THIS instance. On
+		// anything but a fresh install (only the admin from occ maintenance:install) that is
+		// almost certainly a mistake, so it needs --force.
+		$users = $this->userManager->countUsersTotal(2);
+		if (!$force && ($users === false || $users > 1)) {
+			$output->writeln('<error>This installation already has more than one user: backup:restore:full would replace its whole database and data directory. It is meant for a fresh installation. Use --force if this really is what you want.</error>');
+			return 1;
+		}
+		if (!$force && $input->isInteractive()) {
+			$q = new ConfirmationQuestion('This replaces the database, the data directory and config.php of this installation. Continue? [y/N] ', false);
+			if (!(new QuestionHelper())->ask($input, $output, $q)) {
+				$output->writeln('Cancelled.');
+				return 1;
+			}
+		}
 		try {
 			$target = $this->targets->get($input->getArgument('target'));
 			$snapshot = (string)$input->getArgument('snapshot');
 			$result = $this->restoreFull->restoreFull($target, $snapshot);
 		} catch (\Throwable $e) {
 			$output->writeln('<error>' . $e->getMessage() . '</error>');
+			$output->writeln('If this happened after the restore had started, the instance may be partly restored: fix the cause and run the same command again with --force.');
 			return 1;
 		}
 		$output->writeln(sprintf(
@@ -65,6 +88,9 @@ HELP);
 			$result['configKeys'],
 			$result['dataFiles'],
 		));
+		if (!$result['dbConsistent']) {
+			$output->writeln('<comment>Warning: this snapshot\'s database dump was made over several steps (web cron), so it is not one consistent point in time. Check the instance carefully; prefer a snapshot made with system cron.</comment>');
+		}
 		$output->writeln('Next: check trusted_domains and overwritehost/overwriteprotocol match this server, then occ maintenance:repair.');
 		return 0;
 	}

lib/Command/RestoreUser.php

@@ -10,7 +10,10 @@ namespace OCA\NgBackup\Command;
 use OCA\NgBackup\Service\TargetService;
 use OCA\NgBackup\Service\UserRestoreService;
 use OCA\NgBackup\UserMigration\UserMigrationInstaller;
+use OCP\IUserManager;
 use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Helper\QuestionHelper;
+use Symfony\Component\Console\Question\ConfirmationQuestion;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
@@ -21,6 +24,7 @@ final class RestoreUser extends Command {
 		private TargetService $targets,
 		private UserRestoreService $userRestore,
 		private UserMigrationInstaller $installer,
+		private IUserManager $userManager,
 	) {
 		parent::__construct();
 	}
@@ -31,6 +35,7 @@ final class RestoreUser extends Command {
 			->addArgument('target', InputArgument::REQUIRED, 'Location name or id')
 			->addArgument('manifest', InputArgument::REQUIRED, 'Manifest path printed by backup:user:backup (users/<uid>/<exportId>)')
 			->addOption('mode', 'm', InputOption::VALUE_REQUIRED, 'replace or as-backup', UserRestoreService::MODE_REPLACE)
+			->addOption('force', null, InputOption::VALUE_NONE, 'Replace an existing account without asking (required when not interactive)')
 			->addOption('install-user-migration', null, InputOption::VALUE_NONE, 'Install and enable user_migration if it is missing (non-interactive consent)')
 			->setHelp(<<<'HELP'
 Never imports over a live account. Modes:
@@ -47,11 +52,26 @@ HELP);
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output): int {
+		$mode = (string)$input->getOption('mode');
+		$uid = explode('/', (string)$input->getArgument('manifest'))[1] ?? '';
+		if ($mode === UserRestoreService::MODE_REPLACE && $uid !== '' && $this->userManager->userExists($uid) && !$input->getOption('force')) {
+			// Replace deletes the live account (after a safety export). Shares to others and data of
+			// apps without a user_migration migrator do not come back, so never do it unasked.
+			if (!$input->isInteractive()) {
+				$output->writeln("<error>$uid exists: --mode=replace deletes and recreates it. Add --force, or use --mode=as-backup.</error>");
+				return 1;
+			}
+			$q = new ConfirmationQuestion("$uid exists and will be deleted and recreated from the backup (a safety export is made first; its shares to others and data of apps without a migrator are not restored). Continue? [y/N] ", false);
+			if (!(new QuestionHelper())->ask($input, $output, $q)) {
+				$output->writeln('Cancelled.');
+				return 1;
+			}
+		}
 		try {
 			$output->writeln('<comment>Note: on restore, user_migration only imports the app settings on its own allowlist (e.g. calendar view and reminder settings); other app settings of this user are skipped. Reason: NG Backup uses user_migration\'s importer; widening the list would mean building a settings importer of our own.</comment>');
 			$target = $this->targets->get($input->getArgument('target'));
 			$this->installer->ensureEnabled($input, $output);
-			$user = $this->userRestore->restoreUser($target, (string)$input->getArgument('manifest'), (string)$input->getOption('mode'));
+			$user = $this->userRestore->restoreUser($target, (string)$input->getArgument('manifest'), $mode);
 		} catch (\Throwable $e) {
 			$output->writeln('<error>' . $e->getMessage() . '</error>');
 			return 1;

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/Controller/AdminApiController.php

@@ -168,6 +168,7 @@ final class AdminApiController extends Controller {
 		return new JSONResponse($this->targets->availableBackends());
 	}
 
+	#[PasswordConfirmationRequired]
 	public function addTarget(string $name, string $backend, string $auth, array $options = [], string $path = 'ng_backup', ?float $maxGb = null): JSONResponse {
 		try {
 			$r = $this->targets->add($name, $backend, $auth, $options, $path);

lib/Crypto/KeyRing.php

@@ -96,6 +96,12 @@ final class KeyRing {
 	/** @param array{v:int, salt:string, ops:int, mem:int, nonce:string, key:string} $wrapped */
 	public static function unwrap(array $wrapped, #[\SensitiveParameter] string $passphrase): self {
 		$b64 = static fn (string $s): string => sodium_base642bin($s, SODIUM_BASE64_VARIANT_ORIGINAL);
+		// ops/mem come from unauthenticated JSON (kit file, repository config): bound them, so a
+		// damaged or crafted file cannot make the KDF use unbounded memory or time.
+		if ($wrapped['ops'] < SODIUM_CRYPTO_PWHASH_OPSLIMIT_INTERACTIVE || $wrapped['ops'] > SODIUM_CRYPTO_PWHASH_OPSLIMIT_SENSITIVE
+			|| $wrapped['mem'] < SODIUM_CRYPTO_PWHASH_MEMLIMIT_INTERACTIVE || $wrapped['mem'] > SODIUM_CRYPTO_PWHASH_MEMLIMIT_SENSITIVE) {
+			throw new CryptoException('Unsupported key derivation parameters (damaged or foreign key file)');
+		}
 		$kek = sodium_crypto_pwhash(SODIUM_CRYPTO_SECRETBOX_KEYBYTES, $passphrase, $b64($wrapped['salt']),
 			$wrapped['ops'], $wrapped['mem'], SODIUM_CRYPTO_PWHASH_ALG_ARGON2ID13);
 		$master = sodium_crypto_secretbox_open($b64($wrapped['key']), $b64($wrapped['nonce']), $kek);

lib/Crypto/StreamCipher.php

@@ -122,6 +122,7 @@ final class StreamCipher {
 	/** fread() may return short reads on network streams; keep reading until $length or EOF. */
 	private static function readFull($in, int $length): string {
 		$buffer = '';
+		$empty = 0;
 		while (strlen($buffer) < $length && !feof($in)) {
 			$chunk = fread($in, $length - strlen($buffer));
 			if ($chunk === false) {
@@ -132,8 +133,15 @@ final class StreamCipher {
 				if (feof($in)) {
 					break;
 				}
+				// A stalled network stream returns '' without EOF forever: back off, then give up
+				// (about a minute) instead of spinning at full CPU until the lease expires.
+				if (++$empty > 600) {
+					throw new CryptoException('Read stalled');
+				}
+				usleep(100000);
 				continue;
 			}
+			$empty = 0;
 			$buffer .= $chunk;
 		}
 		return $buffer;

lib/Db/DbDumper.php

@@ -256,7 +256,11 @@ final class DbDumper {
 					} elseif (is_string($row[$i]) && !mb_check_encoding($row[$i], 'UTF-8')) {
 						// Keep invalid UTF-8 byte-exact instead of substituting characters.
 						$row[$i] = ['$b' => base64_encode($row[$i])];
-					} elseif (is_int($row[$i]) || is_float($row[$i]) || is_bool($row[$i])) {
+					} elseif (is_float($row[$i])) {
+						// var_export uses serialize_precision (shortest exact form), not the lossy
+						// 14-digit 'precision' of a (string) cast.
+						$row[$i] = var_export($row[$i], true);
+					} elseif (is_int($row[$i]) || is_bool($row[$i])) {
 						// Normalise: drivers differ in returning numbers as int or string.
 						$row[$i] = (string)(is_bool($row[$i]) ? (int)$row[$i] : $row[$i]);
 					}

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,31 +380,61 @@ 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->copyRaw('snapshots/' . $snapshotId, 'trash/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->copyRaw('trash/snapshots/' . $snapshotId, '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);
 	}
 
+	/** Copy an object as raw bytes, buffered locally (remote read streams cannot be uploaded directly everywhere). */
+	private function copyRaw(string $from, string $to): void {
+		$in = $this->backend->get($from);
+		$buffer = fopen('php://temp/maxmemory:' . (4 * 1048576), 'w+b');
+		if ($buffer === false) {
+			fclose($in);
+			throw new RepositoryException("Cannot buffer $from");
+		}
+		try {
+			if (stream_copy_to_stream($in, $buffer) === false) {
+				throw new RepositoryException("Cannot read $from");
+			}
+		} finally {
+			fclose($in);
+		}
+		rewind($buffer);
+		$this->backend->put($to, $buffer);
+	}
+
 	/** Record a user_migration export's manifest in the catalog (rollback-protected like a snapshot). */
 	public function recordUserExport(string $manifestPath): void {
 		$this->catalog->userExports[$manifestPath] = true;
@@ -530,7 +565,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/RestoreFullService.php

@@ -64,7 +64,7 @@ final class RestoreFullService {
 		];
 	}
 
-	/** @return array{dbReport:array, dataFiles:int, configKeys:int} */
+	/** @return array{dbReport:array, dataFiles:int, configKeys:int, dbConsistent:bool} */
 	public function restoreFull(Target $target, string $snapshotId): array {
 		$check = $this->checkCompatibility($target, $snapshotId);
 		if (!$check['ok']) {
@@ -76,6 +76,7 @@ final class RestoreFullService {
 			$meta = $repo->snapshotMeta($snapshotId);
 
 			$dbReport = null;
+			$manifest = [];
 			if (isset($meta['db'])) {
 				$manifest = json_decode($repo->getObject($meta['db']), true, 512, JSON_THROW_ON_ERROR);
 				$prefix = $this->config->getSystemValueString('dbtableprefix', 'oc_');
@@ -124,7 +125,8 @@ final class RestoreFullService {
 				);
 			}
 
-			return ['dbReport' => $dbReport ?? ['skipped' => [], 'restored' => []], 'dataFiles' => $dataFiles, 'configKeys' => count($backupConfig)];
+			return ['dbReport' => $dbReport ?? ['skipped' => [], 'restored' => []], 'dataFiles' => $dataFiles, 'configKeys' => count($backupConfig),
+				'dbConsistent' => (bool)($manifest['consistent'] ?? true)];
 		});
 	}
 

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']);
+	}
+}

tests/integration/full-restore-roundtrip.sh

@@ -68,7 +68,7 @@ run_on ngb-frt-restore backup:target:add offsite local -a null::null -o datadir=
 run_on ngb-frt-restore backup:list offsite | grep -q "$SNAP" || fail "the origin's snapshot is not listed after connecting the location"
 ok "recovery kit imported, existing repository adopted, snapshot visible"
 
-run_on ngb-frt-restore backup:restore:full offsite "$SNAP" >/dev/null || fail "restore:full"
+run_on ngb-frt-restore backup:restore:full offsite "$SNAP" --no-interaction >/dev/null || fail "restore:full"
 ok "backup:restore:full completed"
 
 # Everything below must work in a FRESH occ process (a new PHP process per docker exec), since

tests/integration/user-roundtrip.sh

@@ -48,7 +48,8 @@ ok "as-backup restores into <uid>-bak, byte-identical, original account untouche
 # replace: damage the account, then restore it in place under the same uid.
 $X sh -c "rm '$DATA/$U/files/Docs/f1.bin' && echo damaged > '$DATA/$U/files/Docs/f1.bin'"
 occ files:scan "$U" >/dev/null
-occ backup:user:restore ci "$MANIFEST" --mode=replace >/dev/null || fail "restore replace"
+if occ backup:user:restore ci "$MANIFEST" --mode=replace --no-interaction >/dev/null 2>&1; then fail "replace of an existing account ran without --force"; fi
+occ backup:user:restore ci "$MANIFEST" --mode=replace --force >/dev/null || fail "restore replace"
 occ files:scan "$U" >/dev/null
 SUM_AFTER=$($X sh -c "cd $DATA/$U/files && find . -type f -print0 | sort -z | xargs -0 md5sum | md5sum")
 [ "$SUM_BEFORE" = "$SUM_AFTER" ] || fail "files differ after a replace restore"