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

Setup trivy scanning #27

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Setup trivy scanning #27

wants to merge 10 commits into from

Conversation

eswdd
Copy link
Contributor

@eswdd eswdd commented Mar 25, 2022

No description provided.

@jgiannuzzi
Copy link
Member

Should we maybe instead run Trivy daily on the existing latest image + on pull requests and push to master?
This will let us scan what is currently released + avoid pushing vulnerable images


- name: Build an image from Dockerfile
run: |
docker build -t docker.io/my-organization/my-app:${{ github.sha }} .
Copy link

Choose a reason for hiding this comment

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

Are those really the literals my-organization and my-app, or are they placeholders?

Also I know nothing about this thing, could you please link to a reference for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i would if i knew of one :D

@eswdd
Copy link
Contributor Author

eswdd commented Mar 25, 2022

Should we maybe instead run Trivy daily on the existing latest image + on pull requests and push to master?
This will let us scan what is currently released + avoid pushing vulnerable images

Yes i think so, TBH i hadn't noticed that the content of this was non-sensicle

@eswdd
Copy link
Contributor Author

eswdd commented Mar 25, 2022

@jgiannuzzi i think we should just move the 2 scan items to the docker build action we already have. i don't see the value in doing docker build twice. That also means we don't need to maintain the image name in 2 places which resolves what @greed42 flagged

@jgiannuzzi
Copy link
Member

@eswdd agreed — and as a future improvement, we should also build the Docker image (but not push, of course) on pull requests

@@ -21,17 +24,22 @@ jobs:

- name: Build image
run: docker build -t image .

- name: Sanitize repo slug
uses: actions/github-script@v4
Copy link
Member

Choose a reason for hiding this comment

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

I think we should use the latest version of this action

Suggested change
uses: actions/github-script@v4
uses: actions/github-script@v6

Comment on lines +71 to +83
- name: Run Trivy vulnerability scanner
uses: aquasecurity/trivy-action@2a2157eb22c08c9a1fac99263430307b8d1bc7a2
with:
image-ref: '${{ steps.github_id.outputs.result }}:${{ github.sha }}'
format: 'template'
template: '@/contrib/sarif.tpl'
output: 'trivy-results.sarif'
severity: 'CRITICAL,HIGH'

- name: Upload Trivy scan results to GitHub Security tab
uses: github/codeql-action/upload-sarif@v1
with:
sarif_file: 'trivy-results.sarif'
Copy link
Member

Choose a reason for hiding this comment

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

We may want to scan the image before we push it, to avoid pushing known vulnerable images.

In that case, we may want to refactor further, by using the proper image name straight for building, instead of image.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants