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

[SCB-Bot] Upgraded trivy from 0.19.2 to 0.20.2 #777

Merged
merged 12 commits into from
Nov 9, 2021

Conversation

github-actions[bot]
Copy link

This is an automated Pull Request by the SCB-Bot. It upgrades trivy from 0.19.2 to 0.20.2

Signed-off-by: secureCodeBoxBot <securecodebox@iteratec.com>
@github-actions github-actions bot added dependencies Pull requests that update a dependency file scanner Implement or update a security scanner labels Oct 26, 2021
@Ilyesbdlala Ilyesbdlala closed this Nov 2, 2021
@Ilyesbdlala Ilyesbdlala reopened this Nov 2, 2021
Ilyesbdlala and others added 2 commits November 3, 2021 12:59
Signed-off-by: Ilyes Ben Dlala <ilyes.bendlala@iteratec.com>
Signed-off-by: GitHub Actions <securecodebox@iteratec.com>
@Ilyesbdlala
Copy link
Member

Changes have to be made to trivy's parser.js. This is due to the new json schema : Info here.

@malexmave
Copy link
Member

Thanks for the heads up. I'll give this a shot, probably tomorrow, if no one else wants to claim it first.

@malexmave malexmave self-assigned this Nov 3, 2021
@malexmave malexmave added this to In progress in secureCodeBox v3 via automation Nov 3, 2021
Signed-off-by: Max Maass <max.maass@iteratec.com>
Signed-off-by: Max Maass <max.maass@iteratec.com>
@malexmave
Copy link
Member

While rewriting the parser, I found that the location for each of the found vulnerabilities is set to the name of the scanned image. The individual Result objects also contain a Target that point to the exact place where the issue is (e.g., a specific dependency file). To maintain the same output format as before the change, I have not changed this - should I make the change / add a new key to the results? My preferred solution would be to use the location key to designate the exact place the issue was found (the Target from the result object) and then also include a scan_target or something similar that contains the ArtifactName, but that would be a breaking change for anyone who depends on our findings...

I have also removed most of the test cases since they all seem to test the same (a scan of a docker image) and have instead introduced a result from a repository scan. Was there any specific reason why there were so many similar test case? If so, I can bring them back - it just means re-running all of the analyses for the example cases and putting the JSONs in the repo + updating the snapshots.

Signed-off-by: Max Maass <max.maass@iteratec.com>
Signed-off-by: Max Maass <max.maass@iteratec.com>
malexmave and others added 4 commits November 8, 2021 09:50
Signed-off-by: Max Maass <max.maass@iteratec.com>
Signed-off-by: GitHub Actions <securecodebox@iteratec.com>
Signed-off-by: Max Maass <max.maass@iteratec.com>
Signed-off-by: GitHub Actions <securecodebox@iteratec.com>
@malexmave
Copy link
Member

malexmave commented Nov 8, 2021

I have added two more things to the documentation: the extra parameters required to scan things other than docker images (#796) and some information on using the client-server mode to avoid issues with GitHub rate limiting on rule downloads. With this, I consider the PR ready for review and merge, aside from the questions raised above about the output format and unit tests.

@Ilyesbdlala
Copy link
Member

While rewriting the parser, I found that the location for each of the found vulnerabilities is set to the name of the scanned image. The individual Result objects also contain a Target that point to the exact place where the issue is (e.g., a specific dependency file). To maintain the same output format as before the change, I have not changed this - should I make the change / add a new key to the results? My preferred solution would be to use the location key to designate the exact place the issue was found (the Target from the result object) and then also include a scan_target or something similar that contains the ArtifactName, but that would be a breaking change for anyone who depends on our findings...

I think a custom attribute as defined here and/or including it in the description field are more appropriate for the ArtifactName variable. The location attribute is a required field in the finding, and it doesn't make sense to make it something scanner-specific (artifact/dependency name) or something that not all scanners can provide (the exact place where the issue is). I always thought of the location attribute as an indication of what the finding pertains to. And then the user can then inspect the finding to find more information in the other attributes (including the custom ones)
But having a non-standarized location field is a known issue (#619) that we decided not to address for lack of feasible solutions.

Pertaining to #796, I agree. I think a documentation warning and adding some repo scans examples are the best we can do for now. It's a Trivy related issue. Silently ignoring parameters seems like something that will always cause issues.

Signed-off-by: Max Maass <max.maass@iteratec.com>
@malexmave
Copy link
Member

I have added the extra metadata, so this should be ready to review and merge from my side.

secureCodeBox v3 automation moved this from In progress to Reviewer approved Nov 9, 2021
@Ilyesbdlala Ilyesbdlala merged commit ac9192c into main Nov 9, 2021
secureCodeBox v3 automation moved this from Reviewer approved to Done Nov 9, 2021
@Ilyesbdlala Ilyesbdlala deleted the dependencies/upgrading-trivy-to-0.20.2 branch November 9, 2021 14:55
@J12934 J12934 moved this from Done to counter in secureCodeBox v3 Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file scanner Implement or update a security scanner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants