From 7e9b276685245d2137c40b51b3e65a3dd14d75c2 Mon Sep 17 00:00:00 2001 From: Charles Gauthereau Date: Fri, 9 Oct 2026 17:38:32 +0200 Subject: [PATCH] fix(mssql): keep the target usable when the restore drop fails --- src/domain/mssql/backup.rs | 4 +- src/domain/mssql/restore.rs | 21 +++++++-- src/tests/domain/mssql.rs | 86 +++++++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 6 deletions(-) diff --git a/src/domain/mssql/backup.rs b/src/domain/mssql/backup.rs index 0c6c90f..ccef82e 100644 --- a/src/domain/mssql/backup.rs +++ b/src/domain/mssql/backup.rs @@ -14,13 +14,13 @@ pub async fn run( logger: Arc, ) -> Result { tokio::task::spawn_blocking(move || -> Result { - logger.log("info", format!("Starting MSSQL backup for database {}", cfg.name)); - let (format, unknown_format) = MssqlFormat::from_config(&cfg); if let Some(v) = unknown_format { logger.log("warn", format!("Unknown backup_format '{}' for {}, falling back to 'dacpac'", v, cfg.name)); } + logger.log("info", format!("Starting MSSQL {} backup for database {}", &format.extension()[1..], cfg.name)); + let file_path = backup_dir.join(format!("{}{}", cfg.generated_id, file_extension)); let connection_string = format!( "Server=tcp:{},{};Database={};User Id={};Password={};TrustServerCertificate=True;Encrypt=False", diff --git a/src/domain/mssql/restore.rs b/src/domain/mssql/restore.rs index 21ea161..1568c39 100644 --- a/src/domain/mssql/restore.rs +++ b/src/domain/mssql/restore.rs @@ -10,11 +10,24 @@ use std::time::Instant; const DROP_DATABASE_SQL: &str = "DECLARE @q nvarchar(258) = QUOTENAME(@P1); IF DB_ID(@P1) IS NOT NULL BEGIN - IF HAS_PERMS_BY_NAME(NULL, 'DATABASE', 'CREATE DATABASE') = 0 + IF DB_ID(@P1) <= 4 + THROW 50000, N'Refusing to restore over a system database; nothing was dropped', 1; + IF ISNULL(HAS_PERMS_BY_NAME(NULL, 'DATABASE', 'CREATE DATABASE'), 0) = 0 THROW 50000, N'Restore needs CREATE DATABASE permission (dbcreator) to recreate the target database; nothing was dropped', 1; + DECLARE @single bit = 0; IF CAST(SERVERPROPERTY('EngineEdition') AS int) <> 5 + BEGIN EXEC(N'ALTER DATABASE ' + @q + N' SET SINGLE_USER WITH ROLLBACK IMMEDIATE'); - EXEC(N'DROP DATABASE ' + @q); + SET @single = 1; + END + BEGIN TRY + EXEC(N'DROP DATABASE ' + @q); + END TRY + BEGIN CATCH + IF @single = 1 AND DB_ID(@P1) IS NOT NULL + EXEC(N'ALTER DATABASE ' + @q + N' SET MULTI_USER'); + THROW; + END CATCH END"; pub async fn run(cfg: DatabaseConfig, restore_file: PathBuf, logger: Arc) -> Result<()> { @@ -80,7 +93,7 @@ pub async fn run(cfg: DatabaseConfig, restore_file: PathBuf, logger: Arc Result<()> { - logger.log("info", format!("Dropping MSSQL database {} before restore", cfg.database)); + logger.log("info", format!("Replacing MSSQL database {} (drop if it exists, then recreate from the backup)", cfg.database)); let start = Instant::now(); let mut client = super::connection::build_client(cfg, Some("master")) @@ -92,6 +105,6 @@ async fn drop_database(cfg: &DatabaseConfig, logger: &JobLogger) -> Result<()> { .with_context(|| format!("Failed to drop MSSQL database {}", cfg.database))?; let duration_ms = start.elapsed().as_millis() as f64; - logger.log_command(format!("DROP DATABASE [{}]", cfg.database), None, Some(0), Some(duration_ms)); + logger.log_command(format!("DROP DATABASE IF EXISTS [{}]", cfg.database), None, Some(0), Some(duration_ms)); Ok(()) } diff --git a/src/tests/domain/mssql.rs b/src/tests/domain/mssql.rs index 68db97c..4aa6f6a 100644 --- a/src/tests/domain/mssql.rs +++ b/src/tests/domain/mssql.rs @@ -373,6 +373,92 @@ async fn mssql_restore_refuses_without_create_database_test() { assert_eq!(scalar(&host, port, "ownerdb", "sa", SA_PASSWORD, "SELECT COUNT(*) FROM dbo.users").await, 3); } +#[tokio::test] +async fn mssql_restore_failed_drop_keeps_multi_user_test() { + init_tracing_for_test(); + let (_container, host, port) = start().await; + create_user_database(&host, port, "alterdb").await; + exec( + &host, + port, + "master", + "CREATE LOGIN alter_login WITH PASSWORD = 'Alter!Str0ng1'; GRANT CREATE ANY DATABASE TO alter_login;", + ) + .await; + exec( + &host, + port, + "alterdb", + "CREATE TABLE dbo.t (id INT PRIMARY KEY); INSERT dbo.t VALUES (1); \ + CREATE USER alter_user FOR LOGIN alter_login; GRANT ALTER TO alter_user;", + ) + .await; + + let temp_dir = TempDir::new().unwrap(); + let file_path = temp_dir.path().join("backup.dacpac"); + std::fs::write(&file_path, b"not a package").unwrap(); + + let mut config = make_config(host.clone(), port, "alterdb", "5a445eb4-c2c6-4bde-a423-ee1385dcf6dc"); + config.username = "alter_login".to_string(); + config.password = "Alter!Str0ng1".to_string(); + let result = DatabaseFactory::create_for_restore(config, &file_path) + .await + .restore(&file_path, Arc::new(JobLogger::new())) + .await; + + assert!(!format!("{:#}", result.unwrap_err()).contains("nothing was dropped")); + assert_eq!( + scalar(&host, port, "master", "sa", SA_PASSWORD, "SELECT CASE WHEN DB_ID('alterdb') IS NULL THEN 0 ELSE 1 END").await, + 1 + ); + assert_eq!( + scalar( + &host, + port, + "master", + "sa", + SA_PASSWORD, + "SELECT CASE WHEN DATABASEPROPERTYEX('alterdb', 'UserAccess') = 'MULTI_USER' THEN 1 ELSE 0 END" + ) + .await, + 1 + ); +} + +#[tokio::test] +async fn mssql_restore_refuses_system_database_test() { + init_tracing_for_test(); + let (_container, host, port) = start().await; + + let temp_dir = TempDir::new().unwrap(); + let file_path = temp_dir.path().join("backup.dacpac"); + std::fs::write(&file_path, b"not a package").unwrap(); + + let config = make_config(host.clone(), port, "msdb", "5a445eb4-c2c6-4bde-a423-ee1385dcf6dd"); + let result = DatabaseFactory::create_for_restore(config, &file_path) + .await + .restore(&file_path, Arc::new(JobLogger::new())) + .await; + + assert!(format!("{:#}", result.unwrap_err()).contains("system database")); + assert_eq!( + scalar(&host, port, "master", "sa", SA_PASSWORD, "SELECT CASE WHEN DB_ID('msdb') IS NULL THEN 0 ELSE 1 END").await, + 1 + ); + assert_eq!( + scalar( + &host, + port, + "master", + "sa", + SA_PASSWORD, + "SELECT CASE WHEN DATABASEPROPERTYEX('msdb', 'UserAccess') = 'MULTI_USER' THEN 1 ELSE 0 END" + ) + .await, + 1 + ); +} + mod format_tests { use super::{make_config, with_backup_format}; use crate::domain::mssql::format::MssqlFormat;