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
--vuln-type library returning null in JSON when no vulnerabilites are found #828
Comments
You specified |
Shouldn't specifying
However, it returns just
|
I see. It makes sense. Trivy probably should return a list of OS packages if |
Hi! 👋 I'm looking at this issue in the context of Hacktoberfest 2021. After spending some time looking at the source, it seems to be that this is more of a usability issue: the flag
And in fact, if we pass
And if you pass it an image with Application packages, it lists them correctly (but not the OS packages):
I believe this is somewhat the opposite of what was described here:
It is not All this being said, I'd like to propose 2 possible solutions:
Personally I'd prefer option 1, has it does not require adding yet another entry to Thanks for any input on how to proceed, @knqyf263 and @OwenMatsuda ! 🙇 |
Not even specific to
$ JQ_FILTER='.Results | map(.Vulnerabilities) | add | any(.VulnerabilityID == "CVE-2021-44228")'
$ trivy -q image -f json ghcr.io/christophetd/log4shell-vulnerable-app | jq "${JQ_FILTER}"
true
$ trivy -q image -f json alpine:3.13.6 | jq "${JQ_FILTER}"
false
$ trivy -q image -f json alpine:3.13.7 | jq "${JQ_FILTER}"
jq: error (at <stdin>:60): Cannot iterate over null (null)
$ trivy -q image -f json alpine:3.13.7 | jq .Results
[
{
"Target": "alpine:3.13.7 (alpine 3.13.7)",
"Class": "os-pkgs",
"Type": "alpine"
}
] In my opinion, the only reasonable way to represent the empty set of vulnerabilities in JSON is Unfortunately, fixing this may not be trivial due to Go's garbage type system, where diff --git a/pkg/detector/ospkg/alpine/alpine.go b/pkg/detector/ospkg/alpine/alpine.go
index d4b39b10..d8c89724 100644
--- a/pkg/detector/ospkg/alpine/alpine.go
+++ b/pkg/detector/ospkg/alpine/alpine.go
@@ -87,7 +87,8 @@ func (s *Scanner) Detect(osVer string, pkgs []ftypes.Package) ([]types.DetectedV
log.Logger.Debugf("alpine: os version: %s", osVer)
log.Logger.Debugf("alpine: the number of packages: %d", len(pkgs))
- var vulns []types.DetectedVulnerability
+ var vulns = []types.DetectedVulnerability{}
+
for _, pkg := range pkgs {
advisories, err := s.vs.Get(osVer, pkg.SrcName)
if err != nil {
diff --git a/pkg/report/writer.go b/pkg/report/writer.go
index 0e5769e0..ab55195e 100644
--- a/pkg/report/writer.go
+++ b/pkg/report/writer.go
@@ -58,7 +58,7 @@ type Result struct {
Class ResultClass `json:"Class,omitempty"`
Type string `json:"Type,omitempty"`
Packages []ftypes.Package `json:"Packages,omitempty"`
- Vulnerabilities []types.DetectedVulnerability `json:"Vulnerabilities,omitempty"`
+ Vulnerabilities []types.DetectedVulnerability `json:"Vulnerabilities"`
MisconfSummary *MisconfSummary `json:"MisconfSummary,omitempty"`
Misconfigurations []types.DetectedMisconfiguration `json:"Misconfigurations,omitempty"`
} the value is |
Hey, I was looking to contribute to the repository and this looks like a nice issue :)
I'm going work only on resolving the 3rd one as it was acknowledged. |
… configuration Resolves aquasecurity#828 The current workflow of the `scan` module is calling the `checkVulnerabilities` function with is vulnerabilities oriented. It checks each vulnerability type (currently library and OS) that it was provided with. In addition, in case `--list-all-pkgs` is specified, it also looks for each vulnerability type packages. This PR supports that when providing the `--list-all-pkgs` flag, it will look into every vulnerability type, no matter the `--vuln-type` flag that was provided. This behavior should require more design changes but for now, in order to keep this code backward compatible, this is a valid solution.
… configuration Resolves aquasecurity#828 The current workflow of the `scan` module is calling the `checkVulnerabilities` function with is vulnerabilities oriented. It checks each vulnerability type (currently library and OS) that it was provided with. In addition, in case `--list-all-pkgs` is specified, it also looks for each vulnerability type packages. This PR supports that when providing the `--list-all-pkgs` flag, it will look into every vulnerability type, no matter the `--vuln-type` flag that was provided. This behavior should require more design changes but for now, in order to keep this code backward compatible, this is a valid solution.
Hi @YuviGold, thanks for summarizing. After some consideration, I think the name of the option
If So, I think we should fix #1. To keep simplicity, we should not support #3. I'm sorry to confuse you, @YuviGold. We have updated the JSON schema, so it no longer returns null. If null |
I'll start from the bottom - regarding the last point, you are right. Regarding the WDYT? @knqyf263 |
|
Honestly In the spirit of renaming flags, would you like to rename |
Trivy supports misconfigurations as well as vulnerabilities. I'm concerned that Trivy supports two types of packages now.
I have summarized the expected behavior.
Also, "all" in |
Thank you for the elaborative explanation! I think that whenever a user wants to check an image its either to scan it or to list it. I don't see why to do both, it would just print a lot of data and could mislead what actually is vulnerable. In that case, it might be more trivial to have a subcommand for In addition, you mentioned misconfiguration as a security check. I think that these two different levels are a bit confusing...
What about flattening it to be a 1 level security-check/scan-target? I believe misconfiguration is also an exploit/vulnerability and this distinction causes part of the confusion |
This will be a breaking change. I want to keep the command as is because Trivy is a vulnerability scanner and the package listing feature should be done in conjunction with vulnerability detection.
We've never heard of the confusion due to vulnerabilities/misconfigurations so far. Actually, vulnerabilities are patch-related things. In a general sense, misconfiguration is also a vulnerability as you said, but in the IT industry, I think they are recognized as different things. |
I see.
@knqyf263 Yes, I think |
Resolves aquasecurity#828 In order to avoid confusion on the effect of `--list-all-pkgs` flag, rename the flag to solely `--list-pkgs` which will result in listing the packages alongside scanning for vulnerabilities according to the given behavior. In essence, - | OS package list | OS package vulnerabilities | Langueage-specific package list | Language-specific vulnerabilities ------------------------------------- | --------------- | -------------------------- | ------------------------------- | --------------------------------- `--vuln-type os` | x | v | x | x `--vuln-type library` | x | x | x | v `--vuln-type os,library` | x | v | x | v `--vuln-type os --list-pkgs` | v | v | x | x `--vuln-type library --list-pkgs` | x | x | v | v `--vuln-type os,library --list-pkgs` | v | v | v | v Signed-off-by: Yuval Goldberg <yuvigoldi@gmail.com>
Resolves aquasecurity#828 In order to avoid confusion on the effect of `--list-all-pkgs` flag, rename the flag to solely `--list-pkgs` which will result in listing the packages alongside scanning for vulnerabilities according to the given behavior. In essence, - | OS package list | OS package vulnerabilities | Langueage-specific package list | Language-specific vulnerabilities ------------------------------------- | --------------- | -------------------------- | ------------------------------- | --------------------------------- `--vuln-type os` | x | v | x | x `--vuln-type library` | x | x | x | v `--vuln-type os,library` | x | v | x | v `--vuln-type os --list-pkgs` | v | v | x | x `--vuln-type library --list-pkgs` | x | x | v | v `--vuln-type os,library --list-pkgs` | v | v | v | v Signed-off-by: Yuval Goldberg <yuvigoldi@gmail.com>
Resolves aquasecurity#828 In order to avoid confusion on the effect of `--list-all-pkgs` flag, rename the flag to solely `--list-pkgs` which will result in listing the packages alongside scanning for vulnerabilities according to the given behavior. In essence, - | OS package list | OS package vulnerabilities | Langueage-specific package list | Language-specific vulnerabilities ------------------------------------- | --------------- | -------------------------- | ------------------------------- | --------------------------------- `--vuln-type os` | x | v | x | x `--vuln-type library` | x | x | x | v `--vuln-type os,library` | x | v | x | v `--vuln-type os --list-pkgs` | v | v | x | x `--vuln-type library --list-pkgs` | x | x | v | v `--vuln-type os,library --list-pkgs` | v | v | v | v Signed-off-by: Yuval Goldberg <yuvigoldi@gmail.com>
Resolves aquasecurity#828 In order to avoid confusion on the effect of `--list-all-pkgs` flag, rename the flag to solely `--list-pkgs` which will result in listing the packages alongside scanning for vulnerabilities according to the given behavior. In essence, - | OS package list | OS package vulnerabilities | Langueage-specific package list | Language-specific vulnerabilities ------------------------------------- | --------------- | -------------------------- | ------------------------------- | --------------------------------- `--vuln-type os` | x | v | x | x `--vuln-type library` | x | x | x | v `--vuln-type os,library` | x | v | x | v `--vuln-type os --list-pkgs` | v | v | x | x `--vuln-type library --list-pkgs` | x | x | v | v `--vuln-type os,library --list-pkgs` | v | v | v | v Signed-off-by: Yuval Goldberg <yuvigoldi@gmail.com>
Resolves aquasecurity#828 In order to avoid confusion on the effect of `--list-all-pkgs` flag, rename the flag to solely `--list-pkgs` which will result in listing the packages alongside scanning for vulnerabilities according to the given behavior. In essence, - | OS package list | OS package vulnerabilities | Langueage-specific package list | Language-specific vulnerabilities ------------------------------------- | --------------- | -------------------------- | ------------------------------- | --------------------------------- `--vuln-type os` | x | v | x | x `--vuln-type library` | x | x | x | v `--vuln-type os,library` | x | v | x | v `--vuln-type os --list-pkgs` | v | v | x | x `--vuln-type library --list-pkgs` | x | x | v | v `--vuln-type os,library --list-pkgs` | v | v | v | v Signed-off-by: Yuval Goldberg <yuvigoldi@gmail.com>
Description
Command running:
trivy image --list-all-pkgs --vuln-type library -f json debian:10.6
Option
--vuln-type library
returns null in JSON when no vulnerabilities are found, even if--list-all-pkgs
is also present.What did you expect to happen?
I would expect it to return in JSON the
Target
,Type
, the list ofPackages
, and an empty list ofVulnerabilities
.What happened instead?
It simply returns
null
.Output of run with
-debug
:Output of
trivy -v
:Additional details (base image name, container registry info...):
Just tested using
Debian:10.6
.The text was updated successfully, but these errors were encountered: