[zizmor] ignore superfluous-actions - #7548
Conversation
π WalkthroughWalkthroughReplaced several uses of a pinned or manual Rust toolchain installation with Changes
Estimated code review effortπ― 3 (Moderate) | β±οΈ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
π₯ Pre-merge checks | β 2 | β 1β Failed checks (1 warning)
β Passed checks (2 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (2)
.github/workflows/ci.yaml (2)
414-427:β οΈ Potential issue | π΄ CriticalUpdate the miri command to use the correct toolchain variable.
Line 414 defines the job environment variable
RUSTUP_TOOLCHAIN: nightly, but Line 427 references the non-existentenv.NIGHTLY_CHANNEL. SinceRUSTUP_TOOLCHAINis already set at the job level, the+${{ env.NIGHTLY_CHANNEL }}prefix should be removed entirelyβcargo will automatically use the nightly toolchain without it.π οΈ Proposed fix
- name: Run tests under miri - run: cargo +${{ env.NIGHTLY_CHANNEL }} miri test -p rustpython-vm -- miri_test + run: cargo miri test -p rustpython-vm -- miri_testπ€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 414 - 427, The workflow uses RUSTUP_TOOLCHAIN: nightly but the "Run tests under miri" step calls cargo with a non-existent toolchain variable (+${{ env.NIGHTLY_CHANNEL }}); remove the +${{ env.NIGHTLY_CHANNEL }} prefix from the "cargo +... miri test -p rustpython-vm -- miri_test" command so it simply runs "cargo miri test -p rustpython-vm -- miri_test" and relies on the job-level RUSTUP_TOOLCHAIN setting.
360-364:β οΈ Potential issue | π΄ CriticalSplit the
rustfmtinstall into its own step.This step has both
usesandrun, which violates GitHub Actions workflow syntax. A single step cannot define both keys. Moverustup component add rustfmt --toolchain=stableinto a separate step.π οΈ Proposed fix
- uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: python-version: ${{ env.PYTHON_VERSION }} - run: rustup component add rustfmt --toolchain=stable + - name: Install rustfmt + run: rustup component add rustfmt --toolchain=stableπ€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 360 - 364, The workflow step currently mixes an action usage and a run command (the step with uses: actions/setup-python@a309ff8b426b58ec0e45f0f869d46889d02405 and the run: rustup component add rustfmt --toolchain=stable), which is invalid; fix it by removing the run from that step and creating a new standalone step that only runs the command "rustup component add rustfmt --toolchain=stable" (ensure the new step appears after the setup-python step and uses only the run key), referencing the existing actions/setup-python step to keep ordering.
π€ Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 414-427: The workflow uses RUSTUP_TOOLCHAIN: nightly but the "Run
tests under miri" step calls cargo with a non-existent toolchain variable (+${{
env.NIGHTLY_CHANNEL }}); remove the +${{ env.NIGHTLY_CHANNEL }} prefix from the
"cargo +... miri test -p rustpython-vm -- miri_test" command so it simply runs
"cargo miri test -p rustpython-vm -- miri_test" and relies on the job-level
RUSTUP_TOOLCHAIN setting.
- Around line 360-364: The workflow step currently mixes an action usage and a
run command (the step with uses:
actions/setup-python@a309ff8b426b58ec0e45f0f869d46889d02405 and the run: rustup
component add rustfmt --toolchain=stable), which is invalid; fix it by removing
the run from that step and creating a new standalone step that only runs the
command "rustup component add rustfmt --toolchain=stable" (ensure the new step
appears after the setup-python step and uses only the run key), referencing the
existing actions/setup-python step to keep ordering.
βΉοΈ Review info
βοΈ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b9692e1e-c0cf-422a-9d2f-06acfe2af859
π Files selected for processing (1)
.github/workflows/ci.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (1)
.github/workflows/ci.yaml (1)
414-427:β οΈ Potential issue | π΄ CriticalUpdate the Miri command to reference the correct environment variable.
Line 427 references
env.NIGHTLY_CHANNEL, which is undefined. The job definesRUSTUP_TOOLCHAIN: nightlyat line 414. The cargo command will fail when expanded tocargo +(with empty toolchain). Update to use the defined variable:Suggested fix
- run: cargo +${{ env.NIGHTLY_CHANNEL }} miri test -p rustpython-vm -- miri_test + run: cargo +${{ env.RUSTUP_TOOLCHAIN }} miri test -p rustpython-vm -- miri_testπ€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yaml around lines 414 - 427, The Miri test step uses an undefined env var NIGHTLY_CHANNEL; update the "Run tests under miri" step to use the defined RUSTUP_TOOLCHAIN variable (or the literal "nightly") so the cargo +<toolchain> invocation expands correctly β change the cargo command that references env.NIGHTLY_CHANNEL to reference env.RUSTUP_TOOLCHAIN (or hardcode "nightly") in that step.
π€ Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yaml:
- Around line 50-53: The Miri job is using an undefined env var `${{
env.NIGHTLY_CHANNEL }}` and several "stable" jobs install stable but don't force
it for subsequent cargo commands; update the Miri step that runs `cargo +${{
env.NIGHTLY_CHANNEL }} miri test` to use a concrete toolchain (either `cargo
+nightly miri test` or `cargo +${{ env.RUSTUP_TOOLCHAIN }} miri test`) and
ensure the MUST-FIX env is `RUSTUP_TOOLCHAIN: nightly` where that job sets it;
for each job that installs `stable` (where `rustup toolchain install stable` is
run) either add `RUSTUP_TOOLCHAIN: stable` at the job-level env or change
subsequent invocations like `cargo clippy`, `cargo shear`, and other `cargo ...`
steps to explicitly run `cargo +stable ...` so they deterministically use the
stable toolchain.
---
Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 414-427: The Miri test step uses an undefined env var
NIGHTLY_CHANNEL; update the "Run tests under miri" step to use the defined
RUSTUP_TOOLCHAIN variable (or the literal "nightly") so the cargo +<toolchain>
invocation expands correctly β change the cargo command that references
env.NIGHTLY_CHANNEL to reference env.RUSTUP_TOOLCHAIN (or hardcode "nightly") in
that step.
πͺ Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
βΉοΈ Review info
βοΈ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 5bda2a95-4ca3-4b4f-9a59-1eb46e269f5e
π Files selected for processing (1)
.github/workflows/ci.yaml
|
any reason to change? |
The initial motivation of this PR was to resolve "note" level warnings from zizmor, as the functionality of |
|
After reading zizmorcore/zizmor#1817 I've changed my mind. I'll make zizmor to ignore this, and shift this PR to ignore this rule instead |
rustup in ci.yamlsuperfluous-actions
There was a problem hiding this comment.
Actionable comments posted: 2
π€ Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yaml:
- Line 53: Replace all uses of the reusable action reference
dtolnay/rust-toolchain that use mutable tags (e.g., `@stable` and `@master`) with
their corresponding full 40-character commit SHAs to avoid mutable refs; update
each occurrence (the instances currently written as
dtolnay/rust-toolchain@stable and dtolnay/rust-toolchain@master) to the exact
commit SHA and add a short comment on the same line preserving the original
tag/intent (for example: # originally `@stable`) so CI remains reproducible and
reviewers know the intended channel.
In @.github/workflows/release.yml:
- Line 55: Replace both mutable references of the dtolnay/rust-toolchain action
("uses: dtolnay/rust-toolchain@stable") with the repository's immutable
40-character commit SHA for the current stable tag and add an inline comment
noting which stable tag the SHA corresponds to; locate and update the two
occurrences in the release workflow where "uses: dtolnay/rust-toolchain@stable"
appears, fetch the current commit SHA from the dtolnay/rust-toolchain GitHub
repo for the stable tag, and use that SHA in place of "@stable" so the workflow
is pinned to a specific commit.
πͺ Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
βΉοΈ Review info
βοΈ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a74f15d1-21fe-4bae-b98f-96e3a0c6ab80
π Files selected for processing (3)
.github/workflows/ci.yaml.github/workflows/release.yml.github/zizmor.yml
β Files skipped from review due to trivial changes (1)
- .github/zizmor.yml
| persist-credentials: false | ||
|
|
||
| - uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9 | ||
| - uses: dtolnay/rust-toolchain@stable |
There was a problem hiding this comment.
π§© Analysis chain
π Script executed:
#!/bin/bash
set -euo pipefail
echo "All dtolnay/rust-toolchain references in workflows:"
rg -nP 'uses:\s*dtolnay/rust-toolchain@' .github/workflows
echo
echo "Non-SHA refs (should be empty after pinning):"
rg -nP 'uses:\s*dtolnay/rust-toolchain@(?![0-9a-f]{40}\b)\S+' .github/workflows
echo
echo "Current upstream heads for convenience (pick reviewed SHAs, don't auto-trust):"
git ls-remote https://github.com/dtolnay/rust-toolchain refs/heads/master refs/heads/stable || trueRepository: RustPython/RustPython
Length of output: 2329
π Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Verifying exact line ranges in ci.yaml ==="
echo "Line 53:"
sed -n '51,55p' .github/workflows/ci.yaml
echo
echo "Lines 172β174:"
sed -n '170,176p' .github/workflows/ci.yaml
echo
echo "Line 255:"
sed -n '253,257p' .github/workflows/ci.yaml
echo
echo "Line 368:"
sed -n '366,370p' .github/workflows/ci.yaml
echo
echo "Lines 426β429:"
sed -n '424,431p' .github/workflows/ci.yaml
echo
echo "Lines 452β454:"
sed -n '450,456p' .github/workflows/ci.yaml
echo
echo "Lines 529β531:"
sed -n '527,533p' .github/workflows/ci.yamlRepository: RustPython/RustPython
Length of output: 1512
Pin dtolnay/rust-toolchain action references to commit SHAs instead of mutable tags.
Lines 53, 172β174, 255, 368, 426β429, 452β454, and 529β531 use mutable refs (@stable and @master). These can change without notice, compromising CI reproducibility and introducing supply chain risk. Line 426 with @master is especially problematic. Pin all references to full 40-character commit SHAs with a comment preserving the original tag/intent.
Suggested pattern
- - uses: dtolnay/rust-toolchain@stable
+ - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable
with:
components: clippy- - uses: dtolnay/rust-toolchain@master
+ - uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9 # master
with:
toolchain: ${{ env.NIGHTLY_CHANNEL }}
components: miriπ Committable suggestion
βΌοΈ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: dtolnay/rust-toolchain@stable | |
| - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable |
π€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yaml at line 53, Replace all uses of the reusable
action reference dtolnay/rust-toolchain that use mutable tags (e.g., `@stable` and
`@master`) with their corresponding full 40-character commit SHAs to avoid mutable
refs; update each occurrence (the instances currently written as
dtolnay/rust-toolchain@stable and dtolnay/rust-toolchain@master) to the exact
commit SHA and add a short comment on the same line preserving the original
tag/intent (for example: # originally `@stable`) so CI remains reproducible and
reviewers know the intended channel.
| persist-credentials: false | ||
|
|
||
| - uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9 | ||
| - uses: dtolnay/rust-toolchain@stable |
There was a problem hiding this comment.
π§© Analysis chain
π Script executed:
#!/bin/bash
set -euo pipefail
echo "All rust-toolchain refs in release workflow:"
rg -nP 'uses:\s*dtolnay/rust-toolchain@' .github/workflows/release.yml
echo
echo "Non-SHA refs (should be empty after fix):"
rg -nP 'uses:\s*dtolnay/rust-toolchain@(?![0-9a-f]{40}\b)\S+' .github/workflows/release.ymlRepository: RustPython/RustPython
Length of output: 340
Pin dtolnay/rust-toolchain to an immutable commit SHA in the release workflow.
Lines 55 and 92 use mutable @stable reference. In a release workflow, mutable refs allow upstream changes to alter build and release behavior without review.
Suggested change
Replace both occurrences of @stable with a specific commit SHA (40 characters) and add a comment indicating the stable tag.
- - uses: dtolnay/rust-toolchain@stable
+ - uses: dtolnay/rust-toolchain@<commit-sha> # stableDetermine the current commit SHA for the stable tag from the dtolnay/rust-toolchain repository.
π€ Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml at line 55, Replace both mutable references of
the dtolnay/rust-toolchain action ("uses: dtolnay/rust-toolchain@stable") with
the repository's immutable 40-character commit SHA for the current stable tag
and add an inline comment noting which stable tag the SHA corresponds to; locate
and update the two occurrences in the release workflow where "uses:
dtolnay/rust-toolchain@stable" appears, fetch the current commit SHA from the
dtolnay/rust-toolchain GitHub repo for the stable tag, and use that SHA in place
of "@stable" so the workflow is pinned to a specific commit.
Summary by CodeRabbit