Skip to content
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.

Sign up for GitHub

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

Add workflow for dependabot and cargo vet #537

Merged
merged 2 commits into from
Oct 26, 2023
Merged

Add workflow for dependabot and cargo vet #537

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
734029b
Add workflow for dependabot and cargo vet
jeffcharles Oct 24, 2023
6c1241c
Remove regenerating exemptions
jeffcharles Oct 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions .github/workflows/dependabot-cargo-vet.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Runs cargo vet and cargo vet regenerate exemptions for Dependabot PRs
name: Dependabot update cargo vet
on:
push:
branches:
- "dependabot/cargo/**"

jobs:
vet:
runs-on: ubuntu-latest

permissions:
contents: write

env:
CARGO_VET_VERSION: 0.8.0

steps:
- uses: actions/checkout@v4

- uses: actions/cache@v3
with:
path: ${{ runner.tool_cache }}/cargo-vet
key: cargo-vet-bin-${{ env.CARGO_VET_VERSION }}

- name: Add the tool cache directory to the search path
run: echo "${{ runner.tool_cache }}/cargo-vet/bin" >> $GITHUB_PATH

- name: Ensure that the tool cache is populated with the cargo-vet binary
run: cargo install --root ${{ runner.tool_cache }}/cargo-vet --version ${{ env.CARGO_VET_VERSION }} cargo-vet

- run: cargo vet
continue-on-error: true

# These all ask for input on the terminal to select the trusted criteria but take the default of `safe-to-deploy`.

- run: cargo vet trust --all BurntSushi
continue-on-error: true

- run: cargo vet trust --all sunfishcode
continue-on-error: true

- run: cargo vet trust --all dtolnay
continue-on-error: true

- run: cargo vet trust --all cuviper
continue-on-error: true

- run: cargo vet trust --all Amanieu
continue-on-error: true

- run: cargo vet regenerate exemptions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel comfortable us blindly doing this, for several reasons:

  • cargo-vet's docs explicitly recommend doing this as a last resort:

    The most aggressive solution is to run cargo vet regenerate exemptions which will add whatever exemptions necessary to make check pass (and remove uneeded ones). Ideally you should avoid doing this and prefer adding audits, but if you've done all the audits you plan on doing, that's the way to finish the job.

  • Even though this is probably the path of least resistance, following this approach will potentially reduce the possibility of allowing us to shrink our exemptions list, which is what I personally think we should be going after: every time a dependency is updated, we could (and probably should) check if audits or trust criteria for a particular crate has changed in the other supply chains that we import, for example, I noticed that we have exemptions for clap, but Wasmtime trusts it, is there a good reason for us not to trust it as well? This also seems to be a side effect of us relying mostly on cargo vet --locked and perhaps not running cargo vet (without --locked), which as per the docs, will fetch the imports and give suggestions, according to the imports, for example if I remove clap's exemption from our list and run cargo vet I get the following suggestion to trust epage

1 unvetted dependencies:
  clap:2.34.0 missing ["safe-to-deploy"]

recommended audits for safe-to-deploy:
    Command                        Publisher  Used By                  Audit Size
    cargo vet inspect clap 2.34.0  kbknapp    criterion and structopt  25252 lines
      NOTE: mozilla and bytecode-alliance trust Ed Page (epage), who published another version of this crate - consider cargo vet trust clap epage

clap is just one example, but I think that if we don't keep regenerating the exemptions, we'd be able to get rid of other exemptions in the exemptions list by relying on all the delta audits that are already published.

Copy link
Collaborator Author

@jeffcharles jeffcharles Oct 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove this line. I don't know how to automate adding additional trusted users given cargo vet doesn't appear to have a stable output format I can use a readily available parser on.

This also seems to be a side effect of us relying mostly on cargo vet --locked and perhaps not running cargo vet (without --locked), which as per the docs, will fetch the imports and give suggestions, according to the imports, for example if I remove clap's exemption from our list and run cargo vet I get the following suggestion to trust epage

Yeah I already run cargo vet without the --locked parameter whenever I make a dependency change and add trusted publishers if our imports also trust the author.

What happened in clap's case is it's owned by multiple users. epage published a different version of the crate. So running cargo vet trust --all epage would not result in the crate being trusted. We have to run cargo vet trust clap epage because epage published a different version but we're vouching that we trust the version we're using (I'm not clear on why we specify the user in this case).

Should we adopt the policy that if a publisher trusted by one of our imports has published a different version of the crate, we should trust the version of the crate we're using if it's been published by a different publisher who is not trusted by any of our imports?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's right, if I trust epage and run cargo-vet after (having removed the exemption for clap) -- I still get a warning about our current version.

Should we adopt the policy that if a publisher trusted by one of our imports has published a different version of the crate, we should trust the version of the crate we're using if it's been published by a different publisher who is not trusted by any of our imports?

I'd prefer if in this case we still require an audit/exemption since even though we assume that it's probably fine, we are trusting explicitly what a particular user publishes. Either that or updating to a dependency published by a trusted user. Like in the case of clap, if we update to a newer version, we can get rid of the exemption. (Sidenote: I'm curious why dependabot hasn't updated clap, we're in a old version)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if in this case we still require an audit/exemption since even though we assume that it's probably fine, we are trusting explicitly what a particular user publishes.

Agreed!

I'm curious why dependabot hasn't updated clap, we're in a old version

clap is a transitive dependency used by criterion and structop. structopt depends on version 2.x of clap which pins it to a 2.x version in our project. structop is in maintenance mode so they're likely not updating their dependencies regularly. If we want to use a newer version of clap we should complete #321.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that explains it -- I thought we had migrated.


- name: commit and push
shell: bash
run: |
if ! git diff --exit-code; then
git config --global user.name 'github-actions[bot]'
git config --global user.email 'github-actions[bot]@users.noreply.github.com'
git commit -am "[dependabot skip] Regenerate cargo vet"
git push
fi