ci: realign CI/release/publish workflows#932
Conversation
Modernize the GitHub Actions setup and fix the "always publishing" bug. - Replace validate.yml with ci.yml: PRs + push-to-main, concurrency-cancel, fmt/clippy/test/check(windows) jobs, Swatinem/rust-cache@v2 (save only from main), toolchain pinned via a new rust-toolchain.toml. - Embed crates.io publish into the cargo-dist release as a reusable publish-jobs entry (publish-crates.yml), regenerated into release.yml. Publish now runs only on a real tag release after host succeeds. - Delete publish.yml, whose workflow_run trigger fired crates publish after every PR's Release run. e2e remains a manual workflow_dispatch step, unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR replaces older validation and publish workflow paths with a new CI workflow, a reusable crates.io publish workflow, release-job wiring for that publish step, an updated dist publish job list, and a pinned Rust toolchain with lint and format components. ChangesWorkflow automation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/release.yml (1)
335-347: ⚡ Quick winThe elevated permissions may be unnecessary for crates.io publishing.
The
custom-publish-cratesjob grantsid-token: writeandpackages: writepermissions. However, crates.io publishing only requires theCARGO_REGISTRY_TOKENsecret (which is provided viasecrets: inherit).
id-token: writeis for OIDC/trusted publishing (not applicable to crates.io)packages: writeis for GitHub Packages (not crates.io)Consider removing the
permissionsblock or restricting it tocontents: readunless these elevated permissions are needed for a future use case.🔒 Proposed fix to remove excessive permissions
custom-publish-crates: needs: - plan - host if: ${{ !fromJson(needs.plan.outputs.val).announcement_is_prerelease || fromJson(needs.plan.outputs.val).publish_prereleases }} uses: ./.github/workflows/publish-crates.yml with: plan: ${{ needs.plan.outputs.val }} secrets: inherit - # publish jobs get escalated permissions - permissions: - "id-token": "write" - "packages": "write"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 335 - 347, The `custom-publish-crates` job in the release.yml workflow has unnecessary elevated permissions. Remove the `permissions` block (containing `id-token: write` and `packages: write`) from this job since crates.io publishing only requires the `CARGO_REGISTRY_TOKEN` secret provided via `secrets: inherit`. The `id-token` permission is for OIDC/trusted publishing which is not applicable to crates.io, and `packages` is for GitHub Packages which is also not needed. Either delete the permissions block entirely or replace it with minimal permissions like `contents: read` if any permission is needed..github/workflows/ci.yml (1)
1-81: ⚖️ Poor tradeoffConsider tightening workflow security posture (optional improvements).
The workflow follows standard patterns but could adopt stricter security practices:
Pin actions to commit SHAs: Currently using version tags (
@v4,@v3). Pinning to full commit hashes prevents supply-chain attacks via tag hijacking, though it increases maintenance overhead.Restrict workflow permissions: Add a top-level
permissions:block to limit the workflow to read-only access. The current default grants broader permissions than needed for CI checks.Disable credential persistence: Set
persist-credentials: falsein allactions/checkoutsteps to prevent credentials from being accessible to subsequent steps or artifacts.🔒 Example hardened configuration
name: CI + +permissions: + contents: read on: pull_request:steps: - uses: actions/checkout@v4 + with: + persist-credentials: falseFor SHA pinning, tools like Dependabot can automate updates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 1 - 81, Harden the CI workflow by applying the requested security improvements in the top-level workflow definition: add a restrictive permissions block, set persist-credentials to false on every actions/checkout step, and replace the version-tagged action references used by actions/checkout, arduino/setup-protoc, and Swatinem/rust-cache with pinned commit SHAs. Keep the existing job structure and behavior unchanged while updating the workflow configuration..github/workflows/publish-crates.yml (1)
8-12: 💤 Low valueThe
planinput is declared but never used.The workflow accepts a
planinput (described as the cargo-dist release plan) but doesn't reference it anywhere. If the plan determines which packages to publish or provides metadata for the release, consider extracting and using that information. Otherwise, this input can be removed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish-crates.yml around lines 8 - 12, The publish-crates workflow declares a `plan` input but never uses it; update the `publish-crates` job/workflow logic to either consume the `plan` value in the relevant publishing steps (for example in the main publish path that uses cargo-dist release metadata) or remove the unused `plan` input from the workflow definition if it is not needed anywhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 1-81: Harden the CI workflow by applying the requested security
improvements in the top-level workflow definition: add a restrictive permissions
block, set persist-credentials to false on every actions/checkout step, and
replace the version-tagged action references used by actions/checkout,
arduino/setup-protoc, and Swatinem/rust-cache with pinned commit SHAs. Keep the
existing job structure and behavior unchanged while updating the workflow
configuration.
In @.github/workflows/publish-crates.yml:
- Around line 8-12: The publish-crates workflow declares a `plan` input but
never uses it; update the `publish-crates` job/workflow logic to either consume
the `plan` value in the relevant publishing steps (for example in the main
publish path that uses cargo-dist release metadata) or remove the unused `plan`
input from the workflow definition if it is not needed anywhere.
In @.github/workflows/release.yml:
- Around line 335-347: The `custom-publish-crates` job in the release.yml
workflow has unnecessary elevated permissions. Remove the `permissions` block
(containing `id-token: write` and `packages: write`) from this job since
crates.io publishing only requires the `CARGO_REGISTRY_TOKEN` secret provided
via `secrets: inherit`. The `id-token` permission is for OIDC/trusted publishing
which is not applicable to crates.io, and `packages` is for GitHub Packages
which is also not needed. Either delete the permissions block entirely or
replace it with minimal permissions like `contents: read` if any permission is
needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f1ae5ad4-81e6-415c-b26d-25ea7393c8c5
📒 Files selected for processing (7)
.github/workflows/ci.yml.github/workflows/publish-crates.yml.github/workflows/publish.yml.github/workflows/release.yml.github/workflows/validate.ymldist-workspace.tomlrust-toolchain.toml
💤 Files with no reviewable changes (2)
- .github/workflows/publish.yml
- .github/workflows/validate.yml
1 participant
Why
The GitHub Actions setup had drifted and carried a real bug:
publish.ymltriggered onworkflow_runof theReleaseworkflow, which itself runs on every pull request — so crates.io publish was attempted after essentially every PR's Release run, not just on real tag releases. CI (validate.yml) also used the deprecatedactions-rs/*actions with no Rust caching.This PR realigns things into a clean, modern set of workflows.
Changes
CI — replace
validate.ymlwithci.yml:main, with aconcurrencygroup that cancels superseded runs.fmt,clippy,test, andcheckon Windows (preserving the old cross-OS coverage).Swatinem/rust-cache@v2on every job, writing the cache only frommain(save-if) so PRs read but don't pollute it.rust-toolchain.tomlpins1.89+ rustfmt/clippy as the single source of truth for CI, local dev, and dist.Release embeds publish — crates.io publish is now a cargo-dist
publish-jobsreusable workflow (publish-crates.yml), wired into the regeneratedrelease.ymlascustom-publish-crates(needs: [plan, host],secrets: inherit). It runs only on a real tag release, afterhostsucceeds, structurally eliminating the every-PR publish.dist-workspace.toml:publish-jobs = ["homebrew", "./publish-crates"].release.ymlwas regenerated with dist 0.29.0 (version-matched) — the diff is just the publish-crates job.Delete
publish.yml— the brokenworkflow_run-triggered file.e2e — unchanged: stays a manual
workflow_dispatchpreliminary step, intentionally not wired into the release pipeline.Verification
release.ymlis dist-generated anddist generate --checkreports in-sync.main(CI will exercise them on this PR).🤖 Generated with Claude Code
Summary by CodeRabbit