From 8be55031ab8250c82084019a1d66031816a9c155 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Mon, 24 Aug 2026 23:17:37 -0400 Subject: [PATCH] =?UTF-8?q?[FIX]=20suivi=20:=20effacer=20depuis=20un=20sui?= =?UTF-8?q?vi=20rouvert=20v=C3=A9rifie=20d'abord=20l'identit=C3=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le tableau de bord se rouvre sur un manifeste passé — c'est fait pour, les installations partent détachées. Mais un nom de domaine se réemploie et un VMID libéré est RÉATTRIBUÉ : effacer « le 101 » d'un run de mars, c'est effacer ce qui porte le 101 aujourd'hui, et « erplibre-ubuntu-2604 » de mars n'est pas celui d'aujourd'hui. Même famille que tout le reste — on jugeait sur le nom, avec ici la pire conséquence. La commande porte donc son garde, et non l'écran : elle protège ainsi tous ses appelants, et la vérification se fait SUR la machine, à l'instant d'effacer. Sur Proxmox, le VMID doit encore porter ce nom. En local, l'UUID du domaine — relevé au lancement, seul instant où l'on sait que ce nom désigne bien cette machine-là. Un manifeste écrit avant ce correctif n'en a pas : il retombe sur la protection d'avant plutôt que de bloquer. Le garde du VMID est une fonction à part, exécutable telle quelle. Il traverse deux « shlex.quote » avant d'atteindre un dash, et un garde qu'on ne sait pas éprouver s'OUVRE le jour où il casse. Vérifié sur erplibre-proxmox-9 sans rien détruire : le VMID 100 refusé sous un nom périmé, accepté sous le sien. --- EN --- The dashboard reopens on a past manifest — by design, since installs run detached. But a domain name gets reused and a freed VMID is REASSIGNED: deleting "the 101" from a March run deletes whatever holds 101 today, and March's "erplibre-ubuntu-2604" is not today's. Same family as the rest — we judged by name, here with the worst consequence. The command carries its guard, not the screen: that protects every caller, and the check happens ON the machine, at the moment of deletion. On Proxmox the VMID must still bear that name. Locally, the domain's UUID — recorded at launch, the only moment we know that name means that machine. A manifest written before this fix has none: it falls back to the previous protection rather than blocking. The VMID guard is its own function, runnable as is. It crosses two "shlex.quote" layers before reaching a dash, and a guard you cannot exercise OPENS the day it breaks. Verified on erplibre-proxmox-9 without destroying anything: VMID 100 refused under a stale name, accepted under its own. Assisted-by: Claude Opus 5 --- script/todo/qemu_install_monitor.py | 87 ++++++++++++++++++++++++++--- test/test_qemu_monitor_pve.py | 63 +++++++++++++++++++++ 2 files changed, 141 insertions(+), 9 deletions(-) diff --git a/script/todo/qemu_install_monitor.py b/script/todo/qemu_install_monitor.py index 5e085a1..c2a9849 100644 --- a/script/todo/qemu_install_monitor.py +++ b/script/todo/qemu_install_monitor.py @@ -313,6 +313,12 @@ def launch_installs(vms: list[dict], branch: str, remote_cmd: str) -> str: # Une VM posée sur un hôte Proxmox : c'est LUI qui connaît son état. if vm.get("pve"): entree["pve"] = vm["pve"] + else: + # L'UUID du domaine, relevé MAINTENANT : c'est le seul instant où + # l'on sait que ce nom désigne bien cette machine. Rouvert des + # semaines plus tard, le suivi ne peut plus le savoir — et c'est + # lui qui arme le garde de la suppression. + entree["uuid"] = local_uuid(vm["name"]) entries.append(entree) manifest = { "branch": branch, @@ -324,6 +330,26 @@ def launch_installs(vms: list[dict], branch: str, remote_cmd: str) -> str: return manifest_path +def local_uuid(name: str) -> str: + """UUID du domaine libvirt local, ou "" s'il est illisible. + + Sans sudo d'abord : l'appartenance au groupe libvirt suffit souvent. Une + chaîne vide DÉSARME le garde plutôt que de bloquer — mieux vaut la + protection d'avant que refuser toute suppression sur un poste où virsh + demande un mot de passe.""" + base = ["virsh", "--connect", "qemu:///system", "domuuid", name] + for argv in (base, ["sudo", "-n"] + base): + try: + res = subprocess.run( + argv, capture_output=True, text=True, timeout=15 + ) + except (OSError, subprocess.SubprocessError): + continue + if res.returncode == 0 and res.stdout.strip(): + return res.stdout.strip() + return "" + + def finished_at(log_path: str, fallback: float) -> float: """Instant où l'installation s'est RÉELLEMENT arrêtée. @@ -1743,15 +1769,41 @@ def restart_odoo_cmd() -> str: ) -def delete_vm_cmd_pve(info, purge: bool = True) -> str: +def pve_identity_guard(vmid: int, name: str) -> str: + """Shell qui S'ARRÊTE si le VMID ne porte plus ce nom. + + Un VMID libéré est RÉATTRIBUÉ, et le suivi se rouvre sur un manifeste qui + peut avoir des semaines : effacer « le 101 » d'un run de mars, c'est + effacer ce qui porte le 101 aujourd'hui. + + Une fonction à part, et exécutable telle quelle : c'est ce qui la rend + vérifiable. Enfouie dans la commande, elle ne se testait qu'à travers deux + « shlex.quote » — et un garde qu'on ne sait pas éprouver s'OUVRE le jour + où il casse, au lieu de se fermer.""" + q = shlex.quote(name) + return ( + f"vu=$(qm config {int(vmid)} 2>/dev/null" + " | sed -n 's/^name: //p' | head -1); " + f'if [ "$vu" != {q} ]; then ' + f'echo "REFUS : le VMID {int(vmid)} porte maintenant $vu,"' + f' "et non {name}. Rien n\'a ete efface."; exit 1; fi; ' + ) + + +def delete_vm_cmd_pve(info, purge: bool = True, name: str = "") -> str: """Efface une VM sur son hôte PROXMOX, par son VMID. « virsh undefine » y aurait effacé le domaine LOCAL homonyme — le - même piège que partout ailleurs, avec la pire conséquence.""" + même piège que partout ailleurs, avec la pire conséquence. + + `name` arme le garde d'identité (voir `pve_identity_guard`) : sans lui, la + commande efface le VMID quoi qu'il porte aujourd'hui.""" vmid = int((info or {}).get("vmid") or 0) - suite = ( + suite = pve_identity_guard(vmid, name) if name else "" + suite += ( f"qm stop {vmid} --skiplock 1 || true; " - f"qm destroy {vmid}{' --purge 1 --destroy-unreferenced-disks 1' if purge else ''}" + f"qm destroy {vmid}" + f"{' --purge 1 --destroy-unreferenced-disks 1' if purge else ''}" ) return pve_host_cmd(info, suite) @@ -1789,12 +1841,26 @@ def delete_lines(vm) -> list: ] -def delete_vm_cmd(name: str, with_disks: bool) -> str: +def delete_vm_cmd(name: str, with_disks: bool, uuid: str = "") -> str: """Efface la VM sur l'HÔTE. Même séquence que « TODO._qemu_delete_vm » : arrêt, retrait de la définition (nvram si UEFI, repli sinon), puis les - disques à la demande.""" + disques à la demande. + + `uuid` arme un GARDE. Le suivi se rouvre sur un manifeste passé, et un nom + de domaine se réemploie : « erplibre-ubuntu-2604 » d'un run de mars n'est + pas forcément celui d'aujourd'hui. L'UUID, lui, naît avec le domaine et + meurt avec lui — c'est la seule chose qui distingue deux machines du même + nom.""" q = shlex.quote(name) - cmd = ( + cmd = "" + if uuid: + cmd = ( + f"vu=$(sudo virsh domuuid {q} 2>/dev/null | tr -d '[:space:]'); " + f'if [ "$vu" != {shlex.quote(uuid)} ]; then ' + f'echo "REFUS : {name} n\'est plus le même domaine"' + f' "($vu). Rien n\'a été effacé."; exit 1; fi; ' + ) + cmd += ( f"sudo virsh destroy {q} 2>/dev/null; " f"sudo virsh undefine {q} --nvram 2>/dev/null " f"|| sudo virsh undefine {q}" @@ -3081,10 +3147,13 @@ def run_monitor(manifest_path: str, run_app: bool = True): if not yes: return info = vm.get("pve") + # Le garde d'identité voyage avec la VM : c'est ce qui + # rend une suppression sûre depuis un suivi ROUVERT, dont le + # manifeste peut avoir des semaines. cmd = ( - delete_vm_cmd_pve(info) + delete_vm_cmd_pve(info, name=vm["name"]) if info - else delete_vm_cmd(vm["name"], True) + else delete_vm_cmd(vm["name"], True, vm.get("uuid") or "") ) with self.suspend(): print(f"\n=== Suppression — {vm['name']} ===") diff --git a/test/test_qemu_monitor_pve.py b/test/test_qemu_monitor_pve.py index d69f5ef..ef94522 100644 --- a/test/test_qemu_monitor_pve.py +++ b/test/test_qemu_monitor_pve.py @@ -230,6 +230,69 @@ class TestLesAutresCheminsVersLaPoubelle(unittest.TestCase): self.assertGreaterEqual(mon.PVE_ABSENCES_AVANT_EFFACEE, 2) +class TestEffacerDepuisUnSuiviRouvert(unittest.TestCase): + """Le suivi se ROUVRE sur un manifeste passé — c'est fait pour. + + Mais un nom de domaine se réemploie, et un VMID libéré est RÉATTRIBUÉ. + Effacer « le 101 » d'un run de mars, c'est effacer ce qui porte le 101 + aujourd'hui, et « erplibre-ubuntu-2604 » de mars n'est pas celui + d'aujourd'hui. Même famille que tout le reste : on jugeait sur le nom. + + La commande porte donc son garde. Dans la commande et non dans l'écran : + elle protège ainsi tous ses appelants, et la vérification se fait SUR la + machine, à l'instant d'effacer.""" + + def test_a_proxmox_delete_checks_the_vmid_still_bears_the_name(self): + cmd = mon.delete_vm_cmd_pve( + {"target": "pve9", "vmid": 101}, name="vm-a" + ) + self.assertIn("qm config 101", cmd) + self.assertIn("exit 1", cmd) + self.assertIn("qm destroy 101", cmd) + # Le garde vient AVANT la destruction, sinon il ne garde rien. + self.assertLess(cmd.index("qm config 101"), cmd.index("qm destroy")) + + def test_a_local_delete_checks_the_uuid(self): + cmd = mon.delete_vm_cmd("vm-a", True, "5d55d05a-1e77") + self.assertIn("virsh domuuid vm-a", cmd) + self.assertIn("5d55d05a-1e77", cmd) + self.assertLess(cmd.index("domuuid"), cmd.index("virsh destroy vm-a")) + + def test_an_old_manifest_without_identity_still_deletes(self): + # Un manifeste écrit avant ce correctif n'a pas d'UUID. Refuser toute + # suppression y serait une régression : on retombe sur la protection + # d'avant, la confirmation à deux mains. + cmd = mon.delete_vm_cmd("vm-a", True) + self.assertNotIn("domuuid", cmd) + self.assertIn("virsh undefine vm-a", cmd) + sans_nom = mon.delete_vm_cmd_pve({"target": "pve9", "vmid": 101}) + self.assertNotIn("qm config", sans_nom) + self.assertIn("qm destroy 101", sans_nom) + + def test_the_guard_is_shell_correct(self): + """Le garde est EXÉCUTÉ, « qm » bouchonné par une fonction shell. + + Il traverse ensuite deux « shlex.quote » avant d'atteindre un dash : + chaque niveau est une occasion de le casser, et un garde cassé + s'OUVRE au lieu de se fermer. Éprouvé aussi sur le vrai hôte, où il a + refusé un nom périmé et laissé passer le bon.""" + import subprocess + + garde = mon.pve_identity_guard(101, "vm-a") + for vu, attendu in (("vm-a", 0), ("autre-vm", 1)): + res = subprocess.run( + [ + "sh", + "-c", + f"qm() {{ echo 'name: {vu}'; }}; {garde} exit 0", + ], + capture_output=True, + text=True, + ) + self.assertEqual(res.returncode, attendu, f"{vu} : {res.stdout}") + self.assertIn("vm-a", garde) + + class TestCeQueLaConfirmationPromet(unittest.TestCase): """La confirmation de suppression annonçait un fichier qcow2 local à TOUTE VM, Proxmox comprise.