From 802e3cd05bd1b718aa34d6b319ef7f55688fab22 Mon Sep 17 00:00:00 2001 From: v0l Date: Fri, 17 Jul 2026 20:05:48 +0100 Subject: [PATCH 1/3] feat(admin): permanently delete never-paid VMs and super-admin VM purge Never-paid (new) VMs are now hard-deleted from the database instead of soft-deleted, both in the worker's unpaid-VM cleanup and in admin deletes. Adds a super_admin-only `purge` flag to DELETE /api/admin/v1/vms/{id} to permanently delete any VM (including ones with payment history), clearing up all related entities. - New LNVpsDb::hard_delete_vm removes the VM plus vm_history, vm_firewall_rule, vm_ip_assignment, and the VM's subscription (line items + payment history) - WorkJob::DeleteVm gains a purge flag; worker skips vm_history logging on hard delete (the row is gone) - AdminAuth::is_super_admin gates the purge flag (403 for non-super-admins) - Tests: mock + provisioner purge coverage, updated worker cleanup asserts, e2e RBAC purge-forbidden test + delete_auth_body helper - Docs: API_CHANGELOG.md + ADMIN_API_ENDPOINTS.md Fixes #168 --- ADMIN_API_ENDPOINTS.md | 11 +- API_CHANGELOG.md | 4 + lnvps_api/src/provisioner/rollback_tests.rs | 59 +++++++-- lnvps_api/src/provisioner/vm.rs | 25 +++- lnvps_api/src/subscription/vm.rs | 2 +- lnvps_api/src/worker.rs | 71 +++++++---- lnvps_api_admin/src/admin/auth.rs | 15 +++ lnvps_api_admin/src/admin/vms.rs | 16 +++ lnvps_api_common/src/mock.rs | 126 ++++++++++++++++++++ lnvps_api_common/src/work/mod.rs | 6 + lnvps_db/src/lib.rs | 12 ++ lnvps_db/src/mysql.rs | 52 ++++++++ lnvps_e2e/src/client.rs | 22 ++++ lnvps_e2e/src/rbac.rs | 18 +++ 14 files changed, 399 insertions(+), 40 deletions(-) diff --git a/ADMIN_API_ENDPOINTS.md b/ADMIN_API_ENDPOINTS.md index 320d0bad..696f94f8 100644 --- a/ADMIN_API_ENDPOINTS.md +++ b/ADMIN_API_ENDPOINTS.md @@ -427,10 +427,19 @@ Body (optional): ```json { - "reason": "string" + "reason": "string", + "purge": false } ``` +- `reason` — optional admin reason recorded in the VM history audit trail. +- `purge` — optional. When `true`, permanently deletes the VM and **all** related + records (history, firewall rules, IP assignments, and its subscription, line + items and payment history) instead of soft-deleting. Requires the + `super_admin` role — non-super-admins receive `403`. Intended for removing + test VMs. VMs that have never been paid are always purged regardless of this + flag. + Response: ```json diff --git a/API_CHANGELOG.md b/API_CHANGELOG.md index 06d35e02..5f73155e 100644 --- a/API_CHANGELOG.md +++ b/API_CHANGELOG.md @@ -23,6 +23,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - `DELETE /api/admin/v1/users/{id}/passkeys/{passkey_id}` (`users::update`) revokes a single passkey. Returns `404` if the passkey doesn't belong to the user, and refuses (`400`) to remove the **last** passkey of a passwordless (`webauthn`) account to avoid locking the user out. - `GET /api/admin/v1/users/{id}` now also returns `account_type` (`nostr` | `oauth` | `webauthn`) and `passkey_count`. +- **Permanently delete VMs (admin)** — `DELETE /api/admin/v1/vms/{id}` now accepts an optional `purge` flag in the request body (issue #168). + - VMs that have **never been paid** (their subscription was never set up) are now **hard-deleted** from the database instead of being soft-deleted, both when an admin deletes them and when the worker's hourly cleanup removes unpaid VMs. Nothing is left behind: the VM row and all related records (history, firewall rules, IP assignments, and the VM's own subscription + line items + payment history) are removed. + - `purge: true` lets a **super_admin** permanently delete *any* VM — including VMs with payment history — clearing up all related entities. This is intended for removing test VMs. The flag is rejected with `403` for non-super-admins (checked before the VM lookup); regular deletes and never-paid purges are unaffected. + - **Import existing host VMs (admin)** — new admin endpoints to adopt VMs that exist on a host but aren't tracked in the database (issue #166). - `GET /api/admin/v1/hosts/{id}/vms/unmanaged` lists VMs present on the host with no matching database record (returns host vmid, mapped database id, name, CPU/memory/disk specs, storage, MAC, running state). The admin service dispatches a discovery job to the worker and reads the reply over a temporary Redis pub/sub channel. - `POST /api/admin/v1/hosts/{id}/vms/import` (`{ host_vm_id, user_id, reason? }`) imports one VM: it is assigned to `user_id` and billed via the region's **custom pricing** (required — import fails if the region has none), capturing the VM's current CPU/memory/disk specs into a custom template. Currently supports Proxmox hosts. Returns a `job_id`. diff --git a/lnvps_api/src/provisioner/rollback_tests.rs b/lnvps_api/src/provisioner/rollback_tests.rs index 74928dbc..b714566a 100644 --- a/lnvps_api/src/provisioner/rollback_tests.rs +++ b/lnvps_api/src/provisioner/rollback_tests.rs @@ -147,7 +147,7 @@ mod tests { ); // Now clean up and verify cleanup works (this tests the delete_vm rollback) - provisioner.delete_vm(vm.id).await?; + provisioner.delete_vm(vm.id, false).await?; // Verify ARP entry for this VM was removed let cleanup_arp_entries = router.list_arp_entry().await?; @@ -203,7 +203,7 @@ mod tests { ); // Delete the VM to trigger cleanup (simulating rollback scenario) - provisioner.delete_vm(vm.id).await?; + provisioner.delete_vm(vm.id, false).await?; // Verify IP assignments are marked deleted let ips_after = db.list_vm_ip_assignments(vm.id).await?; @@ -290,7 +290,7 @@ mod tests { ); // Delete the VM - provisioner.delete_vm(vm.id).await?; + provisioner.delete_vm(vm.id, false).await?; // Verify DNS records are removed (count should be back to initial or less) { @@ -358,7 +358,7 @@ mod tests { } // Delete VM - provisioner.delete_vm(vm.id).await?; + provisioner.delete_vm(vm.id, false).await?; // After delete - IPs should be soft-deleted (marked deleted=true) let ips_after_delete = db.list_vm_ip_assignments(vm.id).await?; @@ -408,7 +408,7 @@ mod tests { assert_eq!(ips.len(), 2, "Should still have exactly 2 IP assignments"); // Cleanup - provisioner.delete_vm(vm.id).await?; + provisioner.delete_vm(vm.id, false).await?; Ok(()) } @@ -448,7 +448,7 @@ mod tests { ); // Delete and verify cleanup - provisioner.delete_vm(vm.id).await?; + provisioner.delete_vm(vm.id, false).await?; // The ARP entry with this MAC should be gone let router = MockRouter::new(); @@ -619,7 +619,7 @@ mod tests { assert!(arp_before.iter().any(|e| e.mac_address == mac_address)); // Delete the VM - provisioner.delete_vm(vm_id).await?; + provisioner.delete_vm(vm_id, false).await?; // Verify complete cleanup: // VM should be soft-deleted (deleted = true), matching production MySQL behavior @@ -784,7 +784,7 @@ mod tests { assert!(!ips_before.is_empty(), "Should have IP assignments"); // Delete should work - let result = provisioner.delete_vm(vm_id).await; + let result = provisioner.delete_vm(vm_id, false).await; assert!(result.is_ok(), "Delete should succeed"); // Verify VM is soft-deleted (matching production MySQL behavior) @@ -800,6 +800,49 @@ mod tests { Ok(()) } + /// A purging delete (`purge = true`, used for never-paid VMs and super-admin + /// forced deletions) removes the VM row entirely and hard-deletes its IP + /// assignments instead of soft-deleting them. + #[tokio::test] + async fn test_delete_vm_purge_removes_vm_row() -> Result<()> { + clear_mock_state().await; + let settings = mock_settings(); + let db = Arc::new(MockDb::default()); + let rates = Arc::new(MockExchangeRate::new()); + const MOCK_RATE: f32 = 69_420.0; + rates.set_rate(Ticker::btc_rate("EUR")?, MOCK_RATE).await; + + setup_db_with_static_arp(&db).await?; + + let provisioner = VmProvisioner::new(settings, db.clone()); + + let (user, ssh_key) = add_user(&db).await?; + let vm = provisioner + .provision(user.id, 1, 1, ssh_key.id, None) + .await?; + let vm_id = vm.id; + + let pipeline = provisioner.spawn_vm_pipeline(vm_id).await?; + pipeline.execute().await?; + assert!(!db.list_vm_ip_assignments(vm_id).await?.is_empty()); + + // Purge the VM. + provisioner.delete_vm(vm_id, true).await?; + + // The VM row is gone entirely (not just soft-deleted). + assert!( + db.get_vm(vm_id).await.is_err(), + "VM row should be hard-deleted" + ); + // IP assignments are hard-deleted too. + assert!( + db.list_vm_ip_assignments(vm_id).await?.is_empty(), + "IP assignments should be hard-deleted" + ); + + Ok(()) + } + /// Test that when the router's generate_mac returns an ArpEntry, the vm.mac_address /// is set from ArpEntry.mac_address (the actual MAC) and not ArpEntry.address (the IP). /// This covers the regression where mac and IP were mixed up in the OVH router path. diff --git a/lnvps_api/src/provisioner/vm.rs b/lnvps_api/src/provisioner/vm.rs index d5ed9ffb..9e498a3b 100644 --- a/lnvps_api/src/provisioner/vm.rs +++ b/lnvps_api/src/provisioner/vm.rs @@ -673,8 +673,17 @@ impl VmProvisioner { )) } - /// Delete a VM and its associated resources - pub async fn delete_vm(&self, vm_id: u64) -> OpResult<()> { + /// Delete a VM and its associated resources. + /// + /// When `purge` is set the VM is permanently removed from the database along + /// with all of its related records (history, firewall rules, IP + /// assignments, and its subscription + payment history) via + /// [`LNVpsDb::hard_delete_vm`]. Otherwise the VM is soft-deleted + /// (`deleted = 1`), matching production behaviour for paid VMs. + /// + /// Callers pass `purge = true` for never-paid (new) VMs and super-admin + /// forced deletions so that they leave nothing behind in the database. + pub async fn delete_vm(&self, vm_id: u64, purge: bool) -> OpResult<()> { let vm = self.db.get_vm(vm_id).await?; let host = self.db.get_host(vm.host_id).await?; @@ -684,8 +693,14 @@ impl VmProvisioner { .step("delete_ips", |ctx| { Box::pin(ctx.2.delete_all_ip_assignments(vm_id)) }) - .step("delete_vm_db", |ctx| { - Box::pin(async { Ok(ctx.0.delete_vm(vm_id).await?) }) + .step("delete_vm_db", move |ctx| { + Box::pin(async move { + if purge { + Ok(ctx.0.hard_delete_vm(vm_id).await?) + } else { + Ok(ctx.0.delete_vm(vm_id).await?) + } + }) }); pipeline.execute().await?; Ok(()) @@ -1231,7 +1246,7 @@ mod tests { } // now expire - provisioner.delete_vm(vm.id).await?; + provisioner.delete_vm(vm.id, false).await?; // test arp/dns is removed let arp = router.list_arp_entry().await?; diff --git a/lnvps_api/src/subscription/vm.rs b/lnvps_api/src/subscription/vm.rs index e1f1c6a5..98067a1d 100644 --- a/lnvps_api/src/subscription/vm.rs +++ b/lnvps_api/src/subscription/vm.rs @@ -249,7 +249,7 @@ impl SubscriptionLineItemHandler for VmLineItemHandler { return Ok(()); } - if let Err(e) = self.provisioner.delete_vm(vm_id).await { + if let Err(e) = self.provisioner.delete_vm(vm_id, false).await { warn!("Failed to delete expired VM {}: {}", vm_id, e); } else { if let Err(e) = self diff --git a/lnvps_api/src/worker.rs b/lnvps_api/src/worker.rs index 30701e01..06f8bd99 100644 --- a/lnvps_api/src/worker.rs +++ b/lnvps_api/src/worker.rs @@ -1107,7 +1107,9 @@ impl Worker { continue; } info!("Deleting unpaid VM {}", vm.id); - if let Err(e) = provisioner.delete_vm(vm.id).await { + // Never-paid (new) VMs carry no customer data, so purge them entirely + // rather than leaving a soft-deleted row and orphaned subscription. + if let Err(e) = provisioner.delete_vm(vm.id, true).await { error!("Failed to delete unpaid VM {}: {}", vm.id, e); self.queue_admin_notification( format!("Failed to delete unpaid VM {}:\n{}", vm.id, e), @@ -2095,31 +2097,49 @@ impl Worker { vm_id, reason, admin_user_id, + purge, } => { let vm = self.db.get_vm(*vm_id).await?; if vm.deleted { return Ok(None); } + // A VM that has never had its first (purchase) payment confirmed + // carries no customer data, so purge it entirely. Super-admins + // can also force a purge of any VM (including paid ones) via the + // `purge` flag. + let ever_paid = self + .db + .get_subscription_by_line_item_id(vm.subscription_line_item_id) + .await + .map(|s| s.is_setup) + .unwrap_or(false); + let hard_delete = *purge || !ever_paid; + // Delete the VM via provisioner let provisioner = self.subscription_handler.vm_provisioner(); - provisioner.delete_vm(*vm_id).await?; - - // Log VM deletion - let metadata = if let Some(admin_id) = admin_user_id { - Some(serde_json::json!({ - "admin_user_id": admin_id, - "admin_action": true - })) - } else { - Some(serde_json::json!({ - "admin_action": true - })) - }; - - self.vm_history_logger - .log_vm_deleted(*vm_id, *admin_user_id, reason.as_deref(), metadata) - .await?; + provisioner.delete_vm(*vm_id, hard_delete).await?; + + // A hard delete removes the VM row (and its history), so logging + // a vm_history entry afterwards would fail the foreign key. + // Only record deletion history for soft-deleted VMs. + if !hard_delete { + // Log VM deletion + let metadata = if let Some(admin_id) = admin_user_id { + Some(serde_json::json!({ + "admin_user_id": admin_id, + "admin_action": true + })) + } else { + Some(serde_json::json!({ + "admin_action": true + })) + }; + + self.vm_history_logger + .log_vm_deleted(*vm_id, *admin_user_id, reason.as_deref(), metadata) + .await?; + } // Send notifications let reason_text = reason.as_deref().unwrap_or("Admin requested deletion"); @@ -3274,10 +3294,12 @@ mod tests { let worker = setup_worker(db.clone()).await?; worker.check_vms().await?; - // VM should be soft-deleted + // Never-paid VMs are purged entirely, not just soft-deleted. let vms = db.vms.lock().await; - let deleted = vms.get(&vm_id).map(|v| v.deleted).unwrap_or(false); - assert!(deleted, "Unpaid VM older than 1 hour should be deleted"); + assert!( + !vms.contains_key(&vm_id), + "Unpaid VM older than 1 hour should be purged" + ); Ok(()) } @@ -3383,12 +3405,11 @@ mod tests { let worker = setup_worker(db.clone()).await?; worker.check_vms().await?; - // VM should be soft-deleted because the only payment is expired. + // VM should be purged because the only payment is expired. let vms = db.vms.lock().await; - let deleted = vms.get(&vm_id).map(|v| v.deleted).unwrap_or(false); assert!( - deleted, - "Unpaid VM with only an expired payment should still be deleted" + !vms.contains_key(&vm_id), + "Unpaid VM with only an expired payment should still be purged" ); Ok(()) } diff --git a/lnvps_api_admin/src/admin/auth.rs b/lnvps_api_admin/src/admin/auth.rs index 1f9314a7..50a6b5e7 100644 --- a/lnvps_api_admin/src/admin/auth.rs +++ b/lnvps_api_admin/src/admin/auth.rs @@ -44,6 +44,21 @@ impl AdminAuth { }) } + /// Check whether the authenticated admin holds the `super_admin` role. + /// + /// Permissions alone can't express "super admin only" actions (a custom role + /// could be granted the same permission tuples), so destructive operations + /// like permanently purging a paid VM are gated on the role by name. + pub async fn is_super_admin(&self, db: &Arc) -> Result { + let role_ids = db.get_user_roles(self.user_id).await?; + for role_id in role_ids { + if db.get_role(role_id).await?.name == "super_admin" { + return Ok(true); + } + } + Ok(false) + } + /// Check if the authenticated admin has a specific permission pub fn has_permission(&self, resource: AdminResource, action: AdminAction) -> bool { self.permissions.contains(&Permission { resource, action }) diff --git a/lnvps_api_admin/src/admin/vms.rs b/lnvps_api_admin/src/admin/vms.rs index 0f9a7a66..0d6b0d00 100644 --- a/lnvps_api_admin/src/admin/vms.rs +++ b/lnvps_api_admin/src/admin/vms.rs @@ -389,6 +389,11 @@ async fn admin_stop_vm( #[serde(default)] struct AdminDeleteVmRequest { reason: Option, + /// Permanently purge the VM and all related records (history, payments, + /// subscription) from the database, even if it has payment history. + /// Requires the `super_admin` role. Never-paid VMs are always purged + /// regardless of this flag. + purge: Option, } #[derive(Deserialize)] @@ -415,6 +420,16 @@ async fn admin_delete_vm( // Check permission auth.require_permission(AdminResource::VirtualMachines, AdminAction::Delete)?; + // Purging a VM with payment history is destructive and irreversible, so it + // is restricted to super-admins. Never-paid VMs are purged automatically by + // the worker regardless of this flag. Authorize before looking up the VM. + let purge = req.purge.unwrap_or(false); + if purge && !auth.is_super_admin(&this.db).await? { + return Err(ApiError::forbidden( + "Only super admins can permanently purge a VM", + )); + } + // Verify VM exists let vm = this.db.get_vm(id).await?; @@ -427,6 +442,7 @@ async fn admin_delete_vm( vm_id: id, reason: req.reason.clone(), admin_user_id: Some(auth.user_id), + purge, }; match this.work_commander.send(delete_job).await { diff --git a/lnvps_api_common/src/mock.rs b/lnvps_api_common/src/mock.rs index 155d3a8b..3c107d87 100644 --- a/lnvps_api_common/src/mock.rs +++ b/lnvps_api_common/src/mock.rs @@ -1111,6 +1111,41 @@ impl LNVpsDbBase for MockDb { Ok(()) } + async fn hard_delete_vm(&self, vm_id: u64) -> DbResult<()> { + // Resolve the subscription for this VM (via its line item) before removal. + let subscription_id = { + let vms = self.vms.lock().await; + let line_items = self.subscription_line_items.lock().await; + vms.get(&vm_id) + .and_then(|vm| line_items.get(&vm.subscription_line_item_id)) + .map(|li| li.subscription_id) + }; + + self.vms.lock().await.remove(&vm_id); + self.vm_history.lock().await.retain(|_, h| h.vm_id != vm_id); + self.firewall_rules + .lock() + .await + .retain(|_, r| r.vm_id != vm_id); + self.ip_assignments + .lock() + .await + .retain(|_, a| a.vm_id != vm_id); + + if let Some(subscription_id) = subscription_id { + self.subscription_payments + .lock() + .await + .retain(|p| p.subscription_id != subscription_id); + self.subscription_line_items + .lock() + .await + .retain(|_, li| li.subscription_id != subscription_id); + self.subscriptions.lock().await.remove(&subscription_id); + } + Ok(()) + } + async fn update_vm(&self, vm: &Vm) -> DbResult<()> { let mut vms = self.vms.lock().await; if let Some(v) = vms.get_mut(&vm.id) { @@ -4026,6 +4061,97 @@ mod tests { } } + /// hard_delete_vm removes the VM and every record that references it: + /// history, firewall rules, IP assignments, and the VM's subscription along + /// with its line items and payment history. + #[tokio::test] + async fn test_hard_delete_vm_purges_related_records() { + use lnvps_db::{ + VmFirewallDirection, VmFirewallProtocol, VmFirewallRule, VmFirewallRuleAction, + VmHistory, VmHistoryActionType, VmIpAssignment, + }; + + let db = MockDb::default(); + // subscription_payment inserts validate the owning user exists. + db.upsert_user(&[1u8; 32]).await.unwrap(); + + // The default MockDb has subscription 1 with Vps line item 1. + let vm_id = db + .insert_vm(&Vm { + ssh_key_id: None, + ..MockDb::mock_vm() + }) + .await + .unwrap(); + let sub_id = db.get_subscription_by_line_item_id(1).await.unwrap().id; + + // Related records referencing the VM. + db.insert_vm_ip_assignment(&VmIpAssignment { + id: 0, + vm_id, + ip_range_id: 1, + ip: "10.0.0.5".to_string(), + ..Default::default() + }) + .await + .unwrap(); + db.insert_vm_firewall_rule(&VmFirewallRule { + id: 0, + vm_id, + priority: 1, + direction: VmFirewallDirection::Inbound, + protocol: VmFirewallProtocol::Tcp, + action: VmFirewallRuleAction::Accept, + src_cidr: None, + dst_port_start: Some(22), + dst_port_end: None, + enabled: true, + created: Utc::now(), + updated: Utc::now(), + }) + .await + .unwrap(); + db.insert_vm_history(&VmHistory { + id: 0, + vm_id, + action_type: VmHistoryActionType::Created, + timestamp: Utc::now(), + initiated_by_user: None, + previous_state: None, + new_state: None, + metadata: None, + description: None, + }) + .await + .unwrap(); + db.insert_subscription_payment(&make_payment(sub_id, Some(3600))) + .await + .unwrap(); + + // Sanity: everything is present before the purge. + assert!(db.get_vm(vm_id).await.is_ok()); + assert_eq!(db.list_vm_ip_assignments(vm_id).await.unwrap().len(), 1); + assert_eq!(db.list_vm_firewall_rules(vm_id).await.unwrap().len(), 1); + assert_eq!(db.list_vm_history(vm_id).await.unwrap().len(), 1); + assert_eq!(db.subscription_payments.lock().await.len(), 1); + + db.hard_delete_vm(vm_id).await.unwrap(); + + // The VM and every related record are gone. + assert!(db.get_vm(vm_id).await.is_err()); + assert!(db.list_vm_ip_assignments(vm_id).await.unwrap().is_empty()); + assert!(db.list_vm_firewall_rules(vm_id).await.unwrap().is_empty()); + assert!(db.list_vm_history(vm_id).await.unwrap().is_empty()); + assert!(db.get_subscription(sub_id).await.is_err()); + assert!( + db.list_subscription_line_items(sub_id) + .await + .unwrap() + .is_empty() + ); + assert!(db.subscription_payments.lock().await.is_empty()); + } + /// Firewall rule CRUD via the mock DB. #[tokio::test] async fn test_firewall_rule_crud() { diff --git a/lnvps_api_common/src/work/mod.rs b/lnvps_api_common/src/work/mod.rs index 5e7af1a6..6dfc131f 100644 --- a/lnvps_api_common/src/work/mod.rs +++ b/lnvps_api_common/src/work/mod.rs @@ -63,6 +63,12 @@ pub enum WorkJob { vm_id: u64, reason: Option, admin_user_id: Option, + /// Permanently purge the VM and all related records (history, + /// payments, subscription) from the database instead of soft-deleting. + /// Reserved for super-admin forced deletions; never-paid VMs are always + /// purged regardless of this flag. + #[serde(default)] + purge: bool, }, /// Start a VM StartVm { diff --git a/lnvps_db/src/lib.rs b/lnvps_db/src/lib.rs index 6d9ce01d..6541e5a5 100644 --- a/lnvps_db/src/lib.rs +++ b/lnvps_db/src/lib.rs @@ -377,6 +377,18 @@ pub trait LNVpsDbBase: Send + Sync { /// Delete a VM by id async fn delete_vm(&self, vm_id: u64) -> DbResult<()>; + /// Permanently delete a VM and all of its related records from the database. + /// + /// Unlike [`delete_vm`](Self::delete_vm) (which soft-deletes by setting + /// `deleted = 1`), this removes the VM row entirely along with every entity + /// that references it: `vm_history`, `vm_firewall_rule`, `vm_ip_assignment`, + /// and the VM's own `subscription` (its `subscription_line_item` rows and + /// `subscription_payment` history). Intended for purging never-paid (new) + /// VMs and for super-admin forced deletions of test VMs. This is + /// irreversible and discards payment history, so callers must gate it + /// appropriately. + async fn hard_delete_vm(&self, vm_id: u64) -> DbResult<()>; + /// Update a VM async fn update_vm(&self, vm: &Vm) -> DbResult<()>; diff --git a/lnvps_db/src/mysql.rs b/lnvps_db/src/mysql.rs index dbd2e993..b5db14de 100644 --- a/lnvps_db/src/mysql.rs +++ b/lnvps_db/src/mysql.rs @@ -1022,6 +1022,58 @@ impl LNVpsDbBase for LNVpsDbMysql { Ok(()) } + async fn hard_delete_vm(&self, vm_id: u64) -> DbResult<()> { + let mut tx = self.db.begin().await?; + + // Resolve the VM's subscription (via its line item) before we delete the + // VM row, so we can clean up the subscription and its payment history. + let subscription_id: Option = sqlx::query_scalar( + "select li.subscription_id from vm v \ + join subscription_line_item li on li.id = v.subscription_line_item_id \ + where v.id = ?", + ) + .bind(vm_id) + .fetch_optional(&mut *tx) + .await?; + + // Remove rows that reference the VM directly. + sqlx::query("delete from vm_history where vm_id = ?") + .bind(vm_id) + .execute(&mut *tx) + .await?; + sqlx::query("delete from vm_firewall_rule where vm_id = ?") + .bind(vm_id) + .execute(&mut *tx) + .await?; + sqlx::query("delete from vm_ip_assignment where vm_id = ?") + .bind(vm_id) + .execute(&mut *tx) + .await?; + + // Delete the VM row itself (frees the FK to subscription_line_item). + sqlx::query("delete from vm where id = ?") + .bind(vm_id) + .execute(&mut *tx) + .await?; + + // Delete the VM's subscription. subscription_payment has no ON DELETE + // CASCADE so it must be cleared first; subscription_line_item does + // cascade, so deleting the subscription removes the line items. + if let Some(subscription_id) = subscription_id { + sqlx::query("delete from subscription_payment where subscription_id = ?") + .bind(subscription_id) + .execute(&mut *tx) + .await?; + sqlx::query("delete from subscription where id = ?") + .bind(subscription_id) + .execute(&mut *tx) + .await?; + } + + tx.commit().await?; + Ok(()) + } + async fn update_vm(&self, vm: &Vm) -> DbResult<()> { sqlx::query( "update vm set image_id=?,template_id=?,custom_template_id=?,subscription_line_item_id=?,ssh_key_id=?,disk_id=?,mac_address=?,disabled=?,fw_policy_in=?,fw_policy_out=? where id=?", diff --git a/lnvps_e2e/src/client.rs b/lnvps_e2e/src/client.rs index 4010c360..20f43b9d 100644 --- a/lnvps_e2e/src/client.rs +++ b/lnvps_e2e/src/client.rs @@ -145,6 +145,28 @@ impl TestClient { Ok(resp) } + /// Make an authenticated DELETE request with a JSON body. + pub async fn delete_auth_body( + &self, + path: &str, + body: &impl serde::Serialize, + ) -> anyhow::Result { + let keys = self + .keys + .as_ref() + .ok_or_else(|| anyhow::anyhow!("No keys configured for authenticated request"))?; + let url = self.url(path); + let auth = make_nip98_auth(keys, &url, "DELETE")?; + let resp = self + .http + .delete(&url) + .header("Authorization", auth) + .json(body) + .send() + .await?; + Ok(resp) + } + /// Make an authenticated PUT request with JSON body. pub async fn put_auth( &self, diff --git a/lnvps_e2e/src/rbac.rs b/lnvps_e2e/src/rbac.rs index 8aa262a7..7a3211b5 100644 --- a/lnvps_e2e/src/rbac.rs +++ b/lnvps_e2e/src/rbac.rs @@ -193,6 +193,24 @@ mod tests { assert_eq!(resp.status(), StatusCode::FORBIDDEN); } + /// A vm_manager has VM Delete permission but is not a super_admin, so it + /// cannot request a permanent purge (`purge = true`). The purge + /// authorization is checked before the VM lookup, so a non-existent VM id + /// still yields 403 rather than 404. + #[tokio::test] + async fn test_vm_manager_cannot_purge_vm() { + setup_rbac().await; + let client = admin_client_with_keys(vm_manager_keys().clone()); + let body = serde_json::json!({ "purge": true }); + let resp = client + .delete_auth_body("/api/admin/v1/vms/999999999", &body) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); + let text = resp.text().await.unwrap(); + assert!(text.contains("Only super admins can permanently purge")); + } + // ======================================================================== // super_admin role: full access // ======================================================================== From 492a879de5c970880a7db4157bb2edc2b9fa9f8a Mon Sep 17 00:00:00 2001 From: Kieran Date: Mon, 20 Jul 2026 11:36:20 +0100 Subject: [PATCH 2/3] test(e2e): expect never-paid VMs to be purged (hard-deleted) in cleanup --- lnvps_e2e/src/lifecycle.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lnvps_e2e/src/lifecycle.rs b/lnvps_e2e/src/lifecycle.rs index af00632e..7ead992d 100644 --- a/lnvps_e2e/src/lifecycle.rs +++ b/lnvps_e2e/src/lifecycle.rs @@ -1490,7 +1490,10 @@ mod tests { crate::worker::trigger_check_vms().await.unwrap(); eprintln!("[cleanup] Published CheckVms job"); - // Poll the admin API until vm.deleted = true (up to 30 s). + // Poll the admin API until the VM is gone (up to 30 s). + // Never-paid VMs are hard-deleted, so the row is removed entirely and the + // admin GET returns a non-OK status. (We also treat a soft-delete flag as + // "deleted" for robustness.) let deleted = poll_until(30, 500, || { let admin = admin.clone(); async move { @@ -1498,6 +1501,10 @@ mod tests { .get_auth(&format!("/api/admin/v1/vms/{unpaid_vm_id}")) .await .unwrap(); + if r.status() != reqwest::StatusCode::OK { + // Row hard-deleted (purged). + return true; + } let body: serde_json::Value = serde_json::from_str(&r.text().await.unwrap()).unwrap(); body["data"]["deleted"].as_bool().unwrap_or(false) @@ -1507,9 +1514,9 @@ mod tests { assert!( deleted, - "Unpaid VM {unpaid_vm_id} should be deleted by check_vms within 30 s" + "Unpaid VM {unpaid_vm_id} should be purged by check_vms within 30 s" ); - eprintln!("[cleanup] Unpaid VM {unpaid_vm_id} deleted by worker ✓"); + eprintln!("[cleanup] Unpaid VM {unpaid_vm_id} purged by worker ✓"); // After deletion the user should no longer see the VM. let list_after = json_ok(user.get_auth("/api/v1/vm").await.unwrap()).await; From 0a1a63fd66fedecbc6aebe71efa28ce69429de4d Mon Sep 17 00:00:00 2001 From: Kieran Date: Mon, 20 Jul 2026 11:45:09 +0100 Subject: [PATCH 3/3] fix(db): delete_user no longer references dropped vm_payment table The vm_payment table was dropped by migration 20260716130000; delete_user still tried to `delete from vm_payment`, failing with 'Table lnvps.vm_payment doesn't exist'. Payment rows are removed via subscription_payment lower in the same transaction. --- lnvps_db/src/mysql.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/lnvps_db/src/mysql.rs b/lnvps_db/src/mysql.rs index b5db14de..e40ca450 100644 --- a/lnvps_db/src/mysql.rs +++ b/lnvps_db/src/mysql.rs @@ -240,7 +240,6 @@ impl LNVpsDbBase for LNVpsDbMysql { // Delete VM child records for every VM owned by the user (incl. soft-deleted). for child in [ "delete from vm_ip_assignment where vm_id in (select id from vm where user_id = ?)", - "delete from vm_payment where vm_id in (select id from vm where user_id = ?)", "delete from vm_firewall_rule where vm_id in (select id from vm where user_id = ?)", "delete from vm_history where vm_id in (select id from vm where user_id = ?)", ] {