Auto merge of #7973 - ehuss:index-updates, r=alexcrichton

Several updates to token/index handling.

This attempts to tighten up the usage of token/index handling, to prevent accidental leakage of the crates.io token.

* Make `registry.index` config a hard error. This was deprecated 4 years ago in #2857, and removing it helps simplify things.
* Don't allow both `--index` and `--registry` to be specified at the same time. Otherwise `--index` was being silently ignored.
* `registry.token` is not allowed to be used with the `--index` flag. The intent here is to avoid possibly leaking a crates.io token to another host.
* Added a warning if source replacement is used and the token is loaded from `registry.token`.

Closes #6545

Author: bors

Cargo.toml

@@ -22,7 +22,7 @@ path = "src/cargo/lib.rs"
 atty = "0.2"
 bytesize = "1.0"
 cargo-platform = { path = "crates/cargo-platform", version = "0.1.1" }
-crates-io = { path = "crates/crates-io", version = "0.31" }
+crates-io = { path = "crates/crates-io", version = "0.31.1" }
 crossbeam-utils = "0.7"
 crypto-hash = "0.3.1"
 curl = { version = "0.4.23", features = ["http2"] }

crates/crates-io/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "crates-io"
-version = "0.31.0"
+version = "0.31.1"
 edition = "2018"
 authors = ["Alex Crichton <[email protected]>"]
 license = "MIT OR Apache-2.0"

crates/crates-io/lib.rs

@@ -139,9 +139,7 @@ impl Registry {
     }
 
     pub fn host_is_crates_io(&self) -> bool {
-        Url::parse(self.host())
-            .map(|u| u.host_str() == Some("crates.io"))
-            .unwrap_or(false)
+        is_url_crates_io(&self.host)
     }
 
     pub fn add_owners(&mut self, krate: &str, owners: &[&str]) -> Result<String> {
@@ -420,3 +418,10 @@ fn reason(code: u32) -> &'static str {
         _ => "<unknown>",
     }
 }
+
+/// Returns `true` if the host of the given URL is "crates.io".
+pub fn is_url_crates_io(url: &str) -> bool {
+    Url::parse(url)
+        .map(|u| u.host_str() == Some("crates.io"))
+        .unwrap_or(false)
+}

src/cargo/core/source/source_id.rs

@@ -4,8 +4,6 @@ use std::fmt::{self, Formatter};
 use std::hash::{self, Hash};
 use std::path::Path;
 use std::ptr;
-use std::sync::atomic::AtomicBool;
-use std::sync::atomic::Ordering::SeqCst;
 use std::sync::Mutex;
 
 use log::trace;
@@ -14,7 +12,6 @@ use serde::ser;
 use url::Url;
 
 use crate::core::PackageId;
-use crate::ops;
 use crate::sources::DirectorySource;
 use crate::sources::{GitSource, PathSource, RegistrySource, CRATES_IO_INDEX};
 use crate::util::{CanonicalUrl, CargoResult, Config, IntoUrl};
@@ -189,22 +186,8 @@ impl SourceId {
     /// a `.cargo/config`.
     pub fn crates_io(config: &Config) -> CargoResult<SourceId> {
         config.crates_io_source_id(|| {
-            let cfg = ops::registry_configuration(config, None)?;
-            let url = if let Some(ref index) = cfg.index {
-                static WARNED: AtomicBool = AtomicBool::new(false);
-                if !WARNED.swap(true, SeqCst) {
-                    config.shell().warn(
-                        "custom registry support via \
-                         the `registry.index` configuration is \
-                         being removed, this functionality \
-                         will not work in the future",
-                    )?;
-                }
-                &index[..]
-            } else {
-                CRATES_IO_INDEX
-            };
-            let url = url.into_url()?;
+            config.check_registry_index_not_set()?;
+            let url = CRATES_IO_INDEX.into_url().unwrap();
             SourceId::for_registry(&url)
         })
     }

src/cargo/ops/registry.rs

@@ -7,7 +7,7 @@ use std::time::Duration;
 use std::{cmp, env};
 
 use anyhow::{bail, format_err};
-use crates_io::{NewCrate, NewCrateDependency, Registry};
+use crates_io::{self, NewCrate, NewCrateDependency, Registry};
 use curl::easy::{Easy, InfoType, SslOpt, SslVersion};
 use log::{log, Level};
 use percent_encoding::{percent_encode, NON_ALPHANUMERIC};
@@ -25,8 +25,13 @@ use crate::util::IntoUrl;
 use crate::util::{paths, validate_package_name};
 use crate::version;
 
+/// Registry settings loaded from config files.
+///
+/// This is loaded based on the `--registry` flag and the config settings.
 pub struct RegistryConfig {
+    /// The index URL. If `None`, use crates.io.
     pub index: Option<String>,
+    /// The authentication token.
     pub token: Option<String>,
 }
 
@@ -316,10 +321,15 @@ fn transmit(
     }
 }
 
+/// Returns the index and token from the config file for the given registry.
+///
+/// `registry` is typically the registry specified on the command-line. If
+/// `None`, `index` is set to `None` to indicate it should use crates.io.
 pub fn registry_configuration(
     config: &Config,
     registry: Option<String>,
 ) -> CargoResult<RegistryConfig> {
+    // `registry.default` is handled in command-line parsing.
     let (index, token) = match registry {
         Some(registry) => {
             validate_package_name(&registry, "registry name", "")?;
@@ -331,19 +341,26 @@ pub fn registry_configuration(
             )
         }
         None => {
-            // Checking for default index and token
-            (
-                config
-                    .get_default_registry_index()?
-                    .map(|url| url.to_string()),
-                config.get_string("registry.token")?.map(|p| p.val),
-            )
+            // Use crates.io default.
+            config.check_registry_index_not_set()?;
+            (None, config.get_string("registry.token")?.map(|p| p.val))
         }
     };
 
     Ok(RegistryConfig { index, token })
 }
 
+/// Returns the `Registry` and `Source` based on command-line and config settings.
+///
+/// * `token`: The token from the command-line. If not set, uses the token
+///   from the config.
+/// * `index`: The index URL from the command-line. This is ignored if
+///   `registry` is set.
+/// * `registry`: The registry name from the command-line. If neither
+///   `registry`, or `index` are set, then uses `crates-io`, honoring
+///   `[source]` replacement if defined.
+/// * `force_update`: If `true`, forces the index to be updated.
+/// * `validate_token`: If `true`, the token must be set.
 fn registry(
     config: &Config,
     token: Option<String>,
@@ -352,13 +369,17 @@ fn registry(
     force_update: bool,
     validate_token: bool,
 ) -> CargoResult<(Registry, SourceId)> {
+    if index.is_some() && registry.is_some() {
+        // Otherwise we would silently ignore one or the other.
+        bail!("both `--index` and `--registry` should not be set at the same time");
+    }
     // Parse all configuration options
     let RegistryConfig {
         token: token_config,
         index: index_config,
     } = registry_configuration(config, registry.clone())?;
-    let token = token.or(token_config);
-    let sid = get_source_id(config, index_config.or(index), registry)?;
+    let opt_index = index_config.as_ref().or(index.as_ref());
+    let sid = get_source_id(config, opt_index, registry.as_ref())?;
     if !sid.is_remote_registry() {
         bail!(
             "{} does not support API commands.\n\
@@ -386,10 +407,51 @@ fn registry(
         cfg.and_then(|cfg| cfg.api)
             .ok_or_else(|| format_err!("{} does not support API commands", sid))?
     };
-    let handle = http_handle(config)?;
-    if validate_token && token.is_none() {
-        bail!("no upload token found, please run `cargo login`");
+    let token = match (&index, &token, &token_config) {
+        // No token.
+        (None, None, None) => {
+            if validate_token {
+                bail!("no upload token found, please run `cargo login` or pass `--token`");
+            }
+            None
+        }
+        // Token on command-line.
+        (_, Some(_), _) => token,
+        // Token in config, no --index, loading from config is OK for crates.io.
+        (None, None, Some(_)) => {
+            // Check `is_default_registry` so that the crates.io index can
+            // change config.json's "api" value, and this won't affect most
+            // people. It will affect those using source replacement, but
+            // hopefully that's a relatively small set of users.
+            if registry.is_none()
+                && !sid.is_default_registry()
+                && !crates_io::is_url_crates_io(&api_host)
+            {
+                if validate_token {
+                    config.shell().warn(
+                        "using `registry.token` config value with source \
+                        replacement is deprecated\n\
+                        This may become a hard error in the future; \
+                        see <https://github.com/rust-lang/cargo/issues/xxx>.\n\
+                        Use the --token command-line flag to remove this warning.",
+                    )?;
+                    token_config
+                } else {
+                    None
+                }
+            } else {
+                token_config
+            }
+        }
+        // --index, no --token
+        (Some(_), None, _) => {
+            if validate_token {
+                bail!("command-line argument --index requires --token to be specified")
+            }
+            None
+        }
     };
+    let handle = http_handle(config)?;
     Ok((Registry::new_handle(api_host, token, handle), sid))
 }
 
@@ -739,10 +801,14 @@ pub fn yank(
     Ok(())
 }
 
+/// Gets the SourceId for an index or registry setting.
+///
+/// The `index` and `reg` values are from the command-line or config settings.
+/// If both are None, returns the source for crates.io.
 fn get_source_id(
     config: &Config,
-    index: Option<String>,
-    reg: Option<String>,
+    index: Option<&String>,
+    reg: Option<&String>,
 ) -> CargoResult<SourceId> {
     match (reg, index) {
         (Some(r), _) => SourceId::alt_registry(config, &r),

src/cargo/util/config/mod.rs

@@ -1016,12 +1016,15 @@ impl Config {
         )
     }
 
-    /// Gets the index for the default registry.
-    pub fn get_default_registry_index(&self) -> CargoResult<Option<Url>> {
-        Ok(match self.get_string("registry.index")? {
-            Some(index) => Some(self.resolve_registry_index(index)?),
-            None => None,
-        })
+    /// Returns an error if `registry.index` is set.
+    pub fn check_registry_index_not_set(&self) -> CargoResult<()> {
+        if self.get_string("registry.index")?.is_some() {
+            bail!(
+                "the `registry.index` config value is no longer supported\n\
+                Use `[source]` replacement to alter the default index for crates.io."
+            );
+        }
+        Ok(())
     }
 
     fn resolve_registry_index(&self, index: Value<String>) -> CargoResult<Url> {

src/doc/src/reference/config.md

@@ -686,7 +686,7 @@ specified.
 
 ##### `registry.index`
 
-This value is deprecated and should not be used.
+This value is no longer accepted and should not be used.
 
 ##### `registry.default`
 * Type: string