From 8fac63d4ce1786deeb91992de15eb62dc331cfb9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 13 Aug 2024 21:30:02 -0400 Subject: [PATCH] Redact Git credentials from `pyproject.toml` (#6074) ## Summary We retain them if you use `--raw-sources`, but otherwise they're removed. We still respect them in the subsequent `uv.lock` via an in-process store. Closes #6056. --- Cargo.lock | 1 + crates/uv-auth/src/credentials.rs | 18 ++- crates/uv-auth/src/lib.rs | 22 +-- crates/uv-git/Cargo.toml | 1 + crates/uv-git/src/credentials.rs | 21 +++ crates/uv-git/src/lib.rs | 10 ++ crates/uv-git/src/source.rs | 18 ++- crates/uv/src/commands/project/add.rs | 38 ++++- crates/uv/tests/edit.rs | 192 +++++++++++++++++++++++++- crates/uv/tests/lock.rs | 3 - 10 files changed, 301 insertions(+), 23 deletions(-) create mode 100644 crates/uv-git/src/credentials.rs diff --git a/Cargo.lock b/Cargo.lock index 23285e6f4..543bec5ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4885,6 +4885,7 @@ dependencies = [ "tokio", "tracing", "url", + "uv-auth", "uv-fs", ] diff --git a/crates/uv-auth/src/credentials.rs b/crates/uv-auth/src/credentials.rs index 535dbc734..7e86715e8 100644 --- a/crates/uv-auth/src/credentials.rs +++ b/crates/uv-auth/src/credentials.rs @@ -10,7 +10,7 @@ use std::io::Write; use url::Url; #[derive(Clone, Debug, PartialEq)] -pub(crate) struct Credentials { +pub struct Credentials { /// The name of the user for authentication. username: Username, /// The password to use for authentication. @@ -114,7 +114,7 @@ impl Credentials { /// Parse [`Credentials`] from a URL, if any. /// /// Returns [`None`] if both [`Url::username`] and [`Url::password`] are not populated. - pub(crate) fn from_url(url: &Url) -> Option { + pub fn from_url(url: &Url) -> Option { if url.username().is_empty() && url.password().is_none() { return None; } @@ -203,6 +203,20 @@ impl Credentials { header } + /// Apply the credentials to the given URL. + /// + /// Any existing credentials will be overridden. + #[must_use] + pub fn apply(&self, mut url: Url) -> Url { + if let Some(username) = self.username() { + let _ = url.set_username(username); + } + if let Some(password) = self.password() { + let _ = url.set_password(Some(password)); + } + url + } + /// Attach the credentials to the given request. /// /// Any existing credentials will be overridden. diff --git a/crates/uv-auth/src/lib.rs b/crates/uv-auth/src/lib.rs index 947b6aa83..61c1c2825 100644 --- a/crates/uv-auth/src/lib.rs +++ b/crates/uv-auth/src/lib.rs @@ -1,20 +1,20 @@ +use std::sync::{Arc, LazyLock}; + +use tracing::trace; +use url::Url; + +use cache::CredentialsCache; +pub use credentials::Credentials; +pub use keyring::KeyringProvider; +pub use middleware::AuthMiddleware; +use realm::Realm; + mod cache; mod credentials; mod keyring; mod middleware; mod realm; -use std::sync::{Arc, LazyLock}; - -use cache::CredentialsCache; -use credentials::Credentials; - -pub use keyring::KeyringProvider; -pub use middleware::AuthMiddleware; -use realm::Realm; -use tracing::trace; -use url::Url; - // TODO(zanieb): Consider passing a cache explicitly throughout /// Global authentication cache for a uv invocation diff --git a/crates/uv-git/Cargo.toml b/crates/uv-git/Cargo.toml index c459387f8..916d6cffa 100644 --- a/crates/uv-git/Cargo.toml +++ b/crates/uv-git/Cargo.toml @@ -15,6 +15,7 @@ workspace = true [dependencies] cache-key = { workspace = true } uv-fs = { workspace = true } +uv-auth = { workspace = true } anyhow = { workspace = true } cargo-util = { workspace = true } diff --git a/crates/uv-git/src/credentials.rs b/crates/uv-git/src/credentials.rs new file mode 100644 index 000000000..b64ec2e2f --- /dev/null +++ b/crates/uv-git/src/credentials.rs @@ -0,0 +1,21 @@ +use std::collections::HashMap; +use std::sync::{Arc, RwLock}; + +use cache_key::RepositoryUrl; +use uv_auth::Credentials; + +/// A store for Git credentials. +#[derive(Debug, Default)] +pub struct GitStore(RwLock>>); + +impl GitStore { + /// Insert [`Credentials`] for the given URL into the store. + pub fn insert(&self, url: RepositoryUrl, credentials: Credentials) -> Option> { + self.0.write().unwrap().insert(url, Arc::new(credentials)) + } + + /// Get the [`Credentials`] for the given URL, if they exist. + pub fn get(&self, url: &RepositoryUrl) -> Option> { + self.0.read().unwrap().get(url).cloned() + } +} diff --git a/crates/uv-git/src/lib.rs b/crates/uv-git/src/lib.rs index f55b97cd3..5fa96f57b 100644 --- a/crates/uv-git/src/lib.rs +++ b/crates/uv-git/src/lib.rs @@ -1,5 +1,8 @@ +use std::sync::LazyLock; + use url::Url; +use crate::credentials::GitStore; pub use crate::git::GitReference; pub use crate::resolver::{ GitResolver, GitResolverError, RepositoryReference, ResolvedRepositoryReference, @@ -7,11 +10,17 @@ pub use crate::resolver::{ pub use crate::sha::{GitOid, GitSha, OidParseError}; pub use crate::source::{Fetch, GitSource, Reporter}; +mod credentials; mod git; mod resolver; mod sha; mod source; +/// Global authentication cache for a uv invocation. +/// +/// This is used to share Git credentials within a single process. +pub static GIT_STORE: LazyLock = LazyLock::new(GitStore::default); + /// A URL reference to a Git repository. #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Hash, Ord)] pub struct GitUrl { @@ -44,6 +53,7 @@ impl GitUrl { } } + /// Set the precise [`GitSha`] to use for this Git URL. #[must_use] pub fn with_precise(mut self, precise: GitSha) -> Self { self.precise = Some(precise); diff --git a/crates/uv-git/src/source.rs b/crates/uv-git/src/source.rs index dec18f25a..ff47fc468 100644 --- a/crates/uv-git/src/source.rs +++ b/crates/uv-git/src/source.rs @@ -1,6 +1,8 @@ //! Git support is derived from Cargo's implementation. //! Cargo is dual-licensed under either Apache 2.0 or MIT, at the user's choice. //! Source: + +use std::borrow::Cow; use std::path::{Path, PathBuf}; use anyhow::Result; @@ -11,7 +13,7 @@ use url::Url; use cache_key::{cache_digest, RepositoryUrl}; use crate::git::GitRemote; -use crate::{GitOid, GitSha, GitUrl}; +use crate::{GitOid, GitSha, GitUrl, GIT_STORE}; /// A remote Git source that can be checked out locally. pub struct GitSource { @@ -52,11 +54,21 @@ impl GitSource { /// Fetch the underlying Git repository at the given revision. #[instrument(skip(self), fields(repository = %self.git.repository, rev = ?self.git.precise))] pub fn fetch(self) -> Result { + // Compute the canonical URL for the repository. + let canonical = RepositoryUrl::new(&self.git.repository); + // The path to the repo, within the Git database. - let ident = cache_digest(&RepositoryUrl::new(&self.git.repository)); + let ident = cache_digest(&canonical); let db_path = self.cache.join("db").join(&ident); - let remote = GitRemote::new(&self.git.repository); + // Authenticate the URL, if necessary. + let remote = if let Some(credentials) = GIT_STORE.get(&canonical) { + Cow::Owned(credentials.apply(self.git.repository.clone())) + } else { + Cow::Borrowed(&self.git.repository) + }; + + let remote = GitRemote::new(&remote); let (db, actual_rev, task) = match (self.git.precise, remote.db_at(&db_path).ok()) { // If we have a locked revision, and we have a preexisting database // which has that revision, then no update needs to happen. diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index aa43a5abf..46da8e053 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -2,12 +2,12 @@ use std::collections::hash_map::Entry; use std::path::PathBuf; use anyhow::{Context, Result}; +use cache_key::RepositoryUrl; use owo_colors::OwoColorize; +use pep508_rs::{ExtraName, Requirement, VersionOrUrl}; use rustc_hash::{FxBuildHasher, FxHashMap}; use tracing::debug; - -use pep508_rs::{ExtraName, Requirement, VersionOrUrl}; -use uv_auth::store_credentials_from_url; +use uv_auth::{store_credentials_from_url, Credentials}; use uv_cache::Cache; use uv_client::{BaseClientBuilder, Connectivity, FlatIndexClient, RegistryClientBuilder}; use uv_configuration::{ @@ -16,6 +16,7 @@ use uv_configuration::{ use uv_dispatch::BuildDispatch; use uv_distribution::DistributionDatabase; use uv_fs::CWD; +use uv_git::GIT_STORE; use uv_normalize::PackageName; use uv_python::{ request_from_version_file, EnvironmentPreference, Interpreter, PythonDownloads, @@ -330,6 +331,37 @@ pub(crate) async fn add( } }; + // Redact any credentials. By default, we avoid writing sensitive credentials to files that + // will be checked into version control (e.g., `pyproject.toml` and `uv.lock`). Instead, + // we store the credentials in a global store, and reuse them during resolution. The + // expectation is that subsequent resolutions steps will succeed by reading from (e.g.) the + // user's credentials store, rather than by reading from the `pyproject.toml` file. + let source = match source { + Some(Source::Git { + mut git, + subdirectory, + rev, + tag, + branch, + }) => { + let credentials = Credentials::from_url(&git); + 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); + }; + Some(Source::Git { + git, + subdirectory, + rev, + tag, + branch, + }) + } + _ => source, + }; + // Update the `pyproject.toml`. let edit = match dependency_type { DependencyType::Production => toml.add_dependency(&requirement, source.as_ref())?, diff --git a/crates/uv/tests/edit.rs b/crates/uv/tests/edit.rs index e89b767a0..309d8e4c9 100644 --- a/crates/uv/tests/edit.rs +++ b/crates/uv/tests/edit.rs @@ -5,7 +5,7 @@ use assert_fs::prelude::*; use indoc::indoc; use insta::assert_snapshot; -use crate::common::packse_index_url; +use crate::common::{decode_token, packse_index_url}; use common::{uv_snapshot, TestContext}; mod common; @@ -291,6 +291,196 @@ fn add_git() -> Result<()> { Ok(()) } +/// Add a Git requirement from a private repository, with credentials. The resolution should +/// succeed, but the `pyproject.toml` should omit the credentials. +#[test] +#[cfg(feature = "git")] +fn add_git_private_source() -> Result<()> { + let context = TestContext::new("3.12"); + let token = decode_token(common::READ_ONLY_GITHUB_TOKEN); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = [] + "#})?; + + uv_snapshot!(context.filters(), context.add(&[&format!("uv-private-pypackage @ git+https://{token}@github.com/astral-test/uv-private-pypackage")]).arg("--preview"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Prepared 2 packages in [TIME] + Installed 2 packages in [TIME] + + project==0.1.0 (from file://[TEMP_DIR]/) + + uv-private-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-private-pypackage@d780faf0ac91257d4d5a4f0c5a0e4509608c0071) + "###); + + let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?; + + insta::with_settings!({ + filters => context.filters(), + }, { + assert_snapshot!( + pyproject_toml, @r###" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = [ + "uv-private-pypackage", + ] + + [tool.uv.sources] + uv-private-pypackage = { git = "https://github.com/astral-test/uv-private-pypackage" } + "### + ); + }); + + let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock"))?; + + insta::with_settings!({ + filters => context.filters(), + }, { + assert_snapshot!( + lock, @r###" + version = 1 + requires-python = ">=3.12" + + [options] + exclude-newer = "2024-03-25 00:00:00 UTC" + + [[package]] + name = "project" + version = "0.1.0" + source = { editable = "." } + dependencies = [ + { name = "uv-private-pypackage" }, + ] + + [[package]] + name = "uv-private-pypackage" + version = "0.1.0" + source = { git = "https://github.com/astral-test/uv-private-pypackage#d780faf0ac91257d4d5a4f0c5a0e4509608c0071" } + "### + ); + }); + + // Install from the lockfile. + uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv sync` is experimental and may change without warning + Audited 2 packages in [TIME] + "###); + + Ok(()) +} + +/// Add a Git requirement from a private repository, with credentials. Since `--raw-sources` is +/// specified, the `pyproject.toml` should retain the credentials. +#[test] +#[cfg(feature = "git")] +fn add_git_private_raw() -> Result<()> { + let context = TestContext::new("3.12"); + let token = decode_token(common::READ_ONLY_GITHUB_TOKEN); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! {r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = [] + "#})?; + + uv_snapshot!(context.filters(), context.add(&[&format!("uv-private-pypackage @ git+https://{token}@github.com/astral-test/uv-private-pypackage")]).arg("--raw-sources").arg("--preview"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Prepared 2 packages in [TIME] + Installed 2 packages in [TIME] + + project==0.1.0 (from file://[TEMP_DIR]/) + + uv-private-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-private-pypackage@d780faf0ac91257d4d5a4f0c5a0e4509608c0071) + "###); + + let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?; + + let filters: Vec<_> = [(token.as_str(), "***")] + .into_iter() + .chain(context.filters()) + .collect(); + + insta::with_settings!({ + filters => filters + }, { + assert_snapshot!( + pyproject_toml, @r###" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = [ + "uv-private-pypackage @ git+https://***@github.com/astral-test/uv-private-pypackage", + ] + "### + ); + }); + + let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock"))?; + + insta::with_settings!({ + filters => context.filters(), + }, { + assert_snapshot!( + lock, @r###" + version = 1 + requires-python = ">=3.12" + + [options] + exclude-newer = "2024-03-25 00:00:00 UTC" + + [[package]] + name = "project" + version = "0.1.0" + source = { editable = "." } + dependencies = [ + { name = "uv-private-pypackage" }, + ] + + [[package]] + name = "uv-private-pypackage" + version = "0.1.0" + source = { git = "https://github.com/astral-test/uv-private-pypackage#d780faf0ac91257d4d5a4f0c5a0e4509608c0071" } + "### + ); + }); + + // Install from the lockfile. + uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `uv sync` is experimental and may change without warning + Audited 2 packages in [TIME] + "###); + + Ok(()) +} + #[test] #[cfg(feature = "git")] fn add_git_error() -> Result<()> { diff --git a/crates/uv/tests/lock.rs b/crates/uv/tests/lock.rs index f0ed15597..3699bb5dc 100644 --- a/crates/uv/tests/lock.rs +++ b/crates/uv/tests/lock.rs @@ -5271,9 +5271,6 @@ fn lock_redact_git() -> Result<()> { version = "0.1.0" requires-python = ">=3.12" dependencies = ["uv-private-pypackage @ git+https://{token}@github.com/astral-test/uv-private-pypackage"] - - [tool.uv] - index-url = "https://public:heron@pypi-proxy.fly.dev/basic-auth/simple" "#, token = token, })?;