Merge pull request #458 from f3rs3n/fix/vm-iso-downloads

fix(vm): reject failed ISO downloads and repair image sources
This commit is contained in:
MacRimi
2026-10-16 17:14:53 +02:00
committed by GitHub
5 changed files with 523 additions and 28 deletions
@@ -0,0 +1,275 @@
"""The VM wizard must not accept failed ISO downloads as installation media.
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
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,
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' "$*"; }
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",
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)
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_keeps_native_storage_despite_caller_environment(self):
self.iso.write_bytes(PAYLOAD)
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)
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()
@@ -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, "<html>No release links</html>"),
):
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 = (
'<a href="https://rockstor.com/downloads/installer/leap/15.6/x86_64/'
'Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso">'
'Rockstor-Leap15.6-generic.x86_64-5.1.0-0.install.iso</a> <br/>'
)
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, "<html>No installer links</html>"),
):
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 = (
'<a href="/projects/openmediavault/files/iso/8.3.1/" '
'title="Click to enter 8.3.1">8.3.1</a>\n'
'<a href="/projects/openmediavault/files/iso/7.4.17/" '
'title="Click to enter 7.4.17">7.4.17</a>'
)
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 = ('<a href="/projects/openmediavault/files/iso/8.10.0/">new</a>'
'<a href="/projects/openmediavault/files/iso/8.9.0/">old</a>')
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()