From 87feeb3ca36387d15d0000d8a6d241fc860a8171 Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Thu, 5 Jan 2023 09:31:21 -0700 Subject: [PATCH 01/10] add Secret wrapper type --- src/cargo/util/auth.rs | 65 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/cargo/util/auth.rs b/src/cargo/util/auth.rs index 0235f517ac6..949abff0e0c 100644 --- a/src/cargo/util/auth.rs +++ b/src/cargo/util/auth.rs @@ -10,6 +10,7 @@ use serde::Deserialize; use std::collections::HashMap; use std::error::Error; use std::io::{Read, Write}; +use std::ops::Deref; use std::path::PathBuf; use std::process::{Command, Stdio}; use time::format_description::well_known::Rfc3339; @@ -21,6 +22,70 @@ use crate::ops::RegistryCredentialConfig; use super::config::CredentialCacheValue; +/// A wrapper for values that should not be printed. +/// +/// This type does not implement `Display`, and has a `Debug` impl that hides +/// the contained value. +#[derive(Clone, PartialEq, Eq)] +pub struct Secret { + inner: T, +} + +impl Secret { + /// Converts a `Secret` to a `Secret<&T::Target>`. + /// + /// For example, this can be used to convert from `&Secret` to + /// `Secret<&str>`. + pub fn as_deref(&self) -> Secret<&::Target> + where + T: Deref, + { + Secret::from(self.inner.deref()) + } + + pub fn map(self, f: F) -> Secret + where + F: FnOnce(T) -> U, + { + Secret::from(f(self.inner)) + } + + fn into_inner(self) -> T { + self.inner + } +} + +impl Secret<&T> { + /// Converts a `Secret` containing a borrowed type to a `Secret` containing the + /// corresponding owned type. + /// + /// For example, this can be used to convert from `Secret<&str>` to + /// `Secret`. + pub fn owned(&self) -> Secret<::Owned> { + Secret::from(self.inner.to_owned()) + } +} + +impl> Secret { + pub fn is_empty(&self) -> bool { + self.inner.as_ref().is_empty() + } +} + +impl From for Secret { + fn from(inner: T) -> Self { + Self { inner } + } +} + +impl fmt::Debug for Secret { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Secret") + .field("inner", &"REDACTED") + .finish() + } +} + /// Get the credential configuration for a `SourceId`. pub fn registry_credential_config( config: &Config, From 9e732c6d2197127d9367ea56c85426be3b2a3d81 Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Mon, 9 Jan 2023 16:54:35 -0700 Subject: [PATCH 02/10] change Secret::into_inner to Secret::expose and make public; add a few more needed utility methods --- src/cargo/util/auth.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/cargo/util/auth.rs b/src/cargo/util/auth.rs index 949abff0e0c..e8a571aa348 100644 --- a/src/cargo/util/auth.rs +++ b/src/cargo/util/auth.rs @@ -32,6 +32,14 @@ pub struct Secret { } impl Secret { + /// Unwraps the contained value. + /// + /// Use of this method marks the boundary of where the contained value is + /// hidden. + pub fn expose(self) -> T { + self.inner + } + /// Converts a `Secret` to a `Secret<&T::Target>`. /// /// For example, this can be used to convert from `&Secret` to @@ -43,16 +51,16 @@ impl Secret { Secret::from(self.inner.deref()) } + pub fn as_ref(&self) -> Secret<&T> { + Secret::from(&self.inner) + } + pub fn map(self, f: F) -> Secret where F: FnOnce(T) -> U, { Secret::from(f(self.inner)) } - - fn into_inner(self) -> T { - self.inner - } } impl Secret<&T> { @@ -66,6 +74,12 @@ impl Secret<&T> { } } +impl Secret> { + pub fn transpose(self) -> Result, E> { + self.inner.map(|v| Secret::from(v)) + } +} + impl> Secret { pub fn is_empty(&self) -> bool { self.inner.as_ref().is_empty() From e69a18ec79834e59e8a24c75698d8d34e0991c3d Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Mon, 9 Jan 2023 17:04:47 -0700 Subject: [PATCH 03/10] add Secret to RegistryCredentialConfig --- src/cargo/ops/registry.rs | 24 ++++++++++++------------ src/cargo/util/auth.rs | 31 +++++++++++++++++++++---------- src/cargo/util/config/mod.rs | 4 ++-- 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index d70e86cd149..eef8404d3fe 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -30,7 +30,7 @@ use crate::ops; use crate::ops::Packages; use crate::sources::{RegistrySource, SourceConfigMap, CRATES_IO_DOMAIN, CRATES_IO_REGISTRY}; use crate::util::auth::{ - paserk_public_from_paserk_secret, {self, AuthorizationError}, + paserk_public_from_paserk_secret, Secret, {self, AuthorizationError}, }; use crate::util::config::{Config, SslVersionConfig, SslVersionConfigRange}; use crate::util::errors::CargoResult; @@ -45,11 +45,11 @@ use crate::{drop_print, drop_println, version}; pub enum RegistryCredentialConfig { None, /// The authentication token. - Token(String), + Token(Secret), /// Process used for fetching a token. Process((PathBuf, Vec)), /// Secret Key and subject for Asymmetric tokens. - AsymmetricKey((String, Option)), + AsymmetricKey((Secret, Option)), } impl RegistryCredentialConfig { @@ -71,9 +71,9 @@ impl RegistryCredentialConfig { pub fn is_asymmetric_key(&self) -> bool { matches!(self, Self::AsymmetricKey(..)) } - pub fn as_token(&self) -> Option<&str> { + pub fn as_token(&self) -> Option> { if let Self::Token(v) = self { - Some(&*v) + Some(v.as_deref()) } else { None } @@ -85,7 +85,7 @@ impl RegistryCredentialConfig { None } } - pub fn as_asymmetric_key(&self) -> Option<&(String, Option)> { + pub fn as_asymmetric_key(&self) -> Option<&(Secret, Option)> { if let Self::AsymmetricKey(v) = self { Some(v) } else { @@ -830,13 +830,13 @@ pub fn registry_login( } _ => (None, None), }; - let secret_key: String; + let secret_key: Secret; if generate_keypair { assert!(!secret_key_required); let kp = AsymmetricKeyPair::::generate().unwrap(); let mut key = String::new(); FormatAsPaserk::fmt(&kp.secret, &mut key).unwrap(); - secret_key = key; + secret_key = Secret::from(key); } else if secret_key_required { assert!(!generate_keypair); drop_println!(config, "please paste the API secret key below"); @@ -846,13 +846,13 @@ pub fn registry_login( .lock() .read_line(&mut line) .with_context(|| "failed to read stdin")?; - secret_key = line.trim().to_string(); + secret_key = Secret::from(line.trim().to_string()); } else { secret_key = old_secret_key .cloned() .ok_or_else(|| anyhow!("need a secret_key to set a key_subject"))?; } - if let Some(p) = paserk_public_from_paserk_secret(&secret_key) { + if let Some(p) = paserk_public_from_paserk_secret(secret_key.as_deref()) { drop_println!(config, "{}", &p); } else { bail!("not a validly formated PASERK secret key"); @@ -866,7 +866,7 @@ pub fn registry_login( )); } else { new_token = RegistryCredentialConfig::Token(match token { - Some(token) => token.to_string(), + Some(token) => Secret::from(token.to_string()), None => { if let Some(login_url) = login_url { drop_println!( @@ -890,7 +890,7 @@ pub fn registry_login( .with_context(|| "failed to read stdin")?; // Automatically remove `cargo login` from an inputted token to // allow direct pastes from `registry.host()`/me. - line.replace("cargo login", "").trim().to_string() + Secret::from(line.replace("cargo login", "").trim().to_string()) } }); diff --git a/src/cargo/util/auth.rs b/src/cargo/util/auth.rs index e8a571aa348..33a2aaaca9e 100644 --- a/src/cargo/util/auth.rs +++ b/src/cargo/util/auth.rs @@ -281,13 +281,13 @@ fn registry_credential_config_inner( registry )); } - (Some(token), _, _, _) => RegistryCredentialConfig::Token(token), + (Some(token), _, _, _) => RegistryCredentialConfig::Token(Secret::from(token)), (_, Some(process), _, _) => RegistryCredentialConfig::Process(( process.path.resolve_program(config), process.args, )), (None, None, Some(key), subject) => { - RegistryCredentialConfig::AsymmetricKey((key, subject)) + RegistryCredentialConfig::AsymmetricKey((Secret::from(key), subject)) } (None, None, None, _) => { if !is_crates_io { @@ -432,15 +432,19 @@ fn auth_token_optional( let credential = registry_credential_config(config, sid)?; let (independent_of_endpoint, token) = match credential { RegistryCredentialConfig::None => return Ok(None), - RegistryCredentialConfig::Token(config_token) => (true, config_token.to_string()), + RegistryCredentialConfig::Token(config_token) => (true, config_token.expose()), RegistryCredentialConfig::Process(process) => { // todo: PASETO with process run_command(config, &process, sid, Action::Get)?.unwrap() } RegistryCredentialConfig::AsymmetricKey((secret_key, secret_key_subject)) => { - let secret: AsymmetricSecretKey = - secret_key.as_str().try_into()?; - let public: AsymmetricPublicKey = (&secret).try_into()?; + let secret: Secret> = + secret_key.map(|key| key.as_str().try_into()).transpose()?; + let public: AsymmetricPublicKey = secret + .as_ref() + .map(|key| key.try_into()) + .transpose()? + .expose(); let kip: pasetors::paserk::Id = (&public).try_into()?; let iat = OffsetDateTime::now_utc(); @@ -493,7 +497,7 @@ fn auth_token_optional( ( false, pasetors::version3::PublicToken::sign( - &secret, + &secret.expose(), serde_json::to_string(&message) .expect("cannot serialize") .as_bytes(), @@ -598,6 +602,7 @@ pub fn login(config: &Config, sid: &SourceId, token: RegistryCredentialConfig) - let token = token .as_token() .expect("credential_process cannot use login with a secret_key") + .expose() .to_owned(); run_command(config, &process, sid, Action::Store(token))?; } @@ -609,9 +614,15 @@ pub fn login(config: &Config, sid: &SourceId, token: RegistryCredentialConfig) - } /// Checks that a secret key is valid, and returns the associated public key in Paserk format. -pub(crate) fn paserk_public_from_paserk_secret(secret_key: &str) -> Option { - let secret: AsymmetricSecretKey = secret_key.try_into().ok()?; - let public: AsymmetricPublicKey = (&secret).try_into().ok()?; +pub(crate) fn paserk_public_from_paserk_secret(secret_key: Secret<&str>) -> Option { + let secret: Secret> = + secret_key.map(|key| key.try_into()).transpose().ok()?; + let public: AsymmetricPublicKey = secret + .as_ref() + .map(|key| key.try_into()) + .transpose() + .ok()? + .expose(); let mut paserk_pub_key = String::new(); FormatAsPaserk::fmt(&public, &mut paserk_pub_key).unwrap(); Some(paserk_pub_key) diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index e6e2c41b619..a8dad07ba78 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -2179,7 +2179,7 @@ pub fn save_credentials( // login with token let key = "token".to_string(); - let value = ConfigValue::String(token, path_def.clone()); + let value = ConfigValue::String(token.expose(), path_def.clone()); let map = HashMap::from([(key, value)]); let table = CV::Table(map, path_def.clone()); @@ -2194,7 +2194,7 @@ pub fn save_credentials( // login with key let key = "secret-key".to_string(); - let value = ConfigValue::String(secret_key, path_def.clone()); + let value = ConfigValue::String(secret_key.expose(), path_def.clone()); let mut map = HashMap::from([(key, value)]); if let Some(key_subject) = key_subject { let key = "secret-key-subject".to_string(); From b0bf846ec933fb3c4aed13c09ce01dd6f7eacabf Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Mon, 9 Jan 2023 17:08:30 -0700 Subject: [PATCH 04/10] add Secret to CredentialCacheValue --- src/cargo/util/auth.rs | 6 +++--- src/cargo/util/config/mod.rs | 14 +++----------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/cargo/util/auth.rs b/src/cargo/util/auth.rs index 33a2aaaca9e..4741210a6aa 100644 --- a/src/cargo/util/auth.rs +++ b/src/cargo/util/auth.rs @@ -384,7 +384,7 @@ pub fn cache_token(config: &Config, sid: &SourceId, token: &str) { CredentialCacheValue { from_commandline: true, independent_of_endpoint: true, - token_value: token.to_string(), + token_value: Secret::from(token.to_string()), }, ); } @@ -425,7 +425,7 @@ fn auth_token_optional( || cache_token_value.independent_of_endpoint || mutation.is_none() { - return Ok(Some(cache_token_value.token_value.clone())); + return Ok(Some(cache_token_value.token_value.clone().expose())); } } @@ -518,7 +518,7 @@ fn auth_token_optional( CredentialCacheValue { from_commandline: false, independent_of_endpoint, - token_value: token.to_string(), + token_value: Secret::from(token.to_string()), }, ); } diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index a8dad07ba78..3fb7e4d0598 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -70,6 +70,7 @@ use crate::core::compiler::rustdoc::RustdocExternMap; use crate::core::shell::Verbosity; use crate::core::{features, CliUnstable, Shell, SourceId, Workspace, WorkspaceRootConfig}; use crate::ops::{self, RegistryCredentialConfig}; +use crate::util::auth::Secret; use crate::util::errors::CargoResult; use crate::util::validate_package_name; use crate::util::CanonicalUrl; @@ -137,6 +138,7 @@ enum WhyLoad { } /// A previously generated authentication token and the data needed to determine if it can be reused. +#[derive(Debug)] pub struct CredentialCacheValue { /// If the command line was used to override the token then it must always be reused, /// even if reading the configuration files would lead to a different value. @@ -144,17 +146,7 @@ pub struct CredentialCacheValue { /// If nothing depends on which endpoint is being hit, then we can reuse the token /// for any future request even if some of the requests involve mutations. pub independent_of_endpoint: bool, - pub token_value: String, -} - -impl fmt::Debug for CredentialCacheValue { - /// This manual implementation helps ensure that the token value is redacted from all logs. - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("CredentialCacheValue") - .field("from_commandline", &self.from_commandline) - .field("token_value", &"REDACTED") - .finish() - } + pub token_value: Secret, } /// Configuration information for cargo. This is not specific to a build, it is information From 79b02168d996ba6a31f8c61533c5a1e1a39f4e8d Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Mon, 9 Jan 2023 17:21:33 -0700 Subject: [PATCH 05/10] add Secret in private fn signatures in ops::registry and util::auth --- src/cargo/ops/registry.rs | 13 ++++++----- src/cargo/util/auth.rs | 46 ++++++++++++++++++++++----------------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index eef8404d3fe..75476b1cd84 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -174,7 +174,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { let (mut registry, reg_ids) = registry( opts.config, - opts.token.as_deref(), + opts.token.as_deref().map(Secret::from), opts.index.as_deref(), publish_registry.as_deref(), true, @@ -512,7 +512,7 @@ fn wait_for_publish( /// * `token_required`: If `true`, the token will be set. fn registry( config: &Config, - token_from_cmdline: Option<&str>, + token_from_cmdline: Option>, index: Option<&str>, registry: Option<&str>, force_update: bool, @@ -795,7 +795,8 @@ pub fn registry_login( let source_ids = get_source_id(config, None, reg)?; let reg_cfg = auth::registry_credential_config(config, &source_ids.original)?; - let login_url = match registry(config, token, None, reg, false, None) { + let token = token.map(Secret::from); + let login_url = match registry(config, token.clone(), None, reg, false, None) { Ok((registry, _)) => Some(format!("{}/me", registry.host())), Err(e) if e.is::() => e .downcast::() @@ -866,7 +867,7 @@ pub fn registry_login( )); } else { new_token = RegistryCredentialConfig::Token(match token { - Some(token) => Secret::from(token.to_string()), + Some(token) => token.owned(), None => { if let Some(login_url) = login_url { drop_println!( @@ -960,7 +961,7 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> { let (mut registry, _) = registry( config, - opts.token.as_deref(), + opts.token.as_deref().map(Secret::from), opts.index.as_deref(), opts.registry.as_deref(), true, @@ -1051,7 +1052,7 @@ pub fn yank( let (mut registry, _) = registry( config, - token.as_deref(), + token.as_deref().map(Secret::from), index.as_deref(), reg.as_deref(), true, diff --git a/src/cargo/util/auth.rs b/src/cargo/util/auth.rs index 4741210a6aa..ad0708c5130 100644 --- a/src/cargo/util/auth.rs +++ b/src/cargo/util/auth.rs @@ -377,14 +377,14 @@ my-registry = {{ index = "{}" }} } // Store a token in the cache for future calls. -pub fn cache_token(config: &Config, sid: &SourceId, token: &str) { +pub fn cache_token(config: &Config, sid: &SourceId, token: Secret<&str>) { let url = sid.canonical_url(); config.credential_cache().insert( url.clone(), CredentialCacheValue { from_commandline: true, independent_of_endpoint: true, - token_value: Secret::from(token.to_string()), + token_value: token.owned(), }, ); } @@ -399,7 +399,7 @@ pub fn auth_token( mutation: Option>, ) -> CargoResult { match auth_token_optional(config, sid, mutation.as_ref())? { - Some(token) => Ok(token), + Some(token) => Ok(token.expose()), None => Err(AuthorizationError { sid: sid.clone(), login_url: login_url.cloned(), @@ -414,7 +414,7 @@ fn auth_token_optional( config: &Config, sid: &SourceId, mutation: Option<&'_ Mutation<'_>>, -) -> CargoResult> { +) -> CargoResult>> { let mut cache = config.credential_cache(); let url = sid.canonical_url(); @@ -425,17 +425,19 @@ fn auth_token_optional( || cache_token_value.independent_of_endpoint || mutation.is_none() { - return Ok(Some(cache_token_value.token_value.clone().expose())); + return Ok(Some(cache_token_value.token_value.clone())); } } let credential = registry_credential_config(config, sid)?; let (independent_of_endpoint, token) = match credential { RegistryCredentialConfig::None => return Ok(None), - RegistryCredentialConfig::Token(config_token) => (true, config_token.expose()), + RegistryCredentialConfig::Token(config_token) => (true, config_token), RegistryCredentialConfig::Process(process) => { // todo: PASETO with process - run_command(config, &process, sid, Action::Get)?.unwrap() + let (independent_of_endpoint, token) = + run_command(config, &process, sid, Action::Get)?.unwrap(); + (independent_of_endpoint, Secret::from(token)) } RegistryCredentialConfig::AsymmetricKey((secret_key, secret_key_subject)) => { let secret: Secret> = @@ -496,18 +498,22 @@ fn auth_token_optional( ( false, - pasetors::version3::PublicToken::sign( - &secret.expose(), - serde_json::to_string(&message) - .expect("cannot serialize") - .as_bytes(), - Some( - serde_json::to_string(&footer) - .expect("cannot serialize") - .as_bytes(), - ), - None, - )?, + secret + .map(|secret| { + pasetors::version3::PublicToken::sign( + &secret, + serde_json::to_string(&message) + .expect("cannot serialize") + .as_bytes(), + Some( + serde_json::to_string(&footer) + .expect("cannot serialize") + .as_bytes(), + ), + None, + ) + }) + .transpose()?, ) } }; @@ -518,7 +524,7 @@ fn auth_token_optional( CredentialCacheValue { from_commandline: false, independent_of_endpoint, - token_value: Secret::from(token.to_string()), + token_value: token.clone(), }, ); } From 994ffbcdff94c863ee369a6f631f5f18bb3f025a Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Tue, 10 Jan 2023 16:08:30 -0700 Subject: [PATCH 06/10] add Secret to pub fns in ops::registry and corresponding commands --- src/bin/cargo/commands/login.rs | 3 ++- src/bin/cargo/commands/owner.rs | 3 ++- src/bin/cargo/commands/publish.rs | 5 ++++- src/bin/cargo/commands/yank.rs | 3 ++- src/cargo/ops/registry.rs | 14 +++++++------- src/cargo/util/auth.rs | 16 ++++++++-------- 6 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/bin/cargo/commands/login.rs b/src/bin/cargo/commands/login.rs index d2bd9c166df..14b4fc07891 100644 --- a/src/bin/cargo/commands/login.rs +++ b/src/bin/cargo/commands/login.rs @@ -1,6 +1,7 @@ use crate::command_prelude::*; use cargo::ops; +use cargo::util::auth::Secret; pub fn cli() -> Command { subcommand("login") @@ -36,7 +37,7 @@ pub fn cli() -> Command { pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { ops::registry_login( config, - args.get_one("token").map(String::as_str), + args.get_one("token").map(String::as_str).map(Secret::from), args.get_one("registry").map(String::as_str), args.flag("generate-keypair"), args.flag("secret-key"), diff --git a/src/bin/cargo/commands/owner.rs b/src/bin/cargo/commands/owner.rs index 170dd69e71e..493072b7b3a 100644 --- a/src/bin/cargo/commands/owner.rs +++ b/src/bin/cargo/commands/owner.rs @@ -1,6 +1,7 @@ use crate::command_prelude::*; use cargo::ops::{self, OwnersOptions}; +use cargo::util::auth::Secret; pub fn cli() -> Command { subcommand("owner") @@ -34,7 +35,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let registry = args.registry(config)?; let opts = OwnersOptions { krate: args.get_one::("crate").cloned(), - token: args.get_one::("token").cloned(), + token: args.get_one::("token").cloned().map(Secret::from), index: args.get_one::("index").cloned(), to_add: args .get_many::("add") diff --git a/src/bin/cargo/commands/publish.rs b/src/bin/cargo/commands/publish.rs index 7dd6414bada..1e37b87c106 100644 --- a/src/bin/cargo/commands/publish.rs +++ b/src/bin/cargo/commands/publish.rs @@ -1,6 +1,7 @@ use crate::command_prelude::*; use cargo::ops::{self, PublishOpts}; +use cargo::util::auth::Secret; pub fn cli() -> Command { subcommand("publish") @@ -36,7 +37,9 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { &ws, &PublishOpts { config, - token: args.get_one::("token").map(|s| s.to_string()), + token: args + .get_one::("token") + .map(|s| Secret::from(s.to_string())), index, verify: !args.flag("no-verify"), allow_dirty: args.flag("allow-dirty"), diff --git a/src/bin/cargo/commands/yank.rs b/src/bin/cargo/commands/yank.rs index dda766dd13c..3dee522793b 100644 --- a/src/bin/cargo/commands/yank.rs +++ b/src/bin/cargo/commands/yank.rs @@ -1,6 +1,7 @@ use crate::command_prelude::*; use cargo::ops; +use cargo::util::auth::Secret; pub fn cli() -> Command { subcommand("yank") @@ -37,7 +38,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { config, krate.map(|s| s.to_string()), version.map(|s| s.to_string()), - args.get_one::("token").cloned(), + args.get_one::("token").cloned().map(Secret::from), args.get_one::("index").cloned(), args.flag("undo"), registry, diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 75476b1cd84..3a68b329b1a 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -96,7 +96,7 @@ impl RegistryCredentialConfig { pub struct PublishOpts<'cfg> { pub config: &'cfg Config, - pub token: Option, + pub token: Option>, pub index: Option, pub verify: bool, pub allow_dirty: bool, @@ -174,7 +174,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { let (mut registry, reg_ids) = registry( opts.config, - opts.token.as_deref().map(Secret::from), + opts.token.as_ref().map(Secret::as_deref), opts.index.as_deref(), publish_registry.as_deref(), true, @@ -786,7 +786,7 @@ fn http_proxy_exists(config: &Config) -> CargoResult { pub fn registry_login( config: &Config, - token: Option<&str>, + token: Option>, reg: Option<&str>, generate_keypair: bool, secret_key_required: bool, @@ -939,7 +939,7 @@ pub fn registry_logout(config: &Config, reg: Option<&str>) -> CargoResult<()> { pub struct OwnersOptions { pub krate: Option, - pub token: Option, + pub token: Option>, pub index: Option, pub to_add: Option>, pub to_remove: Option>, @@ -961,7 +961,7 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> { let (mut registry, _) = registry( config, - opts.token.as_deref().map(Secret::from), + opts.token.as_ref().map(Secret::as_deref), opts.index.as_deref(), opts.registry.as_deref(), true, @@ -1020,7 +1020,7 @@ pub fn yank( config: &Config, krate: Option, version: Option, - token: Option, + token: Option>, index: Option, undo: bool, reg: Option, @@ -1052,7 +1052,7 @@ pub fn yank( let (mut registry, _) = registry( config, - token.as_deref().map(Secret::from), + token.as_ref().map(Secret::as_deref), index.as_deref(), reg.as_deref(), true, diff --git a/src/cargo/util/auth.rs b/src/cargo/util/auth.rs index ad0708c5130..20efc68089c 100644 --- a/src/cargo/util/auth.rs +++ b/src/cargo/util/auth.rs @@ -140,9 +140,9 @@ pub fn registry_credential_config( return registry_credential_config_inner( true, None, - token, + token.map(Secret::from), credential_process, - secret_key, + secret_key.map(Secret::from), secret_key_subject, config, ); @@ -231,9 +231,9 @@ pub fn registry_credential_config( registry_credential_config_inner( false, name.as_deref(), - token, + token.map(Secret::from), credential_process, - secret_key, + secret_key.map(Secret::from), secret_key_subject, config, ) @@ -242,9 +242,9 @@ pub fn registry_credential_config( fn registry_credential_config_inner( is_crates_io: bool, name: Option<&str>, - token: Option, + token: Option>, credential_process: Option, - secret_key: Option, + secret_key: Option>, secret_key_subject: Option, config: &Config, ) -> CargoResult { @@ -281,13 +281,13 @@ fn registry_credential_config_inner( registry )); } - (Some(token), _, _, _) => RegistryCredentialConfig::Token(Secret::from(token)), + (Some(token), _, _, _) => RegistryCredentialConfig::Token(token), (_, Some(process), _, _) => RegistryCredentialConfig::Process(( process.path.resolve_program(config), process.args, )), (None, None, Some(key), subject) => { - RegistryCredentialConfig::AsymmetricKey((Secret::from(key), subject)) + RegistryCredentialConfig::AsymmetricKey((key, subject)) } (None, None, None, _) => { if !is_crates_io { From e2832a80a5ab5bf8d945ccc5f051f52bc81165f8 Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Tue, 17 Jan 2023 14:37:59 -0700 Subject: [PATCH 07/10] clean up wrapping with Secret in commands --- src/bin/cargo/commands/login.rs | 3 +-- src/bin/cargo/commands/publish.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/bin/cargo/commands/login.rs b/src/bin/cargo/commands/login.rs index 14b4fc07891..dac0457d9e0 100644 --- a/src/bin/cargo/commands/login.rs +++ b/src/bin/cargo/commands/login.rs @@ -1,7 +1,6 @@ use crate::command_prelude::*; use cargo::ops; -use cargo::util::auth::Secret; pub fn cli() -> Command { subcommand("login") @@ -37,7 +36,7 @@ pub fn cli() -> Command { pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { ops::registry_login( config, - args.get_one("token").map(String::as_str).map(Secret::from), + args.get_one::("token").map(|s| s.as_str().into()), args.get_one("registry").map(String::as_str), args.flag("generate-keypair"), args.flag("secret-key"), diff --git a/src/bin/cargo/commands/publish.rs b/src/bin/cargo/commands/publish.rs index 1e37b87c106..c831d399f59 100644 --- a/src/bin/cargo/commands/publish.rs +++ b/src/bin/cargo/commands/publish.rs @@ -1,7 +1,6 @@ use crate::command_prelude::*; use cargo::ops::{self, PublishOpts}; -use cargo::util::auth::Secret; pub fn cli() -> Command { subcommand("publish") @@ -39,7 +38,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { config, token: args .get_one::("token") - .map(|s| Secret::from(s.to_string())), + .map(|s| s.to_string().into()), index, verify: !args.flag("no-verify"), allow_dirty: args.flag("allow-dirty"), From 36cad92e047aefdc41cacc63dea15b0df9663d4a Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Tue, 17 Jan 2023 14:38:59 -0700 Subject: [PATCH 08/10] remove redundant Secret::from --- src/cargo/ops/registry.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 3a68b329b1a..585592194f2 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -795,7 +795,6 @@ pub fn registry_login( let source_ids = get_source_id(config, None, reg)?; let reg_cfg = auth::registry_credential_config(config, &source_ids.original)?; - let token = token.map(Secret::from); let login_url = match registry(config, token.clone(), None, reg, false, None) { Ok((registry, _)) => Some(format!("{}/me", registry.host())), Err(e) if e.is::() => e From a32cef9e43a4ccfc2d8242c90b389c8db97413f2 Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Thu, 19 Jan 2023 15:01:34 -0700 Subject: [PATCH 09/10] improve docs for Secret; add doctest to assert that inner val is hidden --- src/cargo/util/auth.rs | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/auth.rs b/src/cargo/util/auth.rs index 20efc68089c..2e7606810d2 100644 --- a/src/cargo/util/auth.rs +++ b/src/cargo/util/auth.rs @@ -26,6 +26,16 @@ use super::config::CredentialCacheValue; /// /// This type does not implement `Display`, and has a `Debug` impl that hides /// the contained value. +/// +/// ``` +/// # use cargo::util::auth::Secret; +/// let token = Secret::from("super secret string"); +/// assert_eq!(format!("{:?}", token), "Secret { inner: \"REDACTED\" }"); +/// ``` +/// +/// Currently, we write a borrowed `Secret` as `Secret<&T>`. +/// The [`as_deref`](Secret::as_deref) and [`owned`](Secret::owned) methods can +/// be used to convert back and forth between `Secret` and `Secret<&str>`. #[derive(Clone, PartialEq, Eq)] pub struct Secret { inner: T, @@ -41,9 +51,11 @@ impl Secret { } /// Converts a `Secret` to a `Secret<&T::Target>`. - /// - /// For example, this can be used to convert from `&Secret` to - /// `Secret<&str>`. + /// ``` + /// # use cargo::util::auth::Secret; + /// let owned: Secret = Secret::from(String::from("token")); + /// let borrowed: Secret<&str> = owned.as_deref(); + /// ``` pub fn as_deref(&self) -> Secret<&::Target> where T: Deref, @@ -51,10 +63,12 @@ impl Secret { Secret::from(self.inner.deref()) } + /// Converts a `Secret` to a `Secret<&T>`. pub fn as_ref(&self) -> Secret<&T> { Secret::from(&self.inner) } + /// Converts a `Secret` to a `Secret` by applying `f` to the contained value. pub fn map(self, f: F) -> Secret where F: FnOnce(T) -> U, @@ -66,21 +80,25 @@ impl Secret { impl Secret<&T> { /// Converts a `Secret` containing a borrowed type to a `Secret` containing the /// corresponding owned type. - /// - /// For example, this can be used to convert from `Secret<&str>` to - /// `Secret`. + /// ``` + /// # use cargo::util::auth::Secret; + /// let borrowed: Secret<&str> = Secret::from("token"); + /// let owned: Secret = borrowed.owned(); + /// ``` pub fn owned(&self) -> Secret<::Owned> { Secret::from(self.inner.to_owned()) } } impl Secret> { + /// Converts a `Secret>` to a `Result, E>`. pub fn transpose(self) -> Result, E> { self.inner.map(|v| Secret::from(v)) } } impl> Secret { + /// Checks if the contained value is empty. pub fn is_empty(&self) -> bool { self.inner.as_ref().is_empty() } From efb972ae76c4e755b94466304c57dfc2b7a829d7 Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Thu, 19 Jan 2023 15:02:20 -0700 Subject: [PATCH 10/10] refactor some uses of Secret to avoid generating a token first before wrapping in Secret --- src/cargo/ops/registry.rs | 24 ++++++++++++++---------- src/cargo/util/auth.rs | 4 ++-- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 585592194f2..f8e884f39a7 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -834,19 +834,23 @@ pub fn registry_login( if generate_keypair { assert!(!secret_key_required); let kp = AsymmetricKeyPair::::generate().unwrap(); - let mut key = String::new(); - FormatAsPaserk::fmt(&kp.secret, &mut key).unwrap(); - secret_key = Secret::from(key); + secret_key = Secret::default().map(|mut key| { + FormatAsPaserk::fmt(&kp.secret, &mut key).unwrap(); + key + }); } else if secret_key_required { assert!(!generate_keypair); drop_println!(config, "please paste the API secret key below"); - let mut line = String::new(); - let input = io::stdin(); - input - .lock() - .read_line(&mut line) - .with_context(|| "failed to read stdin")?; - secret_key = Secret::from(line.trim().to_string()); + secret_key = Secret::default() + .map(|mut line| { + let input = io::stdin(); + input + .lock() + .read_line(&mut line) + .with_context(|| "failed to read stdin") + .map(|_| line.trim().to_string()) + }) + .transpose()?; } else { secret_key = old_secret_key .cloned() diff --git a/src/cargo/util/auth.rs b/src/cargo/util/auth.rs index 2e7606810d2..1dfa85acb47 100644 --- a/src/cargo/util/auth.rs +++ b/src/cargo/util/auth.rs @@ -34,9 +34,9 @@ use super::config::CredentialCacheValue; /// ``` /// /// Currently, we write a borrowed `Secret` as `Secret<&T>`. -/// The [`as_deref`](Secret::as_deref) and [`owned`](Secret::owned) methods can +/// The [`as_deref`](Secret::as_deref) and [`owned`](Secret::owned) methods can /// be used to convert back and forth between `Secret` and `Secret<&str>`. -#[derive(Clone, PartialEq, Eq)] +#[derive(Default, Clone, PartialEq, Eq)] pub struct Secret { inner: T, }