fix(npm): support old packages and registries with no integrity, but with a sha1sum (#17289)

Closes #17281

cli/args/lockfile.rs

@@ -289,12 +289,7 @@ impl Lockfile {
   ) -> Result<(), LockfileError> {
     let specifier = package.id.as_serialized();
     if let Some(package_info) = self.content.npm.packages.get(&specifier) {
-      let integrity = package
+      if package_info.integrity.as_str() != package.dist.integrity().as_str() {
-        .dist
-        .integrity
-        .as_ref()
-        .unwrap_or(&package.dist.shasum);
-      if &package_info.integrity != integrity {
         return Err(LockfileError(format!(
           "Integrity check failed for npm package: \"{}\". Unable to verify that the package
 is the same as when the lockfile was generated.
@@ -321,15 +316,10 @@ Use \"--lock-write\" flag to regenerate the lockfile at \"{}\".",
       .map(|(name, id)| (name.to_string(), id.as_serialized()))
       .collect::<BTreeMap<String, String>>();
 
-    let integrity = package
-      .dist
-      .integrity
-      .as_ref()
-      .unwrap_or(&package.dist.shasum);
     self.content.npm.packages.insert(
       package.id.as_serialized(),
       NpmPackageInfo {
-        integrity: integrity.to_string(),
+        integrity: package.dist.integrity().to_string(),
         dependencies,
       },
     );
@@ -545,11 +535,11 @@ mod tests {
         peer_dependencies: Vec::new(),
       },
       copy_index: 0,
-      dist: NpmPackageVersionDistInfo {
+      dist: NpmPackageVersionDistInfo::new(
-        tarball: "foo".to_string(),
+        "foo".to_string(),
-        shasum: "foo".to_string(),
+        "shasum".to_string(),
-        integrity: Some("sha512-MqBkQh/OHTS2egovRtLk45wEyNXwF+cokD+1YPf9u5VfJiRdAiRwB2froX5Co9Rh20xs4siNPm8naNotSD6RBw==".to_string())
+        Some("sha512-MqBkQh/OHTS2egovRtLk45wEyNXwF+cokD+1YPf9u5VfJiRdAiRwB2froX5Co9Rh20xs4siNPm8naNotSD6RBw==".to_string()),
-      },
+      ),
       dependencies: HashMap::new(),
     };
     let check_ok = lockfile.check_or_insert_npm_package(&npm_package);
@@ -562,11 +552,11 @@ mod tests {
         peer_dependencies: Vec::new(),
       },
       copy_index: 0,
-      dist: NpmPackageVersionDistInfo {
+      dist: NpmPackageVersionDistInfo::new(
-        tarball: "foo".to_string(),
+        "foo".to_string(),
-        shasum: "foo".to_string(),
+        "shasum".to_string(),
-        integrity: Some("sha512-1fygroTLlHu66zi26VoTDv8yRgm0Fccecssto+MhsZ0D/DGW2sm8E8AjW7NU5VVTRt5GxbeZ5qBuJr+HyLYkjQ==".to_string())
+        Some("sha512-1fygroTLlHu66zi26VoTDv8yRgm0Fccecssto+MhsZ0D/DGW2sm8E8AjW7NU5VVTRt5GxbeZ5qBuJr+HyLYkjQ==".to_string()),
-      },
+      ),
       dependencies: HashMap::new(),
     };
     // Integrity is borked in the loaded lockfile
@@ -580,11 +570,11 @@ mod tests {
         peer_dependencies: Vec::new(),
       },
       copy_index: 0,
-      dist: NpmPackageVersionDistInfo {
+      dist: NpmPackageVersionDistInfo::new(
-        tarball: "foo".to_string(),
+        "foo".to_string(),
-        shasum: "foo".to_string(),
+        "foo".to_string(),
-        integrity: Some("sha512-R0XvVJ9WusLiqTCEiGCmICCMplcCkIwwR11mOSD9CR5u+IXYdiseeEuXCVAjS54zqwkLcPNnmU4OeJ6tUrWhDw==".to_string())
+        Some("sha512-R0XvVJ9WusLiqTCEiGCmICCMplcCkIwwR11mOSD9CR5u+IXYdiseeEuXCVAjS54zqwkLcPNnmU4OeJ6tUrWhDw==".to_string()),
-      },
+      ),
       dependencies: HashMap::new(),
     };
     // Not present in lockfile yet, should be inserted and check passed.
@@ -598,11 +588,11 @@ mod tests {
         peer_dependencies: Vec::new(),
       },
       copy_index: 0,
-      dist: NpmPackageVersionDistInfo {
+      dist: NpmPackageVersionDistInfo::new(
-        tarball: "foo".to_string(),
+        "foo".to_string(),
-        shasum: "foo".to_string(),
+        "foo".to_string(),
-        integrity: Some("sha512-foobar".to_string()),
+        Some("sha512-foobar".to_string()),
-      },
+      ),
       dependencies: HashMap::new(),
     };
     // Now present in lockfile, should file due to borked integrity

cli/npm/registry.rs

@@ -1,5 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
+use std::borrow::Cow;
 use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::collections::HashSet;
@@ -178,8 +179,31 @@ impl NpmPackageVersionInfo {
 pub struct NpmPackageVersionDistInfo {
   /// URL to the tarball.
   pub tarball: String,
-  pub shasum: String,
+  shasum: String,
-  pub integrity: Option<String>,
+  integrity: Option<String>,
+}
+
+impl NpmPackageVersionDistInfo {
+  #[cfg(test)]
+  pub fn new(
+    tarball: String,
+    shasum: String,
+    integrity: Option<String>,
+  ) -> Self {
+    Self {
+      tarball,
+      shasum,
+      integrity,
+    }
+  }
+
+  pub fn integrity(&self) -> Cow<String> {
+    self
+      .integrity
+      .as_ref()
+      .map(Cow::Borrowed)
+      .unwrap_or_else(|| Cow::Owned(format!("sha1-{}", self.shasum)))
+  }
 }
 
 pub trait NpmRegistryApi: Clone + Sync + Send + 'static {

cli/npm/resolution/snapshot.rs

@@ -275,11 +275,7 @@ impl NpmResolutionSnapshot {
           id: package_id.clone(),
           copy_index: copy_index_resolver.resolve(&package_id),
           // temporary dummy value
-          dist: NpmPackageVersionDistInfo {
+          dist: NpmPackageVersionDistInfo::default(),
-            tarball: "foobar".to_string(),
-            shasum: "foobar".to_string(),
-            integrity: Some("foobar".to_string()),
-          },
           dependencies,
         };
 

cli/npm/tarball.rs

@@ -21,16 +21,7 @@ pub fn verify_and_extract_tarball(
   dist_info: &NpmPackageVersionDistInfo,
   output_folder: &Path,
 ) -> Result<(), AnyError> {
-  if let Some(integrity) = &dist_info.integrity {
+  verify_tarball_integrity(package, data, &dist_info.integrity())?;
-    verify_tarball_integrity(package, data, integrity)?;
-  } else {
-    // todo(dsherret): check shasum here
-    bail!(
-      "Errored on '{}@{}': npm packages with no integrity are not implemented.",
-      package.0,
-      package.1,
-    );
-  }
 
   with_folder_sync_lock(package, output_folder, || {
     extract_tarball(data, output_folder)
@@ -43,11 +34,11 @@ fn verify_tarball_integrity(
   npm_integrity: &str,
 ) -> Result<(), AnyError> {
   use ring::digest::Context;
-  use ring::digest::SHA512;
   let (algo, expected_checksum) = match npm_integrity.split_once('-') {
     Some((hash_kind, checksum)) => {
       let algo = match hash_kind {
-        "sha512" => &SHA512,
+        "sha512" => &ring::digest::SHA512,
+        "sha1" => &ring::digest::SHA1_FOR_LEGACY_USE_ONLY,
         hash_kind => bail!(
           "Not implemented hash function for {}@{}: {}",
           package.0,
@@ -144,11 +135,20 @@ mod test {
         .to_string(),
       "Not implemented integrity kind for package@1.0.0: test",
     );
+    assert_eq!(
+      verify_tarball_integrity(package, &Vec::new(), "notimplemented-test")
+        .unwrap_err()
+        .to_string(),
+      "Not implemented hash function for package@1.0.0: notimplemented",
+    );
     assert_eq!(
       verify_tarball_integrity(package, &Vec::new(), "sha1-test")
         .unwrap_err()
         .to_string(),
-      "Not implemented hash function for package@1.0.0: sha1",
+      concat!(
+        "Tarball checksum did not match what was provided by npm ",
+        "registry for package@1.0.0.\n\nExpected: test\nActual: 2jmj7l5rsw0yvb/vlwaykk/ybwk=",
+      ),
     );
     assert_eq!(
       verify_tarball_integrity(package, &Vec::new(), "sha512-test")