fix(publish): reject invalid JSR package names

@@ -5,6 +5,7 @@ use deno_core::error::AnyError; use deno_core::serde_json; use deno_core::url::Url; use deno_runtime::deno_fetch;+use deno_semver::jsr::JsrPackageReqReference; use serde::de::DeserializeOwned;  use crate::http_util;@@ -134,28 +135,67 @@ pub fn get_package_api_url(   registry_api_url: &Url,   scope: &str,   package: &str,-) -> String {-  format!("{}scopes/{}/packages/{}", registry_api_url, scope, package)+) -> Result<Url, AnyError> {+  append_path_segments(+    registry_api_url,+    &["scopes", scope, "packages", package],+  ) }  pub fn get_package_version_api_url(   registry_api_url: &Url,   scope: &str,   package: &str,   version: &str,-  params: Option<&str>,-) -> String {-  if let Some(params) = params {-    format!(-      "{}scopes/{}/packages/{}/versions/{}?{}",-      registry_api_url, scope, package, version, params-    )-  } else {-    format!(-      "{}scopes/{}/packages/{}/versions/{}",-      registry_api_url, scope, package, version-    )+  config_path: Option<&str>,+) -> Result<Url, AnyError> {+  let mut url = append_path_segments(+    registry_api_url,+    &["scopes", scope, "packages", package, "versions", version],+  )?;+  if let Some(config_path) = config_path {+    url.query_pairs_mut().append_pair("config", config_path);   }+  Ok(url)+}++pub fn get_package_version_provenance_api_url(+  registry_api_url: &Url,+  scope: &str,+  package: &str,+  version: &str,+) -> Result<Url, AnyError> {+  append_path_segments(+    registry_api_url,+    &[+      "scopes",+      scope,+      "packages",+      package,+      "versions",+      version,+      "provenance",+    ],+  )+}++fn append_path_segments(+  base_url: &Url,+  segments: &[&str],+) -> Result<Url, AnyError> {+  let mut url = base_url.clone();+  url.set_query(None);+  url.set_fragment(None);+  url+    .path_segments_mut()+    .map_err(|_| {+      deno_core::anyhow::anyhow!(+        "Registry API URL cannot be used as a base URL"+      )+    })?+    .pop_if_empty()+    .extend(segments);+  Ok(url) }  pub async fn get_package(@@ -164,20 +204,41 @@ pub async fn get_package(   scope: &str,   package: &str, ) -> Result<http::Response<deno_fetch::ResBody>, AnyError> {-  let package_url = get_package_api_url(registry_api_url, scope, package);-  let response = client.get(package_url.parse()?)?.send().await?;+  let package_url = get_package_api_url(registry_api_url, scope, package)?;+  let response = client.get(package_url)?.send().await?;   Ok(response) }  /// Splits a fully qualified JSR package name (e.g. `@scope/package`) into its /// `(scope, package)` parts. pub fn parse_package_name(name: &str) -> Result<(&str, &str), AnyError> {-  let Some((scope, package)) = name-    .strip_prefix('@')-    .and_then(|no_at| no_at.split_once('/'))+  // Keep the explicit path-safety checks below even if the JSR grammar changes.+  let reference = JsrPackageReqReference::from_str(&format!("jsr:{name}@*"))+    .map_err(|_| {+      deno_core::anyhow::anyhow!(+        "package name must use the '@<scope>/<package>' format"+      )+    })?;+  if reference.sub_path().is_some() {+    bail!("package name must not contain additional path segments");+  }++  let Some((scope, package)) =+    name.strip_prefix('@').and_then(|name| name.split_once('/'))   else {-    bail!("Invalid package name, use '@<scope_name>/<package_name>' format");+    bail!("package name must use the '@<scope>/<package>' format");   };+  for component in [scope, package] {+    if component == "." || component == ".." {+      bail!("package name must not contain dot path segments");+    }+    if component+      .chars()+      .any(|c| matches!(c, '/' | '\\' | '?' | '#' | '%'))+    {+      bail!("package name contains a URL path or delimiter character");+    }+  }   Ok((scope, package)) } @@ -201,8 +262,8 @@ pub async fn check_version_exists(     package,     version,     None,-  );-  let response = client.get(url.parse()?)?.send().await?;+  )?;+  let response = client.get(url)?.send().await?;   Ok(response.status() == 200) } @@ -269,8 +330,84 @@ mod test {   #[test]   fn test_parse_package_name() {     assert_eq!(parse_package_name("@deno/doc").unwrap(), ("deno", "doc"));-    assert!(parse_package_name("deno/doc").is_err());-    assert!(parse_package_name("@deno").is_err());+    assert_eq!(+      parse_package_name("@scope-1/package-2").unwrap(),+      ("scope-1", "package-2")+    );+    assert_eq!(parse_package_name("@a/b").unwrap(), ("a", "b"));++    for invalid_name in [+      "deno/doc",+      "@deno",+      "@deno/../doc",+      "@deno/doc/other",+      "@deno/doc?other",+      "@deno/doc#other",+      "@deno/doc\\other",+      "@deno/doc%2Fother",+    ] {+      assert!(+        parse_package_name(invalid_name).is_err(),+        "{invalid_name} should be invalid"+      );+    }+  }++  #[test]+  fn package_api_urls_use_path_segments() {+    let base = Url::parse("https://registry.example/custom/api/").unwrap();+    let package_url = get_package_api_url(&base, "scope", "package").unwrap();+    assert_eq!(+      package_url.as_str(),+      "https://registry.example/custom/api/scopes/scope/packages/package"+    );++    let version_url = get_package_version_api_url(+      &base,+      "scope",+      "package",+      "1.2.3+build.1",+      Some("/deno.json"),+    )+    .unwrap();+    assert_eq!(+      version_url.path(),+      "/custom/api/scopes/scope/packages/package/versions/1.2.3+build.1"+    );+    assert_eq!(+      version_url.query_pairs().collect::<Vec<_>>(),+      vec![("config".into(), "/deno.json".into())]+    );++    let provenance_url = get_package_version_provenance_api_url(+      &base, "scope", "package", "1.2.3",+    )+    .unwrap();+    assert_eq!(+      provenance_url.as_str(),+      "https://registry.example/custom/api/scopes/scope/packages/package/versions/1.2.3/provenance"+    );+  }++  #[test]+  fn package_api_url_components_cannot_change_the_endpoint() {+    let base = Url::parse("https://registry.example/custom/api/").unwrap();+    let url = get_package_version_api_url(+      &base,+      "scope/../../other",+      "package?mode=other#fragment",+      "1.0.0/../../other",+      None,+    )+    .unwrap();+    assert_eq!(+      url.origin().ascii_serialization(),+      "https://registry.example"+    );+    assert_eq!(+      url.as_str(),+      "https://registry.example/custom/api/scopes/scope%2F..%2F..%2Fother/packages/package%3Fmode=other%23fragment/versions/1.0.0%2F..%2F..%2Fother"+    );   }    #[test]

@@ -28,6 +28,7 @@ use deno_core::serde_json::json; use deno_core::url::Url; use deno_resolver::collections::FolderScopedMap; use deno_runtime::deno_fetch;+use deno_semver::Version; use deno_terminal::colors; use http_body_util::BodyExt; use serde::Deserialize;@@ -80,9 +81,6 @@ pub async fn publish( ) -> Result<(), AnyError> {   let cli_factory = CliFactory::from_flags(flags); -  let auth_method =-    get_auth_method(publish_flags.token, publish_flags.dry_run)?;-   let cli_options = cli_factory.cli_options()?;   let directory_path = cli_options.initial_cwd();   let mut publish_configs = cli_options.start_dir.jsr_packages_for_publish();@@ -123,6 +121,11 @@ pub async fn publish(     }   } +  validate_publish_configs(&publish_configs)?;++  let auth_method =+    get_auth_method(publish_flags.token, publish_flags.dry_run)?;+   // Bail out early if the version is already published, before doing the   // expensive type checking and tarball preparation. Already-published   // versions are skipped with a warning (rather than erroring) so that@@ -226,6 +229,34 @@ pub async fn publish(   Ok(()) } +fn validate_publish_configs(+  publish_configs: &[JsrPackageConfig],+) -> Result<(), AnyError> {+  for config in publish_configs {+    registry::parse_package_name(&config.name).with_context(|| {+      format!(+        "Invalid package name '{}' in '{}'",+        config.name, config.config_file.specifier+      )+    })?;++    let version =+      config.config_file.json.version.as_deref().ok_or_else(|| {+        deno_core::anyhow::anyhow!(+          "{} is missing 'version' field",+          config.config_file.specifier+        )+      })?;+    Version::parse_standard(version).with_context(|| {+      format!(+        "Invalid package version '{}' in '{}'",+        version, config.config_file.specifier+      )+    })?;+  }+  Ok(())+}+ /// Queries the registry for each package's version concurrently and returns the /// subset of configs whose versions are not yet published. Already-published /// versions are reported with a warning and dropped from the returned list.@@ -865,11 +896,11 @@ async fn ensure_scopes_and_packages_exist(       registry_api_url,       &create_package_info.scope,       &create_package_info.package,-    );+    )?;      loop {       tokio::time::sleep(std::time::Duration::from_secs(3)).await;-      let response = client.get(package_api_url.parse()?)?.send().await?;+      let response = client.get(package_api_url.clone())?.send().await?;       if response.status() == 200 {         let name = format!(           "@{}/{}",@@ -1043,17 +1074,18 @@ async fn publish_package(     package.version   ); +  let config_path = format!("/{}", package.config);   let url = registry::get_package_version_api_url(     registry_api_url,     &package.scope,     &package.package,     &package.version,-    Some(&format!("config=/{}", package.config)),-  );+    Some(&config_path),+  )?;    let body = deno_fetch::ReqBody::full(package.tarball.bytes.clone());   let response = http_client-    .post(url.parse()?, body)?+    .post(url, body)?     .header(       http::header::AUTHORIZATION,       authorization.parse().map_err(http::Error::from)?,@@ -1179,10 +1211,12 @@ async fn publish_package(       format!("https://search.sigstore.dev/?logIndex={log_index}");      // Submit bundle to JSR-    let provenance_url = format!(-      "{}scopes/{}/packages/{}/versions/{}/provenance",-      registry_api_url, package.scope, package.package, package.version-    );+    let provenance_url = registry::get_package_version_provenance_api_url(+      registry_api_url,+      &package.scope,+      &package.package,+      &package.version,+    )?;     match submit_provenance_bundle(       http_client,       &provenance_url,@@ -1335,12 +1369,12 @@ struct VersionManifest { /// provenance badge. That is how jsr-io/jsr#1474 went unnoticed for a month. async fn submit_provenance_bundle(   http_client: &HttpClient,-  provenance_url: &str,+  provenance_url: &Url,   authorization: &str,   bundle: &provenance::ProvenanceBundle, ) -> Result<(), AnyError> {   let response = http_client-    .post_json(provenance_url.parse()?, &json!({ "bundle": bundle }))?+    .post_json(provenance_url.clone(), &json!({ "bundle": bundle }))?     .header(http::header::AUTHORIZATION, authorization.parse()?)     .send()     .await?;

@@ -77,6 +77,43 @@ fn publish_warning_not_in_graph() {     ); } +#[test]+fn publish_rejects_invalid_identities_before_authentication() {+  let context = TestContextBuilder::new().use_temp_cwd().build();+  let temp_dir = context.temp_dir().path();+  temp_dir.join("LICENSE").write("");+  temp_dir.join("mod.ts").write("export const value = 1;");++  for name in [+    "@foo/../../other",+    "@foo/package/other",+    "@foo/package?other",+    "@foo/package#other",+    "@foo/package\\other",+  ] {+    temp_dir.join("deno.json").write_json(&json!({+      "name": name,+      "version": "1.0.0",+      "exports": "./mod.ts",+    }));++    let output = context.new_command().arg("publish").run();+    output.assert_exit_code(1);+    assert_contains!(output.combined_output(), "Invalid package name");+    assert_not_contains!(output.combined_output(), "No means to authenticate");+  }++  temp_dir.join("deno.json").write_json(&json!({+    "name": "@foo/package",+    "version": "1.0.0?other",+    "exports": "./mod.ts",+  }));+  let output = context.new_command().arg("publish").run();+  output.assert_exit_code(1);+  assert_contains!(output.combined_output(), "Invalid package version");+  assert_not_contains!(output.combined_output(), "No means to authenticate");+}+ #[test] fn provenance() {   TestContextBuilder::new()

@@ -1,7 +1,4 @@-Check [WILDCARD]mod.ts-Checking for slow types in the public API...-Check [WILDCARD]mod.ts-error: Failed preparing 'project'.+error: Invalid package name 'project' in 'file:///[WILDCARD]/project/deno.json'  Caused by:-    Invalid package name, use '@<scope_name>/<package_name>' format+    package name must use the '@<scope>/<package>' format

@@ -0,0 +1,5 @@+{+  "name": "@deno/no-token",+  "version": "1.0.0",+  "exports": "./mod.ts"+}