diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 53e0b923d..1bd7ddc0b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1164,7 +1164,7 @@ jobs: - name: "Add password to keyring" run: | # `keyrings.alt` contains the plaintext keyring - ./uv tool install --with keyrings.alt "keyring<25.4.0" # TODO(konsti): Remove upper bound once fix is released + ./uv tool install --with keyrings.alt keyring echo $UV_TEST_PUBLISH_KEYRING | keyring set https://test.pypi.org/legacy/?astral-test-keyring __token__ env: UV_TEST_PUBLISH_KEYRING: ${{ secrets.UV_TEST_PUBLISH_KEYRING }} diff --git a/crates/uv-auth/src/credentials.rs b/crates/uv-auth/src/credentials.rs index 753b9a244..bb8f28d4a 100644 --- a/crates/uv-auth/src/credentials.rs +++ b/crates/uv-auth/src/credentials.rs @@ -76,7 +76,7 @@ impl Credentials { } } - pub(crate) fn username(&self) -> Option<&str> { + pub fn username(&self) -> Option<&str> { self.username.as_deref() } @@ -84,7 +84,7 @@ impl Credentials { self.username.clone() } - pub(crate) fn password(&self) -> Option<&str> { + pub fn password(&self) -> Option<&str> { self.password.as_deref() } diff --git a/crates/uv-auth/src/keyring.rs b/crates/uv-auth/src/keyring.rs index 7cc98a39c..e9b7fede7 100644 --- a/crates/uv-auth/src/keyring.rs +++ b/crates/uv-auth/src/keyring.rs @@ -35,7 +35,7 @@ impl KeyringProvider { /// Returns [`None`] if no password was found for the username or if any errors /// are encountered in the keyring backend. #[instrument(skip_all, fields(url = % url.to_string(), username))] - pub(crate) async fn fetch(&self, url: &Url, username: &str) -> Option { + pub async fn fetch(&self, url: &Url, username: &str) -> Option { // Validate the request debug_assert!( url.host_str().is_some(), diff --git a/crates/uv/src/commands/publish.rs b/crates/uv/src/commands/publish.rs index 71bc2ef9a..d0259ed5d 100644 --- a/crates/uv/src/commands/publish.rs +++ b/crates/uv/src/commands/publish.rs @@ -8,7 +8,7 @@ use std::fmt::Write; use std::iter; use std::sync::Arc; use std::time::Duration; -use tracing::info; +use tracing::{debug, info}; use url::Url; use uv_cache::Cache; use uv_client::{AuthIntegration, BaseClientBuilder, Connectivity, RegistryClientBuilder}; @@ -17,6 +17,7 @@ use uv_distribution_types::{Index, IndexCapabilities, IndexLocations, IndexUrl}; use uv_publish::{ check_trusted_publishing, files_for_publishing, upload, CheckUrlClient, TrustedPublishResult, }; +use uv_warnings::warn_user_once; pub(crate) async fn publish( paths: Vec, @@ -68,7 +69,7 @@ pub(crate) async fn publish( .wrap_existing(&upload_client); // Initialize the registry client. - let check_url_client = if let Some(index_url) = check_url { + let check_url_client = if let Some(index_url) = &check_url { let index_urls = IndexLocations::new( vec![Index::from_index_url(index_url.clone())], Vec::new(), @@ -82,7 +83,7 @@ pub(crate) async fn publish( .keyring(keyring_provider) .allow_insecure_host(allow_insecure_host.to_vec()); Some(CheckUrlClient { - index_url, + index_url: index_url.clone(), registry_client_builder, client: &upload_client, index_capabilities: IndexCapabilities::default(), @@ -103,7 +104,7 @@ pub(crate) async fn publish( ) .await?; - let (username, password) = + let (username, mut password) = if let TrustedPublishResult::Configured(password) = &trusted_publishing_token { (Some("__token__".to_string()), Some(password.to_string())) } else { @@ -151,6 +152,34 @@ pub(crate) async fn publish( } } + // If applicable, fetch the password from the keyring eagerly to avoid user confusion about + // missing keyring entries later. + if let Some(keyring_provider) = keyring_provider.to_provider() { + if password.is_none() { + if let Some(username) = &username { + debug!("Fetching password from keyring"); + if let Some(keyring_password) = keyring_provider + .fetch(&publish_url, username) + .await + .as_ref() + .and_then(|credentials| credentials.password()) + { + password = Some(keyring_password.to_string()); + } else { + warn_user_once!( + "Keyring has no password for URL `{publish_url}` and username `{username}`" + ); + } + } + } else if check_url.is_none() { + warn_user_once!( + "Using `--keyring-provider` with a password or token and no check url has no effect" + ); + } else { + // We may be using the keyring for the simple index. + } + } + for (file, raw_filename, filename) in files { if let Some(check_url_client) = &check_url_client { if uv_publish::check_url(check_url_client, &file, &filename).await? { diff --git a/crates/uv/tests/it/publish.rs b/crates/uv/tests/it/publish.rs index c45c1801e..46a5b463a 100644 --- a/crates/uv/tests/it/publish.rs +++ b/crates/uv/tests/it/publish.rs @@ -1,5 +1,7 @@ -use crate::common::{uv_snapshot, TestContext}; +use crate::common::{uv_snapshot, venv_bin_path, TestContext}; +use assert_cmd::assert::OutputAssertExt; use assert_fs::fixture::{FileTouch, PathChild}; +use std::env; use uv_static::EnvVars; #[test] @@ -197,3 +199,128 @@ fn dubious_filenames() { "### ); } + +/// Check that we (don't) use the keyring and warn for missing keyring behaviors correctly. +#[test] +fn check_keyring_behaviours() { + let context = TestContext::new("3.12"); + + // Install our keyring plugin + context + .pip_install() + .arg( + context + .workspace_root + .join("scripts") + .join("packages") + .join("keyring_test_plugin"), + ) + .assert() + .success(); + + // Ok: The keyring may be used for the index page. + uv_snapshot!(context.filters(), context.publish() + .arg("-u") + .arg("dummy") + .arg("-p") + .arg("dummy") + .arg("--keyring-provider") + .arg("subprocess") + .arg("--check-url") + .arg("https://test.pypi.org/simple/") + .arg("--publish-url") + .arg("https://test.pypi.org/legacy/?ok") + .arg("../../scripts/links/ok-1.0.0-py3-none-any.whl") + .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + warning: `uv publish` is experimental and may change without warning + Publishing 1 file to https://test.pypi.org/legacy/?ok + error: Failed to query check URL + Caused by: Package `ok` was not found in the registry + "### + ); + + // Warn: The keyring is unused. + uv_snapshot!(context.filters(), context.publish() + .arg("-u") + .arg("dummy") + .arg("-p") + .arg("dummy") + .arg("--keyring-provider") + .arg("subprocess") + .arg("--publish-url") + .arg("https://test.pypi.org/legacy/?ok") + .arg("../../scripts/links/ok-1.0.0-py3-none-any.whl") + .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + warning: `uv publish` is experimental and may change without warning + Publishing 1 file to https://test.pypi.org/legacy/?ok + warning: Using `--keyring-provider` with a password or token and no check url has no effect + Uploading ok-1.0.0-py3-none-any.whl ([SIZE]) + error: Failed to publish `../../scripts/links/ok-1.0.0-py3-none-any.whl` to https://test.pypi.org/legacy/?ok + Caused by: Upload failed with status code 403 Forbidden. Server says: 403 Username/Password authentication is no longer supported. Migrate to API Tokens or Trusted Publishers instead. See https://test.pypi.org/help/#apitoken and https://test.pypi.org/help/#trusted-publishers + "### + ); + + // Warn: There is no keyring entry for the user dummy. + // https://github.com/astral-sh/uv/issues/7963#issuecomment-2453558043 + uv_snapshot!(context.filters(), context.publish() + .arg("-u") + .arg("dummy") + .arg("--keyring-provider") + .arg("subprocess") + .arg("--check-url") + .arg("https://test.pypi.org/simple/") + .arg("--publish-url") + .arg("https://test.pypi.org/legacy/?ok") + .arg("../../scripts/links/ok-1.0.0-py3-none-any.whl") + .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + warning: `uv publish` is experimental and may change without warning + Publishing 1 file to https://test.pypi.org/legacy/?ok + Request for dummy@https://test.pypi.org/legacy/?ok + Request for dummy@test.pypi.org + warning: Keyring has no password for URL `https://test.pypi.org/legacy/?ok` and username `dummy` + error: Failed to query check URL + Caused by: Package `ok` was not found in the registry + "### + ); + + // Ok: There is a keyring entry for the user dummy. + // https://github.com/astral-sh/uv/issues/7963#issuecomment-2453558043 + uv_snapshot!(context.filters(), context.publish() + .arg("-u") + .arg("dummy") + .arg("--keyring-provider") + .arg("subprocess") + .arg("--publish-url") + .arg("https://test.pypi.org/legacy/?ok") + .arg("../../scripts/links/ok-1.0.0-py3-none-any.whl") + .env(EnvVars::KEYRING_TEST_CREDENTIALS, r#"{"https://test.pypi.org/legacy/?ok": {"dummy": "dummy"}}"#) + .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + warning: `uv publish` is experimental and may change without warning + Publishing 1 file to https://test.pypi.org/legacy/?ok + Request for dummy@https://test.pypi.org/legacy/?ok + Uploading ok-1.0.0-py3-none-any.whl ([SIZE]) + error: Failed to publish `../../scripts/links/ok-1.0.0-py3-none-any.whl` to https://test.pypi.org/legacy/?ok + Caused by: Upload failed with status code 403 Forbidden. Server says: 403 Username/Password authentication is no longer supported. Migrate to API Tokens or Trusted Publishers instead. See https://test.pypi.org/help/#apitoken and https://test.pypi.org/help/#trusted-publishers + "### + ); +} diff --git a/scripts/packages/keyring_test_plugin/keyrings/test_keyring.py b/scripts/packages/keyring_test_plugin/keyrings/test_keyring.py index 2895477f4..c1c70bff1 100644 --- a/scripts/packages/keyring_test_plugin/keyrings/test_keyring.py +++ b/scripts/packages/keyring_test_plugin/keyrings/test_keyring.py @@ -10,7 +10,7 @@ class KeyringTest(backend.KeyringBackend): def get_password(self, service, username): print(f"Request for {username}@{service}", file=sys.stderr) - credentials = json.loads(os.environ.get("KEYRING_TEST_CREDENTIALS") or {}) + credentials = json.loads(os.environ.get("KEYRING_TEST_CREDENTIALS", "{}")) return credentials.get(service, {}).get(username) def set_password(self, service, username, password):