diff --git a/src/vault.rs b/src/vault.rs index ff8b0d8..eb78e3f 100644 --- a/src/vault.rs +++ b/src/vault.rs @@ -154,16 +154,20 @@ pub fn legacy_vault_paths() -> Vec { /// Load the vault, migrating a legacy vault on first use if necessary. /// /// Never panics: a missing or empty file yields an empty vault; a malformed -/// file yields a structured error. +/// file yields a structured error. Leftover legacy vaults (which may hold +/// plaintext secret keys under lax permissions) are tightened either way. pub fn load_vault() -> Result { let path = vault_path(); if !path.exists() { if let Some(vault) = try_migrate_legacy_vault()? { return Ok(vault); } + harden_stray_legacy_vaults(); return Ok(Vault::empty()); } - read_vault_from(&path) + let vault = read_vault_from(&path)?; + harden_stray_legacy_vaults(); + Ok(vault) } /// Read and parse a vault from an explicit path. @@ -287,11 +291,39 @@ pub fn backup_file(path: &Path) -> Result { Ok(backup) } +/// Restrict `path` to owner-only access. Best effort: failures are ignored so +/// unusual filesystems can never break vault loading. +fn harden_legacy_file(path: &Path) { + let _ = fs::set_permissions(path, fs::Permissions::from_mode(0o600)); +} + +/// Whether `path` is a recognisable vault holding at least one profile. +fn is_populated_vault_file(path: &Path) -> bool { + fs::read_to_string(path) + .map(|content| matches!(parse_vault(&content), Ok(vault) if vault.has_profiles())) + .unwrap_or(false) +} + +/// Tighten permissions on leftover legacy vault files. +/// +/// The original CLI stored secret keys as plaintext with whatever the process +/// umask allowed (often group-readable). Any file in a legacy location that we +/// recognise as a populated vault is restricted to owner-only access — whether +/// or not it was ever migrated. +pub fn harden_stray_legacy_vaults() { + for legacy in legacy_vault_paths() { + if legacy.exists() && is_populated_vault_file(&legacy) { + harden_legacy_file(&legacy); + } + } +} + /// Look for a vault written by the old CLI and migrate it into the stable /// application-data location. /// /// A backup of the legacy file is created before migrating. The legacy file -/// itself is left untouched. Returns `None` when no migratable vault exists. +/// itself is left untouched (but tightened to owner-only permissions). Returns +/// `None` when no migratable vault exists. fn try_migrate_legacy_vault() -> Result, AppError> { for legacy in legacy_vault_paths() { if !legacy.exists() { @@ -313,6 +345,7 @@ fn try_migrate_legacy_vault() -> Result, AppError> { // Nothing to migrate; leave the legacy file alone. continue; } + harden_legacy_file(&legacy); let backup = backup_file(&legacy)?; @@ -444,4 +477,47 @@ mod tests { let mode = fs::metadata(&path).unwrap().permissions().mode(); assert_eq!(mode & 0o777, 0o600, "vault must be readable only by owner"); } + + #[test] + fn recognised_legacy_vault_is_tightened_to_0600() { + let path = temp_vault_path(); + let legacy = r#"[ + { "label": "Alice", "public_key": "npub1abc", "secret_key": "deadbeef", "created_at": 1700000000 } + ]"#; + fs::write(&path, legacy).unwrap(); + fs::set_permissions(&path, fs::Permissions::from_mode(0o664)).unwrap(); + assert!(is_populated_vault_file(&path), "must recognise the vault"); + + harden_legacy_file(&path); + + let mode = fs::metadata(&path).unwrap().permissions().mode(); + assert_eq!( + mode & 0o777, + 0o600, + "stray legacy vault must become owner-only" + ); + } + + #[test] + fn unparsable_file_is_not_treated_as_a_vault() { + let path = temp_vault_path(); + fs::write(&path, "not json at all").unwrap(); + assert!(!is_populated_vault_file(&path)); + } + + #[test] + fn empty_or_keyless_vault_is_not_populated() { + let path = temp_vault_path(); + fs::write(&path, "[]").unwrap(); + assert!(!is_populated_vault_file(&path)); + + // Encrypted blob where a key would be still counts as populated: the + // file is ours and must be tightened even though it is not plaintext. + let encrypted = r#"[ + { "label": "Alice", "public_key": "npub1abc", + "secret_key": "AAAAaGVsbG8gd29ybGQAAAAAAAAAAAAAAAAAAAAAAAAA", "created_at": 1700000000 } + ]"#; + fs::write(&path, encrypted).unwrap(); + assert!(is_populated_vault_file(&path)); + } }