fix: reject malformed namespaced hash mappings (#216)

Author: presempathy-awb

src/git/versioned_store/namespaced.rs

@@ -1883,30 +1883,42 @@ impl<const N: usize> NamespacedKvStore<N, GitNodeStorage<N>, GitMetadataBackend>
         // Load from both mapping files
         for path in [&ns_mapping_path, &global_mapping_path] {
             if let Ok(data) = self.inner.metadata.read_file_at_commit(commit_id, path) {
-                let mapping_str = String::from_utf8(data).unwrap_or_default();
-                for line in mapping_str.lines() {
-                    if let Some((hash_hex, object_hex)) = line.split_once(':') {
-                        if hash_hex.len() == N * 2 {
-                            let mut hash_bytes = Vec::new();
-                            for i in 0..N {
-                                if let Ok(byte) =
-                                    u8::from_str_radix(&hash_hex[i * 2..i * 2 + 2], 16)
-                                {
-                                    hash_bytes.push(byte);
-                                } else {
-                                    break;
-                                }
-                            }
-                            if hash_bytes.len() == N {
-                                if let Ok(object_id) =
-                                    gix::ObjectId::from_hex(object_hex.as_bytes())
-                                {
-                                    let hash = ValueDigest::raw_hash(&hash_bytes);
-                                    hash_mappings.insert(hash, object_id);
-                                }
-                            }
-                        }
+                let mapping_str = String::from_utf8(data).map_err(|e| {
+                    GitKvError::GitObjectError(format!("Invalid UTF-8 in {path}: {e}"))
+                })?;
+                for (line_index, line) in mapping_str.lines().enumerate() {
+                    if line.trim().is_empty() {
+                        continue;
                     }
+
+                    let line_number = line_index + 1;
+                    let (hash_hex, object_hex) = line.split_once(':').ok_or_else(|| {
+                        GitKvError::GitObjectError(format!(
+                            "Malformed hash mapping in {path} on line {line_number}"
+                        ))
+                    })?;
+
+                    if hash_hex.len() != N * 2 {
+                        return Err(GitKvError::GitObjectError(format!(
+                            "Invalid prolly hash in {path} on line {line_number}: expected {} hex chars, got {}",
+                            N * 2,
+                            hash_hex.len()
+                        )));
+                    }
+
+                    let hash_bytes = hex::decode(hash_hex).map_err(|e| {
+                        GitKvError::GitObjectError(format!(
+                            "Invalid prolly hash in {path} on line {line_number}: {e}"
+                        ))
+                    })?;
+                    let object_id =
+                        gix::ObjectId::from_hex(object_hex.as_bytes()).map_err(|e| {
+                            GitKvError::GitObjectError(format!(
+                                "Invalid git object id in {path} on line {line_number}: {e}"
+                            ))
+                        })?;
+                    let hash = ValueDigest::raw_hash(&hash_bytes);
+                    hash_mappings.insert(hash, object_id);
                 }
             }
         }

src/git/versioned_store/tests.rs

@@ -291,12 +291,13 @@ mod proof_tests {
 
 #[cfg(test)]
 mod tests {
+    use crate::diff::IgnoreConflictsResolver;
     use crate::git::types::DiffOperation;
     #[cfg(feature = "rocksdb_storage")]
     use crate::git::versioned_store::RocksDBVersionedKvStore;
     use crate::git::versioned_store::{
-        FileVersionedKvStore, GitVersionedKvStore, HistoricalAccess, HistoricalCommitAccess,
-        InMemoryVersionedKvStore, ThreadSafeGitVersionedKvStore,
+        FileVersionedKvStore, GitNamespacedKvStore, GitVersionedKvStore, HistoricalAccess,
+        HistoricalCommitAccess, InMemoryVersionedKvStore, ThreadSafeGitVersionedKvStore,
     };
     use crate::tree::Tree;
     use tempfile::TempDir;
@@ -341,6 +342,67 @@ mod tests {
         assert!(store.is_ok());
     }
 
+    #[test]
+    fn namespaced_merge_rejects_invalid_committed_hash_mapping_entry() {
+        let temp_dir = TempDir::new().unwrap();
+        gix::init(temp_dir.path()).unwrap();
+        let dataset_dir = temp_dir.path().join("dataset");
+        std::fs::create_dir_all(&dataset_dir).unwrap();
+        let _cwd = CwdGuard::set(&dataset_dir);
+
+        std::process::Command::new("git")
+            .args(["config", "user.name", "ProllyTree Test"])
+            .current_dir(temp_dir.path())
+            .output()
+            .expect("git config user.name");
+        std::process::Command::new("git")
+            .args(["config", "user.email", "prollytree@example.test"])
+            .current_dir(temp_dir.path())
+            .output()
+            .expect("git config user.email");
+
+        let mut store = GitNamespacedKvStore::<32>::init(&dataset_dir).unwrap();
+        store
+            .namespace("personal")
+            .insert(b"base".to_vec(), b"base-value".to_vec())
+            .unwrap();
+        store.commit("base").unwrap();
+
+        store.create_branch("feature").unwrap();
+        store
+            .namespace("personal")
+            .insert(b"feature".to_vec(), b"feature-value".to_vec())
+            .unwrap();
+        store.commit("feature data").unwrap();
+
+        let mapping_path = dataset_dir.join("prolly_hash_mappings");
+        let mut mappings = std::fs::read_to_string(&mapping_path).unwrap();
+        mappings.push_str("not-hex:also-not-a-git-object\n");
+        std::fs::write(&mapping_path, mappings).unwrap();
+
+        let add = std::process::Command::new("git")
+            .args(["add", "dataset/prolly_hash_mappings"])
+            .current_dir(temp_dir.path())
+            .output()
+            .expect("git add");
+        assert!(add.status.success(), "git add failed: {add:?}");
+        let commit = std::process::Command::new("git")
+            .args(["commit", "-m", "corrupt feature mappings"])
+            .current_dir(temp_dir.path())
+            .output()
+            .expect("git commit");
+        assert!(commit.status.success(), "git commit failed: {commit:?}");
+
+        store.checkout("main").unwrap();
+        let err = store
+            .merge("feature", &IgnoreConflictsResolver)
+            .expect_err("invalid committed mapping entries must fail merge");
+        assert!(
+            err.to_string().contains("Invalid prolly hash"),
+            "unexpected error: {err}"
+        );
+    }
+
     #[test]
     fn test_basic_kv_operations() {
         let temp_dir = TempDir::new().unwrap();