First attempt using aquasecurity/trivy-action to scan for CVEs during build. #3461
Conversation
mitchell-as
force-pushed
the
mitchell/dx-2978-2
branch
from
f256e95 to
dd9b717
Compare
mitchell-as
force-pushed
the
mitchell/dx-2978-2
branch
from
dd9b717 to
cab0b0b
Compare
.github/workflows/build.yml
Outdated
| @@ -214,6 +214,17 @@ jobs: | |||
| shell: bash | |||
| run: parallelize results Build-Executor | |||
|
|
|||
| - # === Scan for CVEs (Linux only) === | |||
| name: Scan for CVEs | |||
| if: runner.os == 'Linux' | |||
There was a problem hiding this comment.
We will need to scan for all platforms. It's ok if this runs ON linux, so long as we verify all binaries for all platforms.
Additionally, we should only run this on release branches and the nightly I think, no point slowing down regular PRs.
We could add a new job that runs in between build.yml and release.yml. Because we are already transferring all platform bits from build to release.yml, so we could inject something in the middle there for CVE validation.
There was a problem hiding this comment.
Well, that's what I was going to say...
For additional information you can run go version -m <executable> on a Go binary to see that there are differences in the modules used. It doesn't appear that versions are different but the Windows build may include modules that Linux does not and vice versa.
It also appears that Trivy has releases for MacOS and Windows, if that helps: https://github.com/aquasecurity/trivy/releases
There was a problem hiding this comment.
Very well, I've added a new Scan action inbetween build and deploy that scans all platform-specific binaries in one go. I disagree that it should only run on release branches and nightlies. PRs should not introduce CVEs only for them to be caught later. We should be proactive. It took a whopping 5s to scan all generated binaries.
There was a problem hiding this comment.
@MDrakos did I hijack your review? 😅 Not my intention. When I reviewed this I was under the impression I was flagged, maybe I got my wires crossed.
| @@ -424,7 +435,7 @@ jobs: | |||
| name: Install Go | |||
| uses: actions/setup-go@v3 | |||
| with: | |||
| go-version: ${{ matrix.go-version }} | |||
| go-version: 1.22.x | |||
There was a problem hiding this comment.
Should update the matrix version instead.
There was a problem hiding this comment.
The matrix does not exist for this job. Our action linter caught this.
mitchell-as
force-pushed
the
mitchell/dx-2978-2
branch
from
cf10a5a to
6410f3e
Compare
| with: | ||
| scan-type: rootfs | ||
| scan-ref: build | ||
| ignore-unfixed: true |
There was a problem hiding this comment.
| ignore-unfixed: true | |
| ignore-unfixed: false |
If I understand this correctly true would only report it the first time.
There was a problem hiding this comment.
No, this will ignore any CVEs that do not have fixes. We currently have an unfixed CVE (archiver/v3 does not have a fix, so there's no safe version we can update to). Without this option, the check will fail and we will have to force PRs to merge.
There was a problem hiding this comment.
Ahh I see.. ok yeah that makes sense 👍
| - name: Scan for CVEs | ||
| if: runner.os == 'Linux' | ||
| uses: aquasecurity/[email protected] | ||
| with: |
There was a problem hiding this comment.
Could you try adding list-all-pkgs? I'd like to see a report of the files that were scanned so we can assert that it did in fact scan the things we want it to.
| @@ -397,11 +397,32 @@ jobs: | |||
| name: session-build-${{ matrix.sys.os }} | |||
| path: build/ | |||
|
|
|||
| scan: | |||
There was a problem hiding this comment.
Please add the same if conditional as deploy. Which has it only run on nightly and release branches.
eg.
if: contains(fromJSON('["refs/heads/master", "refs/heads/beta", "refs/heads/release", "refs/heads/LTS"]'), github.ref) || startsWith(github.event.pull_request.head.ref, 'version/')
There was a problem hiding this comment.
You missed my comment about this:
I disagree that it should only run on release branches and nightlies. PRs should not introduce CVEs only for them to be caught later. We should be proactive. It took a whopping 5s to scan all generated binaries.
Until that is addressed, I am not okay with doing what you requested.
Remediate go-retryablehttp and gqlparser CVEs.