Review fixes: a missed passphrase change follows at the next backup, a key from before 1.1.0 counts as used, schedules edited in their own time zone

- A location that a passphrase change could not reach gets it at its next backup
- A key from before 1.1.0 is never replaced as an unused one
- An unreadable settings file on a location is told apart from a missing one
- A schedule is shown and saved in the zone it was saved in
- Saving an unchanged schedule keeps its moment; --timezone needs a schedule
- Schedule coverage uses the time the backup began reading its data
- Tests: catch-up scenario, upgrade test restores the new file, autumn clock change

lib/BackgroundJob/BackupJob.php

@@ -116,7 +116,7 @@ final class BackupJob extends TimedJob {
 			}
 			$attempts = $this->runs->startedSince($t->getId(), BackupService::KIND_FULL, $moment);
 			if (!\OCA\NgBackup\Service\LocationSchedule::isDue($moment, (int)$t->getScheduleSince(),
-				$this->runs->lastFinished($t->getId(), BackupService::KIND_FULL)?->getStartedAt(),
+				$this->dataStarted($this->runs->lastFinished($t->getId(), BackupService::KIND_FULL)),
 				count($attempts), ($attempts[0] ?? null)?->getFinishedAt(), $now)) {
 				continue;
 			}
@@ -127,4 +127,15 @@ final class BackupJob extends TimedJob {
 			}
 		}
 	}
+
+	/** When a backup began reading its data: a queued one can start well after it was asked for. */
+	private function dataStarted(?\OCA\NgBackup\Db\Run $run): ?int {
+		if ($run === null) {
+			return null;
+		}
+		$state = json_decode($run->getState(), true);
+		$started = is_array($state) ? ($state['files']['started'] ?? null) : null;
+		$at = is_string($started) ? strtotime($started) : false;
+		return $at !== false ? $at : $run->getStartedAt();
+	}
 }

lib/Command/TargetAdd.php

@@ -161,9 +161,12 @@ HELP);
 			$r['target']->getRepositoryId(), $r['created'] ? 'created' : 'found and adopted'));
 		if ($passphrase !== null) {
 			$output->writeln(($replace ? 'The unused key was replaced. ' : '') . 'Backup key taken from the location, fingerprint <info>' . $this->keys->fingerprint() . '</info>.');
-			$output->writeln(array_key_exists('deleteDelayDays', $r) && $r['deleteDelayDays'] !== null
-				? 'Deletion delay for forgotten snapshots: <info>' . $this->keys->deleteDelayDays() . '</info> days (from the location).'
-				: 'Deletion delay for forgotten snapshots: <info>' . $this->keys->deleteDelayDays() . '</info> days (the default: the location has none stored, its backups were made before NG Backup 1.1.0).');
+			$days = 'Deletion delay for forgotten snapshots: <info>' . $this->keys->deleteDelayDays() . '</info> days';
+			$output->writeln(match (true) {
+				($r['deleteDelayDays'] ?? null) !== null => $days . ' (from the location).',
+				$r['settingsUnreadable'] ?? false => '<comment>' . $days . ' (the default: the settings file on the location could not be read, it may be damaged). If this server used a longer delay, removed restore points leave the Bin sooner than before.</comment>',
+				default => $days . ' (the default: the location has none stored, its backups were made before NG Backup 1.1.0).',
+			});
 			$output->writeln('Next: <info>occ backup:list</info> and <info>occ backup:restore:full</info>. Before backups run again, download a new recovery kit and confirm it (<info>occ backup:key:kit</info>, <info>occ backup:key:confirm</info>).');
 		}
 		return 0;

lib/Command/TargetSchedule.php

@@ -34,6 +34,10 @@ final class TargetSchedule extends Command {
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		$t = $this->targets->get($input->getArgument('target'));
 		$schedule = $input->getArgument('schedule');
+		if ($schedule === null && $input->getOption('timezone') !== null) {
+			$output->writeln('<error>--timezone goes with a schedule, e.g. backup:target:schedule ' . $t->getName() . ' 02:30 --timezone=Europe/Amsterdam</error>');
+			return 1;
+		}
 		$zone = $input->getOption('timezone') ?? $this->schedule->zoneOr($t->getScheduleZone());
 		if ($schedule !== null) {
 			try {

lib/Migration/Version000009Date20261108000000.php

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+// SPDX-FileCopyrightText: 2026 Dillard Blom
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+namespace OCA\NgBackup\Migration;
+
+use Closure;
+use OCA\NgBackup\Service\KeyService;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+/**
+ * A key from before 1.1.0 counts as used: those versions did not keep track, and it may protect
+ * backups on a location that was removed from this server later (user exports included). Only a
+ * key made with 1.1.0 or newer can be replaced as an unused one.
+ */
+final class Version000009Date20261108000000 extends SimpleMigrationStep {
+	public function __construct(
+		private KeyService $keys,
+	) {
+	}
+
+	public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
+		if ($this->keys->isInitialized()) {
+			$this->keys->markUsed();
+		}
+	}
+}

lib/Repository/Repository.php

@@ -239,8 +239,8 @@ final class Repository {
 	/**
 	 * Settings of the installation that never change (the deletion delay), sealed with the
 	 * master key so that, unlike the config and the recovery kit, nobody can alter them unseen.
-	 * Empty when the location has none yet (made before 1.1.0, until its next backup) or they
-	 * cannot be read.
+	 * Empty when the location has none yet (made before 1.1.0, until its next backup); only
+	 * ['unreadable' => true] when they are there but cannot be read.
 	 *
 	 * @return array<string, mixed>
 	 */
@@ -253,9 +253,9 @@ final class Repository {
 			$raw = (string)stream_get_contents($fh);
 			fclose($fh);
 			$settings = json_decode((new StreamCipher($keys))->decryptString($raw, 'settings'), true, 16, JSON_THROW_ON_ERROR);
-			return is_array($settings) ? $settings : [];
+			return is_array($settings) ? $settings : ['unreadable' => true];
 		} catch (BackendException|\OCA\NgBackup\Crypto\CryptoException|\JsonException) {
-			return [];
+			return ['unreadable' => true];
 		}
 	}
 

lib/Service/BackupService.php

@@ -227,6 +227,7 @@ final class BackupService {
 					$state['files']['started'] = gmdate('c');
 					// Locations from before 1.1.0 get their sealed settings at their next backup.
 					$this->targets->recordSettings($repo);
+					$this->targets->catchUpWrappedKey($target, $repo);
 				}
 				$dumper = new DbDumper($this->db->getInner(), $this->config->getSystemValueString('dbtableprefix', 'oc_'));
 				$dbStats = $state['dbStats'] ?? [];

lib/Service/KeyService.php

@@ -32,6 +32,7 @@ final class KeyService {
 	private const KEY_KIT_CODE = 'recovery_kit_code';
 	/** Set once a location was created or adopted with this key: from then on it is never replaced. */
 	private const KEY_USED = 'master_key_used';
+	private const KEY_WRAPPED_PENDING = 'master_key_wrapped_pending';
 	public const DEFAULT_DELETE_DELAY = 7;
 	/**
 	 * The location holds the passphrase-wrapped key too, so whoever can read it can try
@@ -127,6 +128,24 @@ final class KeyService {
 		}
 	}
 
+	/**
+	 * Locations that missed the last passphrase change (offline at the time): they still hold
+	 * the previous slots until the change reaches them, which is tried again at each backup.
+	 */
+	public function wrappedKeyPending(int $targetId): bool {
+		return $this->appConfig->getValueBool(Application::APP_ID, self::KEY_WRAPPED_PENDING . '_' . $targetId);
+	}
+
+	/** One flag per location, so that two processes never overwrite each other's. */
+	public function setWrappedKeyPending(int $targetId, bool $pending): void {
+		$key = self::KEY_WRAPPED_PENDING . '_' . $targetId;
+		if ($pending) {
+			$this->appConfig->setValueBool(Application::APP_ID, $key, true);
+		} elseif ($this->wrappedKeyPending($targetId)) {
+			$this->appConfig->deleteKey(Application::APP_ID, $key);
+		}
+	}
+
 	/**
 	 * A key made by mistake on a server that is being restored: remove it, so the key of the
 	 * backups can take its place. Only after the new key was opened with its passphrase, and

lib/Service/KeySlotService.php

@@ -43,8 +43,10 @@ final class KeySlotService {
 		foreach ($this->targets->list() as $t) {
 			try {
 				$this->targets->repository($t)->updateWrappedKey($wrapped);
+				$this->keys->setWrappedKeyPending($t->getId(), false);
 			} catch (\Throwable $e) {
 				$failed[] = $t->getName();
+				$this->keys->setWrappedKeyPending($t->getId(), true);
 			}
 		}
 		$label = '';
@@ -55,7 +57,7 @@ final class KeySlotService {
 		}
 		$this->alerts->securityEvent($event, ['slot' => (string)$slot, 'label' => $label]);
 		if ($failed !== []) {
-			throw new \RuntimeException('Saved, but these locations could not be updated (they still accept the previous passphrases): ' . implode(', ', $failed));
+			throw new \RuntimeException('Saved, but these locations could not be updated (they still accept the previous passphrases; the change is tried again at their next backup): ' . implode(', ', $failed));
 		}
 	}
 }

lib/Service/TargetService.php

@@ -172,7 +172,7 @@ final class TargetService {
 	 *
 	 * @param array<string, mixed> $options
 	 * @param ?callable(int, int): void $progress
-	 * @return array{target: Target, created: bool, deleteDelayDays: ?int} the deletion delay the location had stored, if any
+	 * @return array{target: Target, created: bool, deleteDelayDays: ?int, settingsUnreadable: bool} the deletion delay the location had stored, if any, and whether its settings could not be read
 	 * @throws \OCA\NgBackup\Crypto\CryptoException wrong passphrase
 	 */
 	public function addWithPassphrase(string $name, string $backend, string $auth, array $options, string $basePath,
@@ -194,13 +194,26 @@ final class TargetService {
 		if (!Repository::madeWithKey($storageBackend, $keys)) {
 			throw new \RuntimeException('The key in the config at this location does not open its backups: the config is damaged or was replaced. Only the recovery kit can help now: occ backup:key:import <kit file>, then this command again.');
 		}
-		$delay = Repository::settingsAt($storageBackend, $keys)['delete_delay_days'] ?? null;
+		$settings = Repository::settingsAt($storageBackend, $keys);
+		$delay = $settings['delete_delay_days'] ?? null;
 		$delay = is_int($delay) ? $delay : null;
 		$this->keys->adoptKey($keys, $wrapped, $delay, $replaceUnusedKey);
 		if ($old !== null) {
 			$this->alerts->securityEvent('key_replaced', ['old' => $old, 'new' => $this->keys->fingerprint()]);
 		}
-		return $this->add($name, $backend, $auth, $options, $basePath, $progress) + ['deleteDelayDays' => $delay];
+		return $this->add($name, $backend, $auth, $options, $basePath, $progress) + ['deleteDelayDays' => $delay, 'settingsUnreadable' => ($settings['unreadable'] ?? false) === true];
+	}
+
+	/** A passphrase change that missed this location (offline at the time) reaches it now. A failure is tried again at the next backup. */
+	public function catchUpWrappedKey(Target $target, Repository $repo): void {
+		if (!$this->keys->wrappedKeyPending($target->getId())) {
+			return;
+		}
+		try {
+			$repo->updateWrappedKey($this->keys->wrappedKey());
+			$this->keys->setWrappedKeyPending($target->getId(), false);
+		} catch (\Throwable) {
+		}
 	}
 
 	/** Seal the settings fixed at installation on the location, once. A failure is tried again at the next backup. */
@@ -402,6 +415,10 @@ final class TargetService {
 		}
 		$new = LocationSchedule::normalize($schedule);
 		$turnedOff = $new === '' && (string)$target->getSchedule() !== '';
+		if ($new === (string)$target->getSchedule() && $zone === $target->getScheduleZone()) {
+			// Saved again unchanged: a moment that already passed still gets its backup
+			return $target;
+		}
 		$target->setSchedule($new);
 		$target->setScheduleZone($zone);
 		$target->setScheduleSince(time());