From ebc8e04e21cfd9d6245d0641239bbb659b314fe7 Mon Sep 17 00:00:00 2001 From: Dusty Date: Wed, 21 Oct 2026 11:47:37 -0700 Subject: [PATCH] fix(desktop): never reattach a previous installation's volumes on a fresh install A desktop install set up again after its folder was deleted refused to reuse only the old database. Every other volume the previous install left was reattached silently: Compose project volumes (agent-workspace, agent-profiles, spire-*) and the supervisor's per-Bot volumes (-workspace-, -profile-). Default Bot ids are stable across installs, so a new Bot opened the old Bot's files and Chromium profile, including its website sign-ins. On Start, a verified-fresh installation (no usable key, no database URL, no saved model) now lists every leftover it can prove it owns: Compose volumes and containers by project label, supervisor volumes and containers by all three supervisor labels in this namespace. Start is refused with one confirmed reset that removes the stopped containers, then the volumes, bound to the exact set shown. A running owned container blocks the reset and nothing is removed. Installations with their saved key keep their data as before. --- CHANGELOG.md | 9 + desktop/src-tauri/src/main.rs | 499 +++++++++++-- desktop/src-tauri/src/problem.rs | 29 + desktop/src-tauri/src/stack.rs | 1119 ++++++++++++++++++++++++++++-- desktop/src/App.test.tsx | 60 +- desktop/src/App.tsx | 22 +- desktop/src/DatabaseReset.tsx | 64 +- desktop/src/Problem.tsx | 4 + 8 files changed, 1639 insertions(+), 167 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b1e6a8afc..74401d3efe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,15 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. `code_required` (below). A server and a computer must be upgraded together: an older computer has no `/sign-in/code`. +### A reinstall no longer picks up the previous installation's Bot files and sign-ins + +A desktop installation set up again after its folder was deleted reused whatever the previous one +left in the container engine: Bot workspace files, and each Bot's browser profile with its website +sign-ins, under the same Bot ids. Only the database was checked. Start now lists every volume and +stopped container the previous installation left, and offers one confirmed reset that removes them; +nothing is reused without asking. A container from it that is still running stops the reset until it +is stopped. An installation with its saved key keeps its data as before. + ### A new desktop install starts its server again A desktop install of v0.2.0 stopped at start with `server stopped straight away (exit status: 1)` and diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index 3877d8b906..2e32e1402a 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -13,8 +13,9 @@ mod test_support; use openbot_desktop_lib::{ acquire, deployment, deployment_release, engine, env as openbot_env, harness, host_access, - install, preparation, problem::Problem, provider, pull_metrics, quiet, stack, supervise, - telemetry, tray, windows as win, + install, preparation, + problem::{LeftoverReset, Problem}, + provider, pull_metrics, quiet, stack, supervise, telemetry, tray, windows as win, }; const QUIT_CLEANUP_NOTICE_FILE: &str = ".openbot-quit-cleanup-notice"; @@ -57,7 +58,7 @@ struct Shell { last_failure: Mutex>, /// Reading the notification must not make a partially running deployment adoptable again. recovery_required: Mutex>, - /// An explicit reset remains bound to the engine that found the leftover database. + /// An explicit reset remains bound to the engine and the exact volumes it offered. leftover_database: Mutex>, selected_root: Mutex>, root: Mutex>, @@ -102,7 +103,10 @@ struct ContainerDeployment { struct LeftoverDatabase { root: PathBuf, - volume: String, + /// Sorted, exactly as offered: the only set a confirmation may remove. + volumes: Vec, + /// Sorted names of the stopped containers removed before those volumes. + containers: Vec, address: engine::Address, } @@ -1094,46 +1098,113 @@ fn require_existing_encryption_key( Ok(()) } +/** +Refuse a Start that would mint a key for, or reattach, a previous installation's data. + +A deleted installation folder takes `.env` and the vault with it, but the engine keeps every +volume. Without this, a new installation reattaches the previous one's Bots' workspace files and +browser profiles by name, and only the database was ever checked. An installation with its usable +key is a returning one and is never probed: it keeps its data. +*/ fn require_existing_encryption_key_with_recovery( root: &Path, secrets: &stack::Secrets, existing_postgres_volume: impl FnOnce() -> Result, - resettable_volume: impl FnOnce() -> Result, Problem>, + leftovers: impl FnOnce() -> Result, ) -> Result<(), Problem> { + if secrets + .get("KEY_ENCRYPTION_KEY") + .is_some_and(|value| openbot_env::usable_encryption_key(value)) + { + return Ok(()); + } let mut volume_exists = false; - require_existing_encryption_key(root, secrets, || { + let guarded = require_existing_encryption_key(root, secrets, || { volume_exists = existing_postgres_volume()?; Ok(volume_exists) - }) - .map_err(|mut problem| { + }); + let mut problem = match guarded { // A failed probe is not proof of an existing volume. Configured roots never reach here. - if volume_exists { - let recovery = fresh_root_without_encryption_key(root, secrets).and_then(|fresh| { - if fresh { - resettable_volume() - } else { - Ok(None) + Err(problem) if !volume_exists => return Err(problem), + Err(problem) => problem, + Ok(()) => { + // No database, so a key would be minted. Anything else left behind would be reused. + let found = leftovers()?; + if found.volumes.is_empty() { + return Ok(()); + } + let mut problem = Problem::plain("OpenBot found data from a previous installation, such as Bots' files and browser sign-ins. OpenBot will not reuse it in a new installation. Restore the previous installation's folder to keep it, or reset the leftover data to start fresh."); + match fresh_root_without_encryption_key(root, secrets) { + Ok(true) if found.resettable() => { + problem.leftover = Some(Box::new(LeftoverReset { + containers: found.container_names(), + volumes: found.volumes, + })); } - }); - match recovery { - Ok(volume) => { - if volume.is_some() { - problem.said = "OpenBot found a database from a previous installation, but its encryption key is unavailable. Restore the original key to keep its saved data, or reset the leftover database to start fresh.".into(); + Ok(true) => { + if let Some(running) = found.running_said() { + problem.said = format!("{} {running}", problem.said); } - problem.database_reset = volume; - } - Err(verification) => { - problem.detail = Some(match verification.detail { - Some(detail) => format!("{}\n{detail}", verification.said), - None => verification.said, - }); } + Ok(false) => {} + Err(verification) => problem.detail = Some(verification_detail(verification)), } + return Err(problem); } - problem + }; + let recovery = fresh_root_without_encryption_key(root, secrets).and_then(|fresh| { + if fresh { + leftovers().map(Some) + } else { + Ok(None) + } + }); + match recovery { + Ok(Some(found)) if found.database.is_some() && found.resettable() => { + problem.said = if found.volumes.len() > 1 { + "OpenBot found a database and other data from a previous installation, but its encryption key is unavailable. Restore the original key to keep its saved data, or reset the leftover data to start fresh." + } else { + "OpenBot found a database from a previous installation, but its encryption key is unavailable. Restore the original key to keep its saved data, or reset the leftover database to start fresh." + } + .into(); + problem.leftover = Some(Box::new(LeftoverReset { + containers: found.container_names(), + volumes: found.volumes, + })); + problem.database_reset = found.database; + } + Ok(Some(found)) if found.database.is_some() => { + if let Some(running) = found.running_said() { + problem.said = format!("{} {running}", problem.said); + } + } + Ok(_) => {} + Err(verification) => problem.detail = Some(verification_detail(verification)), + } + Err(problem) +} + +/// The reset a refused Start offered, bound to the exact containers and volumes it named. +fn leftover_offer( + problem: &Problem, + root: &Path, + address: &engine::Address, +) -> Option { + (!problem.leftover_volumes().is_empty()).then(|| LeftoverDatabase { + root: root.to_path_buf(), + volumes: problem.leftover_volumes().to_vec(), + containers: problem.leftover_containers().to_vec(), + address: address.clone(), }) } +fn verification_detail(verification: Problem) -> String { + match verification.detail { + Some(detail) => format!("{}\n{detail}", verification.said), + None => verification.said, + } +} + /// Destructive recovery needs positive evidence that root metadata is readable and unconfigured. /// The ordinary startup guard keeps its existing behavior when metadata is unknown. fn fresh_root_without_encryption_key( @@ -1147,7 +1218,7 @@ fn fresh_root_without_encryption_key( return Ok(false); } let unknown = || { - Problem::plain("OpenBot could not verify that this is an unconfigured installation. Its leftover database cannot be reset here.") + Problem::plain("OpenBot could not verify that this is an unconfigured installation. Its leftover data cannot be reset here.") }; let settings = openbot_env::read_already_set(&root.join(".env"), &["DATABASE_URL"]) .map_err(|_| unknown())?; @@ -1173,13 +1244,16 @@ fn fresh_root_without_encryption_key( fn reset_leftover_database_with( shell: &Shell, root: &Path, - volume: &str, + volumes: &[String], + containers: &[String], confirmed: bool, read_secrets: impl FnOnce() -> Result, reset: impl FnOnce(&engine::Address, &stack::Secrets) -> Result<(), Problem>, ) -> Result<(), Problem> { if !confirmed { - return Err(Problem::plain("Confirm that you want to permanently delete the leftover database before resetting it.")); + return Err(Problem::plain( + "Confirm that you want to permanently delete the leftover data before resetting it.", + )); } let attempt = StartAttempt::begin(shell)?; let _startup = attempt.lock_current()?; @@ -1187,7 +1261,7 @@ fn reset_leftover_database_with( || shell.root.lock().unwrap().is_some() || !shell.children.lock().unwrap().is_empty() { - return Err(Problem::plain("OpenBot still owns running services. Choose Stop OpenBot before resetting a leftover database.")); + return Err(Problem::plain("OpenBot still owns running services. Choose Stop OpenBot before resetting leftover data.")); } if shell .selected_root @@ -1196,15 +1270,26 @@ fn reset_leftover_database_with( .as_ref() .is_some_and(|selected| selected != root) { - return Err(Problem::plain("The selected installation changed. Try Start again before resetting its leftover database.")); + return Err(Problem::plain("The selected installation changed. Try Start again before resetting its leftover data.")); } + let sorted = |names: &[String]| { + let mut names = names.to_vec(); + names.sort(); + names + }; + let (volumes, containers) = (sorted(volumes), sorted(containers)); let address = shell.leftover_database.lock().unwrap().as_ref() - .filter(|offer| offer.root == root && offer.volume == volume) + .filter(|offer| { + offer.root == root + && !volumes.is_empty() + && offer.volumes == volumes + && offer.containers == containers + }) .map(|offer| offer.address.clone()) - .ok_or_else(|| Problem::plain("The leftover database reset offer is no longer current. Try Start again before confirming a reset."))?; + .ok_or_else(|| Problem::plain("The leftover data reset offer is no longer current. Try Start again before confirming a reset."))?; let secrets = read_secrets()?; if !fresh_root_without_encryption_key(root, &secrets)? { - return Err(Problem::plain("This installation is already configured or has its original encryption key. Its database cannot be reset here.")); + return Err(Problem::plain("This installation is already configured or has its original encryption key. Its data cannot be reset here.")); } reset(&address, &secrets)?; *shell.leftover_database.lock().unwrap() = None; @@ -1214,7 +1299,8 @@ fn reset_leftover_database_with( #[tauri::command] async fn reset_leftover_database( root: String, - volume: String, + volumes: Vec, + containers: Vec, confirmed: bool, app: tauri::AppHandle, ) -> Result<(), Problem> { @@ -1223,7 +1309,8 @@ async fn reset_leftover_database( reset_leftover_database_with( &shell, &root, - &volume, + &volumes, + &[], confirmed, || { openbot_desktop_lib::vault::already_given_no_ui( @@ -1232,7 +1319,9 @@ async fn reset_leftover_database( &openbot_env::MINTED[..], ) }, - |address, secrets| stack::reset_leftover_database(address, &root, secrets, &volume), + |address, secrets| { + stack::reset_leftover_volumes(address, &root, secrets, &volumes, &containers) + }, ) } @@ -1421,18 +1510,10 @@ async fn start_stack_inner( &root, &existing_secrets, || stack::postgres_volume_exists(&found, &root, &existing_secrets), - || stack::leftover_database_volume(&found, &root, &existing_secrets), + || stack::leftover_volumes(&found, &root, &existing_secrets), ) .inspect_err(|problem| { - *shell.leftover_database.lock().unwrap() = - problem - .database_reset - .as_ref() - .map(|volume| LeftoverDatabase { - root: root.clone(), - volume: volume.clone(), - address: found.clone(), - }); + *shell.leftover_database.lock().unwrap() = leftover_offer(problem, &root, &found); })?; // Only this deployment's recorded hosts are reclaimed; its existing containers are reusable. @@ -5061,12 +5142,14 @@ mod tests { &root, &stack::Secrets::new(), || Ok(true), - || Ok(Some("openbot_postgres-data".into())), + || Ok(database_leftovers(&[])), ) .unwrap_err(); + let offered = serde_json::to_value(error).unwrap(); + assert_eq!(offered["database_reset"], "openbot_postgres-data"); assert_eq!( - serde_json::to_value(error).unwrap()["database_reset"], - "openbot_postgres-data" + offered["leftover_reset"], + serde_json::json!(["openbot_postgres-data"]) ); for (file, content) in [ (".env", "DATABASE_URL=postgres://fixture\n"), @@ -5091,10 +5174,9 @@ mod tests { || panic!("unknown or configured roots must not offer deletion"), ) .unwrap_err(); - assert!(serde_json::to_value(error) - .unwrap() - .get("database_reset") - .is_none()); + let refused = serde_json::to_value(error).unwrap(); + assert!(refused.get("database_reset").is_none()); + assert!(refused.get("leftover_reset").is_none()); std::fs::remove_file(root.join(file)).unwrap(); } let secrets = stack::Secrets::from([( @@ -5111,6 +5193,245 @@ mod tests { std::fs::remove_dir_all(root).unwrap(); } + fn database_leftovers(others: &[&str]) -> stack::LeftoverVolumes { + let mut volumes: Vec = others.iter().map(|name| name.to_string()).collect(); + volumes.push("openbot_postgres-data".into()); + volumes.sort(); + stack::LeftoverVolumes { + database: Some("openbot_postgres-data".into()), + database_exists: true, + volumes, + ..Default::default() + } + } + + fn leftovers_without_database(volumes: &[&str]) -> stack::LeftoverVolumes { + stack::LeftoverVolumes { + database: None, + database_exists: false, + volumes: volumes.iter().map(|name| name.to_string()).collect(), + ..Default::default() + } + } + + const BOT_LEFTOVERS: [&str; 3] = [ + "openbot-profile-general-assistant", + "openbot-workspace-general-assistant", + "openbot_agent-profiles", + ]; + + /// Deleting the folder leaves Bots' files and sign-ins in the engine even with no database. + #[test] + fn a_fresh_root_refuses_to_reattach_any_leftover_volume() { + let root = temp_root("leftover-volumes-offer"); + std::fs::create_dir_all(&root).unwrap(); + + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Ok(leftovers_without_database(&BOT_LEFTOVERS)), + ) + .unwrap_err(); + assert!( + error.said.contains("previous installation"), + "{}", + error.said + ); + let offered = serde_json::to_value(error).unwrap(); + assert!(offered.get("database_reset").is_none()); + assert_eq!(offered["leftover_reset"], serde_json::json!(BOT_LEFTOVERS)); + + // Nothing left behind starts exactly as before. + assert!(require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Ok(stack::LeftoverVolumes::default()), + ) + .is_ok()); + + // A probe that fails is not permission to reattach whatever is there. + assert!(require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Err(Problem::plain("inventory failed")), + ) + .is_err()); + + // A database joins the same confirmation as everything else it would be reset with. + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(true), + || Ok(database_leftovers(&BOT_LEFTOVERS)), + ) + .unwrap_err(); + assert_eq!( + error.database_reset.as_deref(), + Some("openbot_postgres-data") + ); + assert_eq!( + error.leftover_volumes(), + database_leftovers(&BOT_LEFTOVERS).volumes + ); + + // A database OpenBot cannot verify keeps the existing refusal, with nothing offered. + let mut unverified = leftovers_without_database(&BOT_LEFTOVERS); + unverified.database_exists = true; + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(true), + || Ok(unverified), + ) + .unwrap_err(); + assert_eq!(error.database_reset, None); + assert!(error.leftover_volumes().is_empty()); + assert!(error + .said + .contains("OpenBot will not replace the key automatically")); + + // Stopped containers that hold the volumes join the confirmation; running ones block it. + let mut held = leftovers_without_database(&BOT_LEFTOVERS); + held.containers = vec![stack::LeftoverContainer { + name: "openbot-computer-general-assistant".into(), + id: "c-computer".into(), + }]; + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Ok(held.clone()), + ) + .unwrap_err(); + assert_eq!(error.leftover_volumes(), BOT_LEFTOVERS); + assert_eq!( + error.leftover_containers(), + ["openbot-computer-general-assistant"] + ); + assert_eq!( + serde_json::to_value(&error).unwrap()["leftover_containers"], + serde_json::json!(["openbot-computer-general-assistant"]) + ); + held.running = vec!["openbot-agent-computer-1".into()]; + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Ok(held), + ) + .unwrap_err(); + assert!(error.leftover_volumes().is_empty() && error.leftover_containers().is_empty()); + assert!( + error.said.contains("openbot-agent-computer-1") && error.said.contains("still running"), + "{}", + error.said + ); + + // Unknown root metadata refuses Start but never offers deletion. + std::fs::write( + root.join(openbot_desktop_lib::saved_intent::FILE), + "invalid-settings-secret", + ) + .unwrap(); + let error = require_existing_encryption_key_with_recovery( + &root, + &stack::Secrets::new(), + || Ok(false), + || Ok(leftovers_without_database(&BOT_LEFTOVERS)), + ) + .unwrap_err(); + assert!(error.leftover_volumes().is_empty()); + assert!(!format!("{error:?}").contains("invalid-settings-secret")); + std::fs::remove_file(root.join(openbot_desktop_lib::saved_intent::FILE)).unwrap(); + + // A returning installation with its key keeps its data and is never probed. + let secrets = stack::Secrets::from([( + "KEY_ENCRYPTION_KEY".into(), + "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=".into(), + )]); + std::fs::write(root.join(".env"), "DATABASE_URL=postgres://fixture\n").unwrap(); + assert!(require_existing_encryption_key_with_recovery( + &root, + &secrets, + || panic!("original key needs no probe"), + || panic!("original key keeps its volumes") + ) + .is_ok()); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn a_refused_start_offers_exactly_the_containers_and_volumes_it_named() { + let root = temp_root("leftover-offer"); + let address = engine::Address::new(engine::Engine::Docker, None); + assert!(leftover_offer(&Problem::plain("refused"), &root, &address).is_none()); + let mut problem = Problem::plain("refused"); + problem.leftover = Some(Box::new(LeftoverReset { + volumes: BOT_LEFTOVERS.map(String::from).to_vec(), + containers: vec!["openbot-computer-general-assistant".into()], + })); + let offer = leftover_offer(&problem, &root, &address).unwrap(); + assert_eq!(offer.root, root); + assert_eq!(offer.volumes, BOT_LEFTOVERS); + assert_eq!(offer.containers, ["openbot-computer-general-assistant"]); + assert_eq!(offer.address, address); + } + + #[test] + fn leftover_reset_is_bound_to_the_exact_offered_set() { + let root = temp_root("leftover-volumes-command"); + std::fs::create_dir_all(&root).unwrap(); + let shell = Shell::default(); + let offered: Vec = BOT_LEFTOVERS.map(String::from).to_vec(); + *shell.leftover_database.lock().unwrap() = Some(LeftoverDatabase { + root: root.clone(), + volumes: offered.clone(), + containers: vec!["openbot-computer-general-assistant".into()], + address: engine::Address::new(engine::Engine::Docker, None), + }); + let mut superset = offered.clone(); + superset.push("other-profile-knowledge".into()); + let containers = ["openbot-computer-general-assistant".to_string()]; + for (stale, stale_containers) in [ + (offered[..2].to_vec(), &containers[..]), + (superset, &containers[..]), + (Vec::new(), &containers[..]), + (offered.clone(), &[][..]), + ( + offered.clone(), + &[containers[0].clone(), "other-computer-knowledge".into()][..], + ), + ] { + assert!(reset_leftover_database_with( + &shell, + &root, + &stale, + stale_containers, + true, + || panic!("a changed set must refuse before reading credentials"), + |_, _| panic!("a changed set must not remove anything") + ) + .is_err()); + } + let mut reordered = offered.clone(); + reordered.reverse(); + reset_leftover_database_with( + &shell, + &root, + &reordered, + &containers, + true, + || Ok(stack::Secrets::new()), + |_, _| Ok(()), + ) + .unwrap(); + assert!(shell.leftover_database.lock().unwrap().is_none()); + std::fs::remove_dir_all(root).unwrap(); + } + #[test] fn leftover_database_reset_rechecks_confirmation_configuration_key_and_ownership() { let root = temp_root("leftover-database-command"); @@ -5119,7 +5440,8 @@ mod tests { let offer = || { *shell.leftover_database.lock().unwrap() = Some(LeftoverDatabase { root: root.clone(), - volume: "openbot_postgres-data".into(), + volumes: vec!["openbot_postgres-data".into()], + containers: Vec::new(), address: engine::Address::new( engine::Engine::Podman, Some("original-machine".into()), @@ -5130,7 +5452,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], false, || panic!("unconfirmed must not access credentials"), |_, _| panic!("unconfirmed must not delete") @@ -5139,7 +5462,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || Ok(stack::Secrets::new()), |_, _| Ok(()) @@ -5161,7 +5485,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || Ok(stack::Secrets::new()), |_, _| panic!("configured or unknown root must not delete") @@ -5173,7 +5498,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || Ok(stack::Secrets::new()), |_, _| panic!("unreadable settings must not delete") @@ -5187,7 +5513,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || Ok(secrets), |_, _| panic!("restored key must prevent deletion") @@ -5197,7 +5524,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || panic!("owned hosts must refuse before credential access"), |_, _| panic!("owned hosts must prevent deletion") @@ -5211,7 +5539,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || panic!("owned containers must refuse"), |_, _| panic!("owned containers must prevent deletion") @@ -5222,7 +5551,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || panic!("concurrent startup must refuse"), |_, _| panic!("concurrent startup must prevent deletion") @@ -5232,7 +5562,8 @@ mod tests { assert!(reset_leftover_database_with( &shell, &root, - "openbot_postgres-data", + &["openbot_postgres-data".to_string()], + &[], true, || Ok(stack::Secrets::new()), |_, _| { @@ -5263,11 +5594,12 @@ mod tests { let shell = Shell::default(); let original = engine::Address::new(engine::Engine::Podman, Some("original-machine".into())); - let volume = "openbot_postgres-data"; + let volume = &["openbot_postgres-data".to_string()][..]; assert!(reset_leftover_database_with( &shell, &root, volume, + &[], true, || panic!("a missing offer must refuse before reading credentials"), |_, _| panic!("a missing offer must not remove anything") @@ -5275,17 +5607,19 @@ mod tests { .is_err()); *shell.leftover_database.lock().unwrap() = Some(LeftoverDatabase { root: root.clone(), - volume: volume.into(), + volumes: volume.to_vec(), + containers: Vec::new(), address: original.clone(), }); for (selected_root, selected_volume) in [ - (&root, "other-volume"), + (&root, &["other-volume".to_string()][..]), (&root.join("another-root"), volume), ] { assert!(reset_leftover_database_with( &shell, selected_root, selected_volume, + &[], true, || panic!("a mismatched offer must refuse before reading credentials"), |_, _| panic!("a mismatched offer must not remove anything") @@ -5302,6 +5636,7 @@ mod tests { &shell, &root, volume, + &[], true, || Ok(stack::Secrets::new()), |address, _| { @@ -5315,6 +5650,7 @@ mod tests { &shell, &root, volume, + &[], true, || Ok(stack::Secrets::new()), |address, _| { @@ -5388,23 +5724,33 @@ mod tests { &root, &secrets, || stack::postgres_volume_exists(&address, &root, &secrets), - || stack::leftover_database_volume(&address, &root, &secrets), + || stack::leftover_volumes(&address, &root, &secrets), ) .unwrap_err(); assert_eq!(guarded.database_reset.as_deref(), Some(volume.as_str())); + let volumes = [volume.clone()]; + assert_eq!(guarded.leftover_volumes(), volumes); let shell = Shell::default(); *shell.leftover_database.lock().unwrap() = Some(LeftoverDatabase { root: root.clone(), - volume: volume.clone(), + volumes: volumes.to_vec(), + containers: Vec::new(), address: address.clone(), }); assert!(reset_leftover_database_with( &shell, &root, - &volume, + &volumes, + &[], false, || Ok(secrets.clone()), - |address, secrets| stack::reset_leftover_database(address, &root, secrets, &volume) + |address, secrets| stack::reset_leftover_volumes( + address, + &root, + secrets, + &volumes, + &[] + ) ) .is_err()); assert!(stack::postgres_volume_exists(&address, &root, &secrets).unwrap()); @@ -5413,10 +5759,13 @@ mod tests { reset_leftover_database_with( &shell, &root, - &volume, + &volumes, + &[], true, || Ok(secrets.clone()), - |address, secrets| stack::reset_leftover_database(address, &root, secrets, &volume), + |address, secrets| { + stack::reset_leftover_volumes(address, &root, secrets, &volumes, &[]) + }, ) .unwrap(); require_existing_encryption_key(&root, &secrets, || { @@ -5443,7 +5792,7 @@ mod tests { String::from_utf8_lossy(&started.stderr) ); assert!( - stack::reset_leftover_database(&address, &root, &secrets, &volume).is_err(), + stack::reset_leftover_volumes(&address, &root, &secrets, &volumes, &[]).is_err(), "the real engine must refuse an attached database volume" ); let ready = address diff --git a/desktop/src-tauri/src/problem.rs b/desktop/src-tauri/src/problem.rs index 285418fd84..16dfa06b4f 100644 --- a/desktop/src-tauri/src/problem.rs +++ b/desktop/src-tauri/src/problem.rs @@ -36,6 +36,21 @@ pub struct Problem { /// The full, verified Compose volume name offered for an explicit fresh-install reset. #[serde(skip_serializing_if = "Option::is_none")] pub database_reset: Option, + /// A verified fresh-install reset of everything a previous installation left behind. + /// Boxed and flattened: the window reads `leftover_reset` and `leftover_containers`. + #[serde(flatten, skip_serializing_if = "Option::is_none")] + pub leftover: Option>, +} + +/// Exactly what one confirmed reset removes: the containers first, then the volumes. +#[derive(Clone, Debug, Default, Serialize, PartialEq, Eq)] +pub struct LeftoverReset { + /// Every verified volume, the database included. + #[serde(rename = "leftover_reset", skip_serializing_if = "Vec::is_empty")] + pub volumes: Vec, + /// Stopped containers that still hold those volumes. + #[serde(rename = "leftover_containers", skip_serializing_if = "Vec::is_empty")] + pub containers: Vec, } impl Problem { @@ -46,6 +61,7 @@ impl Problem { detail: None, connection: None, database_reset: None, + leftover: None, } } @@ -57,9 +73,22 @@ impl Problem { detail: (!detail.trim().is_empty()).then_some(detail), connection: None, database_reset: None, + leftover: None, } } + /// The volumes a confirmed reset removes, or none when no reset is offered. + pub fn leftover_volumes(&self) -> &[String] { + self.leftover.as_deref().map_or(&[], |reset| &reset.volumes) + } + + /// The stopped containers a confirmed reset removes before those volumes. + pub fn leftover_containers(&self) -> &[String] { + self.leftover + .as_deref() + .map_or(&[], |reset| &reset.containers) + } + pub fn connection(mut self, connection: Connection) -> Self { self.connection = Some(connection); self diff --git a/desktop/src-tauri/src/stack.rs b/desktop/src-tauri/src/stack.rs index 22e01c8455..79d5a7df1f 100644 --- a/desktop/src-tauri/src/stack.rs +++ b/desktop/src-tauri/src/stack.rs @@ -213,6 +213,12 @@ fn postgres_configuration( } fn named_volume_exists(engine: &Address, root: &Path, volume: &str) -> Result { + Ok(volume_inventory(engine, root)? + .iter() + .any(|name| name == volume)) +} + +fn volume_inventory(engine: &Address, root: &Path) -> Result, Problem> { let inventory = engine .command() .current_dir(root) @@ -227,7 +233,12 @@ fn named_volume_exists(engine: &Address, root: &Path, volume: &str) -> Result) -> Problem { @@ -243,65 +254,120 @@ pub fn leftover_database_volume( root: &Path, secrets: &Secrets, ) -> Result, Problem> { + let config = leftover_configuration(engine, root, secrets)?; + let inventory = volume_inventory(engine, root)?; + owned_database_volume(engine, root, &config, &inventory) +} + +fn leftover_configuration( + engine: &Address, + root: &Path, + secrets: &Secrets, +) -> Result { let configuration = postgres_configuration(engine, root, secrets)?; - let config: serde_json::Value = serde_json::from_slice(&configuration) - .map_err(|_| postgres_volume_problem("Compose config did not return valid JSON."))?; - let Some((source, definition)) = postgres_data_volume(&config) else { + serde_json::from_slice(&configuration) + .map_err(|_| postgres_volume_problem("Compose config did not return valid JSON.")) +} + +fn owned_database_volume( + engine: &Address, + root: &Path, + config: &serde_json::Value, + inventory: &[String], +) -> Result, Problem> { + let Some((source, definition)) = postgres_data_volume(config) else { return Ok(None); }; let Some(project) = config["name"].as_str().filter(|name| !name.is_empty()) else { return Ok(None); }; - let Some(name) = definition["name"].as_str() else { + let Some(name) = ordinary_project_volume(project, source, definition) else { return Ok(None); }; - // Compose's explicit names and external volumes can refer to somebody else's database. - // Permit only ordinary project-scoped, local storage, even if a custom volume has labels. - if name != format!("{project}_{source}") - || !name.bytes().enumerate().all(|(index, byte)| { + // Another service mounting the database volume, under any alias, makes it shared storage. + if config["services"].as_object().is_some_and(|services| { + services.iter().any(|(service, configuration)| { + service != "postgres" + && configuration["volumes"].as_array().is_some_and(|mounts| { + mounts.iter().any(|mount| { + mount["source"].as_str().is_some_and(|other_source| { + other_source == source + || config["volumes"][other_source]["name"].as_str() == Some(name) + }) + }) + }) + }) + }) { + return Ok(None); + } + owned_compose_volume(engine, root, project, source, name, inventory) +} + +/// Compose's explicit names and external volumes can refer to somebody else's data. +/// Permit only ordinary project-scoped, local storage, even if a custom volume has labels. +fn ordinary_project_volume<'a>( + project: &str, + source: &str, + definition: &'a serde_json::Value, +) -> Option<&'a str> { + let name = definition["name"].as_str()?; + (name == format!("{project}_{source}") + && name.bytes().enumerate().all(|(index, byte)| { byte.is_ascii_alphanumeric() || (index > 0 && b"_.-".contains(&byte)) }) - || !matches!( + && matches!( definition.get("external"), None | Some(serde_json::Value::Bool(false)) ) - || definition["driver"] + && !definition["driver"] .as_str() .is_some_and(|driver| driver != "local") - || !empty_volume_options(&definition["driver_opts"]) - || config["services"].as_object().is_some_and(|services| { - services.iter().any(|(service, configuration)| { - service != "postgres" - && configuration["volumes"].as_array().is_some_and(|mounts| { - mounts.iter().any(|mount| { - mount["source"].as_str().is_some_and(|other_source| { - other_source == source - || config["volumes"][other_source]["name"].as_str() - == Some(name) - }) - }) - }) - }) - }) - { + && empty_volume_options(&definition["driver_opts"])) + .then_some(name) +} + +fn owned_compose_volume( + engine: &Address, + root: &Path, + project: &str, + source: &str, + name: &str, + inventory: &[String], +) -> Result, Problem> { + if !inventory.iter().any(|listed| listed == name) { return Ok(None); } - if !named_volume_exists(engine, root, name)? { + let Some(labels) = inspect_local_volume(engine, root, name)? else { return Ok(None); - } + }; + Ok( + (labels["com.docker.compose.project"].as_str() == Some(project) + && labels["com.docker.compose.volume"].as_str() == Some(source)) + .then(|| name.to_owned()), + ) +} + +/// The labels of exactly this local volume, or `None` when the engine describes anything else. +fn inspect_local_volume( + engine: &Address, + root: &Path, + name: &str, +) -> Result, Problem> { let inspect = engine .command() .current_dir(root) .args(["volume", "inspect", name]) .output() .map_err(|error| { - postgres_volume_problem(format!("Could not inspect the database volume: {error}")) + postgres_volume_problem(format!("Could not inspect volume {name}: {error}")) })?; if !inspect.status.success() { - return Err(postgres_volume_problem(format!("Database volume inspection exited with {}. Output omitted because it can contain credentials.", inspect.status))); + return Err(postgres_volume_problem(format!("Inspection of volume {name} exited with {}. Output omitted because it can contain credentials.", inspect.status))); } let inspected: serde_json::Value = serde_json::from_slice(&inspect.stdout).map_err(|_| { - postgres_volume_problem("Database volume inspection did not return valid JSON.") + postgres_volume_problem(format!( + "Inspection of volume {name} did not return valid JSON." + )) })?; let Some(volumes) = inspected.as_array().filter(|volumes| volumes.len() == 1) else { return Ok(None); @@ -311,36 +377,402 @@ pub fn leftover_database_volume( // tied to the full name the person confirmed, never a similarly named backup. Ok((volume["Name"].as_str() == Some(name) && volume["Driver"].as_str() == Some("local") - && empty_volume_options(&volume["Options"]) - && volume["Labels"]["com.docker.compose.project"].as_str() == Some(project) - && volume["Labels"]["com.docker.compose.volume"].as_str() == Some(source)) + && empty_volume_options(&volume["Options"])) + .then(|| volume["Labels"].clone())) +} + +/// A Bot's profile or workspace volume, created by the supervisor in this namespace. +/// Matches `supervisor/src/names.ts` names and `labelsFor` in `supervisor/src/docker.ts`; +/// all three labels are required, so an older or foreign volume is never claimed. +fn owned_supervisor_volume( + engine: &Address, + root: &Path, + namespace: &str, + name: &str, +) -> Result, Problem> { + let Some(bot) = ["profile", "workspace"] + .iter() + .find_map(|kind| name.strip_prefix(&format!("{namespace}-{kind}-"))) + .filter(|bot| supervisor_identifier(bot)) + else { + return Ok(None); + }; + let Some(labels) = inspect_local_volume(engine, root, name)? else { + return Ok(None); + }; + Ok((labels["openbot.supervisor"].as_str() == Some("true") + && labels["openbot.bot-id"].as_str() == Some(bot) + && labels["openbot.namespace"].as_str() == Some(namespace)) .then(|| name.to_owned())) } -pub fn reset_leftover_database( +/// The namespace the selected supervisor will run with, as `supervisor/src/names.ts` reads it. +/// No supervisor, or a namespace it would refuse, owns no volumes. +fn supervisor_namespace(config: &serde_json::Value) -> Option { + let supervisor = config["services"]["supervisor"].as_object()?; + let configured = match supervisor + .get("environment") + .and_then(|environment| environment.get("COMPUTER_NAMESPACE")) + { + None | Some(serde_json::Value::Null) => "", + Some(value) => value.as_str()?, + }; + resolve_namespace(configured).map(str::to_owned) +} + +/// Match supervisor/src/names.ts: trim, default only an empty value, then the same 64-character +/// ASCII identifier grammar. +fn resolve_namespace(configured: &str) -> Option<&str> { + let namespace = match configured.trim() { + "" => "openbot", + value => value, + }; + supervisor_identifier(namespace).then_some(namespace) +} + +fn supervisor_identifier(value: &str) -> bool { + !value.is_empty() + && value.len() <= 64 + && value.as_bytes()[0].is_ascii_alphanumeric() + && value + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-' || byte == b'_') +} + +/// What a previous installation of this deployment left on the selected engine. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct LeftoverVolumes { + /// The Postgres volume, when Compose and the engine agree this project owns it. + pub database: Option, + /// Whether the volume Compose selects for Postgres exists at all, owned or not. + pub database_exists: bool, + /// Every verified, owned volume, sorted. This is exactly what a reset removes. + pub volumes: Vec, + /// Stopped containers this deployment provably owns, sorted by name. They still hold the + /// volumes, so a reset removes them first. + pub containers: Vec, + /// Owned containers that are not stopped, by name. Any one of them blocks a reset. + pub running: Vec, +} + +/// A stopped container a reset may remove, named for the person and removed by its exact id. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct LeftoverContainer { + pub name: String, + pub id: String, +} + +impl LeftoverVolumes { + /// A reset must not leave a database it cannot verify for the new installation to reuse, + /// and never touches a container that is still running. + pub fn resettable(&self) -> bool { + !self.volumes.is_empty() + && (self.database.is_some() || !self.database_exists) + && self.running.is_empty() + } + + pub fn container_names(&self) -> Vec { + self.containers + .iter() + .map(|container| container.name.clone()) + .collect() + } + + /// The sentence for a reset that a running container blocks, or `None` when none is running. + pub fn running_said(&self) -> Option { + (!self.running.is_empty()).then(|| { + format!( + "A container from the previous installation is still running: {}. Stop it in Docker or Podman, then try Start again.", + self.running.join(", ") + ) + }) + } +} + +fn leftover_container_problem(detail: impl Into) -> Problem { + Problem::with( + "OpenBot could not verify which containers a previous installation left. Nothing was changed. Check the selected container engine, then try again.", + detail, + ) +} + +/// The ids carrying every one of these labels. Only a hint: each one is inspected before use. +fn labelled_containers( engine: &Address, root: &Path, - secrets: &Secrets, - confirmed_volume: &str, -) -> Result<(), Problem> { - if leftover_database_volume(engine, root, secrets)?.as_deref() != Some(confirmed_volume) { - return Err(Problem::plain("The leftover database no longer matches the volume you confirmed, or OpenBot could not verify that it owns the volume. Nothing was removed. Try Start again.")); + labels: &[(&str, &str)], +) -> Result, Problem> { + let mut command = engine.command(); + command + .current_dir(root) + .args(["ps", "--all", "--quiet", "--no-trunc"]); + for (key, value) in labels { + command.args(["--filter", &format!("label={key}={value}")]); } - // Never force this operation: the engine must refuse any container attachment, including a - // stopped container or one created after inspection. Do not stop containers to make it pass. - let removed = engine + let listed = command.output().map_err(|error| { + leftover_container_problem(format!("Could not list containers: {error}")) + })?; + if !listed.status.success() { + return Err(leftover_container_problem(format!( + "Container listing exited with {}. Output omitted because it can contain credentials.", + listed.status + ))); + } + let listed = std::str::from_utf8(&listed.stdout) + .map_err(|_| leftover_container_problem("Container listing was not valid UTF-8."))?; + Ok(listed.split_whitespace().map(str::to_owned).collect()) +} + +enum ContainerState { + Stopped, + Running, +} + +/// The name and state of a container whose labels prove this deployment owns it, or `None`. +fn owned_container( + engine: &Address, + root: &Path, + id: &str, + project: Option<&str>, + namespace: Option<&str>, +) -> Result, Problem> { + let inspect = engine .command() .current_dir(root) - .args(["volume", "rm", confirmed_volume]) + .args(["container", "inspect", id]) .output() .map_err(|error| { - Problem::with( - "OpenBot could not reset the leftover database. Nothing else was removed.", - format!("Could not run volume removal: {error}"), - ) + leftover_container_problem(format!("Could not inspect container {id}: {error}")) })?; - if !removed.status.success() { - return Err(Problem::with("OpenBot could not reset the leftover database. It may still be attached to a container. Nothing else was removed; stop the installation using it before trying again.", format!("Volume removal exited with {}. Output omitted because it can contain credentials.", removed.status))); + if !inspect.status.success() { + return Err(leftover_container_problem(format!( + "Inspection of container {id} exited with {}. Output omitted because it can contain credentials.", + inspect.status + ))); + } + let inspected: serde_json::Value = serde_json::from_slice(&inspect.stdout).map_err(|_| { + leftover_container_problem(format!( + "Inspection of container {id} did not return valid JSON." + )) + })?; + let Some(containers) = inspected + .as_array() + .filter(|containers| containers.len() == 1) + else { + return Ok(None); + }; + let container = &containers[0]; + // Exact ids only: an engine resolving a prefix to something else is not this container. + if container["Id"].as_str() != Some(id) { + return Ok(None); + } + // Docker reports `/name`; Podman reports `name`. + let Some(name) = container["Name"] + .as_str() + .map(|name| name.strip_prefix('/').unwrap_or(name)) + .filter(|name| !name.is_empty()) + else { + return Ok(None); + }; + let labels = &container["Config"]["Labels"]; + let compose_owned = project + .is_some_and(|project| labels["com.docker.compose.project"].as_str() == Some(project)); + // Matches `namesFor` in supervisor/src/names.ts and `labelsFor` in supervisor/src/docker.ts. + let supervisor_owned = namespace.is_some_and(|namespace| { + labels["openbot.supervisor"].as_str() == Some("true") + && labels["openbot.namespace"].as_str() == Some(namespace) + && labels["openbot.bot-id"].as_str().is_some_and(|bot| { + supervisor_identifier(bot) && name == format!("{namespace}-computer-{bot}") + }) + }); + if !compose_owned && !supervisor_owned { + return Ok(None); + } + let state = &container["State"]; + // Only states a plain removal accepts count as stopped. Anything else, or anything unknown, + // is treated as running so it is never touched. + let stopped = state["Running"].as_bool() == Some(false) + && state["Status"].as_str().is_some_and(|status| { + matches!(status, "created" | "exited" | "stopped" | "configured") + }); + Ok(Some(( + name.to_owned(), + if stopped { + ContainerState::Stopped + } else { + ContainerState::Running + }, + ))) +} + +fn leftover_containers( + engine: &Address, + root: &Path, + project: Option<&str>, + namespace: Option<&str>, +) -> Result<(Vec, Vec), Problem> { + let mut ids = Vec::new(); + if let Some(project) = project { + ids.extend(labelled_containers( + engine, + root, + &[("com.docker.compose.project", project)], + )?); + } + if let Some(namespace) = namespace { + ids.extend(labelled_containers( + engine, + root, + &[ + ("openbot.supervisor", "true"), + ("openbot.namespace", namespace), + ], + )?); + } + ids.sort(); + ids.dedup(); + let mut stopped = Vec::new(); + let mut running = Vec::new(); + for id in ids { + match owned_container(engine, root, &id, project, namespace)? { + Some((name, ContainerState::Stopped)) => stopped.push(LeftoverContainer { name, id }), + Some((name, ContainerState::Running)) => running.push(name), + None => {} + } + } + stopped.sort_by(|a, b| a.name.cmp(&b.name)); + running.sort(); + Ok((stopped, running)) +} + +/** +Every volume a previous installation of this deployment left behind, and that it provably owns. + +A deleted installation folder removes `.env` and the vault, but not the engine's volumes. Compose +and the supervisor reattach them by name, so a new installation silently inherits the old one's +Bots' workspace files and browser sign-ins. Only three kinds are claimed: the verified Postgres +volume, the other ordinary project-scoped Compose volumes labelled for this project, and the +per-Bot volumes the supervisor labelled for this namespace. Never include config output in +diagnostics: interpolation may have put credentials in it. +*/ +pub fn leftover_volumes( + engine: &Address, + root: &Path, + secrets: &Secrets, +) -> Result { + let config = leftover_configuration(engine, root, secrets)?; + let inventory = volume_inventory(engine, root)?; + let postgres = postgres_data_volume(&config); + let database_exists = postgres + .and_then(|(_, definition)| definition["name"].as_str()) + .is_some_and(|name| inventory.iter().any(|listed| listed == name)); + let database = owned_database_volume(engine, root, &config, &inventory)?; + let mut volumes: Vec = database.iter().cloned().collect(); + let project = config["name"].as_str().filter(|name| !name.is_empty()); + let namespace = supervisor_namespace(&config); + if let (Some(project), Some(definitions)) = (project, config["volumes"].as_object()) { + for (source, definition) in definitions { + // The database has its own, stricter test above. + if postgres.is_some_and(|(postgres_source, _)| postgres_source == source) { + continue; + } + let Some(name) = ordinary_project_volume(project, source, definition) else { + continue; + }; + volumes.extend(owned_compose_volume( + engine, root, project, source, name, &inventory, + )?); + } + } + if let Some(namespace) = &namespace { + for name in &inventory { + volumes.extend(owned_supervisor_volume(engine, root, namespace, name)?); + } + } + volumes.sort(); + volumes.dedup(); + // Stopped containers keep their volumes attached, so the engine refuses to remove them. Only + // worth listing when there is data to reset. + let (containers, running) = if volumes.is_empty() { + Default::default() + } else { + leftover_containers(engine, root, project, namespace.as_deref())? + }; + Ok(LeftoverVolumes { + database, + database_exists, + volumes, + containers, + running, + }) +} + +/// Remove exactly the leftover containers and volumes the person confirmed, containers first, +/// or nothing when either set changed or an owned container is running. +pub fn reset_leftover_volumes( + engine: &Address, + root: &Path, + secrets: &Secrets, + confirmed: &[String], + confirmed_containers: &[String], +) -> Result<(), Problem> { + let leftovers = leftover_volumes(engine, root, secrets)?; + if let Some(said) = leftovers.running_said() { + return Err(Problem::plain(format!("{said} Nothing was removed."))); + } + let sorted = |names: &[String]| { + let mut names = names.to_vec(); + names.sort(); + names + }; + if !leftovers.resettable() + || leftovers.volumes != sorted(confirmed) + || leftovers.container_names() != sorted(confirmed_containers) + { + return Err(Problem::plain("The leftover data no longer matches the containers and volumes you confirmed, or OpenBot could not verify that it owns them. Nothing was removed. Try Start again.")); + } + let mut removed: Vec<&str> = Vec::new(); + let removals = leftovers + .containers + .iter() + .map(|container| ("container", container.id.as_str(), container.name.as_str())) + .chain( + leftovers + .volumes + .iter() + .map(|volume| ("volume", volume.as_str(), volume.as_str())), + ); + for (kind, target, name) in removals { + let so_far = || match removed.as_slice() { + [] => "Nothing was removed.".to_owned(), + removed => format!("Already removed: {}.", removed.join(", ")), + }; + // Never force this operation: the engine must refuse a running container and any + // container attachment, including one created after inspection. Do not stop containers + // to make it pass. + let output = engine + .command() + .current_dir(root) + .args([kind, "rm", target]) + .output() + .map_err(|error| { + Problem::with( + format!("OpenBot could not remove {name}. {}", so_far()), + format!("Could not run {kind} removal: {error}"), + ) + })?; + if !output.status.success() { + let why = if kind == "container" { + "It may have started again." + } else { + "It may still be attached to a container." + }; + return Err(Problem::with( + format!("OpenBot could not remove {name}. {why} {} Stop the installation using it, then try Start again.", so_far()), + format!("{kind} removal exited with {}. Output omitted because it can contain credentials.", output.status), + )); + } + removed.push(name); } Ok(()) } @@ -732,20 +1164,9 @@ fn computer_namespace(engine: &Address, root: &Path) -> Result, S .pointer("/services/supervisor/environment/COMPUTER_NAMESPACE") .and_then(serde_json::Value::as_str) .ok_or("could not resolve computer namespace: supervisor configuration has no namespace")?; - // Match supervisor/src/names.ts: trim, default only an empty value, then the same 64-character - // ASCII identifier grammar. A malformed/missing response never becomes an unscoped filter. - let namespace = match configured.trim() { - "" => "openbot", - value => value, - }; - if namespace.len() > 64 - || !namespace.as_bytes()[0].is_ascii_alphanumeric() - || !namespace - .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-' || byte == b'_') - { - return Err("could not resolve computer namespace: supervisor namespace is invalid".into()); - } + // A malformed/missing response never becomes an unscoped filter. + let namespace = resolve_namespace(configured) + .ok_or("could not resolve computer namespace: supervisor namespace is invalid")?; Ok(Some(namespace.to_string())) } @@ -3209,14 +3630,25 @@ mod tests { leftover_database_volume(&address, &root, &Secrets::new()).unwrap(), Some("openbot_postgres-data".into()) ); - assert!( - reset_leftover_database(&address, &root, &Secrets::new(), "unrelated").is_err() - ); + assert!(reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &["unrelated".into()], + &[] + ) + .is_err()); assert!(!std::fs::read_to_string(&record) .unwrap() .contains("volume rm")); - reset_leftover_database(&address, &root, &Secrets::new(), "openbot_postgres-data") - .unwrap(); + reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &["openbot_postgres-data".into()], + &[], + ) + .unwrap(); assert_eq!( std::fs::read_to_string(root.join(".fixture-volumes")).unwrap(), "unrelated\n" @@ -3329,9 +3761,14 @@ mod tests { if let Some(marker) = marker { std::fs::write(root.join(marker), "").unwrap(); } - let error = - reset_leftover_database(&address, &root, &Secrets::new(), "openbot_postgres-data") - .expect_err(scenario); + let error = reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &["openbot_postgres-data".into()], + &[], + ) + .expect_err(scenario); assert!( !format!("{error:?}").contains("synthetic-secret"), "{scenario}" @@ -3357,6 +3794,462 @@ mod tests { } } + /// A reinstall's Compose project and supervisor volumes, beside look-alikes it does not own. + fn leftover_volumes_fixture(root: &Path, namespace: &str) { + let config = serde_json::json!({ + "name": "openbot", + "services": { + "postgres": {"volumes": [{ + "type": "volume", "source": "postgres-data", "target": "/var/lib/postgresql/data" + }]}, + "supervisor": {"environment": {"COMPUTER_NAMESPACE": namespace}}, + "agent-computer": {"volumes": [ + {"type": "volume", "source": "agent-profiles", "target": "/profiles"}, + {"type": "volume", "source": "agent-workspace", "target": "/workspace"} + ]} + }, + "volumes": { + "postgres-data": {"name": "openbot_postgres-data"}, + "agent-profiles": {"name": "openbot_agent-profiles"}, + "agent-workspace": {"name": "openbot_agent-workspace"}, + "spire-server-data": {"name": "openbot_spire-server-data"}, + "unlabeled": {"name": "openbot_unlabeled"}, + "foreign": {"name": "openbot_foreign"}, + "shared-cache": {"name": "shared-cache", "external": true}, + "explicit": {"name": "explicitly-named"}, + "remote": {"name": "openbot_remote", "driver_opts": {"device": "/shared"}} + } + }); + std::fs::write(root.join(".fixture-config"), config.to_string()).unwrap(); + std::fs::write( + root.join(".fixture-volumes"), + [ + "unrelated", + "openbot_agent-profiles", + "openbot_agent-workspace", + "openbot_unlabeled", + "openbot_foreign", + "shared-cache", + "explicitly-named", + "openbot_remote", + "openbot-profile-general-assistant", + "openbot-workspace-general-assistant", + "openbot-profile-unlabeled", + "openbot-workspace-elsewhere", + "openbot-profile-legacy", + "other-profile-knowledge", + ] + .map(|name| format!("{name}\n")) + .concat(), + ) + .unwrap(); + let inspect = |name: &str, labels: serde_json::Value| { + std::fs::write( + root.join(format!(".fixture-inspect-{name}")), + serde_json::json!([{ + "Name": name, "Driver": "local", "Options": {}, "Labels": labels + }]) + .to_string(), + ) + .unwrap(); + }; + let compose = |project: &str, source: &str| { + serde_json::json!({ + "com.docker.compose.project": project, "com.docker.compose.volume": source + }) + }; + let supervisor = |bot: &str, namespace: &str| { + serde_json::json!({ + "openbot.supervisor": "true", "openbot.bot-id": bot, "openbot.namespace": namespace + }) + }; + inspect( + "openbot_agent-profiles", + compose("openbot", "agent-profiles"), + ); + inspect( + "openbot_agent-workspace", + compose("openbot", "agent-workspace"), + ); + inspect("openbot_unlabeled", serde_json::json!({})); + inspect("openbot_foreign", compose("another-project", "foreign")); + inspect( + "openbot-profile-general-assistant", + supervisor("general-assistant", "openbot"), + ); + inspect( + "openbot-workspace-general-assistant", + supervisor("general-assistant", "openbot"), + ); + inspect("openbot-profile-unlabeled", serde_json::json!({})); + inspect( + "openbot-workspace-elsewhere", + supervisor("elsewhere", "other"), + ); + inspect( + "openbot-profile-legacy", + serde_json::json!({"openbot.supervisor": "true", "openbot.bot-id": "legacy"}), + ); + inspect("other-profile-knowledge", supervisor("knowledge", "other")); + } + + const OWNED_LEFTOVERS: [&str; 4] = [ + "openbot-profile-general-assistant", + "openbot-workspace-general-assistant", + "openbot_agent-profiles", + "openbot_agent-workspace", + ]; + + #[test] + fn leftover_volumes_are_only_volumes_this_project_or_namespace_provably_owns() { + if crate::test_support::isolated_process( + "stack::tests::leftover_volumes_are_only_volumes_this_project_or_namespace_provably_owns", + ) { + return; + } + let path = PathFixture::with_fake_engine("postgres-volume"); + let address = computer_stop_addresses(&path)[0].clone(); + let root = path.bin.join("leftover-volumes"); + std::fs::create_dir(&root).unwrap(); + let record = root.join("commands.log"); + std::env::set_var("OPENBOT_TEST_ENGINE_RECORD", &record); + + leftover_volumes_fixture(&root, "openbot"); + let found = leftover_volumes(&address, &root, &Secrets::new()).unwrap(); + assert_eq!(found.volumes, OWNED_LEFTOVERS.map(String::from)); + assert_eq!(found.database, None); + assert!(!found.database_exists); + assert!(found.resettable()); + let log = std::fs::read_to_string(&record).unwrap(); + for never_inspected in ["shared-cache", "explicitly-named", "openbot_remote"] { + assert!( + !log.contains(&format!("volume inspect {never_inspected}")), + "{never_inspected}: {log}" + ); + } + assert!(!log.contains("volume rm")); + + // Another deployment's namespace selects only that namespace's Bots. + leftover_volumes_fixture(&root, "other"); + assert_eq!( + leftover_volumes(&address, &root, &Secrets::new()) + .unwrap() + .volumes, + [ + "openbot_agent-profiles", + "openbot_agent-workspace", + "other-profile-knowledge" + ] + ); + + // The database joins the set when it is verified, and blocks the offer when it is not. + leftover_volumes_fixture(&root, "openbot"); + let mut inventory = std::fs::read_to_string(root.join(".fixture-volumes")).unwrap(); + inventory.push_str("openbot_postgres-data\n"); + std::fs::write(root.join(".fixture-volumes"), inventory).unwrap(); + let database = serde_json::json!([{ + "Name": "openbot_postgres-data", "Driver": "local", "Options": {}, + "Labels": {"com.docker.compose.project":"openbot", "com.docker.compose.volume":"postgres-data"} + }]); + std::fs::write(root.join(".fixture-inspect"), database.to_string()).unwrap(); + let found = leftover_volumes(&address, &root, &Secrets::new()).unwrap(); + assert_eq!(found.database.as_deref(), Some("openbot_postgres-data")); + assert!(found.database_exists && found.resettable()); + let mut expected = OWNED_LEFTOVERS.map(String::from).to_vec(); + expected.push("openbot_postgres-data".into()); + expected.sort(); + assert_eq!(found.volumes, expected); + + let mut foreign = database.clone(); + foreign[0]["Labels"]["com.docker.compose.project"] = "another-project".into(); + std::fs::write(root.join(".fixture-inspect"), foreign.to_string()).unwrap(); + let found = leftover_volumes(&address, &root, &Secrets::new()).unwrap(); + assert_eq!(found.database, None); + assert!(found.database_exists); + assert_eq!(found.volumes, OWNED_LEFTOVERS.map(String::from)); + assert!( + !found.resettable(), + "an unverified database must not be left behind by a reset" + ); + std::fs::write(record, "").unwrap(); + assert!(reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &OWNED_LEFTOVERS.map(String::from), + &[] + ) + .is_err()); + assert!(!std::fs::read_to_string(root.join("commands.log")) + .unwrap() + .contains("volume rm")); + } + + #[test] + fn leftover_volume_reset_removes_exactly_the_confirmed_owned_set() { + if crate::test_support::isolated_process( + "stack::tests::leftover_volume_reset_removes_exactly_the_confirmed_owned_set", + ) { + return; + } + let path = PathFixture::with_fake_engine("postgres-volume"); + for address in computer_stop_addresses(&path) { + let root = path + .bin + .join(format!("{}-leftover-reset", address.engine.binary())); + std::fs::create_dir(&root).unwrap(); + let record = root.join("commands.log"); + std::env::set_var("OPENBOT_TEST_ENGINE_RECORD", &record); + leftover_volumes_fixture(&root, "openbot"); + let owned = OWNED_LEFTOVERS.map(String::from).to_vec(); + let mut duplicated = owned.clone(); + duplicated.push(owned[0].clone()); + let mut superset = owned.clone(); + superset.push("other-profile-knowledge".into()); + for stale in [ + owned[..3].to_vec(), + superset, + duplicated, + vec!["unrelated".into()], + Vec::new(), + ] { + let error = reset_leftover_volumes(&address, &root, &Secrets::new(), &stale, &[]) + .expect_err("a changed set must not be removed"); + assert!(error.said.contains("Nothing was removed"), "{}", error.said); + assert!(!std::fs::read_to_string(&record) + .unwrap_or_default() + .contains("volume rm")); + } + + // The engine refuses a volume a container still uses; nothing is forced. + std::fs::write( + root.join(".fixture-attached-openbot-profile-general-assistant"), + "", + ) + .unwrap(); + let error = reset_leftover_volumes(&address, &root, &Secrets::new(), &owned, &[]) + .expect_err("an attached volume is refused"); + assert!( + error.said.contains("openbot-profile-general-assistant"), + "{}", + error.said + ); + assert!(error.said.contains("Nothing was removed"), "{}", error.said); + assert!(!format!("{error:?}").contains("synthetic-secret")); + std::fs::remove_file(root.join(".fixture-attached-openbot-profile-general-assistant")) + .unwrap(); + + let mut reordered = owned.clone(); + reordered.reverse(); + reset_leftover_volumes(&address, &root, &Secrets::new(), &reordered, &[]).unwrap(); + let remaining = std::fs::read_to_string(root.join(".fixture-volumes")).unwrap(); + for name in &owned { + assert!(!remaining.lines().any(|line| line == name), "{name}"); + } + for kept in [ + "unrelated", + "openbot_unlabeled", + "openbot_foreign", + "shared-cache", + "explicitly-named", + "openbot_remote", + "openbot-profile-unlabeled", + "openbot-workspace-elsewhere", + "openbot-profile-legacy", + "other-profile-knowledge", + ] { + assert!(remaining.lines().any(|line| line == kept), "{kept}"); + } + let log = std::fs::read_to_string(&record).unwrap(); + let removals: Vec<_> = log + .lines() + .filter_map(|line| line.split_once("volume rm ")) + .map(|(_, name)| name) + .collect(); + // One refused attempt, then each confirmed volume once. + assert_eq!(removals[0], "openbot-profile-general-assistant"); + assert_eq!(removals[1..], OWNED_LEFTOVERS); + assert!(!log.contains("--force") && !log.contains("prune") && !log.contains("down")); + if address.engine == crate::engine::Engine::Podman { + assert!(log.lines().all(|line| line + .split_once('\t') + .unwrap() + .1 + .starts_with("--connection fixture-machine "))); + } + } + } + + /// The previous installation's stopped computers and Compose containers, beside ones it + /// does not own. Each still mounts the volumes it used, as a real engine reports it. + fn leftover_containers_fixture(root: &Path, computer_status: &str) { + let supervisor = |bot: &str, namespace: &str| { + format!("openbot.supervisor=true,openbot.bot-id={bot},openbot.namespace={namespace}") + }; + let lines = [ + format!( + "c-computer\topenbot-computer-general-assistant\t{computer_status}\t{}\topenbot-profile-general-assistant,openbot-workspace-general-assistant", + supervisor("general-assistant", "openbot") + ), + "c-compose\topenbot-agent-computer-1\texited\tcom.docker.compose.project=openbot,com.docker.compose.service=agent-computer\topenbot_agent-profiles,openbot_agent-workspace".into(), + format!("c-other-namespace\tother-computer-knowledge\texited\t{}\tother-profile-knowledge", supervisor("knowledge", "other")), + "c-unlabeled\topenbot-computer-unlabeled\texited\t\topenbot-profile-unlabeled".into(), + "c-other-project\tanother-project-postgres-1\texited\tcom.docker.compose.project=another-project\t".into(), + format!("c-renamed\topenbot-computer-not-this-bot\texited\t{}\t", supervisor("general-assistant", "openbot")), + "c-legacy\topenbot-computer-legacy\texited\topenbot.supervisor=true,openbot.bot-id=legacy\topenbot-profile-legacy".into(), + ]; + std::fs::write( + root.join(".fixture-containers"), + lines.map(|line| format!("{line}\n")).concat(), + ) + .unwrap(); + } + + const OWNED_CONTAINERS: [&str; 2] = [ + "openbot-agent-computer-1", + "openbot-computer-general-assistant", + ]; + + #[test] + fn leftover_reset_removes_stopped_owned_containers_before_their_volumes() { + if crate::test_support::isolated_process( + "stack::tests::leftover_reset_removes_stopped_owned_containers_before_their_volumes", + ) { + return; + } + let path = PathFixture::with_fake_engine("postgres-volume"); + for address in computer_stop_addresses(&path) { + let root = path + .bin + .join(format!("{}-leftover-containers", address.engine.binary())); + std::fs::create_dir(&root).unwrap(); + let record = root.join("commands.log"); + std::env::set_var("OPENBOT_TEST_ENGINE_RECORD", &record); + leftover_volumes_fixture(&root, "openbot"); + leftover_containers_fixture(&root, "exited"); + + let found = leftover_volumes(&address, &root, &Secrets::new()).unwrap(); + assert_eq!(found.volumes, OWNED_LEFTOVERS.map(String::from)); + assert_eq!( + found + .containers + .iter() + .map(|container| container.name.as_str()) + .collect::>(), + OWNED_CONTAINERS + ); + assert!(found.running.is_empty() && found.resettable()); + + let volumes = OWNED_LEFTOVERS.map(String::from).to_vec(); + let containers = OWNED_CONTAINERS.map(String::from).to_vec(); + for (stale_volumes, stale_containers) in [ + (volumes.clone(), Vec::new()), + (volumes.clone(), containers[..1].to_vec()), + ( + volumes.clone(), + [containers.clone(), vec!["other-computer-knowledge".into()]].concat(), + ), + (Vec::new(), containers.clone()), + ] { + let error = reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &stale_volumes, + &stale_containers, + ) + .expect_err("a changed set must not be removed"); + assert!(error.said.contains("Nothing was removed"), "{}", error.said); + assert!(!std::fs::read_to_string(&record).unwrap().contains(" rm ")); + } + + reset_leftover_volumes(&address, &root, &Secrets::new(), &volumes, &containers) + .unwrap(); + let log = std::fs::read_to_string(&record).unwrap(); + let removals: Vec<_> = log + .lines() + .filter_map(|line| line.split_once('\t')) + .map(|(_, command)| { + command + .strip_prefix("--connection fixture-machine ") + .unwrap_or(command) + }) + .filter(|command| command.contains(" rm ")) + .collect(); + assert_eq!( + removals, + [ + "container rm c-compose", + "container rm c-computer", + "volume rm openbot-profile-general-assistant", + "volume rm openbot-workspace-general-assistant", + "volume rm openbot_agent-profiles", + "volume rm openbot_agent-workspace", + ] + ); + assert!(!log.contains("--force") && !log.contains("prune") && !log.contains("down")); + let kept = std::fs::read_to_string(root.join(".fixture-containers")).unwrap(); + let kept: Vec<_> = kept + .lines() + .map(|line| line.split('\t').next().unwrap()) + .collect(); + assert_eq!( + kept, + [ + "c-other-namespace", + "c-unlabeled", + "c-other-project", + "c-renamed", + "c-legacy" + ] + ); + } + } + + #[test] + fn a_running_owned_container_blocks_the_reset_and_removes_nothing() { + if crate::test_support::isolated_process( + "stack::tests::a_running_owned_container_blocks_the_reset_and_removes_nothing", + ) { + return; + } + let path = PathFixture::with_fake_engine("postgres-volume"); + let address = computer_stop_addresses(&path)[0].clone(); + let root = path.bin.join("leftover-running"); + std::fs::create_dir(&root).unwrap(); + let record = root.join("commands.log"); + std::env::set_var("OPENBOT_TEST_ENGINE_RECORD", &record); + leftover_volumes_fixture(&root, "openbot"); + for status in ["running", "paused", "restarting"] { + leftover_containers_fixture(&root, status); + let found = leftover_volumes(&address, &root, &Secrets::new()).unwrap(); + assert_eq!(found.running, ["openbot-computer-general-assistant"]); + assert!(!found.resettable(), "{status}"); + let error = reset_leftover_volumes( + &address, + &root, + &Secrets::new(), + &OWNED_LEFTOVERS.map(String::from), + &OWNED_CONTAINERS.map(String::from), + ) + .expect_err("a running container blocks the reset"); + assert!( + error.said.contains("openbot-computer-general-assistant") + && error.said.contains("still running") + && error.said.contains("Nothing was removed"), + "{}", + error.said + ); + assert!(!format!("{error:?}").contains("synthetic-secret")); + assert!(!std::fs::read_to_string(&record).unwrap().contains(" rm ")); + } + // A container listing that fails is not proof that nothing is running. + leftover_containers_fixture(&root, "exited"); + std::fs::write(root.join(".fixture-ps-failure"), "").unwrap(); + let error = leftover_volumes(&address, &root, &Secrets::new()).unwrap_err(); + assert!(!format!("{error:?}").contains("synthetic-secret")); + } + #[test] fn desktop_approval_transport_credential_reaches_only_the_server() { let secrets = Secrets::from([ @@ -4726,6 +5619,24 @@ fn main() { const FAKE_ENGINE_SOURCE: &str = r#" use std::io::Write; +/// `.fixture-containers`: one container per line, `idnamestatusk=v,k=vvolume,volume`. +struct FixtureContainer { id: String, name: String, status: String, labels: Vec<(String, String)>, mounts: Vec } + +fn fixture_containers() -> Vec { + let Ok(listing) = std::fs::read_to_string(".fixture-containers") else { return Vec::new() }; + listing.lines().filter(|line| !line.is_empty()).map(|line| { + let fields: Vec<&str> = line.split('\t').collect(); + let list = |field: &str| field.split(',').filter(|item| !item.is_empty()).map(str::to_owned).collect::>(); + FixtureContainer { + id: fields[0].into(), + name: fields[1].into(), + status: fields[2].into(), + labels: list(fields[3]).into_iter().map(|pair| { let (k, v) = pair.split_once('=').unwrap(); (k.to_owned(), v.to_owned()) }).collect(), + mounts: list(fields.get(4).copied().unwrap_or("")), + } + }).collect() +} + fn main() { let args: Vec = std::env::args().skip(1).collect(); let joined = args.join(" "); @@ -4747,15 +5658,71 @@ fn main() { (".fixture-volumes", ".fixture-volume-failure") } else if actual == ["volume", "inspect", "openbot_postgres-data"] { (".fixture-inspect", ".fixture-inspect-failure") - } else if actual == ["volume", "rm", "openbot_postgres-data"] { - if std::path::Path::new(".fixture-attached").exists() { + } else if actual.len() == 3 && actual[0] == "volume" && actual[1] == "inspect" { + let file = format!(".fixture-inspect-{}", actual[2]); + if !std::path::Path::new(&file).exists() { + eprintln!("no such volume: {}", actual[2]); + std::process::exit(1); + } + print!("{}", std::fs::read_to_string(file).unwrap()); + return; + } else if actual.len() == 3 && actual[0] == "volume" && actual[1] == "rm" { + let volume = actual[2].as_str(); + if (volume == "openbot_postgres-data" && std::path::Path::new(".fixture-attached").exists()) + || std::path::Path::new(&format!(".fixture-attached-{volume}")).exists() + || fixture_containers().iter().any(|container| container.mounts.iter().any(|mount| mount == volume)) + { eprintln!("synthetic-secret attached container"); std::process::exit(2); } let inventory = std::fs::read_to_string(".fixture-volumes").unwrap(); - let remaining: String = inventory.lines().filter(|name| *name != "openbot_postgres-data").map(|name| format!("{name}\n")).collect(); + let remaining: String = inventory.lines().filter(|name| *name != volume).map(|name| format!("{name}\n")).collect(); std::fs::write(".fixture-volumes", remaining).unwrap(); return; + } else if actual.first().map(String::as_str) == Some("ps") { + assert_eq!(actual[1..4], ["--all", "--quiet", "--no-trunc"], "{actual:?}"); + let wanted: Vec<(&str, &str)> = actual[4..].chunks(2).map(|pair| { + assert_eq!(pair[0], "--filter"); + pair[1].strip_prefix("label=").unwrap().split_once('=').unwrap() + }).collect(); + if std::path::Path::new(".fixture-ps-failure").exists() { + eprintln!("synthetic-secret-must-not-appear-in-diagnostics"); + std::process::exit(17); + } + for container in fixture_containers() { + if wanted.iter().all(|(key, value)| container.labels.iter().any(|(k, v)| k == key && v == value)) { + println!("{}", container.id); + } + } + return; + } else if actual.len() == 3 && actual[0] == "container" && actual[1] == "inspect" { + let Some(container) = fixture_containers().into_iter().find(|container| container.id == actual[2]) else { + eprintln!("no such container: {}", actual[2]); + std::process::exit(1); + }; + // Docker reports names with a leading slash; Podman does not. + let slash = if args.first().map(String::as_str) == Some("--connection") { "" } else { "/" }; + let labels: Vec = container.labels.iter().map(|(k, v)| format!("\"{k}\":\"{v}\"")).collect(); + let mounts: Vec = container.mounts.iter().map(|m| format!("{{\"Type\":\"volume\",\"Name\":\"{m}\"}}")).collect(); + print!( + "[{{\"Id\":\"{}\",\"Name\":\"{slash}{}\",\"State\":{{\"Status\":\"{}\",\"Running\":{}}},\"Config\":{{\"Labels\":{{{}}}}},\"Mounts\":[{}]}}]", + container.id, container.name, container.status, container.status == "running", labels.join(","), mounts.join(",") + ); + return; + } else if actual.len() == 3 && actual[0] == "container" && actual[1] == "rm" { + let containers = fixture_containers(); + let Some(container) = containers.iter().find(|container| container.id == actual[2]) else { + eprintln!("no such container: {}", actual[2]); + std::process::exit(1); + }; + if container.status == "running" { + eprintln!("synthetic-secret cannot remove a running container"); + std::process::exit(2); + } + let inventory = std::fs::read_to_string(".fixture-containers").unwrap(); + let remaining: String = inventory.lines().filter(|line| line.split('\t').next() != Some(actual[2].as_str())).map(|line| format!("{line}\n")).collect(); + std::fs::write(".fixture-containers", remaining).unwrap(); + return; } else { panic!("unexpected volume probe command: {actual:?}"); }; if std::path::Path::new(failure).exists() { eprintln!("synthetic-secret-must-not-appear-in-diagnostics"); diff --git a/desktop/src/App.test.tsx b/desktop/src/App.test.tsx index 98e42fe0ef..4f14f30f6d 100644 --- a/desktop/src/App.test.tsx +++ b/desktop/src/App.test.tsx @@ -1237,6 +1237,7 @@ test("leftover database recovery requires confirmation and keeps setup ready to throw { said: "A previous OpenBot database remains without its encryption key.", database_reset: "fixture_postgres-data", + leftover_reset: ["fixture_postgres-data"], }; if (command === "reset_leftover_database") return reset.promise; return previous(command, args); @@ -1253,6 +1254,7 @@ test("leftover database recovery requires confirmation and keeps setup ready to await view.findByRole("button", { name: "Reset leftover database" }), ); expect(view.getByText(/permanently deletes/)).toBeTruthy(); + expect(view.getByText("fixture_postgres-data")).toBeTruthy(); expect(resets()).toHaveLength(0); await userEvent.click(view.getByRole("button", { name: "Keep saved data" })); expect(resets()).toHaveLength(0); @@ -1268,7 +1270,8 @@ test("leftover database recovery requires confirmation and keeps setup ready to command: "reset_leftover_database", args: { root: "/tmp/openbot-app-test", - volume: "fixture_postgres-data", + volumes: ["fixture_postgres-data"], + containers: [], confirmed: true, }, }, @@ -1291,6 +1294,61 @@ test("leftover database recovery requires confirmation and keeps setup ready to expect(installationCalls()).toHaveLength(1); }); +test("leftover data recovery names every container and volume and removes only that list", async () => { + useCompatibleEndpointSetup({}); + const previous = invokeHandler; + const volumes = [ + "openbot-profile-general-assistant", + "openbot-workspace-general-assistant", + "openbot_agent-profiles", + ]; + const containers = [ + "openbot-agent-computer-1", + "openbot-computer-general-assistant", + ]; + invokeHandler = async (command, args) => { + if (command === "start_stack") + throw { + said: "OpenBot found data from a previous installation.", + leftover_reset: volumes, + leftover_containers: containers, + }; + if (command === "reset_leftover_database") return null; + return previous(command, args); + }; + const view = await enterCompatibleEndpoint( + "https://model.example/v1", + "synthetic-key", + ); + await userEvent.click(view.getByRole("button", { name: "Continue" })); + await userEvent.click(view.getByRole("button", { name: "Start OpenBot" })); + await userEvent.click( + await view.findByRole("button", { name: "Reset leftover data" }), + ); + expect(view.queryByText(/local OpenBot database/)).toBeNull(); + expect(view.getByText(/browser sign-ins/)).toBeTruthy(); + for (const volume of volumes) expect(view.getByText(volume)).toBeTruthy(); + expect(view.getByText(/stopped containers/)).toBeTruthy(); + for (const container of containers) + expect(view.getByText(container)).toBeTruthy(); + await userEvent.click( + view.getByRole("button", { name: "Delete saved data" }), + ); + expect( + invokeCalls.filter((call) => call.command === "reset_leftover_database"), + ).toEqual([ + { + command: "reset_leftover_database", + args: { + root: "/tmp/openbot-app-test", + volumes, + containers, + confirmed: true, + }, + }, + ]); +}); + test("database recovery failure is shown without restarting services", async () => { useCompatibleEndpointSetup({}); const previous = invokeHandler; diff --git a/desktop/src/App.tsx b/desktop/src/App.tsx index ad54068c62..61794c2bc1 100644 --- a/desktop/src/App.tsx +++ b/desktop/src/App.tsx @@ -296,6 +296,13 @@ export function App() { if (problem.connection === "organization") setRefreshOrganization(true); }, []); const displayedFailure = failure ?? recoveryFailure; + // An older shell offered only the database. The list is exactly what a reset may remove. + const leftoverVolumes = displayedFailure?.leftover_reset?.length + ? displayedFailure.leftover_reset + : displayedFailure?.database_reset + ? [displayedFailure.database_reset] + : []; + const leftoverContainers = displayedFailure?.leftover_containers ?? []; const credentialContext = useRef([ root, model, @@ -602,12 +609,13 @@ export function App() { } } - async function resetLeftoverDatabase(volume: string) { + async function resetLeftoverData(volumes: string[], containers: string[]) { setBusy(true); try { await invoke("reset_leftover_database", { root: root.trim(), - volume, + volumes, + containers, confirmed: true, }); setFailure(null); @@ -982,12 +990,14 @@ export function App() { {/* A failure outranks `running`. The supervisor gives up on a process and sends the window back here, and a heading that still says everything is running while the box underneath names the process that stopped is a screen arguing with itself. */} - {!running && displayedFailure?.database_reset && ( + {!running && leftoverVolumes.length > 0 && ( )} diff --git a/desktop/src/DatabaseReset.tsx b/desktop/src/DatabaseReset.tsx index 8d3dcd46a7..c528a179d1 100644 --- a/desktop/src/DatabaseReset.tsx +++ b/desktop/src/DatabaseReset.tsx @@ -1,14 +1,21 @@ import { useState } from "react"; -/** Only rendered when native setup has identified a recoverable leftover database. */ +/** Only rendered when native setup has identified leftover data it can reset. */ export function DatabaseReset({ busy, + containers, + database, onReset, - volume, + volumes, }: { busy: boolean; - volume: string; - onReset: (volume: string) => Promise; + /** Stopped containers that still hold the volumes. The reset removes them first. */ + containers: string[]; + /** Whether the local OpenBot database is one of the volumes. */ + database: boolean; + /** Exactly the volumes native setup verified, and the only ones a reset removes. */ + volumes: string[]; + onReset: (volumes: string[], containers: string[]) => Promise; }) { const [confirming, setConfirming] = useState(false); if (!confirming) { @@ -19,20 +26,55 @@ export function DatabaseReset({ disabled={busy} onClick={() => setConfirming(true)} > - Reset leftover database + {database ? "Reset leftover database" : "Reset leftover data"} ); } + const others = volumes.length > (database ? 1 : 0); return (

Delete the previous installation’s data?

+ {database && ( +

+ This permanently deletes the local OpenBot database, including saved + chats, Bots, and settings. +

+ )} + {others && ( +

+ {database + ? "It also permanently deletes other data it left, including Bot files and browser sign-ins." + : "This permanently deletes data the previous installation left, including Bot files and browser sign-ins."} +

+ )}

- This permanently deletes the local OpenBot database, including saved - chats, Bots, and settings. Only continue if you want a fresh - installation and do not need that data. + Only continue if you want a fresh installation and do not need that + data. OpenBot deletes these volumes:

+
    + {volumes.map((volume) => ( +
  • + {volume} +
    • + ))} +
+ {containers.length > 0 && ( + <> +

+ It first removes these stopped containers from the previous + installation, which still use those volumes: +

+
    + {containers.map((container) => ( +
  • + {container} +
    • + ))} +
+ + )}

To keep your data, restore its original encryption key instead.

-
diff --git a/desktop/src/Problem.tsx b/desktop/src/Problem.tsx index 723f93edee..72a10b1b9b 100644 --- a/desktop/src/Problem.tsx +++ b/desktop/src/Problem.tsx @@ -13,6 +13,10 @@ export type Problem = { detail?: string | null; connection?: "model" | "intelligence" | "organization" | null; database_reset?: string | null; + /** Every volume a confirmed fresh-install reset removes, the database included. */ + leftover_reset?: string[] | null; + /** Stopped containers that still hold those volumes, removed first by the same reset. */ + leftover_containers?: string[] | null; }; /** Anything thrown, as a problem. A bare string keeps working and reads as it always did. */