diff --git a/Cargo.lock b/Cargo.lock index 7150c38664..e7fe6299ca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2118,6 +2118,7 @@ dependencies = [ "utoipa", "uuid", "webpki-roots 0.26.11", + "windows-sys 0.61.2", "winnow 1.0.4", "zip", "zstd", diff --git a/agents/drivers/duckdb/Cargo.lock b/agents/drivers/duckdb/Cargo.lock index eaa8bf015c..b534a02737 100644 --- a/agents/drivers/duckdb/Cargo.lock +++ b/agents/drivers/duckdb/Cargo.lock @@ -1263,6 +1263,7 @@ dependencies = [ "unicode-width", "uuid", "webpki-roots 0.26.11", + "windows-sys 0.61.2", "winnow", "zip 4.6.1", "zstd", diff --git a/apps/desktop/src/components/backup/ScheduledDatabaseBackupSettings.vue b/apps/desktop/src/components/backup/ScheduledDatabaseBackupSettings.vue index 6a41472362..bb14a9dd5b 100644 --- a/apps/desktop/src/components/backup/ScheduledDatabaseBackupSettings.vue +++ b/apps/desktop/src/components/backup/ScheduledDatabaseBackupSettings.vue @@ -203,6 +203,7 @@ async function submitSchedule() { if (!canSave.value) return; saving.value = true; try { + await api.recordDatabaseExportDestination(draft.value.destinationDirectory); saveSchedule({ ...draft.value, databases: allDatabases.value ? [] : [...selectedDatabases.value], diff --git a/apps/desktop/src/components/backup/__tests__/ScheduledDatabaseBackupSettings.spec.ts b/apps/desktop/src/components/backup/__tests__/ScheduledDatabaseBackupSettings.spec.ts index e1a3f2d028..0277895f01 100644 --- a/apps/desktop/src/components/backup/__tests__/ScheduledDatabaseBackupSettings.spec.ts +++ b/apps/desktop/src/components/backup/__tests__/ScheduledDatabaseBackupSettings.spec.ts @@ -11,6 +11,7 @@ const mocks = vi.hoisted(() => ({ schedules: [] as DatabaseBackupSchedule[], ensureConnected: vi.fn(async () => {}), listDatabases: vi.fn(async () => [{ name: "app" }]), + recordDatabaseExportDestination: vi.fn(async (_directory: string) => {}), toast: vi.fn(), saveSchedule: vi.fn(), setScheduleEnabled: vi.fn(), @@ -53,6 +54,7 @@ vi.mock("@/lib/backend/api", () => ({ listDatabases: mocks.listDatabases, deleteDatabaseBackupFiles: vi.fn(), revealPathInFileManager: vi.fn(), + recordDatabaseExportDestination: mocks.recordDatabaseExportDestination, })); const mountedApps: App[] = []; @@ -127,6 +129,8 @@ afterEach(() => { mocks.schedules.splice(0); mocks.ensureConnected.mockClear(); mocks.listDatabases.mockClear(); + mocks.recordDatabaseExportDestination.mockReset(); + mocks.recordDatabaseExportDestination.mockResolvedValue(undefined); mocks.toast.mockClear(); mocks.saveSchedule.mockClear(); }); @@ -197,4 +201,20 @@ describe("ScheduledDatabaseBackupSettings schedule dialog", () => { expect(mocks.saveSchedule).toHaveBeenCalledWith(expect.objectContaining({ databases: ["app", "archive"] })); }); + + it("does not save when the destination identity cannot be recorded", async () => { + mocks.connections.push({ id: "mysql-1", name: "Local MySQL", db_type: "mysql" }); + mocks.schedules.push(schedule()); + mocks.recordDatabaseExportDestination.mockRejectedValueOnce(new Error("backup destination unavailable")); + await mountSettings(); + + buttonWithTitle(String(i18n.global.t("databaseBackup.edit"))).click(); + await flush(); + saveScheduleButton().click(); + await flush(); + + expect(mocks.recordDatabaseExportDestination).toHaveBeenCalledWith("/backups"); + expect(mocks.saveSchedule).not.toHaveBeenCalled(); + expect(mocks.toast).toHaveBeenCalledWith("backup destination unavailable", 5000); + }); }); diff --git a/apps/desktop/src/lib/backend/api.ts b/apps/desktop/src/lib/backend/api.ts index b09f97af3e..e8394cd731 100644 --- a/apps/desktop/src/lib/backend/api.ts +++ b/apps/desktop/src/lib/backend/api.ts @@ -416,6 +416,7 @@ export const releaseTableImportSource = forward("releaseTableImportSource"); export const beginDatabaseBackupSnapshot = forward("beginDatabaseBackupSnapshot"); export const exportDatabaseSql = forward("exportDatabaseSql"); export const cancelDatabaseExport = forward("cancelDatabaseExport"); +export const recordDatabaseExportDestination = forward("recordDatabaseExportDestination"); export const exportQueryResultCsv = forward("exportQueryResultCsv"); export const exportTableDataCsv = forward("exportTableDataCsv"); export const exportQueryResultXlsx = forward("exportQueryResultXlsx"); diff --git a/apps/desktop/src/lib/backend/http.ts b/apps/desktop/src/lib/backend/http.ts index ee66e5271d..ea00b2df06 100644 --- a/apps/desktop/src/lib/backend/http.ts +++ b/apps/desktop/src/lib/backend/http.ts @@ -2224,6 +2224,10 @@ export async function cancelDatabaseExport(exportId: string): Promise { await post("/api/export/database/cancel", { exportId }); } +export async function recordDatabaseExportDestination(_directory: string): Promise { + throw new Error("Scheduled database backups are only available in the desktop app."); +} + // --- Table Export --- export async function startTableExport(request: TableExportRequest, onProgress: (progress: TableExportProgress) => void): Promise { diff --git a/apps/desktop/src/lib/backend/tauri.ts b/apps/desktop/src/lib/backend/tauri.ts index 0ad63b2943..6f566d6d81 100644 --- a/apps/desktop/src/lib/backend/tauri.ts +++ b/apps/desktop/src/lib/backend/tauri.ts @@ -4405,6 +4405,10 @@ export async function cancelDatabaseExport(exportId: string): Promise { await invoke("cancel_database_export", { exportId }); } +export async function recordDatabaseExportDestination(directory: string): Promise { + await invoke("record_database_export_destination", { directory }); +} + export async function exportQueryResultCsv(filePath: string, columns: string[], rows: readonly (readonly XlsxCellValue[])[]): Promise { return invoke("export_query_result_csv", { request: { diff --git a/crates/dbx-core/Cargo.toml b/crates/dbx-core/Cargo.toml index 94b3a5807a..93b8e7b152 100644 --- a/crates/dbx-core/Cargo.toml +++ b/crates/dbx-core/Cargo.toml @@ -85,5 +85,8 @@ tempfile = "3" [target.'cfg(not(all(windows, target_vendor = "win7")))'.dependencies] iana-time-zone = "0.1" +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.61", features = ["Win32_Storage_FileSystem", "Win32_Foundation", "Win32_Security"] } + [dev-dependencies] mysql_common = { git = "https://github.com/t8y2/rust_mysql_common.git", rev = "47740d85a8277167a5717afac785d37b46a36296", default-features = false } diff --git a/crates/dbx-core/src/database_export.rs b/crates/dbx-core/src/database_export.rs index 6dc2426c33..f7fcba5830 100644 --- a/crates/dbx-core/src/database_export.rs +++ b/crates/dbx-core/src/database_export.rs @@ -1630,6 +1630,195 @@ fn emit_database_export_running( }); } +fn export_destination_state_key(dir: &std::path::Path) -> String { + format!("database_export_destination:{}", dir.to_string_lossy()) +} + +/// Records the destination identity for a scheduled backup as soon as it is +/// configured, not just after its first successful export. Scheduled plans +/// live in the frontend and may not run for hours after being saved; without +/// an eager record here, a destination whose mount disappears before its +/// very first run is indistinguishable from a brand-new local folder to +/// `ensure_export_destination_dir` (both have no recorded state) and gets +/// silently recreated on the wrong filesystem. The schedule editor only +/// accepts directories selected from the filesystem, so a missing path here +/// means the destination vanished before its identity could be recorded and +/// the schedule must not be saved. See #6327. +pub async fn record_export_destination_identity( + state: &crate::connection::AppState, + dir: &std::path::Path, +) -> Result<(), String> { + if !dir.is_dir() { + return Err(format!( + "Backup directory {} does not exist or is not a directory. Select an existing destination before saving the schedule.", + dir.display() + )); + } + save_export_destination_dir_device_id(state, dir).await +} + +async fn save_export_destination_dir_device_id( + state: &crate::connection::AppState, + dir: &std::path::Path, +) -> Result<(), String> { + let value = match export_destination_device_id_for_path(dir) { + Some(dev) => dev.to_le_bytes().to_vec(), + None => Vec::new(), + }; + state.storage.save_state(&export_destination_state_key(dir), &value, "application/octet-stream").await +} + +/// Ensures `dir` exists for an export destination, without ever silently +/// recreating a directory that previously produced a successful export (or +/// was recorded via [`record_export_destination_identity`]) and has since +/// disappeared. Auto-creating on every run is what the original fix for +/// #6109 did, but that is unsafe for a destination on a removable or network +/// drive: if the mount is temporarily gone when a run executes, blindly +/// recreating the path resurrects it on the local root filesystem and the +/// export "succeeds" while silently writing to the wrong disk. A directory +/// dbx has never seen before is safe to create (normal first-time +/// configuration of a local folder); a directory dbx has seen before but +/// that is now missing, or that now resolves to a different filesystem than +/// last time, is refused instead. See #6327. +/// +/// Returns the device identity that was just verified (or recorded for a +/// newly created directory), if the current platform can determine one, so +/// the caller can re-verify it against the file it actually opens -- the +/// directory check here and the later `File::create` are separate +/// operations, and the mount can change in between. +async fn ensure_export_destination_dir( + state: &crate::connection::AppState, + dir: &std::path::Path, +) -> Result, String> { + let key = export_destination_state_key(dir); + let recorded_dev: Option> = state + .storage + .load_state(&key) + .await? + .map(|(bytes, _content_type)| <[u8; 8]>::try_from(bytes.as_slice()).ok().map(u64::from_le_bytes)); + + if dir.is_dir() { + if let Some(Some(recorded_dev)) = recorded_dev { + if let Some(current_dev) = export_destination_device_id_for_path(dir) { + if current_dev != recorded_dev { + return Err(format!( + "Backup directory {} now resolves to a different filesystem than the last \ + successful export to this location. Refusing to write here automatically -- \ + if this directory is on a removable or network drive, make sure the correct \ + drive is connected before running the backup.", + dir.display() + )); + } + } + } + } else { + if recorded_dev.is_some() { + return Err(format!( + "Backup directory {} is missing. It was configured or previously used for exports to \ + this location, so dbx will not recreate it automatically -- if this is on a removable \ + or network drive, reconnect it and try again.", + dir.display() + )); + } + std::fs::create_dir_all(dir).map_err(|e| format!("Failed to create backup directory: {e}"))?; + } + + save_export_destination_dir_device_id(state, dir).await?; + Ok(export_destination_device_id_for_path(dir)) +} + +/// Whether a destination's identity, checked once via [`ensure_export_destination_dir`] +/// and then again against the file dbx actually opened, indicates the mount +/// changed in between. An unknown expected identity has nothing to compare, +/// but once an expected identity is known, failing to identify the opened +/// handle must fail closed rather than allowing an unverified write. +fn export_destination_identity_mismatch(expected: Option, actual: Option) -> bool { + expected.is_some_and(|expected| actual != Some(expected)) +} + +#[cfg(unix)] +fn export_destination_device_id_for_path(dir: &std::path::Path) -> Option { + use std::os::unix::fs::MetadataExt; + std::fs::metadata(dir).ok().map(|metadata| metadata.dev()) +} + +#[cfg(unix)] +fn export_destination_device_id_for_file(file: &std::fs::File) -> Option { + use std::os::unix::fs::MetadataExt; + file.metadata().ok().map(|metadata| metadata.dev()) +} + +#[cfg(windows)] +fn export_destination_device_id_for_path(dir: &std::path::Path) -> Option { + windows_export_destination::device_id_for_path(dir) +} + +#[cfg(windows)] +fn export_destination_device_id_for_file(file: &std::fs::File) -> Option { + windows_export_destination::device_id_for_handle(file) +} + +#[cfg(not(any(unix, windows)))] +fn export_destination_device_id_for_path(_dir: &std::path::Path) -> Option { + None +} + +#[cfg(not(any(unix, windows)))] +fn export_destination_device_id_for_file(_file: &std::fs::File) -> Option { + None +} + +/// Windows has no stable `std` API for a directory or file's volume identity +/// (`MetadataExt::volume_serial_number` is still gated behind the unstable +/// `windows_by_handle` feature), so this queries `dwVolumeSerialNumber` from +/// `BY_HANDLE_FILE_INFORMATION` directly. Using the *handle* rather than +/// re-resolving the path is what makes `export_destination_device_id_for_file` +/// safe to call on an already-open `File`: it reports the volume the handle +/// was actually opened against, not whatever currently sits at that path. +#[cfg(windows)] +mod windows_export_destination { + use std::os::windows::ffi::OsStrExt; + use std::os::windows::io::AsRawHandle; + use std::path::Path; + use windows_sys::Win32::Foundation::{CloseHandle, HANDLE, INVALID_HANDLE_VALUE}; + use windows_sys::Win32::Storage::FileSystem::{ + CreateFileW, GetFileInformationByHandle, BY_HANDLE_FILE_INFORMATION, FILE_FLAG_BACKUP_SEMANTICS, + FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_EXISTING, + }; + + pub(super) fn device_id_for_path(path: &Path) -> Option { + let mut wide: Vec = path.as_os_str().encode_wide().collect(); + wide.push(0); + let handle = unsafe { + CreateFileW( + wide.as_ptr(), + 0, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + std::ptr::null(), + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, + std::ptr::null_mut(), + ) + }; + if handle == INVALID_HANDLE_VALUE { + return None; + } + let result = volume_serial_number(handle); + unsafe { CloseHandle(handle) }; + result + } + + pub(super) fn device_id_for_handle(file: &std::fs::File) -> Option { + volume_serial_number(file.as_raw_handle() as HANDLE) + } + + fn volume_serial_number(handle: HANDLE) -> Option { + let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { std::mem::zeroed() }; + let ok = unsafe { GetFileInformationByHandle(handle, &mut info) }; + (ok != 0).then_some(info.dwVolumeSerialNumber as u64) + } +} + pub async fn export_database_sql_core( state: &crate::connection::AppState, request: &DatabaseExportRequest, @@ -1673,7 +1862,28 @@ pub async fn export_database_sql_core( ) .await?; // 4. Create file + let mut expected_destination_dev = None; + if let Some(parent) = std::path::Path::new(&request.file_path).parent() { + if !parent.as_os_str().is_empty() { + expected_destination_dev = ensure_export_destination_dir(state, parent).await?; + } + } let file = std::fs::File::create(&request.file_path).map_err(|e| format!("Failed to write file: {e}"))?; + // The directory check above and this `File::create` are separate + // operations: the mount can disappear and be replaced by something else + // at the same path in between. Re-check the identity of the handle we + // actually opened, not just the path, and refuse to keep a backup that + // landed on the wrong filesystem. See #6327. + if export_destination_identity_mismatch(expected_destination_dev, export_destination_device_id_for_file(&file)) { + drop(file); + let _ = std::fs::remove_file(&request.file_path); + return Err(format!( + "Backup destination for {} changed while opening the output file -- the directory now \ + resolves to a different filesystem than the one just verified. If a removable or network \ + drive was disconnected and reconnected, retry the backup.", + request.file_path + )); + } let mut file = BufWriter::new(file); let create_database_preamble = if request.include_create_database && matches!(db_type, DatabaseType::Mysql) { @@ -2519,19 +2729,22 @@ fn build_database_export_object_source_sql( mod tests { use super::{ build_database_export_object_source_sql, build_database_sql_export, build_export_insert_statements, - database_export_select_sql, database_export_total_objects, drop_table_if_exists_sql, filter_export_table_infos, + database_export_select_sql, database_export_total_objects, drop_table_if_exists_sql, + ensure_export_destination_dir, export_destination_identity_mismatch, filter_export_table_infos, format_export_sql_literal, format_export_table_ddl, format_mysql_spatial_export_literal, generate_postgres_extension_ddl, generate_postgres_sequence_create_ddl, generate_postgres_sequence_owner_ddl, generate_postgres_sequence_setval_sql, is_postgres_extension_member_routine, mysql_database_export_preamble, mysql_view_dependencies_from_rows, mysql_view_dependencies_sql, normalize_export_table_ddl, - record_export_error, replace_database_export_select_list, sort_export_views_by_dependencies, - write_database_export_rows, BuildDatabaseSqlExportOptions, BuildExportInsertStatementsOptions, - DatabaseExportObjectCounts, DatabaseExportRequest, DdlNormalizeOptions, ExportedTableSql, - PostgresExportExtension, PostgresExportSequence, PostgresExtensionMembers, DATABASE_EXPORT_INSERT_BATCH_SIZE, - DATABASE_EXPORT_ROW_LIMIT, + record_export_destination_identity, record_export_error, replace_database_export_select_list, + sort_export_views_by_dependencies, write_database_export_rows, BuildDatabaseSqlExportOptions, + BuildExportInsertStatementsOptions, DatabaseExportObjectCounts, DatabaseExportRequest, DdlNormalizeOptions, + ExportedTableSql, PostgresExportExtension, PostgresExportSequence, PostgresExtensionMembers, + DATABASE_EXPORT_INSERT_BATCH_SIZE, DATABASE_EXPORT_ROW_LIMIT, }; use super::{concurrent_metadata_prefetch_allowed, database_export_metadata_prefetch_concurrency}; + use crate::connection::AppState; use crate::models::connection::DatabaseType; + use crate::storage::Storage; use crate::types::{ObjectInfo, ObjectSourceKind, TableInfo}; use serde_json::{json, Value}; @@ -3926,4 +4139,161 @@ mod tests { assert!(std::fs::read_to_string(&path).unwrap().is_empty()); let _ = std::fs::remove_file(path); } + + async fn test_app_state(scratch_dir: &std::path::Path) -> AppState { + let storage = Storage::open(&scratch_dir.join("storage.db")).await.unwrap(); + AppState::new(storage) + } + + // Regression tests for #6327: create_dir_all on every export run is unsafe + // when the destination is on a removable/network drive, because a mount + // that is temporarily gone at write time would be silently recreated on + // the local root filesystem, and the backup would "succeed" while writing + // to the wrong disk. + + #[tokio::test] + async fn ensure_export_destination_dir_creates_a_never_before_seen_local_directory() { + let scratch = std::env::temp_dir().join(format!("dbx-export-dest-new-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&scratch).unwrap(); + let state = test_app_state(&scratch).await; + + let destination = scratch.join("backups").join("mydb"); + assert!(!destination.exists()); + + ensure_export_destination_dir(&state, &destination) + .await + .expect("first-time local directory should be created"); + assert!(destination.is_dir()); + + let _ = std::fs::remove_dir_all(&scratch); + } + + #[tokio::test] + async fn ensure_export_destination_dir_refuses_to_recreate_a_destination_that_disappeared() { + let scratch = std::env::temp_dir().join(format!("dbx-export-dest-vanished-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&scratch).unwrap(); + let state = test_app_state(&scratch).await; + + // Simulates a previously configured, already-used backup destination + // (e.g. on an external or network drive) by successfully exporting to + // it once first. + let destination = scratch.join("mounted-drive").join("mydb"); + ensure_export_destination_dir(&state, &destination).await.expect("initial export should create the directory"); + assert!(destination.is_dir()); + + // Simulates the mount disappearing (unplugged drive, unmounted share, + // etc.) before the next run. + std::fs::remove_dir_all(scratch.join("mounted-drive")).unwrap(); + assert!(!destination.exists()); + + let result = ensure_export_destination_dir(&state, &destination).await; + + assert!(result.is_err(), "a destination that existed before should not be silently recreated"); + assert!(!destination.exists(), "the backup directory must not be resurrected on the wrong filesystem"); + + let _ = std::fs::remove_dir_all(&scratch); + } + + // Regression test for review feedback on #6327: `ensure_export_destination_dir` + // only remembered a destination *after* a successful export, so a mount + // that was present when a schedule was configured but vanished before its + // very first run looked identical to a brand-new local folder (no + // recorded state either way) and got silently recreated on the local + // disk. `record_export_destination_identity` closes that gap by letting + // the schedule-configuration flow record the destination eagerly. + #[tokio::test] + async fn record_export_destination_identity_protects_a_mount_that_vanishes_before_its_first_run() { + let scratch = std::env::temp_dir().join(format!("dbx-export-dest-eager-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&scratch).unwrap(); + let state = test_app_state(&scratch).await; + + // Simulates the user picking an already-mounted external/network + // drive in the schedule editor and saving the schedule -- the + // directory exists at configuration time, but no export has run yet. + let destination = scratch.join("mounted-drive").join("mydb"); + std::fs::create_dir_all(&destination).unwrap(); + record_export_destination_identity(&state, &destination) + .await + .expect("configuring the schedule should succeed"); + + // The mount disappears before the scheduler ever runs this schedule + // for the first time. + std::fs::remove_dir_all(scratch.join("mounted-drive")).unwrap(); + assert!(!destination.exists()); + + let result = ensure_export_destination_dir(&state, &destination).await; + + assert!(result.is_err(), "a mount that was recorded at configuration time must not be silently recreated"); + assert!(!destination.exists(), "the backup directory must not be resurrected on the wrong filesystem"); + + let _ = std::fs::remove_dir_all(&scratch); + } + + #[tokio::test] + async fn record_export_destination_identity_rejects_a_directory_that_disappeared_before_save() { + let scratch = std::env::temp_dir().join(format!("dbx-export-dest-eager-new-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&scratch).unwrap(); + let state = test_app_state(&scratch).await; + + // The schedule editor only accepts an existing directory. If that + // directory disappears before the save request reaches the backend, + // the schedule must not be persisted without a recorded identity. + let destination = scratch.join("backups").join("mydb"); + assert!(!destination.exists()); + let error = record_export_destination_identity(&state, &destination) + .await + .expect_err("a missing scheduled destination must be rejected"); + + assert!(error.contains("does not exist or is not a directory")); + assert!(!destination.exists()); + + let _ = std::fs::remove_dir_all(&scratch); + } + + // Regression test for review feedback on #6327: the directory identity + // check and the later `File::create` are separate operations, so the + // mount can disappear and be replaced by something else at the same path + // in between. `export_destination_identity_mismatch` is the comparison + // `export_database_sql_core` runs against the handle it actually opened; + // exercised directly here since reproducing a real cross-filesystem swap + // mid-write is not something a portable unit test can simulate. + #[test] + fn export_destination_identity_mismatch_detects_a_changed_device() { + assert!(export_destination_identity_mismatch(Some(1), Some(2))); + assert!(!export_destination_identity_mismatch(Some(1), Some(1))); + assert!( + !export_destination_identity_mismatch(None, Some(2)), + "an unknown expected device has nothing to compare against" + ); + assert!( + export_destination_identity_mismatch(Some(1), None), + "an opened file with unknown identity must not bypass a known expected device" + ); + assert!(!export_destination_identity_mismatch(None, None)); + } + + // Regression test for review feedback on #6327: the non-Unix path used + // to report no device identity at all, so replacing a Windows drive or + // mount at the same path went undetected. This only runs on native + // Windows (this repo's CI has no Windows job that executes `cargo test`, + // only `cargo check`, so it is exercised locally by Windows contributors + // and by the compile-check itself). + #[cfg(windows)] + #[test] + fn export_destination_device_id_for_path_and_open_file_agree_on_windows() { + let scratch = std::env::temp_dir().join(format!("dbx-export-dest-win-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&scratch).unwrap(); + + let dir_dev = super::export_destination_device_id_for_path(&scratch); + assert!(dir_dev.is_some(), "a real local directory should report a volume serial number"); + + let file_path = scratch.join("probe.txt"); + let file = std::fs::File::create(&file_path).unwrap(); + let file_dev = super::export_destination_device_id_for_file(&file); + drop(file); + + assert_eq!(dir_dev, file_dev, "a file and its parent directory must resolve to the same volume"); + + let _ = std::fs::remove_dir_all(&scratch); + } } diff --git a/crates/dbx-core/tests/live_mysql_database_export.rs b/crates/dbx-core/tests/live_mysql_database_export.rs index a4a971de0b..4084f95ed6 100644 --- a/crates/dbx-core/tests/live_mysql_database_export.rs +++ b/crates/dbx-core/tests/live_mysql_database_export.rs @@ -1,5 +1,5 @@ use dbx_core::connection::AppState; -use dbx_core::database_export::{export_database_sql_core, DatabaseExportRequest}; +use dbx_core::database_export::{export_database_sql_core, record_export_destination_identity, DatabaseExportRequest}; use dbx_core::models::connection::{ConnectionConfig, DatabaseType}; use dbx_core::query::execute_sql_statement; use dbx_core::sql::SqlFileRequest; @@ -133,3 +133,229 @@ async fn live_mysql_database_export_restores_dependent_views() { assert_eq!(result.rows, vec![vec![serde_json::json!("7")]]); assert_eq!(spatial_result.rows, vec![vec![serde_json::json!(4326), serde_json::json!(1)]]); } + +/// Regression test for #6109 ("backup always errors"): the very first export +/// to a destination directory that has never been configured or used before +/// (a normal, not-yet-created local folder -- e.g. the user just typed a new +/// path into the schedule editor and saved it) must create it rather than +/// failing on every run with a raw std::fs::File::create OS error. +/// +/// This must NOT be confused with a destination directory that previously +/// existed -- either because it produced a successful export before, or +/// because it was recorded at configuration time via +/// `record_export_destination_identity` -- and has since disappeared (e.g. +/// an unmounted external/network drive). See +/// `live_mysql_database_export_refuses_to_recreate_a_destination_that_disappeared` +/// and `live_mysql_database_export_refuses_a_destination_that_vanished_before_its_first_run` +/// below, and the #6327 discussion, for why those cases are refused instead. +#[tokio::test] +#[ignore = "requires a disposable MySQL endpoint"] +async fn live_mysql_database_export_creates_missing_destination_directory() { + let suffix = uuid::Uuid::new_v4().simple().to_string(); + let connection_id = format!("live-mysql-export-missing-dir-{suffix}"); + let database = format!("dbx_export_missing_dir_{suffix}"); + let dir = std::env::temp_dir().join(format!("dbx-live-mysql-export-missing-dir-{suffix}")); + std::fs::create_dir_all(&dir).unwrap(); + let storage = Storage::open(&dir.join("storage.db")).await.unwrap(); + let state = AppState::new(storage); + state.configs.write().await.insert(connection_id.clone(), live_mysql_config(&connection_id)); + + for sql in [ + format!("DROP DATABASE IF EXISTS `{database}`"), + format!("CREATE DATABASE `{database}`"), + format!("CREATE TABLE `{database}`.`widgets` (id INT PRIMARY KEY, name VARCHAR(50))"), + format!("INSERT INTO `{database}`.`widgets` VALUES (1, 'alpha')"), + ] { + execute_sql_statement(&state, &connection_id, "", &sql, None, None).await.unwrap(); + } + + // Destination directory deliberately never created and never recorded -- + // simulates a brand-new local folder the user just typed into the + // schedule editor, not a mount that used to exist. + let missing_destination_dir = dir.join("brand-new-local-folder"); + let file_path = missing_destination_dir.join("export.sql"); + let export_request = DatabaseExportRequest { + export_id: format!("live-mysql-export-missing-dir-{suffix}"), + connection_id: connection_id.clone(), + database: database.clone(), + schema: database.clone(), + file_path: file_path.to_string_lossy().to_string(), + selected_tables: Vec::new(), + excluded_tables: Vec::new(), + include_structure: true, + include_data: true, + include_objects: true, + include_create_database: true, + drop_table_if_exists: true, + omit_auto_increment: false, + fail_on_error: true, + snapshot_session_id: None, + batch_size: 1000, + }; + + let result = export_database_sql_core(&state, &export_request, |_| {}).await; + + execute_sql_statement(&state, &connection_id, "", &format!("DROP DATABASE `{database}`"), None, None) + .await + .unwrap(); + + result.expect("export should succeed even when the destination directory is missing"); + assert!(missing_destination_dir.exists(), "destination directory should have been auto-created"); + let exported = std::fs::read_to_string(&file_path).unwrap(); + assert!(exported.contains("'alpha'"), "exported SQL should contain the seeded row"); + + std::fs::remove_dir_all(dir).unwrap(); +} + +/// Regression test for the #6327 review of the #6109 fix: once a destination +/// directory has produced a successful export, its later disappearance (e.g. +/// an external/network drive that got unmounted between scheduled runs) must +/// make the next export fail with a clear error instead of silently +/// recreating the directory -- which, for a vanished mount, would resurrect +/// it on the local root filesystem and write the backup to the wrong disk. +#[tokio::test] +#[ignore = "requires a disposable MySQL endpoint"] +async fn live_mysql_database_export_refuses_to_recreate_a_destination_that_disappeared() { + let suffix = uuid::Uuid::new_v4().simple().to_string(); + let connection_id = format!("live-mysql-export-vanished-dir-{suffix}"); + let database = format!("dbx_export_vanished_dir_{suffix}"); + let dir = std::env::temp_dir().join(format!("dbx-live-mysql-export-vanished-dir-{suffix}")); + std::fs::create_dir_all(&dir).unwrap(); + let storage = Storage::open(&dir.join("storage.db")).await.unwrap(); + let state = AppState::new(storage); + state.configs.write().await.insert(connection_id.clone(), live_mysql_config(&connection_id)); + + for sql in [ + format!("DROP DATABASE IF EXISTS `{database}`"), + format!("CREATE DATABASE `{database}`"), + format!("CREATE TABLE `{database}`.`widgets` (id INT PRIMARY KEY, name VARCHAR(50))"), + format!("INSERT INTO `{database}`.`widgets` VALUES (1, 'alpha')"), + ] { + execute_sql_statement(&state, &connection_id, "", &sql, None, None).await.unwrap(); + } + + // Simulates a backup destination on an external/network drive that is + // currently connected and gets used successfully once. + let destination_dir = dir.join("mounted-drive-destination"); + let file_path = destination_dir.join("export.sql"); + let export_request = DatabaseExportRequest { + export_id: format!("live-mysql-export-vanished-dir-{suffix}"), + connection_id: connection_id.clone(), + database: database.clone(), + schema: database.clone(), + file_path: file_path.to_string_lossy().to_string(), + selected_tables: Vec::new(), + excluded_tables: Vec::new(), + include_structure: true, + include_data: true, + include_objects: true, + include_create_database: true, + drop_table_if_exists: true, + omit_auto_increment: false, + fail_on_error: true, + snapshot_session_id: None, + batch_size: 1000, + }; + + export_database_sql_core(&state, &export_request, |_| {}).await.expect("first export should succeed"); + assert!(destination_dir.exists()); + + // Simulates the drive being disconnected/unmounted before the next run: + // the whole destination directory is gone, not just emptied. + std::fs::remove_dir_all(&destination_dir).unwrap(); + assert!(!destination_dir.exists()); + + let result = export_database_sql_core(&state, &export_request, |_| {}).await; + + execute_sql_statement(&state, &connection_id, "", &format!("DROP DATABASE `{database}`"), None, None) + .await + .unwrap(); + + assert!(result.is_err(), "export must not silently recreate a destination that previously existed"); + assert!(!destination_dir.exists(), "the backup directory must not be resurrected on the wrong filesystem"); + + std::fs::remove_dir_all(dir).unwrap(); +} + +/// Regression test for review feedback on #6327: scheduled plans live in the +/// frontend and may not run for hours after being saved. Recording a +/// destination's identity only after a *successful* export (as in +/// `live_mysql_database_export_refuses_to_recreate_a_destination_that_disappeared` +/// above) means a mount that was connected when the schedule was configured, +/// but disappears before its very first scheduled run, is indistinguishable +/// from a brand-new local folder and gets silently recreated on the wrong +/// filesystem. `record_export_destination_identity` -- called from the +/// schedule editor when the schedule is saved, before any export ever runs +/// -- closes that gap. This exercises the same "recorded, then vanished" +/// scenario through the public export API end-to-end, but with the identity +/// recorded eagerly (simulating schedule configuration) instead of via a +/// prior successful run. +#[tokio::test] +#[ignore = "requires a disposable MySQL endpoint"] +async fn live_mysql_database_export_refuses_a_destination_that_vanished_before_its_first_run() { + let suffix = uuid::Uuid::new_v4().simple().to_string(); + let connection_id = format!("live-mysql-export-preconfigured-vanished-dir-{suffix}"); + // MySQL identifiers are capped at 64 characters -- keep this well under + // that even with the 32-character uuid suffix. + let database = format!("dbx_export_precfg_vanished_{suffix}"); + let dir = std::env::temp_dir().join(format!("dbx-live-mysql-export-preconfigured-vanished-dir-{suffix}")); + std::fs::create_dir_all(&dir).unwrap(); + let storage = Storage::open(&dir.join("storage.db")).await.unwrap(); + let state = AppState::new(storage); + state.configs.write().await.insert(connection_id.clone(), live_mysql_config(&connection_id)); + + for sql in [ + format!("DROP DATABASE IF EXISTS `{database}`"), + format!("CREATE DATABASE `{database}`"), + format!("CREATE TABLE `{database}`.`widgets` (id INT PRIMARY KEY, name VARCHAR(50))"), + format!("INSERT INTO `{database}`.`widgets` VALUES (1, 'alpha')"), + ] { + execute_sql_statement(&state, &connection_id, "", &sql, None, None).await.unwrap(); + } + + // Simulates the user picking an already-mounted external/network drive + // in the schedule editor and saving the schedule. The directory exists + // at configuration time, but no export has run against it yet. + let destination_dir = dir.join("preconfigured-mounted-drive-destination"); + let file_path = destination_dir.join("export.sql"); + std::fs::create_dir_all(&destination_dir).unwrap(); + record_export_destination_identity(&state, &destination_dir) + .await + .expect("configuring the schedule should succeed"); + + let export_request = DatabaseExportRequest { + export_id: format!("live-mysql-export-preconfigured-vanished-dir-{suffix}"), + connection_id: connection_id.clone(), + database: database.clone(), + schema: database.clone(), + file_path: file_path.to_string_lossy().to_string(), + selected_tables: Vec::new(), + excluded_tables: Vec::new(), + include_structure: true, + include_data: true, + include_objects: true, + include_create_database: true, + drop_table_if_exists: true, + omit_auto_increment: false, + fail_on_error: true, + snapshot_session_id: None, + batch_size: 1000, + }; + + // The mount disappears before the scheduler ever runs this schedule for + // the first time. + std::fs::remove_dir_all(&destination_dir).unwrap(); + assert!(!destination_dir.exists()); + + let result = export_database_sql_core(&state, &export_request, |_| {}).await; + + execute_sql_statement(&state, &connection_id, "", &format!("DROP DATABASE `{database}`"), None, None) + .await + .unwrap(); + + assert!(result.is_err(), "a mount that was recorded at configuration time must not be silently recreated"); + assert!(!destination_dir.exists(), "the backup directory must not be resurrected on the wrong filesystem"); + assert!(!file_path.exists(), "no backup should have been written to the resurrected directory"); + + std::fs::remove_dir_all(dir).unwrap(); +} diff --git a/src-tauri/src/commands/database_export.rs b/src-tauri/src/commands/database_export.rs index 8149e3079e..2f11097497 100644 --- a/src-tauri/src/commands/database_export.rs +++ b/src-tauri/src/commands/database_export.rs @@ -65,3 +65,16 @@ pub async fn cancel_database_export(export_id: String) -> Result<(), String> { dbx_core::database_export::set_export_cancelled(&export_id).await; Ok(()) } + +/// Records a scheduled backup destination's filesystem identity as soon as +/// the schedule is saved, not just after its first successful export. See +/// `record_export_destination_identity` for why this eager recording is +/// needed. Called from the schedule editor when a schedule is created or +/// edited (`apps/desktop/src/components/backup/ScheduledDatabaseBackupSettings.vue`). +#[tauri::command] +pub async fn record_database_export_destination( + state: State<'_, Arc>, + directory: String, +) -> Result<(), String> { + dbx_core::database_export::record_export_destination_identity(&state, std::path::Path::new(&directory)).await +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 6ffae8e65d..ebc01ef291 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -2233,6 +2233,7 @@ pub fn run() { commands::database_export::begin_database_backup_snapshot, commands::database_export::export_database_sql, commands::database_export::cancel_database_export, + commands::database_export::record_database_export_destination, commands::table_export::start_table_export, commands::table_export::cancel_table_export, commands::query_result_export::start_query_result_export,