Skip to content

ci: run trivy on last release binary#275

Open
upils wants to merge 3 commits intocanonical:mainfrom
upils:feat/rework-trivy
Open

ci: run trivy on last release binary#275
upils wants to merge 3 commits intocanonical:mainfrom
upils:feat/rework-trivy

Conversation

@upils
Copy link
Collaborator

@upils upils commented Mar 13, 2026

  • Have you signed the CLA?

Trivy was previously executed on the main branch, so the report was not
reflecting the level of vulnerability in the binary actually distributed to
users. Moreover, fs type scan only uses the go version from go.mod as a
hint and so cannot conclude on the stdb lib actually used to build the binary.
As a consequence, the report produced was imprecise and incomplete.

This PR does two main changes:

  • Run the scan on the last release, pulling the built amd64 binary;
  • Run a rootfs type scan on the binary, ensuring the the std lib included
    is analysed.

Actions are also pinned to protect against supply chain attacks on them.

Note: I manually tested both type of trivy scan on the binary and confirmed
only the rootfs one identifies the vulnerabilities in the std lib.

@upils upils added the Simple Nice for a quick look on a minute or two label Mar 13, 2026
@upils upils requested review from cjdcordeiro and letFunny March 13, 2026 13:41
Comment on lines +22 to +23
# TODO: replace with fine grained token once created.
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need a token for this. It's a public asset. This will only help with the making of authn request for avoiding rate limits, but given the cadence of this workflow, I doubt you'll run into such limits, even if unauthenticated

id: release
run: |
url=$(gh release view --json assets --jq \
'.assets[] | select(.name | test("linux_amd64.tar.gz$")) | .url' \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this run on a matrix? I.e. to catch architecture-specific vulnerabilities

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should, tbh if it is easier we can download all the files, extract them and look for binaries.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would gh release download work as well? From documentation it says it downloads the latest release and it is simpler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this run on a matrix? I.e. to catch architecture-specific vulnerabilities

In Go, dependencies in go.mod are not architecture-dependent. Vulnerabilities are not architecture-dependent either, so AFAIK when vulnerabilities are found, they are associated to a specific version (or a range) of a libraries, but not to a specific architecture. I agree that in reality, a vulnerability could only affect an arch-specific path, but this vulnerability will be associated to the library anyway. In go files, specific directive could be used to tune the imports based on the target arch, but these dependencies must be listed anyway in go.mod. So that could lead to false positives but not false negatives.

Given that, the architecture we select to scan is actually irrelevant. Do you agree?

I picked amd64 because this is the most common and the one we are most likely to keep building over time.

scan-type: 'fs'
scan-ref: '.'
scan-type: 'rootfs'
scan-ref: './release-scan/chisel'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given that this path is reused in multiple steps, I recommend either setting it in an env or creating an output in the step above (where it is introduced for the 1st time)

Comment on lines +37 to +38
# TODO: Replace with the fine grained token once created.
github-pat: ${{ secrets.GITHUB_TOKEN }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know the scope of such a PAT? the trivy action docs don't specify

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs at least security-events: write to update the status of code scanning alerts.

severity: 'CRITICAL,HIGH'
exit-code: '1'
# TODO: Replace with the fine grained token once created.
github-pat: ${{ secrets.GITHUB_TOKEN }}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this token here. the report will have been uploaded already above, and this step is merely a tailored duplication that aims at making the workflow fail if there are CRITICAL,HIGH vulnerabilities.

one could even just parse the sarif file and get the same result . e.g.

grep -E "Severity: (HIGH|CRITICAL)" ${{ env.TRIVY_RESULTS }}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point for the token. However there is no way to tell trivy to output in 2 formats at the same time and the SARIF files are JSON, not ideal to read in the job output. I understand these were the main motivation to run trivy twice and I think they still stand (especially considering how fast it is to run in our case).

Copy link
Collaborator

@letFunny letFunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking more into this Paul, great diligence! Just a couple of comments.

id: release
run: |
url=$(gh release view --json assets --jq \
'.assets[] | select(.name | test("linux_amd64.tar.gz$")) | .url' \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should, tbh if it is easier we can download all the files, extract them and look for binaries.

uses: aquasecurity/trivy-action@master
uses: aquasecurity/trivy-action@57a97c7e7821a5776cebc9bb87c984fa69cba8f1 # v0.35.0
with:
scan-type: 'fs'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we not going to do the 'fs' check then? If we could have it "for free" it would be even better as it would cause problems in the go.mod itself. Let's say that go.mod says go 1.23 is used but our CI uses go.24 to build the binary. Then the binary might not have vulnerabilities but users building the binary themselves might not be safe. In this case I think we should probably bump the minimal go version in go.mod as well even if it is more troublesome for us.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If our go.mod says Go 1.23, it only means our code needs at least this version to work properly. This is not a promise on the level of security, but a minimum functional requirement. In an ideal world (and forgetting about new language features) I suppose we would like this value to be as low as possible so the project could be built with a wide variety of go versions.

I don't think we can do any promise on binaries we do not distribute ourselves, nor should we try to.

Also, as explained in the PR description, in fs mode trivy is not looking at the go ... directive given in go.mod because it does not say what version of Go will actually be used. So in the case of a Go project, the fs mode executed on the code could only find less vulnerabilities than the rootfs mode on the binary, because the rootfs mode will identify the Go version used.

id: release
run: |
url=$(gh release view --json assets --jq \
'.assets[] | select(.name | test("linux_amd64.tar.gz$")) | .url' \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would gh release download work as well? From documentation it says it downloads the latest release and it is simpler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Simple Nice for a quick look on a minute or two

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants