-
Notifications
You must be signed in to change notification settings - Fork 0
FREYA-1889: Add 'trivy' actions #47
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
base: main
Are you sure you want to change the base?
Conversation
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
Ziip-dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work! We just need to agree on failing the action or not as mentioned here before approving :)
Fixed ✅ |
|
@Ziip-dev Could you have a look at this when you have some time? 🙏🏻 |
Oups I had forgotten this one sorry, thanks for the reminder! |
No worries! |
Ziip-dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work :)
Some minor changes, the rest is more open questions and trails for improvement that I would like to have the team's opinion on
| push: | ||
| branches: [main] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this one is necessary as we can't push to main
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I saw your comment about this after publishing the review, but I'm curious why then as I missed the discussion :)
| scan-type: "fs" | ||
| scan-ref: "." # Default but being explicit | ||
| scanners: "vuln,secret" # Default but being explicit | ||
| # ignore-unfixed: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is ignore-unfixed for initially? And why is it commented out here?
Genuine question, I'm not that familiar with actions so if you can briefly explain to me :) (asking because it's set as true in the old portal repo)
| security-events: write | ||
| name: Build | ||
| runs-on: ubuntu-latest | ||
| steps: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need the checkout action here?
|
|
||
| - name: Upload Trivy scan results to GitHub Security tab | ||
| if: always() # Publish results to security tab even if scan fails | ||
| uses: github/codeql-action/upload-sarif@v3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although we still have one year before deprecation, since we have a warning already and the v4 has been released:
| uses: github/codeql-action/upload-sarif@v3 | |
| uses: github/codeql-action/upload-sarif@v4 |
|
|
||
| - name: Upload Trivy scan results to GitHub Security tab | ||
| if: always() # Publish results to security tab even if scan fails | ||
| uses: github/codeql-action/upload-sarif@v3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| uses: github/codeql-action/upload-sarif@v3 | |
| uses: github/codeql-action/upload-sarif@v4 |
| TRIVY_DB_REPOSITORY: >- | ||
| ghcr.io/aquasecurity/trivy-db,public.ecr.aws/aquasecurity/trivy-db |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issue here, just an idea: could we benefit from trivy built-in cache step to reduce runtime? This would avoid downloading the entire db on every run
| with: | ||
| scan-type: "fs" | ||
| scan-ref: "." # Default but being explicit | ||
| scanners: "vuln,secret" # Default but being explicit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would adding the misconfig scanner be beneficial?
| # ignore-unfixed: true | ||
| format: 'sarif' | ||
| output: 'trivy-results.sarif' | ||
| severity: 'CRITICAL,HIGH' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another open question :)
Are we interested in broader severity coverage? Like MEDIUM severity at least? Or do we want to ignore everything below HIGH severity
| ghcr.io/aquasecurity/trivy-db,public.ecr.aws/aquasecurity/trivy-db | ||
| uses: aquasecurity/[email protected] | ||
| with: | ||
| image-ref: ghcr.io/scilifelabdatacentre/swedish-pathogens-portal:main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this workflow scans the built Docker image via image-ref, which is good, but never the repository file system.
Is it intended and why?
📋 Summary
This PR adds the github action trivy. Taken from the pathogens portal repo and slightly altered.
🛠️ Changes Made
exit-codeandif: always()-- the action will fail if it detects a vulnerability but it will always publish the results to the security tabtrivy-scheduled.yamlI kept the scan onmainanyway (discussion on slack), but removed some unnecessary steps.🔍 Notes for Reviewers
As discussed on slack, I looked at only scanning
latestinstead ofmain. However we don't publish any tag called latest if I've understood correctly, so I decided to go for main anyway instead of altering the publishing.✅ Checklist
FREYA-XXXX: Clear and short description🔗 Jira Issue
Closes: FREYA-1889