Skip to content

Commit d5336f8

Browse files
committed
Auto merge of #12671 - arlosi:cred-shadow, r=epage
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
2 parents 64642cc + 8f18f2b commit d5336f8

File tree

2 files changed

+54
-3
lines changed

2 files changed

+54
-3
lines changed

src/cargo/util/auth/mod.rs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -335,10 +335,19 @@ fn registry_credential_config_raw_uncached(
335335
/// Use the `[credential-alias]` table to see if the provider name has been aliased.
336336
fn resolve_credential_alias(config: &Config, mut provider: PathAndArgs) -> Vec<String> {
337337
if provider.args.is_empty() {
338-
let key = format!("credential-alias.{}", provider.path.raw_value());
339-
if let Ok(alias) = config.get::<PathAndArgs>(&key) {
338+
let name = provider.path.raw_value();
339+
let key = format!("credential-alias.{name}");
340+
if let Ok(alias) = config.get::<Value<PathAndArgs>>(&key) {
340341
tracing::debug!("resolving credential alias '{key}' -> '{alias:?}'");
341-
provider = alias;
342+
if BUILT_IN_PROVIDERS.contains(&name) {
343+
let _ = config.shell().warn(format!(
344+
"credential-alias `{name}` (defined in `{}`) will be \
345+
ignored because it would shadow a built-in credential-provider",
346+
alias.definition
347+
));
348+
} else {
349+
provider = alias.val;
350+
}
342351
}
343352
}
344353
provider.args.insert(
@@ -470,6 +479,17 @@ pub fn cache_token_from_commandline(config: &Config, sid: &SourceId, token: Secr
470479
);
471480
}
472481

482+
/// List of credential providers built-in to Cargo.
483+
/// Keep in sync with the `match` in `credential_action`.
484+
static BUILT_IN_PROVIDERS: &[&'static str] = &[
485+
"cargo:token",
486+
"cargo:paseto",
487+
"cargo:token-from-stdout",
488+
"cargo:wincred",
489+
"cargo:macos-keychain",
490+
"cargo:libsecret",
491+
];
492+
473493
fn credential_action(
474494
config: &Config,
475495
sid: &SourceId,
@@ -497,6 +517,7 @@ fn credential_action(
497517
.collect();
498518
let process = args[0];
499519
tracing::debug!("attempting credential provider: {args:?}");
520+
// If the available built-in providers are changed, update the `BUILT_IN_PROVIDERS` list.
500521
let provider: Box<dyn Credential> = match process {
501522
"cargo:token" => Box::new(TokenCredential::new(config)),
502523
"cargo:paseto" if config.cli_unstable().asymmetric_token => {

tests/testsuite/credential_process.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,3 +745,33 @@ Caused by:
745745
)
746746
.run();
747747
}
748+
749+
#[cargo_test]
750+
fn alias_builtin_warning() {
751+
let registry = registry::RegistryBuilder::new()
752+
.credential_provider(&[&"cargo:token"])
753+
.build();
754+
755+
cargo_util::paths::append(
756+
&paths::home().join(".cargo/config"),
757+
format!(
758+
r#"
759+
[credential-alias]
760+
"cargo:token" = ["ignored"]
761+
"#,
762+
)
763+
.as_bytes(),
764+
)
765+
.unwrap();
766+
767+
cargo_process("login -Z credential-process abcdefg")
768+
.masquerade_as_nightly_cargo(&["credential-process"])
769+
.replace_crates_io(registry.index_url())
770+
.with_stderr(
771+
r#"[UPDATING] [..]
772+
[WARNING] credential-alias `cargo:token` (defined in `[..]`) will be ignored because it would shadow a built-in credential-provider
773+
[LOGIN] token for `crates-io` saved
774+
"#,
775+
)
776+
.run();
777+
}

0 commit comments

Comments
 (0)