actions-rs / audit-check

🛡️ GitHub Action for security audits
https://github.com/marketplace/actions/rust-audit-check
MIT License
170 stars 39 forks source link

`generate-lockfile` overwrites a checked-in Cargo.lock #163

Open mullr opened 3 years ago

mullr commented 3 years ago

Description

I have a repo where I've checked in Cargo.lock, since it's producing a binary which I'm shipping. I've just started getting audit violations in CI for this that I cannot reproduce locally. I've tracked this down to the generate-lockfile call at the beginning; this updates the checked-in Cargo-lock. In my case, it brings in a new vulnerability due to a transitive dependency update.

Workflow code

name: Security audit
on:
  push:
    paths:
      - '**/Cargo.toml'
      - '**/Cargo.lock'
jobs:
  security_audit:
    timeout-minutes: 30
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v1
      - uses: actions-rs/audit-check@v1
        with:
          token: ${{ secrets.GITHUB_TOKEN }}

Expected behavior

If a Cargo.lock is in source control, it should be used as-is.

mullr commented 3 years ago

actually, I was wrong. It appears that I misunderstood how cargo build manages the lock file. My checked in version was just out of date. My apologies.

mullr commented 3 years ago

Sorry for the thrash here; this is in fact real. I've put up a repro at https://github.com/mullr/cargo-audit-action-repro/.

Nemo157 commented 3 years ago

Rather than generate-lockfile this should call cargo metadata --format-version=1 >/dev/null as a relatively quick no-op "ensure the lockfile is up to date". It would be good for the action to also take a locked: boolean flag to determine whether to pass --locked to this call for repositories that expect the lockfile to always be up to date.

faern commented 3 years ago

I also ran into this for a binary crate with checked in Cargo.lock. I knew I had vulnerabilities, but this action did not find them :open_mouth:. Turned out it's beacuse it's automatically updating all my dependenices before doing the audit check. This is a serious miss since I won't get a CI error reported, but the binaries I keep building will have the vulnerabilities.

eugene-babichenko commented 3 years ago

Another problem this behavior incurres:

And it doesn't need to, because a working Cargo.toml is already there.