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

Analyzer support for git-based dependencies #3067

Open
nejch opened this issue Oct 22, 2022 · 10 comments
Open

Analyzer support for git-based dependencies #3067

nejch opened this issue Oct 22, 2022 · 10 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@nejch
Copy link

nejch commented Oct 22, 2022

Hey people, I'd like to add support for a few types of analyzers and parsers that all intersect a bit, so would like to get some feedback first.

My use case is mostly for SBOM creation, but could be useful for vulnerabilities as well. Imagine a GitLab project that builds in CI in a Debian container using image:, uses an external pipeline template using include:, pulls in a boost library via a submodule and a yocto layer via kas repo:. Silly example, but I know projects covering 3 out of 4 here in a single repo. It could also be used to detect tokens committed to git URLs accidentally perhaps.


I have a rough but working draft including some testing for the first three, but before opening PRs I would like to clarify a few things

First, for kas, gitlab-ci includes, and gitlab-ci docker images, we are parsing a yaml file, so this could IMO be done in go-dep-parser. As they are technically not a "language" parser, would they need special namespacing like frameworks/wordpress? Maybe something like this:

github.com/aquasecurity/go-dep-parser/pkg/gitlab/include
github.com/aquasecurity/go-dep-parser/pkg/gitlab/image
github.com/aquasecurity/go-dep-parser/pkg/git/kas

For git submodules, since we're not analyzing a file but a command (well, via go-git), I think the parser would need to stay in trivy itself, like for example, apk packages. Also, trivy already has go-git as a dependency. How would the analyzers then be placed? Maybe something like:

github.com/aquasecurity/trivy/pkg/fanal/analyzer/git/kas
github.com/aquasecurity/trivy/pkg/fanal/analyzer/git/submodule
github.com/aquasecurity/trivy/pkg/fanal/analyzer/gitlab/include
github.com/aquasecurity/trivy/pkg/fanal/analyzer/gitlab/image

I'm not sure if this is the best way, especially for image: as it might be similar to future analyzers that use the same data source (#1984), so /docker/ might make more sense for that. Just want to start somewhere. Some other managers also take git deps (e.g. go, ansible-galaxy,...) so it's all a bit of gray zone. 😁

Ok this is a bit of a wall of text - but thanks in advance for any feedback! ❤️ I'll start opening draft PRs referencing this issue in the coming days for consistency.

Edit: I just noticed #3019 by @knqyf263 might also be relevant here as it deals with another kind of generic type of dependency.

@nejch nejch added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 22, 2022
@nejch
Copy link
Author

nejch commented Nov 17, 2022

Sorry for the ping again here, just wondering if these types of analyzers is something you'd consider for core trivy, or if you think it's better to have them as some kind of community plugins? :)

@nejch nejch changed the title Add analyzer (+parser) support for git-based dependencies Add analyzer/parser support for git-based dependencies Dec 28, 2022
@nejch nejch changed the title Add analyzer/parser support for git-based dependencies Add analyzer support for git-based dependencies Dec 28, 2022
@nejch nejch changed the title Add analyzer support for git-based dependencies Analyzer support for git-based dependencies Dec 28, 2022
@sisp
Copy link

sisp commented Feb 21, 2023

Also related: #3662

@github-actions
Copy link

This issue is stale because it has been labeled with inactivity.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label Apr 24, 2023
@nejch
Copy link
Author

nejch commented Apr 29, 2023

This issue is stale because it has been labeled with inactivity.

Still relevant but let's wait and see what the outcome of #3345 (comment) is eventually.

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label May 13, 2023
@github-actions
Copy link

This issue is stale because it has been labeled with inactivity.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label Jul 12, 2023
@nejch
Copy link
Author

nejch commented Jul 12, 2023

This issue is stale because it has been labeled with inactivity.

For anyone following, I'll probably try to convert the existing PRs into a community plugin when I find some time, as they are unlikely to make it into upstream repos I think from what I can tell.

@itaysk
Copy link
Contributor

itaysk commented Jul 12, 2023

@nejch I think this is something that we will be happy to see incorporated in Trivy eventually (at least the git submodule). I realize this has been dragging along for too long, I'll try to see if we can give this more attention in the next release

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label Jul 13, 2023
@itaysk
Copy link
Contributor

itaysk commented Jul 16, 2023

I'd like to clarify your use case if you don't mind. I'm assuming you use Trivy to find vulnerabilities in code (C code by the sound of it). But even if Trivy detects a git submodule dependency (as suggested here), it won't detect a vulnerability in it. because:

  1. the package needs to be classified under the relevant programming language (for example C). This is technically solvable but still a challenge.
  2. there will be no info in Trivy DB on this package (the ID won't match anything).

@nejch
Copy link
Author

nejch commented Jul 19, 2023

  1. the package needs to be classified under the relevant programming language (for example C). This is technically solvable but still a challenge.
  2. there will be no info in Trivy DB on this package (the ID won't match anything).

Thanks a lot for the response @itaysk! As I might have mentioned above or in the PR, I see value already in being able to produce a standard CycloneDX SBOM with these generic dependencies, even if trivy itself is not aware of vulnerabilities associated with them.

Just like trivy itself can analyze an SBOM, other tools can provide their own analyses potentially. But even just including them is a win for me as it would increase visibility in the software supply chain.

Rather than trying to classify them as a language (it might be mixed in a submodule), I'd treat them more like the existing non-packaged deps in https://github.com/aquasecurity/trivy/blob/main/pkg/fanal/analyzer/const.go#L93. Does that make sense?

There are other analyzers that currently also only support SBOM generation - see https://github.com/aquasecurity/trivy/blob/main/pkg/detector/library/driver.go#L63.

@itaysk
Copy link
Contributor

itaysk commented Aug 14, 2023

I think it makes sense. to treat git submodules as dependencies. I'll wait for @knqyf263 for his feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants