feat(nodejs): add license parser to pnpm analyser

aquasecurity / trivy

Find vulnerabilities, misconfigurations, secrets, SBOM in containers, Kubernetes, code repositories, clouds and more

https://aquasecurity.github.io/trivy

Apache License 2.0

22.1k stars 2.18k forks source link

feat(nodejs): add license parser to pnpm analyser #7036

Closed oscarbc96 closed 2 days ago

oscarbc96 commented 1 week ago

Description

pnpm-lock.yaml files do not contain dependency license information. This PR parses */node_modules//package.json files alongside pnpm-lock.json and identify licenses.

Related Issues

Checklist

[x] I've read the guidelines for contributing to this repository.
[x] I've followed the conventions in the PR title.
[x] I've added tests that prove my fix is effective or that my feature works.
[x] I've updated the documentation with the relevant information (if needed).
[ ] I've added usage information (if the PR introduces new options)
[ ] I've included a "before" and "after" example to the description (if the PR is a user interface change).

CLAassistant commented 1 week ago

All committers have signed the CLA.

knqyf263 commented 1 week ago

You can reference this page to pass tests. Please feel free to ask if you need assistance.

knqyf263 commented 4 days ago

Thanks for your contribution. LGTM. One comment: I want as few test files as possible so we can easily maintain tests in the future. Can you please provide a minimum file structure that meets the case you want to test?

oscarbc96 commented 4 days ago

Thanks for your feedback! The tests are already consolidated into a minimal structure. The happy path test requires only two dependencies: ms, which has no additional dependencies, and @babel/parser, which has several indirect dependencies. This allows us to comprehensively test that everything is working correctly.

Due to the nature of these dependencies, it might appear that we have several test data files. However, to keep things simple, everything except the package.json file has been removed. This ensures that we maintain the necessary coverage while adhering to the minimal file structure.

knqyf263 commented 4 days ago

I still feel like @babel/parser has too many dependencies. We don't need to use an actual package for unit tests. You can add it to integration tests.

oscarbc96 commented 4 days ago

You're right. After giving it a second thought, I removed @babel/parser since testing the detection of indirect dependencies should be handled by the pnpm parser. The pnpm parser already has tests for these dependencies.

knqyf263 commented 4 days ago

@DmitriyLewen Could you also take a look?