remove most usage of read_manifest_unsigned for safety

Author: Taxrosdev

src/build/mod.rs

@@ -28,7 +28,7 @@ pub fn build(build_manifest_path: &Path, repo_path: &Path) -> Result<PackageMani
     let build_manifest: BuildManifest =
         serde_yaml::from_str(&fs::read_to_string(build_manifest_path)?)?;
 
-    let repo_manifest = repo::read_manifest_unsigned(repo_path)?;
+    let repo_manifest = repo::read_manifest(repo_path)?;
 
     let chunks = save_tree(
         &build_manifest_path.join(build_manifest.directory),

src/main.rs

@@ -130,7 +130,7 @@ fn main() -> Result<()> {
                     let repo_name = repo_dir.file_name();
                     let repo_name_str = repo_name.to_str().unwrap();
 
-                    let repo = repo::read_manifest_unsigned(&repo_dir.path())?;
+                    let repo = repo::read_manifest(&repo_dir.path())?;
 
                     table.add_row(vec![
                         &repo_name_str,
@@ -161,7 +161,7 @@ fn main() -> Result<()> {
                 repo_name,
             } => {
                 let repo_path = &path.join(repo_name);
-                let mut repo = repo::read_manifest_unsigned(repo_path)?;
+                let mut repo = repo::read_manifest(repo_path)?;
 
                 if title.is_some() {
                     repo.metadata.title = title;

src/repo/manifest_io.rs

@@ -7,18 +7,6 @@ use crate::{
     repo::RepoManifest,
 };
 
-/// Reads a manifest without verifying. This is best for AFTER it has been downloaded.
-///
-/// # Errors
-///
-/// - Filesystem errors (Permissions or doesn't exist)
-pub fn read_manifest_unsigned(repo_path: &Path) -> Result<RepoManifest> {
-    let manifest_serialized = fs::read_to_string(repo_path.join("manifest.yml"))?;
-    let manifest = serde_yaml::from_str(&manifest_serialized)?;
-
-    Ok(manifest)
-}
-
 /// Reads a manifest and verifys it from the EXISTING key. This is best for GENERAL reading.
 ///
 /// # Warning
@@ -42,6 +30,14 @@ pub fn read_manifest(repo_path: &Path) -> Result<RepoManifest> {
     Ok(manifest)
 }
 
+fn read_manifest_unsigned(repo_path: &Path) -> Result<RepoManifest> {
+    let manifest_serialized = fs::read_to_string(repo_path.join("manifest.yml"))?;
+
+    let manifest: RepoManifest = serde_yaml::from_str(&manifest_serialized)?;
+
+    Ok(manifest)
+}
+
 /// Reads a manifest and verifys it. This is best for WHEN it has been downloaded.
 ///
 /// # Errors
@@ -62,8 +58,7 @@ pub fn read_manifest_signed(repo_path: &Path, public_key_serialized: &str) -> Re
     Ok(manifest)
 }
 
-/// Replaces the existing manifest with another one
-/// Verifies that it is correct
+/// Replaces the existing manifest with another one, and verifies that it is correct
 ///
 /// # Errors
 ///
@@ -98,11 +93,79 @@ pub fn update_manifest(
     Ok(())
 }
 
-fn atomic_replace(repo_path: &Path, filename: &str, contents: &[u8]) -> Result<()> {
-    let new_path = &repo_path.join(filename.to_owned() + ".new");
+fn atomic_replace(base_path: &Path, filename: &str, contents: &[u8]) -> Result<()> {
+    let new_path = &base_path.join(filename.to_owned() + ".new");
 
     fs::write(new_path, contents)?;
-    fs::rename(new_path, repo_path.join(filename))?;
+    fs::rename(new_path, base_path.join(filename))?;
 
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use temp_dir::TempDir;
+
+    use crate::{crypto::signing::sign, repo::create};
+
+    use super::*;
+
+    #[test]
+    fn test_atomic_replace_basic() -> Result<()> {
+        let temp_dir = TempDir::new()?;
+        fs::write(temp_dir.path().join("file"), "previous_contents")?;
+        atomic_replace(temp_dir.path(), "file", b"new_contents")?;
+
+        assert_eq!(
+            fs::read_to_string(temp_dir.path().join("file"))?,
+            "new_contents"
+        );
+        assert!(!temp_dir.path().join("file.new").exists());
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_update_manifest_valid_and_invalid() -> Result<()> {
+        let repo = TempDir::new()?;
+        let repo_path = repo.path();
+        create(repo_path)?;
+
+        let old_manifest = read_manifest(repo_path)?;
+
+        // Build a new manifest with small change
+        let mut new_manifest = old_manifest;
+        new_manifest.metadata.title = Some("NewName".into());
+
+        let serialized = serde_yaml::to_string(&new_manifest)?;
+
+        // Sign it with the right key
+        let signature = sign(repo_path, &serialized)?;
+
+        // Update should succeed
+        update_manifest(repo_path, &serialized, &signature.to_bytes())?;
+
+        let updated = read_manifest(repo_path)?;
+        assert_eq!(updated.metadata.title, Some("NewName".into()));
+
+        // Now try with invalid signature
+        let bad_signature = b"garbage_signature";
+        assert!(update_manifest(repo_path, &serialized, bad_signature).is_err());
+
+        Ok(())
+    }
+
+    #[test]
+    fn test_read_signed_manifest() -> Result<()> {
+        let repo = TempDir::new()?;
+        let repo_path = repo.path();
+        create(repo_path)?;
+
+        let manifest = read_manifest(repo_path)?;
+        let manifest_signed = read_manifest_signed(repo_path, &manifest.public_key)?;
+
+        assert_eq!(manifest.edition, manifest_signed.edition);
+
+        Ok(())
+    }
+}

src/repo/mod.rs

@@ -170,15 +170,15 @@ mod tests {
     }
 
     #[test]
-    fn test_create_and_read_unsigned() {
+    fn test_create_and_read() {
         let tmp = TempDir::new().unwrap();
         let repo_path = tmp.path();
 
         // Create repo
         create(repo_path).unwrap();
 
         // Read unsigned manifest
-        let manifest = read_manifest_unsigned(repo_path).unwrap();
+        let manifest = read_manifest(repo_path).unwrap();
         assert_eq!(manifest.edition, "2025");
         assert!(manifest.public_key.len() > 10);
         assert!(manifest.packages.is_empty());
@@ -188,18 +188,6 @@ mod tests {
         assert!(repo_path.join("manifest.yml.sig").exists());
     }
 
-    #[test]
-    fn test_read_signed_manifest() {
-        let tmp = TempDir::new().unwrap();
-        let repo_path = tmp.path();
-        create(repo_path).unwrap();
-
-        let manifest = read_manifest_unsigned(repo_path).unwrap();
-        let manifest_signed = read_manifest_signed(repo_path, &manifest.public_key).unwrap();
-
-        assert_eq!(manifest.edition, manifest_signed.edition);
-    }
-
     #[test]
     fn test_tampered_manifest_fails() -> Result<()> {
         let tmp = TempDir::new()?;
@@ -211,8 +199,7 @@ mod tests {
         contents.push_str("\n# sneaky hacker change");
         fs::write(repo_path.join("manifest.yml"), contents)?;
 
-        let manifest = read_manifest_unsigned(repo_path)?;
-        let result = read_manifest_signed(repo_path, &manifest.public_key);
+        let result = read_manifest(repo_path);
 
         assert!(
             result.is_err(),
@@ -221,33 +208,4 @@ mod tests {
 
         Ok(())
     }
-
-    #[test]
-    fn test_update_manifest_valid_and_invalid() {
-        let tmp = TempDir::new().unwrap();
-        let repo_path = tmp.path();
-        create(repo_path).unwrap();
-
-        let old_manifest = read_manifest_unsigned(repo_path).unwrap();
-
-        // Build a new manifest with small change
-        let mut new_manifest = old_manifest;
-        new_manifest.metadata.title = Some("NewName".into());
-
-        let serialized = serde_yaml::to_string(&new_manifest).unwrap();
-
-        // Sign it with the right key
-        let signature = sign(repo_path, &serialized).unwrap();
-
-        // Update should succeed
-        update_manifest(repo_path, &serialized, &signature.to_bytes()).unwrap();
-
-        let updated = read_manifest_unsigned(repo_path).unwrap();
-        assert_eq!(updated.metadata.title, Some("NewName".into()));
-
-        // Now try with invalid signature
-        let bad_sig = b"garbage_signature";
-        let result = update_manifest(repo_path, &serialized, bad_sig);
-        assert!(result.is_err());
-    }
 }