snyk / nodejs-lockfile-parser

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

fix: throw 'not implemented' error in buildDepTree for npm v2/v3 lockfiles #200

Open milahu opened 11 months ago

milahu commented 11 months ago

What this does

throw not implemented error in buildDepTree for npm v2/v3 lockfiles

better than OutOfSyncError from getDependencyTree

OutOfSyncError: Dependency @cycle/http was not found in package-lock.json. Your package.json and package-lock.json are probably out of sync. Please run "npm install" and try again.
    at PackageLockParser.getDependencyTree (node_modules/snyk-nodejs-lockfile-parser/dist/parsers/lock-parser-base.js:124:27)
    at async PackageLockParser.getDependencyTree (node_modules/snyk-nodejs-lockfile-parser/dist/parsers/package-lock-parser.js:28:32)
  code: 422,
  dependencyName: '@cycle/http',
  lockFileType: 'npm7'

problem in PackageLockParser.getDepMap: packageLock.dependencies is undefined, it should use packageLock.packages

https://github.com/snyk/nodejs-lockfile-parser/blob/103bb2de2e8391fab2b35c57e8c64b14e8205e3e/lib/parsers/package-lock-parser.ts#L110

so the not implemented error is thrown in getDepMap before calling flattenLockfileRec

to implement support for v2/v3 npm lockfiles, a good place would be getDepMap

  protected getDepMapV2(packageLock: PackageLock): DepMap {
    // TODO implement
  }

  protected getDepMap(lockfile: Lockfile): DepMap {
    const packageLock = lockfile as PackageLock;

    if (packageLock.lockfileVersion == 2) {
      return this.getDepMapV2(packageLock);
    }

Notes for the reviewer

low priority

calderonth commented 10 months ago

Does that mean that currently calling buildDepTree is expected to fail for npm v2/v3 lockfiles?

milahu commented 10 months ago

yes, see the readme

This is a small utility package that parses lock file and returns either a [dependency tree](https://github.com/snyk/nodejs-lockfile-parser/blob/1a495302089614205478d57611bf7c39d29ce66d/lib/parsers/index.ts#L51) or a [dependency graph](https://github.com/snyk/dep-graph). Dependency graphs are the more modern data type and we plan to migrate fully over. Dep graph generation supported for: - `package-lock.json` (at Versions 2 and 3) - `yarn.lock` Legacy dep tree supported for: - `package-lock.json` - yarn 1 `yarn.lock` - yarn 2 `yarn.lock`
calderonth commented 10 months ago

ok so say for the time being I wanted to cover all manifest version I would have to add logic in order to detect manifest/lockfile types/versions and then call either dep-graph or dep-tree?

milahu commented 10 months ago

yes, thats what im doing in my pnpm-install-only

  const lockfileContent = read(lockfilePath);
  const lockfile = JSON.parse(lockfileContent);

  const [deps, walk_deps] = (
    lockfile.lockfileVersion == 3 ? await getDepgraph(lockfilePath) : // TODO verify
    lockfile.lockfileVersion == 2 ? await getDepgraph(lockfilePath) :
    lockfile.lockfileVersion == 1 ? await getDeptree(lockfilePath) :
    [null, null]
  )

  if (deps == null && walk_deps == null) {
    throw new Error('failed to recognize the lockfile type');
  }

edit: see also my parse-package-lock

calderonth commented 10 months ago

Right, I thought I could use this as a somewhat universal library to parse various lockfile format, so in top what you're doing you'd have to have edge cases for Yarn lockfiles which aren't JSON?

EDIT: I realise you have that logic backed into your code so I might get some inspiration from you :-)

milahu commented 10 months ago

yarn lockfiles are not-yet handled by my code

other limitations of snyk-nodejs-lockfile-parser: