Wire Psalm into CI, fix a real bug it found in user-migration import

RepositoryImportSource::copyToFolder() called putContent() on the generic
return type of Node::get(), which doesn't declare that method -- only
File does. Existing-file overwrite during a per-user restore would have
fatally errored at runtime; added an instanceof check and a resource leak
fix (finally-close) while there. Also guarded getFileAsStream() against
fopen() returning false.

Everything else Psalm (strict level) currently flags is mechanical
(missing `final` on leaf classes -- applied everywhere safe) or pre-existing
array-shape/type gaps, captured in psalm-baseline.xml so CI fails only on
new issues from here on, not the whole backlog at once.
This commit is contained in:
2026-10-05 11:45:37 +02:00
parent 48ea6bb33d
commit 807353cbe4
58 changed files with 626 additions and 60 deletions
+1 -1
View File
@@ -12,7 +12,7 @@ use OCP\AppFramework\Bootstrap\IBootContext;
use OCP\AppFramework\Bootstrap\IBootstrap;
use OCP\AppFramework\Bootstrap\IRegistrationContext;
class Application extends App implements IBootstrap {
final class Application extends App implements IBootstrap {
public const APP_ID = 'ng_backup';
public function __construct() {
+1 -1
View File
@@ -7,5 +7,5 @@ declare(strict_types=1);
namespace OCA\NgBackup\Backend;
class BackendException extends \RuntimeException {
final class BackendException extends \RuntimeException {
}
+1 -1
View File
@@ -22,7 +22,7 @@ use Psr\Log\LoggerInterface;
* With system cron a step may take minutes; with webcron/AJAX cron it is kept short so it fits
* in a normal web request (hosted Nextcloud).
*/
class BackupJob extends TimedJob {
final class BackupJob extends TimedJob {
public function __construct(
ITimeFactory $time,
private BackupService $backups,
+1 -1
View File
@@ -16,7 +16,7 @@ use OCP\BackgroundJob\QueuedJob;
use Psr\Log\LoggerInterface;
/** Runs a restore requested from the web interface, outside the web request. */
class RestoreJob extends QueuedJob {
final class RestoreJob extends QueuedJob {
public function __construct(
ITimeFactory $time,
private RunMapper $runs,
+1 -1
View File
@@ -15,7 +15,7 @@ use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
class Browse extends Command {
final class Browse extends Command {
public function __construct(
private TargetService $targets,
private RestoreService $restore,
+1 -1
View File
@@ -14,7 +14,7 @@ use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Question\Question;
class KeyConfirm extends Command {
final class KeyConfirm extends Command {
public const STATEMENT = 'I have downloaded the recovery kit and stored it safely, outside this server. '
. 'I understand that keeping these keys safe is my own responsibility and that backups cannot be restored without them.';
public const CONFIRMATION = 'Yes, I confirm';
+1 -1
View File
@@ -14,7 +14,7 @@ use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Question\Question;
class KeyInit extends Command {
final class KeyInit extends Command {
public function __construct(
private KeyService $keys,
) {
+1 -1
View File
@@ -13,7 +13,7 @@ use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
class KeyKit extends Command {
final class KeyKit extends Command {
public function __construct(
private KeyService $keys,
private \OCA\NgBackup\Service\AlertService $alerts,
+1 -1
View File
@@ -10,7 +10,7 @@ namespace OCA\NgBackup\Command;
use OCA\NgBackup\Service\KeyService;
use OCA\NgBackup\Service\KeySlotService;
class KeySlotAdd extends KeySlots {
final class KeySlotAdd extends KeySlots {
public function __construct(KeyService $keys, KeySlotService $slots) {
parent::__construct($keys, $slots, 'add');
}
+1 -1
View File
@@ -10,7 +10,7 @@ namespace OCA\NgBackup\Command;
use OCA\NgBackup\Service\KeyService;
use OCA\NgBackup\Service\KeySlotService;
class KeySlotRemove extends KeySlots {
final class KeySlotRemove extends KeySlots {
public function __construct(KeyService $keys, KeySlotService $slots) {
parent::__construct($keys, $slots, 'remove');
}
+1 -1
View File
@@ -10,7 +10,7 @@ namespace OCA\NgBackup\Command;
use OCA\NgBackup\Service\KeyService;
use OCA\NgBackup\Service\KeySlotService;
class KeySlotReplace extends KeySlots {
final class KeySlotReplace extends KeySlots {
public function __construct(KeyService $keys, KeySlotService $slots) {
parent::__construct($keys, $slots, 'replace');
}
+1 -1
View File
@@ -15,7 +15,7 @@ use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
class ListSnapshots extends Command {
final class ListSnapshots extends Command {
public function __construct(
private TargetService $targets,
private SnapshotMapper $snapshots,
+1 -1
View File
@@ -15,7 +15,7 @@ use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
class Prune extends Command {
final class Prune extends Command {
public function __construct(
private TargetService $targets,
private PruneService $prune,
+1 -1
View File
@@ -15,7 +15,7 @@ use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
class Restore extends Command {
final class Restore extends Command {
public function __construct(
private TargetService $targets,
private RestoreService $restore,
+1 -1
View File
@@ -14,7 +14,7 @@ use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
class Retention extends Command {
final class Retention extends Command {
public function __construct(
private PruneService $prune,
) {
+1 -1
View File
@@ -16,7 +16,7 @@ use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
class RunBackup extends Command {
final class RunBackup extends Command {
public function __construct(
private BackupService $backups,
private TargetService $targets,
+1 -1
View File
@@ -15,7 +15,7 @@ use Symfony\Component\Console\Helper\Table;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
class Status extends Command {
final class Status extends Command {
public function __construct(
private KeyService $keys,
private TargetService $targets,
+1 -1
View File
@@ -15,7 +15,7 @@ use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
class TargetAdd extends Command {
final class TargetAdd extends Command {
public function __construct(
private TargetService $targets,
private KeyService $keys,
+1 -1
View File
@@ -13,7 +13,7 @@ use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
class TargetAppendOnly extends Command {
final class TargetAppendOnly extends Command {
public function __construct(
private TargetService $targets,
private \OCA\NgBackup\Service\AlertService $alerts,
+1 -1
View File
@@ -12,7 +12,7 @@ use OCA\NgBackup\Service\TargetService;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
class TargetBackends extends Base {
final class TargetBackends extends Base {
public function __construct(
private TargetService $targets,
) {
+1 -1
View File
@@ -13,7 +13,7 @@ use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
class TargetLimit extends Command {
final class TargetLimit extends Command {
public function __construct(
private TargetService $targets,
) {
+1 -1
View File
@@ -13,7 +13,7 @@ use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
class TargetList extends Base {
final class TargetList extends Base {
public function __construct(
private TargetService $targets,
) {
+1 -1
View File
@@ -13,7 +13,7 @@ use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
class TargetRemove extends Command {
final class TargetRemove extends Command {
public function __construct(
private TargetService $targets,
private \OCA\NgBackup\Service\AlertService $alerts,
+1 -1
View File
@@ -22,7 +22,7 @@ use Symfony\Component\Console\Question\Question;
* legitimate cases (e.g. the location was restored from the provider's own snapshot), only from
* occ, audited and announced to all administrators.
*/
class TargetTrustCurrent extends Command {
final class TargetTrustCurrent extends Command {
public function __construct(
private TargetService $targets,
private IAppConfig $appConfig,
+1 -1
View File
@@ -15,7 +15,7 @@ use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
class Trash extends Command {
final class Trash extends Command {
public function __construct(
private TargetService $targets,
private KeyService $keys,
+1 -1
View File
@@ -14,7 +14,7 @@ use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
class TrashRestore extends Command {
final class TrashRestore extends Command {
public function __construct(
private TargetService $targets,
private PruneService $prune,
+1 -1
View File
@@ -32,7 +32,7 @@ use OCP\IUserSession;
* JSON API for the admin settings page. Every method requires an admin (the AppFramework
* default for controllers without NoAdminRequired) and a valid CSRF token.
*/
class AdminApiController extends Controller {
final class AdminApiController extends Controller {
public function __construct(
IRequest $request,
private KeyService $keys,
+1 -1
View File
@@ -7,5 +7,5 @@ declare(strict_types=1);
namespace OCA\NgBackup\Crypto;
class CryptoException extends \RuntimeException {
final class CryptoException extends \RuntimeException {
}
+1 -1
View File
@@ -12,7 +12,7 @@ use OCA\NgBackup\Repository\ICatalogAnchor;
use OCP\IAppConfig;
/** Keeps the newest catalog generation seen per repository in the app config (outside the location). */
class AppConfigCatalogAnchor implements ICatalogAnchor {
final class AppConfigCatalogAnchor implements ICatalogAnchor {
public function __construct(
private IAppConfig $appConfig,
) {
+1 -1
View File
@@ -11,7 +11,7 @@ use OCA\NgBackup\Repository\IIndexCache;
use OCP\DB\Exception;
use OCP\IDBConnection;
class DbIndexCache implements IIndexCache {
final class DbIndexCache implements IIndexCache {
public function __construct(
private IDBConnection $db,
) {
+1 -1
View File
@@ -33,7 +33,7 @@ use OCP\AppFramework\Db\Entity;
* @method int|null getFinishedAt()
* @method void setFinishedAt(?int $v)
*/
class Run extends Entity {
final class Run extends Entity {
public const RUNNING = 'running';
public const DONE = 'done';
public const FAILED = 'failed';
+1 -1
View File
@@ -12,7 +12,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
/** @template-extends QBMapper<Run> */
class RunMapper extends QBMapper {
final class RunMapper extends QBMapper {
public function __construct(IDBConnection $db) {
parent::__construct($db, 'ngb_runs', Run::class);
}
+1 -1
View File
@@ -25,7 +25,7 @@ use OCP\AppFramework\Db\Entity;
* @method int|null getBytes()
* @method void setBytes(?int $v)
*/
class Snapshot extends Entity {
final class Snapshot extends Entity {
protected int $targetId = 0;
protected string $snapshotId = '';
protected string $kind = '';
+1 -1
View File
@@ -12,7 +12,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
/** @template-extends QBMapper<Snapshot> */
class SnapshotMapper extends QBMapper {
final class SnapshotMapper extends QBMapper {
public function __construct(IDBConnection $db) {
parent::__construct($db, 'ngb_snapshots', Snapshot::class);
}
+1 -1
View File
@@ -29,7 +29,7 @@ use OCP\AppFramework\Db\Entity;
* @method int getCreatedAt()
* @method void setCreatedAt(int $v)
*/
class Target extends Entity {
final class Target extends Entity {
protected string $name = '';
protected string $backend = '';
protected string $auth = '';
+1 -1
View File
@@ -11,7 +11,7 @@ use OCP\AppFramework\Db\QBMapper;
use OCP\IDBConnection;
/** @template-extends QBMapper<Target> */
class TargetMapper extends QBMapper {
final class TargetMapper extends QBMapper {
public function __construct(IDBConnection $db) {
parent::__construct($db, 'ngb_targets', Target::class);
}
@@ -13,7 +13,7 @@ use OCP\DB\Types;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
class Version000001Date20261002000000 extends SimpleMigrationStep {
final class Version000001Date20261002000000 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
@@ -14,7 +14,7 @@ use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
/** Local cache of the per-pack index files, so opening a repository does not download them all. */
class Version000002Date20261002010000 extends SimpleMigrationStep {
final class Version000002Date20261002010000 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
@@ -14,7 +14,7 @@ use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
/** Append-only locations: NG Backup never deletes anything there (cleanup by the location's own lifecycle rules). */
class Version000003Date20261002020000 extends SimpleMigrationStep {
final class Version000003Date20261002020000 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
@@ -14,7 +14,7 @@ use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
/** Optional size limit per location (cost control; S3 has no bucket quota). */
class Version000004Date20261002030000 extends SimpleMigrationStep {
final class Version000004Date20261002030000 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
@@ -14,7 +14,7 @@ use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
/** Leases with an expiry: a killed process cannot block a location for longer than its lease. */
class Version000005Date20261002040000 extends SimpleMigrationStep {
final class Version000005Date20261002040000 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
+1 -1
View File
@@ -14,7 +14,7 @@ use OCP\Notification\INotification;
use OCP\Notification\INotifier;
use OCP\Notification\UnknownNotificationException;
class Notifier implements INotifier {
final class Notifier implements INotifier {
public function __construct(
private IFactory $l10n,
private IURLGenerator $url,
+1 -1
View File
@@ -7,5 +7,5 @@ declare(strict_types=1);
namespace OCA\NgBackup\Repository;
class QuotaExceededException extends RepositoryException {
final class QuotaExceededException extends RepositoryException {
}
+1 -1
View File
@@ -15,7 +15,7 @@ use OCP\IGroupManager;
use OCP\Notification\IManager;
/** Notifies administrators about failed runs and locations without a recent successful backup. */
class AlertService {
final class AlertService {
public const STALE_AFTER = 2 * 86400;
public function __construct(
+1 -1
View File
@@ -28,7 +28,7 @@ use Psr\Log\LoggerInterface;
* Excluded by default: previews, log files, the updater's backups and each user's cache and
* temporary uploads (all regenerable or temporary).
*/
class BackupService {
final class BackupService {
public const KIND_FULL = 'full';
public function __construct(
+1 -1
View File
@@ -22,7 +22,7 @@ use OCP\Security\ICrypto;
* into the recovery kit, so backups can be restored on a fresh server with the passphrase.
* - The admin must confirm having stored the recovery kit before the first backup runs.
*/
class KeyService {
final class KeyService {
private const KEY_MASTER = 'master_key';
private const KEY_WRAPPED = 'master_key_wrapped';
private const KEY_CONFIRMED = 'recovery_kit_confirmed';
+1 -1
View File
@@ -13,7 +13,7 @@ namespace OCA\NgBackup\Service;
* is written to all locations and announced to all administrators; a new recovery kit must be
* downloaded and confirmed afterwards (backups keep running meanwhile).
*/
class KeySlotService {
final class KeySlotService {
public function __construct(
private KeyService $keys,
private TargetService $targets,
+1 -1
View File
@@ -19,7 +19,7 @@ use OCP\Lock\LockedException;
* holder keeps refreshing it. Nextcloud's lock is only held for the few milliseconds in which a
* lease is checked and written.
*/
class LeaseService {
final class LeaseService {
public const SHARED = 's';
public const EXCLUSIVE = 'x';
+1 -1
View File
@@ -20,7 +20,7 @@ use OCP\Lock\LockedException;
* prunes data no remaining snapshot uses. Never runs on append-only locations, never while a
* backup to the same location is running (the location lock is shared with BackupService).
*/
class PruneService {
final class PruneService {
public function __construct(
private TargetService $targets,
private RunMapper $runs,
+1 -1
View File
@@ -28,7 +28,7 @@ use OCP\Files\NotFoundException;
* - merge: into the original place, existing files are overwritten (old content kept as a version)
* - replace: like merge, and files that are not in the snapshot are moved to the trash
*/
class RestoreService {
final class RestoreService {
public const MODE_NEW_FOLDER = 'new-folder';
public const MODE_MERGE = 'merge';
public const MODE_REPLACE = 'replace';
+1 -1
View File
@@ -22,7 +22,7 @@ use OCP\Server;
* object is built only while it is used; nothing is mounted. files_external is loaded for its
* classes but its enabled/disabled state is left as the admin set it.
*/
class TargetService {
final class TargetService {
public function __construct(
private TargetMapper $mapper,
private KeyService $keys,
+1 -1
View File
@@ -11,7 +11,7 @@ use OCP\AppFramework\Http\TemplateResponse;
use OCP\Settings\ISettings;
use OCP\Util;
class Admin implements ISettings {
final class Admin implements ISettings {
public function getForm(): TemplateResponse {
Util::addScript('ng_backup', 'ng_backup-admin');
Util::addStyle('ng_backup', 'ng_backup-admin');
+1 -1
View File
@@ -11,7 +11,7 @@ use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\Settings\IIconSection;
class AdminSection implements IIconSection {
final class AdminSection implements IIconSection {
public function __construct(
private IL10N $l,
private IURLGenerator $url,
+1 -1
View File
@@ -19,7 +19,7 @@ use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
/** Shows the backup state in the admin overview (Security & setup warnings). */
class BackupStatusCheck implements ISetupCheck {
final class BackupStatusCheck implements ISetupCheck {
public function __construct(
private IL10N $l,
private IURLGenerator $url,
+18 -6
View File
@@ -8,6 +8,7 @@ declare(strict_types=1);
namespace OCA\NgBackup\UserMigration;
use OCA\NgBackup\Repository\Repository;
use OCP\Files\File;
use OCP\Files\Folder;
use OCP\UserMigration\IImportSource;
use OCP\UserMigration\UserMigrationException;
@@ -30,10 +31,14 @@ final class RepositoryImportSource implements IImportSource {
return $data;
}
/** @return resource */
public function getFileAsStream(string $path) {
$entry = $this->entry($path, 'f');
// Phase 0: buffer in php://temp (memory up to 8 MiB, then a temp file for this one file).
$stream = fopen('php://temp/maxmemory:' . (8 * 1048576), 'w+b');
if ($stream === false) {
throw new UserMigrationException('Cannot buffer ' . $path);
}
foreach ($this->repo->readBlobs($entry['b']) as $data) {
for ($done = 0, $len = strlen($data); $done < $len; $done += $n) {
$n = fwrite($stream, $done === 0 ? $data : substr($data, $done));
@@ -51,6 +56,7 @@ final class RepositoryImportSource implements IImportSource {
$prefix = trim($path, '/');
$prefix = $prefix === '' ? '' : $prefix . '/';
$names = [];
/** @var string $p */
foreach (array_keys($this->manifest['entries']) as $p) {
if ($prefix === '' || str_starts_with($p, $prefix)) {
$rest = substr($p, strlen($prefix));
@@ -70,6 +76,7 @@ final class RepositoryImportSource implements IImportSource {
if (isset($this->manifest['entries'][$p])) {
return true;
}
/** @var string $k */
foreach (array_keys($this->manifest['entries']) as $k) {
if (str_starts_with($k, $p . '/')) {
return true;
@@ -91,12 +98,17 @@ final class RepositoryImportSource implements IImportSource {
$this->copyToFolder($sub, $path);
} else {
$stream = $this->getFileAsStream($path);
if ($destination->nodeExists($name)) {
$destination->get($name)->putContent($stream);
} else {
$destination->newFile($name, $stream);
}
if (is_resource($stream)) {
try {
if ($destination->nodeExists($name)) {
$existing = $destination->get($name);
if (!$existing instanceof File) {
throw new UserMigrationException("$path exists and is not a file");
}
$existing->putContent($stream);
} else {
$destination->newFile($name, $stream);
}
} finally {
fclose($stream);
}
}