refactor

crates/signed_git/src/lib.rs

@@ -25,8 +25,8 @@ impl GitCache {
     /// Local path of the clone for a repository.
     pub fn repo_path(&self, addr: &RepoAddr) -> PathBuf {
         self.root
-            .join(addr.owner.to_hex())
-            .join(sanitize_path_component(&addr.id))
+            .join(addr.public_key.to_hex())
+            .join(sanitize_path_component(&addr.identifier))
     }
 
     /// Open an existing clone.
@@ -116,8 +116,14 @@ fn clone(url: &str, path: &Path) -> Result<gix::Repository> {
     Ok(repo)
 }
 
+/// Map an untrusted repository id to a safe single path component.
+///
+/// Replaces everything outside `[A-Za-z0-9._-]` with `_`, and rejects the
+/// special components `.` and `..` so the id can't escape the cache root
+/// when joined onto the owner directory.
 fn sanitize_path_component(id: &str) -> String {
-    id.chars()
+    let sanitized: String = id
+        .chars()
         .map(|c| {
             if c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.') {
                 c
@@ -125,5 +131,54 @@ fn sanitize_path_component(id: &str) -> String {
                 '_'
             }
         })
-        .collect()
+        .collect();
+
+    if sanitized == "." || sanitized == ".." {
+        return "_".to_owned();
+    }
+
+    sanitized
+}
+
+#[cfg(test)]
+mod tests {
+    use nostr::prelude::*;
+    use signed_core::repo_addr;
+
+    use super::*;
+
+    #[test]
+    fn keeps_plain_ids() {
+        assert_eq!(sanitize_path_component("my-repo"), "my-repo");
+        assert_eq!(sanitize_path_component("repo.v2"), "repo.v2");
+        assert_eq!(sanitize_path_component("a_b-c"), "a_b-c");
+    }
+
+    #[test]
+    fn replaces_unsafe_characters() {
+        assert_eq!(sanitize_path_component("a/b\\c:d"), "a_b_c_d");
+        assert_eq!(sanitize_path_component(""), "");
+    }
+
+    #[test]
+    fn blocks_parent_components() {
+        assert_eq!(sanitize_path_component(".."), "_");
+        assert_eq!(sanitize_path_component("."), "_");
+        // Separators are neutralized before the check, so these stay safe.
+        assert_eq!(sanitize_path_component("../.."), ".._..");
+        assert_eq!(sanitize_path_component("a/../b"), "a_.._b");
+    }
+
+    #[test]
+    fn repo_path_stays_inside_root() {
+        let cache = GitCache::new("/cache".into());
+        let owner = Keys::generate().public_key();
+
+        let path = cache.repo_path(&repo_addr(owner, ".."));
+        assert!(path.starts_with("/cache"));
+        assert_eq!(
+            path.file_name().map(|n| n.to_string_lossy().into_owned()),
+            Some("_".into())
+        );
+    }
 }