diff --git a/crates/relicario-cli/src/commands/add.rs b/crates/relicario-cli/src/commands/add.rs index 9f62d97..b8eb2d0 100644 --- a/crates/relicario-cli/src/commands/add.rs +++ b/crates/relicario-cli/src/commands/add.rs @@ -36,8 +36,7 @@ pub fn cmd_add(kind: AddKind) -> Result<()> { vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; - crate::refresh_groups_cache(vault.root(), &manifest); + vault.after_manifest_change(&manifest)?; let mut paths: Vec = vec![ format!("items/{}.enc", item.id.as_str()), diff --git a/crates/relicario-cli/src/commands/attach.rs b/crates/relicario-cli/src/commands/attach.rs index 8b99496..8185334 100644 --- a/crates/relicario-cli/src/commands/attach.rs +++ b/crates/relicario-cli/src/commands/attach.rs @@ -72,7 +72,7 @@ pub fn cmd_attach(query: String, file: PathBuf) -> Result<()> { item.modified = now_unix(); vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; + vault.after_manifest_change(&manifest)?; let paths = [ format!("items/{}.enc", item.id.as_str()), @@ -161,7 +161,7 @@ pub fn cmd_detach(query: String, aid: String) -> Result<()> { item.modified = now_unix(); vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; + vault.after_manifest_change(&manifest)?; let item_path = format!("items/{}.enc", item.id.as_str()); let blob_relpath = format!("attachments/{}/{}.enc", item.id.as_str(), removed.id.as_str()); diff --git a/crates/relicario-cli/src/commands/edit.rs b/crates/relicario-cli/src/commands/edit.rs index a480021..798aa52 100644 --- a/crates/relicario-cli/src/commands/edit.rs +++ b/crates/relicario-cli/src/commands/edit.rs @@ -41,8 +41,7 @@ pub fn cmd_edit(query: String, totp_qr: Option) -> Result<()> { item.modified = now_unix(); vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; - crate::refresh_groups_cache(vault.root(), &manifest); + vault.after_manifest_change(&manifest)?; super::commit_paths(&vault, &format!("edit: {} ({})", crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()), &[&format!("items/{}.enc", item.id.as_str()), "manifest.enc"])?; eprintln!("Updated {}", item.id.as_str()); diff --git a/crates/relicario-cli/src/commands/get.rs b/crates/relicario-cli/src/commands/get.rs index 4618a3a..14cb595 100644 --- a/crates/relicario-cli/src/commands/get.rs +++ b/crates/relicario-cli/src/commands/get.rs @@ -8,7 +8,7 @@ pub fn cmd_get(query: String, show: bool, copy: bool) -> Result<()> { let vault = crate::session::UnlockedVault::unlock_interactive()?; let manifest = vault.load_manifest()?; - crate::refresh_groups_cache(vault.root(), &manifest); + crate::helpers::refresh_groups_cache(vault.root(), &manifest); let entry = super::resolve_query(&manifest, &query)?; let item = vault.load_item(&entry.id)?; diff --git a/crates/relicario-cli/src/commands/import.rs b/crates/relicario-cli/src/commands/import.rs index f4b5713..5353305 100644 --- a/crates/relicario-cli/src/commands/import.rs +++ b/crates/relicario-cli/src/commands/import.rs @@ -49,7 +49,7 @@ fn cmd_import_lastpass(csv_path: PathBuf) -> Result<()> { } } - vault.save_manifest(&manifest)?; + vault.after_manifest_change(&manifest)?; written_paths.push("manifest.enc".into()); let path_refs: Vec<&str> = written_paths.iter().map(String::as_str).collect(); diff --git a/crates/relicario-cli/src/commands/init.rs b/crates/relicario-cli/src/commands/init.rs index c5aedc1..f6612b2 100644 --- a/crates/relicario-cli/src/commands/init.rs +++ b/crates/relicario-cli/src/commands/init.rs @@ -65,17 +65,7 @@ pub fn cmd_init(image: PathBuf, output: PathBuf) -> Result<()> { fs::write(relicario_dir.join("salt"), salt)?; fs::write( relicario_dir.join("params.json"), - serde_json::to_string_pretty(&ParamsFile { - format_version: 2, - kdf: ParamsKdf { - algorithm: "argon2id-v0x13".into(), - argon2_m: params.argon2_m, - argon2_t: params.argon2_t, - argon2_p: params.argon2_p, - }, - aead: "xchacha20poly1305".into(), - salt_path: ".relicario/salt".into(), - })?, + serde_json::to_string_pretty(&crate::session::ParamsFile::for_new_vault(¶ms))?, )?; let manifest = Manifest::new(); fs::write(root.join("manifest.enc"), encrypt_manifest(&manifest, &master_key)?)?; @@ -106,20 +96,3 @@ pub fn cmd_init(image: PathBuf, output: PathBuf) -> Result<()> { eprintln!(" \u{2192} back this file up somewhere safe; it is your second factor."); Ok(()) } - -#[derive(serde::Serialize)] -struct ParamsFile { - format_version: u32, - kdf: ParamsKdf, - aead: String, - salt_path: String, -} - -#[derive(serde::Serialize)] -#[serde(rename_all = "snake_case")] -struct ParamsKdf { - algorithm: String, - argon2_m: u32, - argon2_t: u32, - argon2_p: u32, -} diff --git a/crates/relicario-cli/src/commands/list.rs b/crates/relicario-cli/src/commands/list.rs index 1c153a5..d192b5a 100644 --- a/crates/relicario-cli/src/commands/list.rs +++ b/crates/relicario-cli/src/commands/list.rs @@ -12,7 +12,7 @@ pub fn cmd_list( let vault = crate::session::UnlockedVault::unlock_interactive()?; let manifest = vault.load_manifest()?; - crate::refresh_groups_cache(vault.root(), &manifest); + crate::helpers::refresh_groups_cache(vault.root(), &manifest); let parsed_type: Option = match type_filter.as_deref() { None => None, diff --git a/crates/relicario-cli/src/commands/trash.rs b/crates/relicario-cli/src/commands/trash.rs index 65b41cd..1d28222 100644 --- a/crates/relicario-cli/src/commands/trash.rs +++ b/crates/relicario-cli/src/commands/trash.rs @@ -15,8 +15,7 @@ pub fn cmd_rm(query: String) -> Result<()> { item.soft_delete(); vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; - crate::refresh_groups_cache(vault.root(), &manifest); + vault.after_manifest_change(&manifest)?; super::commit_paths(&vault, &format!("trash: {} ({})", crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()), &[&format!("items/{}.enc", item.id.as_str()), "manifest.enc"])?; eprintln!("Moved to trash: {}", item.title); @@ -33,37 +32,41 @@ pub fn cmd_restore(query: String) -> Result<()> { item.restore(); vault.save_item(&item)?; manifest.upsert(&item); - vault.save_manifest(&manifest)?; - crate::refresh_groups_cache(vault.root(), &manifest); + vault.after_manifest_change(&manifest)?; super::commit_paths(&vault, &format!("restore: {} ({})", crate::helpers::sanitize_for_commit(&item.title), item.id.as_str()), &[&format!("items/{}.enc", item.id.as_str()), "manifest.enc"])?; eprintln!("Restored: {}", item.title); Ok(()) } -/// Inner purge: assumes vault is already unlocked and manifest is loaded. -/// Caller is responsible for saving the manifest and committing afterwards. -pub(super) fn purge_item( +/// Filesystem-only purge: removes the item.enc, attachments//, and updates +/// the manifest in memory. Returns the relative paths the caller must stage +/// via `git rm` after the loop. Does NOT invoke any git commands — the caller +/// batches them. +pub(super) fn purge_item_filesystem( vault: &crate::session::UnlockedVault, manifest: &mut relicario_core::Manifest, id: &relicario_core::ItemId, title: &str, -) -> Result<()> { - use std::fs; +) -> Result> { + use std::{fs, io::ErrorKind}; - let item_path = vault.item_path(id); - if item_path.exists() { fs::remove_file(&item_path)?; } - let att_dir = vault.root().join("attachments").join(id.as_str()); - if att_dir.exists() { fs::remove_dir_all(&att_dir)?; } + let item_rel = format!("items/{}.enc", id.as_str()); + let att_rel = format!("attachments/{}", id.as_str()); + + let ignore_missing = |r: std::io::Result<()>| -> Result<()> { + match r { + Ok(()) => Ok(()), + Err(e) if e.kind() == ErrorKind::NotFound => Ok(()), + Err(e) => Err(e.into()), + } + }; + ignore_missing(fs::remove_file(vault.item_path(id)))?; + ignore_missing(fs::remove_dir_all(vault.root().join("attachments").join(id.as_str())))?; manifest.remove(id); - let _ = crate::helpers::git_command(vault.root(), &["rm", "-rf", "--ignore-unmatch", - &format!("items/{}.enc", id.as_str()), - &format!("attachments/{}", id.as_str()), - ]).status()?; - // Note: caller adds+commits manifest.enc after processing all purges. eprintln!("Purged: {title}"); - Ok(()) + Ok(vec![item_rel, att_rel]) } pub fn cmd_purge(query: String) -> Result<()> { @@ -74,12 +77,16 @@ pub fn cmd_purge(query: String) -> Result<()> { let title = entry.title.clone(); let _ = entry; - purge_item(&vault, &mut manifest, &id, &title)?; - vault.save_manifest(&manifest)?; - crate::refresh_groups_cache(vault.root(), &manifest); + let paths = purge_item_filesystem(&vault, &mut manifest, &id, &title)?; + vault.after_manifest_change(&manifest)?; let purge_ctx = format!("purge \"{}\" ({})", title, id.as_str()); - crate::helpers::git_run(vault.root(), &["add", "manifest.enc"], &format!("{purge_ctx}: git add manifest.enc"))?; + crate::helpers::git_rm(vault.root(), &paths, &format!("{purge_ctx}: git rm"))?; + crate::helpers::git_run( + vault.root(), + &["add", "manifest.enc"], + &format!("{purge_ctx}: git add manifest.enc"), + )?; crate::helpers::git_run( vault.root(), &["commit", "-m", &format!("purge: {} ({})", title, id.as_str())], @@ -116,13 +123,16 @@ pub fn cmd_trash_empty() -> Result<()> { return Ok(()); } - let mut purged_titles = Vec::new(); + let mut all_paths: Vec = Vec::new(); + let purged_count = purgeable.len(); for (id, title) in purgeable { - purge_item(&vault, &mut manifest, &id, &title)?; - purged_titles.push(title); + let mut paths = purge_item_filesystem(&vault, &mut manifest, &id, &title)?; + all_paths.append(&mut paths); } - vault.save_manifest(&manifest)?; + vault.after_manifest_change(&manifest)?; + + crate::helpers::git_rm(vault.root(), &all_paths, "trash empty: git rm")?; crate::helpers::git_run( vault.root(), &["add", "manifest.enc"], @@ -130,10 +140,10 @@ pub fn cmd_trash_empty() -> Result<()> { )?; crate::helpers::git_run( vault.root(), - &["commit", "-m", &format!("trash empty: purged {} item(s)", purged_titles.len())], + &["commit", "-m", &format!("trash empty: purged {} item(s)", purged_count)], "trash empty: git commit", )?; - eprintln!("Emptied trash: {} item(s)", purged_titles.len()); + eprintln!("Emptied trash: {} item(s)", purged_count); Ok(()) } diff --git a/crates/relicario-cli/src/helpers.rs b/crates/relicario-cli/src/helpers.rs index f07b266..4c58906 100644 --- a/crates/relicario-cli/src/helpers.rs +++ b/crates/relicario-cli/src/helpers.rs @@ -86,6 +86,16 @@ pub fn git_run(repo: &Path, args: &[&str], context: &str) -> Result<()> { Ok(()) } +/// Stage `paths` for removal in one `git rm -rf --ignore-unmatch` invocation. +/// `--ignore-unmatch` is load-bearing: a previous partial-write crash can +/// leave the manifest entry without the corresponding `items/.enc` on +/// disk, and we want the rm to succeed regardless. +pub fn git_rm(repo: &Path, paths: &[String], context: &str) -> Result<()> { + let mut args: Vec<&str> = vec!["rm", "-rf", "--ignore-unmatch"]; + args.extend(paths.iter().map(String::as_str)); + git_run(repo, &args, context) +} + /// Format a Unix-seconds timestamp as an ISO-8601 UTC string. /// Audit M11: replaces the old `now_iso8601` helper that actually returned /// a numeric string. @@ -126,6 +136,24 @@ pub fn groups_cache_path(vault_dir: &Path) -> PathBuf { vault_dir.join(".relicario").join("groups.cache") } +/// Collect all non-empty group names from the manifest and write them to the +/// plaintext `groups.cache` file so shell completion can enumerate `--group` +/// candidates without prompting for the vault passphrase. +/// +/// Failures are silently swallowed — a missing cache is merely a UX degradation, +/// not a correctness problem. +pub fn refresh_groups_cache(vault_dir: &Path, manifest: &relicario_core::Manifest) { + let mut set = std::collections::BTreeSet::::new(); + for entry in manifest.items.values() { + if let Some(g) = entry.group.as_ref() { + if !g.is_empty() { + set.insert(g.clone()); + } + } + } + let _ = write_groups_cache(vault_dir, &set); +} + /// Write the sorted set of group names to `/.relicario/groups.cache`, /// one name per line. In debug builds, setting `RELICARIO_NO_GROUPS_CACHE` /// suppresses the write (developer debugging tool). In release builds the env diff --git a/crates/relicario-cli/src/main.rs b/crates/relicario-cli/src/main.rs index 8a32f5a..ab59cdb 100644 --- a/crates/relicario-cli/src/main.rs +++ b/crates/relicario-cli/src/main.rs @@ -457,24 +457,6 @@ fn main() -> Result<()> { } } -/// Collect all non-empty group names from the manifest and write them to the -/// plaintext `groups.cache` file so shell completion can enumerate `--group` -/// candidates without prompting for the vault passphrase. -/// -/// Failures are silently swallowed — a missing cache is merely a UX degradation, -/// not a correctness problem. -pub(crate) fn refresh_groups_cache(vault_dir: &std::path::Path, manifest: &relicario_core::Manifest) { - let mut set = std::collections::BTreeSet::::new(); - for entry in manifest.items.values() { - if let Some(g) = entry.group.as_ref() { - if !g.is_empty() { - set.insert(g.clone()); - } - } - } - let _ = helpers::write_groups_cache(vault_dir, &set); -} - /// Check for test passphrase override (debug builds only; stripped from release). #[cfg(debug_assertions)] pub(crate) fn test_passphrase_override() -> Option { diff --git a/crates/relicario-cli/src/session.rs b/crates/relicario-cli/src/session.rs index ec42f1b..0e4f0ed 100644 --- a/crates/relicario-cli/src/session.rs +++ b/crates/relicario-cli/src/session.rs @@ -69,9 +69,15 @@ impl UnlockedVault { Ok(decrypt_manifest(&bytes, &self.master_key)?) } - pub fn save_manifest(&self, manifest: &Manifest) -> Result<()> { + /// Save the manifest and refresh the plaintext groups.cache. This is the + /// canonical "I just mutated the manifest" funnel — every command that + /// changes the manifest goes through this method, so cache freshness is + /// a compile-time invariant rather than a discipline rule. + pub fn after_manifest_change(&self, manifest: &Manifest) -> Result<()> { let bytes = encrypt_manifest(manifest, &self.master_key)?; - atomic_write(&self.manifest_path(), &bytes) + atomic_write(&self.manifest_path(), &bytes)?; + crate::helpers::refresh_groups_cache(&self.root, manifest); + Ok(()) } pub fn load_settings(&self) -> Result { @@ -107,17 +113,52 @@ fn read_salt(root: &Path) -> Result<[u8; 32]> { Ok(salt) } -fn read_params(root: &Path) -> Result { - // params.json layout: { "format_version": 2, "kdf": { "argon2_m": ..., ... }, ... } - // We extract only the "kdf" sub-object and deserialize it as KdfParams. - #[derive(serde::Deserialize)] - struct ParamsFile { - kdf: KdfParams, +#[derive(serde::Serialize, serde::Deserialize)] +pub(crate) struct ParamsFile { + pub format_version: u32, + pub kdf: ParamsKdf, + pub aead: String, + pub salt_path: String, +} + +#[derive(serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "snake_case")] +pub(crate) struct ParamsKdf { + pub algorithm: String, + pub argon2_m: u32, + pub argon2_t: u32, + pub argon2_p: u32, +} + +impl ParamsFile { + pub fn for_new_vault(params: &KdfParams) -> Self { + Self { + format_version: 2, + kdf: ParamsKdf { + algorithm: "argon2id-v0x13".into(), + argon2_m: params.argon2_m, + argon2_t: params.argon2_t, + argon2_p: params.argon2_p, + }, + aead: "xchacha20poly1305".into(), + salt_path: ".relicario/salt".into(), + } } + + pub fn to_kdf_params(&self) -> KdfParams { + KdfParams { + argon2_m: self.kdf.argon2_m, + argon2_t: self.kdf.argon2_t, + argon2_p: self.kdf.argon2_p, + } + } +} + +fn read_params(root: &Path) -> Result { let s = fs::read_to_string(root.join(".relicario").join("params.json")) .context("failed to read .relicario/params.json")?; let pf: ParamsFile = serde_json::from_str(&s).context("failed to parse params.json")?; - Ok(pf.kdf) + Ok(pf.to_kdf_params()) } /// Locate the reference image path via `RELICARIO_IMAGE` env var or interactive prompt. @@ -149,3 +190,78 @@ fn atomic_write(path: &Path, data: &[u8]) -> Result<()> { fs::rename(&tmp, path).with_context(|| format!("failed to rename {}", path.display()))?; Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + const FIXTURE: &str = r#"{ + "format_version": 2, + "kdf": { + "algorithm": "argon2id-v0x13", + "argon2_m": 65536, + "argon2_t": 3, + "argon2_p": 4 + }, + "aead": "xchacha20poly1305", + "salt_path": ".relicario/salt" +}"#; + + #[test] + fn params_file_round_trips_current_layout() { + let pf: ParamsFile = serde_json::from_str(FIXTURE).expect("parse fixture"); + assert_eq!(pf.format_version, 2); + assert_eq!(pf.kdf.algorithm, "argon2id-v0x13"); + assert_eq!(pf.kdf.argon2_m, 65536); + assert_eq!(pf.kdf.argon2_t, 3); + assert_eq!(pf.kdf.argon2_p, 4); + assert_eq!(pf.aead, "xchacha20poly1305"); + assert_eq!(pf.salt_path, ".relicario/salt"); + + let kdf = pf.to_kdf_params(); + assert_eq!(kdf.argon2_m, 65536); + assert_eq!(kdf.argon2_t, 3); + assert_eq!(kdf.argon2_p, 4); + + let serialized = serde_json::to_string(&pf).expect("re-serialize"); + let pf2: ParamsFile = serde_json::from_str(&serialized).expect("parse re-serialized"); + assert_eq!(pf2.format_version, 2); + assert_eq!(pf2.kdf.algorithm, "argon2id-v0x13"); + assert_eq!(pf2.kdf.argon2_m, 65536); + assert_eq!(pf2.kdf.argon2_t, 3); + assert_eq!(pf2.kdf.argon2_p, 4); + assert_eq!(pf2.aead, "xchacha20poly1305"); + assert_eq!(pf2.salt_path, ".relicario/salt"); + } + + #[test] + fn for_new_vault_produces_expected_shape() { + let params = KdfParams { argon2_m: 65536, argon2_t: 3, argon2_p: 4 }; + let pf = ParamsFile::for_new_vault(¶ms); + let v = serde_json::to_value(&pf).expect("to_value"); + assert_eq!(v["format_version"], 2); + assert_eq!(v["kdf"]["algorithm"], "argon2id-v0x13"); + assert_eq!(v["kdf"]["argon2_m"], 65536); + assert_eq!(v["kdf"]["argon2_t"], 3); + assert_eq!(v["kdf"]["argon2_p"], 4); + assert_eq!(v["aead"], "xchacha20poly1305"); + assert_eq!(v["salt_path"], ".relicario/salt"); + } + + #[test] + fn after_manifest_change_writes_manifest_and_groups_cache() { + let dir = tempfile::TempDir::new().unwrap(); + let root = dir.path().to_path_buf(); + std::fs::create_dir_all(root.join(".relicario")).unwrap(); + std::fs::create_dir_all(root.join("items")).unwrap(); + let vault = UnlockedVault { + root: root.clone(), + master_key: Zeroizing::new([0u8; 32]), + }; + let manifest = Manifest::new(); + + vault.after_manifest_change(&manifest).unwrap(); + assert!(root.join("manifest.enc").exists()); + assert!(root.join(".relicario/groups.cache").exists()); + } +} diff --git a/crates/relicario-cli/tests/basic_flows.rs b/crates/relicario-cli/tests/basic_flows.rs index 8f85095..5d73077 100644 --- a/crates/relicario-cli/tests/basic_flows.rs +++ b/crates/relicario-cli/tests/basic_flows.rs @@ -109,6 +109,72 @@ fn rm_restore_purge_cycle() { assert!(!String::from_utf8(out.stdout).unwrap().contains("target")); } +#[test] +fn trash_empty_batches_into_one_commit() { + let v = TestVault::init(); + + // Add 3 items. + for title in ["alpha", "bravo", "charlie"] { + let out = v.run(&[ + "add", "login", + "--title", title, + "--username", "u", + "--password", "p", + ]); + assert!(out.status.success(), "add {title} failed"); + } + + // Soft-delete all 3. + for title in ["alpha", "bravo", "charlie"] { + let out = v.run(&["rm", title]); + assert!(out.status.success(), "rm {title} failed"); + } + + // Set retention to 0 days so the recently-trashed items become purgeable + // (should_purge: now - trashed_at > 0 * 86400 = 0). + let out = v.run(&["settings", "trash-retention", "--days", "0"]); + assert!(out.status.success(), "settings trash-retention failed"); + + // should_purge uses strict > on (now - trashed_at), so equal-second + // timestamps don't qualify. + std::thread::sleep(std::time::Duration::from_secs(1)); + + // Count commits before. + let before = std::process::Command::new("git") + .args(["rev-list", "--count", "HEAD"]) + .current_dir(v.path()) + .output() + .unwrap(); + let before_count: u32 = String::from_utf8(before.stdout).unwrap().trim().parse().unwrap(); + + // Run trash empty. + let out = v.run(&["trash", "empty"]); + assert!(out.status.success(), "trash empty failed: stderr={}", + String::from_utf8_lossy(&out.stderr)); + + // Count commits after. + let after = std::process::Command::new("git") + .args(["rev-list", "--count", "HEAD"]) + .current_dir(v.path()) + .output() + .unwrap(); + let after_count: u32 = String::from_utf8(after.stdout).unwrap().trim().parse().unwrap(); + + assert_eq!( + after_count - before_count, 1, + "trash empty should fire exactly one commit; before={before_count} after={after_count}" + ); + + // The remaining `list --trashed` should be empty. + let out = v.run(&["list", "--trashed"]); + let stdout = String::from_utf8(out.stdout).unwrap(); + let stderr = String::from_utf8(out.stderr).unwrap(); + assert!( + !stdout.contains("alpha") && !stdout.contains("bravo") && !stdout.contains("charlie"), + "items still in trashed list: stdout={stdout} stderr={stderr}" + ); +} + #[test] fn generate_random_and_bip39() { let dir = tempfile::TempDir::new().unwrap();