From 3b42bf7015b97d60cfaa103955b0e06b54f77b18 Mon Sep 17 00:00:00 2001 From: "Bruce J. Hart" Date: Mon, 13 Jul 2026 16:41:33 -0400 Subject: [PATCH 1/6] Fix pruning of GraphQL entry fields --- src/commands/prune.rs | 12 ++++++++++++ tests/integration.rs | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/commands/prune.rs b/src/commands/prune.rs index a5b3ccc..f20d12e 100644 --- a/src/commands/prune.rs +++ b/src/commands/prune.rs @@ -65,6 +65,18 @@ pub fn run_prune_with_options( hashes }; + let has_graphql_fields: bool = tx.query_row( + "SELECT EXISTS(SELECT 1 FROM sqlite_master WHERE type='table' AND name='graphql_fields')", + [], + |row| row.get(0), + )?; + if has_graphql_fields { + tx.execute( + "DELETE FROM graphql_fields WHERE entry_id IN (SELECT id FROM entries WHERE import_id = ?1)", + params![import_id], + )?; + } + let entries_deleted = tx.execute( "DELETE FROM entries WHERE import_id = ?1", params![import_id], diff --git a/tests/integration.rs b/tests/integration.rs index d145c04..7dcd5b4 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -146,6 +146,14 @@ fn test_imports_list_and_prune() { let import_id: i64 = conn .query_row("SELECT id FROM imports LIMIT 1", [], |r| r.get(0)) .unwrap(); + let graphql_fields_inserted = conn + .execute( + "INSERT INTO graphql_fields (entry_id, field) SELECT id, 'viewer' FROM entries WHERE import_id = ?1 LIMIT 1", + [import_id], + ) + .unwrap(); + assert_eq!(graphql_fields_inserted, 1); + drop(conn); harlite() .args(["prune", "--import-id", &import_id.to_string()]) @@ -166,11 +174,15 @@ fn test_imports_list_and_prune() { let blob_count: i64 = conn .query_row("SELECT COUNT(*) FROM blobs", [], |r| r.get(0)) .unwrap(); + let graphql_field_count: i64 = conn + .query_row("SELECT COUNT(*) FROM graphql_fields", [], |r| r.get(0)) + .unwrap(); assert_eq!(entry_count, 0); assert_eq!(import_count, 0); assert_eq!(page_count, 0); assert_eq!(blob_count, 0); + assert_eq!(graphql_field_count, 0); } #[test] From e987692da92ceb3f34ad7eb9d682d0cbd192ba6a Mon Sep 17 00:00:00 2001 From: "Bruce J. Hart" Date: Mon, 13 Jul 2026 17:20:06 -0400 Subject: [PATCH 2/6] Make sensitive rewrites and prune cleanup atomic Stage output database copies until redaction completes so failures preserve any existing destination. Wrap PII entry rewrites in a single transaction, defer prune filesystem deletion until after commit, and retain external files still referenced by surviving blobs. Add regressions for rollback, publication failure, overwrite, and shared external paths. --- src/commands/pii.rs | 76 ++++++----- src/commands/prune.rs | 29 +++- src/commands/redact.rs | 61 +++++---- src/commands/util.rs | 101 +++++++++++++- tests/integration.rs | 296 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 499 insertions(+), 64 deletions(-) diff --git a/src/commands/pii.rs b/src/commands/pii.rs index ce07f6a..6d4c0b1 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, 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,14 @@ pub fn run_pii_with_external_paths( drop(update); drop(stmt); if write { + 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..b076fba 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; @@ -88,6 +89,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( @@ -151,11 +153,7 @@ pub fn run_prune_with_options( 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 +167,29 @@ 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))?; + rows.filter_map(|row| row.ok()) + .filter_map(|raw_path| external_path_policy.resolve_file(&raw_path)) + .collect() + }; + 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..7fc5a01 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, 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 { @@ -867,6 +869,13 @@ pub fn run_redact_with_external_paths( 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 +894,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..42eea25 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,10 +75,108 @@ 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, +} + +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.overwrite { + return Err(HarliteError::InvalidArgs(format!( + "Output database already exists: {} (use --force to overwrite)", + self.destination.display() + ))); + } + + remove_database_sidecars(&self.path)?; + remove_database_sidecars(&self.destination)?; + if let Err(err) = fs::rename(&self.path, &self.destination) { + // Windows does not replace an existing file with rename. Preserve + // atomic replacement where the platform supports it, then fall + // back to remove-and-rename only for an explicit overwrite. + if self.overwrite && self.destination.exists() { + fs::remove_file(&self.destination)?; + fs::rename(&self.path, &self.destination)?; + } else { + return Err(err.into()); + } + } + 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); + } + } +} + pub fn remove_database_with_sidecars(database: &Path) -> Result<()> { if database.exists() { fs::remove_file(database)?; } + remove_database_sidecars(database) +} + +fn remove_database_sidecars(database: &Path) -> Result<()> { for suffix in ["-wal", "-shm"] { let mut sidecar = database.as_os_str().to_os_string(); sidecar.push(suffix); diff --git a/tests/integration.rs b/tests/integration.rs index 7dcd5b4..a2be17b 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -217,6 +217,141 @@ 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); +} + #[cfg(unix)] #[test] fn test_project_config_cannot_implicitly_execute_plugin() { @@ -1554,6 +1689,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 +1705,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 +1726,164 @@ 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_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(); From abd0db304a0d20127f326e3638f095f56a41575a Mon Sep 17 00:00:00 2001 From: "Bruce J. Hart" Date: Mon, 13 Jul 2026 17:26:11 -0400 Subject: [PATCH 3/6] Fail closed on malformed prune paths Propagate SQLite decoding errors while protecting surviving external blob paths so malformed databases roll back before filesystem deletion. Clean rollback-journal sidecars with staged databases and add regressions for both cases. --- src/commands/prune.rs | 11 +++++-- src/commands/util.rs | 21 ++++++++++-- tests/integration.rs | 77 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 5 deletions(-) diff --git a/src/commands/prune.rs b/src/commands/prune.rs index b076fba..398ac84 100644 --- a/src/commands/prune.rs +++ b/src/commands/prune.rs @@ -173,9 +173,14 @@ pub fn run_prune_with_options( 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))?; - rows.filter_map(|row| row.ok()) - .filter_map(|raw_path| external_path_policy.resolve_file(&raw_path)) - .collect() + 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()?; diff --git a/src/commands/util.rs b/src/commands/util.rs index 42eea25..0b07a77 100644 --- a/src/commands/util.rs +++ b/src/commands/util.rs @@ -177,7 +177,7 @@ pub fn remove_database_with_sidecars(database: &Path) -> Result<()> { } fn remove_database_sidecars(database: &Path) -> Result<()> { - for suffix in ["-wal", "-shm"] { + for suffix in ["-journal", "-wal", "-shm"] { let mut sidecar = database.as_os_str().to_os_string(); sidecar.push(suffix); let sidecar = PathBuf::from(sidecar); @@ -313,7 +313,7 @@ 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, ExternalPathPolicy}; use crate::error::HarliteError; use tempfile::TempDir; @@ -327,6 +327,23 @@ 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 resolve_database_in_dir_errors_when_missing() { let tmp = TempDir::new().unwrap(); diff --git a/tests/integration.rs b/tests/integration.rs index a2be17b..4cc80f4 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -352,6 +352,83 @@ fn test_prune_preserves_external_file_referenced_by_surviving_blob() { 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); +} + #[cfg(unix)] #[test] fn test_project_config_cannot_implicitly_execute_plugin() { From 54edfbc74e64a8d70818043909a68a2c253fd54e Mon Sep 17 00:00:00 2001 From: "Bruce J. Hart" Date: Mon, 13 Jul 2026 17:33:51 -0400 Subject: [PATCH 4/6] Preserve outputs across final cleanup failures Back up an existing destination database and its SQLite sidecars as a recoverable set before publishing staged output. Move orphaned-blob cleanup into the PII and redact transactions so cleanup errors roll back every logical rewrite. Add restore, replacement, and cleanup-failure regressions. --- src/commands/pii.rs | 5 +- src/commands/redact.rs | 5 +- src/commands/util.rs | 200 +++++++++++++++++++++++++++++++++++++---- tests/integration.rs | 109 ++++++++++++++++++++++ 4 files changed, 298 insertions(+), 21 deletions(-) diff --git a/src/commands/pii.rs b/src/commands/pii.rs index 6d4c0b1..200bd47 100644 --- a/src/commands/pii.rs +++ b/src/commands/pii.rs @@ -12,8 +12,8 @@ use crate::error::{HarliteError, Result}; use super::csv::write_csv_field; use super::query::OutputFormat; use super::util::{ - canonicalize_path_for_compare, finalize_sensitive_write, prepare_sensitive_write, - resolve_database, ExternalPathPolicy, StagedDatabase, + canonicalize_path_for_compare, delete_orphaned_blobs, finalize_sensitive_write, + prepare_sensitive_write, resolve_database, ExternalPathPolicy, StagedDatabase, }; #[derive(Clone, Copy, Debug, serde::Serialize)] @@ -378,6 +378,7 @@ 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)?; } diff --git a/src/commands/redact.rs b/src/commands/redact.rs index 7fc5a01..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, finalize_sensitive_write, prepare_sensitive_write, - resolve_database, ExternalPathPolicy, StagedDatabase, + 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)] @@ -865,6 +865,7 @@ pub fn run_redact_with_external_paths( true, &external_paths, )?; + delete_orphaned_blobs(&tx)?; tx.commit()?; finalize_sensitive_write(&conn)?; report diff --git a/src/commands/util.rs b/src/commands/util.rs index 0b07a77..9b29459 100644 --- a/src/commands/util.rs +++ b/src/commands/util.rs @@ -89,6 +89,14 @@ pub struct StagedDatabase { 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 @@ -136,6 +144,12 @@ impl StagedDatabase { } 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)", @@ -144,17 +158,21 @@ impl StagedDatabase { } remove_database_sidecars(&self.path)?; - remove_database_sidecars(&self.destination)?; - if let Err(err) = fs::rename(&self.path, &self.destination) { - // Windows does not replace an existing file with rename. Preserve - // atomic replacement where the platform supports it, then fall - // back to remove-and-rename only for an explicit overwrite. - if self.overwrite && self.destination.exists() { - fs::remove_file(&self.destination)?; - fs::rename(&self.path, &self.destination)?; - } else { - return Err(err.into()); - } + 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()) @@ -169,6 +187,83 @@ impl Drop for StagedDatabase { } } +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)?; @@ -177,10 +272,8 @@ pub fn remove_database_with_sidecars(database: &Path) -> Result<()> { } fn remove_database_sidecars(database: &Path) -> Result<()> { - for suffix in ["-journal", "-wal", "-shm"] { - let mut sidecar = database.as_os_str().to_os_string(); - sidecar.push(suffix); - let sidecar = PathBuf::from(sidecar); + for suffix in database_sidecar_suffixes() { + let sidecar = database_sidecar_path(database, suffix); if sidecar.exists() { fs::remove_file(sidecar)?; } @@ -188,6 +281,16 @@ fn remove_database_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( @@ -215,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);", )?; @@ -313,7 +415,10 @@ fn resolve_database_in_dir(dir: &Path) -> Result { #[cfg(test)] mod tests { - use super::{remove_database_with_sidecars, 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; @@ -344,6 +449,67 @@ mod tests { } } + #[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 resolve_database_in_dir_errors_when_missing() { let tmp = TempDir::new().unwrap(); diff --git a/tests/integration.rs b/tests/integration.rs index 4cc80f4..a00cd39 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1872,6 +1872,115 @@ fn test_pii_redaction_rolls_back_every_entry_on_failure() { 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(); From 3a8b4b7e2cd69d4052022a37ae6fd0bc5052e81a Mon Sep 17 00:00:00 2001 From: "Bruce J. Hart" Date: Mon, 13 Jul 2026 17:35:24 -0400 Subject: [PATCH 5/6] Test staged install rollback Simulate a failed final rename and verify the prior destination database and every SQLite sidecar are restored without leaving a backup directory. --- src/commands/util.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/commands/util.rs b/src/commands/util.rs index 9b29459..9d2751f 100644 --- a/src/commands/util.rs +++ b/src/commands/util.rs @@ -510,6 +510,42 @@ mod tests { .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(); From 716dcda9acf1620ba821f37b53985b1b136f1473 Mon Sep 17 00:00:00 2001 From: "Bruce J. Hart" Date: Mon, 13 Jul 2026 17:38:14 -0400 Subject: [PATCH 6/6] Fail closed on every prune row decode Propagate malformed entry hashes, orphan blob hashes, and pruned external paths instead of silently skipping them. Add regressions proving malformed pruned paths and entry hashes roll back the import transaction. --- src/commands/prune.rs | 9 ++-- tests/integration.rs | 105 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 6 deletions(-) diff --git a/src/commands/prune.rs b/src/commands/prune.rs index 398ac84..b8ed74d 100644 --- a/src/commands/prune.rs +++ b/src/commands/prune.rs @@ -61,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 }; @@ -123,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; @@ -145,8 +143,7 @@ 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 { diff --git a/tests/integration.rs b/tests/integration.rs index a00cd39..b374464 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -429,6 +429,111 @@ fn test_prune_rolls_back_on_malformed_surviving_external_path() { 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() {