Exclude devDependencies from package-lock.json parsing

[njv299]

Description

This updates the Javascript cataloger so that packages that are only present as dev dependencies found in package-lock.json files are ignored and not included in the resulting SBOM. This behavior more closely aligns with that of other similar catalogers, such as the Python Pipfile.lock parser.

This change supports all current package-lock.json schema versions (1, 2, and 3) in accordance with the spec found here.

• Fixes Syft outputs devDependencies for package-lock.json files #2348

Type of change

• New feature (non-breaking change which adds functionality)

Checklist:

• I have added unit tests that cover changed behavior
• I have tested my code in common scenarios and confirmed there are no regressions
• I have added comments to my code, particularly in hard-to-understand sections

[joaquinricci]

Sorry to drop by without more than a proposal right now. I believe that excluding development dependencies should be optional instead of imposed by the tool. They could even be excluded by default, but there are cases in which makes sense to present a more complete SBOM including dev dependencies.

What about adding a flag? The trivy tool has a flag for it:
--include-dev-deps

[njv299]

@joaquinricci I would certainly be in support of a --include-dev-deps configuration option, but I would argue that that should be an option that is consistently applied across all catalogers (or made available as a configurable option for all relevant catalogers). As of right now, there is different behavior across catalogers with respect to dev dependencies: Pipfile.lock file parsing does not include dev deps, package.json also doesn't include them, while package-lock.json parsing does.

It's obviously not my decision to make, but in the near-term my opinion is that this PR makes the behavior more consistent as-is and there should be follow-on issues and PRs to holistically add a consistent, comprehensive set of --include-dev-deps functionality.

[kzantow]

For what it's worth, I agree with all of the comments here: @joaquinricci and @njv299 that this probably should be configurable as part of this PR, the configuration should be applied consistently across pertinent catalogers, and also agree that the default behavior should be to exclude development-only time dependencies.

This is absolutely something that other ecosystems would benefit from: Java, for example, should probably exclude test-scoped dependencies by default. I think it would be worth spending a little time thinking about how configuration should work. It may be useful to move this discussion to Discourse (there is a related topic).

[njv299]

To clarify my stance on this: In my opinion this PR should be merged as-is. Based on the discussion in the relevant issue, the user-feedback that devDependencies are generally not preferred to be included, and my own N=1 experience, it seems very valuable to change the default behavior of Syft today to exclude devDependencies. This solves what appears to be a fairly pressing need for a not-insignificant number of people (myself included), and makes the behavior of Syft more consistent across all catalogers.

I am also fully in support of subsequently making this behavior configurable as discussed above. Personally, though, I think this should be done via follow-on PRs so that the advantages mentioned above can be realized as soon as possible. If time permits I will be happy to help with the implementation of the configuration options, but getting this PR merged and released soon will fix some significant issues in my personal use of Syft.