Add a new workflow to check vulnerabilities using trivy #9044
Conversation
gortiz
force-pushed
the
add_vuln_check
branch
13 times, most recently
from
1fda43b
to
a865eed
Compare
gortiz
changed the title
gortiz
force-pushed
the
add_vuln_check
branch
4 times, most recently
from
63c19ee
to
684a681
Compare
Codecov Report
@@ Coverage Diff @@
## master #9044 +/- ##
============================================
+ Coverage 68.96% 70.11% +1.14%
- Complexity 4727 4743 +16
============================================
Files 1831 1831
Lines 96368 96382 +14
Branches 14411 14408 -3
============================================
+ Hits 66462 67576 +1114
+ Misses 25271 24141 -1130
- Partials 4635 4665 +30
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
gortiz
force-pushed
the
add_vuln_check
branch
11 times, most recently
from
4f64534
to
78c8da9
Compare
| paths-ignore: | ||
| - "contrib/**" | ||
| - "docs/**" | ||
| - "licenses/**" | ||
| - "licenses-binary/**" | ||
| - "**.md" |
There was a problem hiding this comment.
instead of ignore path, let's explicitly run this when **/pom.xml is changed.
There was a problem hiding this comment.
That is a very good idea. We also have to add some extra files like the package.json files or dockerfiles, but it totally makes sense
| for tag in "${tags[@]}" | ||
| do | ||
| echo "Plan to build docker images for: ${DOCKER_IMAGE_NAME}:${tag}" | ||
| DOCKER_BUILD_TAGS+=" --tag ${DOCKER_IMAGE_NAME}:${tag} " | ||
| done |
There was a problem hiding this comment.
is there a reason we have to add multiple docker build tags?
There was a problem hiding this comment.
This has been copied from .pinot_docker_image_build_and_push.sh. In the workflow I always use a single tag (the commit sha)
| vuln-type: 'os,library' | ||
| severity: 'CRITICAL,HIGH' | ||
| - name: Upload Trivy scan results to GitHub Security tab | ||
| uses: github/codeql-action/upload-sarif@v2 |
There was a problem hiding this comment.
is there an option to not create a failed GHA task but instead comment on the PR like the codecov bot?
There was a problem hiding this comment.
I didn't find it. But the task compares the vulns in source and target branch. If no new vulnerability is added, it should be empty even if we have vulns in master. At least that is what I infer from the code scanning result.
I would recommend to merge the PR as it is to make it clear there are some vulnerabilities. We can add to .trivyignore the once we don't plan to fix soon.
There was a problem hiding this comment.
i see. so if I understand correctly:
- the reason why this PR has so many vulns is b/c current master doesn't have a baseline vulnerability SARIF result.
- once merged, the first commit on master will have a large SARIF but the next one should be empty if it didn't modify any dependencies. correct?
walterddr
force-pushed
the
add_vuln_check
branch
from
4d0f4bd
to
577cbd9
Compare
gortiz
changed the title
|
I think this is ready to merge |
* Add a new workflow to check vulnerabilities using trivy * Add EOF new line * Run Pinot dependencies check only when dependency related files change
This PR adds a new workflow that executes Trivy in order to look for vulnerabilities. Every PR will trigger that workflow and it will add a new check that will include all known vulnerabilities added in the PR.
Some vulnerabilities may do not have a fix yet or it may not be possible to update to the version that fixes the vulnerability (for example, due to some breaking change). In that case we add the vulnerabilities that have to be skipped in a new file called
.trivyignore. For example, see the Conjur repo.Trivy is an open source program with a huge vulnerability db that analyzes artifacts looking for vulnerabilities. Although Trivy can be used in different ways, the most common way to use it is to analyze a docker image. By doing that it can analyze the code dependencies (for example, jars) but also SO dependencies (like the zlib version that is used). Trivy can also be used to analyze infrastructure as code and other configs, but I don't have experience doing so.