Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

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

Open
mullr opened this issue Dec 10, 2020 · 5 comments · May be fixed by #192
Open

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

mullr opened this issue Dec 10, 2020 · 5 comments · May be fixed by #192
Labels
bug Something isn't working

Comments

@mullr
Copy link

mullr commented Dec 10, 2020

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 mullr added the bug Something isn't working label Dec 10, 2020
@mullr
Copy link
Author

mullr commented Dec 10, 2020

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 mullr closed this as completed Dec 10, 2020
@mullr
Copy link
Author

mullr commented Dec 10, 2020

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.

@Nemo157
Copy link

Nemo157 commented Mar 2, 2021

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
Copy link

faern commented Mar 24, 2021

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 😮. 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 eugene-babichenko linked a pull request May 6, 2021 that will close this issue
@eugene-babichenko
Copy link

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

4 participants