audit-check icon indicating copy to clipboard operation
audit-check copied to clipboard

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

Open mullr opened this issue 4 years ago • 5 comments

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 avatar Dec 10 '20 16:12 mullr

~~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 avatar Dec 10 '20 16:12 mullr

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/.

  • There's crate in Cargo.toml for which a semver-compatible update has been released
  • Cargo.lock specifies the older one version, not the updated one.
  • When running cargo build, the older version is used
  • The audit-check action update Cargo.lock, and checks against the newer version.

mullr avatar Dec 10 '20 17:12 mullr

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.

Nemo157 avatar Mar 02 '21 11:03 Nemo157

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.

faern avatar Mar 24 '21 15:03 faern

Another problem this behavior incurres:

  • Let's say we have a Cargo.lock that works perfectly fine.
  • It has a dependency (in my case transitive) that is yanked from the crates.io registry but still downloadable.
  • Due to how yanked crates work cargo generate-lockfile just will not be able to create Cargo.lock.

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

eugene-babichenko avatar May 06 '21 11:05 eugene-babichenko