From d07b587f3facc2f0f24865001e9bb5846c27a1c8 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Tue, 20 Feb 2024 18:12:56 -0600 Subject: [PATCH] Retain passwords in Git URLs (#1717) Fixes handling of GitHub PATs in HTTPS URLs, which were otherwise dropped. We now supporting the following authentication schemes: ``` git+https://:/... git+https:///... ``` On Windows, the username is required. We can consider adding a special-case for this in the future, but this just matches libgit2's behavior. I tested with fine-grained tokens, OAuth tokens, and "classic" tokens. There's test coverage for fine-grained tokens in CI where we use a real private repository and PAT. Yes, the PAT is committed to make this test usable by anyone. It has read-only permissions to the single repository, expires Feb 1 2025, and is in an isolated organization and GitHub account. Does not yet address SSH authentication. Related: - https://github.com/astral-sh/uv/issues/1514 - https://github.com/astral-sh/uv/issues/1452 --- Cargo.lock | 1 + crates/cache-key/src/canonical_url.rs | 8 +- crates/uv-git/src/lib.rs | 11 ++- crates/uv/Cargo.toml | 1 + crates/uv/tests/common/mod.rs | 9 ++ crates/uv/tests/pip_install.rs | 136 ++++++++++++++++++++++++++ 6 files changed, 161 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7de6fe126..663866504 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4142,6 +4142,7 @@ dependencies = [ "anyhow", "assert_cmd", "assert_fs", + "base64 0.21.7", "chrono", "clap", "clap_complete_command", diff --git a/crates/cache-key/src/canonical_url.rs b/crates/cache-key/src/canonical_url.rs index 62868f5f2..86db6af2d 100644 --- a/crates/cache-key/src/canonical_url.rs +++ b/crates/cache-key/src/canonical_url.rs @@ -109,8 +109,12 @@ impl RepositoryUrl { // If a Git URL ends in a reference (like a branch, tag, or commit), remove it. if url.scheme().starts_with("git+") { - if let Some((prefix, _)) = url.as_str().rsplit_once('@') { - url = prefix.parse().unwrap(); + if let Some(prefix) = url + .path() + .rsplit_once('@') + .map(|(prefix, _suffix)| prefix.to_string()) + { + url.set_path(&prefix); } } diff --git a/crates/uv-git/src/lib.rs b/crates/uv-git/src/lib.rs index f120a8e77..682c59db6 100644 --- a/crates/uv-git/src/lib.rs +++ b/crates/uv-git/src/lib.rs @@ -65,10 +65,15 @@ impl TryFrom for GitUrl { // If the URL ends with a reference, like `https://git.example.com/MyProject.git@v1.0`, // extract it. let mut reference = GitReference::DefaultBranch; - if let Some((prefix, rev)) = url.as_str().rsplit_once('@') { - reference = GitReference::from_rev(rev); - url = Url::parse(prefix)?; + if let Some((prefix, suffix)) = url + .path() + .rsplit_once('@') + .map(|(prefix, suffix)| (prefix.to_string(), suffix.to_string())) + { + reference = GitReference::from_rev(&suffix); + url.set_path(&prefix); } + let precise = if let GitReference::FullCommit(rev) = &reference { Some(GitSha::from_str(rev)?) } else { diff --git a/crates/uv/Cargo.toml b/crates/uv/Cargo.toml index 120091b1d..20c8fa9e0 100644 --- a/crates/uv/Cargo.toml +++ b/crates/uv/Cargo.toml @@ -39,6 +39,7 @@ requirements-txt = { path = "../requirements-txt" } anstream = { workspace = true } anyhow = { workspace = true } +base64 = { workspace = true } chrono = { workspace = true } clap = { workspace = true, features = ["derive"] } clap_complete_command = { workspace = true } diff --git a/crates/uv/tests/common/mod.rs b/crates/uv/tests/common/mod.rs index 22cd714bb..633ab58ad 100644 --- a/crates/uv/tests/common/mod.rs +++ b/crates/uv/tests/common/mod.rs @@ -87,6 +87,15 @@ impl TestContext { .current_dir(&self.temp_dir) .assert() } + + /// Assert a package is installed with the given version. + pub fn assert_installed(&self, package: &'static str, version: &'static str) { + self.assert_command( + format!("import {package} as package; print(package.__version__, end='')").as_str(), + ) + .success() + .stdout(version); + } } pub fn venv_to_interpreter(venv: &Path) -> PathBuf { diff --git a/crates/uv/tests/pip_install.rs b/crates/uv/tests/pip_install.rs index 1af3ac748..7cb8e1fd3 100644 --- a/crates/uv/tests/pip_install.rs +++ b/crates/uv/tests/pip_install.rs @@ -5,7 +5,9 @@ use std::process::Command; use anyhow::Result; use assert_cmd::prelude::*; use assert_fs::prelude::*; +use base64::{prelude::BASE64_STANDARD as base64, Engine}; use indoc::indoc; +use itertools::Itertools; use url::Url; use common::{uv_snapshot, TestContext, EXCLUDE_NEWER, INSTA_FILTERS}; @@ -14,6 +16,24 @@ use crate::common::get_bin; mod common; +// This is a fine-grained token that only has read-only access to the `uv-private-pypackage` repository +const READ_ONLY_GITHUB_TOKEN: &[&str] = &[ + "Z2l0aHViX3BhdA==", + "MTFCR0laQTdRMGdXeGsweHV6ekR2Mg==", + "NVZMaExzZmtFMHZ1ZEVNd0pPZXZkV040WUdTcmk2WXREeFB4TFlybGlwRTZONEpHV01FMnFZQWJVUm4=", +]; + +/// Decode a split, base64 encoded authentication token. +/// We split and encode the token to bypass revoke by GitHub's secret scanning +fn decode_token(content: &[&str]) -> String { + let token = content + .iter() + .map(|part| base64.decode(part).unwrap()) + .map(|decoded| std::str::from_utf8(decoded.as_slice()).unwrap().to_string()) + .join("_"); + token +} + /// Create a `pip install` command with options shared across scenarios. fn command(context: &TestContext) -> Command { let mut command = Command::new(get_bin()); @@ -769,6 +789,122 @@ fn install_no_index_version() { context.assert_command("import flask").failure(); } +/// Install a package from a public GitHub repository +#[test] +#[cfg(feature = "git")] +fn install_git_public_https() { + let context = TestContext::new("3.8"); + + uv_snapshot!(command(&context) + .arg("uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage") + , @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + + uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979) + "###); + + context.assert_installed("uv_public_pypackage", "0.1.0"); +} + +/// Install a package from a private GitHub repository using a PAT +#[test] +#[cfg(all(not(windows), feature = "git"))] +fn install_git_private_https_pat() { + let context = TestContext::new("3.8"); + let token = decode_token(READ_ONLY_GITHUB_TOKEN); + + let mut filters = INSTA_FILTERS.to_vec(); + filters.insert(0, (&token, "***")); + + uv_snapshot!(filters, command(&context) + .arg(format!("uv-private-pypackage @ git+https://{token}@github.com/astral-test/uv-private-pypackage")) + , @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + + uv-private-pypackage==0.1.0 (from git+https://***@github.com/astral-test/uv-private-pypackage@6c09ce9ae81f50670a60abd7d95f30dd416d00ac) + "###); + + context.assert_installed("uv_private_pypackage", "0.1.0"); +} + +/// Install a package from a private GitHub repository at a specific commit using a PAT +#[test] +#[cfg(feature = "git")] +fn install_git_private_https_pat_at_ref() { + let context = TestContext::new("3.8"); + let token = decode_token(READ_ONLY_GITHUB_TOKEN); + + let mut filters = INSTA_FILTERS.to_vec(); + filters.insert(0, (&token, "***")); + filters.push((r"git\+https://", "")); + + // A user is _required_ on Windows + let user = if cfg!(windows) { + filters.push((r"git:", "")); + "git:" + } else { + "" + }; + + uv_snapshot!(filters, command(&context) + .arg(format!("uv-private-pypackage @ git+https://{user}{token}@github.com/astral-test/uv-private-pypackage@6c09ce9ae81f50670a60abd7d95f30dd416d00ac")) + , @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + + uv-private-pypackage==0.1.0 (from ***@github.com/astral-test/uv-private-pypackage@6c09ce9ae81f50670a60abd7d95f30dd416d00ac) + "###); + + context.assert_installed("uv_private_pypackage", "0.1.0"); +} + +/// Install a package from a private GitHub repository using a PAT and username +/// An arbitrary username is supported when using a PAT. +#[test] +#[cfg(feature = "git")] +fn install_git_private_https_pat_and_username() { + let context = TestContext::new("3.8"); + let token = decode_token(READ_ONLY_GITHUB_TOKEN); + let user = "astral-test-bot"; + + let mut filters = INSTA_FILTERS.to_vec(); + filters.insert(0, (&token, "***")); + filters.push(("failed to clone into: .*", "failed to clone into: [PATH]")); + + uv_snapshot!(filters, command(&context) + .arg(format!("uv-private-pypackage @ git+https://{user}:{token}@github.com/astral-test/uv-private-pypackage")) + , @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + + uv-private-pypackage==0.1.0 (from git+https://astral-test-bot:***@github.com/astral-test/uv-private-pypackage@6c09ce9ae81f50670a60abd7d95f30dd416d00ac) + "###); + + context.assert_installed("uv_private_pypackage", "0.1.0"); +} + /// Install a package without using pre-built wheels. #[test] fn reinstall_no_binary() {