add cargo audit check

[Ekleog-NEAR]

No description provided.

[Ekleog-NEAR]

Note that this can only land after #10201 and #10214 have landed, which should make the test pass. However, it can already be reviewed, this way I can land as soon as the other PRs have landed.

I’ll also need to make it into a required check.

[nagisa]

Please add this as a nextest style test while we’re hashing out the justfile questions. Otherwise this is now just another example of a check that only runs in the CI and one that one will most likely not notice failing before waiting ~15 minutes of CI (especially since the CI is always "red" anyway)

[nagisa]

Hm… OTOH I’m not sure if we should make this a required check (which adding it as a style check would enforce.) One issue with cargo audit checks is that more often than not this check will fail due to changes in the environment (advisories being published) rather than the changes to the PR code, and I’m not sure if we should stop the development project-wide when something like that happens…

[Ekleog-NEAR]

The CI is no longer always red, since #10218 :) (it really annoyed me too) So this would probably be noticed within a 20 seconds of the job starting, considering the current time to run the action.

I agree with the issues with making this required. TBH I’m not sure either. I think it’d probably make sense to make it required, but will definitely ask on zulip before doing so.

As for the justfile story, TBH I think this will probably not be ready to land before we complete it, considering that #10201 will probably be slow to review, and I’m planning on writing the Justfile tomorrow, whether #10180 has landed or not :) (though I’m hoping it can have landed before and I can submit it in another PR, because with the current timezone difference the number of round-trips would mean I’d make a fourth day in a row working late to get #10180 to land before going OOO tomorrow evening)

[nagisa]

Approval conditional on not making this a blocking check. The specific solution there will need more discussion (file an issue to have this discussion please, so it does not get lost)

[nikurt]

when cargo audit fails, which command should I run to auto-resolve it?
I would strongly prefer to upgrade dependencies in separate boring PRs. If a complicated PR adds a new feature, then making it also upgrade dependencies seems counter-productive.

[Ekleog-NEAR]

The specific solution there will need more discussion (file an issue to have this discussion please, so it does not get lost)

I hope the zulip thread I started just a bit ago is going to reach a good solution :)

when cargo audit fails, which command should I run to auto-resolve it?

It depends on the specific failure reason (described in the error message), which is the reason why the check is currently informational-only (like nightly nayduck tests were before I removed it).

The goal of running it on every PR is to hopefully notice before we add vulnerable dependencies, in which case the solution is to just not do it, and pick another dependency or version of it :)

I would strongly prefer to upgrade dependencies in separate boring PRs. If a complicated PR adds a new feature, then making it also upgrade dependencies seems counter-productive.

Totally agreed, the goal is definitely not to have unrelated dependency upgrades be forced into PRs. If there’s a red checkmark and you didn’t change a dependency, it’s the same kind of information as if you saw nightly nayduck tests failing on your PR. The difference is, that if you did change a dependency, then it’s probably you who made the check turn red :)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (34e65c0) 71.83% compared to head (891e4b7) 71.82%.
Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10216      +/-   ##
==========================================
- Coverage   71.83%   71.82%   -0.02%     
==========================================
  Files         707      707              
  Lines      141904   142111     +207     
  Branches   141904   142111     +207     
==========================================
+ Hits       101942   102072     +130     
- Misses      35248    35323      +75     
- Partials     4714     4716       +2     

Flag	Coverage Δ
backward-compatibility	0.08% <ø> (-0.01%)	⬇️
db-migration	0.08% <ø> (-0.01%)	⬇️
genesis-check	1.23% <ø> (-0.01%)	⬇️
integration-tests	36.22% <ø> (+<0.01%)	⬆️
linux	71.71% <ø> (-0.02%)	⬇️
linux-nightly	71.59% <ø> (+<0.01%)	⬆️
macos	53.92% <ø> (-1.54%)	⬇️
pytests	1.46% <ø> (-0.01%)	⬇️
sanity-checks	1.26% <ø> (-0.01%)	⬇️
unittests	68.14% <ø> (-0.06%)	⬇️
upgradability	0.13% <ø> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Ekleog-NEAR]

Thank you for pushing this through! :D