Skip to content

NuGet (packages.lock.json) parser.#14

Merged
knqyf263 merged 13 commits into
aquasecurity:masterfrom
Johannestegner:feature/nuget-parser
Oct 28, 2020
Merged

NuGet (packages.lock.json) parser.#14
knqyf263 merged 13 commits into
aquasecurity:masterfrom
Johannestegner:feature/nuget-parser

Conversation

@Johannestegner

@Johannestegner Johannestegner commented Oct 14, 2020

Copy link
Copy Markdown
Contributor

This pull request includes a parser which parses lockfiles created by nuget/dotnet when using a lockfile.
NuGet does not force (or auto) create a lock file, so a *.csproj file parser might also be a good idea, as a fallback.

As a first stage, this should be good enough though.

…sources.

Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
@Johannestegner Johannestegner changed the title WIP: NuGet parser WIP: NuGet (packages.lock.json) parser. Oct 14, 2020
@Johannestegner Johannestegner marked this pull request as ready for review October 14, 2020 12:55
@Johannestegner Johannestegner changed the title WIP: NuGet (packages.lock.json) parser. NuGet (packages.lock.json) parser. Oct 14, 2020
@Johannestegner

Copy link
Copy Markdown
Contributor Author

Reference: Issue with feature request can be found here aquasecurity/trivy#681

@simar7

simar7 commented Oct 16, 2020

Copy link
Copy Markdown

thanks for the PR @Johannestegner – I'll be taking a look at it and making comments as I go through it.

@simar7 simar7 self-requested a review October 16, 2020 20:01
Comment thread pkg/nuget/parse_testcase.go
Comment thread pkg/nuget/parse.go Outdated
Comment thread pkg/nuget/parse.go Outdated
Comment thread pkg/nuget/parse.go Outdated
Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
…ded).

Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
… a read not write operation).

Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
@Johannestegner

Copy link
Copy Markdown
Contributor Author

Thank you for the review @simar7 ! Fixed the issues you mentioned, let me know if there is anything else you wish me to fix! :)

@simar7 simar7 requested review from knqyf263 and simar7 October 19, 2020 22:08

@simar7 simar7 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm – @knqyf263 would you like to take a second look before we merge?

@Johannestegner

Copy link
Copy Markdown
Contributor Author

Not sure yet, but depending on how the version should be handled in Trivy (aquasecurity/trivy#686 (comment)) I guess that this part should use a similar approach to convert the version.
This could of course be done later on, but might want to do it in a one off :)

@Johannestegner

Copy link
Copy Markdown
Contributor Author

Actually, I guess this should not be used in the dependency parser, but rather when checking in Trivy or similar.
I will add a test case for legacy packages though, so that it's in the codebase.

Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>

@knqyf263 knqyf263 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

Comment thread pkg/nuget/parse.go Outdated
Comment thread pkg/nuget/parse.go
Comment thread pkg/nuget/parse.go Outdated
Comment thread pkg/nuget/parse.go Outdated
Comment thread pkg/nuget/parse.go Outdated
…(added comments on generation steps).

Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
…ally with package resolving.

Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
…get parser.

Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
…r sub-packages, as they are added in top-level as Transitive type.

Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
@Johannestegner

Copy link
Copy Markdown
Contributor Author

Let me know if you want me to squash some (or all) commits :)

@knqyf263

Copy link
Copy Markdown
Collaborator

I'll squash all commits when merging this PR, so you don't have to care about it. We should keep the history in the PR.

@knqyf263

Copy link
Copy Markdown
Collaborator

I left one more comment, but it looks almost good! Thank you, again!

…!), updated parse to again use version.

Signed-off-by: Johannes Tegnér <johannes@jitesoft.com>
@Johannestegner

Copy link
Copy Markdown
Contributor Author

I left one more comment, but it looks almost good! Thank you, again!

Good catch!

Updated the parser to use name + version again (tried with the code in an earlier note, but it didn't work out at all as I expected! hehe).
I removed the complex file/test and replaced it with a multi which contains targets for net4.0;netstandard2.0;netstandard1.0;net35;net2.0. Took me quite a while to find a package which required different dependencies!
Now I think it is working just as it should! :)

@knqyf263

Copy link
Copy Markdown
Collaborator

Thank you for adding multiple targets file! I've refactored a bit. It looks good to me now.

@knqyf263 knqyf263 merged commit 889d4a9 into aquasecurity:master Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants