[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