-
Notifications
You must be signed in to change notification settings - Fork 122
CI: add cargo-audit #3250
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: master
Are you sure you want to change the base?
CI: add cargo-audit #3250
Conversation
cbc7cdd
to
9ea77c1
Compare
9ea77c1
to
8ffa533
Compare
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.
Workflows should be related to triggers, not jobs. You can have multiple jobs per trigger. There are some workflows that we may want to independently change the trigger and that's when it makes the most sense to have a separate workflow with the same trigger.
Currently, these are all the workflows that we have defined:
benches-mina-prover-set-baseline.yml
benches-mina-prover.yml
benches.yml
ci-nightly.yml
ci.yml
fmt.yml
o1vm-ci.yml
o1vm-upload-mips-build.yml
saffron.yml
wasm.yml
saffron
and o1vm
are definitely separate projects where it probably makes sense to have separate workflows. ci
and ci-nightly
are based on different triggers. I would argue that fmt
should just be a part of ci
...
Is there anywhere else that you think this job could be more appropriately placed? Why not make it part of ci
?
- name: Install Rust | ||
uses: actions-rs/toolchain@v1 | ||
with: | ||
toolchain: stable | ||
override: 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.
This is deprecated, please use the same pattern that's in the rest of the CI jobs.
- name: Install Rust | |
uses: actions-rs/toolchain@v1 | |
with: | |
toolchain: stable | |
override: true | |
- name: Use shared Rust toolchain setting up steps | |
uses: ./.github/actions/toolchain-shared | |
with: | |
rust_toolchain_version: stable | |
- name: Install cargo-audit | ||
run: cargo install cargo-audit |
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.
- name: Install cargo-audit | |
run: cargo install cargo-audit | |
- uses: taiki-e/install-action@v2 | |
with: | |
tool: cargo-audit |
- name: Run cargo audit | ||
run: cargo audit --json > audit.json || true | ||
|
||
- name: Check for critical vulnerabilities | ||
run: | | ||
CRITICAL_COUNT=$(jq '[.vulnerabilities.list[] | select(.advisory.severity == "critical")] | length' audit.json) | ||
echo "Found $CRITICAL_COUNT critical vulnerabilities" | ||
if [ "$CRITICAL_COUNT" -gt 0 ]; then | ||
echo "Critical vulnerabilities detected!" | ||
exit 1 | ||
fi |
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.
... it looks like you're doing this in hard mode...
Why not just do cargo audit
? It won't fail on warnings.
But I just went to look up the docs to see what kind of severity it will fail on and the docs say to use
https://github.com/rustsec/audit-check
|
||
- name: Upload audit report artifact | ||
if: always() | ||
uses: actions/upload-artifact@v4 |
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.
Do we really need to do this? Do the logs get deleted? Is anyone actually going to pull these?
See initial PR in Mina: MinaProtocol/mina#16525
Opened #3251