diff --git a/src/commands/pii.rs b/src/commands/pii.rs index ce07f6a..200bd47 100644 --- a/src/commands/pii.rs +++ b/src/commands/pii.rs @@ -9,11 +9,11 @@ use url::Url; use crate::db::store_blob; use crate::error::{HarliteError, Result}; -use super::query::OutputFormat; use super::csv::write_csv_field; +use super::query::OutputFormat; use super::util::{ - canonicalize_path_for_compare, copy_database_consistent, finalize_sensitive_write, - prepare_sensitive_write, remove_database_with_sidecars, resolve_database, ExternalPathPolicy, + canonicalize_path_for_compare, delete_orphaned_blobs, finalize_sensitive_write, + prepare_sensitive_write, resolve_database, ExternalPathPolicy, StagedDatabase, }; #[derive(Clone, Copy, Debug, serde::Serialize)] @@ -139,8 +139,16 @@ pub fn run_pii_with_external_paths( allow_external_paths, external_path_root, )?; + + let matchers = build_matchers(options)?; + if matchers.is_empty() { + return Err(HarliteError::InvalidArgs( + "No PII patterns provided".to_string(), + )); + } + let write = options.redact && !options.dry_run; - let target_db = if write { + let staged_output = if write { if let Some(out) = &options.output { let input_cmp = canonicalize_path_for_compare(&input_db)?; let out_cmp = canonicalize_path_for_compare(out)?; @@ -155,22 +163,17 @@ pub fn run_pii_with_external_paths( out.display() ))); } - remove_database_with_sidecars(out)?; - copy_database_consistent(&input_db, out)?; - out.clone() + Some(StagedDatabase::copy_from(&input_db, out, options.force)?) } else { - input_db.clone() + None } } else { - input_db.clone() + None }; - - let matchers = build_matchers(options)?; - if matchers.is_empty() { - return Err(HarliteError::InvalidArgs( - "No PII patterns provided".to_string(), - )); - } + let target_db = staged_output + .as_ref() + .map(|staged| staged.path().to_path_buf()) + .unwrap_or_else(|| input_db.clone()); let conn = if write { let conn = Connection::open(&target_db)?; @@ -180,7 +183,12 @@ pub fn run_pii_with_external_paths( super::query::open_readonly_connection(&target_db)? }; - let mut stmt = conn.prepare( + if write { + conn.execute_batch("BEGIN IMMEDIATE")?; + } + let work_conn = &conn; + + let mut stmt = work_conn.prepare( "SELECT id, url, query_string, request_body_hash, request_body_size, response_body_hash, response_body_size, response_body_hash_raw, response_body_size_raw FROM entries ORDER BY id", )?; @@ -198,11 +206,11 @@ pub fn run_pii_with_external_paths( )) })?; - let mut update = conn.prepare( + let mut update = work_conn.prepare( "UPDATE entries SET url=?1, query_string=?2, request_body_hash=?3, request_body_size=?4, response_body_hash=?5, response_body_size=?6, response_body_hash_raw=?7, response_body_size_raw=?8 WHERE id=?9", )?; - let has_fts: bool = conn + let has_fts: bool = work_conn .query_row( "SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='response_body_fts'", [], @@ -262,9 +270,7 @@ pub fn run_pii_with_external_paths( } if let Some(hash) = req_body_hash.as_deref() { - if let Some(text) = - load_blob_text(&conn, hash, &mut text_cache, &external_paths)? - { + if let Some(text) = load_blob_text(work_conn, hash, &mut text_cache, &external_paths)? { append_findings( &mut findings, entry_id, @@ -275,7 +281,7 @@ pub fn run_pii_with_external_paths( if options.redact { if let Some(redacted) = redact_blob_cached( - &conn, + work_conn, hash, &matchers, &options.token, @@ -292,9 +298,7 @@ pub fn run_pii_with_external_paths( } if let Some(hash) = resp_body_hash.as_deref() { - if let Some(text) = - load_blob_text(&conn, hash, &mut text_cache, &external_paths)? - { + if let Some(text) = load_blob_text(work_conn, hash, &mut text_cache, &external_paths)? { append_findings( &mut findings, entry_id, @@ -305,7 +309,7 @@ pub fn run_pii_with_external_paths( if options.redact { if let Some(redacted) = redact_blob_cached( - &conn, + work_conn, hash, &matchers, &options.token, @@ -322,7 +326,7 @@ pub fn run_pii_with_external_paths( if write { changed_response_hashes.insert(hash.to_string()); if has_fts { - let has_old_fts = conn + let has_old_fts = work_conn .query_row( "SELECT 1 FROM response_body_fts WHERE hash = ?1 LIMIT 1", params![hash], @@ -331,7 +335,11 @@ pub fn run_pii_with_external_paths( .optional()? .is_some(); if has_old_fts { - upsert_response_fts(&conn, &redacted.new_hash, &redacted.text)?; + upsert_response_fts( + work_conn, + &redacted.new_hash, + &redacted.text, + )?; } } } @@ -357,8 +365,8 @@ pub fn run_pii_with_external_paths( if write && has_fts && !changed_response_hashes.is_empty() { let mut check_stmt = - conn.prepare("SELECT COUNT(*) FROM entries WHERE response_body_hash = ?1")?; - let mut delete_stmt = conn.prepare("DELETE FROM response_body_fts WHERE hash = ?1")?; + work_conn.prepare("SELECT COUNT(*) FROM entries WHERE response_body_hash = ?1")?; + let mut delete_stmt = work_conn.prepare("DELETE FROM response_body_fts WHERE hash = ?1")?; for hash in changed_response_hashes { let count: i64 = check_stmt.query_row([hash.as_str()], |row| row.get(0))?; if count == 0 { @@ -370,8 +378,15 @@ pub fn run_pii_with_external_paths( drop(update); drop(stmt); if write { + delete_orphaned_blobs(work_conn)?; + conn.execute_batch("COMMIT")?; finalize_sensitive_write(&conn)?; } + drop(conn); + + if let Some(staged) = staged_output { + staged.publish()?; + } match options.format { OutputFormat::Json => write_json(&findings), diff --git a/src/commands/prune.rs b/src/commands/prune.rs index f20d12e..b8ed74d 100644 --- a/src/commands/prune.rs +++ b/src/commands/prune.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::fs; use std::path::PathBuf; @@ -60,8 +61,7 @@ pub fn run_prune_with_options( )?; let hashes = stmt .query_map(params![import_id], |row| row.get(0))? - .filter_map(|row| row.ok()) - .collect(); + .collect::>>()?; hashes }; @@ -88,6 +88,7 @@ pub fn run_prune_with_options( let mut fts_deleted = 0usize; let mut external_deleted = 0usize; let mut external_skipped = 0usize; + let mut external_delete_candidates: HashSet = HashSet::new(); if !hashes.is_empty() { let has_fts: i64 = tx.query_row( @@ -121,8 +122,7 @@ pub fn run_prune_with_options( let orphan_hashes: Vec = tx .prepare(&sql_orphans)? .query_map(params_vec.as_slice(), |row| row.get(0))? - .filter_map(|row| row.ok()) - .collect(); + .collect::>>()?; if orphan_hashes.is_empty() { continue; @@ -143,19 +143,14 @@ pub fn run_prune_with_options( "SELECT external_path FROM blobs WHERE hash IN ({orphan_placeholders}) AND external_path IS NOT NULL" ))? .query_map(orphan_params.as_slice(), |row| row.get(0))? - .filter_map(|row| row.ok()) - .collect(); + .collect::>>()?; for raw_path in external_paths { let Some(path) = external_path_policy.resolve_file(&raw_path) else { external_skipped += 1; continue; }; - if fs::remove_file(&path).is_ok() { - external_deleted += 1; - } else { - external_skipped += 1; - } + external_delete_candidates.insert(path); } if has_fts > 0 { @@ -169,8 +164,34 @@ pub fn run_prune_with_options( } } + let external_paths_in_use: HashSet = if external_delete_candidates.is_empty() { + HashSet::new() + } else { + let mut stmt = + tx.prepare("SELECT DISTINCT external_path FROM blobs WHERE external_path IS NOT NULL")?; + let rows = stmt.query_map([], |row| row.get::<_, String>(0))?; + let mut paths = HashSet::new(); + for row in rows { + let raw_path = row?; + if let Some(path) = external_path_policy.resolve_file(&raw_path) { + paths.insert(path); + } + } + paths + }; + tx.commit()?; + for path in external_delete_candidates { + if external_paths_in_use.contains(&path) { + external_skipped += 1; + } else if fs::remove_file(&path).is_ok() { + external_deleted += 1; + } else { + external_skipped += 1; + } + } + println!( "Pruned import {import_id} ({source_file}). Removed {imports_deleted} import record, {entries_deleted} entries, {pages_deleted} pages, {blobs_deleted} blobs, {fts_deleted} FTS rows, deleted {external_deleted} external files (skipped {external_skipped})." ); diff --git a/src/commands/redact.rs b/src/commands/redact.rs index a4cef0c..f7b30a3 100644 --- a/src/commands/redact.rs +++ b/src/commands/redact.rs @@ -10,8 +10,8 @@ use crate::db::store_blob; use crate::error::{HarliteError, Result}; use super::util::{ - canonicalize_path_for_compare, copy_database_consistent, finalize_sensitive_write, - prepare_sensitive_write, remove_database_with_sidecars, resolve_database, ExternalPathPolicy, + canonicalize_path_for_compare, delete_orphaned_blobs, finalize_sensitive_write, + prepare_sensitive_write, resolve_database, ExternalPathPolicy, StagedDatabase, }; #[derive(Clone, Copy, Debug, ValueEnum, serde::Serialize, serde::Deserialize)] @@ -771,29 +771,6 @@ pub fn run_redact_with_external_paths( external_path_root, )?; - let target_db = if options.dry_run { - input_db.clone() - } else if let Some(out) = &options.output { - let input_cmp = canonicalize_path_for_compare(&input_db)?; - let out_cmp = canonicalize_path_for_compare(out)?; - if out_cmp == input_cmp { - return Err(HarliteError::InvalidArgs( - "Output database must be different from input database".to_string(), - )); - } - if out.exists() && !options.force { - return Err(HarliteError::InvalidArgs(format!( - "Output database already exists: {} (use --force to overwrite)", - out.display() - ))); - } - remove_database_with_sidecars(out)?; - copy_database_consistent(&input_db, out)?; - out.clone() - } else { - input_db.clone() - }; - let mut header_patterns: Vec = Vec::new(); let mut cookie_patterns: Vec = Vec::new(); let mut query_patterns: Vec = Vec::new(); @@ -832,6 +809,31 @@ pub fn run_redact_with_external_paths( ))); } + let staged_output = if options.dry_run { + None + } else if let Some(out) = &options.output { + let input_cmp = canonicalize_path_for_compare(&input_db)?; + let out_cmp = canonicalize_path_for_compare(out)?; + if out_cmp == input_cmp { + return Err(HarliteError::InvalidArgs( + "Output database must be different from input database".to_string(), + )); + } + if out.exists() && !options.force { + return Err(HarliteError::InvalidArgs(format!( + "Output database already exists: {} (use --force to overwrite)", + out.display() + ))); + } + Some(StagedDatabase::copy_from(&input_db, out, options.force)?) + } else { + None + }; + let target_db = staged_output + .as_ref() + .map(|staged| staged.path().to_path_buf()) + .unwrap_or_else(|| input_db.clone()); + let mut conn = if options.dry_run { super::query::open_readonly_connection(&target_db)? } else { @@ -863,10 +865,18 @@ pub fn run_redact_with_external_paths( true, &external_paths, )?; + delete_orphaned_blobs(&tx)?; tx.commit()?; finalize_sensitive_write(&conn)?; report }; + drop(conn); + + let result_db = if let Some(staged) = staged_output { + staged.publish()? + } else { + target_db + }; if options.dry_run { println!( @@ -885,7 +895,7 @@ pub fn run_redact_with_external_paths( "Redacted {} values across {} entries in {}", report.total(), report.entries_changed, - target_db.display() + result_db.display() ); } diff --git a/src/commands/util.rs b/src/commands/util.rs index 8b1dc60..9d2751f 100644 --- a/src/commands/util.rs +++ b/src/commands/util.rs @@ -1,5 +1,6 @@ -use std::fs; +use std::fs::{self, OpenOptions}; use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; use chrono::{DateTime, NaiveDate, TimeZone, Utc}; use rusqlite::{Connection, DatabaseName, OpenFlags}; @@ -74,14 +75,205 @@ pub fn copy_database_consistent(source: &Path, destination: &Path) -> Result<()> Ok(()) } +static NEXT_STAGED_DATABASE_ID: AtomicU64 = AtomicU64::new(0); + +/// A consistent database copy staged beside its final destination. +/// +/// The staged database and its sidecars are removed on error or early return. +/// Publishing uses a same-directory rename so callers never expose an +/// unredacted source copy at the requested output path. +pub struct StagedDatabase { + path: PathBuf, + destination: PathBuf, + overwrite: bool, + published: bool, +} + +struct DestinationBackup { + destination: PathBuf, + directory: PathBuf, + moved_main: bool, + moved_sidecars: Vec<&'static str>, + active: bool, +} + +impl StagedDatabase { + pub fn copy_from(source: &Path, destination: &Path, overwrite: bool) -> Result { + let parent = destination + .parent() + .filter(|parent| !parent.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + let file_name = destination + .file_name() + .ok_or_else(|| HarliteError::InvalidArgs("Output path must be a file".to_string()))? + .to_string_lossy(); + + let path = loop { + let id = NEXT_STAGED_DATABASE_ID.fetch_add(1, Ordering::Relaxed); + let candidate = parent.join(format!( + ".{file_name}.harlite-stage-{}-{id}", + std::process::id() + )); + match OpenOptions::new() + .read(true) + .write(true) + .create_new(true) + .open(&candidate) + { + Ok(file) => { + drop(file); + break candidate; + } + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(err) => return Err(err.into()), + } + }; + + let staged = Self { + path, + destination: destination.to_path_buf(), + overwrite, + published: false, + }; + copy_database_consistent(source, &staged.path)?; + Ok(staged) + } + + pub fn path(&self) -> &Path { + &self.path + } + + pub fn publish(mut self) -> Result { + if self.destination.exists() && !self.destination.is_file() { + return Err(HarliteError::InvalidArgs(format!( + "Output path must be a file: {}", + self.destination.display() + ))); + } + if self.destination.exists() && !self.overwrite { + return Err(HarliteError::InvalidArgs(format!( + "Output database already exists: {} (use --force to overwrite)", + self.destination.display() + ))); + } + + remove_database_sidecars(&self.path)?; + let has_destination_files = self.destination.exists() + || database_sidecar_suffixes() + .iter() + .any(|suffix| database_sidecar_path(&self.destination, suffix).exists()); + let backup = if has_destination_files { + let mut backup = DestinationBackup::new(&self.destination)?; + backup.capture()?; + Some(backup) + } else { + None + }; + + fs::rename(&self.path, &self.destination)?; + if let Some(backup) = backup { + backup.discard(); + } + self.published = true; + Ok(self.destination.clone()) + } +} + +impl Drop for StagedDatabase { + fn drop(&mut self) { + if !self.published { + let _ = remove_database_with_sidecars(&self.path); + } + } +} + +impl DestinationBackup { + fn new(destination: &Path) -> Result { + let parent = destination + .parent() + .filter(|parent| !parent.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + let file_name = destination + .file_name() + .ok_or_else(|| HarliteError::InvalidArgs("Output path must be a file".to_string()))? + .to_string_lossy(); + let directory = loop { + let id = NEXT_STAGED_DATABASE_ID.fetch_add(1, Ordering::Relaxed); + let candidate = parent.join(format!( + ".{file_name}.harlite-backup-{}-{id}", + std::process::id() + )); + match fs::create_dir(&candidate) { + Ok(()) => break candidate, + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(err) => return Err(err.into()), + } + }; + + Ok(Self { + destination: destination.to_path_buf(), + directory, + moved_main: false, + moved_sidecars: Vec::new(), + active: true, + }) + } + + fn capture(&mut self) -> Result<()> { + if self.destination.exists() { + fs::rename(&self.destination, self.directory.join("database"))?; + self.moved_main = true; + } + for suffix in database_sidecar_suffixes() { + let source = database_sidecar_path(&self.destination, suffix); + if source.exists() { + fs::rename(&source, self.directory.join(format!("database{suffix}")))?; + self.moved_sidecars.push(suffix); + } + } + Ok(()) + } + + fn restore(&mut self) -> Result<()> { + while let Some(suffix) = self.moved_sidecars.pop() { + fs::rename( + self.directory.join(format!("database{suffix}")), + database_sidecar_path(&self.destination, suffix), + )?; + } + if self.moved_main { + fs::rename(self.directory.join("database"), &self.destination)?; + self.moved_main = false; + } + fs::remove_dir(&self.directory)?; + self.active = false; + Ok(()) + } + + fn discard(mut self) { + self.active = false; + let _ = fs::remove_dir_all(&self.directory); + } +} + +impl Drop for DestinationBackup { + fn drop(&mut self) { + if self.active { + let _ = self.restore(); + } + } +} + pub fn remove_database_with_sidecars(database: &Path) -> Result<()> { if database.exists() { fs::remove_file(database)?; } - for suffix in ["-wal", "-shm"] { - let mut sidecar = database.as_os_str().to_os_string(); - sidecar.push(suffix); - let sidecar = PathBuf::from(sidecar); + remove_database_sidecars(database) +} + +fn remove_database_sidecars(database: &Path) -> Result<()> { + for suffix in database_sidecar_suffixes() { + let sidecar = database_sidecar_path(database, suffix); if sidecar.exists() { fs::remove_file(sidecar)?; } @@ -89,6 +281,16 @@ pub fn remove_database_with_sidecars(database: &Path) -> Result<()> { Ok(()) } +fn database_sidecar_suffixes() -> [&'static str; 3] { + ["-journal", "-wal", "-shm"] +} + +fn database_sidecar_path(database: &Path, suffix: &str) -> PathBuf { + let mut sidecar = database.as_os_str().to_os_string(); + sidecar.push(suffix); + PathBuf::from(sidecar) +} + /// Remove blobs and FTS rows no longer referenced by any entry. pub fn delete_orphaned_blobs(conn: &Connection) -> Result { let has_fts: bool = conn.query_row( @@ -116,7 +318,6 @@ pub fn prepare_sensitive_write(conn: &Connection) -> Result<()> { } pub fn finalize_sensitive_write(conn: &Connection) -> Result<()> { - delete_orphaned_blobs(conn)?; conn.execute_batch( "PRAGMA wal_checkpoint(TRUNCATE); VACUUM; PRAGMA wal_checkpoint(TRUNCATE);", )?; @@ -214,7 +415,10 @@ fn resolve_database_in_dir(dir: &Path) -> Result { #[cfg(test)] mod tests { - use super::{resolve_database_in_dir, ExternalPathPolicy}; + use super::{ + remove_database_with_sidecars, resolve_database_in_dir, DestinationBackup, + ExternalPathPolicy, StagedDatabase, + }; use crate::error::HarliteError; use tempfile::TempDir; @@ -228,6 +432,120 @@ mod tests { assert_eq!(resolved, db_path); } + #[test] + fn remove_database_cleans_sqlite_sidecars() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("staged.db"); + std::fs::write(&db_path, b"database").unwrap(); + for suffix in ["-journal", "-wal", "-shm"] { + std::fs::write(tmp.path().join(format!("staged.db{suffix}")), b"sidecar").unwrap(); + } + + remove_database_with_sidecars(&db_path).unwrap(); + + assert!(!db_path.exists()); + for suffix in ["-journal", "-wal", "-shm"] { + assert!(!tmp.path().join(format!("staged.db{suffix}")).exists()); + } + } + + #[test] + fn destination_backup_restores_database_and_sidecars_on_drop() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("existing.db"); + std::fs::write(&db_path, b"database").unwrap(); + for suffix in ["-journal", "-wal", "-shm"] { + std::fs::write(tmp.path().join(format!("existing.db{suffix}")), suffix).unwrap(); + } + + { + let mut backup = DestinationBackup::new(&db_path).unwrap(); + backup.capture().unwrap(); + assert!(!db_path.exists()); + assert!(!tmp.path().join("existing.db-wal").exists()); + } + + assert_eq!(std::fs::read(&db_path).unwrap(), b"database"); + for suffix in ["-journal", "-wal", "-shm"] { + assert_eq!( + std::fs::read(tmp.path().join(format!("existing.db{suffix}"))).unwrap(), + suffix.as_bytes() + ); + } + } + + #[test] + fn staged_database_replaces_existing_database_and_sidecars() { + let tmp = TempDir::new().unwrap(); + let source = tmp.path().join("source.db"); + let destination = tmp.path().join("destination.db"); + let conn = rusqlite::Connection::open(&source).unwrap(); + conn.execute_batch("CREATE TABLE value (number INTEGER); INSERT INTO value VALUES (42);") + .unwrap(); + drop(conn); + std::fs::write(&destination, b"previous database").unwrap(); + for suffix in ["-journal", "-wal", "-shm"] { + std::fs::write( + tmp.path().join(format!("destination.db{suffix}")), + b"previous sidecar", + ) + .unwrap(); + } + + let staged = StagedDatabase::copy_from(&source, &destination, true).unwrap(); + staged.publish().unwrap(); + + let conn = rusqlite::Connection::open(&destination).unwrap(); + let value: i64 = conn + .query_row("SELECT number FROM value", [], |row| row.get(0)) + .unwrap(); + assert_eq!(value, 42); + for suffix in ["-journal", "-wal", "-shm"] { + assert!(!tmp.path().join(format!("destination.db{suffix}")).exists()); + } + assert!(!std::fs::read_dir(tmp.path()).unwrap().any(|entry| entry + .unwrap() + .file_name() + .to_string_lossy() + .contains("harlite-backup"))); + } + + #[test] + fn staged_database_restores_existing_files_when_install_fails() { + let tmp = TempDir::new().unwrap(); + let source = tmp.path().join("source.db"); + let destination = tmp.path().join("destination.db"); + let conn = rusqlite::Connection::open(&source).unwrap(); + conn.execute_batch("CREATE TABLE value (number INTEGER);") + .unwrap(); + drop(conn); + std::fs::write(&destination, b"previous database").unwrap(); + for suffix in ["-journal", "-wal", "-shm"] { + std::fs::write( + tmp.path().join(format!("destination.db{suffix}")), + suffix.as_bytes(), + ) + .unwrap(); + } + + let staged = StagedDatabase::copy_from(&source, &destination, true).unwrap(); + std::fs::remove_file(staged.path()).unwrap(); + assert!(staged.publish().is_err()); + + assert_eq!(std::fs::read(&destination).unwrap(), b"previous database"); + for suffix in ["-journal", "-wal", "-shm"] { + assert_eq!( + std::fs::read(tmp.path().join(format!("destination.db{suffix}"))).unwrap(), + suffix.as_bytes() + ); + } + assert!(!std::fs::read_dir(tmp.path()).unwrap().any(|entry| entry + .unwrap() + .file_name() + .to_string_lossy() + .contains("harlite-backup"))); + } + #[test] fn resolve_database_in_dir_errors_when_missing() { let tmp = TempDir::new().unwrap(); diff --git a/tests/integration.rs b/tests/integration.rs index 7dcd5b4..b374464 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -217,6 +217,323 @@ fn test_prune_does_not_delete_untrusted_external_path_by_default() { assert!(protected_path.exists()); } +#[test] +fn test_prune_external_file_deletion_waits_for_commit() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("test.db"); + let bodies_dir = tmp.path().join("bodies"); + + harlite() + .args([ + "import", + "tests/fixtures/simple.har", + "--bodies", + "--extract-bodies", + ]) + .arg(&bodies_dir) + .args(["-o"]) + .arg(&db_path) + .assert() + .success(); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let import_id: i64 = conn + .query_row("SELECT id FROM imports LIMIT 1", [], |row| row.get(0)) + .unwrap(); + let external_paths: Vec = conn + .prepare("SELECT external_path FROM blobs WHERE external_path IS NOT NULL") + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .map(|row| row.unwrap()) + .collect(); + assert!(!external_paths.is_empty()); + conn.execute_batch( + "CREATE TRIGGER fail_blob_delete BEFORE DELETE ON blobs BEGIN SELECT RAISE(ABORT, 'blocked'); END;", + ) + .unwrap(); + drop(conn); + + harlite() + .args([ + "prune", + "--import-id", + &import_id.to_string(), + "--allow-external-paths", + "--external-path-root", + ]) + .arg(&bodies_dir) + .arg(&db_path) + .assert() + .failure() + .stderr(predicate::str::contains("blocked")); + + assert!(external_paths + .iter() + .all(|path| std::path::Path::new(path).exists())); + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let entries: i64 = conn + .query_row("SELECT COUNT(*) FROM entries", [], |row| row.get(0)) + .unwrap(); + assert_eq!(entries, 2); +} + +#[test] +fn test_prune_preserves_external_file_referenced_by_surviving_blob() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("test.db"); + let bodies_dir = tmp.path().join("bodies"); + let shared_path = bodies_dir.join("shared-body"); + fs::create_dir(&bodies_dir).unwrap(); + fs::write(&shared_path, b"shared").unwrap(); + + for _ in 0..2 { + harlite() + .args(["import", "tests/fixtures/simple.har", "--bodies", "-o"]) + .arg(&db_path) + .assert() + .success(); + } + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let import_ids: Vec = conn + .prepare("SELECT id FROM imports ORDER BY id") + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .map(|row| row.unwrap()) + .collect(); + assert_eq!(import_ids.len(), 2); + let shared = shared_path.to_string_lossy(); + conn.execute( + "INSERT INTO blobs(hash, content, size, mime_type, external_path) VALUES('orphan', X'', 6, 'text/plain', ?1)", + [shared.as_ref()], + ) + .unwrap(); + conn.execute( + "INSERT INTO blobs(hash, content, size, mime_type, external_path) VALUES('keeper', X'', 6, 'text/plain', ?1)", + [shared.as_ref()], + ) + .unwrap(); + conn.execute( + "UPDATE entries SET request_body_hash='orphan', request_body_size=6 WHERE id=(SELECT MIN(id) FROM entries WHERE import_id=?1)", + [import_ids[0]], + ) + .unwrap(); + conn.execute( + "UPDATE entries SET request_body_hash='keeper', request_body_size=6 WHERE id=(SELECT MIN(id) FROM entries WHERE import_id=?1)", + [import_ids[1]], + ) + .unwrap(); + drop(conn); + + harlite() + .args([ + "prune", + "--import-id", + &import_ids[0].to_string(), + "--allow-external-paths", + "--external-path-root", + ]) + .arg(&bodies_dir) + .arg(&db_path) + .assert() + .success(); + + assert!(shared_path.exists()); + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let surviving_refs: i64 = conn + .query_row( + "SELECT COUNT(*) FROM entries WHERE request_body_hash='keeper'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(surviving_refs, 1); +} + +#[test] +fn test_prune_rolls_back_on_malformed_surviving_external_path() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("test.db"); + let bodies_dir = tmp.path().join("bodies"); + let shared_path = bodies_dir.join("shared-body"); + fs::create_dir(&bodies_dir).unwrap(); + fs::write(&shared_path, b"shared").unwrap(); + + for _ in 0..2 { + harlite() + .args(["import", "tests/fixtures/simple.har", "--bodies", "-o"]) + .arg(&db_path) + .assert() + .success(); + } + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let import_ids: Vec = conn + .prepare("SELECT id FROM imports ORDER BY id") + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .map(|row| row.unwrap()) + .collect(); + let shared = shared_path.to_string_lossy(); + conn.execute( + "INSERT INTO blobs(hash, content, size, mime_type, external_path) VALUES('orphan', X'', 6, 'text/plain', ?1)", + [shared.as_ref()], + ) + .unwrap(); + conn.execute( + "INSERT INTO blobs(hash, content, size, mime_type, external_path) VALUES('keeper', X'', 6, 'text/plain', CAST(?1 AS BLOB))", + [shared.as_ref()], + ) + .unwrap(); + conn.execute( + "UPDATE entries SET request_body_hash='orphan', request_body_size=6 WHERE id=(SELECT MIN(id) FROM entries WHERE import_id=?1)", + [import_ids[0]], + ) + .unwrap(); + conn.execute( + "UPDATE entries SET request_body_hash='keeper', request_body_size=6 WHERE id=(SELECT MIN(id) FROM entries WHERE import_id=?1)", + [import_ids[1]], + ) + .unwrap(); + drop(conn); + + harlite() + .args([ + "prune", + "--import-id", + &import_ids[0].to_string(), + "--allow-external-paths", + "--external-path-root", + ]) + .arg(&bodies_dir) + .arg(&db_path) + .assert() + .failure(); + + assert!(shared_path.exists()); + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let import_count: i64 = conn + .query_row("SELECT COUNT(*) FROM imports", [], |row| row.get(0)) + .unwrap(); + let orphan_count: i64 = conn + .query_row( + "SELECT COUNT(*) FROM blobs WHERE hash='orphan'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(import_count, 2); + assert_eq!(orphan_count, 1); +} + +#[test] +fn test_prune_rolls_back_on_malformed_pruned_external_path() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("test.db"); + let bodies_dir = tmp.path().join("bodies"); + let external_path = bodies_dir.join("external-body"); + fs::create_dir(&bodies_dir).unwrap(); + fs::write(&external_path, b"external").unwrap(); + + harlite() + .args(["import", "tests/fixtures/simple.har", "--bodies", "-o"]) + .arg(&db_path) + .assert() + .success(); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let import_id: i64 = conn + .query_row("SELECT id FROM imports LIMIT 1", [], |row| row.get(0)) + .unwrap(); + let external = external_path.to_string_lossy(); + conn.execute( + "INSERT INTO blobs(hash, content, size, mime_type, external_path) VALUES('malformed-path', X'', 8, 'text/plain', CAST(?1 AS BLOB))", + [external.as_ref()], + ) + .unwrap(); + conn.execute( + "UPDATE entries SET request_body_hash='malformed-path', request_body_size=8 WHERE id=(SELECT MIN(id) FROM entries)", + [], + ) + .unwrap(); + drop(conn); + + harlite() + .args([ + "prune", + "--import-id", + &import_id.to_string(), + "--allow-external-paths", + "--external-path-root", + ]) + .arg(&bodies_dir) + .arg(&db_path) + .assert() + .failure(); + + assert!(external_path.exists()); + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let import_count: i64 = conn + .query_row("SELECT COUNT(*) FROM imports", [], |row| row.get(0)) + .unwrap(); + let blob_count: i64 = conn + .query_row( + "SELECT COUNT(*) FROM blobs WHERE hash='malformed-path'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(import_count, 1); + assert_eq!(blob_count, 1); +} + +#[test] +fn test_prune_rolls_back_on_malformed_entry_hash() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("test.db"); + + harlite() + .args(["import", "tests/fixtures/simple.har", "--bodies", "-o"]) + .arg(&db_path) + .assert() + .success(); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let import_id: i64 = conn + .query_row("SELECT id FROM imports LIMIT 1", [], |row| row.get(0)) + .unwrap(); + conn.execute( + "INSERT INTO blobs(hash, content, size, mime_type) VALUES(CAST('malformed-hash' AS BLOB), X'', 0, 'text/plain')", + [], + ) + .unwrap(); + conn.execute( + "UPDATE entries SET request_body_hash=CAST('malformed-hash' AS BLOB) WHERE id=(SELECT MIN(id) FROM entries)", + [], + ) + .unwrap(); + drop(conn); + + harlite() + .args(["prune", "--import-id", &import_id.to_string()]) + .arg(&db_path) + .assert() + .failure(); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let import_count: i64 = conn + .query_row("SELECT COUNT(*) FROM imports", [], |row| row.get(0)) + .unwrap(); + let entry_count: i64 = conn + .query_row("SELECT COUNT(*) FROM entries", [], |row| row.get(0)) + .unwrap(); + assert_eq!(import_count, 1); + assert_eq!(entry_count, 2); +} + #[cfg(unix)] #[test] fn test_project_config_cannot_implicitly_execute_plugin() { @@ -1554,6 +1871,7 @@ fn test_pii_redaction_physically_removes_superseded_body() { let db_path = tmp.path().join("source.db"); let out_path = tmp.path().join("redacted.db"); let secret = "physical-secret@example.com"; + fs::write(&out_path, b"previous output").unwrap(); harlite() .args(["import", "tests/fixtures/redact.har", "--bodies", "-o"]) @@ -1569,7 +1887,7 @@ fn test_pii_redaction_physically_removes_superseded_body() { drop(conn); harlite() - .args(["pii", "--redact", "--format", "json", "--output"]) + .args(["pii", "--redact", "--format", "json", "--force", "--output"]) .arg(&out_path) .arg(&db_path) .assert() @@ -1590,6 +1908,273 @@ fn test_pii_redaction_physically_removes_superseded_body() { assert!(!raw.windows(secret.len()).any(|window| window == secret.as_bytes())); } +#[test] +fn test_pii_redaction_rolls_back_every_entry_on_failure() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("source.db"); + + harlite() + .args(["import", "tests/fixtures/simple.har", "--bodies", "-o"]) + .arg(&db_path) + .assert() + .success(); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let original_hashes: Vec = conn + .prepare("SELECT response_body_hash FROM entries ORDER BY id") + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .map(|row| row.unwrap()) + .collect(); + assert_eq!(original_hashes.len(), 2); + conn.execute( + "UPDATE blobs SET content=?1, size=?2 WHERE hash=?3", + rusqlite::params![ + b"first@example.com".as_slice(), + "first@example.com".len() as i64, + original_hashes[0] + ], + ) + .unwrap(); + conn.execute( + "UPDATE blobs SET content=?1, size=?2 WHERE hash=?3", + rusqlite::params![ + b"second@example.com".as_slice(), + "second@example.com".len() as i64, + original_hashes[1] + ], + ) + .unwrap(); + let blobs_before: i64 = conn + .query_row("SELECT COUNT(*) FROM blobs", [], |row| row.get(0)) + .unwrap(); + conn.execute_batch( + "CREATE TRIGGER fail_second_pii_update BEFORE UPDATE ON entries WHEN OLD.id=2 BEGIN SELECT RAISE(ABORT, 'blocked'); END;", + ) + .unwrap(); + drop(conn); + + harlite() + .args(["pii", "--redact", "--format", "json"]) + .arg(&db_path) + .assert() + .failure() + .stderr(predicate::str::contains("blocked")); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let hashes_after: Vec = conn + .prepare("SELECT response_body_hash FROM entries ORDER BY id") + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .map(|row| row.unwrap()) + .collect(); + let blobs_after: i64 = conn + .query_row("SELECT COUNT(*) FROM blobs", [], |row| row.get(0)) + .unwrap(); + assert_eq!(hashes_after, original_hashes); + assert_eq!(blobs_after, blobs_before); +} + +#[test] +fn test_pii_redaction_rolls_back_when_orphan_cleanup_fails() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("source.db"); + + harlite() + .args(["import", "tests/fixtures/simple.har", "--bodies", "-o"]) + .arg(&db_path) + .assert() + .success(); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let original_hash: String = conn + .query_row( + "SELECT response_body_hash FROM entries ORDER BY id LIMIT 1", + [], + |row| row.get(0), + ) + .unwrap(); + conn.execute( + "UPDATE blobs SET content=?1, size=?2 WHERE hash=?3", + rusqlite::params![ + b"cleanup@example.com".as_slice(), + "cleanup@example.com".len() as i64, + original_hash.as_str() + ], + ) + .unwrap(); + let blobs_before: i64 = conn + .query_row("SELECT COUNT(*) FROM blobs", [], |row| row.get(0)) + .unwrap(); + conn.execute_batch( + "CREATE TRIGGER fail_pii_blob_cleanup BEFORE DELETE ON blobs BEGIN SELECT RAISE(ABORT, 'cleanup blocked'); END;", + ) + .unwrap(); + drop(conn); + + harlite() + .args(["pii", "--redact", "--format", "json"]) + .arg(&db_path) + .assert() + .failure() + .stderr(predicate::str::contains("cleanup blocked")); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let hash_after: String = conn + .query_row( + "SELECT response_body_hash FROM entries ORDER BY id LIMIT 1", + [], + |row| row.get(0), + ) + .unwrap(); + let blobs_after: i64 = conn + .query_row("SELECT COUNT(*) FROM blobs", [], |row| row.get(0)) + .unwrap(); + assert_eq!(hash_after, original_hash); + assert_eq!(blobs_after, blobs_before); +} + +#[test] +fn test_redact_rolls_back_when_orphan_cleanup_fails() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("source.db"); + + harlite() + .args(["import", "tests/fixtures/redact.har", "--bodies", "-o"]) + .arg(&db_path) + .assert() + .success(); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let original: (String, String) = conn + .query_row( + "SELECT request_headers, response_body_hash FROM entries LIMIT 1", + [], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .unwrap(); + let blobs_before: i64 = conn + .query_row("SELECT COUNT(*) FROM blobs", [], |row| row.get(0)) + .unwrap(); + conn.execute_batch( + "CREATE TRIGGER fail_redact_blob_cleanup BEFORE DELETE ON blobs BEGIN SELECT RAISE(ABORT, 'cleanup blocked'); END;", + ) + .unwrap(); + drop(conn); + + harlite() + .args(["redact", "--body-regex", "ok"]) + .arg(&db_path) + .assert() + .failure() + .stderr(predicate::str::contains("cleanup blocked")); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let after: (String, String) = conn + .query_row( + "SELECT request_headers, response_body_hash FROM entries LIMIT 1", + [], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .unwrap(); + let blobs_after: i64 = conn + .query_row("SELECT COUNT(*) FROM blobs", [], |row| row.get(0)) + .unwrap(); + assert_eq!(after, original); + assert_eq!(blobs_after, blobs_before); +} + +#[test] +fn test_redact_failures_do_not_publish_output_database() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("source.db"); + let invalid_output = tmp.path().join("invalid-pattern.db"); + let runtime_output = tmp.path().join("runtime-failure.db"); + let previous_output = b"previous output"; + + harlite() + .args(["import", "tests/fixtures/redact.har", "--bodies", "-o"]) + .arg(&db_path) + .assert() + .success(); + + fs::write(&invalid_output, previous_output).unwrap(); + harlite() + .args(["redact", "--body-regex", "[", "--force", "--output"]) + .arg(&invalid_output) + .arg(&db_path) + .assert() + .failure(); + assert_eq!(fs::read(&invalid_output).unwrap(), previous_output); + + let conn = rusqlite::Connection::open(&db_path).unwrap(); + conn.execute_batch( + "CREATE TRIGGER fail_redact_update BEFORE UPDATE ON entries BEGIN SELECT RAISE(ABORT, 'blocked'); END;", + ) + .unwrap(); + drop(conn); + fs::write(&runtime_output, previous_output).unwrap(); + harlite() + .args(["redact", "--force", "--output"]) + .arg(&runtime_output) + .arg(&db_path) + .assert() + .failure() + .stderr(predicate::str::contains("blocked")); + assert_eq!(fs::read(&runtime_output).unwrap(), previous_output); + assert!(!fs::read_dir(tmp.path()).unwrap().any(|entry| { + entry + .unwrap() + .file_name() + .to_string_lossy() + .contains("harlite-stage") + })); +} + +#[test] +fn test_pii_failure_does_not_publish_output_database() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("source.db"); + let out_path = tmp.path().join("redacted.db"); + let previous_output = b"previous output"; + + harlite() + .args(["import", "tests/fixtures/simple.har", "--bodies", "-o"]) + .arg(&db_path) + .assert() + .success(); + let conn = rusqlite::Connection::open(&db_path).unwrap(); + conn.execute( + "UPDATE blobs SET content=?1, size=?2 WHERE hash=(SELECT response_body_hash FROM entries ORDER BY id LIMIT 1)", + rusqlite::params![b"secret@example.com".as_slice(), "secret@example.com".len() as i64], + ) + .unwrap(); + conn.execute_batch( + "CREATE TRIGGER fail_pii_update BEFORE UPDATE ON entries BEGIN SELECT RAISE(ABORT, 'blocked'); END;", + ) + .unwrap(); + drop(conn); + fs::write(&out_path, previous_output).unwrap(); + + harlite() + .args(["pii", "--redact", "--format", "json", "--force", "--output"]) + .arg(&out_path) + .arg(&db_path) + .assert() + .failure() + .stderr(predicate::str::contains("blocked")); + assert_eq!(fs::read(&out_path).unwrap(), previous_output); + assert!(!fs::read_dir(tmp.path()).unwrap().any(|entry| { + entry + .unwrap() + .file_name() + .to_string_lossy() + .contains("harlite-stage") + })); +} + #[test] fn test_query_limit_offset_wraps_query() { let tmp = TempDir::new().unwrap();