From f07dbfe10f134703a480d660b48bbd263c15297f Mon Sep 17 00:00:00 2001 From: martino <32328813+f3rs3n@users.noreply.github.com> Date: Fri, 16 Oct 2026 15:42:58 +0200 Subject: [PATCH 1/2] fix(vm): reject failed ISO downloads and repair image sources --- .github/scripts/tests/test_vm_iso_download.py | 263 ++++++++++++++++++ .github/scripts/tests/test_vm_iso_sources.py | 217 +++++++++++++++ scripts/vm/select_linux_iso.sh | 4 +- scripts/vm/select_nas_iso.sh | 7 +- scripts/vm/vm_creator.sh | 52 ++-- 5 files changed, 513 insertions(+), 30 deletions(-) create mode 100644 .github/scripts/tests/test_vm_iso_download.py create mode 100644 .github/scripts/tests/test_vm_iso_sources.py diff --git a/.github/scripts/tests/test_vm_iso_download.py b/.github/scripts/tests/test_vm_iso_download.py new file mode 100644 index 00000000..62900843 --- /dev/null +++ b/.github/scripts/tests/test_vm_iso_download.py @@ -0,0 +1,263 @@ +"""The VM wizard must not accept failed ISO downloads as installation media. + +Run its real functions against a loopback HTTP fixture and stub only external +Proxmox/UI services. No product script is sourced, no guest operation runs, +and no public network or real ISO storage is accessed. +""" +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +import os +from pathlib import Path +import re +import shlex +import shutil +import subprocess +import tempfile +import threading +import unittest + +ROOT = Path(__file__).resolve().parents[3] +CREATOR = ROOT / "scripts/vm/vm_creator.sh" +PAYLOAD = b"fixture installation media\x00\x01\n" +REAL_WGET = shutil.which("wget") + + +class MediaHandler(BaseHTTPRequestHandler): + def do_GET(self): + if self.path == "/missing.iso": + self.send_response(404) + self.end_headers() + return + if self.path == "/partial.iso": + self.send_response(200) + self.send_header("Content-Length", "1024") + self.end_headers() + self.wfile.write(b"partial") + self.close_connection = True + return + body = b"" if self.path == "/empty.iso" else PAYLOAD + self.send_response(200) + self.send_header("Content-Length", str(len(body))) + self.send_header("Content-Disposition", 'attachment; filename="remote-name.iso"') + self.end_headers() + self.wfile.write(body) + + def log_message(self, format, *args): + pass + + +class VmIsoDownloadTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + if REAL_WGET is None: + raise RuntimeError("These offline tests require wget, as the VM wizard does") + cls.server = ThreadingHTTPServer(("127.0.0.1", 0), MediaHandler) + cls.thread = threading.Thread(target=cls.server.serve_forever, daemon=True) + cls.thread.start() + cls.http = f"http://127.0.0.1:{cls.server.server_port}" + + @classmethod + def tearDownClass(cls): + cls.server.shutdown() + cls.server.server_close() + cls.thread.join() + + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="vm-iso-download-") + self.addCleanup(self.temp.cleanup) + self.directory = Path(self.temp.name) + self.iso = self.directory / "installation media.iso" + self.qm_log = self.directory / "qm.log" + self.wget_log = self.directory / "wget.log" + self.bin = self.directory / "bin" + self.bin.mkdir() + # Route only the optional fixed VirtIO/SourceForge endpoints to our + # fixture. Wget still performs the actual HTTP transfer and file I/O. + assert REAL_WGET is not None + wrapper = self.bin / "wget" + wrapper.write_text( + "#!/usr/bin/env bash\n" + 'printf "%s\\n" "$*" >> "$WGET_LOG"\n' + 'args=("$@")\n' + 'for i in "${!args[@]}"; do\n' + ' case "${args[$i]}" in\n' + ' https://fedorapeople.org/*) args[$i]="$VIRTIO_FIXTURE_URL";;\n' + ' https://sourceforge.net/*) args[$i]="$HTTP_FIXTURE/success.iso";;\n' + ' esac\n' + ' if [[ "${args[$i]}" == http://* || "${args[$i]}" == https://* ]]; then\n' + ' [[ "${args[$i]}" == "$HTTP_FIXTURE"/* ]] || exit 98\n' + ' fi\n' + 'done\n' + 'for i in "${!args[@]}"; do\n' + ' if [[ "${args[$i]}" == -O ]]; then\n' + ' case "${args[$((i+1))]}" in "$FIXTURE_DIR"/*) ;; *) exit 97;; esac\n' + ' fi\n' + 'done\n' + f"exec {shlex.quote(REAL_WGET)} --no-config --no-proxy --tries=1 --timeout=1 " + '"${args[@]}"\n' + ) + wrapper.chmod(0o755) + + def run_wizard(self, route="/success.iso", windows=False, overwrite=True, url=None): + source = CREATOR.read_text() + functions = "\n\n".join(re.findall(r"^function \w+\(\) \{\n.*?^\}", source, re.M | re.S)) + self.assertIn("function create_vm()", functions) + prelude = r''' + translate() { printf '%s' "$1"; } + msg_ok() { printf 'OK:%s\n' "$*"; } + msg_error() { printf 'ERROR:%s\n' "$*"; } + msg_warn() { printf 'WARN:%s\n' "$*"; } + msg_info() { :; } + msg_success() { :; } + qm() { printf '%q ' "$@" >> "$QM_LOG"; printf '\n' >> "$QM_LOG"; } + pvesm() { printf 'Name Type Status\nfixture dir active\n'; } + select_storage_target() { printf fixture; } + # No next menu, start, or guest-facing process is launched. + bash() { :; } + whiptail() { + if [[ " $* " == *' --yesno '* ]]; then + return "$OVERWRITE_STATUS" + fi + if [[ ! -f "$DIALOG_ONCE" ]]; then + : > "$DIALOG_ONCE" + printf '1' >&2 + return 0 + fi + return 1 + } + IMPORT_DISKS=(); PASSTHROUGH_DISKS=(); VIRTUAL_DISKS=(); CONTROLLER_NVME_PCIS=() + VMID=99999; HN=fixture; CORE_COUNT=1; RAM_SIZE=512; BRG=fixture + MAC='02:00:00:00:00:01'; VLAN=''; MTU=''; BIOS_TYPE=''; CPU_TYPE='' + MACHINE=''; SERIAL_PORT=''; START_VM=no; WIZARD_ADD_GPU=no + TAB=''; GN=''; CL='' + create_vm + ''' + env = dict(os.environ, PATH=str(self.bin) + os.pathsep + os.environ["PATH"], + ISO_PATH=str(self.iso), ISO_URL=url if url is not None else self.http + route, + ISO_VOLID="local:iso/installation media.iso", ISO_DIR=str(self.directory), + OS_TYPE="2" if windows else "3", + QM_LOG=str(self.qm_log), WGET_LOG=str(self.wget_log), + DIALOG_ONCE=str(self.directory / "dialog-once"), FIXTURE_DIR=str(self.directory), + OVERWRITE_STATUS="0" if overwrite else "1", HTTP_FIXTURE=self.http, + VIRTIO_FIXTURE_URL=self.http + route) + return subprocess.run(["bash", "--noprofile", "--norc", "-c", functions + prelude], + env=env, stdin=subprocess.DEVNULL, capture_output=True, + text=True, timeout=15) + + def qm_calls(self): + return self.qm_log.read_text() if self.qm_log.exists() else "" + + def test_http_failure_stops_before_creating_a_guest(self): + result = self.run_wizard("/missing.iso") + self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(self.qm_calls(), "") + self.assertNotIn("OK:ISO image downloaded", result.stdout) + self.assertIn("ERROR:Failed to download ISO image", result.stdout) + + def test_partial_transfer_is_not_published_as_cached_media(self): + result = self.run_wizard("/partial.iso") + self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(self.qm_calls(), "") + self.assertFalse(self.iso.exists(), "A failed transfer must not become reusable media") + + def test_an_empty_cached_download_is_retried(self): + self.iso.write_bytes(b"") + result = self.run_wizard() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(self.iso.read_bytes(), PAYLOAD) + self.assertIn("create ", self.qm_calls()) + + def test_virtio_download_uses_the_configured_iso_directory(self): + self.iso.write_bytes(PAYLOAD) + result = self.run_wizard(windows=True) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + driver = self.directory / "virtio-win.iso" + self.assertTrue(driver.is_file(), result.stdout + result.stderr) + self.assertEqual(driver.read_bytes(), PAYLOAD) + self.assertIn("-ide3 ", self.qm_calls()) + + def test_failed_virtio_fetch_is_not_selected_for_attachment(self): + self.iso.write_bytes(PAYLOAD) + result = self.run_wizard("/missing.iso", windows=True) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertNotIn("-ide3 ", self.qm_calls()) + self.assertNotIn("OK:VirtIO driver ISO downloaded successfully.", result.stdout) + self.assertIn("ERROR:Failed to download VirtIO driver ISO.", result.stdout) + self.assertFalse((self.directory / "virtio-win.iso").exists()) + + + def test_successful_virtio_replacement_preserves_existing_permissions(self): + self.iso.write_bytes(PAYLOAD) + driver = self.directory / "virtio-win.iso" + driver.write_bytes(b"previous driver") + driver.chmod(0o640) + result = self.run_wizard(windows=True) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(driver.read_bytes(), PAYLOAD) + self.assertEqual(driver.stat().st_mode & 0o777, 0o640) + + def test_empty_http_success_is_rejected(self): + result = self.run_wizard("/empty.iso") + self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(self.qm_calls(), "") + self.assertFalse(self.iso.exists()) + self.assertEqual({p.name for p in self.directory.iterdir()}, {"bin", "wget.log"}) + + def test_existing_nonempty_media_is_reused_without_fetching(self): + self.iso.write_bytes(b"existing installation media") + result = self.run_wizard("/missing.iso") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(self.iso.read_bytes(), b"existing installation media") + self.assertFalse(self.wget_log.exists()) + self.assertIn("-ide2 ", self.qm_calls()) + + def test_sourceforge_download_keeps_the_selected_destination_name(self): + result = self.run_wizard(url="https://sourceforge.net/projects/fixture/iso/download") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(self.iso.read_bytes(), PAYLOAD) + self.assertFalse((self.directory / "remote-name.iso").exists()) + self.assertEqual({p.name for p in self.directory.iterdir()}, + {"bin", "wget.log", "qm.log", self.iso.name}) + + def test_failed_virtio_replacement_preserves_previous_media(self): + self.iso.write_bytes(PAYLOAD) + driver = self.directory / "virtio-win.iso" + driver.write_bytes(b"previous driver") + driver.chmod(0o640) + result = self.run_wizard("/partial.iso", windows=True) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(driver.read_bytes(), b"previous driver") + self.assertEqual(driver.stat().st_mode & 0o777, 0o640) + self.assertNotIn("-ide3 ", self.qm_calls()) + self.assertNotIn("OK:VirtIO driver ISO downloaded successfully.", result.stdout) + + def test_declining_virtio_replacement_reuses_previous_media(self): + self.iso.write_bytes(PAYLOAD) + driver = self.directory / "virtio-win.iso" + driver.write_bytes(b"previous driver") + result = self.run_wizard("/missing.iso", windows=True, overwrite=False) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(driver.read_bytes(), b"previous driver") + self.assertFalse(self.wget_log.exists()) + self.assertIn("-ide3 ", self.qm_calls()) + + def test_empty_cached_virtio_media_is_retried_not_attached(self): + self.iso.write_bytes(PAYLOAD) + driver = self.directory / "virtio-win.iso" + driver.write_bytes(b"") + result = self.run_wizard("/missing.iso", windows=True, overwrite=False) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertTrue(self.wget_log.exists()) + self.assertNotIn("-ide3 ", self.qm_calls()) + self.assertIn("ERROR:Failed to download VirtIO driver ISO.", result.stdout) + + def test_a_directory_cannot_be_published_as_media(self): + self.iso.mkdir() + result = self.run_wizard() + self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(self.qm_calls(), "") + self.assertTrue(self.iso.is_dir()) + self.assertEqual(list(self.iso.iterdir()), []) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/scripts/tests/test_vm_iso_sources.py b/.github/scripts/tests/test_vm_iso_sources.py new file mode 100644 index 00000000..8eb9e46a --- /dev/null +++ b/.github/scripts/tests/test_vm_iso_sources.py @@ -0,0 +1,217 @@ +"""Offline VM source regressions; execute bounded real functions, never selectors. + +Fixtures are reduced official metadata excerpts captured 2026-10-16. Network +commands are shell stubs: no ISO downloads, utils sourcing, or host changes. +""" +import os +from pathlib import Path +import re +import subprocess +import tempfile +import unittest + + +ROOT = Path(__file__).resolve().parents[3] +LINUX = ROOT / "scripts/vm/select_linux_iso.sh" +NAS = ROOT / "scripts/vm/select_nas_iso.sh" +FIELDS = ("ISO_NAME", "ISO_TYPE", "ISO_URL", "ISO_FILE", "ISO_PATH", "HN") + + +def extract_function(path, name): + """Only top-level function definitions with an unindented closing brace.""" + match = re.search( + rf"^function {re.escape(name)}\(\) \{{\n.*?^\}}(?=\n|$)", + path.read_text(), re.MULTILINE | re.DOTALL, + ) + if match is None: + raise AssertionError(f"Missing bounded function: {name}") + return match.group(0) + + +class IsoSourceTests(unittest.TestCase): + def run_selector(self, path, function, *, menu_name="", metadata="", + metadata_url="", fetch_status=0, has_curl=True): + with tempfile.TemporaryDirectory(prefix="vm-iso-sources-") as directory: + temp = Path(directory) + fixture = temp / "metadata.html" + fixture.write_text(metadata) + trace = temp / "fetches.txt" + trace.touch() + iso_dir = temp / "iso" + iso_dir.mkdir() + env = os.environ.copy() + env.update(ISO_DIR=str(iso_dir), MENU_NAME=menu_name, + METADATA_FILE=str(fixture), METADATA_URL=metadata_url, + FETCH_STATUS=str(fetch_status), FETCH_TRACE=str(trace)) + helpers = "\n".join( + extract_function(NAS, name) + for name in ("_has_curl", "_latest_version_from_lines") + ) + script = helpers + r''' +translate() { printf '%s' "$1"; } +curl() { + local url="${@: -1}" + printf '%s\n' "$url" >> "$FETCH_TRACE" + [[ "$url" == "$METADATA_URL" ]] || return 97 + [[ "$FETCH_STATUS" == 0 ]] || return "$FETCH_STATUS" + cat "$METADATA_FILE" +} +wget() { return 97; } +dialog() { + local arg previous="" name + for arg in "$@"; do + name="${arg%%│*}" + name="${name%"${name##*[![:space:]]}"}" + if [[ "$name" == "$MENU_NAME" ]]; then + printf '%s' "$previous" >&2 + return 0 + fi + previous="$arg" + done + return 1 +} +''' + if not has_curl: + script += "\n_has_curl() { return 1; }\n" + script += extract_function(path, function) + script += f"\n{function} || exit $?\n" + script += 'printf \'%s\\0\' ' + " ".join( + f'"${{{field}:-}}"' for field in FIELDS + ) + result = subprocess.run( + ["bash", "--noprofile", "--norc", "-c", script], + env=env, capture_output=True, timeout=10, + ) + self.assertEqual(result.returncode, 0, result.stderr.decode()) + values = result.stdout.decode().split("\0") + self.assertEqual(len(values), len(FIELDS) + 1, result.stdout) + selected = dict(zip(FIELDS, values)) + self.assertEqual(selected["ISO_PATH"], str(iso_dir / selected["ISO_FILE"])) + return selected, trace.read_text().splitlines() + + def test_debian13_dvd_uses_fixed_official_archive(self): + selected, calls = self.run_selector( + LINUX, "select_linux_iso_official", menu_name="Debian 13", + ) + self.assertEqual(selected["ISO_URL"], + "https://cdimage.debian.org/cdimage/archive/13.6.0/amd64/iso-dvd/" + "debian-13.6.0-amd64-DVD-1.iso") + self.assertEqual(selected["ISO_TYPE"], "Desktop") + self.assertEqual(selected["ISO_FILE"], "debian-13.6.0-amd64-DVD-1.iso") + self.assertEqual(calls, []) + + def test_debian13_netinst_uses_fixed_official_archive(self): + selected, calls = self.run_selector( + LINUX, "select_linux_iso_official", menu_name="Debian 13 Netinst", + ) + self.assertEqual(selected["ISO_URL"], + "https://cdimage.debian.org/cdimage/archive/13.6.0/amd64/iso-cd/" + "debian-13.6.0-amd64-netinst.iso") + self.assertEqual(selected["ISO_TYPE"], "CLI") + self.assertEqual(selected["ISO_FILE"], "debian-13.6.0-amd64-netinst.iso") + self.assertEqual(calls, []) + + def test_omv_metadata_unavailable_uses_verified_fallback(self): + url = "https://sourceforge.net/projects/openmediavault/files/iso/" + for has_curl, fetch_status, metadata in ( + (False, 0, ""), (True, 22, ""), + (True, 0, "No release links"), + ): + with self.subTest(has_curl=has_curl, fetch_status=fetch_status): + selected, calls = self.run_selector( + NAS, "resolve_omv_iso", metadata_url=url, has_curl=has_curl, + fetch_status=fetch_status, metadata=metadata, + ) + self.assertEqual(selected["ISO_URL"], + url + "8.3.1/openmediavault_8.3.1-amd64.iso/download") + self.assertEqual(selected["ISO_NAME"], "OpenMediaVault 8.3.1") + self.assertEqual(selected["ISO_FILE"], "openmediavault_8.3.1-amd64.iso") + self.assertEqual(calls, [url] if has_curl else []) + + def test_rockstor_resolves_official_landing_without_directory_listing(self): + # Exact installer anchor excerpt from https://rockstor.com/dls/index.html. + url = "https://rockstor.com/dls/index.html" + metadata = ( + '' + 'Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso
' + ) + selected, calls = self.run_selector( + NAS, "resolve_rockstor_iso", metadata_url=url, metadata=metadata, + ) + self.assertEqual(selected["ISO_URL"], + "https://rockstor.com/downloads/installer/leap/15.6/x86_64/" + "Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso") + self.assertEqual(selected["ISO_FILE"], + "Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso") + self.assertEqual(selected["HN"], "Rockstor") + self.assertEqual(calls, [url]) + + def test_rockstor_metadata_unavailable_uses_verified_fallback(self): + url = "https://rockstor.com/dls/index.html" + for has_curl, fetch_status, metadata in ( + (False, 0, ""), (True, 22, ""), + (True, 0, "No installer links"), + ): + with self.subTest(has_curl=has_curl, fetch_status=fetch_status): + selected, calls = self.run_selector( + NAS, "resolve_rockstor_iso", metadata_url=url, has_curl=has_curl, + fetch_status=fetch_status, metadata=metadata, + ) + self.assertEqual(selected["ISO_URL"], + "https://rockstor.com/downloads/installer/leap/15.6/x86_64/" + "Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso") + self.assertEqual(selected["ISO_NAME"], "Rockstor") + self.assertEqual(selected["ISO_FILE"], + "Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso") + self.assertEqual(calls, [url] if has_curl else []) + + def test_omv_current_metadata_resolution_is_preserved(self): + # Reduced release links from the official SourceForge ISO index. + url = "https://sourceforge.net/projects/openmediavault/files/iso/" + metadata = ( + '8.3.1\n' + '7.4.17' + ) + selected, calls = self.run_selector( + NAS, "resolve_omv_iso", metadata_url=url, metadata=metadata, + ) + self.assertEqual(selected["ISO_URL"], + url + "8.3.1/openmediavault_8.3.1-amd64.iso/download") + self.assertEqual(calls, [url]) + + def test_omv_detected_release_takes_precedence_over_fallback(self): + url = "https://sourceforge.net/projects/openmediavault/files/iso/" + # Synthetic ordering case: must execute the real version parser. + metadata = ('new' + 'old') + selected, calls = self.run_selector( + NAS, "resolve_omv_iso", metadata_url=url, metadata=metadata, + ) + self.assertEqual(selected["ISO_URL"], + url + "8.10.0/openmediavault_8.10.0-amd64.iso/download") + self.assertEqual(calls, [url]) + + def test_rockstor_retains_generic_x86_64_leap156_edition(self): + url = "https://rockstor.com/dls/index.html" + # Synthetic ordering / edition cases; not claims of published images. + metadata = "\n".join(( + "Rockstor-Leap15.6-generic.x86_64-5.10.0-0.install.iso", + "Rockstor-Leap15.6-generic.x86_64-5.9.0-0.install.iso", + "Rockstor-Leap15.6-generic.aarch64-9.0.0-0.install.iso", + "Rockstor-Leap16.0-generic.x86_64-9.0.0-0.install.iso", + "Rockstor-Leap15.5-generic.x86_64-9.0.0-0.install.iso", + )) + selected, calls = self.run_selector( + NAS, "resolve_rockstor_iso", metadata_url=url, metadata=metadata, + ) + self.assertEqual(selected["ISO_URL"], + "https://rockstor.com/downloads/installer/leap/15.6/x86_64/" + "Rockstor-Leap15.6-generic.x86_64-5.10.0-0.install.iso") + self.assertEqual(calls, [url]) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/vm/select_linux_iso.sh b/scripts/vm/select_linux_iso.sh index 97bee392..4677dd4a 100644 --- a/scripts/vm/select_linux_iso.sh +++ b/scripts/vm/select_linux_iso.sh @@ -113,10 +113,10 @@ function select_linux_iso_official() { "Ubuntu 24.04 Server|CLI|ProxMenux|https://releases.ubuntu.com/24.04/ubuntu-24.04.4-live-server-amd64.iso" "Ubuntu 22.04 Server|CLI|ProxMenux|https://releases.ubuntu.com/22.04/ubuntu-22.04.5-live-server-amd64.iso" "Ubuntu 20.04 Server|CLI|ProxMenux|https://releases.ubuntu.com/20.04/ubuntu-20.04.6-live-server-amd64.iso" - "Debian 13|Desktop|ProxMenux|https://cdimage.debian.org/debian-cd/current/amd64/iso-dvd/debian-13.4.0-amd64-DVD-1.iso" + "Debian 13|Desktop|ProxMenux|https://cdimage.debian.org/cdimage/archive/13.6.0/amd64/iso-dvd/debian-13.6.0-amd64-DVD-1.iso" "Debian 12|Desktop|ProxMenux|https://cdimage.debian.org/cdimage/archive/12.13.0/amd64/iso-dvd/debian-12.13.0-amd64-DVD-1.iso" "Debian 11|Desktop|ProxMenux|https://cdimage.debian.org/cdimage/archive/11.11.0/amd64/iso-dvd/debian-11.11.0-amd64-DVD-1.iso" - "Debian 13 Netinst|CLI|ProxMenux|https://cdimage.debian.org/debian-cd/current/amd64/iso-cd/debian-13.4.0-amd64-netinst.iso" + "Debian 13 Netinst|CLI|ProxMenux|https://cdimage.debian.org/cdimage/archive/13.6.0/amd64/iso-cd/debian-13.6.0-amd64-netinst.iso" "Debian 12 Netinst|CLI|ProxMenux|https://cdimage.debian.org/cdimage/archive/12.13.0/amd64/iso-cd/debian-12.13.0-amd64-netinst.iso" "Debian 11 Netinst|CLI|ProxMenux|https://cdimage.debian.org/cdimage/archive/11.11.0/amd64/iso-cd/debian-11.11.0-amd64-netinst.iso" "Fedora Workstation 42|Desktop|ProxMenux|https://download.fedoraproject.org/pub/fedora/linux/releases/42/Workstation/x86_64/iso/Fedora-Workstation-Live-42-1.1.x86_64.iso" diff --git a/scripts/vm/select_nas_iso.sh b/scripts/vm/select_nas_iso.sh index d45b8b76..53d0828f 100644 --- a/scripts/vm/select_nas_iso.sh +++ b/scripts/vm/select_nas_iso.sh @@ -100,7 +100,7 @@ function resolve_truenas_core_iso() { } function resolve_omv_iso() { - local default_ver="8.1.1" + local default_ver="8.3.1" local detected_ver="" if _has_curl; then @@ -157,13 +157,14 @@ function resolve_xigmanas_iso() { } function resolve_rockstor_iso() { - local default_file="Rockstor-Leap15.6-generic.x86_64-5.0.15-0.install.iso" + local default_file="Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso" local detected_file="" local base_url="https://rockstor.com/downloads/installer/leap/15.6/x86_64" if _has_curl; then detected_file=$( - curl -fsSL "${base_url}/" 2>/dev/null \ + # The installer directory forbids listings; use the official downloads page. + curl -fsSL "https://rockstor.com/dls/index.html" 2>/dev/null \ | grep -Eo 'Rockstor-Leap15\.6-generic\.x86_64-[0-9]+\.[0-9]+\.[0-9]+-[0-9]+\.install\.iso' \ | _latest_version_from_lines ) diff --git a/scripts/vm/vm_creator.sh b/scripts/vm/vm_creator.sh index 19bbc47f..5c5e1128 100644 --- a/scripts/vm/vm_creator.sh +++ b/scripts/vm/vm_creator.sh @@ -259,6 +259,25 @@ function run_gpu_passthrough_wizard() { +# Download beside the destination, publishing only a complete non-empty file. +function download_vm_iso() { + local iso_url="$1" iso_path="$2" + local staging_dir download_status=1 + local -a wget_options=(--no-verbose --show-progress) + [[ "$iso_url" == *"sourceforge.net"* ]] && wget_options+=(--content-disposition) + + staging_dir=$(mktemp -d "${iso_path}.part.XXXXXX") || return 1 + if wget "${wget_options[@]}" -O "$staging_dir/media.iso" "$iso_url" \ + && [[ -s "$staging_dir/media.iso" ]] \ + && { [[ ! -f "$iso_path" ]] || chmod --reference="$iso_path" "$staging_dir/media.iso"; } \ + && mv -fT -- "$staging_dir/media.iso" "$iso_path"; then + download_status=0 + fi + rm -f -- "$staging_dir/media.iso" + rmdir -- "$staging_dir" || return 1 + return "$download_status" +} + # ========================================================== # Create VM # ========================================================== @@ -266,21 +285,12 @@ function create_vm() { local BOOT_ORDER="" local DISK_INFO="" local DISK_INDEX=0 - local ISO_DIR="/var/lib/vz/template/iso" + local ISO_DIR="${ISO_DIR:-/var/lib/vz/template/iso}" - if [[ -n "$ISO_PATH" && -n "$ISO_URL" && ! -f "$ISO_PATH" ]]; then + if [[ -n "$ISO_PATH" && -n "$ISO_URL" ]] && [[ ! -f "$ISO_PATH" || ! -s "$ISO_PATH" ]]; then - if [[ "$ISO_URL" == *"sourceforge.net"* ]]; then - - wget --content-disposition --show-progress -O "$ISO_PATH" "$ISO_URL" - else - - wget --no-verbose --show-progress -O "$ISO_PATH" "$ISO_URL" - fi - - - if [[ -f "$ISO_PATH" ]]; then + if download_vm_iso "$ISO_URL" "$ISO_PATH"; then ISO_VOLID="${ISO_VOLID:-local:iso/$(basename "$ISO_PATH")}" msg_ok "$(translate "ISO image downloaded")" else @@ -647,7 +657,7 @@ fi if [[ "$OS_TYPE" == "2" ]]; then - local VIRTIO_DIR="/var/lib/vz/template/iso" + local VIRTIO_DIR="$ISO_DIR" local VIRTIO_SELECTED="" local VIRTIO_VOLID="" local VIRTIO_DOWNLOAD_URL="https://fedorapeople.org/groups/virt/virtio-win/direct-downloads/stable-virtio/virtio-win.iso" @@ -662,21 +672,13 @@ fi case "$VIRTIO_OPTION" in 1) - if [[ -f "$VIRTIO_DIR/virtio-win.iso" ]]; then - if whiptail --title "ProxMenux" --yesno "$(translate "A VirtIO ISO already exists. Do you want to overwrite it?")" 10 60; then - wget -q --show-progress -O "$VIRTIO_DIR/virtio-win.iso" "$VIRTIO_DOWNLOAD_URL" - if [[ -f "$VIRTIO_DIR/virtio-win.iso" ]]; then - msg_ok "$(translate "VirtIO driver ISO downloaded successfully.")" - else - msg_error "$(translate "Failed to download VirtIO driver ISO.")" - fi - fi - else - wget -q --show-progress -O "$VIRTIO_DIR/virtio-win.iso" "$VIRTIO_DOWNLOAD_URL" - if [[ -f "$VIRTIO_DIR/virtio-win.iso" ]]; then + if [[ ! -f "$VIRTIO_DIR/virtio-win.iso" || ! -s "$VIRTIO_DIR/virtio-win.iso" ]] \ + || whiptail --title "ProxMenux" --yesno "$(translate "A VirtIO ISO already exists. Do you want to overwrite it?")" 10 60; then + if download_vm_iso "$VIRTIO_DOWNLOAD_URL" "$VIRTIO_DIR/virtio-win.iso"; then msg_ok "$(translate "VirtIO driver ISO downloaded successfully.")" else msg_error "$(translate "Failed to download VirtIO driver ISO.")" + continue fi fi From b55eca753df3f7ff492c5f079c1a0f183b969594 Mon Sep 17 00:00:00 2001 From: martino <32328813+f3rs3n@users.noreply.github.com> Date: Fri, 16 Oct 2026 16:28:23 +0200 Subject: [PATCH 2/2] test(vm): keep ISO storage isolation in fixtures --- .github/scripts/tests/test_vm_iso_download.py | 26 ++++++++++++++----- scripts/vm/vm_creator.sh | 4 +-- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/.github/scripts/tests/test_vm_iso_download.py b/.github/scripts/tests/test_vm_iso_download.py index 62900843..8c859386 100644 --- a/.github/scripts/tests/test_vm_iso_download.py +++ b/.github/scripts/tests/test_vm_iso_download.py @@ -1,8 +1,9 @@ """The VM wizard must not accept failed ISO downloads as installation media. -Run its real functions against a loopback HTTP fixture and stub only external -Proxmox/UI services. No product script is sourced, no guest operation runs, -and no public network or real ISO storage is accessed. +Run its function logic against a loopback HTTP fixture. Redirect the fixed +native ISO directory only in the loaded test copy and stub Proxmox/UI services. +No product script is sourced, no guest operation runs, and no public network +or real ISO storage is accessed. """ from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer import os @@ -97,10 +98,15 @@ class VmIsoDownloadTests(unittest.TestCase): ) wrapper.chmod(0o755) - def run_wizard(self, route="/success.iso", windows=False, overwrite=True, url=None): + def run_wizard(self, route="/success.iso", windows=False, overwrite=True, url=None, + caller_iso_dir=None): source = CREATOR.read_text() functions = "\n\n".join(re.findall(r"^function \w+\(\) \{\n.*?^\}", source, re.M | re.S)) self.assertIn("function create_vm()", functions) + # Only relocate this external filesystem anchor. Keep all predicates, + # transfers and promotion logic unchanged, without a production hook. + self.assertIn("/var/lib/vz/template/iso", functions) + functions = functions.replace("/var/lib/vz/template/iso", "${FIXTURE_DIR}") prelude = r''' translate() { printf '%s' "$1"; } msg_ok() { printf 'OK:%s\n' "$*"; } @@ -133,12 +139,15 @@ class VmIsoDownloadTests(unittest.TestCase): ''' env = dict(os.environ, PATH=str(self.bin) + os.pathsep + os.environ["PATH"], ISO_PATH=str(self.iso), ISO_URL=url if url is not None else self.http + route, - ISO_VOLID="local:iso/installation media.iso", ISO_DIR=str(self.directory), + ISO_VOLID="local:iso/installation media.iso", OS_TYPE="2" if windows else "3", QM_LOG=str(self.qm_log), WGET_LOG=str(self.wget_log), DIALOG_ONCE=str(self.directory / "dialog-once"), FIXTURE_DIR=str(self.directory), OVERWRITE_STATUS="0" if overwrite else "1", HTTP_FIXTURE=self.http, VIRTIO_FIXTURE_URL=self.http + route) + env.pop("ISO_DIR", None) + if caller_iso_dir is not None: + env["ISO_DIR"] = str(caller_iso_dir) return subprocess.run(["bash", "--noprofile", "--norc", "-c", functions + prelude], env=env, stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=15) @@ -166,14 +175,17 @@ class VmIsoDownloadTests(unittest.TestCase): self.assertEqual(self.iso.read_bytes(), PAYLOAD) self.assertIn("create ", self.qm_calls()) - def test_virtio_download_uses_the_configured_iso_directory(self): + def test_virtio_download_keeps_native_storage_despite_caller_environment(self): self.iso.write_bytes(PAYLOAD) - result = self.run_wizard(windows=True) + unrelated = self.directory / "unrelated directory" + unrelated.mkdir() + result = self.run_wizard(windows=True, caller_iso_dir=unrelated) self.assertEqual(result.returncode, 0, result.stdout + result.stderr) driver = self.directory / "virtio-win.iso" self.assertTrue(driver.is_file(), result.stdout + result.stderr) self.assertEqual(driver.read_bytes(), PAYLOAD) self.assertIn("-ide3 ", self.qm_calls()) + self.assertEqual(list(unrelated.iterdir()), []) def test_failed_virtio_fetch_is_not_selected_for_attachment(self): self.iso.write_bytes(PAYLOAD) diff --git a/scripts/vm/vm_creator.sh b/scripts/vm/vm_creator.sh index 5c5e1128..f81e84c6 100644 --- a/scripts/vm/vm_creator.sh +++ b/scripts/vm/vm_creator.sh @@ -285,7 +285,7 @@ function create_vm() { local BOOT_ORDER="" local DISK_INFO="" local DISK_INDEX=0 - local ISO_DIR="${ISO_DIR:-/var/lib/vz/template/iso}" + local ISO_DIR="/var/lib/vz/template/iso" if [[ -n "$ISO_PATH" && -n "$ISO_URL" ]] && [[ ! -f "$ISO_PATH" || ! -s "$ISO_PATH" ]]; then @@ -657,7 +657,7 @@ fi if [[ "$OS_TYPE" == "2" ]]; then - local VIRTIO_DIR="$ISO_DIR" + local VIRTIO_DIR="/var/lib/vz/template/iso" local VIRTIO_SELECTED="" local VIRTIO_VOLID="" local VIRTIO_DOWNLOAD_URL="https://fedorapeople.org/groups/virt/virtio-win/direct-downloads/stable-virtio/virtio-win.iso"