Eagerly reject unsupported Git schemes (#11514)

Initially, we were limiting Git schemes to HTTPS and SSH as only
supported schemes. We lost this validation in #3429. This incidentally
allowed file schemes, which apparently work with Git out of the box.

A caveat for this is that in tool.uv.sources, we parse the git field
always as URL. This caused a problem with #11425: repo = { git =
'c:\path\to\repo', rev = "xxxxx" } was parsed as a URL where c: is the
scheme, causing a bad error message down the line.

This PR:

* Puts Git URL validation back in place. It bans everything but HTTPS,
SSH, and file URLs. This could be a breaking change, if users were using
a git transport protocol were not aware of, even though never
intentionally supported.
* Allows file: URL in Git: This seems to be supported by Git and we were
supporting it albeit unintentionally, so it's reasonable to continue to
support it.
* It does not allow relative paths in the git field in tool.uv.sources.
Absolute file URLs are supported, whether we want relative file URLs for
Git too should be discussed separately.

Closes #3429: We reject the input with a proper error message, while
hinting the user towards file:. If there's still desire for relative
path support, we can keep it open.

---------

Co-authored-by: Charlie Marsh <[email protected]>

Author: konstin

Cargo.lock

@@ -714,9 +714,7 @@ impl GitSourceDist {
     /// Return the [`ParsedUrl`] for the distribution.
     pub fn parsed_url(&self) -> ParsedUrl {
         ParsedUrl::Git(ParsedGitUrl::from_source(
-            self.git.repository().clone(),
-            self.git.reference().clone(),
-            self.git.precise(),
+            (*self.git).clone(),
             self.subdirectory.clone(),
         ))
     }

crates/uv-distribution-types/src/lib.rs

@@ -266,10 +266,8 @@ impl From<&ResolvedDist> for RequirementSource {
                     }
                 }
                 Dist::Source(SourceDist::Git(sdist)) => RequirementSource::Git {
+                    git: (*sdist.git).clone(),
                     url: sdist.url.clone(),
-                    repository: sdist.git.repository().clone(),
-                    reference: sdist.git.reference().clone(),
-                    precise: sdist.git.precise(),
                     subdirectory: sdist.subdirectory.clone(),
                 },
                 Dist::Source(SourceDist::Path(sdist)) => RequirementSource::Path {

crates/uv-distribution-types/src/resolution.rs

@@ -8,7 +8,7 @@ use url::Url;
 
 use uv_distribution_filename::DistExtension;
 use uv_distribution_types::{Index, IndexLocations, IndexName, Origin};
-use uv_git_types::GitReference;
+use uv_git_types::{GitReference, GitUrl, GitUrlParseError};
 use uv_normalize::{ExtraName, GroupName, PackageName};
 use uv_pep440::VersionSpecifiers;
 use uv_pep508::{looks_like_git_repository, MarkerTree, VerbatimUrl, VersionOrUrl};
@@ -291,9 +291,7 @@ impl LoweredRequirement {
                                         .expect("Workspace member must be relative");
                                 let subdirectory = uv_fs::normalize_path_buf(subdirectory);
                                 RequirementSource::Git {
-                                    repository: git_member.git_source.git.repository().clone(),
-                                    reference: git_member.git_source.git.reference().clone(),
-                                    precise: git_member.git_source.git.precise(),
+                                    git: git_member.git_source.git.clone(),
                                     subdirectory: if subdirectory == PathBuf::new() {
                                         None
                                     } else {
@@ -497,6 +495,8 @@ pub enum LoweringError {
     UndeclaredWorkspacePackage(PackageName),
     #[error("Can only specify one of: `rev`, `tag`, or `branch`")]
     MoreThanOneGitRef,
+    #[error(transparent)]
+    GitUrlParse(#[from] GitUrlParseError),
     #[error("Package `{0}` references an undeclared index: `{1}`")]
     MissingIndex(PackageName, IndexName),
     #[error("Workspace members are not allowed in non-workspace contexts")]
@@ -575,9 +575,7 @@ fn git_source(
 
     Ok(RequirementSource::Git {
         url,
-        repository,
-        reference,
-        precise: None,
+        git: GitUrl::from_reference(repository, reference)?,
         subdirectory,
     })
 }
@@ -679,9 +677,7 @@ fn path_source(
                 .expect("Workspace member must be relative");
             let subdirectory = uv_fs::normalize_path_buf(subdirectory);
             return Ok(RequirementSource::Git {
-                repository: git_member.git_source.git.repository().clone(),
-                reference: git_member.git_source.git.reference().clone(),
-                precise: git_member.git_source.git.precise(),
+                git: git_member.git_source.git.clone(),
                 subdirectory: if subdirectory == PathBuf::new() {
                     None
                 } else {

crates/uv-distribution/src/metadata/lowering.rs

@@ -1,12 +1,22 @@
 pub use crate::github::GitHubRepository;
 pub use crate::oid::{GitOid, OidParseError};
 pub use crate::reference::GitReference;
+
+use thiserror::Error;
 use url::Url;
 
 mod github;
 mod oid;
 mod reference;
 
+#[derive(Debug, Error)]
+pub enum GitUrlParseError {
+    #[error(
+        "Unsupported Git URL scheme `{0}:` in `{1}` (expected one of `https:`, `ssh:`, or `file:`)"
+    )]
+    UnsupportedGitScheme(String, Url),
+}
+
 /// A URL reference to a Git repository.
 #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Hash, Ord)]
 pub struct GitUrl {
@@ -21,21 +31,42 @@ pub struct GitUrl {
 
 impl GitUrl {
     /// Create a new [`GitUrl`] from a repository URL and a reference.
-    pub fn from_reference(repository: Url, reference: GitReference) -> Self {
-        Self {
-            repository,
-            reference,
-            precise: None,
-        }
+    pub fn from_reference(
+        repository: Url,
+        reference: GitReference,
+    ) -> Result<Self, GitUrlParseError> {
+        Self::from_fields(repository, reference, None)
     }
 
     /// Create a new [`GitUrl`] from a repository URL and a precise commit.
-    pub fn from_commit(repository: Url, reference: GitReference, precise: GitOid) -> Self {
-        Self {
+    pub fn from_commit(
+        repository: Url,
+        reference: GitReference,
+        precise: GitOid,
+    ) -> Result<Self, GitUrlParseError> {
+        Self::from_fields(repository, reference, Some(precise))
+    }
+
+    /// Create a new [`GitUrl`] from a repository URL and a precise commit, if known.
+    pub fn from_fields(
+        repository: Url,
+        reference: GitReference,
+        precise: Option<GitOid>,
+    ) -> Result<Self, GitUrlParseError> {
+        match repository.scheme() {
+            "https" | "ssh" | "file" => {}
+            unsupported => {
+                return Err(GitUrlParseError::UnsupportedGitScheme(
+                    unsupported.to_string(),
+                    repository,
+                ))
+            }
+        }
+        Ok(Self {
             repository,
             reference,
-            precise: Some(precise),
-        }
+            precise,
+        })
     }
 
     /// Set the precise [`GitOid`] to use for this Git URL.
@@ -69,7 +100,7 @@ impl GitUrl {
 }
 
 impl TryFrom<Url> for GitUrl {
-    type Error = OidParseError;
+    type Error = GitUrlParseError;
 
     /// Initialize a [`GitUrl`] source from a URL.
     fn try_from(mut url: Url) -> Result<Self, Self::Error> {
@@ -89,7 +120,7 @@ impl TryFrom<Url> for GitUrl {
             url.set_path(&prefix);
         }
 
-        Ok(Self::from_reference(url, reference))
+        Self::from_reference(url, reference)
     }
 }
 

crates/uv-git-types/src/lib.rs

@@ -97,9 +97,7 @@ impl RequirementSatisfaction {
             }
             RequirementSource::Git {
                 url: _,
-                repository: requested_repository,
-                reference: _,
-                precise: requested_precise,
+                git: requested_git,
                 subdirectory: requested_subdirectory,
             } => {
                 let InstalledDist::Url(InstalledDirectUrlDist { direct_url, .. }) = &distribution
@@ -129,21 +127,25 @@ impl RequirementSatisfaction {
                 }
 
                 if !RepositoryUrl::parse(installed_url).is_ok_and(|installed_url| {
-                    installed_url == RepositoryUrl::new(requested_repository)
+                    installed_url == RepositoryUrl::new(requested_git.repository())
                 }) {
                     debug!(
                         "Repository mismatch: {:?} vs. {:?}",
-                        installed_url, requested_repository
+                        installed_url,
+                        requested_git.repository()
                     );
                     return Ok(Self::Mismatch);
                 }
 
                 // TODO(charlie): It would be more consistent for us to compare the requested
                 // revisions here.
-                if installed_precise.as_deref() != requested_precise.as_ref().map(GitOid::as_str) {
+                if installed_precise.as_deref()
+                    != requested_git.precise().as_ref().map(GitOid::as_str)
+                {
                     debug!(
                         "Precise mismatch: {:?} vs. {:?}",
-                        installed_precise, requested_precise
+                        installed_precise,
+                        requested_git.precise()
                     );
                     return Ok(Self::OutOfDate);
                 }

crates/uv-installer/src/satisfies.rs

@@ -5,7 +5,7 @@ use thiserror::Error;
 use url::{ParseError, Url};
 
 use uv_distribution_filename::{DistExtension, ExtensionError};
-use uv_git_types::{GitOid, GitReference, GitUrl, OidParseError};
+use uv_git_types::{GitUrl, GitUrlParseError};
 use uv_pep508::{
     looks_like_git_repository, Pep508Url, UnnamedRequirementUrl, VerbatimUrl, VerbatimUrlError,
 };
@@ -22,8 +22,8 @@ pub enum ParsedUrlError {
     },
     #[error("Invalid path in file URL: `{0}`")]
     InvalidFileUrl(String),
-    #[error("Failed to parse Git reference from URL: `{0}`")]
-    GitOidParse(String, #[source] OidParseError),
+    #[error(transparent)]
+    GitUrlParse(#[from] GitUrlParseError),
     #[error("Not a valid URL: `{0}`")]
     UrlParse(String, #[source] ParseError),
     #[error(transparent)]
@@ -244,17 +244,7 @@ pub struct ParsedGitUrl {
 
 impl ParsedGitUrl {
     /// Construct a [`ParsedGitUrl`] from a Git requirement source.
-    pub fn from_source(
-        repository: Url,
-        reference: GitReference,
-        precise: Option<GitOid>,
-        subdirectory: Option<PathBuf>,
-    ) -> Self {
-        let url = if let Some(precise) = precise {
-            GitUrl::from_commit(repository, reference, precise)
-        } else {
-            GitUrl::from_reference(repository, reference)
-        };
+    pub fn from_source(url: GitUrl, subdirectory: Option<PathBuf>) -> Self {
         Self { url, subdirectory }
     }
 }
@@ -274,8 +264,7 @@ impl TryFrom<Url> for ParsedGitUrl {
             .strip_prefix("git+")
             .unwrap_or(url_in.as_str());
         let url = Url::parse(url).map_err(|err| ParsedUrlError::UrlParse(url.to_string(), err))?;
-        let url = GitUrl::try_from(url)
-            .map_err(|err| ParsedUrlError::GitOidParse(url_in.to_string(), err))?;
+        let url = GitUrl::try_from(url)?;
         Ok(Self { url, subdirectory })
     }
 }