Publish: Warn when keyring has no password (#8827)

When trying to upload without a password but with the keyring, check
that the keyring has a password for the upload URL and username and warn
if it doesn't.

Fixes #8781
This commit is contained in:
konsti 2024-11-27 20:54:49 +01:00 committed by GitHub
parent 68e3228f2b
commit 0b0d0f44f8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 166 additions and 10 deletions

View file

@ -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 }}

View file

@ -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()
}

View file

@ -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<Credentials> {
pub async fn fetch(&self, url: &Url, username: &str) -> Option<Credentials> {
// Validate the request
debug_assert!(
url.host_str().is_some(),

View file

@ -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<String>,
@ -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? {

View file

@ -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
"###
);
}

View file

@ -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):