[FIX] suivi : effacer depuis un suivi rouvert vérifie d'abord l'identité

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

@@ -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 <nom> » 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']} ===")

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.