5 Commits

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

Backup runs and scheduling:
- A file that keeps changing is read again at most three times; then the
  last read is kept and reported, instead of restarting forever and
  blocking cleanup.
- Unreadable files and folders are recorded, logged, shown by occ
  backup:run, stored in the snapshot and notified to admins, instead of
  being skipped silently.
- After a failed scheduled backup, at most three attempts a day, an hour
  apart, instead of a new run and notification every five minutes.
- The job no longer spins on a run whose lease another process holds, and
  the weekly cleanup catches up after a Sunday spent backing up.
- Notifications for the per-user export retention events.
2026-10-06 23:48:40 +02:00
dillard e8ecb77df5 Refresh the location lease while a per-user restore reads its export
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (32) (push) Has been cancelled
CI / integration-user (33) (push) Has been cancelled
CI / integration-user (34) (push) Has been cancelled
CI / integration-user (35) (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
A long import could outlive the lease, letting a cleanup forget the export
being restored and free its blobs mid-read.
2026-10-06 22:59:49 +02:00
dillard 70965dd4bd Keep leases alive during long restores and exports; PR review fixes
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (32) (push) Has been cancelled
CI / integration-user (33) (push) Has been cancelled
CI / integration-user (34) (push) Has been cancelled
CI / integration-user (35) (push) Has been cancelled
CI / integration-full-restore (push) Has been cancelled
Full restore, raw directory restore, per-user export and the safety export
held a fixed-TTL lease on the location that nothing refreshed. A restore or
export running longer than that TTL let a cleanup take over and delete data
still in use. They now refresh the lease while they work (per blob, per row
batch, per written chunk), throttled to once a minute.

Also:
- backup:prune still applies snapshot retention when user-export retention
  fails for a location.
- The recovery kit carries the deletion delay, so a disaster recovery keeps
  the original protection instead of falling back to 7 days. Older kits
  still import with the default.
- CI checkouts no longer persist the token in the git config.
- recoveryKit() has its real return type; the stale baseline entry is gone.
2026-10-06 22:38:47 +02:00
dillard eb9100a74d Fix review findings: catalog writes, user restore consent, retention validation
Catalog: forgetting a user export or purging trash now writes the catalog
before deleting the objects, so a crash leaves at most an unreferenced file
instead of a catalog entry pointing at nothing. Per-user backup and the
safety export reopen the repository inside the catalog lock, so they build
on the latest catalog generation.

Commands: backup:user:backup and backup:user:restore resolve the location
before asking to install user_migration, and a successful install always
stops with a rerun instruction. backup:retention rejects a malformed
--user-last before changing any policy. backup:key:import fails clearly
when the passphrase file cannot be read.
2026-10-06 17:40:51 +02:00
dillard 6cbc5d79d4 Phase 2: per-user backup and restore via user_migration
CI / unit (8.2) (push) Has been cancelled
CI / unit (8.3) (push) Has been cancelled
CI / unit (8.4) (push) Has been cancelled
CI / psalm (push) Has been cancelled
CI / integration (mariadb, 32) (push) Has been cancelled
CI / integration (mariadb, 33) (push) Has been cancelled
CI / integration (mariadb, 34) (push) Has been cancelled
CI / integration (mariadb, 35) (push) Has been cancelled
CI / integration (pgsql, 32) (push) Has been cancelled
CI / integration (pgsql, 33) (push) Has been cancelled
CI / integration (pgsql, 34) (push) Has been cancelled
CI / integration (pgsql, 35) (push) Has been cancelled
CI / integration (sqlite, 32) (push) Has been cancelled
CI / integration (sqlite, 33) (push) Has been cancelled
CI / integration (sqlite, 34) (push) Has been cancelled
CI / integration (sqlite, 35) (push) Has been cancelled
CI / integration-targets (push) Has been cancelled
CI / integration-user (push) Has been cancelled
Wires the existing RepositoryExportDestination/RepositoryImportSource (see
lib/UserMigration/) into user_migration's own UserMigrationService, resolved on demand
through a new UserMigrationServiceLocator since the app is an optional dependency
(PLAN.md 3.8) -- without it, only this feature is unavailable, the full backup still
works.

New: UserBackupService::backupUser() (occ backup:backup:user) streams a user_migration
export straight into the repository, under the same shared location lock as a regular
backup. UserRestoreService::restoreUser() (occ backup:restore:user) never imports over a
live account: --mode=replace safety-exports the current state first, then deletes and
recreates the account from the backup under the same uid; --mode=as-backup restores into
a new "<uid>-bak" account instead, since Nextcloud cannot rename a uid.

The catalog now also tracks user export manifests (Repository::recordUserExport/
listUserExports/forgetUserExport/isRecordedUserExport), rollback-protected like a
snapshot: listUserExports() verifies each one is still present on the location, and
restore only ever reads a manifest the verified catalog actually recorded, never an
arbitrary caller-supplied path. prune()'s existing raw users/ scan for blob retention is
unchanged. A new short exclusive "catalog" lock (PruneService::catalogLockKey())
serializes the catalog write itself, so two concurrent per-user backups (or one
alongside a full backup) can no longer race to write the same next generation and
silently drop one of them -- the broader per-target shared lock only ever protected
against a concurrent cleanup, not against other shared holders.

Two real bugs found by testing this against a live Nextcloud instance, not by unit
tests (RepositoryExportDestination/RepositoryImportSource implement OCP interfaces that
plain PHPUnit can't load, see tests/Unit/Repository/UserExportTest.php's docblock; the
new tests/integration/user-roundtrip.sh covers the real round trip instead):
- RepositoryImportSource::copyToFolder() double-closed the stream it handed to
  putContent()/newFile() (both already close it themselves), which throws a TypeError on
  PHP 8 and broke every restored file with content.
- A replace-mode restore failed on NotPermittedException: IRootFolder::getUserFolder()
  caches a Folder per uid for the process lifetime, so after delete+recreate of the same
  uid the import still got the deleted account's (now storage-less) folder. Fixed by
  dispatching FilesystemTornDownEvent, which Root already listens for to clear exactly
  that cache -- no internal OC\ classes needed.

Local review (CodeRabbit + Codex) against this diff found three more real issues, all
fixed: the catalog write race above, a failed as-backup import leaving a broken stub
account behind (now deleted on failure), and an export stream leak on a read/backend
failure in RepositoryExportDestination::copyFolder() (now closed in a finally).

Deliberately left out: wiring per-user export into the automatic backup schedule
(on-demand via occ only, for now) and retention/pruning of old user exports -- both open
design questions, not yet decided. The lease covering a single export/import still has a
fixed TTL (3600s) with no mid-call refresh, since UserMigrationService exposes no
progress; a resumable, step-based per-user backup (like the main BackupService run)
would remove that cap but is a bigger change than this pass.
2026-10-05 17:36:46 +02:00