Preserve Git username for SSH dependencies (#6335)

## Summary

We're gonna work on a more comprehensive review of whether we should
preserve the username here, but for now, `git@` is effectively a
convention for GitHub and GitLab etc.

Closes https://github.com/astral-sh/uv/issues/6305.

## Test Plan

I guess we don't have infrastructure for testing SSH private keys right
now, but...

```
❯ cargo run init foo
❯ cd foo
❯ cargo run add git+ssh://git@github.com/astral-sh/mkdocs-material-insiders.git
```
This commit is contained in:
Charlie Marsh 2024-08-21 11:22:45 -04:00 committed by GitHub
parent cabca7bf23
commit d627dea51e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 29 additions and 19 deletions

View File

@ -80,14 +80,12 @@ impl Requirement {
subdirectory,
url,
} => {
// Redact the repository URL.
let _ = repository.set_password(None);
let _ = repository.set_username("");
// Redact the repository URL, but allow `git@`.
redact_git_credentials(&mut repository);
// Redact the PEP 508 URL.
let mut url = url.to_url();
let _ = url.set_password(None);
let _ = url.set_username("");
redact_git_credentials(&mut url);
let url = VerbatimUrl::from_url(url);
Self {
@ -598,8 +596,7 @@ impl From<RequirementSource> for RequirementSourceWire {
let mut url = repository;
// Redact the credentials.
let _ = url.set_username("");
let _ = url.set_password(None);
redact_git_credentials(&mut url);
// Clear out any existing state.
url.set_fragment(None);
@ -699,8 +696,7 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
repository.set_query(None);
// Redact the credentials.
let _ = repository.set_username("");
let _ = repository.set_password(None);
redact_git_credentials(&mut repository);
// Create a PEP 508-compatible URL.
let mut url = Url::parse(&format!("git+{repository}"))?;
@ -765,6 +761,18 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
}
}
/// Remove the credentials from a Git URL, allowing the generic `git` username (without a password)
/// in SSH URLs, as in, `ssh://git@github.com/...`.
pub fn redact_git_credentials(url: &mut Url) {
// For URLs that use the `git` convention (i.e., `ssh://git@github.com/...`), avoid dropping the
// username.
if url.scheme() == "ssh" && url.username() == "git" && url.password().is_none() {
return;
}
let _ = url.set_password(None);
let _ = url.set_username("");
}
#[cfg(test)]
mod tests {
use std::path::{Path, PathBuf};

View File

@ -25,7 +25,10 @@ use distribution_types::{
use pep440_rs::Version;
use pep508_rs::{MarkerEnvironment, MarkerTree, VerbatimUrl, VerbatimUrlError};
use platform_tags::{TagCompatibility, TagPriority, Tags};
use pypi_types::{HashDigest, ParsedArchiveUrl, ParsedGitUrl, Requirement, RequirementSource};
use pypi_types::{
redact_git_credentials, HashDigest, ParsedArchiveUrl, ParsedGitUrl, Requirement,
RequirementSource,
};
use uv_configuration::ExtrasSpecification;
use uv_distribution::DistributionDatabase;
use uv_fs::{relative_to, PortablePath, PortablePathBuf, Simplified};
@ -2367,8 +2370,7 @@ fn locked_git_url(git_dist: &GitSourceDist) -> Url {
let mut url = git_dist.git.repository().clone();
// Redact the credentials.
let _ = url.set_username("");
let _ = url.set_password(None);
redact_git_credentials(&mut url);
// Clear out any existing state.
url.set_fragment(None);
@ -2830,14 +2832,12 @@ fn normalize_requirement(
subdirectory,
url,
} => {
// Redact the repository URL.
let _ = repository.set_password(None);
let _ = repository.set_username("");
// Redact the credentials.
redact_git_credentials(&mut repository);
// Redact the PEP 508 URL.
let mut url = url.to_url();
let _ = url.set_password(None);
let _ = url.set_username("");
redact_git_credentials(&mut url);
let url = VerbatimUrl::from_url(url);
Ok(Requirement {

View File

@ -6,6 +6,7 @@ use anyhow::{bail, Context, Result};
use cache_key::RepositoryUrl;
use owo_colors::OwoColorize;
use pep508_rs::{ExtraName, Requirement, VersionOrUrl};
use pypi_types::redact_git_credentials;
use rustc_hash::{FxBuildHasher, FxHashMap};
use tracing::debug;
use uv_auth::{store_credentials_from_url, Credentials};
@ -360,8 +361,9 @@ pub(crate) async fn add(
if let Some(credentials) = credentials {
debug!("Caching credentials for: {git}");
GIT_STORE.insert(RepositoryUrl::new(&git), credentials);
let _ = git.set_username("");
let _ = git.set_password(None);
// Redact the credentials.
redact_git_credentials(&mut git);
};
Some(Source::Git {
git,