-
Notifications
You must be signed in to change notification settings - Fork 134
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,42 @@ | ||||||||||||||||||||||
name: Cargo Audit | ||||||||||||||||||||||
|
||||||||||||||||||||||
on: | ||||||||||||||||||||||
pull_request: | ||||||||||||||||||||||
push: | ||||||||||||||||||||||
branches: | ||||||||||||||||||||||
- master | ||||||||||||||||||||||
|
||||||||||||||||||||||
jobs: | ||||||||||||||||||||||
audit: | ||||||||||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||||||||||
steps: | ||||||||||||||||||||||
- name: Checkout code | ||||||||||||||||||||||
uses: actions/checkout@v4 | ||||||||||||||||||||||
|
||||||||||||||||||||||
- name: Install Rust | ||||||||||||||||||||||
uses: actions-rs/toolchain@v1 | ||||||||||||||||||||||
with: | ||||||||||||||||||||||
toolchain: stable | ||||||||||||||||||||||
override: true | ||||||||||||||||||||||
Comment on lines
+16
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
- name: Install cargo-audit | ||||||||||||||||||||||
run: cargo install cargo-audit | ||||||||||||||||||||||
Comment on lines
+22
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||
|
||||||||||||||||||||||
- 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 | ||||||||||||||||||||||
Comment on lines
+25
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 |
||||||||||||||||||||||
|
||||||||||||||||||||||
- 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 commentThe 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? |
||||||||||||||||||||||
with: | ||||||||||||||||||||||
name: cargo-audit-report | ||||||||||||||||||||||
path: audit-results/audit.json |
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:
saffron
ando1vm
are definitely separate projects where it probably makes sense to have separate workflows.ci
andci-nightly
are based on different triggers. I would argue thatfmt
should just be a part ofci
...Is there anywhere else that you think this job could be more appropriately placed? Why not make it part of
ci
?