Commit Graph
5 Commits
Author SHA1 Message Date
snegovandMaxim Shulga 33dcbe6c58 Implement a restore command (#13)
* Implement restore command

Add a restore subcommand alongside the existing backup command:
- curateipsum/restore.py: restore_snapshot() and verify_restored(),
  reusing fs.scantree/copy_direntry/rm_direntry so metadata handling
  (mode, ownership, timestamps, symlinks) matches backup exactly.
- backup.find_backup(): resolve a snapshot by name or latest.
- cli.py: argparse subcommands (backup/restore), with backup implied
  when no subcommand is given so existing invocations keep working.

Restore supports dry-run (creates nothing), partial restore by
snapshot-relative path, an overwrite policy (never/always), a
destination-inside-snapshot boundary check, and manifest-checksum
verification of restored content. It takes the same backups_lock a
backup does, since retention could otherwise delete the snapshot
being restored out from under it.

Closes P1 'Provide a safe restoration workflow' in IMPROVEMENTS.md.

* Address Copilot review: symlink-escape boundary check, docstring fix

- restore_snapshot() and verify_restored() now canonicalize each
  destination path and refuse to operate on it if it resolves outside
  dest_dir - a symlink planted under dest_dir (by another process, or
  left over from a prior run) could otherwise redirect a write, delete,
  or read outside the intended destination.
- Fixed restore_snapshot()'s docstring: the skipped-paths list reports
  what would be skipped on dry-run too, it isn't always empty.

* Address Copilot suppressed findings: stream plan_restore, restore-specific lock message

- plan_restore() is now a generator instead of building a full list in
  memory, matching fs.scantree()'s streaming style everywhere else in
  the codebase.
- restore CLI logs a restore-specific message on lock-acquire failure,
  since set_backups_lock()'s own log line talks about a 'previous
  backup', which read wrong for a restore.

* Fix PATH docs: snapshot-relative means source-basename-prefixed

README and CLI help examples for restore's PATH argument omitted the
source directory's basename, which backup.initiate_backup() always
prefixes onto snapshot-relative paths - the given examples would have
matched nothing in an actual snapshot.

---------

Co-authored-by: Maxim Shulga <maxim.shulga@alludo.com>
2026-08-04 15:58:34 +00:00
fd613edad0 Correct retention before enabling automatic deletion (#8)
* Correct retention before enabling automatic deletion

- Compute each retention tier's kept set independently from its own
  name range and union them, instead of carrying a single 'previous
  backup' comparison across tier boundaries - that carry-over was the
  actual cause of bug #32 (a coarser tier could evict a backup a finer
  tier, or the always-keep-latest rule, had already decided to retain)
- Group weekly retention by (ISO year, ISO week) instead of ISO week
  number alone, fixing bug #33 (same week number in different years,
  or late-December dates that belong to next year's ISO week 1, were
  treated as duplicates/collisions)
- Validate the computed retention plan (latest snapshot present, no
  phantom entries) before deleting anything
- Fix cli.py to run initiate_backup before cleanup_old_backups, and
  skip cleanup entirely if the backup raised - retention must only run
  once a new snapshot is durable and complete, and a failed backup must
  never trigger deletion of history
- Quarantine (rename) a deletion candidate before recursively removing
  it, reusing the already-hardened stale-staging-dir cleanup to finish
  the job if the process crashes mid-delete
- Remove both retention xfail markers (bugs #32, #33) now that the
  tests they guarded pass; update pre-existing tests whose expectations
  encoded the old bug (deleting the latest snapshot when it collided
  with the next-newest one in a coarser bucket)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address Copilot review: replace asserts with runtime checks, don't swallow quarantine-delete failures

- Retention-plan validation now raises BackupFailedError instead of
  using assert, which python -O would silently strip ahead of a
  destructive operation
- _quarantine_and_delete no longer passes ignore_errors=True to
  rmtree; a removal failure is now logged with the quarantine path so
  operators can find and retry it, instead of silently leaving a
  hidden directory consuming space

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Maxim Shulga <maxim.shulga@alludo.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-03 22:47:43 +00:00
72a8fe2b59 Validate topology and destructive path boundaries (#7)
* Validate topology and destructive path boundaries

- Canonicalize sources/backups_dir and reject overlap in either direction
- Reject duplicate sources and destination basename collisions
- Reject non-normalized relative paths in nest_hardlink
- Give each backup repository a persistent identity, recorded in every
  snapshot's completion manifest
- Verify repo identity, directory type (no symlinks), containment, and
  device before any recursive snapshot/staging-dir deletion, refusing to
  cross mount points

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address Copilot review: root-path check, symlink/TOCTOU-safe writes, legacy marker adoption

- _is_same_or_within: drop the redundant, root-mishandling startswith
  check; commonpath() alone is already component-aware
- _ensure_repo_id/_write_manifest: write via temp-file + atomic rename
  (O_NOFOLLOW, O_EXCL on the temp file) instead of open+write in place,
  so writes can never go through a pre-existing symlink and are never
  left half-written
- _verify_safe_to_delete: distinguish a legacy empty marker (pre-dates
  the manifest format, can only be a pre-migration artifact) from a
  non-empty-but-invalid one (corrupt or forged); adopt the former into
  the repository instead of refusing to delete it forever

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Maxim Shulga <maxim.shulga@alludo.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-03 22:26:51 +00:00
a48eaf8117 Make snapshot creation transactional (#5)
* Make snapshot creation transactional

Build each snapshot in a private .incomplete-<name> staging directory
instead of mutating the final backup path directly. Any failure while
building the snapshot (hardlink failure, rsync error, or a single
Actions.ERROR entry) is now fatal: the staging directory is discarded
and BackupFailedError is raised, so a failed or interrupted backup can
never silently look like a success.

On success, the completion marker is written and the directory fsync'd
before an atomic os.rename() into the final snapshot name, so a crash
can never leave a partially-populated directory under a real snapshot
name.

Also:
- cleanup_stale_staging_dirs() removes abandoned staging directories
  left by a crashed run before a new backup starts.
- _reserve_snapshot_name() avoids same-second snapshot name collisions.
- cli.py releases the backup lock in finally and returns a nonzero
  exit code when BackupFailedError is raised.

Updates the error-recovery tests that previously asserted failures
were swallowed silently to assert BackupFailedError is raised instead,
matching the new fail-loud contract.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Address Copilot review feedback on transactional snapshots

- Remove unused uuid import.
- _reserve_snapshot_name() also checks for a collision with an existing
  .incomplete-<name> staging directory, not just the final snapshot
  name.
- _fsync_dir() opens with O_DIRECTORY where available.
- Fsync the completion marker file itself, and fsync the parent
  backups_dir after the rename, so the marker and the rename survive a
  crash/power loss.
- Re-raise an already-BackupFailedError unchanged (preserving its
  traceback) instead of wrapping it in a new instance; only
  fs.BackupCreationError gets wrapped.
- Wrap the initial os.mkdir(staging.path) failure in BackupFailedError
  so every failure path uses the documented exception.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Maxim Shulga <maxim.shulga@alludo.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-03 21:30:39 +00:00
snegov dc9bc2e5e4 Add comprehensive CLI tests
Implements 26 tests across 6 test classes covering all CLI functionality:
- Argument parsing and flag handling
- Platform validation (Linux/macOS only)
- External tool availability checks (rsync, cp, gcp)
- Directory validation
- Lock acquisition and release
- Backup execution flow

Brings CLI module from zero coverage to complete test coverage.
Total test count: 99 → 127 tests (all passing).
2026-02-04 19:40:59 -08:00