snyk / nodejs-lockfile-parser

Generate a Snyk dependency tree from package-lock.json or yarn.lock file
Other
56 stars 28 forks source link

NPM package-lock.json TypeError: Cannot read properties of undefined (reading 'packages') #210

Closed matthewarmand closed 7 months ago

matthewarmand commented 7 months ago

I have an older npm project (still using legacy-peer-deps) on which Snyk is failing to monitor and test. I've tracked it down to the 1.1243.0 release, which contained an upgrade of this library from 1.52.1 to 1.52.5.

The message I'm seeing (I'll gist the entire debug output at the bottom) is:

  snyk:run-test Error running test {
  error: TypeError: Cannot read properties of undefined (reading 'packages')
      at resolvedToWorkspace (/snapshot/snyk/dist/cli/webpack:/snyk/node_modules/snyk-nodejs-lockfile-parser/dist/dep-graph-builders/npm-lock-v2/index.js:111:1)
      at getChildNode (/snapshot/snyk/dist/cli/webpack:/snyk/node_modules/snyk-nodejs-lockfile-parser/dist/dep-graph-builders/npm-lock-v2/index.js:122:1)

This project doesn't have a workspaces element, and I think the issue is linked to this updated workspaces parsing:

https://github.com/snyk/nodejs-lockfile-parser/compare/v1.52.1...v1.52.5#diff-81f70fb04fd56113a23046eb84b767b2c22af26fb791de3e071b1ecce7170fceR205-R213

This change pulls from similar code within npm itself:

https://github.com/npm/map-workspaces/blob/ff82968a3dbb78659fb7febfce4841bf58c514de/lib/index.js#L27-L41

Which is slightly safer than the snyk/nodejs-lockfile-parser implementation. The npm code sets workspaces to [] if it doesn't exist, earlier on in the call hierarchy. If you're accessing a property of an array which doesn't exist, there's no issue. The problematic element of the Snyk implementation seems to be the pkgs['']['workspaces']['packages'] in the Array.isArray check. ['workspaces'] doesn't exist in this instance, which is fine. But then attempting to access ['packages'] breaks (recall the above error message Cannot read properties of undefined (reading 'packages')).

Strangely, I can't reproduce this on other projects we have that use a v3 lockfile, even if they don't have the workspaces element. I also can't share this project directly as it's proprietary, but I'll gist the debug information below. Let me know if you need any further information from me!

Full debug log: https://gist.github.com/matthewarmand/3421b38ed6e1f78912cf797401527242

matthewarmand commented 7 months ago

Looks like #211 will probably fix this, thanks @JamesPatrickGill. I'm unfamiliar with Snyk's upgrade process, should I open an issue on snyk/cli requesting an upgrade of this library to 1.52.6? Or will that be handled imminently

matthewarmand commented 7 months ago

Lol nevermind, I just found https://github.com/snyk/cli/pull/4933.

JamesPatrickGill commented 7 months ago

No worries @matthewarmand, this was an oversight which slipped through on a recent contribution, came to my attention today.

I'll close this out when https://github.com/snyk/cli/pull/4933 is released.

Thanks for this excellent issue nonetheless! If I had have seen issue this I would have spotted the issue instantly.

JamesPatrickGill commented 7 months ago

Hi again @matthewarmand

The PR at https://github.com/snyk/cli/pull/4933 has now been released in snyk@1.1247.0. I'll keep this PR open until you have had chance to try the new version and confirmed the regression is resolved.

Thanks again 😃

matthewarmand commented 7 months ago

Yep that did it, thanks @JamesPatrickGill!