diff --git a/README.md b/README.md index c8a9a90fd..b3e49b074 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,30 @@ Give each job its own `--check-name` (Bazel defaults to one would overwrite each other's findings. `--reproduce` and `--rerun-job` set the corresponding sections of the Details page. +## Cargo publish policy + +`fslabscli check-cargo-publish-policy` compares committed Cargo manifests at +`PULL_BASE_SHA` and `PULL_PULL_SHA`. For a pull request, `PULL_PULL_SHA` must +resolve to the prospective merge commit rather than the branch head. This +keeps crates added to the current base visible when an older branch is merged. + +- `--check approval` fails when a crate is newly added with + `[package.metadata.fslabs.publish.cargo] publish = true`, or changes from + missing/false to true, unless `PULL_REQUEST_LABELS` contains `add-crate`. +- `--check registry` fails when a crate marked on both the pull request base + and head does not exist in `--cargo-target-registry` by name. Removing or + unmarking an unpublished crate remains possible as a recovery change. +- `--check all` runs both checks and is the default. + +The command reads manifests directly from Git objects and never publishes. +Normal version changes to an already marked crate do not require `add-crate`. +Publication remains release-tag-driven. If a new crate depends on an +unpublished local crate version, the release plan includes that exact version +and publishes it before the new crate. If the dependency's version already +exists, registry users receive that published artifact; unpublished source +changes with the same version are not distributed. Pull requests and ordinary +main merges publish nothing. + ## Release Process **Version source of truth:** `Cargo.toml` diff --git a/src/cargo_publish_policy.rs b/src/cargo_publish_policy.rs new file mode 100644 index 000000000..c3f281451 --- /dev/null +++ b/src/cargo_publish_policy.rs @@ -0,0 +1,297 @@ +use std::collections::{BTreeMap, BTreeSet}; +use std::path::Path; + +use anyhow::Context; +use gix::bstr::ByteSlice; +use serde::Serialize; + +use crate::utils::cargo::CrateChecker; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct MarkedCargoPackage { + pub name: String, + pub manifest_path: String, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CargoPublishPolicySnapshot { + pub base_marked: BTreeMap, + pub head_marked: BTreeMap, + pub newly_marked: BTreeSet, +} + +fn manifest_is_marked(value: &toml::Value, path: &str) -> anyhow::Result { + let publish = value + .get("package") + .and_then(|package| package.get("metadata")) + .and_then(|metadata| metadata.get("fslabs")) + .and_then(|fslabs| fslabs.get("publish")) + .and_then(|publish| publish.get("cargo")) + .and_then(|cargo| cargo.get("publish")); + match publish { + Some(value) => value.as_bool().with_context(|| { + format!("package.metadata.fslabs.publish.cargo.publish in {path} must be a boolean") + }), + None => Ok(false), + } +} + +pub fn marked_cargo_packages_at( + repo: &gix::Repository, + commit_id: gix::ObjectId, +) -> anyhow::Result> { + let tree = repo.find_commit(commit_id)?.tree()?; + let mut marked = BTreeMap::new(); + + for entry in tree.traverse().breadthfirst.files()? { + let path = entry + .filepath + .to_str() + .context("Cargo manifest path is not UTF-8")?; + if Path::new(path).file_name().and_then(|name| name.to_str()) != Some("Cargo.toml") { + continue; + } + if !entry.mode.is_blob() { + anyhow::bail!("Cargo manifest {path} is not a regular file"); + } + + let blob = repo.find_blob(entry.oid)?; + let manifest = std::str::from_utf8(&blob.data) + .with_context(|| format!("Cargo manifest {path} is not UTF-8"))?; + let value: toml::Value = + toml::from_str(manifest).with_context(|| format!("Could not parse {path}"))?; + if !manifest_is_marked(&value, path)? { + continue; + } + + let name = value + .get("package") + .and_then(|package| package.get("name")) + .and_then(toml::Value::as_str) + .with_context(|| format!("Marked Cargo manifest {path} has no package.name"))? + .to_string(); + let package = MarkedCargoPackage { + name: name.clone(), + manifest_path: path.to_string(), + }; + if let Some(previous) = marked.insert(name.clone(), package) { + anyhow::bail!( + "Marked Cargo package {name} is defined by both {} and {path}", + previous.manifest_path + ); + } + } + + Ok(marked) +} + +pub fn inspect_cargo_publish_policy( + repo: &gix::Repository, + base_commit: gix::ObjectId, + head_commit: gix::ObjectId, +) -> anyhow::Result { + let base_marked = marked_cargo_packages_at(repo, base_commit)?; + let head_marked = marked_cargo_packages_at(repo, head_commit)?; + let newly_marked = head_marked + .keys() + .filter(|name| !base_marked.contains_key(*name)) + .cloned() + .collect(); + + Ok(CargoPublishPolicySnapshot { + base_marked, + head_marked, + newly_marked, + }) +} + +pub async fn missing_marked_package_names( + cargo: &C, + registry: &str, + marked: &BTreeMap, +) -> anyhow::Result> { + let mut missing = BTreeSet::new(); + for name in marked.keys() { + let exists = cargo + .check_crate_name_exists(registry.to_string(), name.clone()) + .await + .with_context(|| { + format!("Could not check whether Cargo package {name} exists in {registry}") + })?; + if !exists { + missing.insert(name.clone()); + } + } + Ok(missing) +} + +#[cfg(test)] +mod tests { + use std::fs; + use std::path::Path; + use std::process::Command; + + use tempfile::TempDir; + + use super::*; + use crate::utils::cargo::tests::MockCargo; + + fn git(repo: &Path, args: &[&str]) -> String { + let output = Command::new("git") + .args(args) + .current_dir(repo) + .env("GIT_AUTHOR_NAME", "Test User") + .env("GIT_AUTHOR_EMAIL", "test@example.com") + .env("GIT_COMMITTER_NAME", "Test User") + .env("GIT_COMMITTER_EMAIL", "test@example.com") + .output() + .unwrap(); + assert!( + output.status.success(), + "git {:?} failed: {}", + args, + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).trim().to_string() + } + + fn write_manifest(repo: &Path, relative_path: &str, name: &str, publish: Option) { + let path = repo.join(relative_path); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + let publish = publish + .map(|publish| { + format!("\n[package.metadata.fslabs.publish.cargo]\npublish = {publish}\n") + }) + .unwrap_or_default(); + fs::write( + path, + format!("[package]\nname = \"{name}\"\nversion = \"1.0.0\"\n{publish}"), + ) + .unwrap(); + } + + fn commit(repo: &Path, message: &str) -> gix::ObjectId { + git(repo, &["add", "."]); + git( + repo, + &["-c", "commit.gpgsign=false", "commit", "-m", message], + ); + git(repo, &["rev-parse", "HEAD"]).parse().unwrap() + } + + #[test] + fn detects_new_and_newly_enabled_marked_packages() { + let temp = TempDir::new().unwrap(); + git(temp.path(), &["init"]); + write_manifest(temp.path(), "existing/Cargo.toml", "existing", Some(true)); + write_manifest(temp.path(), "disabled/Cargo.toml", "disabled", Some(false)); + let base = commit(temp.path(), "base"); + + write_manifest(temp.path(), "disabled/Cargo.toml", "disabled", Some(true)); + write_manifest(temp.path(), "new/Cargo.toml", "new", Some(true)); + write_manifest(temp.path(), "ordinary/Cargo.toml", "ordinary", None); + let head = commit(temp.path(), "head"); + + let repo = gix::open(temp.path()).unwrap(); + let snapshot = inspect_cargo_publish_policy(&repo, base, head).unwrap(); + + assert_eq!( + snapshot.newly_marked, + BTreeSet::from(["disabled".to_string(), "new".to_string()]) + ); + assert!(snapshot.base_marked.contains_key("existing")); + assert!(snapshot.head_marked.contains_key("existing")); + } + + #[test] + fn already_marked_package_changes_do_not_require_new_approval() { + let temp = TempDir::new().unwrap(); + git(temp.path(), &["init"]); + write_manifest(temp.path(), "crate/Cargo.toml", "crate", Some(true)); + let base = commit(temp.path(), "base"); + + fs::write(temp.path().join("crate/src.rs"), "pub fn changed() {}\n").unwrap(); + let head = commit(temp.path(), "head"); + + let repo = gix::open(temp.path()).unwrap(); + let snapshot = inspect_cargo_publish_policy(&repo, base, head).unwrap(); + + assert!(snapshot.newly_marked.is_empty()); + } + + #[test] + fn moved_marked_package_keeps_its_existing_identity() { + let temp = TempDir::new().unwrap(); + git(temp.path(), &["init"]); + write_manifest(temp.path(), "old/Cargo.toml", "crate", Some(true)); + let base = commit(temp.path(), "base"); + + fs::create_dir_all(temp.path().join("new")).unwrap(); + fs::rename( + temp.path().join("old/Cargo.toml"), + temp.path().join("new/Cargo.toml"), + ) + .unwrap(); + let head = commit(temp.path(), "head"); + + let repo = gix::open(temp.path()).unwrap(); + let snapshot = inspect_cargo_publish_policy(&repo, base, head).unwrap(); + + assert!(snapshot.newly_marked.is_empty()); + } + + #[test] + fn non_boolean_publish_setting_fails_closed() { + let temp = TempDir::new().unwrap(); + git(temp.path(), &["init"]); + let path = temp.path().join("crate/Cargo.toml"); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write( + &path, + "[package]\nname = \"crate\"\nversion = \"1.0.0\"\n\n[package.metadata.fslabs.publish.cargo]\npublish = \"yes\"\n", + ) + .unwrap(); + let commit = commit(temp.path(), "invalid metadata"); + + let repo = gix::open(temp.path()).unwrap(); + let error = marked_cargo_packages_at(&repo, commit).unwrap_err(); + + assert!( + error + .to_string() + .contains("package.metadata.fslabs.publish.cargo.publish") + ); + } + + #[tokio::test] + async fn reports_missing_marked_package_names() { + let marked = BTreeMap::from([ + ( + "missing".to_string(), + MarkedCargoPackage { + name: "missing".to_string(), + manifest_path: "missing/Cargo.toml".to_string(), + }, + ), + ( + "present".to_string(), + MarkedCargoPackage { + name: "present".to_string(), + manifest_path: "present/Cargo.toml".to_string(), + }, + ), + ]); + let mut cargo = MockCargo::new(); + cargo + .expect_check_crate_name_exists() + .times(2) + .withf(|registry, _| registry == "fsl") + .returning(|_, name| Ok(name == "present")); + + let missing = missing_marked_package_names(&cargo, "fsl", &marked) + .await + .unwrap(); + + assert_eq!(missing, BTreeSet::from(["missing".to_string()])); + } +} diff --git a/src/commands/check_cargo_publish_policy/mod.rs b/src/commands/check_cargo_publish_policy/mod.rs new file mode 100644 index 000000000..6fdcf0de9 --- /dev/null +++ b/src/commands/check_cargo_publish_policy/mod.rs @@ -0,0 +1,264 @@ +use std::collections::{BTreeMap, BTreeSet, HashSet}; +use std::fmt::{Display, Formatter}; +use std::path::PathBuf; + +use anyhow::Context; +use clap::{Parser, ValueEnum}; +use serde::Serialize; + +use crate::PrettyPrintable; +use crate::cargo_publish_policy::{ + MarkedCargoPackage, inspect_cargo_publish_policy, missing_marked_package_names, +}; +use crate::cli_args::DiffOptions; +use crate::utils::cargo::Cargo; + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, ValueEnum)] +enum PolicyCheck { + #[default] + All, + Approval, + Registry, +} + +impl PolicyCheck { + fn approval(self) -> bool { + matches!(self, Self::All | Self::Approval) + } + + fn registry(self) -> bool { + matches!(self, Self::All | Self::Registry) + } +} + +#[derive(Debug, Parser, Default)] +#[command(about = "Enforce Cargo publication approval and registry state")] +pub struct Options { + #[clap(flatten)] + diff: DiffOptions, + /// Label which approves newly metadata-marked Cargo packages. + #[arg(long, default_value = "add-crate")] + approval_label: String, + /// Current pull request labels. + #[arg(long, env = "PULL_REQUEST_LABELS", value_delimiter = ',')] + pull_request_labels: Vec, + /// Registry where every package marked on the pull request base must exist. + #[arg(long, env, default_value = "fsl")] + cargo_target_registry: String, + /// Run the approval check, registry check, or both. + #[arg(long, value_enum, default_value_t)] + check: PolicyCheck, +} + +#[derive(Debug, Serialize)] +pub struct Result { + pub approval_checked: bool, + pub registry_checked: bool, + pub approval_label: String, + pub approval_label_present: bool, + pub newly_marked: Vec, + pub base_packages_missing_from_registry: Vec, +} + +impl Display for Result { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let mut messages = Vec::new(); + if self.approval_checked { + if self.newly_marked.is_empty() { + messages.push("No newly marked Cargo packages".to_string()); + } else { + messages.push(format!( + "Newly marked Cargo packages: {}", + self.newly_marked.join(", ") + )); + } + } + if self.registry_checked { + if self.base_packages_missing_from_registry.is_empty() { + messages.push("All base-marked Cargo packages exist in the registry".to_string()); + } else { + messages.push(format!( + "Base-marked Cargo packages missing from the registry: {}", + self.base_packages_missing_from_registry.join(", ") + )); + } + } + write!(f, "{}", messages.join("\n")) + } +} + +impl PrettyPrintable for Result { + fn pretty_print(&self) -> String { + self.to_string() + } +} + +fn validate_policy(result: &Result, check: PolicyCheck) -> anyhow::Result<()> { + if check.registry() && !result.base_packages_missing_from_registry.is_empty() { + anyhow::bail!( + "Cargo packages marked for publication on the pull request base are missing from the registry: {}. Publish them before merging other pull requests.", + result.base_packages_missing_from_registry.join(", ") + ); + } + if check.approval() && !result.newly_marked.is_empty() && !result.approval_label_present { + anyhow::bail!( + "The {} label is required for newly marked Cargo packages: {}", + result.approval_label, + result.newly_marked.join(", ") + ); + } + Ok(()) +} + +fn persistent_base_marked( + base_marked: &BTreeMap, + resulting_marked: &BTreeMap, +) -> BTreeMap { + base_marked + .iter() + .filter(|(name, _)| resulting_marked.contains_key(*name)) + .map(|(name, package)| (name.clone(), package.clone())) + .collect() +} + +pub async fn check_cargo_publish_policy( + options: Box, + repo_root: PathBuf, +) -> anyhow::Result { + if options.diff.base_sha.is_some() != options.diff.head_sha.is_some() { + anyhow::bail!("PULL_BASE_SHA and PULL_PULL_SHA must be provided together"); + } + let repo = gix::open(&repo_root) + .with_context(|| format!("Failed to open git repository at {}", repo_root.display()))?; + let (base_commit, head_commit) = options.diff.strategy().git_commits(&repo)?; + let snapshot = inspect_cargo_publish_policy(&repo, base_commit, head_commit)?; + + let persistent_base_marked = + persistent_base_marked(&snapshot.base_marked, &snapshot.head_marked); + + let missing = if options.check.registry() { + let registries = HashSet::from([options.cargo_target_registry.clone()]); + let cargo = Cargo::new(®istries, true)?; + missing_marked_package_names( + &cargo, + &options.cargo_target_registry, + &persistent_base_marked, + ) + .await? + } else { + BTreeSet::new() + }; + let labels = options + .pull_request_labels + .iter() + .map(|label| label.trim()) + .filter(|label| !label.is_empty()) + .collect::>(); + let result = Result { + approval_checked: options.check.approval(), + registry_checked: options.check.registry(), + approval_label: options.approval_label.clone(), + approval_label_present: labels.contains(options.approval_label.as_str()), + newly_marked: snapshot.newly_marked.into_iter().collect(), + base_packages_missing_from_registry: missing.into_iter().collect(), + }; + validate_policy(&result, options.check)?; + Ok(result) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn result(newly_marked: &[&str], missing: &[&str], approved: bool) -> Result { + Result { + approval_checked: true, + registry_checked: true, + approval_label: "add-crate".to_string(), + approval_label_present: approved, + newly_marked: newly_marked.iter().map(|name| name.to_string()).collect(), + base_packages_missing_from_registry: missing + .iter() + .map(|name| name.to_string()) + .collect(), + } + } + + #[test] + fn requires_label_for_newly_marked_packages() { + let error = + validate_policy(&result(&["new_crate"], &[], false), PolicyCheck::All).unwrap_err(); + assert_eq!( + error.to_string(), + "The add-crate label is required for newly marked Cargo packages: new_crate" + ); + } + + #[test] + fn accepts_label_for_newly_marked_packages() { + validate_policy(&result(&["new_crate"], &[], true), PolicyCheck::All).unwrap(); + } + + #[test] + fn missing_base_package_fails_even_with_label() { + let error = + validate_policy(&result(&[], &["unpublished"], true), PolicyCheck::All).unwrap_err(); + assert_eq!( + error.to_string(), + "Cargo packages marked for publication on the pull request base are missing from the registry: unpublished. Publish them before merging other pull requests." + ); + } + + #[test] + fn removing_or_unmarking_missing_base_package_allows_recovery() { + let package = MarkedCargoPackage { + name: "unpublished".to_string(), + manifest_path: "unpublished/Cargo.toml".to_string(), + }; + let base = BTreeMap::from([("unpublished".to_string(), package)]); + let head = BTreeMap::new(); + + assert!(persistent_base_marked(&base, &head).is_empty()); + } + + #[test] + fn approval_only_does_not_require_registry_state() { + validate_policy( + &result(&["new_crate"], &["unpublished"], true), + PolicyCheck::Approval, + ) + .unwrap(); + } + + #[test] + fn approval_only_output_does_not_claim_registry_was_checked() { + let mut result = result(&[], &[], true); + result.registry_checked = false; + assert_eq!(result.to_string(), "No newly marked Cargo packages"); + } + + #[test] + fn registry_only_does_not_require_approval_label() { + validate_policy(&result(&["new_crate"], &[], false), PolicyCheck::Registry).unwrap(); + } + + #[tokio::test] + async fn explicit_base_and_head_must_be_provided_together() { + let options = Options { + diff: DiffOptions { + base_sha: Some("base".to_string()), + ..Default::default() + }, + ..Default::default() + }; + + let error = check_cargo_publish_policy(Box::new(options), PathBuf::new()) + .await + .unwrap_err(); + + assert_eq!( + error.to_string(), + "PULL_BASE_SHA and PULL_PULL_SHA must be provided together" + ); + } +} diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 7ac57c895..4acd4b624 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1,4 +1,5 @@ pub mod annotate; +pub mod check_cargo_publish_policy; pub mod check_workspace; pub mod docker_build_push; pub mod download_artifacts; diff --git a/src/main.rs b/src/main.rs index b51c4cfd9..824733ea4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,6 +10,9 @@ use clap_mangen::Man; use utils::cargo::Cargo; use crate::commands::annotate::{Options as AnnotateOptions, annotate}; +use crate::commands::check_cargo_publish_policy::{ + Options as CheckCargoPublishPolicyOptions, check_cargo_publish_policy, +}; use crate::commands::check_workspace::{Options as CheckWorkspaceOptions, check_workspace}; use crate::commands::docker_build_push::{Options as DockerBuildPushOptions, docker_build_push}; use crate::commands::download_artifacts::{ @@ -43,6 +46,7 @@ use tracing_subscriber::fmt::FormatFields; use tracing_subscriber::fmt::format::{DefaultFields, Writer}; use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; +mod cargo_publish_policy; mod cli_args; mod commands; mod crate_graph; @@ -127,6 +131,8 @@ enum Commands { DraftRelease(Box), /// Post build findings (logs, JUnit XML) as GitHub check-run annotations Annotate(Box), + /// Enforce Cargo publication approval and registry state + CheckCargoPublishPolicy(Box), // Packages Related Commands // @@ -509,6 +515,11 @@ async fn run() -> anyhow::Result { Commands::Annotate(options) => annotate(options, repo_root) .await .map(|r| display_results(cli.json, cli.pretty_print, r)), + Commands::CheckCargoPublishPolicy(options) => { + check_cargo_publish_policy(options, repo_root) + .await + .map(|r| display_results(cli.json, cli.pretty_print, r)) + } Commands::FixLockFiles { common_options, options, diff --git a/src/utils/cargo.rs b/src/utils/cargo.rs index d023091fa..602e1f148 100644 --- a/src/utils/cargo.rs +++ b/src/utils/cargo.rs @@ -389,6 +389,12 @@ pub trait CrateChecker { version: String, ) -> anyhow::Result; + async fn check_crate_name_exists( + &self, + registry_name: String, + name: String, + ) -> anyhow::Result; + fn add_registry(&mut self, registry_name: String, fetch_indexes: bool) -> anyhow::Result<()>; } @@ -418,6 +424,100 @@ impl Cargo { pub fn http_client(&self) -> Option<&HyperClient, Empty>> { self.client.as_ref() } + + async fn sparse_index_body( + &self, + registry_name: &str, + name: &str, + ) -> anyhow::Result> { + let registry = self + .registries + .get(registry_name) + .ok_or_else(|| anyhow::anyhow!("unknown registry {registry_name}"))?; + let index = registry + .index + .as_ref() + .context("Cannot check crate existence without index")?; + let base_url = index + .strip_prefix("sparse+") + .context("Crate existence checks require a sparse registry index")?; + let base_url = if base_url.ends_with('/') { + base_url.to_string() + } else { + format!("{base_url}/") + }; + let package_dir = get_package_file_dir(name)?; + let url: Uri = format!("{base_url}{package_dir}/{name}").parse()?; + let client = self + .client + .as_ref() + .context("HTTP client required for sparse registry")?; + + let mut last_error = None; + for attempt in 1..=3u32 { + let mut req_builder = Request::builder().method(Method::GET).uri(url.clone()); + if let Some(token) = ®istry.token { + req_builder = req_builder.header("Authorization", token); + } + if let Some(user_agent) = ®istry.user_agent { + req_builder = req_builder.header("User-Agent", user_agent); + } + let req = req_builder.body(Empty::default())?; + + match client.request(req).await { + Ok(res) => { + if res.status().as_u16() == 404 { + return Ok(None); + } + if res.status().as_u16() == 429 || res.status().is_server_error() { + last_error = Some(format!("HTTP {}", res.status())); + if attempt < 3 { + tracing::warn!( + crate_name = %name, + attempt, + status = %res.status(), + "sparse registry request failed, retrying" + ); + tokio::time::sleep(std::time::Duration::from_secs(attempt as u64)) + .await; + continue; + } + break; + } + if res.status().as_u16() >= 400 { + anyhow::bail!( + "Failed to fetch crate index for {name}: HTTP {}", + res.status() + ); + } + let body = res + .into_body() + .collect() + .await + .context("Could not get body from sparse registry")? + .to_bytes(); + return Ok(Some(String::from_utf8_lossy(&body).to_string())); + } + Err(error) => { + last_error = Some(error.to_string()); + if attempt < 3 { + tracing::warn!( + crate_name = %name, + attempt, + error = %last_error.as_deref().unwrap_or_default(), + "sparse registry request failed, retrying" + ); + tokio::time::sleep(std::time::Duration::from_secs(attempt as u64)).await; + } + } + } + } + + Err(anyhow::anyhow!( + "Could not fetch from sparse registry after 3 attempts: {url}" + )) + .with_context(|| format!("last error: {}", last_error.unwrap_or_default())) + } } pub fn get_package_file_dir(package_name: &str) -> anyhow::Result { @@ -443,6 +543,22 @@ pub struct IndexPackageVersion { pub checksum: Option, } +fn index_has_package_name(body: &str, expected_name: &str) -> anyhow::Result { + let mut found = false; + for line in body.lines() { + let package: IndexPackageVersion = serde_json::from_str(line) + .with_context(|| format!("Failed to parse JSON line for {expected_name}"))?; + if package.name != expected_name { + anyhow::bail!( + "Registry index for {expected_name} returned package {}", + package.name + ); + } + found = true; + } + Ok(found) +} + impl CrateChecker for Cargo { async fn check_crate_exists( &self, @@ -456,108 +572,57 @@ impl CrateChecker for Cargo { .ok_or_else(|| anyhow::anyhow!("unknown registry"))?; if registry.is_sparse() { - let index = registry - .index - .as_ref() - .context("Cannot check crate existence without index")?; - let base_url = index - .strip_prefix("sparse+") - .context("Invalid sparse index URL")?; - - // Catches URL construction footgun that leads to strange errors. - let base_url = if base_url.ends_with('/') { - base_url - } else { - &format!("{}/", base_url) + let Some(body) = self.sparse_index_body(®istry_name, &name).await? else { + return Ok(false); }; - - let package_dir = get_package_file_dir(&name)?; - let url: Uri = format!("{}{}/{}", base_url, package_dir, name).parse()?; - - let client = self - .client - .as_ref() - .context("HTTP client required for sparse registry")?; - - let mut last_err = None; - for attempt in 1..=3u32 { - // Rebuild the request on each attempt — hyper consumes it on send. - let mut req_builder = Request::builder().method(Method::GET).uri(url.clone()); - if let Some(token) = ®istry.token { - req_builder = req_builder.header("Authorization", token); - } - if let Some(user_agent) = ®istry.user_agent { - req_builder = req_builder.header("User-Agent", user_agent); - } - let req = req_builder.body(Empty::default())?; - - match client.request(req).await { - Ok(res) => { - if res.status().as_u16() == 404 { - return Ok(false); - } - - if res.status().as_u16() >= 400 { - anyhow::bail!( - "Failed to fetch crate index for {}: HTTP {}", - name, - res.status() - ); - } - - let body = res - .into_body() - .collect() - .await - .context("Could not get body from sparse registry")? - .to_bytes(); - - let body_str = String::from_utf8_lossy(&body); - - for line in body_str.lines() { - let pkg_version: IndexPackageVersion = serde_json::from_str(line) - .with_context(|| { - format!("Failed to parse JSON line for {}", name) - })?; - - if pkg_version.version == version && !pkg_version.yanked { - return Ok(true); - } - } - - return Ok(false); - } - Err(e) => { - last_err = Some(e); - if attempt < 3 { - tracing::warn!( - crate_name = %name, - attempt = attempt, - error = %last_err.as_ref().unwrap(), - "sparse registry request failed, retrying" - ); - tokio::time::sleep(std::time::Duration::from_secs(attempt as u64)) - .await; - } - } + for line in body.lines() { + let package: IndexPackageVersion = serde_json::from_str(line) + .with_context(|| format!("Failed to parse JSON line for {name}"))?; + if package.version == version && !package.yanked { + return Ok(true); } } - - return Err(anyhow::anyhow!( - "Could not fetch from sparse registry after 3 attempts: {}", - url - )) - .with_context(|| { - format!( - "last error: {}", - last_err.map(|e| e.to_string()).unwrap_or_default() - ) - }); + return Ok(false); } Ok(false) } + async fn check_crate_name_exists( + &self, + registry_name: String, + name: String, + ) -> anyhow::Result { + let registry = self + .registries + .get(®istry_name) + .ok_or_else(|| anyhow::anyhow!("unknown registry {registry_name}"))?; + if registry.is_sparse() { + let Some(body) = self.sparse_index_body(®istry_name, &name).await? else { + return Ok(false); + }; + return index_has_package_name(&body, &name); + } + + let local_index_path = registry + .local_index_path + .as_ref() + .context("Cannot check crate existence in an unfetched registry")?; + let package_path = local_index_path + .join(get_package_file_dir(&name)?) + .join(&name); + let file = match File::open(package_path) { + Ok(file) => file, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(error) => return Err(error.into()), + }; + let body = BufReader::new(file) + .lines() + .collect::, _>>()? + .join("\n"); + index_has_package_name(&body, &name) + } + fn add_registry(&mut self, registry_name: String, fetch_indexes: bool) -> anyhow::Result<()> { let registry = CargoRegistry::new( registry_name.clone(), @@ -1017,6 +1082,12 @@ pub(crate) mod tests { _version: String, ) -> anyhow::Result; + async fn check_crate_name_exists( + &self, + _registry_name: String, + _name: String, + ) -> anyhow::Result; + fn add_registry( &mut self, _registry_name: String, @@ -1027,6 +1098,28 @@ pub(crate) mod tests { use super::*; use std::fs; + + #[test] + fn crate_name_exists_when_all_versions_are_yanked() { + let body = r#"{"name":"example","vers":"1.0.0","yanked":true,"cksum":"abc"}"#; + assert!(index_has_package_name(body, "example").unwrap()); + } + + #[test] + fn empty_index_means_crate_name_is_missing() { + assert!(!index_has_package_name("", "example").unwrap()); + } + + #[test] + fn mismatched_index_package_name_fails_closed() { + let body = r#"{"name":"other","vers":"1.0.0","yanked":false,"cksum":"abc"}"#; + let error = index_has_package_name(body, "example").unwrap_err(); + assert_eq!( + error.to_string(), + "Registry index for example returned package other" + ); + } + #[test] fn test_publish_key_replaced_if_present() { let original_registry = CargoRegistry::new(