Extract Repo struct to unify path validation

All repo name validation now goes through Repo::from_name(). The exec
command strips the SSH leading slash before calling it, keeping the
Repo type focused on name rules only. Removed the duplicate
validate_repo_path function from exec.rs.

Assisted-by: GLM-5.1 via pi

diff --git a/src/bin/quire/commands/exec.rs b/src/bin/quire/commands/exec.rs
index b1219d5dc6e450410ebf19cc066b84d666c04b6b..d09da5e9b5de0dabe605bbd8fea4d0becdf30649 100644 (file)
--- a/src/bin/quire/commands/exec.rs
+++ b/src/bin/quire/commands/exec.rs
@@ -3,6 +3,7 @@ use std::process::Command;
 
 use miette::{Context, IntoDiagnostic, Result, bail, ensure};
 
+use quire::repo::Repo;
 use quire::Config;
 
 const GIT_COMMANDS: &[&str] = &[
@@ -46,12 +47,14 @@ fn dispatch_git(config: &Config, git_cmd: &str, args: &[String]) -> Result<()> {
         args.len()
     );
 
-    let repo = validate_repo_path(&args[0])?;
+    let path = args[0].trim_start_matches('/');
+    ensure!(!path.is_empty(), "empty repository path");
 
-    let repo_dir = config.repos_dir.join(&repo);
-    ensure!(repo_dir.is_dir(), "repository not found: {repo}");
+    let repo = Repo::from_name(path)?;
+    let repo_dir = repo.path(&config.repos_dir);
+    ensure!(repo_dir.is_dir(), "repository not found: {}", repo.name());
 
-    tracing::info!(%git_cmd, %repo, "dispatching git command");
+    tracing::info!(%git_cmd, name = %repo.name(), "dispatching git command");
     let err = Command::new(git_cmd)
         .arg(".")
         .current_dir(&repo_dir)
@@ -70,55 +73,4 @@ fn dispatch_quire(_config: &Config, args: &[String]) -> Result<()> {
     bail!("exec failed: {err}")
 }
 
-/// Validate a repo path argument from the SSH protocol.
-///
-/// Git sends paths like '/foo.git'. We strip the leading slash,
-/// reject path traversal (..), require a .git suffix, and reject
-/// empty or double-slash paths.
-fn validate_repo_path(raw: &str) -> Result<String> {
-    let path = raw.trim_start_matches('/');
-
-    ensure!(!path.is_empty(), "empty repository path");
-    ensure!(!path.contains(".."), "invalid repository path: {raw}");
-    ensure!(
-        path.ends_with(".git"),
-        "invalid repository path (must end in .git): {raw}"
-    );
-    ensure!(!path.contains("//"), "invalid repository path: {raw}");
-
-    Ok(path.to_string())
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn valid_repo_paths() {
-        assert_eq!(validate_repo_path("/foo.git").unwrap(), "foo.git");
-        assert_eq!(validate_repo_path("foo.git").unwrap(), "foo.git");
-        assert_eq!(validate_repo_path("/work/foo.git").unwrap(), "work/foo.git");
-    }
-
-    #[test]
-    fn rejects_traversal() {
-        assert!(validate_repo_path("/../etc/passwd").is_err());
-        assert!(validate_repo_path("/foo/../../bar.git").is_err());
-    }
-
-    #[test]
-    fn rejects_no_git_suffix() {
-        assert!(validate_repo_path("/foo").is_err());
-    }
 
-    #[test]
-    fn rejects_empty() {
-        assert!(validate_repo_path("").is_err());
-        assert!(validate_repo_path("/").is_err());
-    }
-
-    #[test]
-    fn rejects_double_slash() {
-        assert!(validate_repo_path("/foo//bar.git").is_err());
-    }
-}

diff --git a/src/bin/quire/commands/repo/new.rs b/src/bin/quire/commands/repo/new.rs
index ed03fa9777f6ed028803aad793c55c417b812a68..1ded6d364d2175a246043bacaf32105958611474 100644 (file)
--- a/src/bin/quire/commands/repo/new.rs
+++ b/src/bin/quire/commands/repo/new.rs
@@ -3,15 +3,16 @@ use std::process::Command;
 use miette::{IntoDiagnostic, Result, ensure};
 
 use quire::Config;
-use quire::repo::validate_name;
+use quire::repo::Repo;
 
 pub async fn run(config: &Config, name: &str) -> Result<()> {
-    let name = validate_name(name)?;
-    let repo_dir = config.repos_dir.join(&name);
+    let repo = Repo::from_name(name)?;
+    let repo_dir = repo.path(&config.repos_dir);
 
     ensure!(
         !repo_dir.exists(),
-        "repository already exists: {name}"
+        "repository already exists: {}",
+        repo.name()
     );
 
     // Create parent directory for grouped repos (e.g. work/foo.git).
@@ -20,15 +21,15 @@ pub async fn run(config: &Config, name: &str) -> Result<()> {
     }
 
     let status = Command::new("git")
-        .args(["init", "--bare", &name])
+        .args(["init", "--bare", repo.name()])
         .current_dir(&config.repos_dir)
         .status()
         .into_diagnostic()?;
 
     ensure!(status.success(), "git init failed");
 
-    tracing::info!(%name, "created repository");
-    println!("{name}");
+    tracing::info!(name = %repo.name(), "created repository");
+    println!("{}", repo.name());
 
     Ok(())
 }

diff --git a/src/bin/quire/commands/repo/rm.rs b/src/bin/quire/commands/repo/rm.rs
index 00e6c4ea31ea6af64485dc11e19e14959cde8d8e..77013dd803f886ca7cedc58edcdc92ba85de392a 100644 (file)
--- a/src/bin/quire/commands/repo/rm.rs
+++ b/src/bin/quire/commands/repo/rm.rs
@@ -1,14 +1,14 @@
 use miette::{IntoDiagnostic, Result, ensure};
 
 use quire::Config;
-use quire::repo::validate_name;
+use quire::repo::Repo;
 
 pub async fn run(config: &Config, name: &str) -> Result<()> {
-    let name = validate_name(name)?;
-    let repo_dir = config.repos_dir.join(&name);
+    let repo = Repo::from_name(name)?;
+    let repo_dir = repo.path(&config.repos_dir);
 
-    ensure!(repo_dir.exists(), "repository not found: {name}");
-    ensure!(repo_dir.is_dir(), "not a directory: {name}");
+    ensure!(repo_dir.exists(), "repository not found: {}", repo.name());
+    ensure!(repo_dir.is_dir(), "not a directory: {}", repo.name());
 
     fs_err::remove_dir_all(&repo_dir).into_diagnostic()?;
 
@@ -19,7 +19,7 @@ pub async fn run(config: &Config, name: &str) -> Result<()> {
         let _ = fs_err::remove_dir(parent);
     }
 
-    tracing::info!(%name, "removed repository");
+    tracing::info!(name = %repo.name(), "removed repository");
 
     Ok(())
 }

diff --git a/src/repo.rs b/src/repo.rs
index da4e8e567e7210ad537c4bbe5b16a69d627c603c..451cc37663db9eb393bd6d3f9e5b7ec555b56e3c 100644 (file)
--- a/src/repo.rs
+++ b/src/repo.rs
@@ -1,11 +1,40 @@
+use std::path::{Path, PathBuf};
+
 use miette::{Result, ensure};
 
-/// Validate a repository name for creation.
+/// A validated repository name relative to the repos directory.
+#[derive(Debug, Clone)]
+pub struct Repo {
+    name: String,
+}
+
+impl Repo {
+    /// Parse a repository name (e.g. `foo.git`, `work/foo.git`).
+    ///
+    /// Rejects path traversal, missing `.git` suffix, empty segments,
+    /// reserved path components, and more than one level of grouping.
+    pub fn from_name(name: &str) -> Result<Self> {
+        validate_segments(name)?;
+        Ok(Repo {
+            name: name.to_string(),
+        })
+    }
+
+    pub fn name(&self) -> &str {
+        &self.name
+    }
+
+    pub fn path(&self, repos_dir: &Path) -> PathBuf {
+        repos_dir.join(&self.name)
+    }
+}
+
+/// Validate segments of a repository name.
 ///
 /// Allows at most one level of grouping (e.g. `foo.git` or `work/foo.git`).
 /// Rejects path traversal, missing `.git` suffix, empty segments, and
 /// reserved path components.
-pub fn validate_name(name: &str) -> Result<String> {
+fn validate_segments(name: &str) -> Result<String> {
     ensure!(!name.is_empty(), "repository name cannot be empty");
     ensure!(!name.contains(".."), "invalid repository name: {name}");
     ensure!(
@@ -36,41 +65,43 @@ mod tests {
     use super::*;
 
     #[test]
-    fn valid_names() {
-        assert_eq!(validate_name("foo.git").unwrap(), "foo.git");
-        assert_eq!(validate_name("work/foo.git").unwrap(), "work/foo.git");
+    fn from_name_valid() {
+        assert_eq!(Repo::from_name("foo.git").unwrap().name(), "foo.git");
+        assert_eq!(
+            Repo::from_name("work/foo.git").unwrap().name(),
+            "work/foo.git"
+        );
     }
 
     #[test]
     fn rejects_empty() {
-        assert!(validate_name("").is_err());
+        assert!(Repo::from_name("").is_err());
     }
 
     #[test]
     fn rejects_traversal() {
-        assert!(validate_name("../foo.git").is_err());
-        assert!(validate_name("foo/../../bar.git").is_err());
-        assert!(validate_name("./foo.git").is_err());
+        assert!(Repo::from_name("../foo.git").is_err());
+        assert!(Repo::from_name("foo/../../bar.git").is_err());
+        assert!(Repo::from_name("./foo.git").is_err());
     }
 
     #[test]
     fn rejects_no_git_suffix() {
-        assert!(validate_name("foo").is_err());
+        assert!(Repo::from_name("foo").is_err());
     }
 
     #[test]
     fn rejects_deep_nesting() {
-        assert!(validate_name("a/b/c.git").is_err());
+        assert!(Repo::from_name("a/b/c.git").is_err());
     }
 
     #[test]
     fn rejects_double_slash() {
-        assert!(validate_name("foo//bar.git").is_err());
+        assert!(Repo::from_name("foo//bar.git").is_err());
     }
 
     #[test]
-    fn rejects_empty_segment() {
-        assert!(validate_name("/foo.git").is_err());
-        assert!(validate_name("foo//bar.git").is_err());
+    fn rejects_dot_git_segment() {
+        assert!(Repo::from_name("foo/.git").is_err());
     }
 }