Add security audit workflow [SAME VERSION] #216
Conversation
romoh
requested review from
bfjelds,
Britel,
jiria and
kate-goldenring
as code owners
.github/workflows/security-audit.yml
Outdated
| @@ -0,0 +1,24 @@ | |||
| name: Security audit | |||
There was a problem hiding this comment.
should this be part of https://github.com/deislabs/akri/blob/main/.github/workflows/check-rust.yml?
There was a problem hiding this comment.
I thought to group all security audits in one workflow and later we can expand it to include other security checks too. No strong opinion thou, I can move it to check-rust.yml since they run on the same triggers.
| - uses: actions/checkout@v1 | ||
| - uses: actions-rs/audit-check@v1 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
i'm not sure we should be adding our token here. can we do this in a way that would work from a fork PR?
There was a problem hiding this comment.
@romoh what happens if the token is removed?
There was a problem hiding this comment.
The token is only required for generating a report of the vulnerabilities and warnings as an output of the action. If the token is not found an stdout error is shown but doesn't impact the result of the audit step. I sent a test PR to confirm that this is indeed the case.
@kate-goldenring, to answer your question, We can remove the token if we don't care about the reporting part. I would keep it there just in case github figured how to get this working at some point, the reporting will light up automatically.
There was a problem hiding this comment.
@romoh thanks for checking. That sounds like a good plan to me
There was a problem hiding this comment.
you are still going to add a comment here with that information, right?
What this PR does / why we need it:
Add a security-audit workflow to catch security vulnerabilities. The workflow uses an existing GitHub action maintained by the cargo-audit team.
For now, I chose to trigger it on pull requests and pushes. The workflow can later be expanded to include other security auditing.
The workflow was tested by creating a pull request to a forked branch that introduces a security vulnerability (undo of #213) and the workflow failed. Upon fixing it, the workflow succeeded.
Since this is not a product related, no version update required.
Closes #212
Special notes for your reviewer:
@bfjelds - Please chime in with your experience around token permissions. Not sure if that will be problematic.
Note: The action states that in case of token permission issues, the action will output all found advisories to the stdout instead. Another option is to switch the workflow to a daily cadence instead.
If applicable:
cargo fmt)cargo build)cargo clippy)cargo test)cargo doc)./version.sh)