ci: use cargo deny #6931
ci: use cargo deny #6931
Conversation
|
I think it could make sense to change the pr-audit settings as well tokio/.github/workflows/pr-audit.yml Lines 31 to 32 in 01e04da |
|
Oh I was not aware we had two jobs for this ... |
| { allow = ["Unicode-DFS-2016"], crate = "unicode-ident" }, | ||
| ] | ||
|
|
||
| [bans] |
There was a problem hiding this comment.
I don't usually have bans and sources sections, are the defaults not good enough?
There was a problem hiding this comment.
It's spamming warnings about some duplicate versions of windows-sys.
Understandable, but I usually just set it up like this. In most repositories it's not a big deal. Not sure how common yanking is in your dependency chain? |
|
I mean, it's fine if we can't have it make issues, but I'd like to do that if it's just a config option away. |
| permissions: | ||
| checks: write # for rustsec/audit-check to create check | ||
| contents: read # for actions/checkout to fetch code | ||
| issues: write # for rustsec/audit-check to create issues | ||
| checks: write | ||
| contents: read | ||
| issues: write |
There was a problem hiding this comment.
EmbarkStudios/cargo-deny-action action does not seem create issues (EmbarkStudios/cargo-deny-action#64), so these permissions are probably not needed.
There was a problem hiding this comment.
I mean, not like the rustsec one has ever created an issue ...
There was a problem hiding this comment.
(The last time rustsec action created an issue in this repository was two years ago, which IIRC was before fine-grained permission settings in GitHub actions were supported.)
https://github.com/tokio-rs/tokio/issues?q=is%3Aissue+author%3Aapp%2Fgithub-actions
| @@ -16,17 +16,8 @@ permissions: | |||
| contents: read | |||
|
|
|||
| jobs: | |||
| security-audit: | |||
| cargo-deny: | |||
There was a problem hiding this comment.
If we are doing the same thing with both files, I think it would be better to merge them.
There was a problem hiding this comment.
The docs recommend using this:
name: CI
on: [push, pull_request]
jobs:
cargo-deny:
runs-on: ubuntu-22.04
strategy:
matrix:
checks:
- advisories
- bans licenses sources
# Prevent sudden announcement of a new advisory from failing ci:
continue-on-error: ${{ matrix.checks == 'advisories' }}
steps:
- uses: actions/checkout@v3
- uses: EmbarkStudios/cargo-deny-action@v1
with:
command: check ${{ matrix.checks }}Perhaps we should do this in the toml for master, but keep it as-is on PRs?
There was a problem hiding this comment.
Perhaps we should do this in the toml for master, but keep it as-is on PRs?
Hmm. This will cause the advisory to be ignored until someone next opens a PR that edits Cargo.toml, right?
There was a problem hiding this comment.
I guess that depends on whether we get an email on error ...
There was a problem hiding this comment.
I have an image that continue-on-error treats failures the same as successes, but in practice I have rarely used it.
There was a problem hiding this comment.
Well, this PR is sufficient as-is to address the problem of broken CI, so I think it is fine to leave the fine tuning to future PRs.
| runs-on: ubuntu-latest | ||
| if: "!contains(github.event.head_commit.message, 'ci skip')" |
There was a problem hiding this comment.
We never used that 🤷♀️
The rustsec audit check is not working anymore due to a
Cargo.lockrequirement. Update to cargo deny instead.I'm not completely sure about this configuration. Ideally, we want advisories to result in an issue being opened, rather than in failing CI on random PRs. Thoughts?