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.
This commit is contained in:
John Mumm 2025-04-19 00:01:11 +02:00 committed by GitHub
parent 41ac6649d7
commit ea16fe28b6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 39 additions and 9 deletions

View File

@ -2,6 +2,7 @@ use base64::prelude::BASE64_STANDARD;
use base64::read::DecoderReader; use base64::read::DecoderReader;
use base64::write::EncoderWriter; use base64::write::EncoderWriter;
use std::borrow::Cow; use std::borrow::Cow;
use std::fmt;
use netrc::Netrc; use netrc::Netrc;
use reqwest::header::HeaderValue; use reqwest::header::HeaderValue;
@ -18,7 +19,7 @@ pub enum Credentials {
/// The username to use for authentication. /// The username to use for authentication.
username: Username, username: Username,
/// The password to use for authentication. /// The password to use for authentication.
password: Option<String>, password: Option<Password>,
}, },
Bearer { Bearer {
/// The token to use for authentication. /// The token to use for authentication.
@ -67,13 +68,28 @@ impl From<Option<String>> 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 { impl Credentials {
/// Create a set of HTTP Basic Authentication credentials. /// Create a set of HTTP Basic Authentication credentials.
#[allow(dead_code)] #[allow(dead_code)]
pub fn basic(username: Option<String>, password: Option<String>) -> Self { pub fn basic(username: Option<String>, password: Option<String>) -> Self {
Self::Basic { Self::Basic {
username: Username::new(username), username: Username::new(username),
password, password: password.map(Password),
} }
} }
@ -106,7 +122,7 @@ impl Credentials {
pub fn password(&self) -> Option<&str> { pub fn password(&self) -> Option<&str> {
match self { match self {
Self::Basic { password, .. } => password.as_deref(), Self::Basic { password, .. } => password.as_ref().map(Password::as_str),
Self::Bearer { .. } => None, Self::Bearer { .. } => None,
} }
} }
@ -135,7 +151,7 @@ impl Credentials {
Some(Credentials::Basic { Some(Credentials::Basic {
username: Username::new(Some(entry.login.clone())), 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(), .into(),
password: url.password().map(|password| { password: url.password().map(|password| {
percent_encoding::percent_decode_str(password) Password(
.decode_utf8() percent_encoding::percent_decode_str(password)
.expect("An encoded password should always decode") .decode_utf8()
.into_owned() .expect("An encoded password should always decode")
.into_owned(),
)
}), }),
}) })
} }
@ -228,7 +246,7 @@ impl Credentials {
}; };
return Some(Self::Basic { return Some(Self::Basic {
username: Username::new(username), username: Username::new(username),
password, password: password.map(Password),
}); });
} }
@ -407,4 +425,16 @@ mod tests {
assert_debug_snapshot!(header, @r###""Basic dXNlcjpwYXNzd29yZD09""###); assert_debug_snapshot!(header, @r###""Basic dXNlcjpwYXNzd29yZD09""###);
assert_eq!(Credentials::from_header_value(&header), Some(credentials)); 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(****) }"
);
}
} }