fix(toml): Reject registry-index in user-written manifests

Author: moabo3li

src/cargo/core/source_id.rs

@@ -387,6 +387,11 @@ impl SourceId {
         matches!(self.inner.kind, SourceKind::Git(_))
     }
 
+    /// Returns `true` if this source is from a directory.
+    pub fn is_directory(self) -> bool {
+        self.inner.kind == SourceKind::Directory
+    }
+
     /// Creates an implementation of `Source` corresponding to this ID.
     ///
     /// * `yanked_whitelist` --- Packages allowed to be used, even if they are yanked.

src/cargo/core/workspace.rs

@@ -97,6 +97,11 @@ pub struct Workspace<'gctx> {
     /// `cargo install` or `cargo package` commands.
     is_ephemeral: bool,
 
+    /// `true` if this workspace is being used for package verification.
+    /// This is a special case of ephemeral workspace where we want to preload
+    /// cargo-generated manifests to avoid re-parsing them without the proper flags.
+    is_verifying_package: bool,
+
     /// `true` if this workspace should enforce optional dependencies even when
     /// not needed; false if this workspace should only enforce dependencies
     /// needed by the current configuration (such as in cargo install). In some
@@ -254,6 +259,7 @@ impl<'gctx> Workspace<'gctx> {
             member_ids: HashSet::new(),
             default_members: Vec::new(),
             is_ephemeral: false,
+            is_verifying_package: false,
             require_optional_deps: true,
             loaded_packages: RefCell::new(HashMap::new()),
             ignore_lock: false,
@@ -646,6 +652,15 @@ impl<'gctx> Workspace<'gctx> {
         self.is_ephemeral
     }
 
+    pub fn is_verifying_package(&self) -> bool {
+        self.is_verifying_package
+    }
+
+    pub fn set_verifying_package(&mut self, verifying: bool) -> &mut Workspace<'gctx> {
+        self.is_verifying_package = verifying;
+        self
+    }
+
     pub fn require_optional_deps(&self) -> bool {
         self.require_optional_deps
     }
@@ -1206,7 +1221,10 @@ impl<'gctx> Workspace<'gctx> {
         // `PathSource` with multiple entries in it, so the logic below is
         // mostly just an optimization for normal `cargo build` in workspaces
         // during development.
-        if self.is_ephemeral {
+        //
+        // However, for package verification workspaces, we do want to preload
+        // to avoid re-reading cargo-generated manifests without the proper flags.
+        if self.is_ephemeral && !self.is_verifying_package {
             return;
         }
 

src/cargo/ops/cargo_package/verify.rs

@@ -63,13 +63,16 @@ pub fn run_verify(
     // package has a workspace we can still build our new crate.
     let id = SourceId::for_path(&dst)?;
     let mut src = PathSource::new(&dst, id, ws.gctx());
-    let new_pkg = src.root_package()?;
+    let new_pkg = src.root_cargo_generated_package()?;
     let pkg_fingerprint = hash_all(&dst)?;
 
     // When packaging we use an ephemeral workspace but reuse the build cache to reduce
     // verification time if the user has already compiled the dependencies and the fingerprint
     // is unchanged.
     let mut ws = Workspace::ephemeral(new_pkg, gctx, Some(ws.build_dir()), true)?;
+    // Mark this as a verification workspace so that the package gets preloaded
+    // with the cargo_generated flag set
+    ws.set_verifying_package(true);
     if let Some(local_reg) = local_reg {
         ws.add_local_overlay(
             local_reg.upstream,

src/cargo/ops/cargo_read_manifest.rs

@@ -3,7 +3,7 @@ use std::path::Path;
 use crate::core::{EitherManifest, Package, SourceId};
 use crate::util::GlobalContext;
 use crate::util::errors::CargoResult;
-use crate::util::toml::read_manifest;
+use crate::util::toml::{read_manifest, read_cargo_generated_manifest};
 use tracing::trace;
 
 pub fn read_package(
@@ -28,3 +28,31 @@ pub fn read_package(
 
     Ok(Package::new(manifest, path))
 }
+
+/// Reads a cargo-generated package manifest.
+///
+/// This is for reading packages that were generated by cargo itself
+/// (e.g., during package verification), which may contain internal-only
+/// fields like `registry-index`.
+pub fn read_cargo_generated_package(
+    path: &Path,
+    source_id: SourceId,
+    gctx: &GlobalContext,
+) -> CargoResult<Package> {
+    trace!(
+        "read_cargo_generated_package; path={}; source-id={}",
+        path.display(),
+        source_id
+    );
+    let manifest = read_cargo_generated_manifest(path, source_id, gctx)?;
+    let manifest = match manifest {
+        EitherManifest::Real(manifest) => manifest,
+        EitherManifest::Virtual(..) => anyhow::bail!(
+            "found a virtual manifest at `{}` instead of a package \
+             manifest",
+            path.display()
+        ),
+    };
+
+    Ok(Package::new(manifest, path))
+}

src/cargo/ops/mod.rs

@@ -16,7 +16,7 @@ pub use self::cargo_package::PackageOpts;
 pub use self::cargo_package::check_yanked;
 pub use self::cargo_package::package;
 pub use self::cargo_pkgid::pkgid;
-pub use self::cargo_read_manifest::read_package;
+pub use self::cargo_read_manifest::{read_package, read_cargo_generated_package};
 pub use self::cargo_run::run;
 pub use self::cargo_test::{TestOptions, run_benches, run_tests};
 pub use self::cargo_uninstall::uninstall;

src/cargo/sources/path.rs

@@ -83,6 +83,26 @@ impl<'gctx> PathSource<'gctx> {
         }
     }
 
+    /// Gets the cargo-generated package on the root path.
+    ///
+    /// This is for reading packages that were generated by cargo itself
+    /// (e.g., during package verification).
+    pub fn root_cargo_generated_package(&mut self) -> CargoResult<Package> {
+        trace!("root_cargo_generated_package; source={:?}", self);
+
+        if self.package.is_none() {
+            self.package = Some(self.read_cargo_generated_package()?);
+        }
+
+        match &self.package {
+            Some(pkg) => Ok(pkg.clone()),
+            None => Err(internal(format!(
+                "no package found in source {:?}",
+                self.path
+            ))),
+        }
+    }
+
     /// List all files relevant to building this package inside this source.
     ///
     /// This function will use the appropriate methods to determine the
@@ -128,6 +148,12 @@ impl<'gctx> PathSource<'gctx> {
         let pkg = ops::read_package(&path, self.source_id, self.gctx)?;
         Ok(pkg)
     }
+
+    fn read_cargo_generated_package(&self) -> CargoResult<Package> {
+        let path = self.path.join("Cargo.toml");
+        let pkg = ops::read_cargo_generated_package(&path, self.source_id, self.gctx)?;
+        Ok(pkg)
+    }
 }
 
 impl<'gctx> Debug for PathSource<'gctx> {

src/cargo/util/toml/mod.rs

@@ -65,6 +65,31 @@ pub fn read_manifest(
     path: &Path,
     source_id: SourceId,
     gctx: &GlobalContext,
+) -> CargoResult<EitherManifest> {
+    read_manifest_impl(path, source_id, gctx, false)
+}
+
+/// Loads a cargo-generated `Cargo.toml` from a file on disk.
+///
+/// This is for reading manifests that were generated by cargo itself
+/// (e.g., during package verification), which may contain internal-only
+/// fields like `registry-index`.
+#[tracing::instrument(skip(gctx))]
+pub fn read_cargo_generated_manifest(
+    path: &Path,
+    source_id: SourceId,
+    gctx: &GlobalContext,
+) -> CargoResult<EitherManifest> {
+    read_manifest_impl(path, source_id, gctx, true)
+}
+
+/// Internal implementation with cargo_generated parameter
+#[tracing::instrument(skip(gctx))]
+fn read_manifest_impl(
+    path: &Path,
+    source_id: SourceId,
+    gctx: &GlobalContext,
+    cargo_generated: bool,
 ) -> CargoResult<EitherManifest> {
     let mut warnings = Default::default();
     let mut errors = Default::default();
@@ -99,7 +124,7 @@ pub fn read_manifest(
         )?;
 
         if normalized_toml.package().is_some() {
-            to_real_manifest(
+            to_real_manifest_impl(
                 contents,
                 document,
                 original_toml,
@@ -109,6 +134,7 @@ pub fn read_manifest(
                 source_id,
                 path,
                 is_embedded,
+                cargo_generated,
                 gctx,
                 &mut warnings,
                 &mut errors,
@@ -1276,6 +1302,40 @@ pub fn to_real_manifest(
     gctx: &GlobalContext,
     warnings: &mut Vec<String>,
     _errors: &mut Vec<String>,
+) -> CargoResult<Manifest> {
+    to_real_manifest_impl(
+        contents,
+        document,
+        original_toml,
+        normalized_toml,
+        features,
+        workspace_config,
+        source_id,
+        manifest_file,
+        is_embedded,
+        false,
+        gctx,
+        warnings,
+        _errors,
+    )
+}
+
+/// Internal implementation with cargo_generated parameter
+#[tracing::instrument(skip_all)]
+fn to_real_manifest_impl(
+    contents: String,
+    document: toml::Spanned<toml::de::DeTable<'static>>,
+    original_toml: manifest::TomlManifest,
+    normalized_toml: manifest::TomlManifest,
+    features: Features,
+    workspace_config: WorkspaceConfig,
+    source_id: SourceId,
+    manifest_file: &Path,
+    is_embedded: bool,
+    cargo_generated: bool,
+    gctx: &GlobalContext,
+    warnings: &mut Vec<String>,
+    _errors: &mut Vec<String>,
 ) -> CargoResult<Manifest> {
     let package_root = manifest_file.parent().unwrap();
     if !package_root.is_dir() {
@@ -1582,6 +1642,7 @@ pub fn to_real_manifest(
         warnings,
         platform: None,
         root: package_root,
+        cargo_generated,
     };
     gather_dependencies(
         &mut manifest_ctx,
@@ -1977,6 +2038,7 @@ fn to_virtual_manifest(
             warnings,
             platform: None,
             root,
+            cargo_generated: false,
         };
         (
             replace(&normalized_toml, &mut manifest_ctx)?,
@@ -2045,6 +2107,7 @@ struct ManifestContext<'a, 'b> {
     warnings: &'a mut Vec<String>,
     platform: Option<Platform>,
     root: &'a Path,
+    cargo_generated: bool,
 }
 
 #[tracing::instrument(skip_all)]
@@ -2175,6 +2238,7 @@ pub(crate) fn to_dependency<P: ResolveToPath + Clone>(
             warnings,
             platform,
             root,
+            cargo_generated: false,
         },
         kind,
     )
@@ -2292,6 +2356,19 @@ fn detailed_dep_to_dependency<P: ResolveToPath + Clone>(
         dep.set_registry_id(registry_id);
     }
     if let Some(registry_index) = &orig.registry_index {
+        // `registry-index` is for internal use only.
+        // It should not be used in user-written manifests as it bypasses the need for .cargo/config.toml configuration.
+
+        if !manifest_ctx.source_id.is_registry()
+            && !manifest_ctx.source_id.is_directory()
+            && !manifest_ctx.cargo_generated
+        {
+            bail!(
+                "dependency ({}) specification uses `registry-index` which is for internal use only\n\
+                help: use `registry = \"<name>\"` and configure the registry in `.cargo/config.toml`",
+                name_in_toml
+            );
+        }
         let url = registry_index.into_url()?;
         let registry_id = SourceId::for_registry(&url)?;
         dep.set_registry_id(registry_id);
@@ -2936,7 +3013,8 @@ pub fn prepare_for_publish(
     let mut warnings = Default::default();
     let mut errors = Default::default();
     let gctx = ws.gctx();
-    let manifest = to_real_manifest(
+    let cargo_generated = true;
+    let manifest = to_real_manifest_impl(
         contents.to_owned(),
         document.clone(),
         original_toml,
@@ -2946,6 +3024,7 @@ pub fn prepare_for_publish(
         source_id,
         me.manifest_path(),
         me.manifest().is_embedded(),
+        cargo_generated,
         gctx,
         &mut warnings,
         &mut errors,

tests/testsuite/alt_registry.rs

@@ -285,18 +285,14 @@ fn registry_index_not_allowed_in_user_manifests() {
         .file("src/lib.rs", "")
         .build();
 
-    // FIXME: This currently allows `registry-index` which is a bug.
-    // It should error during manifest parsing because `registry-index` is for internal use only.
-    // Instead, it tries to fetch from the URL and fails with a network error.
     p.cargo("check")
+        .with_status(101)
         .with_stderr_data(str![[r#"
-[UPDATING] `[ROOT]/alternative-registry` index
-[LOCKING] 1 package to latest compatible version
-[DOWNLOADING] crates ...
-[DOWNLOADED] bar v0.1.0 (registry `[ROOT]/alternative-registry`)
-[CHECKING] bar v0.1.0 (registry `[ROOT]/alternative-registry`)
-[CHECKING] foo v0.0.0 ([ROOT]/foo)
-[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
+
+Caused by:
+  dependency (bar) specification uses `registry-index` which is for internal use only
+  [HELP] use `registry = "<name>"` and configure the registry in `.cargo/config.toml`
 
 "#]])
         .run();

tests/testsuite/publish.rs

@@ -4628,26 +4628,14 @@ fn registry_index_not_allowed_in_publish() {
         .file("src/main.rs", "fn main() {}")
         .build();
 
-    // FIXME: This currently allows `registry-index` which is a bug.
-    // It should error during manifest parsing because `registry-index` is for internal use only.
-    // Instead, it actually works and the package can be published.
     p.cargo("publish --registry alternative")
+        .with_status(101)
         .with_stderr_data(str![[r#"
-[UPDATING] `alternative` index
-[PACKAGING] foo v0.0.1 ([ROOT]/foo)
-[UPDATING] `[ROOT]/registry` index
-[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
-[VERIFYING] foo v0.0.1 ([ROOT]/foo)
-[DOWNLOADING] crates ...
-[DOWNLOADED] bar v0.1.0 (registry `[ROOT]/registry`)
-[COMPILING] bar v0.1.0 (registry `[ROOT]/registry`)
-[COMPILING] foo v0.0.1 ([ROOT]/foo/target/package/foo-0.0.1)
-[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
-[UPLOADING] foo v0.0.1 ([ROOT]/foo)
-[UPLOADED] foo v0.0.1 to registry `alternative`
-[NOTE] waiting for foo v0.0.1 to be available at registry `alternative`
-[HELP] you may press ctrl-c to skip waiting; the crate should be available shortly
-[PUBLISHED] foo v0.0.1 at registry `alternative`
+[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
+
+Caused by:
+  dependency (bar) specification uses `registry-index` which is for internal use only
+  [HELP] use `registry = "<name>"` and configure the registry in `.cargo/config.toml`
 
 "#]])
         .run();