Found in review of #370. Smaller items, grouped because each is a few lines; split out if any needs its own discussion.
1. The probe hardcodes the storage pool "default" — lnvps_api/src/provisioner/probe.rs:213. The customer path takes the pool name from the vm_host_disk row (libvirt/mod.rs:99,137,595). A node whose disk is registered under any other name makes the probe either fail outright or measure a pool no customer is placed on — the opposite of what the comment above it claims. Read db.list_host_disks(host.id) and fall back to "default".
2. is_probe is never called outside tests (probe.rs:56). A leaked probe domain (VM18446462598732840961) parses cleanly through vm_id_from_domain_name (libvirt/xml.rs:32) and therefore shows up as an importable VM in list_unmanaged_vms (worker.rs:1408-1424); import_vm would then call insert_vm_with_id with a probe id. It fails today only because vm.id is integer unsigned and the value is out of range — an accident, not a guard. Filter is_probe in both places.
3. Node-controlled failure text is stored unbounded. probe_ssh.rs:154,196,217 interpolate guest-controlled output into the error and probe.rs:293 interpolates the node's error; that string goes into failure TEXT (65535 bytes, migrations/20260813120000_marketplace_node_health.sql:35). Over the cap, a strict-mode MySQL rejects the insert, so the health row is never written — and a probe with no health row is one with no cooldown, so the node is re-probed on the next sweep, building a VM each time. Truncate in ProbeResult::failed / into_health. image VARCHAR(255) versus VmOsImage.url has the same shape.
4. get_host_client's marketplace arm has no blank-host.ip guard (lnvps_api_common/src/host/mod.rs:220-237). node_control::endpoint treats a blank address as a hard error and approve_node states that invariant explicitly, but this arm checks only for cacert.pem and then builds qemu+tls://:16514/system?pkipath=.... Reachable: approval creates the host with ip: String::new(), and a node can register its libvirt certificate before requesting a tunnel, so the anchor file can exist while ip is empty. The host is disabled at that point, so the path is thin, but the failure is an opaque libvirt connect error rather than the named one the design calls for.
5. The per-node trust anchor set is two anchors, not one (marketplace_pki.rs:54): format!("{}\n{}", node_cert_pem, lnvps_ca). gnutls accepts any chain validating to any certificate in the file, so the LNVPS CA is a second trust root for every node connection. Not exploitable today — nodes only ever receive the CA's public PEM, libvirt also checks serverAuth EKU and the SAN against the dialled address, and the only LNVPS-CA-signed key that exists is the client key on the API host — but the moment that CA signs any server-capable certificate, per-node pinning silently degrades to "any LNVPS-CA certificate". Worth a comment stating the invariant, since neither the doc comment at :41-52 nor the test names acknowledge the second anchor. The node side has the symmetric weakening at libvirt.rs:178-190, which at least documents it.
6. MockDb accepts a caller-supplied created on health rows that MySQL ignores (mock.rs:3739-3747 vs mysql.rs:4204-4207, where the column always takes DEFAULT CURRENT_TIMESTAMP). A future test that backdates health rows to exercise trend or retention logic would pass against the mock and be impossible to reproduce against the real database. No current test does this, so it is latent.
7. cloud-init: the PR documents a defect it does not fix (lnvps_api_common/src/host/cloud_init.rs:74-85 vs :117-130). The new comment says widening the prefix makes the guest treat everything in that prefix as on-link and ARP for addresses nothing answers for on a routed block — but :85 still does range.prefix().min(range_gw.prefix()), and the new on_link closure only emits on-link: true when the gateway is outside every widened prefix, which the widening itself almost always prevents. The two mechanisms solve the same problem and the widening wins. The same widening is duplicated in the ipconfig path (proxmox.rs:1197,1218) that every existing single-address customer VM uses. Either drop the widening and let on-link carry it, or delete the contradicting comment. Also: on_link is the only behavioural change in that file and has no test.
Found in review of #370. Smaller items, grouped because each is a few lines; split out if any needs its own discussion.
1. The probe hardcodes the storage pool
"default"—lnvps_api/src/provisioner/probe.rs:213. The customer path takes the pool name from thevm_host_diskrow (libvirt/mod.rs:99,137,595). A node whose disk is registered under any other name makes the probe either fail outright or measure a pool no customer is placed on — the opposite of what the comment above it claims. Readdb.list_host_disks(host.id)and fall back to"default".2.
is_probeis never called outside tests (probe.rs:56). A leaked probe domain (VM18446462598732840961) parses cleanly throughvm_id_from_domain_name(libvirt/xml.rs:32) and therefore shows up as an importable VM inlist_unmanaged_vms(worker.rs:1408-1424);import_vmwould then callinsert_vm_with_idwith a probe id. It fails today only becausevm.idisinteger unsignedand the value is out of range — an accident, not a guard. Filteris_probein both places.3. Node-controlled failure text is stored unbounded.
probe_ssh.rs:154,196,217interpolate guest-controlled output into the error andprobe.rs:293interpolates the node's error; that string goes intofailure TEXT(65535 bytes,migrations/20260813120000_marketplace_node_health.sql:35). Over the cap, a strict-mode MySQL rejects the insert, so the health row is never written — and a probe with no health row is one with no cooldown, so the node is re-probed on the next sweep, building a VM each time. Truncate inProbeResult::failed/into_health.image VARCHAR(255)versusVmOsImage.urlhas the same shape.4.
get_host_client's marketplace arm has no blank-host.ipguard (lnvps_api_common/src/host/mod.rs:220-237).node_control::endpointtreats a blank address as a hard error andapprove_nodestates that invariant explicitly, but this arm checks only forcacert.pemand then buildsqemu+tls://:16514/system?pkipath=.... Reachable: approval creates the host withip: String::new(), and a node can register its libvirt certificate before requesting a tunnel, so the anchor file can exist whileipis empty. The host is disabled at that point, so the path is thin, but the failure is an opaque libvirt connect error rather than the named one the design calls for.5. The per-node trust anchor set is two anchors, not one (
marketplace_pki.rs:54):format!("{}\n{}", node_cert_pem, lnvps_ca). gnutls accepts any chain validating to any certificate in the file, so the LNVPS CA is a second trust root for every node connection. Not exploitable today — nodes only ever receive the CA's public PEM, libvirt also checks serverAuth EKU and the SAN against the dialled address, and the only LNVPS-CA-signed key that exists is the client key on the API host — but the moment that CA signs any server-capable certificate, per-node pinning silently degrades to "any LNVPS-CA certificate". Worth a comment stating the invariant, since neither the doc comment at:41-52nor the test names acknowledge the second anchor. The node side has the symmetric weakening atlibvirt.rs:178-190, which at least documents it.6.
MockDbaccepts a caller-suppliedcreatedon health rows that MySQL ignores (mock.rs:3739-3747vsmysql.rs:4204-4207, where the column always takesDEFAULT CURRENT_TIMESTAMP). A future test that backdates health rows to exercise trend or retention logic would pass against the mock and be impossible to reproduce against the real database. No current test does this, so it is latent.7. cloud-init: the PR documents a defect it does not fix (
lnvps_api_common/src/host/cloud_init.rs:74-85vs:117-130). The new comment says widening the prefix makes the guest treat everything in that prefix as on-link and ARP for addresses nothing answers for on a routed block — but:85still doesrange.prefix().min(range_gw.prefix()), and the newon_linkclosure only emitson-link: truewhen the gateway is outside every widened prefix, which the widening itself almost always prevents. The two mechanisms solve the same problem and the widening wins. The same widening is duplicated in theipconfigpath (proxmox.rs:1197,1218) that every existing single-address customer VM uses. Either drop the widening and leton-linkcarry it, or delete the contradicting comment. Also:on_linkis the only behavioural change in that file and has no test.