From 3af501d8b77a34bbe394d4d54550c0371019ab02 Mon Sep 17 00:00:00 2001
From: Philip Harrison <philip@mailharrison.com>
Date: Mon, 23 May 2022 19:17:35 +0100
Subject: [PATCH] Refactor audit verify signatures class

---
 lib/commands/audit.js | 84 ++++++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 36 deletions(-)

diff --git a/lib/commands/audit.js b/lib/commands/audit.js
index 09c1db16635e4..16649733b74bb 100644
--- a/lib/commands/audit.js
+++ b/lib/commands/audit.js
@@ -42,7 +42,7 @@ class VerifySignatures {
     }
 
     // Prefetch and cache public keys from used registries
-    const registries = this.findAllRegistryUrls(this.edges, this.npm.flatOptions)
+    const registries = this.findAllRegistryUrls(this.edges)
     for (const registry of registries) {
       const keys = await this.getKeys({ registry })
       if (keys) {
@@ -122,7 +122,7 @@ class VerifySignatures {
     }
   }
 
-  findAllRegistryUrls (edges, opts) {
+  findAllRegistryUrls (edges) {
     return new Set(Array.from(edges, (edge) => {
       let alias = false
       try {
@@ -130,7 +130,7 @@ class VerifySignatures {
       } catch (err) {
       }
       const spec = npa(alias ? alias.name : edge.name)
-      return fetch.pickRegistry(spec, opts)
+      return this.getSpecRegistry(spec)
     }))
   }
 
@@ -176,35 +176,63 @@ class VerifySignatures {
     })
   }
 
-  async getVerifiedInfo (edge) {
+  getEdgeType (edge) {
+    return edge.optional ? 'optionalDependencies'
+      : edge.peer ? 'peerDependencies'
+      : edge.dev ? 'devDependencies'
+      : 'dependencies'
+  }
+
+  getEdgeSpec (edge) {
     let alias = false
     try {
       alias = npa(edge.spec).subSpec
     } catch (err) {
     }
-    const spec = npa(alias ? alias.name : edge.name)
-    const node = edge.to || edge
-    const { location } = node
-    const { version } = node.package || {}
-
-    const type = edge.optional ? 'optionalDependencies'
-      : edge.peer ? 'peerDependencies'
-      : edge.dev ? 'devDependencies'
-      : 'dependencies'
-
-    // Skip local workspaces
-    if (node.isWorkspace) {
+    let spec
+    try {
+      spec = npa(`${alias ? alias.name : edge.name}@${edge.spec}`)
+    } catch (_) {
+      // Skip packages with invalid spec
       return
     }
+    return spec
+  }
+
+  buildRegistryConfig (registry) {
+    const keys = this.keys.get(registry) || []
+    const registryConfig = {}
+    const parsedRegistry = new URL(registry)
+    const regKey = `//${parsedRegistry.host}${parsedRegistry.pathname}`
+    registryConfig[`${regKey}:_keys`] = keys
+    return registryConfig
+  }
 
+  getSpecRegistry (spec) {
+    return fetch.pickRegistry(spec, this.npm.flatOptions)
+  }
+
+  async getVerifiedInfo (edge) {
+    const type = this.getEdgeType(edge)
     // Skip potentially optional packages that are not on disk, as these could
     // be omitted during install
     if (edge.error === 'MISSING' && type !== 'dependencies') {
       return
     }
 
-    // Skip packages that don't have a installed version, e.g. optonal dependencies
-    if (!version) {
+    const spec = this.getEdgeSpec(edge)
+    // Skip invalid spec's
+    if (!spec) {
+      return
+    }
+    const node = edge.to || edge
+    const { location } = node
+    const name = spec.name
+    const { version } = node.package || {}
+
+    if (node.isWorkspace || // Skip local workspaces packages
+        !version || // Skip packages that don't have a installed version, e.g. optonal dependencies
+        !spec.registry) { // Skip if not from registry, e.g. git package
       return
     }
 
@@ -214,32 +242,16 @@ class VerifySignatures {
       }
     }
 
-    // Skip if the package is not in a registry, e.g. git or local workspace package
-    try {
-      if (!npa(`${alias ? alias.name : edge.name}@${edge.spec}`).registry) {
-        return null
-      }
-    } catch (err) {
-      return null
-    }
-
     this.audited += 1
-    const name = spec.name
-
-    const registry = fetch.pickRegistry(spec, this.npm.flatOptions)
-    const keys = this.keys.get(registry) || []
 
+    const registry = this.getSpecRegistry(spec)
     try {
-      const registryConfig = {}
-      const parsedRegistry = new URL(registry)
-      const regKey = `//${parsedRegistry.host}${parsedRegistry.pathname}`
-      registryConfig[`${regKey}:_keys`] = keys
       const {
         _integrity: integrity,
         _signatures,
         _resolved: resolved,
       } = await pacote.manifest(`${name}@${version}`, {
-        verifySignatures: true, ...registryConfig, ...this.npm.flatOptions,
+        verifySignatures: true, ...this.buildRegistryConfig(registry), ...this.npm.flatOptions,
       })
       const signatures = _signatures || []