fix: error when passing signature without keys (#176)

Raise an error when the package has a signature, `verifySignatures` is true and there are no registry keys configured.

Updated the error message and fixed the undefined name@version in the error message to match the test cases here:
npm/cli#4827

Possible anti-pattern: I've attached a bunch of error attributes if the signature is invalid so we can present this in the json output to ease debugging.

lib/registry.js

@@ -122,11 +122,12 @@ class RegistryFetcher extends Fetcher {
     }
 
     const packument = await this.packument()
-    const mani = await pickManifest(packument, this.spec.fetchSpec, {
+    let mani = await pickManifest(packument, this.spec.fetchSpec, {
       ...this.opts,
       defaultTag: this.defaultTag,
       before: this.before,
     })
+    mani = rpj.normalize(mani)
     /* XXX add ETARGET and E403 revalidation of cached packuments here */
 
     // add _resolved and _integrity from dist object
@@ -165,49 +166,53 @@ class RegistryFetcher extends Fetcher {
       mani._integrity = String(this.integrity)
       if (dist.signatures) {
         if (this.opts.verifySignatures) {
-          if (this.registryKeys) {
-            // validate and throw on error, then set _signatures
-            const message = `${mani._id}:${mani._integrity}`
-            for (const signature of dist.signatures) {
-              const publicKey = this.registryKeys.filter(key => (key.keyid === signature.keyid))[0]
-              if (!publicKey) {
-                throw Object.assign(new Error(
-                  `${mani._id} has a signature with keyid: ${signature.keyid} ` +
-                  'but no corresponding public key can be found.'
-                ), { code: 'EMISSINGSIGNATUREKEY' })
-              }
-              const validPublicKey =
-                !publicKey.expires || (Date.parse(publicKey.expires) > Date.now())
-              if (!validPublicKey) {
-                throw Object.assign(new Error(
-                  `${mani._id} has a signature with keyid: ${signature.keyid} ` +
+          // validate and throw on error, then set _signatures
+          const message = `${mani._id}:${mani._integrity}`
+          for (const signature of dist.signatures) {
+            const publicKey = this.registryKeys &&
+              this.registryKeys.filter(key => (key.keyid === signature.keyid))[0]
+            if (!publicKey) {
+              throw Object.assign(new Error(
+                  `${mani._id} has a registry signature with keyid: ${signature.keyid} ` +
+                  'but no corresponding public key can be found'
+              ), { code: 'EMISSINGSIGNATUREKEY' })
+            }
+            const validPublicKey =
+              !publicKey.expires || (Date.parse(publicKey.expires) > Date.now())
+            if (!validPublicKey) {
+              throw Object.assign(new Error(
+                  `${mani._id} has a registry signature with keyid: ${signature.keyid} ` +
                   `but the corresponding public key has expired ${publicKey.expires}`
-                ), { code: 'EEXPIREDSIGNATUREKEY' })
-              }
-              const verifier = crypto.createVerify('SHA256')
-              verifier.write(message)
-              verifier.end()
-              const valid = verifier.verify(
-                publicKey.pemkey,
-                signature.sig,
-                'base64'
-              )
-              if (!valid) {
-                throw Object.assign(new Error(
-                  'Integrity checksum signature failed: ' +
-                  `key ${publicKey.keyid} signature ${signature.sig}`
-                ), { code: 'EINTEGRITYSIGNATURE' })
-              }
+              ), { code: 'EEXPIREDSIGNATUREKEY' })
+            }
+            const verifier = crypto.createVerify('SHA256')
+            verifier.write(message)
+            verifier.end()
+            const valid = verifier.verify(
+              publicKey.pemkey,
+              signature.sig,
+              'base64'
+            )
+            if (!valid) {
+              throw Object.assign(new Error(
+                  `${mani._id} has an invalid registry signature with ` +
+                  `keyid: ${publicKey.keyid} and signature: ${signature.sig}`
+              ), {
+                code: 'EINTEGRITYSIGNATURE',
+                keyid: publicKey.keyid,
+                signature: signature.sig,
+                resolved: mani._resolved,
+                integrity: mani._integrity,
+              })
             }
-            mani._signatures = dist.signatures
           }
-          // if no keys, don't set _signatures
+          mani._signatures = dist.signatures
         } else {
           mani._signatures = dist.signatures
         }
       }
     }
-    this.package = rpj.normalize(mani)
+    this.package = mani
     return this.package
   }
 

test/registry.js

@@ -240,9 +240,15 @@ t.test('verifySignatures invalid signature', async t => {
     }],
   })
   return t.rejects(
+    /abbrev@1\.1\.1 has an invalid registry signature/,
     f.manifest(),
     {
       code: 'EINTEGRITYSIGNATURE',
+      keyid: 'SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA',
+      signature: 'nope',
+      resolved: 'https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz',
+      // eslint-disable-next-line max-len
+      integrity: 'sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==',
     }
   )
 })
@@ -258,6 +264,7 @@ t.test('verifySignatures no valid key', async t => {
   })
   return t.rejects(
     f.manifest(),
+    /@isaacs\/namespace-test@1\.0\.0 has a registry signature/,
     {
       code: 'EMISSINGSIGNATUREKEY',
     }
@@ -271,9 +278,13 @@ t.test('verifySignatures no registry keys at all', async t => {
     verifySignatures: true,
     [`//localhost:${port}/:_keys`]: null,
   })
-  return f.manifest().then(mani => {
-    t.notOk(mani._signatures)
-  })
+  return t.rejects(
+    f.manifest(),
+    /@isaacs\/namespace-test@1\.0\.0 has a registry signature/,
+    {
+      code: 'EMISSINGSIGNATUREKEY',
+    }
+  )
 })
 
 t.test('404 fails with E404', t => {