Tighten permissions on leftover legacy vault files

src/vault.rs

@@ -154,16 +154,20 @@ pub fn legacy_vault_paths() -> Vec<PathBuf> {
 /// 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<Vault, AppError> {
     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<PathBuf, AppError> {
     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<Option<Vault>, AppError> {
     for legacy in legacy_vault_paths() {
         if !legacy.exists() {
@@ -313,6 +345,7 @@ fn try_migrate_legacy_vault() -> Result<Option<Vault>, 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));
+    }
 }