From e626da5002fcc07a5802c34a66074783f09a9c40 Mon Sep 17 00:00:00 2001 From: catborise Date: Fri, 2 Oct 2026 23:14:28 +0300 Subject: [PATCH] fix(instances): do not refuse clones for volume-less disks or unusual names Clone refused VMs that cloned fine before: a disk without a volume got a None disk name, and a source image with characters such as a space or "@" produced a derived name outside the allowed pattern. Disks without a volume (which clone_instance does not copy) are no longer validated, and derived disk names are sanitized to the allowed charset. Names typed by a superuser are still validated strictly. --- instances/test_clone_permission.py | 21 +++++++++++++++++++++ instances/utils.py | 5 ++++- instances/views.py | 5 ++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/instances/test_clone_permission.py b/instances/test_clone_permission.py index 6cd04289..9794c8fe 100644 --- a/instances/test_clone_permission.py +++ b/instances/test_clone_permission.py @@ -167,3 +167,24 @@ class ClonePermissionTestCase(TestCase): self.assertEqual(clone_data["disk-vda"], "custom-name.qcow2") self.assertEqual(clone_data["meta-vda"], "true") self.assertEqual(clone_data["clone-net-mac-0"], "52:54:00:12:34:56") + + def test_non_superuser_clone_skips_disks_without_a_volume(self): + empty_disk = {"dev": "vdb", "image": None, "storage": None, "path": None, + "size": 0, "format": None} + with self.mocked_libvirt() as proxy: + proxy.get_disk_devices.return_value = [SRC_DISK, empty_disk] + self._clone(self.owner_change, self._valid_post()) + proxy.clone_instance.assert_called_once() + clone_data = proxy.clone_instance.call_args[0][0] + self.assertEqual(clone_data["disk-vda"], "clone-vm.qcow2") + self.assertIsNone(clone_data["disk-vdb"]) + + def test_derived_disk_names_are_sanitized_instead_of_rejected(self): + odd_disk = dict(SRC_DISK, image="my disk@2.qcow2") + with self.mocked_libvirt() as proxy: + proxy.get_disk_devices.return_value = [odd_disk] + self._clone(self.owner_change, self._valid_post()) + proxy.clone_instance.assert_called_once() + self.assertEqual( + proxy.clone_instance.call_args[0][0]["disk-vda"], "my-disk-2-clone.qcow2" + ) diff --git a/instances/utils.py b/instances/utils.py index 51707c72..b293c115 100644 --- a/instances/utils.py +++ b/instances/utils.py @@ -1,5 +1,6 @@ import os import random +import re import string from accounts.models import UserInstance, UserAttributes @@ -263,7 +264,9 @@ def get_clone_disk_name(disk, prefix, clone_name=""): image = f"{name}-clone.{suffix}" else: image = f"{disk['image']}-clone" - return image + # Derived names go into volume XML and paths: keep them to a safe charset. + image = re.sub(r"[^A-Za-z0-9_.+-]+", "-", image).lstrip("_.+-") + return image or None def can_manage_console(user, instance): diff --git a/instances/views.py b/instances/views.py index ab9122c1..1813238f 100755 --- a/instances/views.py +++ b/instances/views.py @@ -1665,7 +1665,10 @@ def clone(request, pk): invalid_disks = [ value for key, value in clone_data.items() - if key.startswith("disk-") and not VOLUME_NAME_RE.fullmatch(value or "") + # None: a disk without a volume, which clone_instance does not copy. + if key.startswith("disk-") + and value is not None + and not VOLUME_NAME_RE.fullmatch(value) ] if not request.user.is_superuser and quota_msg: