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: Add cargo audit CI action #5160

Merged
merged 9 commits into from
Dec 6, 2023
Merged

Conversation

Jefffrey
Copy link
Contributor

@Jefffrey Jefffrey commented Dec 2, 2023

Which issue does this PR close?

Closes #57

Rationale for this change

Audit dependencies for security vulns

What changes are included in this PR?

Use audit-check action in new CI step

Are there any user-facing changes?

on:
push:
branches:
- master
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not run this for pull requests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean to always run on PRs regardless of if they modified Cargo.toml files? My line of thinking was that only PRs that affect the dependencies should have this check run, to prevent an unrelated PR potentially being blocked.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aah I see, perhaps we could also run on PRs that modify the CI configuration for this check. I mainly want to see this check running on this PR before I merge it 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's fair enough

I went and made some minor updates to descriptions in csv and json crate, hope that's fine

Looks like CI check indeed failed to startup, might have to include the steps to run cargo audit manually instead of relying on existing action from rustsec 🤔

Error: .github#L1
rustsec/[email protected] is not allowed to be used in apache/arrow-rs. Actions in this workflow must be:
within a repository owned by apache, created by GitHub, verified in the GitHub Marketplace, or matching the
following: */*@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]+, AdoptOpenJDK/install-jdk@*,
JamesIves/github-pages-deploy-action@5dc1d5a192aeb5ab5b7d5a77b7d36aea4a7f5c92, TobKed/label-when-approved-action@*,
actions-cool/issues-helper@*, actions-rs/*, al-cheb/configure-pagefile-action@*, amannn/action-semantic-pull-request@*,
apache/*, burrunan/gradle-cache-action@*, bytedeco/javacpp-presets/.github/actions/*, chromaui/action@*,
codecov/codecov-action@*, conda-incubator/setup-miniconda@*, container-tools/kind-action@*,
container-tools/microshift-action@*, dawidd6/action-download-artifact@*, delaguardo/setup-graalvm@*,
docker://jekyll/jekyll:*, docker://pandoc/core:2.9, eps1lon/actions-label-merge-conflict@*, gaurav-nelson/github-action-markdown-link-check@*,
gol...

@github-actions github-actions bot added the arrow Changes to the arrow crate label Dec 5, 2023
@Jefffrey Jefffrey marked this pull request as draft December 6, 2023 07:38
@Jefffrey
Copy link
Contributor Author

Jefffrey commented Dec 6, 2023

Example run with a dependency with vulnerability:

Run audit check step:

Run cargo audit
    Updating crates.io index
    Fetching advisory database from `[https://github.com/RustSec/advisory-db.git`](https://github.com/RustSec/advisory-db.git%60)
      Loaded 580 security advisories (from /home/runner/.cargo/advisory-db)
    Updating crates.io index
    Scanning Cargo.lock for vulnerabilities (353 crate dependencies)
error: 1 vulnerability found!
Crate:     cocoon
Version:   0.3.3
Title:     Sequential calls of encryption API (`encrypt`, `wrap`, and `dump`) result in nonce reuse
Date:      2023-10-15
ID:        RUSTSEC-2023-0068
URL:       https://rustsec.org/advisories/RUSTSEC-2023-0068
Severity:  4.5 (medium)
Solution:  Upgrade to >=0.4.0
Dependency tree:
cocoon 0.3.3
└── arrow-csv 49.0.0
    ├── parquet 49.0.0
    │   ├── parquet_derive_test 49.0.0
    │   └── parquet_derive 49.0.0
    │       └── parquet_derive_test 49.0.0
    └── arrow 49.0.0
        ├── parquet 49.0.0
        ├── arrow-integration-testing 49.0.0
        └── arrow-integration-test 49.0.0
            └── arrow-integration-testing 49.0.0

Error: Process completed with exit code 1.

Subsequently fails.

@Jefffrey
Copy link
Contributor Author

Jefffrey commented Dec 6, 2023

Example of a clean run:

Run cargo audit
    Updating crates.io index
    Fetching advisory database from `[https://github.com/RustSec/advisory-db.git`](https://github.com/RustSec/advisory-db.git%60)
      Loaded 580 security advisories (from /home/runner/.cargo/advisory-db)
    Updating crates.io index
    Scanning Cargo.lock for vulnerabilities (327 crate dependencies)

@Jefffrey Jefffrey marked this pull request as ready for review December 6, 2023 11:11
Copy link
Contributor

@tustvold tustvold left a comment

Choose a reason for hiding this comment

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

Looks good to me

@tustvold tustvold merged commit 298ddfd into apache:master Dec 6, 2023
33 checks passed
@Jefffrey Jefffrey deleted the cargo_audit_ci branch December 6, 2023 11:19
@alamb
Copy link
Contributor

alamb commented Dec 6, 2023

I think this wins the prize for lowest ticket number closed in a long time 🏆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

run "cargo audit" in CI
3 participants