Skip to content

Commit

Permalink
Auto merge of #12671 - arlosi:cred-shadow, r=epage
Browse files Browse the repository at this point in the history
fix: emit a warning for `credential-alias` shadowing

### What does this PR try to resolve?
If a `credential-alias` shadows a built-in provider the user could be confused about which provider is being used.

### How should we review this PR?
See the test to see what the warning looks like.

r? `@epage` who listed this as a concern on the FCP in #8933
  • Loading branch information
bors committed Sep 14, 2023
2 parents 64642cc + 8f18f2b commit d5336f8
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 3 deletions.
27 changes: 24 additions & 3 deletions src/cargo/util/auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,19 @@ fn registry_credential_config_raw_uncached(
/// Use the `[credential-alias]` table to see if the provider name has been aliased.
fn resolve_credential_alias(config: &Config, mut provider: PathAndArgs) -> Vec<String> {
if provider.args.is_empty() {
let key = format!("credential-alias.{}", provider.path.raw_value());
if let Ok(alias) = config.get::<PathAndArgs>(&key) {
let name = provider.path.raw_value();
let key = format!("credential-alias.{name}");
if let Ok(alias) = config.get::<Value<PathAndArgs>>(&key) {
tracing::debug!("resolving credential alias '{key}' -> '{alias:?}'");
provider = alias;
if BUILT_IN_PROVIDERS.contains(&name) {
let _ = config.shell().warn(format!(
"credential-alias `{name}` (defined in `{}`) will be \
ignored because it would shadow a built-in credential-provider",
alias.definition
));
} else {
provider = alias.val;
}
}
}
provider.args.insert(
Expand Down Expand Up @@ -470,6 +479,17 @@ pub fn cache_token_from_commandline(config: &Config, sid: &SourceId, token: Secr
);
}

/// List of credential providers built-in to Cargo.
/// Keep in sync with the `match` in `credential_action`.
static BUILT_IN_PROVIDERS: &[&'static str] = &[
"cargo:token",
"cargo:paseto",
"cargo:token-from-stdout",
"cargo:wincred",
"cargo:macos-keychain",
"cargo:libsecret",
];

fn credential_action(
config: &Config,
sid: &SourceId,
Expand Down Expand Up @@ -497,6 +517,7 @@ fn credential_action(
.collect();
let process = args[0];
tracing::debug!("attempting credential provider: {args:?}");
// If the available built-in providers are changed, update the `BUILT_IN_PROVIDERS` list.
let provider: Box<dyn Credential> = match process {
"cargo:token" => Box::new(TokenCredential::new(config)),
"cargo:paseto" if config.cli_unstable().asymmetric_token => {
Expand Down
30 changes: 30 additions & 0 deletions tests/testsuite/credential_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,3 +745,33 @@ Caused by:
)
.run();
}

#[cargo_test]
fn alias_builtin_warning() {
let registry = registry::RegistryBuilder::new()
.credential_provider(&[&"cargo:token"])
.build();

cargo_util::paths::append(
&paths::home().join(".cargo/config"),
format!(
r#"
[credential-alias]
"cargo:token" = ["ignored"]
"#,
)
.as_bytes(),
)
.unwrap();

cargo_process("login -Z credential-process abcdefg")
.masquerade_as_nightly_cargo(&["credential-process"])
.replace_crates_io(registry.index_url())
.with_stderr(
r#"[UPDATING] [..]
[WARNING] credential-alias `cargo:token` (defined in `[..]`) will be ignored because it would shadow a built-in credential-provider
[LOGIN] token for `crates-io` saved
"#,
)
.run();
}

0 comments on commit d5336f8

Please sign in to comment.