chore: Run trivy scan on git repository and update version #2169

JAORMX · 2022-07-15T07:59:03Z

What this PR does / why we need it:

This uses trivy's filesystem scan [1] to scan the Gatekeeper repository
for package vulnerabilities, hardcoded secrets and config file issues
(e.g. k8se manifests or other IaC manifests).

While doing this, I also updates the trivy version that's being used in
the CI job.

[1] https://aquasecurity.github.io/trivy/v0.29.2/docs/vulnerability/scanning/filesystem/

codecov-commenter · 2022-07-15T13:59:29Z

Codecov Report

Merging #2169 (29bfc5e) into master (6f665b4) will decrease coverage by 0.19%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2169      +/-   ##
==========================================
- Coverage   54.50%   54.30%   -0.20%     
==========================================
  Files         111      111              
  Lines        9529     9529              
==========================================
- Hits         5194     5175      -19     
- Misses       3940     3954      +14     
- Partials      395      400       +5

Flag	Coverage Δ
unittests	`54.30% <ø> (-0.20%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...onstrainttemplate/constrainttemplate_controller.go	`55.15% <0.00%> (-4.56%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f665b4...29bfc5e. Read the comment docs.

sozercan · 2022-07-15T17:45:55Z

.github/workflows/workflow.yaml

+
+      - name: Run trivy on git repository
+        run: |
+          trivy fs --format table --security-checks vuln,config,secret .


It looks like this shows a whole bunch of errors for tests, kustomize files, etc. can we exclude those? we'll probably want to run with --exit-code=1 and --ignore-unfixed too (if it applies)

Thanks for the PR @JAORMX!
Are there other files we want to exclude? e.g. vendor?

@sozercan I updated the PR with the suggestions.
@ritazh I'll look into that next.

@ritazh it seems that this is already ignoring the vendor directory to some extent. So I don't think setting up extensive ignore rules is needed.

However, in general, it seems this actually caught some issues.

@JAORMX yarn issues are due to static docs website, they are known but not fixable at this time and obviously don't affect Gatekeeper. is there a setting to ignore those?

Thanks for looking into this @JAORMX!

@ritazh it seems that this is already ignoring the vendor directory to some extent. So I don't think setting up extensive ignore rules is needed.

Which setting ignores the vendor directory?

However, in general, it seems this actually caught some issues.

Can you give an example of this?

@ritazh there is no specific setting ignoring the vendor directory; however, this is not an issue as the vulnerability graph would either way take into account the dependencies' vulnerabilities even if there was no vendor directory. I think it's relevant that we still include it as we're only scanning for library vulnerabilities.

The build is failing and you can view the issues from the logs. Here's what's being reported:

go.mod (gomod) ============== Total: 2 (UNKNOWN: 0, LOW: 0, MEDIUM: 1, HIGH: 0, CRITICAL: 1) ┌──────────────────────────────────┬───────────────┬──────────┬─────────────────────┬───────────────┬──────────────────────────────────────────────────────────────┐ │ Library │ Vulnerability │ Severity │ Installed Version │ Fixed Version │ Title │ ├──────────────────────────────────┼───────────────┼──────────┼─────────────────────┼───────────────┼──────────────────────────────────────────────────────────────┤ │ github.com/emicklei/go-restful │ CVE-2022-[19](https://github.com/open-policy-agent/gatekeeper/runs/7395657753?check_suite_focus=true#step:4:20)96 │ CRITICAL │ 2.15.0+incompatible │ v3.8.0 │ go-restful: Authorization Bypass Through User-Controlled Key │ │ │ │ │ │ │ https://avd.aquasec.com/nvd/cve-[20](https://github.com/open-policy-agent/gatekeeper/runs/7395657753?check_suite_focus=true#step:4:21)[22](https://github.com/open-policy-agent/gatekeeper/runs/7395657753?check_suite_focus=true#step:4:23)-1996 │ ├──────────────────────────────────┼───────────────┼──────────┼─────────────────────┼───────────────┼──────────────────────────────────────────────────────────────┤ │ github.com/prometheus/prometheus │ CVE-2019-3826 │ MEDIUM │ 0.35.0 │ v2.7.1 │ prometheus: Stored DOM cross-site scripting (XSS) attack via │ │ │ │ │ │ │ crafted URL │ │ │ │ │ │ │ https://avd.aquasec.com/nvd/cve-2019-38[26](https://github.com/open-policy-agent/gatekeeper/runs/7395657753?check_suite_focus=true#step:4:27) │ └──────────────────────────────────┴───────────────┴──────────┴─────────────────────┴───────────────┴──────────────────────────────────────────────────────────────┘ website/yarn.lock (yarn) ======================== Total: 2 (UNKNOWN: 0, LOW: 0, MEDIUM: 1, HIGH: 1, CRITICAL: 0) ┌─────────┬────────────────┬──────────┬───────────────────┬────────────────┬──────────────────────────────────────────────────────────────┐ │ Library │ Vulnerability │ Severity │ Installed Version │ Fixed Version │ Title │ ├─────────┼────────────────┼──────────┼───────────────────┼────────────────┼──────────────────────────────────────────────────────────────┤ │ got │ CVE-2022-[33](https://github.com/open-policy-agent/gatekeeper/runs/7395657753?check_suite_focus=true#step:4:34)987 │ MEDIUM │ 9.6.0 │ 11.8.5, 12.1.0 │ got: missing verification of requested URLs allows redirects │ │ │ │ │ │ │ to UNIX sockets │ │ │ │ │ │ │ https://avd.aquasec.com/nvd/cve-2022-33987 │ ├─────────┼────────────────┼──────────┼───────────────────┼────────────────┼──────────────────────────────────────────────────────────────┤ │ trim │ CVE-2020-7753 │ HIGH │ 0.0.1 │ 0.0.3 │ Regular Expression Denial of Service in trim │ │ │ │ │ │ │ https://avd.aquasec.com/nvd/cve-2020-7753 │ └─────────┴────────────────┴──────────┴───────────────────┴────────────────┴──────────────────────────────────────────────────────────────┘

ritazh · 2022-07-20T15:04:48Z

Thank you for the updates @JAORMX!

One thought for the maintainers: do we want to run scan_vulnerabilities for every PR merging into main? One concern is contributors might not know how to fix the issue such as update the dependencies that are vulnerable, which could be a bad contributing experience, blocking PRs. Perhaps we should just run this PRs merging into release branches? that way we can fix the vulnerabilities before cutting every release. WDYT? @sozercan @maxsmythe

JAORMX · 2022-07-20T15:06:04Z

@ritazh the concern I have with only doing scans before releases is that it's very easy to forget to check and just have failing scans. I've seen a lot more success by scanning each PR.

ritazh · 2022-07-20T15:14:28Z

@ritazh the concern I have with only doing scans before releases is that it's very easy to forget to check and just have failing scans. I've seen a lot more success by scanning each PR.

Thanks @JAORMX! From your experience, how do most maintainers treat the failed vulnerabilities job for PRs? Merge PRs with a failed job or have the PRs blocked until the vulnerability is addressed in a separate PR?

JAORMX · 2022-07-20T15:36:36Z

@ritazh the concern I have with only doing scans before releases is that it's very easy to forget to check and just have failing scans. I've seen a lot more success by scanning each PR.

Thanks @JAORMX! From your experience, how do most maintainers treat the failed vulnerabilities job for PRs? Merge PRs with a failed job or have the PRs blocked until the vulnerability is addressed in a separate PR?

It depends on the vulnerability. Since we're ignoring unfixed CVEs, normally it's not too difficult to fix the issue. You'd normally just update a dependency and that's it. So the disruption is not as visible. e.g. patches wouldn't be blocked for too long. But yeah, CI would normally be green. There is also the possibility to ignore vulnerabilities via a .trivyignore file, so that's a possibility too.

sozercan · 2022-07-20T16:26:27Z

@JAORMX can you add yarn ones to ignore list? they are not fixable and do not affect Gatekeeper facebook/docusaurus#6394 (comment)

ritazh · 2022-07-20T21:06:29Z

As discussed in the community call today, let's remove the exit-code so that the failure is not PR blocking for now. Then as a follow up PR we can leverage something like https://github.com/marketplace/actions/create-or-update-comment to add the vulnerability report as a comment similar to the codecov report to provide signals to the maintainers while reduce burden on the PR owner/contributor.

JAORMX · 2022-07-21T07:05:20Z

@ritazh @sozercan done. Thanks for the feedback!

sozercan

Thanks! LGTM

.github/workflows/workflow.yaml

This uses trivy's filesystem scan [1] to scan the Gatekeeper repository for package vulnerabilities. While doing this, I also updates the trivy version that's being used in the CI job. [1] https://aquasecurity.github.io/trivy/v0.29.2/docs/vulnerability/scanning/filesystem/ Signed-off-by: Juan Antonio Osorio <[email protected]>

ritazh

LGTM

maxsmythe

LGTM

sozercan · 2022-07-25T21:40:47Z

FYI - I looked at the reported vulnerabilities

│ github.com/emicklei/go-restful │ CVE-2022-1996 │ CRITICAL │ v2.15.0+incompatible │ v3.8.0 │ go-restful: Authorization Bypass Through User-Controlled Key │

Gatekeeper has this because of indirect reference to Kubernetes libraries. Kubernetes does not use the impacted feature. Alert will be resolved when libraries are bumped to 1.25. kubernetes/kubernetes#110518

│ github.com/prometheus/prometheus │ CVE-2019-3826 │ MEDIUM │ v0.35.0 │ v2.7.1 │ prometheus: Stored DOM cross-site scripting (XSS) attack via │

This one is a false positive. The version we use (v0.35.0) is significantly newer than v2.7.1. It corresponds to Prometheus v2.35.0 due to Go versioning.

ritazh · 2022-07-26T00:16:16Z

How do we want to track these going forward? Do we want to create an issue for each and close them when it's addressed?

JAORMX · 2022-07-26T06:15:27Z

@ritazh I'm in the gatekeeper slack channel in case you want to bring this up with the rest of the community.

JAORMX changed the title ~~Run trivy scan on git repository and update version~~ chore: Run trivy scan on git repository and update version Jul 15, 2022

sozercan reviewed Jul 15, 2022

View reviewed changes

JAORMX force-pushed the trivy-update-fs branch 2 times, most recently from 62bb540 to f636582 Compare July 18, 2022 17:10

JAORMX requested review from ritazh and sozercan July 20, 2022 11:45

JAORMX force-pushed the trivy-update-fs branch from f636582 to 836e75f Compare July 21, 2022 07:04

sozercan approved these changes Jul 21, 2022

View reviewed changes

sozercan reviewed Jul 21, 2022

View reviewed changes

.github/workflows/workflow.yaml Show resolved Hide resolved

JAORMX force-pushed the trivy-update-fs branch from 836e75f to b282a86 Compare July 22, 2022 07:54

ritazh approved these changes Jul 22, 2022

View reviewed changes

Merge branch 'master' into trivy-update-fs

29bfc5e

maxsmythe approved these changes Jul 23, 2022

View reviewed changes

sozercan merged commit 8f1ef8c into open-policy-agent:master Jul 23, 2022

sozercan mentioned this pull request Jul 26, 2022

chore: bump controller-runtime to v0.13.0 eraser-dev/eraser#333

Merged

ritazh mentioned this pull request Jun 30, 2023

Add trivy scan on git repository and on eraser images eraser-dev/eraser#781

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Run trivy scan on git repository and update version #2169

chore: Run trivy scan on git repository and update version #2169

JAORMX commented Jul 15, 2022

codecov-commenter commented Jul 15, 2022 •

edited

Loading

sozercan Jul 15, 2022 •

edited

Loading

ritazh Jul 15, 2022

JAORMX Jul 18, 2022

JAORMX Jul 19, 2022

sozercan Jul 19, 2022

ritazh Jul 19, 2022

JAORMX Jul 20, 2022

ritazh commented Jul 20, 2022

JAORMX commented Jul 20, 2022

ritazh commented Jul 20, 2022

JAORMX commented Jul 20, 2022

sozercan commented Jul 20, 2022

ritazh commented Jul 20, 2022

JAORMX commented Jul 21, 2022

sozercan left a comment

ritazh left a comment

maxsmythe left a comment

sozercan commented Jul 25, 2022 •

edited

Loading

ritazh commented Jul 26, 2022

JAORMX commented Jul 26, 2022

chore: Run trivy scan on git repository and update version #2169

chore: Run trivy scan on git repository and update version #2169

Conversation

JAORMX commented Jul 15, 2022

codecov-commenter commented Jul 15, 2022 • edited Loading

Codecov Report

sozercan Jul 15, 2022 • edited Loading

Choose a reason for hiding this comment

ritazh Jul 15, 2022

Choose a reason for hiding this comment

JAORMX Jul 18, 2022

Choose a reason for hiding this comment

JAORMX Jul 19, 2022

Choose a reason for hiding this comment

sozercan Jul 19, 2022

Choose a reason for hiding this comment

ritazh Jul 19, 2022

Choose a reason for hiding this comment

JAORMX Jul 20, 2022

Choose a reason for hiding this comment

ritazh commented Jul 20, 2022

JAORMX commented Jul 20, 2022

ritazh commented Jul 20, 2022

JAORMX commented Jul 20, 2022

sozercan commented Jul 20, 2022

ritazh commented Jul 20, 2022

JAORMX commented Jul 21, 2022

sozercan left a comment

Choose a reason for hiding this comment

ritazh left a comment

Choose a reason for hiding this comment

maxsmythe left a comment

Choose a reason for hiding this comment

sozercan commented Jul 25, 2022 • edited Loading

ritazh commented Jul 26, 2022

JAORMX commented Jul 26, 2022

codecov-commenter commented Jul 15, 2022 •

edited

Loading

sozercan Jul 15, 2022 •

edited

Loading

sozercan commented Jul 25, 2022 •

edited

Loading