fix: fail gracefully for npm .package-lock.json files #3654
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* fixes intel#3559 npm has a .package-lock.json file with a similar name but slightly different format than the package-lock.json file we can parse with our javascript language parser. This changes the javascript parser so it fails more gracefully if the format of a package-lock.json file does not appear to be what we can parse. As in, it prints a warning that it was unable to parse the file and does not halt the scan. We should probably build a parser to handle these files correctly in the future, but for now this will skip them. Also, I added some docstrings to the files I changed so interrogate would be happy. Signed-off-by: Terri Oda <[email protected]>
|
Updating branch for tests. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3654 +/- ##
==========================================
- Coverage 79.47% 79.27% -0.21%
==========================================
Files 797 797
Lines 11944 11952 +8
Branches 1603 1605 +2
==========================================
- Hits 9493 9475 -18
- Misses 1991 2040 +49
+ Partials 460 437 -23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
antoniogi
approved these changes
Jan 11, 2024
| if "dependencies" not in data.keys(): | ||
| self.logger.warning( | ||
| f"Cannot parse {self.filename}: no dependencies array found." | ||
| ) |
There was a problem hiding this comment.
That looks good. The only thing I can think of is: is there a case where you can have more than 1 "dependencies" keys? If so, would it be better to also add the product and version to the warning message? (Feel free to ignore if too dumb of a comment)
There was a problem hiding this comment.
Hm, in theory people could give us just about anything so it would be possible, but I haven't seen any tool generate this yet or anyone complain about us not parsing it. I'll put a note about it when I file an issue about handling the npm package locks, though, since it might be relevant then.
inosmeet
pushed a commit
to inosmeet/cve-bin-tool
that referenced
this pull request
Feb 6, 2024
* fixes intel#3559 npm has a .package-lock.json file with a similar name but slightly different format than the package-lock.json file we can parse with our javascript language parser. This changes the javascript parser so it fails more gracefully if the format of a package-lock.json file does not appear to be what we can parse. As in, it prints a warning that it was unable to parse the file and does not halt the scan. We should probably build a parser to handle these files correctly in the future, but for now this will skip them. Also, I added some docstrings to the files I changed so interrogate would be happy. Signed-off-by: Terri Oda <[email protected]>
inosmeet
pushed a commit
to inosmeet/cve-bin-tool
that referenced
this pull request
Feb 16, 2024
* fixes intel#3559 npm has a .package-lock.json file with a similar name but slightly different format than the package-lock.json file we can parse with our javascript language parser. This changes the javascript parser so it fails more gracefully if the format of a package-lock.json file does not appear to be what we can parse. As in, it prints a warning that it was unable to parse the file and does not halt the scan. We should probably build a parser to handle these files correctly in the future, but for now this will skip them. Also, I added some docstrings to the files I changed so interrogate would be happy. Signed-off-by: Terri Oda <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
KeyError: 'dependencies'with npm projects #3559npm has a .package-lock.json file with a similar name but slightly different format than the package-lock.json file we can parse with our javascript language parser. This changes the javascript parser so it fails more gracefully if the format of a package-lock.json file does not appear to be what we can parse. As in, it prints a warning that it was unable to parse the file and does not halt the scan.
We should probably build a parser to handle these files correctly in the future, but for now this will skip them.
Also, I added some docstrings to the files I changed so interrogate would be happy.