v1.6.7: multi-destination fixes + backup compare/history UI

Fixes two backup-path bugs and adds the revision-browsing UI that was
implicitly requested by the "feat: local git repo" issue (local git
destinations already work; what was missing was a way to browse and
compare revisions in the web UI).

Bug fixes
- Manual backup trigger now honors the destination selection.
  /backups/trigger previously dropped destination_ids on the floor and
  fell back to "the first enabled local destination", so picking SMB +
  local from the UI silently saved only to local. POST handler now
  accepts destination_ids and the form renders a destination checkbox
  group.
- Backup row now records every destination that actually received the
  backup, not only the last. The text handler used to overwrite
  destination_type/destination_path inside its save loop, so a backup
  written to both local and SMB would only show one in the UI. A new
  _summarize_results helper aggregates successful (dest_type, path)
  tuples; destination_type is set to a comma-joined label ("local, smb")
  and destination_path keeps the local copy when one exists so the
  in-app Download / Delete buttons stay pointed at the local file.
- Proxmox / binary-archive backups now ship to every selected
  destination, not only local. _handle_binary_backup used to call
  _save_binary_local unconditionally and hardcode destination_type =
  "local". Added an optional save_binary(hostname, data, extension,
  config) method on DestinationBackend (default raises
  NotImplementedError so git-style backends can be cleanly skipped);
  LocalDestination and SMBDestination implement it. The binary handler
  now loops destinations, records all that accepted the archive, and
  only marks the backup failed when none did.

New UI
- Per-device backup history at /backups/device/<id>/history — full
  timeline with First / Changed / Unchanged badges computed from
  config_hash (no difflib per row) and checkboxes to pick any two
  revisions to compare.
- Compare view at /backups/compare?a=<id>&b=<id> — unified diff
  between any two backups of the same device. Normalises older→newer
  automatically, handles identical configs, archive bundles, and
  missing content with distinct banners.
- Destination badges: new partials/destination_badges.html renders
  colored Bootstrap badges per destination type (local gray, smb teal,
  git/gitea/github/forgejo blue); wired into list / detail / history.
- Backup detail gains "History" and "Compare with previous" buttons;
  device detail's Backup History card gains a "View all / Compare"
  link.

Housekeeping
- .dockerignore added to keep .env, *.db, ssh_keys/, backups/,
  staging-backups/, venv/, .git/ out of the Docker build context
  (previous builds baked all of these into the image).
- .gitignore adds staging-backups/* !staging-backups/.gitkeep so test
  artifacts generated during local runs don't show up as untracked.

Verification
- tests/: 50 passed, 1 skipped (baseline: same on upstream 1.6.6)
- Unit smoke: multi-dest text save records "git, local, smb";
  binary-to-SMB calls save_binary on local+smb, skips git with
  NotImplementedError warning; binary-only-git fails cleanly with
  "could not be written to any destination".
- HTTP smoke: history + compare routes render correctly, list +
  detail + history pages show destination badges, trigger form
  renders the destination picker.
- Upgrade smoke: running upstream _apply_migrations against the old
  staging.db (alembic_version=d4e5f6g7h8i9, credentials.username NOT
  NULL) correctly ran v09/v15/v16/v161/v161_repair and preserved all
  3 credentials / 3 devices / 98 backups.

Author: kulunkilabs

.dockerignore

@@ -0,0 +1,45 @@
+# Secrets and local config — never bake into image
+.env
+.env.*
+!.env.example
+
+# Local databases
+*.db
+*.db-journal
+*.db-wal
+*.db-shm
+
+# Local backups and key material
+backups/
+staging-backups/
+ssh_keys/
+
+# Virtual environments
+venv/
+.venv/
+
+# Git + CI metadata
+.git/
+.github/
+.gitignore
+
+# Python build artifacts
+__pycache__/
+*.pyc
+*.pyo
+*.egg-info/
+dist/
+build/
+.pytest_cache/
+
+# Editor / tooling
+.codex
+.vscode/
+.idea/
+*.swp
+
+# Tests and developer scripts (keep image lean)
+tests/
+docs/
+staging.db
+vibenetbackup.db

.gitignore

@@ -5,6 +5,8 @@ __pycache__/
 *.db
 backups/*
 !backups/.gitkeep
+staging-backups/*
+!staging-backups/.gitkeep
 ssh_keys/*
 .venv/
 *.egg-info/

README.md

@@ -2,7 +2,7 @@
 
 Network device configuration backup manager with multi-engine support, automated scheduling, and retention policies.
 
-**Version:** 1.5.7 | **License:** MIT
+**Version:** 1.6.7 | **License:** MIT
 
 <p align="center">
   <img src="docs/screenshots/screensh_01.png" alt="VIBENetBackup Dashboard" width="900"/>
@@ -109,6 +109,14 @@ Open `http://<your-server-ip>:5005` — default credentials are shown during ins
 
 ## Changelog
 
+### v1.6.7 (2026-04-23)
+- **Per-device backup history** — new timeline view at `/backups/device/<id>/history` with "First / Changed / Unchanged" markers computed from config hash, and checkboxes to pick any two revisions to compare
+- **Diff-any-two-backups view** — new compare page at `/backups/compare?a=<id>&b=<id>` renders a unified diff between arbitrary backups of the same device; handles identical-config, archive-bundle, and a/b-order-swap cases
+- **Manual backup now honors destination selection** — the trigger form gained a destination checkbox group; the POST handler accepts `destination_ids` so manual runs write to the destinations you pick instead of falling back to "first local"
+- **Multi-destination recording** — when a backup writes to multiple destinations (e.g. local + SMB), the `Backup` row now records all of them (`destination_type` = `"local, smb"`), displayed as colored badges in the list / detail / history views. Previously only the last successful destination showed
+- **Proxmox → SMB / remote destinations** — binary/archive backups (Proxmox tarballs) now ship to every selected destination, not only local. New optional `save_binary(hostname, data, extension, config)` method on `DestinationBackend`; `LocalDestination` and `SMBDestination` implement it; git-family destinations are skipped with a warning (archives in git are unusual)
+- **`.dockerignore`** — added to keep `.env`, `staging*.db`, `ssh_keys/`, `backups/`, `venv/`, `.git/`, and test caches out of the Docker build context
+
 ### v1.5.7 (2026-04-10)
 - **SSH Proxy / Jump Host** — Netmiko and SCP engines can now connect through a bastion/jump host before reaching the target device. Useful for remote sites where devices are only reachable via an intermediate SSH server (e.g. autossh tunnels). Configure per device: proxy host, proxy port, and optionally a separate proxy credential when the jump host uses different credentials than the device
 - **Separate proxy credentials** — Jump host and target device can authenticate with different username/password pairs, reusing the existing encrypted credential store

app/modules/backup_service.py

@@ -90,6 +90,35 @@ async def run_backup_for_device(
     )
 
 
+def _resolve_destinations(db: Session, destination_ids: list[int] | None) -> list[Destination]:
+    """Enabled Destination rows for the given IDs, or all enabled local
+    destinations as a fallback when nothing was selected."""
+    destinations: list[Destination] = []
+    if destination_ids:
+        destinations = db.query(Destination).filter(
+            Destination.id.in_(destination_ids),
+            Destination.enabled == True,
+        ).all()
+    if not destinations:
+        destinations = db.query(Destination).filter(
+            Destination.dest_type == "local",
+            Destination.enabled == True,
+        ).all()
+    return destinations
+
+
+def _summarize_results(results: list[tuple[str, str]]) -> tuple[str, str | None]:
+    """Pick a comma-joined label and a primary path for a successful multi-dest save.
+    Prefers a local path for `destination_path` so in-app Download/Delete keeps
+    pointing at the local copy."""
+    if not results:
+        return "local", None
+    types = sorted({t for t, _ in results})
+    local_paths = [p for t, p in results if t == "local"]
+    primary = local_paths[0] if local_paths else results[0][1]
+    return ", ".join(types), primary
+
+
 async def _handle_binary_backup(
     db: Session,
     device: Device,
@@ -98,145 +127,107 @@ async def _handle_binary_backup(
     destination_ids: list[int] | None,
     job_run,
 ) -> Backup:
-    """Save a binary (tar.gz/zip) backup to disk and store a JSON manifest in the DB."""
+    """Save a binary (tar.gz/zip) backup to each selected destination and
+    store a JSON manifest in the DB."""
     file_bytes, extension, file_list = binary_result
-
-    # Determine archive type from extension
     archive_type = "tgz" if extension == ".tar.gz" else "zip"
-
     config_hash = hashlib.sha256(file_bytes).hexdigest()
 
-    # Save file to local destination
-    saved_path = await _save_binary_local(device, file_bytes, extension, destination_ids, db)
+    destinations = _resolve_destinations(db, destination_ids)
 
+    results: list[tuple[str, str]] = []
+    save_errors: list[str] = []
+    for dest in destinations:
+        dest_type = dest.dest_type.value
+        try:
+            backend = get_destination(dest_type)
+            path = await backend.save_binary(
+                hostname=device.hostname,
+                data=file_bytes,
+                extension=extension,
+                config=dest.config_json or {},
+            )
+            results.append((dest_type, path))
+            logger.info(
+                "Saved binary backup for %s to %s: %s",
+                device.hostname, dest_type, path,
+            )
+        except NotImplementedError:
+            logger.warning(
+                "Destination '%s' does not support archive backups — skipping for %s",
+                dest_type, device.hostname,
+            )
+        except Exception as e:
+            logger.error(
+                "Failed to save binary backup to %s for %s: %s",
+                dest_type, device.hostname, e,
+            )
+            save_errors.append(f"{dest_type}: {e}")
+
+    if not results:
+        backup.config_hash = config_hash
+        backup.file_size = len(file_bytes)
+        backup.status = BackupStatus.failed
+        backup.error_message = (
+            "Binary backup could not be written to any destination"
+            + (": " + "; ".join(save_errors) if save_errors else "")
+        )
+        db.commit()
+        logger.error("Binary backup for %s failed — no destination accepted the archive", device.hostname)
+        return backup
+
+    dest_label, primary_path = _summarize_results(results)
     manifest = json.dumps({
         "type": archive_type,
-        "path": saved_path,
+        "path": primary_path,
         "files": sorted(file_list),
         "file_count": len(file_list),
     })
 
     backup.config_text = manifest
     backup.config_hash = config_hash
     backup.file_size = len(file_bytes)
-    backup.destination_type = "local"
-    backup.destination_path = saved_path
+    backup.destination_type = dest_label
+    backup.destination_path = primary_path
     backup.status = BackupStatus.success
     db.commit()
 
     logger.info(
-        "Binary backup complete for %s: %d files, %d bytes, hash=%s...",
-        device.hostname, len(file_list), len(file_bytes), config_hash[:12],
+        "Binary backup complete for %s: %d files, %d bytes, hash=%s..., destinations=%s",
+        device.hostname, len(file_list), len(file_bytes), config_hash[:12], dest_label,
     )
     return backup
 
 
-async def _save_binary_local(
-    device: Device,
-    file_bytes: bytes,
-    extension: str,
-    destination_ids: list[int] | None,
-    db: Session,
-) -> str:
-    """Save binary file to the local backup directory, return saved path."""
-    from app.config import get_settings
-    import os
-
-    # Resolve base dir from a configured LOCAL destination only.
-    # Binary (archive) backups are currently local-only; non-local destinations
-    # (SMB, Git) are not supported for binary payloads and are skipped with a warning.
-    base_dir = get_settings().BACKUP_DIR
-    if destination_ids:
-        dests = db.query(Destination).filter(
-            Destination.id.in_(destination_ids),
-            Destination.enabled == True,
-        ).all()
-        local_dest = next((d for d in dests if d.dest_type.value == "local"), None)
-        non_local = [d for d in dests if d.dest_type.value != "local"]
-        if non_local:
-            names = ", ".join(d.dest_type.value for d in non_local)
-            logger.warning(
-                "Binary/archive backups do not yet support non-local destinations (%s) "
-                "for %s — saving locally only.",
-                names, device.hostname,
-            )
-        if local_dest and local_dest.config_json:
-            base_dir = local_dest.config_json.get("path", base_dir)
-
-    safe_hostname = os.path.basename(device.hostname.replace("\\", "/")) or "unknown"
-    device_dir = os.path.join(base_dir, safe_hostname)
-    if not os.path.realpath(device_dir).startswith(os.path.realpath(base_dir)):
-        raise ValueError(f"Invalid hostname for path: {device.hostname}")
-    os.makedirs(device_dir, exist_ok=True)
-
-    timestamp = datetime.now(timezone.utc).strftime("%Y-%m-%d_%H-%M-%S")
-    filename = f"{timestamp}{extension}"
-    filepath = os.path.join(device_dir, filename)
-
-    import asyncio
-    await asyncio.to_thread(_write_binary, filepath, file_bytes)
-
-    # Update latest symlink
-    latest = os.path.join(device_dir, f"latest{extension}")
-    if os.path.islink(latest):
-        os.unlink(latest)
-    try:
-        os.symlink(filepath, latest)
-    except OSError:
-        pass
-
-    logger.info("Saved binary backup to %s (%d bytes)", filepath, len(file_bytes))
-    return filepath
-
-
-def _write_binary(path: str, content: bytes) -> None:
-    with open(path, "wb") as f:
-        f.write(content)
-
-
 async def _handle_text_backup(
     db: Session,
     device: Device,
     backup: Backup,
     config_text: str,
     destination_ids: list[int] | None,
 ) -> Backup:
-    """Save a text config backup (existing flow)."""
+    """Save a text config backup to each selected destination."""
     config_hash = hashlib.sha256(config_text.encode()).hexdigest()
+    destinations = _resolve_destinations(db, destination_ids)
 
-    destinations = []
-    if destination_ids:
-        destinations = db.query(Destination).filter(
-            Destination.id.in_(destination_ids),
-            Destination.enabled == True,
-        ).all()
-
-    if not destinations:
-        destinations = db.query(Destination).filter(
-            Destination.dest_type == "local",
-            Destination.enabled == True,
-        ).all()
-
-    saved_path = None
-    dest_type = "local"
+    results: list[tuple[str, str]] = []
     save_errors: list[str] = []
-
     for dest in destinations:
+        dest_type = dest.dest_type.value
         try:
-            backend = get_destination(dest.dest_type.value)
-            saved_path = await backend.save(
+            backend = get_destination(dest_type)
+            path = await backend.save(
                 hostname=device.hostname,
                 config_text=config_text,
                 config=dest.config_json or {},
             )
-            dest_type = dest.dest_type.value
-            logger.info("Saved backup for %s to %s: %s", device.hostname, dest_type, saved_path)
+            results.append((dest_type, path))
+            logger.info("Saved backup for %s to %s: %s", device.hostname, dest_type, path)
         except Exception as e:
-            logger.error("Failed to save to %s for %s: %s", dest.dest_type.value, device.hostname, e)
-            save_errors.append(f"{dest.dest_type.value}: {e}")
+            logger.error("Failed to save to %s for %s: %s", dest_type, device.hostname, e)
+            save_errors.append(f"{dest_type}: {e}")
 
-    if destinations and saved_path is None:
-        # Every configured destination failed — do not report success
+    if destinations and not results:
         backup.config_text = config_text
         backup.config_hash = config_hash
         backup.file_size = len(config_text)
@@ -246,24 +237,31 @@ async def _handle_text_backup(
         logger.error("Backup for %s failed — no destination saved successfully", device.hostname)
         return backup
 
-    if not destinations:
+    if not results:
+        # No destinations configured at all — last-resort local save so existing
+        # deployments without any enabled destination still produce a file.
         from app.modules.destinations.local import LocalDestination
         local = LocalDestination()
-        saved_path = await local.save(
+        path = await local.save(
             hostname=device.hostname,
             config_text=config_text,
             config={},
         )
+        results.append(("local", path))
 
+    dest_label, primary_path = _summarize_results(results)
     backup.config_text = config_text
     backup.config_hash = config_hash
     backup.file_size = len(config_text)
-    backup.destination_type = dest_type
-    backup.destination_path = saved_path
+    backup.destination_type = dest_label
+    backup.destination_path = primary_path
     backup.status = BackupStatus.success
     db.commit()
 
-    logger.info("Backup complete for %s: %d bytes, hash=%s...", device.hostname, len(config_text), config_hash[:12])
+    logger.info(
+        "Backup complete for %s: %d bytes, hash=%s..., destinations=%s",
+        device.hostname, len(config_text), config_hash[:12], dest_label,
+    )
     return backup
 
 

app/modules/destinations/base.py

@@ -8,6 +8,22 @@ async def save(self, hostname: str, config_text: str, config: dict[str, Any]) ->
         """Save config and return the path/location where it was stored."""
         pass
 
+    async def save_binary(
+        self,
+        hostname: str,
+        data: bytes,
+        extension: str,
+        config: dict[str, Any],
+    ) -> str:
+        """Save a binary archive (e.g. .tar.gz from Proxmox) and return its path.
+
+        Default raises NotImplementedError so callers can skip backends that
+        don't make sense for archives (e.g. committing a tarball to git).
+        """
+        raise NotImplementedError(
+            f"{self.__class__.__name__} does not support binary archive backups"
+        )
+
     @abstractmethod
     async def delete(self, path: str, config: dict[str, Any]) -> None:
         """Delete a backup at the given path."""