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.

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

ci: update rust-sec advisory db in every test run #100

Merged
merged 2 commits into from
Sep 5, 2024

Conversation

hallettj
Copy link
Collaborator

@hallettj hallettj commented Aug 22, 2024

We have had a security vulnerability check on cargo dependencies in place for a long time. But it's necessary to update the advisory db to get the latest advisories. This change updates a github workflow to run the update on every CI test run.

I tested that the check catches problems by running a test with gix-fs installed at version 0.10.2. See https://rustsec.org/advisories/RUSTSEC-2024-0350.html

@hallettj hallettj self-assigned this Aug 22, 2024
@codedmart
Copy link
Collaborator

codedmart commented Aug 26, 2024

Is this the correct command and output? Why are we ignoring yanked and why is it showing errors, but the action passes.

image

@hallettj
Copy link
Collaborator Author

hallettj commented Sep 3, 2024

Is this the correct command and output? Why are we ignoring yanked and why is it showing errors, but the action passes.

Yes, that's correct. That's the way crane has it set up. Since we're building with crane we get that audit configuration for free.

Failed "yanked" checks are a limitation of the Nix setup. The check runs in a sandbox without network access, and the downloaded advisory database doesn't include "yanked" status. But we do get advisories on vulnerabilities whether or not a crate version was yanked, so the audit check works as intended.

Running the audit in a Nix sandbox has the advantage that we cache successful checks. If the hashes of Cargo.lock and the advisory database are unchanged then CI uses the cached success result. We could move the check out of the Nix sandbox, and do live network requests instead. But it would be slower, and I don't think we would gain anything.

I don't think the --ignore yanked option actually does anything. --ignore is for ignoring specific advisory IDs, like --ignore RUSTSEC-2017-0001. The CI check gives the same results with or without that option. My guess is the crane dev was hoping to suppress those error messages, but it didn't work.

@hallettj hallettj merged commit 8df94e9 into main Sep 5, 2024
1 check passed
@hallettj hallettj deleted the jessehallett/ndc-412-ndc-mongodb branch September 5, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants