npm 7 and Yarn v2 support needed

[jdanil]

Is your feature request related to a problem? Please describe.

Yarn v2 introduced a new lockfile format that now adheres to YAML. I believe ATM when trying to run lockfile-lint on a project with yarn v2, lockfile-lint tries to use the v1 parser and fails with an error.

ABORTING lockfile lint process due to error exceptions

Unable to parse lockfile "yarn.lock"

SyntaxError: Unknown token: {line: 3, col: 2, type: ''INVALID', value: undefined } 3:2 in lockfile

Describe the solution you'd like

As far as I can tell, yarn v2 does not have an equivalent lockfile parser. But since it adheres to YAML now, I'm wondering if it is possible to just use a parser like yaml and create a small function to transform it like parseNpmLockfile?

Describe alternatives you've considered

None.

[lirantal]

@jdanil yep, sounds like that would be a great way to support it and shouldn't need too much work besides some try/catch to figure out which yarn version it is when you start parsing. Would you be up to sending a PR to lockfile-lint to support it?

[abdulhannanali]

@lirantal Hey, I would like to work on this issue, however, I see there's this lockfile parser https://github.com/snyk/nodejs-lockfile-parser/ which seems to be covering yarn lockfile v2 already. However, they don't provide the capability of getting the resolved url and the integrity hash.

Do you think I should take this up with the authors of that repo and if they are willing to accept that change? Once done, we can delegate all the parsing related logic to that package.

[abdulhannanali]

In case, if you think they won't be able to accept that change, I can make a PR to this repo adding support for yarn v2 lockfile, I have already done some prior work

[lirantal]

@abdulhannanali

Do you think I should take this up with the authors of that repo and if they are willing to accept that change? Once done, we can delegate all the parsing related logic to that package.

Yep, sounds like a good idea to see if the folks maintaining that nodejs-lockfile-parser are interested to add those metadata items, so we can ultimately use that for the whole parsing we need.

Sounds good with me. Happy to have you collaborate on this pull request. Thanks ❤️

[abdulhannanali]

@lirantal

Awesome, I'll open an issue there and mention you to get this going. Thank you likewise, happy to collaborate <3

[jerone]

It looks like npm 7 support was implemented: https://github.com/snyk/nodejs-lockfile-parser/releases/tag/v1.34.0

[lirantal]

Nice. Thanks for the heads up @jerone @abdulhannanali would you want to go at it?

[abdulhannanali]

@lirantal Thanks for the heads up, sorry I wasn't able to attend to it earlier. I will take a go at it.

[kachkaev]

Same problem here when attempting to upgrade from Yarn 1 to Yarn 4 RC (Berry): https://github.com/blockprotocol/blockprotocol/actions/runs/3274094581/jobs/5387403013#step:8:17

 ℹ ABORTING lockfile lint process due to error exceptions 
 
Unable to parse yarn lockfile "yarn.lock" 

Error: Lockfile does not seem to contain a valid dependency list
    at yarnParseAndVerify (/home/runner/work/blockprotocol/blockprotocol/node_modules/lockfile-lint-api/src/ParseLockfile.js:42:11)
    at ParseLockfile.parseYarnLockfile (/home/runner/work/blockprotocol/blockprotocol/node_modules/lockfile-lint-api/src/ParseLockfile.js:141:20)
    at ParseLockfile.parseSync (/home/runner/work/blockprotocol/blockprotocol/node_modules/lockfile-lint-api/src/ParseLockfile.js:103:27)
    at ValidateHostManager (/home/runner/work/blockprotocol/blockprotocol/node_modules/lockfile-lint/src/validators/index.js:49:27)
    at /home/runner/work/blockprotocol/blockprotocol/node_modules/lockfile-lint/src/main.js:41:28
    at Array.forEach (<anonymous>)
    at Object.runValidators (/home/runner/work/blockprotocol/blockprotocol/node_modules/lockfile-lint/src/main.js:31:14)
    at Object.<anonymous> (/home/runner/work/blockprotocol/blockprotocol/node_modules/lockfile-lint/bin/lockfile-lint.js:80:17)
    at Module._compile (node:internal/modules/cjs/loader:1126:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1180:10) 

/home/runner/work/blockprotocol/blockprotocol/node_modules/lockfile-lint/bin/lockfile-lint.js:89
  error('Error: command failed with exit code 1')
  ^

TypeError: error is not a function
    at Object.<anonymous> (/home/runner/work/blockprotocol/blockprotocol/node_modules/lockfile-lint/bin/lockfile-lint.js:89:3)
    at Module._compile (node:internal/modules/cjs/loader:1126:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1180:10)
    at Module.load (node:internal/modules/cjs/loader:1004:32)
    at Function.Module._load (node:internal/modules/cjs/loader:839:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:17:47

PR: https://github.com/blockprotocol/blockprotocol/pull/680

[brad-decker]

So yarnpkg/parsers is now updated to work with the new lockfile format. I was able to get everything "working" with the following patch:

diff --git a/src/ParseLockfile.js b/src/ParseLockfile.js
index 0f0c951027ec83c61769bb6a48943420dff133b8..bad2d251cf376bf3ef4b444a0d49f03a602d7a6e 100644
--- a/src/ParseLockfile.js
+++ b/src/ParseLockfile.js
@@ -21,13 +21,13 @@ const {
  * @return boolean
  */
 function checkSampleContent (lockfile) {
-  const [sampleKey, sampleValue] = Object.entries(lockfile)[0]
+  const [sampleKey, sampleValue] = Object.entries(lockfile)[1]
   return (
     sampleKey.match(/.*@.*/) &&
     (sampleValue &&
       typeof sampleValue === 'object' &&
       sampleValue.hasOwnProperty('version') &&
-      sampleValue.hasOwnProperty('resolved'))
+      sampleValue.hasOwnProperty('resolution'))
   )
 }
 /**
@@ -41,7 +41,25 @@ function yarnParseAndVerify (lockfileBuffer) {
   if (!hasSensibleContent) {
     throw Error('Lockfile does not seem to contain a valid dependency list')
   }
-  return {type: 'success', object: lockfile}
+  const normalized = Object.fromEntries(Object.entries(lockfile).map(([packageName, packageDetails]) => {
+    const resolution = packageDetails.resolution;
+    if (!resolution) {
+      return [packageName, packageDetails];
+    }
+    const splitByAt = resolution.split('@');
+    let [resolvedPackageName, host] = splitByAt;
+    if (splitByAt.length > 2) {
+      resolvedPackageName = `${splitByAt[0]}${splitByAt[1]}`;
+      host = splitByAt[2];
+    }
+
+    if (splitByAt.length > 2 && resolution[0] !== '@') {
+      [resolvedPackageName, host] = splitByAt;
+    }
+
+    return [packageName, { ...packageDetails, resolved: host}]
+  }))
+  return {type: 'success', object: normalized}
 }
 class ParseLockfile {
   /**

Note, i think you will also have to get the latest version of yarnpkg/parsers resolved, and you'll have to add 'npm:', 'patch:' etc to schemes instead of hosts. The way I broke apart the resolution field is probably really naive but it met our use case. It would be much safer to use a regex or something to pull any urls out of the resolution and evaluate them for hosts, and then somehow leave the scheme (npm:, patch:) etc for evaluation.

[lirantal]

@brad-decker the above is just a Yarn2 compatible update then? I'm thinking, perhaps we could have specific Yarn versions logic spread out, and then have one main code that determines the lockfile version, and then uses the relevant parser. WDYT? If you wanted to take a shot at that, I'm happy to merge a PR.

[brad-decker]

Im about to go on vacation but when i get back and up to speed ill work on it

[lirantal]

Sounds good!

[kachkaev]

@brad-decker are you still interested in crafting a PR? No worries if not, just checking if the task is taken.

[naugtur]

I didn't read through the entire convo, but as yarn above 1 goes, I remember replacing the lib used for reading yarn.lock with the modern one. FYI

[candrews]

Can the title of this issue please be updated to "Yarn v2 support needed" for accuracy since NPM v7 is already supported?

[brad-decker]

@kachkaev if you or someone else wants to tackle please go for it. Otherwise i'll take a stab at it soon

[naugtur]

@brad-decker call me whenever you need a second pair of eyes

[naugtur]

Oh, and are you sure it's not working? I replaced the lockfile parser with the one used by yarn berry earlier this year https://github.com/lirantal/lockfile-lint/commit/18c6ae0e75b8064ec7ca8c8460e7a6f9d9546f9a

[kachkaev]

@naugtur I'm on [email protected] (2022-10-08) and it does not seem to work with [email protected]:

• https://github.com/lirantal/lockfile-lint/issues/101#issuecomment-1282502621
• https://github.com/blockprotocol/blockprotocol/pull/680
• Output for lockfile-lint --path yarn.lock --allowed-hosts registry.yarnpkg.com --allowed-schemes \"https:\":
  Unable to parse yarn lockfile "yarn.lock" (CI output)

[brad-decker]

@lirantal i have authored https://github.com/lirantal/lockfile-lint/pull/147 which is a cleaned up version of my patch for our project. It doesn't do anything in the way of adding support to the command line interface and proper documentation of the way yarn berry appends hostnames to the package is needed to be effective. For example this is our usage:

lockfile-lint --path yarn.lock --allowed-hosts npm yarn github.com codeload.github.com --empty-hostname true --allowed-schemes "https:" "git+https:" "npm:" "patch:" "workspace:"

[lirantal]

Thanks, looking at it, Brad. Appreciate the PR.