From 5a94cbd634929597461d1f3a27fe59bd3f6d96ef Mon Sep 17 00:00:00 2001 From: Alpha Chen Date: Sat, 25 Apr 2026 06:56:41 -0700 Subject: [PATCH] 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 --- src/bin/quire/commands/exec.rs | 62 ++++-------------------------- src/bin/quire/commands/repo/new.rs | 15 ++++---- src/bin/quire/commands/repo/rm.rs | 12 +++--- src/repo.rs | 61 +++++++++++++++++++++-------- 4 files changed, 67 insertions(+), 83 deletions(-) diff --git a/src/bin/quire/commands/exec.rs b/src/bin/quire/commands/exec.rs index b1219d5..d09da5e 100644 --- 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 { - 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 ed03fa9..1ded6d3 100644 --- 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 00e6c4e..77013dd 100644 --- 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 da4e8e5..451cc37 100644 --- 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 { + 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 { +fn validate_segments(name: &str) -> Result { 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()); } } -- 2.54.0