Close remaining review gaps

- restore runs from the web are created under the location's exclusive
  lock, like backup runs
- raw restore writes to an unpredictable temporary name opened with
  exclusive create (never follows a planted symlink) and validates the
  path prefix; the restore service rejects non-canonical paths instead
  of trimming them (occ normalises user input before calling it)
- restore jobs report progress every minute, so a long restore is not
  taken for a dead one
- README: Object Lock turns deletions into delete markers

README.md

@@ -90,6 +90,10 @@ What you should do on the location (strongly recommended):
   for example **90 days**, plus versioning. Then nobody, not even with the server's credentials,
   can delete or overwrite backups younger than 90 days. Combine with an append-only location in
   NG Backup and a lifecycle rule that removes old versions after the lock expires.
+  With Object Lock, a deletion by NG Backup (or by an attacker through it) only adds a *delete
+  marker*: the locked data stays and can be brought back from the bucket's object versions until
+  the lock expires. Tested with an S3-compatible server; still to be verified on AWS itself, which
+  requires a checksum on uploads to locked buckets.
 - Use **credentials without delete rights** for the backup location where the storage supports it.
 - **SFTP / storage boxes:** enable the provider's own snapshots (e.g. daily, kept 30-90 days) and
   give the backup user access to its own folder only, without a shell.

appinfo/info.xml

@@ -5,7 +5,7 @@
     <name>NG Backup</name>
     <summary>Encrypted, incremental backups of your Nextcloud with restore from the web interface</summary>
     <description><![CDATA[Work in progress, not ready for use.]]></description>
-    <version>0.0.12</version>
+    <version>0.0.13</version>
     <licence>agpl</licence>
     <author>Dillard Blom</author>
     <namespace>NgBackup</namespace>

lib/BackgroundJob/RestoreJob.php

@@ -45,7 +45,17 @@ class RestoreJob extends QueuedJob {
 			$run->setUpdatedAt(time());
 			$run = $this->runs->update($run);
 			@set_time_limit(0);
-			$result = $this->restore->restoreUserFiles($this->targets->get((string)$run->getTargetId()), $req['snapshot'], $req['path'], $req['mode']);
+			$lastBeat = time();
+			$result = $this->restore->restoreUserFiles($this->targets->get((string)$run->getTargetId()), $req['snapshot'], $req['path'], $req['mode'],
+				function (int $done, int $total) use (&$run, &$lastBeat): void {
+					// Heartbeat, so a long restore is not mistaken for a dead one.
+					if (time() - $lastBeat >= 60) {
+						$lastBeat = time();
+						$run->setPhase("restoring $done/$total");
+						$run->setUpdatedAt(time());
+						$run = $this->runs->update($run);
+					}
+				});
 			$run->setStatus(Run::DONE);
 			$run->setPhase('done');
 			$run->setStats(json_encode($result, JSON_THROW_ON_ERROR));

lib/Command/Restore.php

@@ -49,7 +49,7 @@ HELP);
 		try {
 			$target = $this->targets->get($input->getArgument('target'));
 			$snapshot = $input->getArgument('snapshot');
-			$path = $input->getArgument('path');
+			$path = trim((string)$input->getArgument('path'), '/'); // CLI convenience; the service itself only accepts canonical paths
 			if ($dir = $input->getOption('to-directory')) {
 				$n = $this->restore->restoreToDirectory($target, $snapshot, $path, $dir);
 				$output->writeln("Restored <info>$n</info> files to $dir.");

lib/Controller/AdminApiController.php

@@ -234,8 +234,13 @@ class AdminApiController extends Controller {
 			|| !\OCA\NgBackup\Repository\Repository::isSafePath($path) || !preg_match('#^data/[^/]+/files(/.*)?$#', $path)) {
 			return new JSONResponse(['error' => 'Invalid path or mode'], Http::STATUS_BAD_REQUEST);
 		}
+		try {
+			$target = $this->targets->get((string)$targetId);
+		} catch (\Throwable) {
+			return new JSONResponse(['error' => 'No such location'], Http::STATUS_NOT_FOUND);
+		}
 		$run = new Run();
-		$run->setTargetId($targetId);
+		$run->setTargetId($target->getId());
 		$run->setKind('restore');
 		$run->setStatus(Run::RUNNING);
 		$run->setPhase('queued');
@@ -244,7 +249,11 @@ class AdminApiController extends Controller {
 		$run->setSnapshotId($snapshotId);
 		$run->setStartedAt(time());
 		$run->setUpdatedAt(time());
-		$run = $this->runs->insert($run);
+		try {
+			$run = $this->prune->underStartLock($target, fn () => $this->runs->insert($run));
+		} catch (\Throwable $e) {
+			return new JSONResponse(['error' => $e->getMessage()], Http::STATUS_CONFLICT);
+		}
 		$this->jobs->add(RestoreJob::class, ['run' => $run->getId()]);
 		return new JSONResponse($this->runJson($run));
 	}

lib/Repository/Repository.php

@@ -162,6 +162,9 @@ final class Repository {
 
 	/** Restore a snapshot (or only paths starting with $prefix) into a local directory. */
 	public function restore(string $snapshotId, string $target, string $prefix = ''): int {
+		if ($prefix !== '' && !self::isSafePath(rtrim($prefix, '/'))) {
+			throw new RepositoryException('Invalid path prefix');
+		}
 		$count = 0;
 		foreach ($this->streamEntries($snapshotId) as $entry) {
 			if ($prefix !== '' && !str_starts_with($entry['p'], $prefix)) {
@@ -179,8 +182,11 @@ final class Repository {
 			}
 			// Write next to the destination and rename after all blobs verified: at most one file
 			// is ever present twice.
-			$tmp = $dest . '.ngb-restore';
-			$out = fopen($tmp, 'wb');
+			$tmp = $dest . '.ngb-restore-' . bin2hex(random_bytes(6));
+			$out = @fopen($tmp, 'xb'); // fails if anything (including a symlink) already exists there
+			if ($out === false) {
+				throw new RepositoryException('Cannot create a temporary file for ' . $entry['p']);
+			}
 			foreach ($entry['b'] as $id) {
 				fwrite($out, $this->loadBlob($id));
 			}

lib/Service/PruneService.php

@@ -41,6 +41,23 @@ class PruneService {
 		}
 	}
 
+	/**
+	 * Run $fn under the location's exclusive lock (to create a run atomically with respect to a
+	 * cleanup, which takes the same lock and then refuses while a run exists).
+	 */
+	public function underStartLock(Target $target, callable $fn): mixed {
+		try {
+			$this->locking->acquireLock(self::lockKey($target), ILockingProvider::LOCK_EXCLUSIVE);
+		} catch (LockedException) {
+			throw new \RuntimeException('A cleanup of ' . $target->getName() . ' is in progress; try again later');
+		}
+		try {
+			return $fn();
+		} finally {
+			$this->locking->releaseLock(self::lockKey($target), ILockingProvider::LOCK_EXCLUSIVE);
+		}
+	}
+
 	/**
 	 * Exclusive lock on the location, then make sure no run of any kind (backup, restore, export)
 	 * is in progress: their data may not be referenced by a snapshot yet, or is being read.

lib/Service/RestoreService.php

@@ -92,7 +92,7 @@ class RestoreService {
 	}
 
 	private function doRestoreUserFiles(Target $target, string $snapshotId, string $logicalPath, string $mode, ?callable $progress): array {
-		if (!Repository::isSafePath(trim($logicalPath, '/')) || !preg_match('#^data/([^/]+)/files(?:/(.*))?$#', trim($logicalPath, '/'), $m)) {
+		if (!Repository::isSafePath($logicalPath) || !preg_match('#^data/([^/]+)/files(?:/(.*))?$#', $logicalPath, $m)) {
 			throw new \InvalidArgumentException('Expected a path like data/<user>/files/<folder or file>');
 		}
 		[$uid, $rel] = [$m[1], trim($m[2] ?? '', '/')];
@@ -144,6 +144,9 @@ class RestoreService {
 
 	/** Raw restore of any snapshot path to a local directory (administrators, disaster recovery). */
 	public function restoreToDirectory(Target $target, string $snapshotId, string $prefix, string $directory): int {
+		if ($prefix !== '' && !Repository::isSafePath($prefix)) {
+			throw new \InvalidArgumentException('Invalid path: ' . $prefix);
+		}
 		if (!is_dir($directory) && !mkdir($directory, 0750, true) && !is_dir($directory)) {
 			throw new \RuntimeException("Cannot create $directory");
 		}