files_external's SFTP read stream asks for the next 256 KiB only after
the previous answer has arrived, so reading costs one round trip per
request whatever the line speed: 4.9 MB/s at 47 ms to a Hetzner Storage
Box. SftpReadStream keeps up to eight READ requests in flight over the
same phpseclib connection (15 MB/s on that link), starting with one so
a small read does not fetch megabytes ahead.
A server that answers less than asked is handled by asking again from
where the answer ended; when more data follows, that size is used for
later reads over the same connection, never below the 32 KiB every
server must support. Without the needed connection methods, or when the
open fails, the storage's own stream is used as before.
Review notes on the open pack stream: it stayed open while a backup run went on to upload
(e.g. the snapshot after reading the file list back), which SFTP cannot do on one connection.
Reading moves to PackReader, and Repository reaches the location through PackReaderGuard,
which closes the stream before every other request. A pack stream that cannot seek is read up
to the offset. Deep verify no longer reports an \Error during decoding as damage.
Restores and deep verifies requested every blob separately: a new file handle and a seek on
the location each time. They read the blobs of a snapshot in the order they were written, so
the stream of the pack read last now stays open and is read on; a jump backwards, to another
pack, or more than 8 MiB ahead opens the pack again at the offset, as before. On a Hetzner
Storage Box over SMB, restoring 86 MB in 202 files went from 0.78 MB/s to 4.2 MB/s.
holdIndexCache() ran before beginTransaction(), outside the try, and the release came after
rollBack(); an exception from either left later cache writes queued for the rest of the run.
Opening a repository lists index/, which holds one file per pack.
StorageBackend::list() checked every entry with is_dir(), and on a
remote storage each check is a round trip. Over SFTP to a Hetzner Storage
Box (33 ms) with a 60 GiB repository, occ backup:run (30 s steps) spent
nearly every step on these requests and uploaded 45 KiB/s; with this
change it uploads 5.8 MB/s. The cost grows with the number of packs.
index/, snapshots/, catalog/ and trash/info/ only ever hold objects:
list them with the new IBackend::listFiles(), one listing without
per-entry checks.
A run only showed "running (files)", which says nothing during a backup
of a terabyte. The files phase now records an expected size on its first
step: the previous snapshot's size, or for a first backup the users'
files, versions and trash from Nextcloud's file cache (approximate, shown
with a ~). The web interface and occ backup:status show the share of it
done so far, including a large file that is still being read; it stays
below 100% until the run finishes.
A database dump that fills a whole pack (32 MiB) uploads that pack while
the dump transaction is still open, and the pack's index cache row was
inserted on the same connection. PostgreSQL refuses that (25006, read-only
transaction) and the run failed; on other databases the row was rolled
back with the dump transaction. The index now holds those cache writes
back until the transaction is over.
A changed password or host name could only be handled by adding a new
location, which starts an empty repository and, with a retention lock at
the provider, keeps the old one billed. Edit now changes a location in
place (web interface and occ backup:target:edit). The settings are only
saved when they lead to the same repository; a failed connection and a
missing repository give separate errors, and the folder is not created
while checking. Stored passwords are never sent to the browser: an empty
password field keeps the stored value.
Catch \Exception (storage adapters throw their own exception types) instead
of \Throwable, so an \Error such as a TypeError is not retried and reported
as an unreadable blob.
An unreadable blob may still be a truncated pack, so don't rule out damage
in the verify message; README: Hetzner needs WebDAV/SMB and external
reachability enabled for the sub-account.
verifyBlob() caught every exception from loading a blob and counted it as
a failed (corrupted or tampered) blob, so a transient network error during
a deep verify over WebDAV looked like data corruption. Read the range
separately, retry it, and report a read that keeps failing as unreadable
with its error; only a blob that was read but does not decrypt or match
its id counts as failed.
step() hands back the same Run object when another process holds the lease;
comparing update times per second could stop the scheduled job early after a
quick step. A failed write of the recovery kit file now closes the handle and
removes the partial file.
forget() and untrash() uploaded a remote read stream directly, which the
new size check could not judge on SFTP, so cleanup failed there. They now
copy through a local buffer, and the size check only trusts local streams
(php://temp, php://memory, plain files).
The disaster recovery test calls backup:restore:full with --no-interaction:
docker exec without a terminal still counts as interactive, so the new
confirmation question read an empty answer and cancelled.
- backup:restore:full refuses on an installation with more than one user
and asks before it starts (--force skips both, also to rerun after a
failure halfway), and warns when the snapshot's database dump was not
one consistent transaction.
- backup:user:restore --mode=replace asks before deleting an existing
account; non-interactive use needs --force.
- The recovery kit file is created owner-only and never over an existing
file or symlink; backup:key:import shows the deletion delay it read from
the kit.
- Adding a location through the web API needs password confirmation, like
removing one.
- Key derivation parameters from a kit or repository config are bounded,
and a stalled network stream no longer spins at full CPU.
- Floating-point columns are dumped exactly instead of with 14 digits.
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.
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.
A full backup opened its repository at the start of a step and wrote the
catalog at the end of it, without the catalog lock. A per-user export or
restore that wrote the catalog in between could lose its entry, or the
snapshot could lose its own.
The final step now only stores the snapshot object and moves the run to a
new 'commit' phase. BackupService then records the snapshot under the
catalog lock, from a repository opened inside that lock; if the lock is
busy, the next step retries the commit. Repository::writeSnapshot() keeps
its old behaviour as putSnapshot() plus recordSnapshot().
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.
backup:user:backup and backup:user:restore now ask the administrator before
installing or enabling user_migration (or take --install-user-migration), so
nothing changes on the server without consent. The restore warns that
user_migration imports only its allowlisted app settings.
Also: a clear error for unknown locations instead of a raw database message;
prune reports the number of affected users, not the number of kept ones; and
--user-last rejects values that are not whole numbers. Adds docs/HOWTO.md and
updates the README and PLAN.md for the beta.
backup:verify accepts a user export path (users/<uid>/<export>): every blob its manifest references
is checked the same way as for a snapshot, shallow or --deep.
The SMB backend is refused at backup:target:add when the native smbclient PECL extension is not
loaded, instead of registering and then failing the first backup with "Seek failed": with only the
smbclient binary files_external falls back to a shell stream that cannot seek.
The per-user integration job now runs on Nextcloud 32-35, not just 35: it calls user_migration's
internal UserMigrationService, whose API may change between versions.
PruneService::userExportRetention()/setUserExportRetention(): a single, app-wide "keep the
newest N" setting (default 3, same default as the snapshot policy's "last"), deliberately simpler
than the full daily/weekly/monthly RetentionPolicy -- per-user exports are on-demand only (occ
backup:backup:user, see the fold-out task about whether they ever join the automatic schedule),
so there is no regular cadence yet to bucket by day/week/month.
PruneService::applyUserExports(): groups a location's user exports by uid (read from each
manifest's own 'time' field, the same field snapshot retention already reads), keeps the newest N
per user, forgets the rest. No trash, unlike a snapshot: these are not yet the primary safety net
the deletion delay protects. Runs under the same exclusive lock as snapshot retention.
occ backup:retention gained --user-last=N; occ backup:prune now also applies it for every target
it processes, reporting counts the same way it already does for snapshots and blob packs.
Verified live against the dev rig (four real exports across four users, confirmed exactly the
expected ones were kept/forgotten at two different --user-last values) and added coverage to
tests/integration/user-roundtrip.sh (an account's manual export plus the automatic safety-export
a replace-mode restore makes of it, pruned down to --user-last=2).
Repository::verify(): every blob a snapshot's files and database dump reference must be in a
freshly re-downloaded, re-authenticated index (never trusted from the local cache, same reasoning
as prune()) and have its pack still present on the location -- shallow, cheap, no blob content
read, catches a deleted pack, a tampered/rolled-back index, or a database manifest that went
missing (reported as a failure, not silently skipped). With $deep, each blob is also downloaded
and decrypted, same as loadBlob() already does on every real read (AEAD authentication tag,
content hash against its blob id), done here deliberately for everything in the snapshot rather
than waiting for it to come up during an actual restore. A blob referenced by more than one file
is only checked once; pack presence is cached per pack, not re-checked per blob.
occ backup:verify <target> <snapshot> [--deep], meant to run periodically (cron, a sample of
recent snapshots), per PLAN.md's "Verificatie: periodiek checksums controleren, steekproef-restore".
The lease covering the audit is refreshed roughly every half of its TTL while blobs are checked,
rather than relying on a single long-lived lease like the other new phase-2 commands do.
Verified against a real S3-backed 1.4 GiB / 603-blob snapshot (shallow: instant; --deep: ~28s,
fully decrypted) in addition to the new unit tests and the roundtrip.sh addition (shallow and deep
on a snapshot with real, varied file sizes, now covered on every CI matrix combination).
Local review (Codex) on the first pass found three real issues, all fixed: a missing database
manifest was silently skipped instead of reported; shallow verification trusted the (possibly
stale or unauthenticated) local index cache instead of re-checking the authenticated index on the
location; and the lease had no refresh, so a --deep audit of a very large snapshot could outrun
its TTL and run unprotected against a concurrent prune.
New commands, per PLAN.md 3.7:
- occ backup:key:import: adopt the master key from a recovery kit on a fresh installation
(instead of occ backup:key:init generating a new one), so occ backup:target:add opens the
existing repository on the location rather than creating an empty one.
- occ backup:restore:full: replace the database (consistent, one transaction), restore the data
directory in place, and merge config.php -- refuses up front and lists what's missing when the
Nextcloud major version or any app enabled at backup time doesn't match here (their tables must
exist before the database restore can use them), rather than guessing or partially applying.
Repository::restore() gained an optional $stripPrefix (write "data/<uid>/..." straight into the
real data directory, not a "data/" subdirectory of it) -- restoring "data/" and "config/" this way
needs no temporary copy of what can be the largest part of an instance.
occ backup:target:add now backfills the local snapshot cache (ngb_snapshots, what occ backup:list
actually reads) when it adopts an existing repository: without this, a freshly connected location
showed no restore points at all, even though the repository itself had them. One unreadable
snapshot is skipped rather than aborting the backfill or the target add itself.
Three self-referential problems found and fixed by testing this against two real, separate
Nextcloud instances sharing only the backup location (tests/integration/full-restore-roundtrip.sh
covers this; the per-process nature of these bugs meant they were invisible from inside a single
occ invocation, including the one used to write the fixes, which is exactly why a second, fresh
process re-running occ backup:status etc. was necessary to catch them):
- The full database dump includes ng_backup's own tables and appconfig rows; restoring them
verbatim would overwrite the key/target that is actively performing the restore, on THIS
installation, with the backed-up instance's. Both are now preserved: ng_backup's own tables
(and, on PostgreSQL, their sequences -- excluding a table does not exclude its sequence) are
excluded from what gets restored, and its appconfig rows are captured before the database
restore and written back after, in their own transaction.
- config.php's 'secret' is merged in from the backup (needed so already-encrypted content in the
restored database, e.g. share tokens, stays decryptable), but Target::options, the master key,
the wrapped key and the recovery kit code are all encrypted with that same instance secret
(OCP's ICrypto, or Nextcloud AppConfig's own "sensitive" flag underneath), not with ng_backup's
own key. Changing it without re-encrypting them first left all four permanently undecryptable
the moment any later occ command ran. Fixed with two helpers
(TargetService::reencryptOptionsAround(), KeyService::reencryptForSecretRotation()) that decrypt
before and re-encrypt after whatever callback actually changes the secret; config.php is applied
dead last in restoreFull(), after every other repository access. Not crash-safe (a process kill
mid-rotation would leave a key missing, not just stale) -- disaster recovery is run once by
hand, right after occ maintenance:install, a window too narrow yet to justify a two-phase write.
- A subtler variant of the same bug: Nextcloud's own AppConfig additionally encrypts a "sensitive"
value itself, tied to the same secret, underneath our own encryption. Overwriting an existing
sensitive key makes it read the old value first to compare, which then also fails against the
new secret. Deleting the key before setting the new value avoids that read entirely.
Roadmap (PLAN.md 9): phase 3 (in-place full restore, user self-service) moved out to v1.3.0; phase
2 continues toward v1.2.0 with FTP/SMB, a verification/checksum-audit command, user-export
retention, and deciding whether per-user export joins the automatic schedule.
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.
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.
Added tests/integration/targets-matrix.sh, which runs the full backup/restore
round trip through files_external's AmazonS3 and SFTP backend classes (S3
via a disposable s3mock container, SFTP via atmoz/sftp), since the existing
matrix only ever exercised the local backend.
This caught a real bug: moving a snapshot to the trash read it from the
location and streamed it straight back into a write on the same connection,
which SFTP can't do reliably (phpseclib fails to interleave a read and a
write on one connection). Backends now expose a native move() and the
id-bound snapshot object is moved with that instead of a read+write copy;
the path-bound database dump still goes through decrypt/re-encrypt, since
its AEAD authentication is tied to its path.
Show the date of the restore point a restore run used, both in the
backup history table and when confirming a restore. Split the admin
page into a "Backup and restore" tab (locations, backups, restore)
and a "Settings" tab (key, slots, recovery kit), so daily work no
longer sits behind the key setup at the top of the page.
Signed-off-by: Dillard Blom <dillard.blom@ensembia.com>
- after each successful backup the stored size is recorded; at 80% of a
location's limit administrators get a notification (once a day)
- locations list shows stored size and limit; the add form has a
'maximum size' field that explains that empty means unlimited and
billed by the provider; PUT /api/targets/{id}/limit (password
confirmation)
Tested on NC34 with the OVH test bucket: a backup above the limit stops,
a successful one at 81% sends the warning.
A backup step killed by a time limit, OOM or kill -9 kept Nextcloud's
lock on its location for up to an hour, blocking every new start and the
run itself. Locations and runs are now guarded by leases in ngb_leases
with an expiry: a backup step holds them for its duration plus two
minutes, restores refresh theirs every minute, prune holds 30 minutes,
untrash 5. Nextcloud's lock is only used for the milliseconds in which a
lease is checked and written.
Also: starting a backup while one is already running simply continues
it (no exclusive lease needed), and running to completion waits instead
of spinning while a crashed step's lease has not expired.
Tested on NC34: a backup killed four times at random moments resumes on
its own after the lease expires (about two minutes), and restores
bit-identically.
S3 has no bucket quota, so NG Backup enforces one: before a pack is
uploaded, the stored size plus the pack must stay under the location's
limit, otherwise the backup stops with an error (and administrators are
notified through the failed run). Repacking during prune is exempt, as it
is how space is freed. occ backup:target:limit NAME [SIZE|none].
Also ignore credential files in the repository.
- tests/integration/roundtrip.sh: key, kit code and confirmation,
location, full and unchanged incremental backup, restore new-folder and
replace (files identical, extra file in the trash), database dump and
restore, retention, trash and restore from trash, no files_external
errors in the log
- tests/integration/matrix.sh: fresh Nextcloud per version and database
(SQLite, MariaDB, PostgreSQL), removes only its own containers and
their volumes; locally green on NC32 and NC35 with all three databases
- GitHub Actions: unit tests on PHP 8.2-8.4, round trip on NC32-35 x
SQLite/MariaDB/PostgreSQL
Fixes found by the matrix:
- files_external is no longer booted when it is disabled: only its
classes are loaded and a private BackendService gets its backends from
its Application class (booting registered its mount provider, which
failed on installations where its tables were never created)
- SQLite: the database file is excluded from the file backup (it is
covered by the logical dump and copying the live file is inconsistent)
- occ backup:key:confirm accepts --code and --confirm for scripts
- DbDumper::dumpStep(): tables are dumped until the step's deadline in one
REPEATABLE READ transaction and resumed at the last primary key in the
next step; the running table checksum is carried over. A dump that fits
in one step is fully consistent, otherwise the snapshot records
dbConsistent=false. BackupService uses it, so the database phase no
longer needs one long request (hosted Nextcloud, web cron)
- restore inserts 100 rows per statement and leaves tables whose checksum
already matches the dump untouched
- spike C: 30-step dump gives the same checksums as the one-shot dump;
restore rewrote only the 3 damaged tables of 147; 300k-row table
restored in 14.6 s instead of 46 s
Every change to the set of snapshots or trash entries writes an
encrypted catalog generation bound to the repository and its position,
with the hash of the previous generation. The server anchors the newest
generation it has seen in its app config; on open, the location must
show at least that generation and chain back to it, and every snapshot
listed must exist. A rolled-back or tampered location is refused (the
run fails and administrators are notified). Opening with the passphrase
on a new server accepts the newest valid chain.
occ backup:target:trust-current accepts a legitimately older state
(e.g. restored from the provider's snapshot) after typing the location
name; audited and announced to all administrators.
Tests: chain advances, rollback, hidden snapshot, swapped generation,
disaster recovery without anchor. Tested on NC34 by deleting the newest
catalog generation on the SFTP location.
dillard