From d487f337006dbaafa7e2271411ed9597ee381f1b Mon Sep 17 00:00:00 2001 From: Jonatan <190036+jcastro@users.noreply.github.com> Date: Fri, 16 Oct 2026 20:23:09 +0200 Subject: [PATCH] Allow selecting and deleting guest backup archives in Monitor --- .github/workflows/test-guest-backups.yml | 54 ++++++ AppImage/components/guest-backup-list.tsx | 129 ++++++++++++ AppImage/components/virtual-machines.tsx | 81 +++----- AppImage/lib/guest-backup-archives.ts | 54 ++++++ AppImage/messages/en/common.json | 16 +- AppImage/scripts/build_appimage.sh | 1 + AppImage/scripts/flask_server.py | 29 +++ AppImage/scripts/guest_backup_archives.py | 164 ++++++++++++++++ tests/test_guest_backup_archives.cjs | 69 +++++++ tests/test_guest_backup_archives.py | 226 ++++++++++++++++++++++ 10 files changed, 763 insertions(+), 60 deletions(-) create mode 100644 .github/workflows/test-guest-backups.yml create mode 100644 AppImage/components/guest-backup-list.tsx create mode 100644 AppImage/lib/guest-backup-archives.ts create mode 100644 AppImage/scripts/guest_backup_archives.py create mode 100644 tests/test_guest_backup_archives.cjs create mode 100644 tests/test_guest_backup_archives.py diff --git a/.github/workflows/test-guest-backups.yml b/.github/workflows/test-guest-backups.yml new file mode 100644 index 00000000..60da0353 --- /dev/null +++ b/.github/workflows/test-guest-backups.yml @@ -0,0 +1,54 @@ +name: Guest backup archives + +on: + pull_request: + paths: + - '.github/workflows/test-guest-backups.yml' + - 'AppImage/scripts/guest_backup_archives.py' + - 'AppImage/scripts/flask_server.py' + - 'AppImage/scripts/jwt_middleware.py' + - 'AppImage/scripts/build_appimage.sh' + - 'AppImage/components/guest-backup-list.tsx' + - 'AppImage/components/virtual-machines.tsx' + - 'AppImage/lib/guest-backup-archives.ts' + - 'AppImage/messages/**' + - 'tests/test_guest_backup_archives.*' + push: + branches: [main, develop] + paths: + - 'AppImage/scripts/guest_backup_archives.py' + - 'AppImage/scripts/flask_server.py' + - 'AppImage/scripts/jwt_middleware.py' + - 'AppImage/components/guest-backup-list.tsx' + - 'AppImage/components/virtual-machines.tsx' + - 'AppImage/lib/guest-backup-archives.ts' + - 'tests/test_guest_backup_archives.*' + workflow_dispatch: + +permissions: + contents: read + +jobs: + guest-backups: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + - uses: actions/setup-node@v6 + with: + node-version: '22.14.0' + - name: Install fixture dependencies + run: | + python -m pip install 'Flask==3.1.2' + npm ci --prefix .github/ci/offline-node --ignore-scripts --no-audit --no-fund + - name: Test native deletion, protection, authorization and cache invalidation + run: python -B -m unittest discover -s tests -p test_guest_backup_archives.py -v + - name: Test frontend deletion and completion handling + env: + NODE_PATH: ${{ github.workspace }}/.github/ci/offline-node/node_modules + run: node tests/test_guest_backup_archives.cjs diff --git a/AppImage/components/guest-backup-list.tsx b/AppImage/components/guest-backup-list.tsx new file mode 100644 index 00000000..b98826f2 --- /dev/null +++ b/AppImage/components/guest-backup-list.tsx @@ -0,0 +1,129 @@ +"use client" + +import { useEffect, useRef, useState } from "react" +import { Archive, Clock, Loader2, LockKeyhole, RefreshCw, Trash2 } from "lucide-react" +import { Badge } from "./ui/badge" +import { Button } from "./ui/button" +import { Checkbox } from "./ui/checkbox" +import { Dialog, DialogContent, DialogDescription, DialogFooter, DialogHeader, DialogTitle } from "./ui/dialog" +import { useT } from "@/lib/i18n/provider" +import { deleteGuestBackups, type GuestBackup } from "../lib/guest-backup-archives" + +export function GuestBackupList({ vmid, backups, loading, onRefresh }: { + vmid: number + backups: GuestBackup[] + loading: boolean + onRefresh: () => Promise +}) { + const t = useT() + const [selected, setSelected] = useState([]) + const [confirm, setConfirm] = useState(false) + const [busy, setBusy] = useState(false) + const [refreshing, setRefreshing] = useState(false) + const [message, setMessage] = useState("") + const [error, setError] = useState("") + const mounted = useRef(true) + useEffect(() => { + mounted.current = true + return () => { mounted.current = false } + }, []) + // A fresh list can remove a selection or reveal a newly protected archive. + useEffect(() => { + setSelected(previous => previous.filter(volid => backups.some(backup => backup.volid === volid && !backup.protected))) + }, [backups]) + + const chosen = backups.filter(backup => selected.includes(backup.volid) && !backup.protected) + const selectable = backups.filter(backup => !backup.protected) + const disabled = loading || busy || refreshing + + async function refresh() { + setRefreshing(true) + setError("") + try { await onRefresh() } catch { if (mounted.current) setError(t("vmLxc.backups.refreshFailed")) } + finally { if (mounted.current) setRefreshing(false) } + } + + async function remove() { + if (!chosen.length || busy) return + setBusy(true) + setError("") + setMessage("") + try { + const results = await deleteGuestBackups(vmid, chosen) + if (!mounted.current) return + const deleted = results.filter(result => result.status === "deleted").length + const unconfirmed = results.length - deleted + setMessage(t("vmLxc.backups.deletedCount", { count: deleted })) + if (unconfirmed) setError(t("vmLxc.backups.unconfirmedCount", { count: unconfirmed })) + setSelected([]) + } catch (error) { + if (mounted.current) setError(`${t("vmLxc.backups.deleteFailed")} ${error instanceof Error ? error.message : ""}`) + } finally { + // A timeout can hide a successful native task. Always bypass both caches. + try { await onRefresh() } catch { if (mounted.current) setError(t("vmLxc.backups.refreshFailed")) } + if (mounted.current) { setBusy(false); setConfirm(false) } + } + } + + return <> +
+
+ {t("vmLxc.backups.available")} + {backups.length} +
+
+ + +
+
+ {message &&

{message}

} + {error &&

{error}

} + {loading ?
+ {t("vmLxc.backups.loading")} +
: backups.length === 0 ?
+ {t("vmLxc.backups.empty")} + {t("vmLxc.backups.emptyHint")} +
: <> + +
+ {backups.map(backup => )} +
+ } + { if (!busy) setConfirm(value) }}> + { if (busy) event.preventDefault() }} onPointerDownOutside={event => { if (busy) event.preventDefault() }}> + + {t("vmLxc.backups.confirmTitle", { count: chosen.length })} + {t("vmLxc.backups.confirmDescription", { vmid })} + +
    + {chosen.map(backup =>
  • {backup.volid}
    • )} +
+ {busy &&

{t("vmLxc.backups.deleting")}

} + + + + + + + +} diff --git a/AppImage/components/virtual-machines.tsx b/AppImage/components/virtual-machines.tsx index 87d1bf07..70250416 100644 --- a/AppImage/components/virtual-machines.tsx +++ b/AppImage/components/virtual-machines.tsx @@ -23,6 +23,8 @@ import { MetricsView } from "./metrics-dialog" import { LxcTerminalModal } from "./lxc-terminal-modal" import { ScriptTerminalModal } from "./script-terminal-modal" import { LxcAppPanel, ThemeAwareLogo, ociImageLabel } from "./lxc-app-panel" +import { GuestBackupList } from "./guest-backup-list" +import type { GuestBackup } from "../lib/guest-backup-archives" import { OciConsoleLogPanel } from "./oci-console-log-panel" import { AppUpdaterEditor, type AppUpdateMethod } from "./app-updater-editor" import { formatStorage } from "../lib/utils" @@ -382,16 +384,7 @@ interface BackupStorage { avail_human?: string } -interface VMBackup { - volid: string - storage: string - type: string - size: number - size_human: string - timestamp: number - date: string - notes?: string -} +type VMBackup = GuestBackup // Sprint 13.29: shape returned by /api/lxc//mount-points. Lives // next to VMBackup since both are LXC-modal data structures. @@ -1598,7 +1591,7 @@ export function VirtualMachines() { } } - const fetchVmBackups = async (vmid: number) => { + const fetchVmBackups = async (vmid: number, force = false) => { // Stale-while-revalidate with a 6-hour freshness gate. // // 1. If we already have backups (bulk hydration or a previous @@ -1615,15 +1608,16 @@ export function VirtualMachines() { // 3. Loading spinner only on the true first load. const GATE_MS = 6 * 60 * 60 * 1000 const seed = vmModalCacheRef.current.backups.get(vmid) - const isStale = !seed || (Date.now() - seed.fetchedAt) > GATE_MS + const isStale = force || !seed || (Date.now() - seed.fetchedAt) > GATE_MS if (!seed) setLoadingBackups(true) try { const url = isStale ? `/api/vms/${vmid}/backups?fresh=1` : `/api/vms/${vmid}/backups` - const response = await fetchApi<{ backups?: VMBackup[] }>(url) + const response = await fetchApi<{ backups?: VMBackup[]; error?: string }>(url) + if (response.error || !Array.isArray(response.backups)) throw new Error(response.error || "Backup inventory unavailable") if (response.backups) { - setVmBackups(response.backups) + if (selectedVMRef.current?.vmid === vmid) setVmBackups(response.backups) vmModalCacheRef.current.backups.set(vmid, { backups: response.backups, fetchedAt: Date.now() }) } } catch (error) { @@ -1631,9 +1625,10 @@ export function VirtualMachines() { // Only clear the visible list if we had nothing to show // in the first place — a transient network hiccup must not // wipe the stale-but-useful view the user is looking at. - if (!seed) setVmBackups([]) + if (!seed && selectedVMRef.current?.vmid === vmid) setVmBackups([]) + if (force) throw error } finally { - if (!seed) setLoadingBackups(false) + if (!seed && selectedVMRef.current?.vmid === vmid) setLoadingBackups(false) } } @@ -7436,7 +7431,7 @@ const handleDownloadLogs = async (vmid: number, vmName: string) => { )} {/* Backups Tab */} - {activeModalTab === "backups" && ( + {activeModalTab === "backups" && selectedVM && ( /* Backups tab layout: the Card fills the whole tab body so the backup list can scroll inside its bordered card instead of leaving empty space @@ -7471,48 +7466,16 @@ const handleDownloadLogs = async (vmid: number, vmName: string) => { {/* Divider */}
- {/* Backup List */} -
- {t("vmLxc.backups.available")} - {vmBackups.length} -
- - {loadingBackups ? ( -
- - {t("vmLxc.backups.loading")} -
- ) : vmBackups.length === 0 ? ( -
- - {t("vmLxc.backups.empty")} - {t("vmLxc.backups.emptyHint")} -
- ) : ( -
- {vmBackups.map((backup, index) => ( -
-
-
- - {backup.date} - - {backup.storage} - -
- - {backup.size_human} - -
- ))} -
- )} + { + vmModalCacheRef.current.backups.delete(selectedVM.vmid) + await fetchVmBackups(selectedVM.vmid, true) + }} + />
diff --git a/AppImage/lib/guest-backup-archives.ts b/AppImage/lib/guest-backup-archives.ts new file mode 100644 index 00000000..36b8dffa --- /dev/null +++ b/AppImage/lib/guest-backup-archives.ts @@ -0,0 +1,54 @@ +import { fetchApi } from './api-config' + +export interface GuestBackup { + volid: string + storage: string + type: string + size: number + size_human: string + timestamp: number + date: string + notes?: string + protected?: boolean +} + +export interface BackupDeletionResult { + volid: string + status: 'deleted' | 'pending' | 'unconfirmed' + task?: string + error?: string +} + +// Exposed separately so callers can invalidate their inventory even after a +// lost POST response. Never retry the destructive request automatically. +export async function deleteGuestBackups(vmid: number, backups: GuestBackup[]): Promise { + const response = await fetchApi<{ results: BackupDeletionResult[] }>(`/api/vms/${vmid}/backups/delete`, { + method: 'POST', + body: JSON.stringify({ confirm: true, backups: backups.map(({ volid, storage }) => ({ volid, storage })) }), + signal: AbortSignal.timeout(120_000), + }) + if (!Array.isArray(response.results) || response.results.length !== backups.length || + response.results.some((result, index) => result.volid !== backups[index].volid || + !['deleted', 'pending', 'unconfirmed'].includes(result.status))) throw new Error('deletionUnconfirmed') + + return Promise.all(response.results.map(async result => { + if (result.status !== 'pending' || !result.task) return result + const deadline = Date.now() + 120_000 + let failures = 0 + while (Date.now() < deadline) { + await new Promise(resolve => setTimeout(resolve, 1500)) + try { + const status = await fetchApi( + `/api/vms/${vmid}/backups/delete-status?task=${encodeURIComponent(result.task!)}`, + { signal: AbortSignal.timeout(10_000) }, + ) + if (status.volid !== result.volid || !['deleted', 'pending', 'unconfirmed'].includes(status.status)) break + if (status.status !== 'pending') return status + failures = 0 + } catch { + if (++failures >= 3) break + } + } + return { ...result, status: 'unconfirmed', error: 'deletionUnconfirmed' } + })) +} diff --git a/AppImage/messages/en/common.json b/AppImage/messages/en/common.json index ca7e536d..014455ed 100644 --- a/AppImage/messages/en/common.json +++ b/AppImage/messages/en/common.json @@ -1131,7 +1131,21 @@ "available": "Available backups", "loading": "Loading backups...", "empty": "No backups found", - "emptyHint": "Create your first backup using the button above" + "emptyHint": "Create your first backup using the button above", + "refresh": "Refresh", + "refreshFailed": "Could not refresh backups. Refresh again to confirm their current state.", + "deleteSelected": "Delete selected ({count})", + "deletedCount": "{count} backup(s) deleted.", + "unconfirmedCount": "Deletion of {count} backup(s) was not confirmed. Refresh the list and check Proxmox Tasks before trying again.", + "deleteFailed": "Deletion could not be confirmed. Refresh the list and check Proxmox Tasks before trying again.", + "selectHint": "Select unprotected backups (up to 20 per batch)", + "selectBackup": "Select backup", + "protected": "Protected", + "confirmTitle": "Delete {count} backup(s)?", + "confirmDescription": "These backup archives for VM/CT {vmid} will be permanently deleted. Protected backups are excluded. This does not delete the guest.", + "deleting": "Waiting for Proxmox to finish deleting the selected archives…", + "cancel": "Cancel", + "confirmDelete": "Delete backups" }, "updatesPanel": { "pendingTitle": "Pending package updates", diff --git a/AppImage/scripts/build_appimage.sh b/AppImage/scripts/build_appimage.sh index b0df1302..fb7d8281 100755 --- a/AppImage/scripts/build_appimage.sh +++ b/AppImage/scripts/build_appimage.sh @@ -181,6 +181,7 @@ done cp "$SCRIPT_DIR/oci_manager.py" "$APP_DIR/usr/bin/" 2>/dev/null || echo "⚠️ oci_manager.py not found" cp "$SCRIPT_DIR/flask_oci_routes.py" "$APP_DIR/usr/bin/" 2>/dev/null || echo "⚠️ flask_oci_routes.py not found" cp "$SCRIPT_DIR/flask_vm_backup_routes.py" "$APP_DIR/usr/bin/" 2>/dev/null || echo "⚠️ flask_vm_backup_routes.py not found" +cp "$SCRIPT_DIR/guest_backup_archives.py" "$APP_DIR/usr/bin/" cp "$SCRIPT_DIR/flask_audit_routes.py" "$APP_DIR/usr/bin/" 2>/dev/null || echo "⚠️ flask_audit_routes.py not found" cp "$SCRIPT_DIR/audit_store.py" "$APP_DIR/usr/bin/" 2>/dev/null || echo "⚠️ audit_store.py not found" cp "$SCRIPT_DIR/audit_checks.py" "$APP_DIR/usr/bin/" 2>/dev/null || echo "⚠️ audit_checks.py not found" diff --git a/AppImage/scripts/flask_server.py b/AppImage/scripts/flask_server.py index 13da0971..2a6ae11d 100644 --- a/AppImage/scripts/flask_server.py +++ b/AppImage/scripts/flask_server.py @@ -95,6 +95,8 @@ from flask_notification_routes import notification_bp # noqa: E402 from flask_oci_routes import oci_bp # noqa: E402 from flask_audit_routes import audit_bp # noqa: E402 from flask_vm_backup_routes import vm_backup_bp # noqa: E402 +from guest_backup_archives import (BackupArchiveError, delete_guest_backups, + guest_backup_deletion_status) # noqa: E402 from notification_manager import notification_manager, resolve_notification_hostname # noqa: E402 import post_install_versions # noqa: E402 — Sprint 12A: detect post-install function updates from jwt_middleware import require_auth, require_auth_or_ticket, require_admin_scope # noqa: E402 @@ -13303,6 +13305,32 @@ def api_create_backup(vmid): except Exception as e: return jsonify({'error': str(e)}), 500 +@app.route('/api/vms//backups/delete', methods=['POST']) +@require_admin_scope +def api_delete_vm_backups(vmid): + """Delete an explicitly confirmed selection, revalidated by Proxmox.""" + try: + return jsonify(delete_guest_backups(vmid, request.get_json(silent=True))) + except BackupArchiveError as error: + return jsonify({'error': str(error)}), error.code + finally: + # Native deletion may have started even if the pvesh response was lost. + _vm_cache_invalidate(vmid, _vm_backups_cache) + + +@app.route('/api/vms//backups/delete-status', methods=['GET']) +@require_admin_scope +def api_vm_backup_deletion_status(vmid): + """Only poll deletion tasks started here for this guest.""" + try: + result = guest_backup_deletion_status(vmid, request.args.get('task')) + if result['status'] != 'pending': + _vm_cache_invalidate(vmid, _vm_backups_cache) + return jsonify(result) + except BackupArchiveError as error: + return jsonify({'error': str(error)}), error.code + + @app.route('/api/vms//backups', methods=['GET']) @require_auth def api_vm_backups(vmid): @@ -13383,6 +13411,7 @@ def api_vm_backups(vmid): 'timestamp': ctime, 'date': datetime.fromtimestamp(ctime).strftime('%Y-%m-%d %H:%M') if ctime else '', 'notes': item.get('notes', ''), + 'protected': item.get('protected', False) not in (False, 0, '0', '', None), }) except Exception: pass diff --git a/AppImage/scripts/guest_backup_archives.py b/AppImage/scripts/guest_backup_archives.py new file mode 100644 index 00000000..c95077fc --- /dev/null +++ b/AppImage/scripts/guest_backup_archives.py @@ -0,0 +1,164 @@ +# ========================================================== +# ProxMenux Monitor - Guest backup archives +# Author : Jonatan +# GitHub : github.com/jcastro +# Maintainer : MacRimi +# Copyright : (c) 2026 MacRimi & contributors +# License : (GPL-3.0) (https://github.com/MacRimi/ProxMenux/blob/main/LICENSE) +# Version : 1.0 +# Last Updated: 16/10/2026 +# ========================================================== +# Description: +# Deletes explicitly selected VM/CT backups through the native Proxmox +# storage API after a fresh inventory/protection check. Tracks native +# deletion tasks so an accepted operation is not mistaken for completion. +# No filesystem paths or cached inventory authorize deletion. +# ========================================================== + +import json +import re +import socket +import subprocess +import threading +import time +from concurrent.futures import ThreadPoolExecutor + + +MAX_SELECTION = 20 +STORAGE_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._-]{0,63}") +NODE_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9-]{0,62}") +_tasks = {} +_lock = threading.Lock() + + +class BackupArchiveError(Exception): + def __init__(self, message, code=400): + super().__init__(message) + self.code = code + + +def _pvesh(args, timeout=20): + try: + result = subprocess.run(["pvesh", *args, "--output-format", "json"], + capture_output=True, text=True, timeout=timeout) + except (OSError, subprocess.TimeoutExpired) as error: + raise BackupArchiveError("Proxmox command unavailable or timed out; outcome unconfirmed", 502) from error + if result.returncode: + raise BackupArchiveError((result.stderr or "Proxmox command failed").strip()[:1000], 502) + try: + # pvesh can print no bytes for the API's successful null result. + return json.loads(result.stdout.strip() or "null") + except (ValueError, TypeError) as error: + raise BackupArchiveError("Invalid Proxmox response; outcome unconfirmed", 502) from error + + +def _node(): + # Same local storage/node scope as the guest's existing backup list. + node = socket.gethostname().split(".")[0] + if not NODE_RE.fullmatch(node): + raise BackupArchiveError("Invalid local node name", 502) + return node + + +def _selection(vmid, data): + if not 100 <= vmid <= 999999999: + raise BackupArchiveError("Invalid VM/CT ID") + if not isinstance(data, dict) or data.get("confirm") is not True: + raise BackupArchiveError("Explicit confirmation is required") + selected = data.get("backups") + if not isinstance(selected, list) or not 1 <= len(selected) <= MAX_SELECTION: + raise BackupArchiveError(f"Select between 1 and {MAX_SELECTION} backups") + seen = set() + for item in selected: + if not isinstance(item, dict): + raise BackupArchiveError("Invalid backup selection") + storage, volid = item.get("storage"), item.get("volid") + if not isinstance(storage, str) or not STORAGE_RE.fullmatch(storage): + raise BackupArchiveError("Invalid backup storage") + if (not isinstance(volid, str) or not volid.startswith(storage + ":") + or len(volid) > 512 or not re.fullmatch(r"[A-Za-z0-9_:./+@-]+", volid) + or any(part in ("", ".", "..") for part in volid.split(":", 1)[1].split("/"))): + raise BackupArchiveError("Invalid backup volume ID") + if volid in seen: + raise BackupArchiveError("Duplicate backup selection") + seen.add(volid) + return [{"storage": item["storage"], "volid": item["volid"]} for item in selected] + + +def _protected(value): + # Fail closed for an unexpected protection value, including the text 'true'. + return value not in (None, False, 0, "0", "") + + +def _remember_task(task, vmid, node, volid): + # These are server-returned task IDs, never arbitrary client paths. + if (not isinstance(task, str) or not task.startswith(f"UPID:{node}:") + or ":imgdel:" not in task or "/" in task or len(task) > 1024): + raise BackupArchiveError("Unexpected deletion task; outcome unconfirmed", 502) + with _lock: + now = time.monotonic() + expired = [key for key, entry in _tasks.items() if now - entry["created"] > 3600] + for key in expired: + del _tasks[key] + # Bound even failed/unpolled jobs; expiry does not cancel a PVE task. + if len(_tasks) >= 512: + del _tasks[min(_tasks, key=lambda key: _tasks[key]["created"])] + _tasks[task] = {"vmid": vmid, "node": node, "volid": volid, "created": now} + + +def delete_guest_backups(vmid, data): + selected = _selection(vmid, data) + node = _node() + inventory = {} + # Preflight the entire batch before deleting anything. An unavailable + # storage or a protected/stale/cross-guest selection fails the whole batch. + for storage in sorted({item["storage"] for item in selected}): + entries = _pvesh(["get", f"/nodes/{node}/storage/{storage}/content", + "--content", "backup", "--vmid", str(vmid)]) + if not isinstance(entries, list): + raise BackupArchiveError("Invalid backup inventory", 502) + for entry in entries: + if (isinstance(entry, dict) and entry.get("content") == "backup" + and isinstance(entry.get("volid"), str)): + inventory[(storage, entry.get("volid"))] = entry + for item in selected: + entry = inventory.get((item["storage"], item["volid"])) + if entry is None or str(entry.get("vmid")) != str(vmid): + raise BackupArchiveError("Backup no longer exists or does not belong to this VM/CT", 409) + if _protected(entry.get("protected")): + raise BackupArchiveError("Protected backups cannot be deleted", 409) + + def remove(item): + try: + # Proxmox also enforces protection at deletion time. It removes + # associated notes/log files and returns null only after success. + task = _pvesh(["delete", f'/nodes/{node}/storage/{item["storage"]}/content/{item["volid"]}', + "--delay", "1"]) + if task is None: + return {**item, "status": "deleted"} + _remember_task(task, vmid, node, item["volid"]) + return {**item, "status": "pending", "task": task} + except BackupArchiveError as error: + return {**item, "status": "unconfirmed", "error": str(error)} + + with ThreadPoolExecutor(max_workers=min(4, len(selected)), thread_name_prefix="backup-delete") as pool: + results = list(pool.map(remove, selected)) + return {"results": results} + + +def guest_backup_deletion_status(vmid, task): + with _lock: + entry = _tasks.get(task) if isinstance(task, str) else None + if not entry or entry["vmid"] != vmid or time.monotonic() - entry["created"] > 3600: + raise BackupArchiveError("Deletion task unknown or expired; refresh the backup list", 409) + entry = entry.copy() + status = _pvesh(["get", f'/nodes/{entry["node"]}/tasks/{task}/status']) + if not isinstance(status, dict) or status.get("status") not in ("running", "stopped"): + raise BackupArchiveError("Invalid deletion task status", 502) + result = {"volid": entry["volid"], "status": "pending"} + if status["status"] == "stopped": + if status.get("exitstatus") == "OK": + result["status"] = "deleted" + else: + result.update(status="unconfirmed", error=status.get("exitstatus") or "Deletion outcome unconfirmed") + return result diff --git a/tests/test_guest_backup_archives.cjs b/tests/test_guest_backup_archives.cjs new file mode 100644 index 00000000..6544c9df --- /dev/null +++ b/tests/test_guest_backup_archives.cjs @@ -0,0 +1,69 @@ +// Run the shipped deletion client with only its authenticated transport replaced. +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const vm = require('node:vm'); +const ts = require('typescript'); +const source = fs.readFileSync('AppImage/lib/guest-backup-archives.ts', 'utf8'); +const js = ts.transpileModule(source, { compilerOptions: { module: ts.ModuleKind.CommonJS, target: ts.ScriptTarget.ES2020 } }).outputText; +const backup = { volid: 'local:backup/vzdump-lxc-100-fixture.tar.zst', storage: 'local' }; + +async function run(responses) { + const calls = []; + const context = { exports: {}, AbortSignal, Date, setTimeout: callback => callback(), require: name => { + assert.equal(name, './api-config'); + return { fetchApi: async (path, options) => { + calls.push({ path, options }); + assert.ok(responses.length, `unexpected call: ${path}`); + const response = responses.shift(); + if (response instanceof Error) throw response; + return response; + } }; + } }; + vm.runInNewContext(js, context); + let results, error; + try { results = JSON.parse(JSON.stringify(await context.exports.deleteGuestBackups(100, [backup]))); } + catch (caught) { error = caught; } + return { calls, results, error }; +} + +(async () => { + const success = await run([{ results: [{ volid: backup.volid, status: 'deleted' }] }]); + assert.equal(success.results[0].status, 'deleted'); + assert.deepEqual(JSON.parse(success.calls[0].options.body), { confirm: true, backups: [backup] }); + assert.equal(success.calls[0].options.method, 'POST'); + assert.equal(success.calls.length, 1); + const task = 'UPID:pve1:01:02:03:imgdel:100:root@pam:'; + const pending = { results: [{ volid: backup.volid, status: 'pending', task }] }; + const completed = await run([pending, { volid: backup.volid, status: 'pending' }, { volid: backup.volid, status: 'deleted' }]); + assert.equal(completed.results[0].status, 'deleted'); + assert.equal(completed.calls.length, 3); + assert.equal(completed.calls[1].path, `/api/vms/100/backups/delete-status?task=${encodeURIComponent(task)}`); + assert.equal(completed.calls.filter(call => call.options.method === 'POST').length, 1); + const failed = await run([pending, { volid: backup.volid, status: 'unconfirmed', error: 'protected' }]); + assert.equal(failed.results[0].status, 'unconfirmed'); + const lostPost = await run([new Error('network failure')]); + assert.equal(lostPost.error.message, 'network failure'); + assert.equal(lostPost.calls.length, 1, 'never retry destructive POST'); + const lostStatus = await run([pending, new Error('timeout'), new Error('timeout'), new Error('timeout')]); + assert.equal(lostStatus.results[0].status, 'unconfirmed'); + assert.equal(lostStatus.calls.length, 4, 'bounded read failures'); + for (const response of [{ results: [] }, { results: [{ volid: 'other', status: 'deleted' }] }, + { results: [{ volid: backup.volid, status: 'accepted' }] }]) { + assert.equal((await run([response])).error.message, 'deletionUnconfirmed'); + } + const foreignStatus = await run([pending, { volid: 'other', status: 'deleted' }]); + assert.equal(foreignStatus.results[0].status, 'unconfirmed'); + + const component = fs.readFileSync('AppImage/components/guest-backup-list.tsx', 'utf8'); + const english = JSON.parse(fs.readFileSync('AppImage/messages/en/common.json')).vmLxc.backups; + for (const [, key] of component.matchAll(/t\("vmLxc\.backups\.([A-Za-z]+)"/g)) { + assert.ok(typeof english[key] === 'string' && english[key].length, `English translation: ${key}`); + } + assert.ok(component.includes('disabled={disabled || !!backup.protected')); + assert.ok(component.includes('!backup.protected')); + assert.ok(component.includes('DialogDescription')); + const parent = fs.readFileSync('AppImage/components/virtual-machines.tsx', 'utf8'); + assert.ok(parent.includes(' { console.error(error); process.exitCode = 1; }); diff --git a/tests/test_guest_backup_archives.py b/tests/test_guest_backup_archives.py new file mode 100644 index 00000000..5d20b328 --- /dev/null +++ b/tests/test_guest_backup_archives.py @@ -0,0 +1,226 @@ +"""Guest archive deletion contract. No real Proxmox commands or backups.""" + +import ast +import copy +import importlib.util +import json +from pathlib import Path +import subprocess +import sys +import types +import unittest +from unittest.mock import Mock, patch + +from flask import Flask, jsonify, request + +ROOT = Path(__file__).resolve().parents[1] +SCRIPTS = ROOT / "AppImage/scripts" +sys.path.insert(0, str(SCRIPTS)) +import guest_backup_archives as archives + +VOLUME = "local:backup/vzdump-lxc-100-2026_10_16-12_00_00.tar.zst" +OTHER = VOLUME.replace("12_00_00", "13_00_00") +TASK = "UPID:pve1:00000001:00000002:00000003:imgdel:100:root@pam:" + + +class DeletionTests(unittest.TestCase): + def setUp(self): + self.node = patch.object(archives, "_node", return_value="pve1") + self.node.start() + self.addCleanup(self.node.stop) + self.inventory = [{"volid": VOLUME, "vmid": 100, "content": "backup", "protected": False}] + self.calls = [] + self.response = None + archives._tasks.clear() + + def run_pvesh(self, args, **kwargs): + self.calls.append(args) + if args[0] == "get" and args[1].endswith("/content"): + return copy.deepcopy(self.inventory) + if args[0] == "delete": + if isinstance(self.response, Exception): + raise self.response + return self.response + return {"status": "stopped", "exitstatus": "OK"} + + def delete(self, volumes=(VOLUME,), **kwargs): + data = {"confirm": True, "backups": [{"storage": v.split(":")[0], "volid": v} for v in volumes]} + data.update(kwargs) + with patch.object(archives, "_pvesh", side_effect=self.run_pvesh): + return archives.delete_guest_backups(100, data) + + def test_success_uses_guest_filtered_native_inventory_and_delete(self): + self.assertEqual(self.delete()["results"], [{"storage": "local", "volid": VOLUME, "status": "deleted"}]) + self.assertEqual(self.calls, [ + ["get", "/nodes/pve1/storage/local/content", "--content", "backup", "--vmid", "100"], + ["delete", f"/nodes/pve1/storage/local/content/{VOLUME}", "--delay", "1"], + ]) + + def test_bad_payloads_and_paths_never_reach_proxmox(self): + payloads = [None, [], {}, {"confirm": 1}, {"confirm": True, "backups": []}, + {"confirm": True, "backups": [None]}, {"confirm": True, "backups": [VOLUME]}, + {"confirm": True, "backups": [{"storage": "local", "volid": VOLUME}] * 21}] + for volid in ("local:../etc/passwd", "local:backup/../x", "local:/etc/pve/storage.cfg", "local:backup//x", + "local:backup/x%2fy", "local:backup/x\n--force", "local:backup/x;touch /tmp/x", "other:backup/x"): + payloads.append({"confirm": True, "backups": [{"storage": "local", "volid": volid}]}) + for storage in ("../local", "-x", "local\n", 2, None): + payloads.append({"confirm": True, "backups": [{"storage": storage, "volid": VOLUME}]}) + with patch.object(archives, "_pvesh") as run: + for data in payloads: + with self.subTest(data=data), self.assertRaises(archives.BackupArchiveError): + archives.delete_guest_backups(100, data) + for vmid in (0, 99, 1000000000): + with self.assertRaises(archives.BackupArchiveError): + archives.delete_guest_backups(vmid, {"confirm": True, "backups": [{"storage": "local", "volid": VOLUME}]}) + run.assert_not_called() + + def test_duplicate_selection_rejected_before_commands(self): + with self.assertRaises(archives.BackupArchiveError): + self.delete((VOLUME, VOLUME)) + self.assertEqual(self.calls, []) + + def test_newly_protected_archives_fail_the_whole_batch(self): + self.inventory.append({"volid": OTHER, "vmid": 100, "content": "backup", "protected": True}) + for protection in (True, 1, "1", "true", "unexpected"): + self.inventory[-1]["protected"] = protection + with self.subTest(protection=protection), self.assertRaises(archives.BackupArchiveError) as error: + self.delete((VOLUME, OTHER)) + self.assertEqual(error.exception.code, 409) + self.assertFalse(any(call[0] == "delete" for call in self.calls)) + + def test_missing_cross_guest_and_non_backup_volumes_are_never_deleted(self): + for entries in ([], [{"volid": VOLUME, "vmid": 101, "content": "backup"}], + [{"volid": VOLUME, "content": "backup"}], + [{"volid": VOLUME, "vmid": 100, "content": "images"}]): + self.inventory = entries + with self.subTest(entries=entries), self.assertRaises(archives.BackupArchiveError): + self.delete() + self.assertFalse(any(call[0] == "delete" for call in self.calls)) + + def test_storage_failure_prevents_every_delete(self): + def unavailable(args): + self.assertEqual(args[0], "get") + raise archives.BackupArchiveError("offline", 502) + with patch.object(archives, "_pvesh", side_effect=unavailable), self.assertRaises(archives.BackupArchiveError): + archives.delete_guest_backups(100, {"confirm": True, "backups": [ + {"volid": VOLUME, "storage": "local"}, {"volid": "nas:backup/x", "storage": "nas"}]}) + + def test_partial_native_errors_remain_unconfirmed(self): + self.inventory.append({"volid": OTHER, "vmid": 100, "content": "backup"}) + def native(args): + if args[0] == "delete" and OTHER in args[1]: + raise archives.BackupArchiveError("backup is protected", 502) + return self.run_pvesh(args) + with patch.object(archives, "_pvesh", side_effect=native): + result = archives.delete_guest_backups(100, {"confirm": True, "backups": [ + {"volid": v, "storage": "local"} for v in (VOLUME, OTHER)]}) + self.assertEqual([item["status"] for item in result["results"]], ["deleted", "unconfirmed"]) + self.assertIn("protected", result["results"][1]["error"]) + + def test_native_task_is_not_a_success_until_exit_ok(self): + self.response = TASK + result = self.delete()["results"][0] + self.assertEqual(result["status"], "pending") + for native, expected in (({"status": "running"}, "pending"), + ({"status": "stopped", "exitstatus": "OK"}, "deleted"), + ({"status": "stopped", "exitstatus": "cannot delete protected backup"}, "unconfirmed"), + ({"status": "stopped"}, "unconfirmed")): + with patch.object(archives, "_pvesh", return_value=native) as run: + answer = archives.guest_backup_deletion_status(100, TASK) + self.assertEqual(answer["status"], expected) + run.assert_called_once_with(["get", f"/nodes/pve1/tasks/{TASK}/status"]) + + def test_poll_cannot_read_arbitrary_tasks_or_other_guests(self): + self.response = TASK + self.delete() + with patch.object(archives, "_pvesh") as run: + for vmid, task in ((101, TASK), (100, "../../status"), (100, None), (100, TASK + "x")): + with self.assertRaises(archives.BackupArchiveError): + archives.guest_backup_deletion_status(vmid, task) + archives._tasks[TASK]["created"] -= 3601 + with self.assertRaises(archives.BackupArchiveError): + archives.guest_backup_deletion_status(100, TASK) + run.assert_not_called() + + def test_invalid_native_task_answers_never_claim_success(self): + for response in ("", {}, [TASK], TASK.replace("imgdel", "vzdump"), TASK.replace("pve1", "pve2")): + self.response = response + with self.subTest(response=response): + self.assertEqual(self.delete()["results"][0]["status"], "unconfirmed") + + def test_pvesh_uses_argv_and_handles_failure_and_timeout(self): + with patch.object(archives.subprocess, "run", return_value=types.SimpleNamespace(returncode=0, stdout="null")) as run: + self.assertIsNone(archives._pvesh(["delete", "/native/path"])) + self.assertEqual(run.call_args.args[0], ["pvesh", "delete", "/native/path", "--output-format", "json"]) + self.assertNotIn("shell", run.call_args.kwargs) + with patch.object(archives.subprocess, "run", return_value=types.SimpleNamespace(returncode=0, stdout="")): + self.assertIsNone(archives._pvesh(["delete", "/native/path"])) + for response in (types.SimpleNamespace(returncode=1, stderr="protected", stdout=""), + types.SimpleNamespace(returncode=0, stdout="not JSON")): + with patch.object(archives.subprocess, "run", return_value=response), self.assertRaises(archives.BackupArchiveError): + archives._pvesh(["delete", "/native/path"]) + for error in (FileNotFoundError(), subprocess.TimeoutExpired("pvesh", 20)): + with patch.object(archives.subprocess, "run", side_effect=error), self.assertRaises(archives.BackupArchiveError): + archives._pvesh(["delete", "/native/path"]) + + +class RouteTests(unittest.TestCase): + """Compile the real route bodies/decorators without starting host services.""" + def setUp(self): + auth = types.ModuleType("auth_manager") + auth.load_auth_config = lambda: {"enabled": True} + auth.verify_token = lambda token: "tester" if token == "admin" else None + auth.verify_token_full = lambda token: ("tester", "full_admin" if token == "admin" else "read_only") + spec = importlib.util.spec_from_file_location("guest_backup_test_middleware", SCRIPTS / "jwt_middleware.py") + middleware = importlib.util.module_from_spec(spec) + with patch.dict(sys.modules, auth_manager=auth): + spec.loader.exec_module(middleware) + self.app = Flask(__name__) + self.service = Mock(return_value={"results": []}) + self.status = Mock(return_value={"volid": VOLUME, "status": "deleted"}) + self.invalidate = Mock() + self.namespace = {"app": self.app, "require_admin_scope": middleware.require_admin_scope, + "request": request, "jsonify": jsonify, "BackupArchiveError": archives.BackupArchiveError, + "delete_guest_backups": self.service, "guest_backup_deletion_status": self.status, + "_vm_cache_invalidate": self.invalidate, "_vm_backups_cache": {}} + source = ast.parse((SCRIPTS / "flask_server.py").read_text()) + wanted = {"api_delete_vm_backups", "api_vm_backup_deletion_status"} + routes = [node for node in source.body if isinstance(node, ast.FunctionDef) and node.name in wanted] + self.assertEqual(len(routes), 2) + exec(compile(ast.Module(body=routes, type_ignores=[]), "flask_server.py", "exec"), self.namespace) + self.client = self.app.test_client() + + def test_both_routes_reject_unauthenticated_and_read_only_requests(self): + for headers, status in (({}, 401), ({"Authorization": "Bearer reader"}, 403)): + self.assertEqual(self.client.post("/api/vms/100/backups/delete", json={}, headers=headers).status_code, status) + self.assertEqual(self.client.get("/api/vms/100/backups/delete-status?task=x", headers=headers).status_code, status) + self.service.assert_not_called() + self.status.assert_not_called() + self.invalidate.assert_not_called() + + def test_admin_route_passes_payload_and_invalidates_even_after_errors(self): + data = {"confirm": True, "backups": [{"storage": "local", "volid": VOLUME}]} + headers = {"Authorization": "Bearer admin"} + self.assertEqual(self.client.post("/api/vms/100/backups/delete", json=data, headers=headers).status_code, 200) + self.service.assert_called_once_with(100, data) + self.invalidate.assert_called_once_with(100, {}) + self.service.side_effect = archives.BackupArchiveError("protected", 409) + self.assertEqual(self.client.post("/api/vms/100/backups/delete", json=data, headers=headers).status_code, 409) + self.assertEqual(self.invalidate.call_count, 2) + + def test_poll_invalidates_only_after_completion(self): + headers = {"Authorization": "Bearer admin"} + self.status.return_value = {"status": "pending"} + self.client.get("/api/vms/100/backups/delete-status?task=x", headers=headers) + self.invalidate.assert_not_called() + self.status.return_value = {"status": "deleted"} + self.client.get("/api/vms/100/backups/delete-status?task=x", headers=headers) + self.status.assert_called_with(100, "x") + self.invalidate.assert_called_once_with(100, {}) + + def test_runtime_module_is_shipped(self): + self.assertIn('cp "$SCRIPT_DIR/guest_backup_archives.py" "$APP_DIR/usr/bin/"', (SCRIPTS / "build_appimage.sh").read_text()) + + +if __name__ == "__main__": + unittest.main()