Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add initial dotnet-support #951

Merged
merged 8 commits into from
May 5, 2022
Merged

feat: add initial dotnet-support #951

merged 8 commits into from
May 5, 2022

Conversation

ckotzbauer
Copy link
Contributor

Signed-off-by: Christian Kotzbauer [email protected]

This PR adds initial support for .NET (Core). It focuses on the parsing of the *.deps.json files in the bin/ folders of a compiled project. This does not add support for parsing NuGet-related files which are usually only available at source-level.

I tested the changes against a compiled .NET project and against a ASP.NET Core image with the compiled project inside.

This changes are done alongside the PR adding dart-support. Please let me know, if anything is missing, wrong or should be changed.

Fixes #726
Ref #373

Signed-off-by: Christian Kotzbauer <[email protected]>
Copy link
Contributor Author

@ckotzbauer ckotzbauer left a comment

Choose a reason for hiding this comment

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

I will open a PR against the package-url/packageurl-go repo when the namings are approved here.

syft/pkg/dotnet_deps_metadata.go Outdated Show resolved Hide resolved
syft/pkg/language.go Outdated Show resolved Hide resolved
syft/pkg/type.go Outdated Show resolved Hide resolved
syft/pkg/type.go Show resolved Hide resolved
@luhring
Copy link
Contributor

luhring commented Apr 26, 2022

Thanks so much, @ckotzbauer! This would be excellent to add in!

@wagoodman PTAL this week when you're able. 😃

Signed-off-by: Christian Kotzbauer <[email protected]>
Signed-off-by: Christian Kotzbauer <[email protected]>
@wagoodman
Copy link
Contributor

re: packageurl-go updates, we use a fork of package-url/packageurl-go for this reason -- so we can get the changes needed for syft in asap without waiting for upstream. We submit PRs to upstream as well, though the merge time can take a while.

Did you want to also create a PR in our fork of packageurl-go for the TypeDotnet update? (if not we can leave it as is too and get the cataloger in)

Copy link
Contributor

@wagoodman wagoodman left a comment

Choose a reason for hiding this comment

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

(not in scope for this PR, just capturing a comment for sometime in the future after this PR is merged)

We could capture *.runtimeconfig.json in the package metadata if it's available (would require significant cataloger changes).

@wagoodman
Copy link
Contributor

I think the linter issues can be fixed with make lint-fix

Signed-off-by: Christian Kotzbauer <[email protected]>
@ckotzbauer
Copy link
Contributor Author

ckotzbauer commented Apr 28, 2022

I think the linter issues can be fixed with make lint-fix

fixed

Is the CLI-Test failure related to these changes?

Did you want to also create a PR in our fork of packageurl-go for the TypeDotnet update?

Yes, I will do a PR against the fork. anchore/packageurl-go#8

@wagoodman
Copy link
Contributor

@ckotzbauer I merged the pURL PR, so you should be good to go there.

re: CLI tests, the failure seems related to:

    packages_cmd_test.go:236: expected package count of 20, but found 32
...

    packages_cmd_test.go:240: STDERR:
         [0000]  INFO could not identify distro
        [0000]  INFO cataloging image
        
    packages_cmd_test.go:241: COMMAND: /home/runner/work/syft/syft/snapshot/linux-build_linux_amd64/syft packages -o json -s squashed docker-archive:test-fixtures/cache/stereoscope-fixture-image-pkg-coverage-d5baa8a0e48b30716c65fda2fecff73c8a85bdc023917b7ccae9ae0903d6d94c.tar

Since the coverageImage now includes dotnet packages, the total number of packages expected has grown:

The easiest way to iterate locally on the CLI tests is:

  • make clean-snapshot snapshot to create a build
  • make cli to run tests against the build

I went ahead an fixed the test in question in 009dd0a, so no action needed 🎉

@wagoodman
Copy link
Contributor

Open question: though I've merged anchore/packageurl-go#8 I'm currently trying to make certain I understand the right usage of package URL for a .NET package relative to what a *.deps.json describes. As I understand it a .deps.json has nothing to do with nuget. However, in terms of pURL that might not matter. That is, the information contained within deps.json simply describe .NET packages, which canonically is nuget.

In short: should we be using the pURL type nuget instead of the proposed dotnet pURL type?

In thinking/reading through this more I think the answer is "yes" we should use "nuget", but wanted to raise this up for comment.

@ckotzbauer
Copy link
Contributor Author

Great, thanks for fixing the CLI test.

I merged the pURL PR, so you should be good to go there.

I will update this PR.

should we be using the pURL type nuget instead of the proposed dotnet pURL type?

No, I don't think so. Nuget is the most popular package-manager for dotnet, but there are alternatives (e.g. https://fsprojects.github.io/Paket/index.html). I always used Nuget before, but I think the result in deps.json would be the same with another tool, as there's no reference to "nuget" in this file.

Signed-off-by: Christian Kotzbauer <[email protected]>
@wagoodman
Copy link
Contributor

I think that all makes sense. I'll note that since the pURL type is non-standard that there is risk of needing to change this in the future, but I think for now we can operate under the assumption for now that a pURL that doesn't automatically couple .NET projects with nuget is the way to go in this instance.

Signed-off-by: Alex Goodman <[email protected]>
@wagoodman
Copy link
Contributor

Useful JSON schema diff for reviewers

# diff schema/json/schema-3.2.2.json  schema/json/schema-3.2.3.json
279a280,295
>     "DotnetDepsMetadata": {
>       "required": [
>         "name",
>         "version"
>       ],
>       "properties": {
>         "name": {
>           "type": "string"
>         },
>         "version": {
>           "type": "string"
>         }
>       },
>       "additionalProperties": true,
>       "type": "object"
>     },
520a537,542
>         },
>         "digest": {
>           "items": {
>             "$ref": "#/definitions/Digest"
>           },
>           "type": "array"
690a713,715
>               "$ref": "#/definitions/DotnetDepsMetadata"
>             },
>             {

Copy link
Contributor

@wagoodman wagoodman left a comment

Choose a reason for hiding this comment

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

cc: @anchore/tools
Looks like there are minor tweaks in grype testing needed -- I can confirm there is no apparent unexpected matching behavior with the stock matcher. We'll still need to add a .NET-specific matcher and DB namespace finder to link up GHSA matches in the future.

Overall, great work @ckotzbauer ! Thanks for taking the time on this ❤️

@ckotzbauer
Copy link
Contributor Author

Oh, just because you posted the schema diff: I extended the Metadata as part of the review, but I think I forgot to regenerate the schema.

Signed-off-by: Alex Goodman <[email protected]>
@wagoodman
Copy link
Contributor

Latest diff:

# diff schema/json/schema-3.2.2.json  schema/json/schema-3.2.3.json
279a280,307
>     "DotnetDepsMetadata": {
>       "required": [
>         "name",
>         "version",
>         "path",
>         "sha512",
>         "hashPath"
>       ],
>       "properties": {
>         "name": {
>           "type": "string"
>         },
>         "version": {
>           "type": "string"
>         },
>         "path": {
>           "type": "string"
>         },
>         "sha512": {
>           "type": "string"
>         },
>         "hashPath": {
>           "type": "string"
>         }
>       },
>       "additionalProperties": true,
>       "type": "object"
>     },
520a549,554
>         },
>         "digest": {
>           "items": {
>             "$ref": "#/definitions/Digest"
>           },
>           "type": "array"
690a725,727
>               "$ref": "#/definitions/DotnetDepsMetadata"
>             },
>             {

@wagoodman wagoodman merged commit 1cea0ec into anchore:main May 5, 2022
@ckotzbauer ckotzbauer deleted the feature/dotnet branch May 6, 2022 12:32
spiffcs added a commit that referenced this pull request May 6, 2022
* main:
  feat: add initial dotnet-support (#951)
  unblock timeout for power-user select CLI tests (#985)
  golang cataloger - main module version as is (#986)
  Fix `github-json` output option (#967)
  read Go main module version as is - (devel) (#981)
  reduce logging severity for non-Go binaries (#983)
  golang.org/x/crypto upgrade (#979)
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
* feat: add initial dotnet-support

Signed-off-by: Christian Kotzbauer <[email protected]>

* fix: add path, sha512 and hashpath

Signed-off-by: Christian Kotzbauer <[email protected]>

* fix: add missing dot

Signed-off-by: Christian Kotzbauer <[email protected]>

* fix: lint warnings

Signed-off-by: Christian Kotzbauer <[email protected]>

* fix CLI test package counts to account for dotnet

Signed-off-by: Alex Goodman <[email protected]>

* fix: updated packagurl-go

Signed-off-by: Christian Kotzbauer <[email protected]>

* tidy go.sum

Signed-off-by: Alex Goodman <[email protected]>

* update json schema

Signed-off-by: Alex Goodman <[email protected]>

Co-authored-by: Alex Goodman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

.NET Core-Support
3 participants