From fdb3c93467d0659e7044ef266c3d24bdcbca38aa Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Jan 2026 21:11:11 +0000 Subject: [PATCH 1/3] Implement migration integrity verification for migrate up/down commands FEATURE: Add comprehensive integrity verification before migration execution This commit implements the migration integrity verification design: Schema verification: - Add verify_schema_checksum() method to MigrationTracker that computes the live database schema's xxhash3 checksum and compares against the expected value stored in tern.migrations - Add get_schema_hash() method to retrieve a migration's schema checksum - Verify schema integrity before executing migrate up/down commands - Detect when database has been modified outside of Tern migrations Migration hash verification: - Verify migration file content matches what was originally applied - Add MigrationModified error for detecting modified migration files - Check migration hash before reverting in migrate down Error handling: - Add SchemaDrift error variant with detailed diagnostic help - Add MigrationModified error variant with resolution steps - Both errors include expected vs actual checksums and guidance CLI updates: - Add --force flag to migrate up and migrate down commands to skip integrity verification in emergency situations - Display warning when --force is used - Update tern verify to compare against database's schema_hash instead of local state.json (with --include-local-state flag for backwards compatibility) The design follows the principle of detecting both: 1. Database modifications made outside of Tern (via schema checksum) 2. Migration file tampering after application (via migration hash) --- src/cli/migrate/down.rs | 62 ++++++++++++- src/cli/migrate/up.rs | 21 ++++- src/cli/up/mod.rs | 4 +- src/cli/verify/mod.rs | 174 ++++++++++++++++++++++++++--------- src/db/execution/error.rs | 30 ++++++ src/db/execution/executor.rs | 54 +++++++++-- src/db/execution/mod.rs | 4 +- src/db/execution/tracker.rs | 60 ++++++++++++ 8 files changed, 349 insertions(+), 60 deletions(-) diff --git a/src/cli/migrate/down.rs b/src/cli/migrate/down.rs index f1dbbcf..1f7bb50 100644 --- a/src/cli/migrate/down.rs +++ b/src/cli/migrate/down.rs @@ -11,7 +11,7 @@ use miette::{Context, IntoDiagnostic}; use serde::Serialize; use crate::cli::{OutputFormat, ensure_backend_initialized, load_backend, print_json}; -use crate::db::execution::MigrationTracker; +use crate::db::execution::{ExecutionError, MigrationTracker, compute_migration_hash}; use crate::db::migrate::{MigrationPlan, PostgresRenderer, RenderConfig}; use crate::db::state::{Migration, StateBackend}; use crate::db::{self}; @@ -42,6 +42,14 @@ pub struct Down { #[arg(long)] pub dry_run: bool, + /// Skip integrity verification (dangerous) + /// + /// This skips both schema checksum verification and migration hash + /// verification. Use only in emergency situations when you understand + /// the risks of schema corruption. + #[arg(long)] + pub force: bool, + /// Output format #[arg(long, default_value = "text")] pub format: OutputFormat, @@ -150,6 +158,15 @@ impl Down { // Ensure tracking infrastructure exists tracker.ensure_schema().await.into_diagnostic()?; + // Warn about --force usage + if self.force && !matches!(self.format, OutputFormat::Json) { + println!(); + println!("WARNING: Running with --force skips integrity verification."); + println!("This can result in schema corruption or data loss."); + println!("Only use this if you understand the risks."); + println!(); + } + // Get the current migration ID from the database let current_id = match tracker.get_current_migration_id().await.into_diagnostic()? { Some(id) => id, @@ -169,6 +186,36 @@ impl Down { } }; + // Get the current migration record (for verification) + let current_record = tracker + .get_migration(¤t_id) + .await + .into_diagnostic()? + .ok_or_else(|| { + miette::miette!( + "Migration {} is recorded as current but not found in tern.migrations table", + current_id + ) + })?; + + // Verify schema integrity (unless --force) + if !self.force { + tracker + .verify_schema_checksum(¤t_id, ¤t_record.schema_hash) + .await + .map_err(|e| match e { + ExecutionError::SchemaDrift { expected, actual, .. } => { + miette::miette!( + "Schema drift detected!\n\nThe live database schema does not match the expected state after migration {}.\n\n Expected checksum: {}\n Actual checksum: {}\n\nThis indicates the database was modified outside of Tern migrations.\n\nTo investigate: tern verify --database-url \nTo resolve:\n Option 1: Revert manual changes to match expected state\n Option 2: Run `tern compile` to capture changes as a new migration\n Option 3: Use `--force` to skip verification (dangerous)", + ¤t_id[..12.min(current_id.len())], + expected, + actual + ) + } + other => miette::miette!("{}", other), + })?; + } + // Find the migration in the local backend let migration = find_migration_by_hex_id(&backend, ¤t_id) .await @@ -185,6 +232,19 @@ impl Down { } }; + // Verify migration hash matches what was applied (unless --force) + if !self.force { + let local_hash = compute_migration_hash(&migration); + if local_hash != current_record.migration_hash { + return Err(miette::miette!( + "Migration file has been modified since it was applied!\n\nMigration {} has different content than what was recorded in the database.\n\n Expected hash: {}\n Actual hash: {}\n\nThis migration was applied with different operations than what's on disk.\n\nTo resolve:\n Option 1: Restore the original migration file from version control\n Option 2: Use `--force` to skip verification (dangerous)\n\nWarning: Reverting with mismatched history can cause schema inconsistencies between environments.", + ¤t_id[..12.min(current_id.len())], + current_record.migration_hash, + local_hash + )); + } + } + // Check if this is a baseline migration if migration.is_baseline() { return Err(miette::miette!( diff --git a/src/cli/migrate/up.rs b/src/cli/migrate/up.rs index 5e16b6c..b7ef9bd 100644 --- a/src/cli/migrate/up.rs +++ b/src/cli/migrate/up.rs @@ -40,6 +40,14 @@ pub struct Up { #[arg(long)] pub dry_run: bool, + /// Skip integrity verification (dangerous) + /// + /// This skips both schema checksum verification and migration hash + /// verification. Use only in emergency situations when you understand + /// the risks of schema corruption. + #[arg(long)] + pub force: bool, + /// Output format #[arg(long, default_value = "text")] pub format: OutputFormat, @@ -173,6 +181,15 @@ impl Up { // Create executor let executor = MigrationExecutor::new(&client, &backend, &self.schema); + // Warn about --force usage + if self.force && !matches!(self.format, OutputFormat::Json) { + println!(); + println!("WARNING: Running with --force skips integrity verification."); + println!("This can result in schema corruption or data loss."); + println!("Only use this if you understand the risks."); + println!(); + } + if self.dry_run { // Just show pending migrations if !matches!(self.format, OutputFormat::Json) { @@ -180,7 +197,7 @@ impl Up { } let pending = executor - .get_pending() + .get_pending(self.force) .await .into_diagnostic() .wrap_err("Failed to get pending migrations")?; @@ -221,7 +238,7 @@ impl Up { } let result = executor - .execute_pending() + .execute_pending(self.force) .await .into_diagnostic() .wrap_err("Failed to execute migrations")?; diff --git a/src/cli/up/mod.rs b/src/cli/up/mod.rs index ce3636c..95f2868 100644 --- a/src/cli/up/mod.rs +++ b/src/cli/up/mod.rs @@ -180,7 +180,7 @@ impl Up { } let pending = executor - .get_pending() + .get_pending(false) .await .into_diagnostic() .wrap_err("Failed to get pending migrations")?; @@ -221,7 +221,7 @@ impl Up { } let result = executor - .execute_pending() + .execute_pending(false) .await .into_diagnostic() .wrap_err("Failed to execute migrations")?; diff --git a/src/cli/verify/mod.rs b/src/cli/verify/mod.rs index 051a6f3..52fdd36 100644 --- a/src/cli/verify/mod.rs +++ b/src/cli/verify/mod.rs @@ -21,8 +21,12 @@ use crate::db::{self}; /// Verify state backend matches database /// -/// Compares the state backend to the live database schema and -/// reports any drift (manual changes not captured in migrations). +/// Compares the live database schema against the schema hash recorded +/// in `tern.migrations` for the current migration. This detects any +/// manual changes made outside of Tern migrations. +/// +/// By default, compares against the database's recorded schema_hash. +/// Use --include-local-state to also compare against local state.json. #[derive(Debug, Clone, Args)] pub struct Verify { /// PostgreSQL connection string @@ -40,6 +44,14 @@ pub struct Verify { /// Path to the state directory #[arg(long)] pub path: Option, + + /// Also compare against local state.json + /// + /// When enabled, additionally compares the database schema against + /// the local state.json file. This is useful for detecting if the + /// local state is out of sync with the database. + #[arg(long)] + pub include_local_state: bool, } /// Verify migration chain integrity @@ -60,12 +72,10 @@ pub struct VerifyChain { impl Verify { /// Dispatch the verify command. pub async fn dispatch(self) -> miette::Result<()> { - // Load the state backend - let backend = load_backend(self.path.as_deref()); - ensure_backend_initialized(&backend).await?; - // Connect to database - println!("Connecting to database..."); + if !matches!(self.format, OutputFormat::Json) { + println!("Connecting to database..."); + } let client = db::connect(&self.database_url) .await .into_diagnostic() @@ -73,41 +83,86 @@ impl Verify { let catalog = PostgresCatalog::new(&client); - println!("Verifying schema '{}'...", self.schema); + if !matches!(self.format, OutputFormat::Json) { + println!("Verifying schema '{}'...", self.schema); + } - // Get cached state (includes pre-computed xxhash3 checksum) - let cached_state = backend.get_cached_state().into_diagnostic()?; - let backend_checksum = cached_state.checksum().to_string(); - let backend_state = cached_state.into_namespace(); + // Get the current migration's expected schema_hash from DB + let tracker = crate::db::execution::MigrationTracker::new(&client, &self.schema); + tracker.ensure_schema().await.into_diagnostic()?; + + let current_id = tracker.get_current_migration_id().await.into_diagnostic()?; + let expected_hash = if let Some(ref id) = current_id { + tracker.get_schema_hash(id).await.into_diagnostic()? + } else { + None + }; // Load database state and compute its checksum let database_state = crate::db::query::load_namespace(&catalog, &self.schema) .await .into_diagnostic()?; let database_checksum = compute_schema_checksum(&database_state); + let database_hash = StateHash::from_namespace(&database_state); - // Quick check using checksums first - let checksums_match = backend_checksum == database_checksum; + // Primary check: compare against DB's recorded schema_hash + let db_verified = match (&expected_hash, ¤t_id) { + (Some(expected), Some(_)) => *expected == database_checksum, + (None, Some(_)) => { + // Migration exists but no schema_hash recorded (shouldn't happen normally) + if !matches!(self.format, OutputFormat::Json) { + println!("Warning: Current migration has no recorded schema_hash"); + } + true + } + (_, None) => { + // No migrations applied yet - nothing to verify against + true + } + }; - // Compute BLAKE3 state hashes for display - let backend_hash = StateHash::from_namespace(&backend_state); - let database_hash = StateHash::from_namespace(&database_state); + // Optional: also compare against local state.json + let (local_verified, backend_checksum, backend_hash) = if self.include_local_state { + let backend = load_backend(self.path.as_deref()); + ensure_backend_initialized(&backend).await?; + + let cached_state = backend.get_cached_state().into_diagnostic()?; + let checksum = cached_state.checksum().to_string(); + let backend_state = cached_state.into_namespace(); + let hash = StateHash::from_namespace(&backend_state); + let verified = checksum == database_checksum; + (Some(verified), Some(checksum), Some(hash.to_hex())) + } else { + (None, None, None) + }; - // Compute drift details if checksums don't match - let drift_details = if !checksums_match { + // Compute drift details if DB verification failed + let drift_details = if !db_verified && self.include_local_state { + let backend = load_backend(self.path.as_deref()); + let cached_state = backend.get_cached_state().into_diagnostic()?; + let backend_state = cached_state.into_namespace(); Some(compute_drift(&backend_state, &database_state)) } else { None }; + // Overall verification: passes if DB check passes (and local check if included) + let verified = db_verified && local_verified.unwrap_or(true); + let output = VerifyOutput { - verified: checksums_match, - backend_state_hash: backend_hash.to_hex(), + verified, + backend_state_hash: backend_hash.unwrap_or_else(|| "N/A".to_string()), database_state_hash: database_hash.to_hex(), - backend_schema_checksum: backend_checksum, - database_schema_checksum: database_checksum, - message: if checksums_match { - "State backend is in sync with database.".to_string() + backend_schema_checksum: backend_checksum.unwrap_or_else(|| "N/A".to_string()), + database_schema_checksum: database_checksum.clone(), + expected_schema_checksum: expected_hash, + current_migration_id: current_id.clone(), + message: if verified { + if current_id.is_none() { + "No migrations applied yet. Database schema is at initial state.".to_string() + } else { + "Database schema matches the expected state.".to_string() + } } else { "Database has been modified outside of Tern migrations.".to_string() }, @@ -120,7 +175,7 @@ impl Verify { } // Return error if verification failed (for CI usage) - if !checksums_match { + if !verified { std::process::exit(1); } @@ -178,14 +233,20 @@ impl VerifyChain { pub struct VerifyOutput { /// Whether the verification passed. pub verified: bool, - /// State backend state hash (BLAKE3). + /// State backend state hash (BLAKE3). Only present with --include-local-state. pub backend_state_hash: String, /// Database state hash (BLAKE3). pub database_state_hash: String, - /// Schema checksum (xxhash3) for the backend state. + /// Schema checksum (xxhash3) for the backend state. Only present with --include-local-state. pub backend_schema_checksum: String, /// Schema checksum (xxhash3) for the database state. pub database_schema_checksum: String, + /// Expected schema checksum from tern.migrations table. + #[serde(skip_serializing_if = "Option::is_none")] + pub expected_schema_checksum: Option, + /// Current migration ID from the database. + #[serde(skip_serializing_if = "Option::is_none")] + pub current_migration_id: Option, /// Detailed message. pub message: String, /// Drift details (if any). @@ -302,31 +363,45 @@ impl std::fmt::Display for VerifyOutput { writeln!(f, "Verification Passed")?; writeln!(f, "===================")?; writeln!(f)?; - writeln!(f, "State backend matches database schema.")?; + writeln!(f, "{}", self.message)?; writeln!(f)?; - writeln!(f, " State hash: {}", &self.backend_state_hash[..16])?; - writeln!(f, " Schema checksum: {}", &self.backend_schema_checksum)?; + if let Some(ref migration_id) = self.current_migration_id { + writeln!( + f, + " Current migration: {}...", + &migration_id[..12.min(migration_id.len())] + )?; + } + writeln!(f, " Schema checksum: {}", &self.database_schema_checksum)?; + if self.backend_state_hash != "N/A" { + writeln!( + f, + " Local state hash: {}...", + &self.backend_state_hash[..16.min(self.backend_state_hash.len())] + )?; + } } else { writeln!(f, "Verification Failed")?; writeln!(f, "===================")?; writeln!(f)?; writeln!(f, "WARNING: Schema drift detected!")?; writeln!(f)?; - writeln!(f, "State Hashes (BLAKE3):")?; - writeln!( - f, - " Backend: {}", - &self.backend_state_hash[..std::cmp::min(16, self.backend_state_hash.len())] - )?; - writeln!( - f, - " Database: {}", - &self.database_state_hash[..std::cmp::min(16, self.database_state_hash.len())] - )?; + if let Some(ref migration_id) = self.current_migration_id { + writeln!( + f, + "Current migration: {}...", + &migration_id[..12.min(migration_id.len())] + )?; + } writeln!(f)?; writeln!(f, "Schema Checksums (xxhash3):")?; - writeln!(f, " Backend: {}", &self.backend_schema_checksum)?; - writeln!(f, " Database: {}", &self.database_schema_checksum)?; + if let Some(ref expected) = self.expected_schema_checksum { + writeln!(f, " Expected: {}", expected)?; + } + writeln!(f, " Actual: {}", &self.database_schema_checksum)?; + if self.backend_schema_checksum != "N/A" { + writeln!(f, " Local: {}", &self.backend_schema_checksum)?; + } writeln!(f)?; writeln!(f, "{}", self.message)?; @@ -770,6 +845,10 @@ mod tests { database_state_hash: "a".repeat(64), backend_schema_checksum: "abc123def456".to_string(), database_schema_checksum: "abc123def456".to_string(), + expected_schema_checksum: Some("abc123def456".to_string()), + current_migration_id: Some( + "abc123def456789012345678901234567890123456789012345678901234".to_string(), + ), message: "All good".to_string(), drift_details: None, }; @@ -787,6 +866,10 @@ mod tests { database_state_hash: "b".repeat(64), backend_schema_checksum: "abc123def456".to_string(), database_schema_checksum: "def456abc123".to_string(), + expected_schema_checksum: Some("abc123def456".to_string()), + current_migration_id: Some( + "abc123def456789012345678901234567890123456789012345678901234".to_string(), + ), message: "Drift detected".to_string(), drift_details: Some(DriftDetails { summary: DriftSummary { @@ -877,6 +960,8 @@ mod tests { database_state_hash: "a".repeat(64), backend_schema_checksum: "abc123def456".to_string(), database_schema_checksum: "abc123def456".to_string(), + expected_schema_checksum: Some("abc123def456".to_string()), + current_migration_id: Some("migration123".to_string()), message: "All good".to_string(), drift_details: None, }; @@ -884,6 +969,7 @@ mod tests { let json = serde_json::to_string(&output).unwrap(); assert!(json.contains("backend_schema_checksum")); assert!(json.contains("database_schema_checksum")); + assert!(json.contains("expected_schema_checksum")); assert!(json.contains("abc123def456")); } } diff --git a/src/db/execution/error.rs b/src/db/execution/error.rs index 61f3d07..2f9c5fa 100644 --- a/src/db/execution/error.rs +++ b/src/db/execution/error.rs @@ -98,6 +98,36 @@ pub enum ExecutionError { help("The baseline migration represents the initial state and cannot be reverted.") )] CannotRevertBaseline(String), + + /// Schema drift detected - database has been modified outside of Tern. + #[error( + "schema drift detected: database schema does not match expected state after migration {migration_id}" + )] + #[diagnostic( + code(tern::execution::schema_drift), + help( + "The database was modified outside of Tern migrations.\n\nExpected checksum: {expected}\nActual checksum: {actual}\n\nTo investigate: tern verify --database-url \nTo resolve:\n Option 1: Revert manual changes to match expected state\n Option 2: Run `tern compile` to capture changes as a new migration\n Option 3: Use `--force` to skip verification (dangerous)" + ) + )] + SchemaDrift { + migration_id: String, + expected: String, + actual: String, + }, + + /// Migration file has been modified since it was applied. + #[error("migration file has been modified since it was applied: {migration_id}")] + #[diagnostic( + code(tern::execution::migration_modified), + help( + "This migration was applied with different operations than what's on disk.\n\nExpected hash: {expected_hash}\nActual hash: {actual_hash}\n\nTo resolve:\n Option 1: Restore the original migration file from version control\n Option 2: Use `--force` to skip verification (dangerous)\n\nWarning: Running migrations with mismatched history can cause schema inconsistencies between environments." + ) + )] + MigrationModified { + migration_id: String, + expected_hash: String, + actual_hash: String, + }, } /// Represents the result of applying a single migration. diff --git a/src/db/execution/executor.rs b/src/db/execution/executor.rs index 976b42b..6df5242 100644 --- a/src/db/execution/executor.rs +++ b/src/db/execution/executor.rs @@ -55,22 +55,37 @@ impl<'a> MigrationExecutor<'a> { /// /// 1. Ensure tern schema and tracking tables exist /// 2. Get the current migration ID from the database - /// 3. Load local migrations and verify history integrity - /// 4. Identify pending migrations (those after current) - /// 5. For each pending migration: + /// 3. Verify schema integrity (unless force=true) + /// 4. Load local migrations and verify history integrity + /// 5. Identify pending migrations (those after current) + /// 6. For each pending migration: /// a. Begin transaction /// b. Render and execute SQL /// c. Compute schema checksum /// d. Record migration in tracking tables /// e. Commit transaction - /// 6. Return execution result - pub async fn execute_pending(&self) -> Result { + /// 7. Return execution result + /// + /// # Arguments + /// + /// * `force` - If true, skip integrity verification checks + pub async fn execute_pending(&self, force: bool) -> Result { // Ensure tracking infrastructure exists self.tracker.ensure_schema().await?; // Get current state let current_id = self.tracker.get_current_migration_id().await?; + // Verify schema integrity before proceeding (unless force=true) + if !force + && let Some(ref id) = current_id + && let Some(expected_hash) = self.tracker.get_schema_hash(id).await? + { + self.tracker + .verify_schema_checksum(id, &expected_hash) + .await?; + } + // Load all local migrations let all_migrations = self .backend @@ -78,8 +93,10 @@ impl<'a> MigrationExecutor<'a> { .await .map_err(|e| ExecutionError::InvalidState(e.to_string()))?; - // Verify history integrity - self.verify_history(&all_migrations).await?; + // Verify history integrity (unless force=true) + if !force { + self.verify_history(&all_migrations).await?; + } // Find pending migrations let pending = self.find_pending_migrations(&all_migrations, current_id.as_deref())?; @@ -108,13 +125,27 @@ impl<'a> MigrationExecutor<'a> { /// Gets the list of pending migrations without executing them. /// /// Useful for dry-run mode. - pub async fn get_pending(&self) -> Result, ExecutionError> { + /// + /// # Arguments + /// + /// * `force` - If true, skip integrity verification checks + pub async fn get_pending(&self, force: bool) -> Result, ExecutionError> { // Ensure tracking infrastructure exists self.tracker.ensure_schema().await?; // Get current state let current_id = self.tracker.get_current_migration_id().await?; + // Verify schema integrity before proceeding (unless force=true) + if !force + && let Some(ref id) = current_id + && let Some(expected_hash) = self.tracker.get_schema_hash(id).await? + { + self.tracker + .verify_schema_checksum(id, &expected_hash) + .await?; + } + // Load all local migrations let all_migrations = self .backend @@ -122,6 +153,11 @@ impl<'a> MigrationExecutor<'a> { .await .map_err(|e| ExecutionError::InvalidState(e.to_string()))?; + // Verify history integrity (unless force=true) + if !force { + self.verify_history(&all_migrations).await?; + } + // Find pending migrations self.find_pending_migrations(&all_migrations, current_id.as_deref()) } @@ -256,7 +292,7 @@ impl<'a> MigrationExecutor<'a> { /// applied. Only the up_operations array is hashed, not the description or /// timestamps, allowing descriptions to be updated without triggering /// divergence errors. -fn compute_migration_hash(migration: &Migration) -> String { +pub fn compute_migration_hash(migration: &Migration) -> String { let mut hasher = blake3::Hasher::new(); let ops_json = serde_json::to_vec(&migration.up_operations).expect("operations should be serializable"); diff --git a/src/db/execution/mod.rs b/src/db/execution/mod.rs index bb61a6b..536bccc 100644 --- a/src/db/execution/mod.rs +++ b/src/db/execution/mod.rs @@ -60,7 +60,7 @@ //! let backend = LocalFileBackend::default_location(); //! //! let executor = MigrationExecutor::new(&client, &backend, "public"); -//! let result = executor.execute_pending().await?; +//! let result = executor.execute_pending(false).await?; //! //! println!("Applied {} migration(s)", result.count()); //! Ok(()) @@ -72,5 +72,5 @@ mod executor; mod tracker; pub use error::{ExecutionError, ExecutionResult, MigrationResult}; -pub use executor::MigrationExecutor; +pub use executor::{MigrationExecutor, compute_migration_hash}; pub use tracker::{MigrationRecord, MigrationTracker}; diff --git a/src/db/execution/tracker.rs b/src/db/execution/tracker.rs index f16596e..85358dd 100644 --- a/src/db/execution/tracker.rs +++ b/src/db/execution/tracker.rs @@ -5,6 +5,8 @@ use tokio_postgres::Client; +use crate::db::checksum::compute_schema_checksum; +use crate::db::query::{PostgresCatalog, load_namespace}; use crate::db::state::MigrationId; use super::error::ExecutionError; @@ -148,6 +150,64 @@ impl<'a> MigrationTracker<'a> { })) } + /// Gets the schema_hash for a specific migration. + /// + /// Returns `None` if the migration is not found. + pub async fn get_schema_hash( + &self, + migration_id: &str, + ) -> Result, ExecutionError> { + let row = self + .client + .query_opt( + "SELECT schema_hash FROM tern.migrations WHERE id = $1", + &[&migration_id], + ) + .await + .map_err(|e| ExecutionError::Query(e.to_string()))?; + + Ok(row.map(|r| r.get(0))) + } + + /// Verifies that the live database schema matches the expected checksum. + /// + /// This method loads the current database schema and computes its xxhash3 + /// checksum, then compares it against the expected checksum (typically + /// from the last applied migration). + /// + /// # Arguments + /// + /// * `migration_id` - The ID of the migration whose schema hash we're comparing against + /// * `expected_checksum` - The expected xxhash3 checksum + /// + /// # Returns + /// + /// Returns `Ok(())` if checksums match, or `ExecutionError::SchemaDrift` if they differ. + pub async fn verify_schema_checksum( + &self, + migration_id: &str, + expected_checksum: &str, + ) -> Result<(), ExecutionError> { + // Load current namespace from database + let catalog = PostgresCatalog::new(self.client); + let namespace = load_namespace(&catalog, self.target_schema()) + .await + .map_err(|e| ExecutionError::Query(e.to_string()))?; + + // Compute actual checksum + let actual_checksum = compute_schema_checksum(&namespace); + + if actual_checksum != expected_checksum { + return Err(ExecutionError::SchemaDrift { + migration_id: migration_id.to_string(), + expected: expected_checksum.to_string(), + actual: actual_checksum, + }); + } + + Ok(()) + } + /// Gets the next sequence number for a new migration. pub async fn next_sequence(&self) -> Result { let row = self From e344b15211020e411e4f4efea9d240779fb8e682 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Jan 2026 21:21:21 +0000 Subject: [PATCH 2/3] Change --force to verify but warn instead of skip verification FEATURE: Improve --force flag behavior for better transparency Previously, --force would skip integrity verification entirely. Now: - Verification is always performed even when --force is used - If verification fails with --force: print warning and proceed - If verification passes with --force: print "force was unnecessary" - Without --force: error on verification failure (unchanged) This gives users better visibility into the state of their database while still allowing emergency overrides when needed. Changes: - Add check_integrity() method to MigrationExecutor for explicit verification without execution - Add VerificationStatus, SchemaMismatch, and HistoryDivergence types to capture verification results - Update migrate up CLI to call check_integrity() when --force is used - Update migrate down CLI to always verify but decide based on --force whether to error or warn --- src/cli/migrate/down.rs | 99 +++++++++++++++++++++----------- src/cli/migrate/up.rs | 48 ++++++++++++++-- src/db/execution/executor.rs | 108 +++++++++++++++++++++++++++++++++++ src/db/execution/mod.rs | 5 +- 4 files changed, 221 insertions(+), 39 deletions(-) diff --git a/src/cli/migrate/down.rs b/src/cli/migrate/down.rs index 1f7bb50..4a2e1ca 100644 --- a/src/cli/migrate/down.rs +++ b/src/cli/migrate/down.rs @@ -158,15 +158,6 @@ impl Down { // Ensure tracking infrastructure exists tracker.ensure_schema().await.into_diagnostic()?; - // Warn about --force usage - if self.force && !matches!(self.format, OutputFormat::Json) { - println!(); - println!("WARNING: Running with --force skips integrity verification."); - println!("This can result in schema corruption or data loss."); - println!("Only use this if you understand the risks."); - println!(); - } - // Get the current migration ID from the database let current_id = match tracker.get_current_migration_id().await.into_diagnostic()? { Some(id) => id, @@ -198,25 +189,7 @@ impl Down { ) })?; - // Verify schema integrity (unless --force) - if !self.force { - tracker - .verify_schema_checksum(¤t_id, ¤t_record.schema_hash) - .await - .map_err(|e| match e { - ExecutionError::SchemaDrift { expected, actual, .. } => { - miette::miette!( - "Schema drift detected!\n\nThe live database schema does not match the expected state after migration {}.\n\n Expected checksum: {}\n Actual checksum: {}\n\nThis indicates the database was modified outside of Tern migrations.\n\nTo investigate: tern verify --database-url \nTo resolve:\n Option 1: Revert manual changes to match expected state\n Option 2: Run `tern compile` to capture changes as a new migration\n Option 3: Use `--force` to skip verification (dangerous)", - ¤t_id[..12.min(current_id.len())], - expected, - actual - ) - } - other => miette::miette!("{}", other), - })?; - } - - // Find the migration in the local backend + // Find the migration in the local backend (needed for both verification and execution) let migration = find_migration_by_hex_id(&backend, ¤t_id) .await .into_diagnostic() @@ -232,10 +205,72 @@ impl Down { } }; - // Verify migration hash matches what was applied (unless --force) - if !self.force { - let local_hash = compute_migration_hash(&migration); - if local_hash != current_record.migration_hash { + // Always perform verification, but handle results based on --force flag + let schema_verification = tracker + .verify_schema_checksum(¤t_id, ¤t_record.schema_hash) + .await; + + let local_hash = compute_migration_hash(&migration); + let hash_matches = local_hash == current_record.migration_hash; + + // Determine if verification passed + let schema_ok = schema_verification.is_ok(); + let all_ok = schema_ok && hash_matches; + + if self.force && !matches!(self.format, OutputFormat::Json) { + if all_ok { + println!(); + println!("Note: --force was unnecessary, all integrity checks passed."); + println!(); + } else { + println!(); + println!("WARNING: Integrity checks failed, but proceeding due to --force flag."); + println!(); + if let Err(ExecutionError::SchemaDrift { + expected, actual, .. + }) = &schema_verification + { + println!( + " Schema drift detected for migration {}...:", + ¤t_id[..12.min(current_id.len())] + ); + println!(" Expected checksum: {}", expected); + println!(" Actual checksum: {}", actual); + println!(); + } + if !hash_matches { + println!( + " Migration file modified for {}...:", + ¤t_id[..12.min(current_id.len())] + ); + println!( + " Expected hash: {}", + ¤t_record.migration_hash + [..16.min(current_record.migration_hash.len())] + ); + println!( + " Actual hash: {}", + &local_hash[..16.min(local_hash.len())] + ); + println!(); + } + println!("Proceeding anyway. This can result in schema corruption or data loss."); + println!(); + } + } else if !self.force { + // When not forcing, error on verification failure + if let Err(ExecutionError::SchemaDrift { + expected, actual, .. + }) = schema_verification + { + return Err(miette::miette!( + "Schema drift detected!\n\nThe live database schema does not match the expected state after migration {}.\n\n Expected checksum: {}\n Actual checksum: {}\n\nThis indicates the database was modified outside of Tern migrations.\n\nTo investigate: tern verify --database-url \nTo resolve:\n Option 1: Revert manual changes to match expected state\n Option 2: Run `tern compile` to capture changes as a new migration\n Option 3: Use `--force` to skip verification (dangerous)", + ¤t_id[..12.min(current_id.len())], + expected, + actual + )); + } + if !hash_matches { return Err(miette::miette!( "Migration file has been modified since it was applied!\n\nMigration {} has different content than what was recorded in the database.\n\n Expected hash: {}\n Actual hash: {}\n\nThis migration was applied with different operations than what's on disk.\n\nTo resolve:\n Option 1: Restore the original migration file from version control\n Option 2: Use `--force` to skip verification (dangerous)\n\nWarning: Reverting with mismatched history can cause schema inconsistencies between environments.", ¤t_id[..12.min(current_id.len())], diff --git a/src/cli/migrate/up.rs b/src/cli/migrate/up.rs index b7ef9bd..1cdd2c3 100644 --- a/src/cli/migrate/up.rs +++ b/src/cli/migrate/up.rs @@ -181,13 +181,49 @@ impl Up { // Create executor let executor = MigrationExecutor::new(&client, &backend, &self.schema); - // Warn about --force usage + // When --force is used, still perform verification but warn instead of error if self.force && !matches!(self.format, OutputFormat::Json) { - println!(); - println!("WARNING: Running with --force skips integrity verification."); - println!("This can result in schema corruption or data loss."); - println!("Only use this if you understand the risks."); - println!(); + let status = executor + .check_integrity() + .await + .into_diagnostic() + .wrap_err("Failed to check integrity")?; + + if status.all_ok() { + println!(); + println!("Note: --force was unnecessary, all integrity checks passed."); + println!(); + } else { + println!(); + println!("WARNING: Integrity checks failed, but proceeding due to --force flag."); + println!(); + if let Some(ref mismatch) = status.schema_mismatch { + println!( + " Schema drift detected for migration {}...:", + &mismatch.migration_id[..12.min(mismatch.migration_id.len())] + ); + println!(" Expected checksum: {}", mismatch.expected); + println!(" Actual checksum: {}", mismatch.actual); + println!(); + } + if let Some(ref divergence) = status.history_diverged { + println!( + " Migration history diverged for {}...:", + &divergence.migration_id[..12.min(divergence.migration_id.len())] + ); + println!( + " Expected hash: {}", + &divergence.expected_hash[..16.min(divergence.expected_hash.len())] + ); + println!( + " Actual hash: {}", + &divergence.actual_hash[..16.min(divergence.actual_hash.len())] + ); + println!(); + } + println!("Proceeding anyway. This can result in schema corruption or data loss."); + println!(); + } } if self.dry_run { diff --git a/src/db/execution/executor.rs b/src/db/execution/executor.rs index 6df5242..34118fb 100644 --- a/src/db/execution/executor.rs +++ b/src/db/execution/executor.rs @@ -14,6 +14,48 @@ use crate::db::state::{Migration, MigrationId}; use super::error::{ExecutionError, ExecutionResult, MigrationResult}; use super::tracker::MigrationTracker; +/// Result of integrity verification checks. +#[derive(Debug, Clone, Default)] +pub struct VerificationStatus { + /// Whether schema checksum verification passed (or was not applicable). + pub schema_ok: bool, + /// Details about schema verification failure, if any. + pub schema_mismatch: Option, + /// Whether migration history verification passed. + pub history_ok: bool, + /// Details about history verification failure, if any. + pub history_diverged: Option, +} + +/// Details about a schema checksum mismatch. +#[derive(Debug, Clone)] +pub struct SchemaMismatch { + /// The migration ID that was verified against. + pub migration_id: String, + /// The expected checksum from the database. + pub expected: String, + /// The actual checksum computed from the live schema. + pub actual: String, +} + +/// Details about migration history divergence. +#[derive(Debug, Clone)] +pub struct HistoryDivergence { + /// The first migration that diverged. + pub migration_id: String, + /// The expected hash from the database. + pub expected_hash: String, + /// The actual hash computed from the local file. + pub actual_hash: String, +} + +impl VerificationStatus { + /// Returns true if all checks passed. + pub fn all_ok(&self) -> bool { + self.schema_ok && self.history_ok + } +} + /// Executes migrations against a live PostgreSQL database. /// /// The executor: @@ -49,6 +91,72 @@ impl<'a> MigrationExecutor<'a> { self.tracker.target_schema() } + /// Checks integrity of the migration state without executing anything. + /// + /// This method verifies: + /// 1. Schema checksum - the live database matches the expected state + /// 2. History integrity - migration files match what was applied + /// + /// Returns a `VerificationStatus` indicating what passed or failed. + pub async fn check_integrity(&self) -> Result { + // Ensure tracking infrastructure exists + self.tracker.ensure_schema().await?; + + let mut status = VerificationStatus { + schema_ok: true, + schema_mismatch: None, + history_ok: true, + history_diverged: None, + }; + + // Get current state + let current_id = self.tracker.get_current_migration_id().await?; + + // Check schema integrity + if let Some(ref id) = current_id + && let Some(expected_hash) = self.tracker.get_schema_hash(id).await? + && let Err(ExecutionError::SchemaDrift { + expected, actual, .. + }) = self + .tracker + .verify_schema_checksum(id, &expected_hash) + .await + { + status.schema_ok = false; + status.schema_mismatch = Some(SchemaMismatch { + migration_id: id.clone(), + expected, + actual, + }); + } + + // Load all local migrations for history check + let all_migrations = self + .backend + .get_all_migrations() + .await + .map_err(|e| ExecutionError::InvalidState(e.to_string()))?; + + // Check history integrity + let local_hashes: Vec<(MigrationId, String)> = all_migrations + .iter() + .map(|m| (m.id, compute_migration_hash(m))) + .collect(); + + let diverged = self.tracker.verify_history(&local_hashes).await?; + + if let Some((id, expected, actual)) = diverged.first() { + status.history_ok = false; + status.history_diverged = Some(HistoryDivergence { + migration_id: id.clone(), + expected_hash: expected.clone(), + actual_hash: actual.clone(), + }); + } + + Ok(status) + } + /// Executes all pending migrations. /// /// # Algorithm diff --git a/src/db/execution/mod.rs b/src/db/execution/mod.rs index 536bccc..5af3b6f 100644 --- a/src/db/execution/mod.rs +++ b/src/db/execution/mod.rs @@ -72,5 +72,8 @@ mod executor; mod tracker; pub use error::{ExecutionError, ExecutionResult, MigrationResult}; -pub use executor::{MigrationExecutor, compute_migration_hash}; +pub use executor::{ + HistoryDivergence, MigrationExecutor, SchemaMismatch, VerificationStatus, + compute_migration_hash, +}; pub use tracker::{MigrationRecord, MigrationTracker}; From a31aeb85842ed14ef412289e40d28cd6283b5839 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Jan 2026 21:23:07 +0000 Subject: [PATCH 3/3] Remove migration integrity verification design document CHORE: Remove design document after implementation complete The migration integrity verification feature has been fully implemented, so the design document is no longer needed. --- .../migration-integrity-verification.md | 604 ------------------ 1 file changed, 604 deletions(-) delete mode 100644 docs/design/migration-integrity-verification.md diff --git a/docs/design/migration-integrity-verification.md b/docs/design/migration-integrity-verification.md deleted file mode 100644 index be07260..0000000 --- a/docs/design/migration-integrity-verification.md +++ /dev/null @@ -1,604 +0,0 @@ -# Design Document: Migration Integrity Verification - -## Overview - -This document describes the design for comprehensive integrity verification during migration execution. The goal is to ensure that both the database schema and migration history are in the expected state before applying or reverting migrations. - -### Core Problem - -When executing migrations, Tern must answer two critical questions: - -1. **Is the database in the expected state?** Before applying migration N, the database should match the schema that resulted from applying migration N-1. - -2. **Has the migration history been tampered with?** The migration files on disk should match what was originally applied to this database. - -Without verifying both conditions, migrations can silently corrupt the database or produce inconsistent states across environments. - -### Why This Matters - -Consider these failure scenarios that proper integrity verification would prevent: - -**Scenario 1: Manual database modification** -``` -Developer A runs migrations 1-5 on staging. -DBA manually adds an index to staging for performance testing. -Developer B runs migration 6, which also tries to create that index. -Result: Migration 6 fails with "index already exists" error. -``` - -**Scenario 2: Modified migration file** -``` -Developer A creates and applies migration 3 (adds column `status VARCHAR(50)`). -Developer A realizes the column should be an enum, modifies migration 3 on disk. -Developer B pulls the modified migration 3 and runs `migrate up`. -Result: Database has VARCHAR column, new environments get ENUM column. Schema drift. -``` - -**Scenario 3: Divergent environments** -``` -Production has migrations 1-10 applied. -A deployment runs `migrate up` assuming it's at migration 8. -Migrations 9-10 run again, or fail, or produce duplicate data. -``` - -All these scenarios are preventable with proper integrity verification. - -## Integrity Verification Design - -### Two Types of Integrity Checks - -Tern maintains two independent hashes for integrity verification: - -| Hash Type | Algorithm | What It Verifies | Stored In | -|-----------|-----------|------------------|-----------| -| **Schema Checksum** | xxhash3 (64-bit) | The actual database schema structure | `tern.migrations.schema_hash` | -| **Migration Hash** | BLAKE3 (256-bit) | The content of migration operations | `tern.migrations.migration_hash` | - -These serve complementary purposes: - -**Schema Checksum (xxhash3)** -- Fast, non-cryptographic hash of the `Namespace` structure -- Detects when the database has been modified outside of Tern -- Answers: "Is the live database what we expect it to be?" - -**Migration Hash (BLAKE3)** -- Cryptographic hash of `up_operations` array -- Detects when migration files have been modified after being applied -- Answers: "Are the migration files authentic?" - -### Why Two Hashes? - -A single hash cannot serve both purposes: - -1. **Schema changes without migration changes**: Someone runs `ALTER TABLE` directly on the database. The migration files haven't changed, but the schema has. Only the schema checksum detects this. - -2. **Migration changes without schema changes**: Someone modifies a migration file that was already applied. The live schema hasn't changed (it was applied with the old version), but future environments will get different schemas. Only the migration hash detects this. - -3. **Both can be correct while state is wrong**: If we only checked one, we might miss the other class of problems. - -### The `tern.migrations` Table - -```sql -CREATE TABLE tern.migrations ( - id TEXT PRIMARY KEY, -- Migration ID (BLAKE3 of content) - sequence INTEGER NOT NULL UNIQUE, -- Sequential ordering - description TEXT NOT NULL, - migration_hash TEXT NOT NULL, -- BLAKE3 of up_operations - schema_hash TEXT NOT NULL, -- xxhash3 of resulting Namespace - applied_at TIMESTAMPTZ NOT NULL DEFAULT now() -); -``` - -After applying migration N: -- `migration_hash` = BLAKE3 hash of migration N's `up_operations` -- `schema_hash` = xxhash3 checksum of the database schema after migration N - -### Verification Points - -Integrity checks should occur at these points: - -| Command | Before Execution | After Execution | -|---------|-----------------|-----------------| -| `migrate up` | Verify schema matches last `schema_hash`; Verify all migration hashes | Compute and store new `schema_hash` | -| `migrate down` | Verify schema matches current `schema_hash`; Verify current migration hash | Compute and store previous `schema_hash` | - -## Current Implementation Status - -### What's Implemented - -**Migration hash verification during `migrate up`:** -- Location: `src/db/execution/executor.rs:82` calls `verify_history()` -- Location: `src/db/execution/tracker.rs:216-239` implements the check -- Compares BLAKE3 hash of local `up_operations` against stored `migration_hash` -- Raises `ExecutionError::HistoryDiverged` if mismatch - -**Schema hash storage:** -- Location: `src/db/execution/executor.rs:223` computes xxhash3 after applying migration -- Location: `src/db/execution/executor.rs:229-236` stores in `tern.migrations.schema_hash` -- The hash is computed and stored but never verified - -**`tern verify` command (manual, separate):** -- Location: `src/cli/verify/mod.rs:62-128` -- Compares local `state.json` checksum against live database -- Not integrated into `migrate up` or `migrate down` -- Uses `state.json` which is machine-local (not in VCS) - -### What's Missing - -| Gap | Impact | Severity | -|-----|--------|----------| -| No schema verification before `migrate up` | Manual DB changes go undetected until migration fails | High | -| No schema verification before `migrate down` | Reverting from unexpected state can corrupt DB | High | -| No migration hash verification during `migrate down` | Modified down_operations could revert incorrectly | Medium | -| `tern verify` uses local `state.json` instead of DB's `schema_hash` | Can't detect drift consistently across machines | Medium | - -### Current Data Flow - -``` -migrate up (current): - - 1. Load migrations from disk (.tern/migrations/*.json) - 2. Query tern.current → get current_id - 3. verify_history() → compare migration_hash values ✓ - 4. Find pending migrations - 5. For each pending: - a. Execute up_operations SQL - b. Compute schema_hash ✓ (stored but never verified) - c. Compute migration_hash ✓ - d. INSERT into tern.migrations - e. UPDATE tern.current - - Missing: Step 3.5 - Verify live schema matches expected schema_hash -``` - -``` -migrate down (current): - - 1. Query tern.current → get current_id - 2. Load migration from disk by ID - 3. Execute down_operations SQL - 4. DELETE from tern.migrations - 5. UPDATE tern.current - - Missing: Step 2.5 - Verify migration_hash matches - Missing: Step 2.5 - Verify live schema matches schema_hash -``` - -## Proposed Design - -### Verification Before `migrate up` - -Before applying any pending migrations, verify: - -1. **Schema integrity**: The live database schema checksum must match the `schema_hash` stored for the current migration (the last one applied). - -2. **Migration history integrity**: All migration files on disk must have `migration_hash` values matching what's stored in `tern.migrations`. - -``` -migrate up (proposed): - - 1. Load migrations from disk - 2. Query tern.current → get current_id - 3. Query tern.migrations WHERE id = current_id → get expected schema_hash - 4. Compute live database schema checksum - 5. VERIFY: live checksum == expected schema_hash - → If mismatch: Error "Schema drift detected" - 6. verify_history() → compare all migration_hash values - → If mismatch: Error "Migration history diverged" - 7. Find pending migrations - 8. For each pending: - a. Execute up_operations SQL - b. Compute and store schema_hash - c. Compute and store migration_hash - d. Record in tern.migrations -``` - -### Verification Before `migrate down` - -Before reverting the current migration, verify: - -1. **Schema integrity**: The live database must match the `schema_hash` stored for the current migration. - -2. **Migration integrity**: The migration file's `up_operations` hash must match the stored `migration_hash`. (This ensures we're reverting what was actually applied.) - -``` -migrate down (proposed): - - 1. Query tern.current → get current_id - 2. Query tern.migrations WHERE id = current_id - → get expected schema_hash, migration_hash - 3. Compute live database schema checksum - 4. VERIFY: live checksum == expected schema_hash - → If mismatch: Error "Schema drift detected" - 5. Load migration from disk by ID - 6. Compute BLAKE3 hash of migration's up_operations - 7. VERIFY: computed hash == stored migration_hash - → If mismatch: Error "Migration file has been modified" - 8. Execute down_operations SQL - 9. Compute new schema checksum (should match previous migration's schema_hash) - 10. DELETE from tern.migrations WHERE id = current_id - 11. UPDATE tern.current to previous migration -``` - -### Fresh Database Handling - -When no migrations have been applied yet (`tern.current` is empty): - -- **Schema verification**: Skip (no expected state) -- **Migration history verification**: Skip (nothing to compare) - -The first migration is always applied without verification since there's no prior state. - -### Error Messages - -**Schema drift detected:** -``` -Error: Schema drift detected - -The live database schema does not match the expected state after migration 5. - - Expected checksum: a1b2c3d4e5f6 - Actual checksum: f6e5d4c3b2a1 - -This indicates the database was modified outside of Tern migrations. - -To investigate: - tern verify --database-url - -To resolve: - Option 1: Revert manual changes to match expected state - Option 2: Run `tern compile` to capture changes as a new migration - Option 3: Use `--force` to skip verification (dangerous) -``` - -**Migration history diverged:** -``` -Error: Migration history diverged - -Migration 00003 (a1b2c3d4) has been modified since it was applied. - - Expected hash: 5e6f7a8b9c0d... - Actual hash: 1a2b3c4d5e6f... - -This migration was applied with different operations than what's on disk. - -To resolve: - Option 1: Restore the original migration file from version control - Option 2: Use `--force` to skip verification (dangerous) - -Warning: Running migrations with mismatched history can cause schema -inconsistencies between environments. -``` - -### Force Flag - -For emergency situations, provide a `--force` flag that skips verification: - -```bash -tern migrate up --force # Skip all integrity checks -tern migrate down --force # Skip all integrity checks -``` - -This should print a prominent warning: - -``` -Warning: Running with --force skips integrity verification. -This can result in schema corruption or data loss. -Only use this if you understand the risks. - -Proceeding in 5 seconds... (Ctrl+C to cancel) -``` - -### Updating `tern verify` - -The `tern verify` command should be updated to compare against the database's `schema_hash` rather than local `state.json`: - -**Current behavior:** -- Compares `state.json` checksum vs live database -- `state.json` is machine-local, not shared - -**Proposed behavior:** -- Compare live database checksum vs `tern.migrations.schema_hash` (for the current migration) -- This uses the shared source of truth in the database itself -- Optionally also compare against local `state.json` with a flag - -```bash -# Compare live DB against DB's recorded schema_hash (new default) -tern verify --database-url - -# Also compare against local state.json -tern verify --database-url --include-local-state -``` - -## Implementation Changes - -### 1. Add `verify_schema_checksum` to `MigrationTracker` - -**File:** `src/db/execution/tracker.rs` - -```rust -/// Verifies that the live database schema matches the expected checksum. -/// -/// Returns Ok(()) if checksums match, or an error describing the mismatch. -pub async fn verify_schema_checksum( - &self, - expected_checksum: &str, -) -> Result<(), ExecutionError> { - // Load current namespace from database - let catalog = PostgresCatalog::new(self.client); - let namespace = load_namespace(&catalog, self.target_schema()).await - .map_err(|e| ExecutionError::Query(e.to_string()))?; - - // Compute actual checksum - let actual_checksum = compute_schema_checksum(&namespace); - - if actual_checksum != expected_checksum { - return Err(ExecutionError::SchemaDrift { - expected: expected_checksum.to_string(), - actual: actual_checksum, - }); - } - - Ok(()) -} - -/// Gets the schema_hash for a specific migration. -pub async fn get_schema_hash(&self, migration_id: &str) -> Result, ExecutionError> { - let row = self - .client - .query_opt( - "SELECT schema_hash FROM tern.migrations WHERE id = $1", - &[&migration_id], - ) - .await - .map_err(|e| ExecutionError::Query(e.to_string()))?; - - Ok(row.map(|r| r.get(0))) -} -``` - -### 2. Add New Error Variants - -**File:** `src/db/execution/error.rs` - -```rust -#[derive(Debug, Error, Diagnostic)] -pub enum ExecutionError { - // ... existing variants ... - - #[error("Schema drift detected")] - #[diagnostic( - code(tern::schema_drift), - help("The database was modified outside of Tern. Run `tern verify` to investigate.") - )] - SchemaDrift { - expected: String, - actual: String, - }, - - #[error("Migration file has been modified since it was applied")] - #[diagnostic( - code(tern::migration_modified), - help("Restore the original migration from version control or use --force.") - )] - MigrationModified { - migration_id: String, - expected_hash: String, - actual_hash: String, - }, -} -``` - -### 3. Update `execute_pending` in `MigrationExecutor` - -**File:** `src/db/execution/executor.rs` - -```rust -pub async fn execute_pending(&self, force: bool) -> Result { - // Ensure tracking infrastructure exists - self.tracker.ensure_schema().await?; - - // Get current state - let current_id = self.tracker.get_current_migration_id().await?; - - // NEW: Verify schema integrity before proceeding - if !force { - if let Some(ref id) = current_id { - if let Some(expected_hash) = self.tracker.get_schema_hash(id).await? { - self.tracker.verify_schema_checksum(&expected_hash).await?; - } - } - } - - // Load all local migrations - let all_migrations = self - .backend - .get_all_migrations() - .await - .map_err(|e| ExecutionError::InvalidState(e.to_string()))?; - - // Verify history integrity (existing) - if !force { - self.verify_history(&all_migrations).await?; - } - - // ... rest of existing implementation ... -} -``` - -### 4. Update `migrate down` Command - -**File:** `src/cli/migrate/down.rs` - -Add schema and migration hash verification before executing down operations: - -```rust -pub async fn execute_down( - client: &Client, - backend: &LocalFileBackend, - target_schema: &str, - force: bool, -) -> Result { - let tracker = MigrationTracker::new(client, target_schema); - - // Get current migration - let current_id = tracker.get_current_migration_id().await? - .ok_or(ExecutionError::NoMigrationsToRevert)?; - - let current_record = tracker.get_migration(¤t_id).await? - .ok_or(ExecutionError::MigrationNotFound(current_id.clone()))?; - - // NEW: Verify schema integrity - if !force { - tracker.verify_schema_checksum(¤t_record.schema_hash).await?; - } - - // Load migration from disk - let migration = backend.get_migration(&MigrationId::from_hex(¤t_id).unwrap()) - .await - .map_err(|e| ExecutionError::InvalidState(e.to_string()))?; - - // NEW: Verify migration hash matches what was applied - if !force { - let local_hash = compute_migration_hash(&migration); - if local_hash != current_record.migration_hash { - return Err(ExecutionError::MigrationModified { - migration_id: current_id, - expected_hash: current_record.migration_hash, - actual_hash: local_hash, - }); - } - } - - // Execute down operations - // ... rest of implementation ... -} -``` - -### 5. Add `--force` Flag to CLI Commands - -**File:** `src/cli/migrate/up.rs` - -```rust -#[derive(Debug, Clone, Args)] -pub struct Up { - // ... existing fields ... - - /// Skip integrity verification (dangerous) - #[arg(long)] - pub force: bool, -} -``` - -**File:** `src/cli/migrate/down.rs` - -```rust -#[derive(Debug, Clone, Args)] -pub struct Down { - // ... existing fields ... - - /// Skip integrity verification (dangerous) - #[arg(long)] - pub force: bool, -} -``` - -### 6. Update `tern verify` Command - -**File:** `src/cli/verify/mod.rs` - -```rust -impl Verify { - pub async fn dispatch(self) -> miette::Result<()> { - // Connect to database - let client = db::connect(&self.database_url).await?; - let tracker = MigrationTracker::new(&client, &self.schema); - - // Get current migration's expected schema_hash from DB - let current_id = tracker.get_current_migration_id().await?; - - let expected_hash = if let Some(ref id) = current_id { - tracker.get_schema_hash(id).await? - } else { - None - }; - - // Compute live schema checksum - let catalog = PostgresCatalog::new(&client); - let namespace = load_namespace(&catalog, &self.schema).await?; - let actual_hash = compute_schema_checksum(&namespace); - - // Compare against DB's recorded hash (primary check) - let db_verified = match expected_hash { - Some(ref expected) => *expected == actual_hash, - None => true, // No migrations applied yet - }; - - // Optionally compare against local state.json - let local_verified = if self.include_local_state { - let backend = load_backend(self.path.as_deref()); - let cached = backend.get_cached_state()?; - cached.checksum() == actual_hash - } else { - true - }; - - // ... output results ... - } -} -``` - -## Summary of Changes - -| Component | Change | Priority | -|-----------|--------|----------| -| `MigrationTracker` | Add `verify_schema_checksum()` method | High | -| `MigrationTracker` | Add `get_schema_hash()` method | High | -| `ExecutionError` | Add `SchemaDrift` and `MigrationModified` variants | High | -| `MigrationExecutor::execute_pending` | Add schema verification before applying | High | -| `migrate down` command | Add schema and migration hash verification | High | -| CLI (`up`, `down`) | Add `--force` flag | Medium | -| `tern verify` | Compare against DB's `schema_hash` instead of `state.json` | Medium | -| Error messages | Add detailed diagnostics with resolution steps | Medium | - -## Testing Strategy - -### Unit Tests - -1. `verify_schema_checksum` returns Ok when checksums match -2. `verify_schema_checksum` returns `SchemaDrift` error when checksums differ -3. `execute_pending` calls verification before applying (mock tracker) -4. `execute_pending` skips verification when `force=true` -5. Down migration fails when migration hash doesn't match -6. Down migration succeeds when `force=true` despite hash mismatch - -### Integration Tests - -1. Create migrations, apply them, manually alter DB, verify `migrate up` fails -2. Create migrations, apply them, modify migration file, verify `migrate up` fails -3. Create migrations, apply them, `migrate down` succeeds -4. Create migrations, apply them, modify migration file, verify `migrate down` fails -5. All above with `--force` flag should succeed (with warnings) - -### Manual Testing Scenarios - -1. Fresh database → apply migrations → verify checksums stored correctly -2. Apply migrations → run `psql ALTER TABLE` → attempt `migrate up` → expect failure -3. Apply migrations → edit migration JSON on disk → attempt `migrate up` → expect failure -4. Apply migrations → `tern verify` → expect success -5. Apply migrations → `psql ALTER TABLE` → `tern verify` → expect failure with diff - -## Appendix: Hash Algorithm Choices - -### Why xxhash3 for Schema Checksum? - -- **Speed**: ~10x faster than BLAKE3 for this use case -- **Sufficient collision resistance**: 64-bit hash with birthday bound of ~2^32 is adequate for detecting accidental changes -- **Not security-critical**: We're detecting accidental drift, not malicious tampering -- **Consistency**: Already used elsewhere in Tern for schema checksums - -### Why BLAKE3 for Migration Hash? - -- **Cryptographic security**: Prevents intentional tampering with migration history -- **Consistency**: Migration IDs are already BLAKE3 hashes -- **Integrity chain**: Each migration's ID includes its parent's hash, creating a tamper-evident chain