From 506b9e7e2eefe92673cb5acd831a31cfabbdc201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Kj=C3=A4ll?= Date: Mon, 14 Sep 2026 19:08:26 +0200 Subject: [PATCH] try to handle NFC vs NFD file names in git/on disk --- Cargo.lock | 1 + Cargo.toml | 1 + crates/ripasso/Cargo.toml | 1 + crates/ripasso/src/git.rs | 55 ++++++++++++++++++++++-- crates/ripasso/src/pass.rs | 19 +++++---- crates/ripasso/src/tests/git.rs | 41 +++++++++++++++++- crates/ripasso/src/tests/pass.rs | 72 ++++++++++++++++++++++++++++++++ 7 files changed, 177 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 416ba42e..c3c383f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3283,6 +3283,7 @@ dependencies = [ "glob", "gpgme", "hex", + "icu_normalizer", "rand", "reqwest", "sequoia-gpg-agent", diff --git a/Cargo.toml b/Cargo.toml index e7aa0fcf..c47ccb18 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ glob = "0.3" gpgme = "0.11" gtk4 = "0.11" hex = "0.4" +icu_normalizer = { version = "2", default-features = false, features = ["compiled_data"] } libadwaita = "0.9" locale_config = "0.3" man = "0.3" diff --git a/crates/ripasso/Cargo.toml b/crates/ripasso/Cargo.toml index 50eb8bc4..72f3b0b4 100644 --- a/crates/ripasso/Cargo.toml +++ b/crates/ripasso/Cargo.toml @@ -20,6 +20,7 @@ whoami.workspace = true toml.workspace = true reqwest = { workspace = true, features = ["blocking"] } hex.workspace = true +icu_normalizer.workspace = true totp-rs = { workspace = true, features = ["otpauth"] } sequoia-openpgp.workspace = true anyhow.workspace = true diff --git a/crates/ripasso/src/git.rs b/crates/ripasso/src/git.rs index 3d58563a..f57f0df1 100644 --- a/crates/ripasso/src/git.rs +++ b/crates/ripasso/src/git.rs @@ -1,4 +1,6 @@ use std::{ + borrow::Cow, + ffi::OsString, fmt::Display, path::{Path, PathBuf}, str, @@ -6,6 +8,7 @@ use std::{ use chrono::{DateTime, Local, TimeZone}; use git2::{Oid, Repository}; +use icu_normalizer::ComposingNormalizerBorrowed; use crate::{ crypto::{Crypto, FindSigningFingerprintStrategy, VerificationError}, @@ -455,16 +458,60 @@ pub fn init_git_repo(base: &Path) -> Result { Ok(Repository::init(base)?) } +const NFC: ComposingNormalizerBorrowed<'static> = ComposingNormalizerBorrowed::new_nfc(); + +/// Filesystem paths and git tree paths can disagree on how they normalize +/// Unicode: HFS+ gives NFD from readdir, while git stores what was committed, +/// which can be NFC. The setting `core.precomposeunicode` in git affects this. +pub(crate) fn to_nfc(path: &Path) -> Cow<'_, Path> { + match path.to_str() { + // separators are unaffected by NFC, so checking the whole path at once + // is the same as checking every component, and lets the common case + // avoid rebuilding the path + Some(s) if NFC.is_normalized(s) => Cow::Borrowed(path), + None => Cow::Borrowed(path), + Some(_) => Cow::Owned( + path.components() + .map(|c| { + c.as_os_str().to_str().map_or_else( + || c.as_os_str().to_os_string(), + |s| OsString::from(NFC.normalize(s).into_owned()), + ) + }) + .collect(), + ), + } +} + +/// A password file that we are looking for in the git history. +pub struct PathWithNormalization { + /// the path, relative to the store root, as it exists on disk + pub relpath: PathBuf, + /// `relpath` normalized in NFC + normalized: PathBuf, +} + +impl PathWithNormalization { + #[must_use] + pub fn new(relpath: PathBuf) -> Self { + let normalized = to_nfc(&relpath).into_owned(); + Self { + relpath, + normalized, + } + } +} + pub fn push_password_if_match( - target: &Path, - found: &Path, + target: &PathWithNormalization, + found: &PathWithNormalization, commit: &git2::Commit, repo: &Repository, passwords: &mut Vec, oid: &Oid, store: &PasswordStore, ) -> bool { - if *target == *found { + if target.normalized == found.normalized { let time = commit.time(); let time_return = to_result(Local.timestamp_opt(time.seconds(), 0)); @@ -474,7 +521,7 @@ pub fn push_password_if_match( passwords.push(PasswordEntry::new( &store.get_store_path(), - target, + &target.relpath, time_return, name_return, signature_return, diff --git a/crates/ripasso/src/pass.rs b/crates/ripasso/src/pass.rs index 4b023206..d7d967e6 100644 --- a/crates/ripasso/src/pass.rs +++ b/crates/ripasso/src/pass.rs @@ -32,9 +32,9 @@ use zeroize::Zeroize; use crate::{ crypto::{Crypto, CryptoImpl, Fingerprint, GpgMe, Sequoia, VerificationError}, git::{ - add_and_commit_internal, commit, find_last_commit, init_git_repo, match_with_parent, - move_and_commit, push_password_if_match, read_git_meta_data, remove_and_commit, - verify_git_signature, + PathWithNormalization, add_and_commit_internal, commit, find_last_commit, init_git_repo, + match_with_parent, move_and_commit, push_password_if_match, read_git_meta_data, + remove_and_commit, verify_git_signature, }, }; pub use crate::{ @@ -505,9 +505,11 @@ impl PasswordStore { // First, collect all files we need to find the first commit for let password_path_glob = self.root.join("**/*.gpg"); let existing_iter = glob::glob(&password_path_glob.to_string_lossy())?; - let mut files_to_find: Vec = vec![]; + let mut files_to_find: Vec = vec![]; for existing_file in existing_iter { - files_to_find.push(existing_file?.strip_prefix(&self.root)?.to_path_buf()); + files_to_find.push(PathWithNormalization::new( + existing_file?.strip_prefix(&self.root)?.to_path_buf(), + )); } if files_to_find.is_empty() { @@ -536,10 +538,11 @@ impl PasswordStore { diff.foreach( &mut |delta: git2::DiffDelta, _f: f32| { if let Some(found) = delta.new_file().path() { + let found = PathWithNormalization::new(found.to_path_buf()); files_to_find.retain(|target| { push_password_if_match( target, - found, + &found, &commit, &repo, &mut passwords, @@ -563,7 +566,7 @@ impl PasswordStore { // files was checked in to the first commit last_tree.walk(git2::TreeWalkMode::PreOrder, |path, entry| { if let Ok(entry_name) = entry.name() { - let found = Path::new(path).join(entry_name); + let found = PathWithNormalization::new(Path::new(path).join(entry_name)); files_to_find.retain(|target| { push_password_if_match( target, @@ -583,7 +586,7 @@ impl PasswordStore { for not_found in files_to_find { passwords.push(PasswordEntry::new( &self.root, - ¬_found, + ¬_found.relpath, Err(Error::from("")), Err(Error::from("")), Err(Error::from("")), diff --git a/crates/ripasso/src/tests/git.rs b/crates/ripasso/src/tests/git.rs index 6dcb4bd4..d0f4efa1 100644 --- a/crates/ripasso/src/tests/git.rs +++ b/crates/ripasso/src/tests/git.rs @@ -1,4 +1,43 @@ -use crate::{error::Result, git::should_sign, test_helpers::UnpackedDir}; +use std::{ + borrow::Cow, + path::{Path, PathBuf}, +}; + +use crate::{ + error::Result, + git::{should_sign, to_nfc}, + test_helpers::UnpackedDir, +}; + +#[test] +fn to_nfc_leaves_already_composed_paths_alone() { + assert!(matches!(to_nfc(Path::new("dir/test")), Cow::Borrowed(_))); + assert!(matches!( + to_nfc(Path::new("dir/test_åÅæÆäÄøØöÖ.gpg")), + Cow::Borrowed(_) + )); +} + +#[test] +fn to_nfc_composes_decomposed_paths() { + let decomposed = Path::new("test_a\u{30a}A\u{30a}æÆa\u{308}A\u{308}øØo\u{308}O\u{308}.gpg"); + + assert_eq!( + PathBuf::from("test_åÅæÆäÄøØöÖ.gpg"), + to_nfc(decomposed).into_owned() + ); +} + +#[test] +fn to_nfc_composes_every_component() { + let decomposed = Path::new("a\u{308}/o\u{308}/c\u{308}.gpg"); + + // "c" has no precomposed form with a diaeresis, so that one stays as it is + assert_eq!( + PathBuf::from("ä/ö/c\u{308}.gpg"), + to_nfc(decomposed).into_owned() + ); +} #[test] fn test_should_sign_true() -> Result<()> { diff --git a/crates/ripasso/src/tests/pass.rs b/crates/ripasso/src/tests/pass.rs index 956eb31b..17dce80b 100644 --- a/crates/ripasso/src/tests/pass.rs +++ b/crates/ripasso/src/tests/pass.rs @@ -126,6 +126,78 @@ fn populate_password_list_small_repo() -> Result<()> { Ok(()) } +/// A password file committed in NFC, but that the filesystem hands back in NFD, +/// which is what happens on HFS+ for a store that was created on linux. +#[test] +fn test_decomposed_filename_matches_composed_git_path() -> Result<()> { + let dir = tempdir()?; + let store_dir = dir.path().join(".password-store"); + create_dir_all(&store_dir)?; + + let on_disk = "test_a\u{30a}A\u{30a}æÆa\u{308}A\u{308}øØo\u{308}O\u{308}.gpg"; + let in_git = "test_åÅæÆäÄøØöÖ.gpg"; + + File::create(store_dir.join(".gpg-id"))?.write_all(b"0xDF0C3D316B7312D5\n")?; + File::create(store_dir.join(on_disk))?.write_all(b"")?; + + // add the paths through the index rather than from the working directory, + // so that git records the composed spelling regardless of what the + // filesystem that the test runs on would have given us + let repo = Repository::init(&store_dir)?; + let mut index = repo.index()?; + for path in [".gpg-id", in_git] { + index.add_frombuffer( + &git2::IndexEntry { + ctime: git2::IndexTime::new(0, 0), + mtime: git2::IndexTime::new(0, 0), + dev: 0, + ino: 0, + mode: 0o100_644, + uid: 0, + gid: 0, + file_size: 0, + id: git2::Oid::ZERO_SHA1, + flags: 0, + flags_extended: 0, + path: path.as_bytes().to_vec(), + }, + b"", + )?; + } + let tree = repo.find_tree(index.write_tree()?)?; + let signature = git2::Signature::new("default", "default@example.com", &git2::Time::new(0, 0))?; + repo.commit( + Some("HEAD"), + &signature, + &signature, + "unit test", + &tree, + &[], + )?; + + let store = PasswordStore { + name: "default".to_owned(), + root: store_dir.canonicalize()?, + valid_gpg_signing_keys: vec![], + passwords: vec![], + style_file: None, + crypto: Box::new(MockCrypto::new()), + user_home: None, + }; + let results = store.all_passwords()?; + + assert_eq!(results.len(), 1); + assert_eq!( + results[0].name, + "test_a\u{30a}A\u{30a}æÆa\u{308}A\u{308}øØo\u{308}O\u{308}" + ); + assert_eq!(results[0].path, store.root.join(on_disk)); + assert_eq!(results[0].is_in_git, RepositoryStatus::InRepo); + assert_eq!(results[0].committed_by, Some("default".to_owned())); + + Ok(()) +} + #[test] fn populate_password_list_repo_with_deleted_files() -> Result<()> { let dir = UnpackedDir::new("populate_password_list_repo_with_deleted_files")?;