diff --git a/crates/uv-auth/Cargo.toml b/crates/uv-auth/Cargo.toml index b973e887351..c083d6d5e9c 100644 --- a/crates/uv-auth/Cargo.toml +++ b/crates/uv-auth/Cargo.toml @@ -45,6 +45,7 @@ rustc-hash = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +tempfile = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } toml = { workspace = true } @@ -53,7 +54,6 @@ url = { workspace = true } [dev-dependencies] insta = { workspace = true } -tempfile = { workspace = true } test-log = { workspace = true } tokio = { workspace = true } wiremock = { workspace = true } diff --git a/crates/uv-auth/src/store.rs b/crates/uv-auth/src/store.rs index bad4a0a23d3..a8a547aa682 100644 --- a/crates/uv-auth/src/store.rs +++ b/crates/uv-auth/src/store.rs @@ -5,7 +5,7 @@ use fs_err as fs; use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; use thiserror::Error; -use uv_fs::{LockedFile, LockedFileError, LockedFileMode}; +use uv_fs::{LockedFile, LockedFileError, LockedFileMode, persist_with_retry_sync}; use uv_preview::{Preview, PreviewFeature}; use uv_redacted::DisplaySafeUrl; @@ -263,10 +263,40 @@ impl TextCredentialStore { /// Acquire a lock on the credentials file at the given path. async fn lock(path: &Path) -> Result { if let Some(parent) = path.parent() { - fs::create_dir_all(parent)?; + Self::create_directory(parent)?; } - let lock = path.with_added_extension("lock"); - Ok(LockedFile::acquire(lock, LockedFileMode::Exclusive, "credentials store").await?) + let lock_path = path.with_added_extension("lock"); + + #[cfg(unix)] + { + use fs_err::os::unix::fs::OpenOptionsExt; + + fs::OpenOptions::new() + .read(true) + .write(true) + .create(true) + .mode(0o600) + .open(&lock_path)?; + } + + Ok(LockedFile::acquire(&lock_path, LockedFileMode::Exclusive, "credentials store").await?) + } + + fn create_directory(path: &Path) -> Result<(), TomlCredentialError> { + #[cfg(unix)] + { + use std::os::unix::fs::DirBuilderExt; + + std::fs::DirBuilder::new() + .recursive(true) + .mode(0o700) + .create(path)?; + } + + #[cfg(not(unix))] + fs::create_dir_all(path)?; + + Ok(()) } /// Read credentials from a file. @@ -325,14 +355,15 @@ impl TextCredentialStore { let toml_creds = TomlCredentials { credentials }; let content = toml::to_string_pretty(&toml_creds)?; - fs::create_dir_all( - path.as_ref() - .parent() - .ok_or(TomlCredentialError::CredentialsDirError)?, - )?; - - // TODO(zanieb): We should use an atomic write here - fs::write(path, content)?; + let path = path.as_ref(); + let parent = path + .parent() + .ok_or(TomlCredentialError::CredentialsDirError)?; + Self::create_directory(parent)?; + + let temp_file = tempfile::NamedTempFile::new_in(parent)?; + fs::write(&temp_file, content)?; + persist_with_retry_sync(temp_file, path)?; Ok(()) } diff --git a/crates/uv/tests/it/auth.rs b/crates/uv/tests/it/auth.rs index 49b119598c8..6d6363a0823 100644 --- a/crates/uv/tests/it/auth.rs +++ b/crates/uv/tests/it/auth.rs @@ -3,6 +3,8 @@ use assert_cmd::assert::OutputAssertExt; use assert_fs::{fixture::PathChild, prelude::FileWriteStr}; use uv_static::EnvVars; +#[cfg(unix)] +use uv_test::get_bin; use uv_test::uv_snapshot; #[tokio::test] @@ -1243,6 +1245,124 @@ fn login_text_store_empty_file() -> Result<()> { Ok(()) } +/// Plaintext credentials and their lock must remain private under a permissive umask. +#[cfg(unix)] +#[test] +fn login_text_store_private_permissions() -> Result<()> { + use std::os::unix::fs::PermissionsExt; + use std::process::Command; + + let context = uv_test::test_context_with_versions!(&[]); + let credentials_dir = context.temp_dir.child("credentials"); + let mut command = Command::new("sh"); + command + .arg("-c") + .arg("umask 000; exec \"$@\"") + .arg("sh") + .arg(get_bin!()) + .arg("auth") + .arg("login") + .arg("--cache-dir") + .arg(context.cache_dir.path()) + .arg("https://example.com/simple") + .arg("--token") + .arg("secret-token") + .env(EnvVars::UV_CREDENTIALS_DIR, credentials_dir.path()); + context.add_shared_env(&mut command, false); + + uv_snapshot!(context.filters(), command, @" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Stored credentials for https://example.com/ + "); + + let credentials = credentials_dir.child("credentials.toml"); + let lock = credentials_dir.child("credentials.toml.lock"); + assert_eq!( + fs_err::metadata(credentials_dir.path())? + .permissions() + .mode() + & 0o777, + 0o700 + ); + assert_eq!( + fs_err::metadata(credentials.path())?.permissions().mode() & 0o777, + 0o600 + ); + assert_eq!( + fs_err::metadata(lock.path())?.permissions().mode() & 0o777, + 0o600 + ); + assert!(fs_err::read_to_string(credentials.path())?.contains("secret-token")); + + Ok(()) +} + +/// Reading an existing shared credential store must not change its permissions. +#[cfg(unix)] +#[test] +fn token_text_store_read_only_permissions() -> Result<()> { + use std::os::unix::fs::PermissionsExt; + + let context = uv_test::test_context_with_versions!(&[]); + let credentials_dir = context.temp_dir.child("credentials"); + let credentials = credentials_dir.child("credentials.toml"); + let lock = credentials_dir.child("credentials.toml.lock"); + credentials.write_str(indoc::indoc! {r#" + [[credential]] + service = "https://example.com" + username = "__token__" + password = "shared-token" + "#})?; + lock.write_str("")?; + + let mut permissions = fs_err::metadata(credentials.path())?.permissions(); + permissions.set_mode(0o444); + fs_err::set_permissions(credentials.path(), permissions)?; + let mut permissions = fs_err::metadata(lock.path())?.permissions(); + permissions.set_mode(0o644); + fs_err::set_permissions(lock.path(), permissions)?; + let mut permissions = fs_err::metadata(credentials_dir.path())?.permissions(); + permissions.set_mode(0o555); + fs_err::set_permissions(credentials_dir.path(), permissions)?; + + uv_snapshot!(context.auth_token() + .arg("https://example.com/simple") + .env(EnvVars::UV_CREDENTIALS_DIR, credentials_dir.path()), @" + success: true + exit_code: 0 + ----- stdout ----- + shared-token + + ----- stderr ----- + "); + + assert_eq!( + fs_err::metadata(credentials_dir.path())? + .permissions() + .mode() + & 0o777, + 0o555 + ); + assert_eq!( + fs_err::metadata(credentials.path())?.permissions().mode() & 0o777, + 0o444 + ); + assert_eq!( + fs_err::metadata(lock.path())?.permissions().mode() & 0o777, + 0o644 + ); + + let mut permissions = fs_err::metadata(credentials_dir.path())?.permissions(); + permissions.set_mode(0o700); + fs_err::set_permissions(credentials_dir.path(), permissions)?; + + Ok(()) +} + #[test] fn login_text_store_comments_only_file() -> Result<()> { let context = uv_test::test_context_with_versions!(&[]);