From 1920dc2937909c17153ada2b079ab5dafa143710 Mon Sep 17 00:00:00 2001 From: MacRimi Date: Sat, 31 Oct 2026 10:19:59 +0100 Subject: [PATCH] fix(backup): check the schedule of a host backup job before saving it --- .../tests/test_backup_schedule_validation.py | 44 +++++++++++++++++++ lang/es.json | 2 + lang/sk.json | 2 + scripts/backup_restore/backup_scheduler.sh | 27 +++++++++--- 4 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 .github/scripts/tests/test_backup_schedule_validation.py diff --git a/.github/scripts/tests/test_backup_schedule_validation.py b/.github/scripts/tests/test_backup_schedule_validation.py new file mode 100644 index 00000000..1fb2046b --- /dev/null +++ b/.github/scripts/tests/test_backup_schedule_validation.py @@ -0,0 +1,44 @@ +"""A schedule systemd rejects leaves a timer that never fires and reports no +error, so the wizard asks for it again instead of saving it.""" +from pathlib import Path +import subprocess +import tempfile +import unittest + +SCHEDULER = Path(__file__).resolve().parents[3] / 'scripts/backup_restore/backup_scheduler.sh' +ANALYZE = '#!/bin/sh\ncase "$2" in "daily 02:00") exit 1;; esac\nexit 0\n' + + +def function(name): + text = SCHEDULER.read_text() + start = text.index(name + '() {') + return text[start:text.index('\n}', start) + 2] + '\n' + + +class ScheduleValidation(unittest.TestCase): + def valid(self, expression, **environment): + with tempfile.TemporaryDirectory() as directory: + analyze = Path(directory) / 'systemd-analyze' + analyze.write_text(ANALYZE) + analyze.chmod(0o755) + return subprocess.run( + ['/bin/bash', '-c', function('_calendar_is_valid') + '_calendar_is_valid "$1"', 'test', expression], + env={'PATH': f'{directory}:/usr/bin:/bin', **environment}, + ).returncode == 0 + + def test_systemd_decides_what_is_valid(self): + self.assertTrue(self.valid('02:30')) + self.assertFalse(self.valid('daily 02:00')) + + def test_a_scheduler_without_systemctl_is_not_checked(self): + self.assertTrue(self.valid('daily 02:00', PMX_BACKUP_NO_SYSTEMCTL='1')) + + def test_the_wizard_asks_again_before_saving_the_job(self): + text = SCHEDULER.read_text() + check = text.index('while ! _calendar_is_valid "$on_calendar"; do') + self.assertLess(check, text.index('"ON_CALENDAR=$on_calendar"')) + self.assertGreater(check, text.index('on_calendar=$(dialog')) + + +if __name__ == '__main__': + unittest.main() diff --git a/lang/es.json b/lang/es.json index 5b6b7497..194834d6 100644 --- a/lang/es.json +++ b/lang/es.json @@ -2385,6 +2385,7 @@ "Example: cp /etc/pve/lxc/110.conf /etc/pve/lxc/110.conf.bak": "Ejemplo: cp /etc/pve/lxc/110.conf /etc/pve/lxc/110.conf.bak", "Example: cp /etc/pve/lxc/114.conf /etc/pve/lxc/114.conf.bak": "Ejemplo: cp /etc/pve/lxc/114.conf /etc/pve/lxc/114.conf.bak", "Example: daily or Mon..Fri 03:00": "Ejemplo: diario o lunes a viernes 03:00", + "Example: daily, 02:30 or Mon..Fri 03:00": "Ejemplo: daily, 02:30 o Mon..Fri 03:00", "Example: mkdir -p /tmp/lxc_convert_114": "Ejemplo: mkdir -p /tmp/lxc_convert_114", "Example: pct config 110 | grep unprivileged": "Ejemplo: pct config 110 | grep unprivileged", "Example: pct stop 110": "Ejemplo: pct stop 110", @@ -7487,6 +7488,7 @@ "This removes the storage from Proxmox. The iSCSI target is not affected.": "Esto elimina el almacenamiento de Proxmox. El destino iSCSI no se ve afectado.", "This replaces the currently installed keyfile.": "Esto reemplaza el archivo de claves instalado actualmente.", "This returns available IQNs.": "Esto devuelve los IQN disponibles.", + "This schedule is not valid. Examples: daily, 02:30 or Mon..Fri 03:00": "Esta programación no es válida. Ejemplos: daily, 02:30 o Mon..Fri 03:00", "This script cannot be executed from the Proxmox web terminal.": "Este script no se puede ejecutar desde la terminal web de Proxmox.", "This script must be run as root.": "Este script debe ejecutarse como root.", "This script must be run on a Proxmox host.": "Este script debe ejecutarse en un host Proxmox.", diff --git a/lang/sk.json b/lang/sk.json index ba19689a..42920444 100644 --- a/lang/sk.json +++ b/lang/sk.json @@ -2388,6 +2388,7 @@ "Example: cp /etc/pve/lxc/110.conf /etc/pve/lxc/110.conf.bak": "Príklad: cp /etc/pve/lxc/110.conf /etc/pve/lxc/110.conf.bak", "Example: cp /etc/pve/lxc/114.conf /etc/pve/lxc/114.conf.bak": "Príklad: cp /etc/pve/lxc/114.conf /etc/pve/lxc/114.conf.bak", "Example: daily or Mon..Fri 03:00": "Príklad: daily alebo Mon..Fri 03:00", + "Example: daily, 02:30 or Mon..Fri 03:00": "Príklad: daily, 02:30 alebo Mon..Fri 03:00", "Example: mkdir -p /tmp/lxc_convert_114": "Príklad: mkdir -p /tmp/lxc_convert_114", "Example: pct config 110 | grep unprivileged": "Príklad: pct config 110 | grep unprivileged", "Example: pct stop 110": "Príklad: pct stop 110", @@ -7514,6 +7515,7 @@ "This removes the storage from Proxmox. The iSCSI target is not affected.": "Týmto sa úložisko odstráni z Proxmoxu. iSCSI target sa nezmení.", "This replaces the currently installed keyfile.": "Týmto sa nahradí aktuálne nainštalovaný keyfile.", "This returns available IQNs.": "Týmto sa zobrazia dostupné IQN.", + "This schedule is not valid. Examples: daily, 02:30 or Mon..Fri 03:00": "Tento plán nie je platný. Príklady: daily, 02:30 alebo Mon..Fri 03:00", "This script cannot be executed from the Proxmox web terminal.": "Tento skript sa nedá spustiť z webového terminálu Proxmoxu.", "This script must be run as root.": "Tento skript musí byť spustený ako root.", "This script must be run on a Proxmox host.": "Tento skript musí byť spustený na Proxmox serveri.", diff --git a/scripts/backup_restore/backup_scheduler.sh b/scripts/backup_restore/backup_scheduler.sh index 409cf007..0c78fb58 100755 --- a/scripts/backup_restore/backup_scheduler.sh +++ b/scripts/backup_restore/backup_scheduler.sh @@ -107,6 +107,12 @@ _job_is_attached() { # Reads a key=val pair from the job .env file (handles `printf %q` # quoting that _write_job_env produces). +_calendar_is_valid() { + [[ "${PMX_BACKUP_NO_SYSTEMCTL:-0}" == "1" ]] && return 0 + command -v systemd-analyze >/dev/null 2>&1 || return 0 + systemd-analyze calendar "$1" >/dev/null 2>&1 +} + _job_env_get() { local id="$1" key="$2" f raw f=$(_job_file "$id") @@ -315,12 +321,10 @@ _reconcile_restored_jobs() { fi on_calendar=$(_job_env_get "$id" ON_CALENDAR || echo "") - if [[ "${PMX_BACKUP_NO_SYSTEMCTL:-0}" != "1" ]] && command -v systemd-analyze >/dev/null 2>&1; then - if ! systemd-analyze calendar "$on_calendar" >/dev/null 2>&1; then - echo "${id}: invalid systemd OnCalendar expression." >&2 - invalid=$((invalid + 1)) - continue - fi + if ! _calendar_is_valid "$on_calendar"; then + echo "${id}: invalid systemd OnCalendar expression." >&2 + invalid=$((invalid + 1)) + continue fi if ! _write_job_units "$id" "$on_calendar"; then @@ -582,9 +586,18 @@ _create_job() { fi on_calendar=$(dialog --backtitle "ProxMenux" --title "$(translate "Schedule")" \ - --inputbox "$(translate "systemd OnCalendar expression")"$'\n'"$(translate "Example: daily or Mon..Fri 03:00")" \ + --inputbox "$(translate "systemd OnCalendar expression")"$'\n'"$(translate "Example: daily, 02:30 or Mon..Fri 03:00")" \ 11 72 "daily" 3>&1 1>&2 2>&3) || return 1 [[ -z "$on_calendar" ]] && return 1 + # A timer with an invalid expression never fires and reports no error. + while ! _calendar_is_valid "$on_calendar"; do + dialog --backtitle "ProxMenux" --title "$(translate "Schedule")" \ + --msgbox "$(translate "This schedule is not valid. Examples: daily, 02:30 or Mon..Fri 03:00")" 8 72 + on_calendar=$(dialog --backtitle "ProxMenux" --title "$(translate "Schedule")" \ + --inputbox "$(translate "systemd OnCalendar expression")"$'\n'"$(translate "Example: daily, 02:30 or Mon..Fri 03:00")" \ + 11 72 "$on_calendar" 3>&1 1>&2 2>&3) || return 1 + [[ -z "$on_calendar" ]] && return 1 + done profile_mode=$(dialog --backtitle "ProxMenux" --title "$(translate "Profile")" \ --menu "\n$(translate "Select backup profile:")" 12 68 4 \