Skip to content

rust: check Cargo.lock downgrades with a pinned action - #375

Merged
prestist merged 1 commit into
mainfrom
rust-lockfile-downgrade-check
Oct 22, 2026
Merged

prestist merged 1 commit into
mainfrom
rust-lockfile-downgrade-check

Conversation

@prestist

@prestist prestist commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Use a temporary patched fork of Garfield1002/cargo-no-downgrades while upstream issue #1 remains unresolved.

The action now comes from prestist/cargo-no-downgrades, branch fix-downgrade-detection, pinned to the full commit SHA e34e8a1894e48119667dbe321f2059d60aaf4d6e rather than a moving branch or the inherited upstream tag. The template identifies the branch and links the upstream issue so we can switch back to a verified upstream SHA once equivalent fixes land.

Fork fixes

  • Compare resolved dependency edges as well as highest locked versions. This catches the original coreos-installer #1772 regression: the installer switches from rand 0.9.5 to 0.8.8 while both versions remain locked, including when the installer release version changes.
  • Correct SemVer prerelease identifier ordering, including alpha.2 < alpha.10.
  • Add 38 standard-library regression tests and a test workflow. Matching behavior and limitations are documented in the fork.

Remaining work before merging

  1. Adding or removing labels currently reruns the whole Rust workflow and can cancel builds already running. The downgrade check should have its own workflow and concurrency group.
  2. coreos-installer has a custom Rust workflow, so this template change does not cover it. A separate downgrade workflow would let us include it without changing its existing tests.
  3. Configure the check as required on the intended Rust repositories once the workflow is rolled out.

This PR remains a draft for the remaining integration work; the upstream action blocker is addressed by the temporary fork.

Validation

  • All 38 fork regression tests pass locally and in GitHub Actions.
  • The patched comparison detects exactly the original rand downgrade using the full coreos-installer lockfiles at 6235c0ad53f66273bee3a357d5687e97afeca635 and 42716b7da1f17ca686c04ac40b5ee4a40427396f. Self-comparisons and the corrected release lockfile pass.
  • Template rendering, renderer unit tests, and actionlint v1.7.12 pass for all 11 affected rendered Rust workflows with the new fork pin.

@Rolv-Apneseth Rolv-Apneseth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Won't block, but thoughts on having this be a standalone action? Like https://github.com/Garfield1002/cargo-no-downgrades, though I guess we probably don't want to trust that one ourselves. So maybe we host one in https://github.com/bootc-dev/actions?

@Rolv-Apneseth

Copy link
Copy Markdown
Member

Or maybe we could trust that action if we pin by sha?

Their approach with using tomllib seems like a reasonable way to go here at the very least.

@cgwalters

Copy link
Copy Markdown
Member

Hmm, this seems like something to fix in renovate/dependabot?

So maybe we host one in https://github.com/bootc-dev/actions?

(Thanks for bringing that up, I would love to have more usage of that in the coreos/ org)

Or maybe we could trust that action if we pin by sha?

Probably yeah, I glanced at the code and it's small and looks sane, but I'd like to understand better how the downgrade happened in the first place

@yasminvalim yasminvalim left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/LGTM

@Rolv-Apneseth

Copy link
Copy Markdown
Member

I'd like to understand better how the downgrade happened in the first place

When I was looking at this earlier I saw quite a few issues open against cargo related to this, e.g.
rust-lang/cargo#13942

@prestist

Copy link
Copy Markdown
Contributor Author

Hey, thanks for the feedback all!

@cgwalters The release branch was cut from main when rand was at 0.9.5. After the branch was cut, main got a cargo update that bumped rand. When we then ran cargo update on the release branch to bump the version number, the resolver re-solved the full graph and quietly landed on 0.8.8 — a transitive dep was constraining it below 0.9.x. No build failure, no test failure, just a noisy Cargo.lock diff that was easy to miss in review.

On renovate/dependabot — agree they'd help keep main fresh, but I don't think they'd catch this specific case. The downgrade happened because of branch-cut timing, not because a dep went stale. rust-lang/cargo#13942 has some good context on why the resolver behaves this way.

@Rolv-Apneseth good call on bootc-dev/actions — happy to move the script there if that's the preferred home for it.

Happy to drop this if you think there's a better place to catch it — just wanted to make sure we had a safety net for this failure mode.

@Rolv-Apneseth

Copy link
Copy Markdown
Member

Happy to drop this if you think there's a better place to catch it — just wanted to make sure we had a safety net for this failure mode.

@prestist I don't think we should drop it, I just meant maybe we can have an action hosted there and use it here. Or use the action I linked if we don't think it's worth hosting our own.

I'd personally lean towards using the existing one as long as we pin it.

@prestist

Copy link
Copy Markdown
Contributor Author

Agreed, no reason to duplicate work, if it exists might as well consume it lol

@prestist prestist changed the title rust: add CI check for Cargo.lock dependency downgrades rust: check Cargo.lock downgrades with a pinned action Sep 24, 2026
@prestist
prestist marked this pull request as draft September 24, 2026 16:24
@prestist
prestist force-pushed the rust-lockfile-downgrade-check branch from ea167ba to f1477d9 Compare September 24, 2026 16:35
@prestist

Copy link
Copy Markdown
Contributor Author

@Rolv-Apneseth

Copy link
Copy Markdown
Member

No reply yet on Garfield1002/cargo-no-downgrades#1 (comment), we might need to patch + host ourselves I suppose.

@prestist

prestist commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor Author

We have now forked and patched the action so we do not need to wait for an upstream response on Garfield1002/cargo-no-downgrades#1.

The fixes are on prestist/cargo-no-downgrades:fix-downgrade-detection. This PR now uses prestist/cargo-no-downgrades@e34e8a1894e48119667dbe321f2059d60aaf4d6e, which I verified is the tip of that patched branch—not the unpatched main branch or inherited upstream tag. We keep the full SHA pin for reproducibility and security.

The fork catches dependency switches even when both versions remain locked, and fixes SemVer prerelease ordering. All 38 regression tests pass, including the original coreos-installer rand downgrade; fork CI is green. Local rendering, renderer tests, and actionlint also pass with the new pin.

This is temporary: we can switch back to a verified upstream commit once equivalent fixes land. I have left this PR as a draft because the separate-workflow/label-concurrency and custom-repository coverage work described above is still outstanding.

Update: this PR is now squashed into one commit. Both its commit and the fork fix commit include Assisted-by: OpenCode (AI tooling). The fork SHA above was refreshed after that message-only rewrite; its code is unchanged.

Use a SHA-pinned patched cargo-no-downgrades fork to detect dependency downgrades, including switches hidden by multiple locked versions. Allow intentional downgrades with a label and skip repositories without a base lockfile while upstream issue #1 is unresolved.

Assisted-by: OpenCode (AI tooling)
@prestist
prestist force-pushed the rust-lockfile-downgrade-check branch from f8112f5 to eee9b20 Compare October 22, 2026 15:07
@prestist
prestist marked this pull request as ready for review October 22, 2026 15:58
@prestist
prestist merged commit 44eaa0e into main Oct 22, 2026
1 check passed
@prestist
prestist deleted the rust-lockfile-downgrade-check branch October 22, 2026 16:34
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.

4 participants