From ea16fe28b6e08c254e895020bc09e926ed1e574a Mon Sep 17 00:00:00 2001 From: John Mumm Date: Sat, 19 Apr 2025 00:01:11 +0200 Subject: [PATCH] Obfuscate password in credentials debug messages (#12944) I noticed in the trace output that we weren't obfuscating the `Credentials` password in a trace message. This PR creates a `Password` newtype with a custom `Debug` implementation. --- crates/uv-auth/src/credentials.rs | 48 +++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/crates/uv-auth/src/credentials.rs b/crates/uv-auth/src/credentials.rs index a015e5b3d..3dbfda87b 100644 --- a/crates/uv-auth/src/credentials.rs +++ b/crates/uv-auth/src/credentials.rs @@ -2,6 +2,7 @@ use base64::prelude::BASE64_STANDARD; use base64::read::DecoderReader; use base64::write::EncoderWriter; use std::borrow::Cow; +use std::fmt; use netrc::Netrc; use reqwest::header::HeaderValue; @@ -18,7 +19,7 @@ pub enum Credentials { /// The username to use for authentication. username: Username, /// The password to use for authentication. - password: Option, + password: Option, }, Bearer { /// The token to use for authentication. @@ -67,13 +68,28 @@ impl From> for Username { } } +#[derive(Clone, PartialEq, Eq, Ord, PartialOrd, Hash, Default)] +pub struct Password(String); + +impl Password { + pub fn as_str(&self) -> &str { + self.0.as_str() + } +} + +impl fmt::Debug for Password { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "****") + } +} + impl Credentials { /// Create a set of HTTP Basic Authentication credentials. #[allow(dead_code)] pub fn basic(username: Option, password: Option) -> Self { Self::Basic { username: Username::new(username), - password, + password: password.map(Password), } } @@ -106,7 +122,7 @@ impl Credentials { pub fn password(&self) -> Option<&str> { match self { - Self::Basic { password, .. } => password.as_deref(), + Self::Basic { password, .. } => password.as_ref().map(Password::as_str), Self::Bearer { .. } => None, } } @@ -135,7 +151,7 @@ impl Credentials { Some(Credentials::Basic { username: Username::new(Some(entry.login.clone())), - password: Some(entry.password.clone()), + password: Some(Password(entry.password.clone())), }) } @@ -161,10 +177,12 @@ impl Credentials { } .into(), password: url.password().map(|password| { - percent_encoding::percent_decode_str(password) - .decode_utf8() - .expect("An encoded password should always decode") - .into_owned() + Password( + percent_encoding::percent_decode_str(password) + .decode_utf8() + .expect("An encoded password should always decode") + .into_owned(), + ) }), }) } @@ -228,7 +246,7 @@ impl Credentials { }; return Some(Self::Basic { username: Username::new(username), - password, + password: password.map(Password), }); } @@ -407,4 +425,16 @@ mod tests { assert_debug_snapshot!(header, @r###""Basic dXNlcjpwYXNzd29yZD09""###); assert_eq!(Credentials::from_header_value(&header), Some(credentials)); } + + // Test that we don't include the password in debug messages. + #[test] + fn test_password_obfuscation() { + let credentials = + Credentials::basic(Some(String::from("user")), Some(String::from("password"))); + let debugged = format!("{credentials:?}"); + assert_eq!( + debugged, + "Basic { username: Username(Some(\"user\")), password: Some(****) }" + ); + } }