Repository navigation
ci: Add static analysis of GitHub Actions with zizmor - #2693
Conversation
matthewfeickert
added
CI
| permissions: | ||
| contents: write # for Git to git push | ||
| runs-on: ubuntu-latest | ||
| environment: ci |
There was a problem hiding this comment.
@henryiii is https://docs.zizmor.sh/audits/#secrets-outside-env going to mean that every GitHub action run triggers a huge number of "deployments" now because this exists in an environment?
There was a problem hiding this comment.
I'm not sure, but I think so. This check is only triggered with the "auditor" level or something like that, right?
There was a problem hiding this comment.
That can be avoided à la data-apis/array-api-extra#699
There was a problem hiding this comment.
There was a problem hiding this comment.
but now the CI isn't running.
what do you mean by this? The required status checks are showing as pending because the job names have been changed
There was a problem hiding this comment.
The required status checks are showing as pending because the job names have been changed
@lucascolley you're (in retrospect, obviously) very correct
test (ubuntu-latest, 3.13)
vs. now
CI/CD / CI (ubuntu-latest, 3.13) (pull_request)
Thanks for pointing this out. Apologies for not having taken the time to think before posting. I didn't realize that in
jobs:
test:
name: CI
runs-on: ${{ matrix.os }}
environment:
name: ci
deployment: false
...that name: CI would override everything else — I thought it was internal naming.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2693 +/- ##
=======================================
Coverage 98.30% 98.30%
=======================================
Files 66 66
Lines 4364 4364
Branches 472 472
=======================================
Hits 4290 4290
Misses 46 46
Partials 28 28
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
366b9bc to
f9c4a87
Compare
📝 WalkthroughWalkthroughThe pull request hardens GitHub Actions checkout behavior, adds CI environment settings and Zizmor analysis, introduces a Dependabot cooldown policy, and refactors version bump commands to use step environment variables. ChangesWorkflow security and automation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
|
On the Scientific Python Discord, Seth Larson pointed out
and @tupi helpfully mentioned that in addition to permissions:
security-events: writeunder https://docs.zizmor.sh/integrations/#manual-integration you can write out a sarif file. |
661ad3b to
a5c3f78
Compare
|
|
||
| on: | ||
| pull_request_target: | ||
| pull_request_target: # zizmor: ignore[dangerous-triggers] |
There was a problem hiding this comment.
I'm not sure if there is anything that can be done about this given https://github.com/amannn/action-semantic-pull-request/tree/45b9ed7cf24087a2f7785bf55be97394ba87e1c2#installation still shows uing pull_request_target.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.pre-commit-config.yaml (1)
82-89: 🧹 Nitpick | 🔵 TrivialAdd zizmor to CI with SARIF output for centralized GitHub Security tab visibility.
Currently zizmor runs only in pre-commit locally. Pair it with a CI workflow step using zizmor's
--format=sarifoutput andgithub/codeql-action/upload-sarifto expose findings centrally in the Security tab. This complements the existing SARIF infrastructure (scorecard.yml, codeql-analysis.yml) and provides auditable, persistent records of workflow security checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pre-commit-config.yaml around lines 82 - 89, Add a CI workflow step that runs the existing zizmor pre-commit hook in CI and emits SARIF so findings appear in the GitHub Security tab: run the zizmor hook (the hook id "zizmor" from the .pre-commit-config entry) in the workflow instead of relying only on local pre-commit, invoke it with the --format=sarif argument (replace or extend the current args array that contains --persona=regular), save the generated SARIF artifact, and add the github/codeql-action/upload-sarif step to upload that SARIF file; ensure the workflow step names and paths reference the same hook id "zizmor" and produce a deterministic SARIF filename for upload..github/workflows/bump-version.yml (1)
52-57:⚠️ Potential issue | 🟠 MajorRe-authenticate before the final
git push.With
persist-credentials: falseat line 57, the PAT token is not persisted to Git config, so thegit pushcommand at line 284 will fail with an authentication error. The workflow needs explicit credential setup before the push step.Suggested fix
- name: Push new tag back to GitHub shell: bash run: | - if [ ${GITHUB_EVENT_INPUTS_DRY_RUN} == 'true' ]; then + if [ "${GITHUB_EVENT_INPUTS_DRY_RUN}" == 'true' ]; then echo "# DRY RUN" else - git push origin ${GITHUB_EVENT_INPUTS_TARGET_BRANCH} --tags + git remote set-url origin "https://x-access-token:${ACCESS_TOKEN}@github.com/${GITHUB_REPOSITORY}.git" + git push origin "${GITHUB_EVENT_INPUTS_TARGET_BRANCH}" --tags fi env: + ACCESS_TOKEN: ${{ secrets.ACCESS_TOKEN }} GITHUB_EVENT_INPUTS_DRY_RUN: ${{ github.event.inputs.dry_run }} GITHUB_EVENT_INPUTS_TARGET_BRANCH: ${{ github.event.inputs.target_branch }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bump-version.yml around lines 52 - 57, The workflow currently sets actions/checkout@v6.0.0 with persist-credentials: false which prevents the PAT being available for the later git push; add an explicit authentication step before the final git push (or change persist-credentials to true) so the push will succeed. Concretely, either set persist-credentials: true on the actions/checkout invocation or add a step prior to the git push that configures credentials (for example, set the remote URL to include the token or run git config/credential helper with the token from secrets.ACCESS_TOKEN) so the git push command can authenticate; update the job that performs the push (the step invoking git push) to run after this authentication 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/semantic-pr-check.yml:
- Line 4: The workflow suppresses the dangerous-triggers audit for
pull_request_target but uses a mutable action tag; update the action reference
amannn/action-semantic-pull-request@v6 to a specific commit SHA (full 40-char
hash) so the workflow is pinned before keeping the suppression for
pull_request_target; locate the action usage string
"amannn/action-semantic-pull-request@v6" and replace the tag with the exact
commit SHA for that release.
In @.github/zizmor.yml:
- Around line 1-3: Replace the global suppression rules.unpinned-uses.disable:
true with a scoped ignore list so the unpinned-uses check remains active
everywhere except trusted workflows; modify the YAML key
rules.unpinned-uses.disable to rules.unpinned-uses.ignore and set it to an array
of specific workflows/locations (e.g., ["ci.yml:100", "tests.yml"]) to suppress
only those findings, leaving the audit enabled for all other workflows.
---
Outside diff comments:
In @.github/workflows/bump-version.yml:
- Around line 52-57: The workflow currently sets actions/checkout@v6.0.0 with
persist-credentials: false which prevents the PAT being available for the later
git push; add an explicit authentication step before the final git push (or
change persist-credentials to true) so the push will succeed. Concretely, either
set persist-credentials: true on the actions/checkout invocation or add a step
prior to the git push that configures credentials (for example, set the remote
URL to include the token or run git config/credential helper with the token from
secrets.ACCESS_TOKEN) so the git push command can authenticate; update the job
that performs the push (the step invoking git push) to run after this
authentication step.
In @.pre-commit-config.yaml:
- Around line 82-89: Add a CI workflow step that runs the existing zizmor
pre-commit hook in CI and emits SARIF so findings appear in the GitHub Security
tab: run the zizmor hook (the hook id "zizmor" from the .pre-commit-config
entry) in the workflow instead of relying only on local pre-commit, invoke it
with the --format=sarif argument (replace or extend the current args array that
contains --persona=regular), save the generated SARIF artifact, and add the
github/codeql-action/upload-sarif step to upload that SARIF file; ensure the
workflow step names and paths reference the same hook id "zizmor" and produce a
deterministic SARIF filename for upload.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 44cb0253-1189-4b27-a811-61998cc7f356
📒 Files selected for processing (17)
.github/dependabot.yml.github/workflows/bump-version.yml.github/workflows/ci-windows.yml.github/workflows/ci.yml.github/workflows/codeql-analysis.yml.github/workflows/dependencies-head.yml.github/workflows/docker.yml.github/workflows/docs.yml.github/workflows/lint.yml.github/workflows/lower-bound-requirements.yml.github/workflows/merged.yml.github/workflows/notebooks.yml.github/workflows/publish-package.yml.github/workflows/release_tests.yml.github/workflows/semantic-pr-check.yml.github/zizmor.yml.pre-commit-config.yaml
|
|
||
| on: | ||
| pull_request_target: | ||
| pull_request_target: # zizmor: ignore[dangerous-triggers] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Finding pull_request_target workflows and non-SHA-pinned actions..."
for wf in $(fd -e yml -e yaml .github/workflows); do
if rg -n '^\s*pull_request_target\s*:' "$wf" >/dev/null; then
echo "== $wf =="
rg -n '^\s*uses:\s+\S+@' "$wf" || true
echo "-- non SHA-pinned uses --"
rg -nP '^\s*uses:\s+\S+@(?![0-9a-f]{40}\b)\S+' "$wf" || true
fi
doneRepository: scikit-hep/pyhf
Length of output: 519
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Correctly search .github/workflows directory
echo "=== Checking .github/workflows directory ==="
fd . '.github/workflows' -e yml -e yaml
echo -e "\n=== Reading semantic-pr-check.yml ==="
cat -n '.github/workflows/semantic-pr-check.yml' 2>/dev/null || echo "File not found"
echo -e "\n=== Checking for actions with non-SHA pins in pull_request_target workflows ==="
for wf in $(fd . '.github/workflows' -e yml -e yaml); do
if rg -q '^\s*pull_request_target\s*:' "$wf"; then
echo "== Workflow: $wf =="
rg -A 50 '^\s*pull_request_target\s*:' "$wf" | head -60
echo ""
fi
doneRepository: scikit-hep/pyhf
Length of output: 2494
Pin action to commit SHA before suppressing dangerous-triggers warning.
Line 4 suppresses the dangerous-triggers audit on pull_request_target, but the workflow runs with an unpinned action tag (amannn/action-semantic-pull-request@v6 on line 28). Replace the mutable tag with a full commit SHA to mitigate supply-chain risk, then the suppression is justified.
🔧 Suggested changes
- pull_request_target: # zizmor: ignore[dangerous-triggers]
+ pull_request_target:- uses: amannn/action-semantic-pull-request@v6
+ uses: amannn/action-semantic-pull-request@<FULL_40_CHAR_COMMIT_SHA>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/semantic-pr-check.yml at line 4, The workflow suppresses
the dangerous-triggers audit for pull_request_target but uses a mutable action
tag; update the action reference amannn/action-semantic-pull-request@v6 to a
specific commit SHA (full 40-char hash) so the workflow is pinned before keeping
the suppression for pull_request_target; locate the action usage string
"amannn/action-semantic-pull-request@v6" and replace the tag with the exact
commit SHA for that release.
|
cc @jarrodmillman @drammock given our recent discussions, for examples of what applying zizmor looks like. |
lucascolley
left a comment
lucascolley
left a comment
There was a problem hiding this comment.
(see comment on ignore)
github-project-automation
Bot
moved this from In progress
to Review in progress
in pyhf v0.8.0
Co-authored-by: Lucas Colley <lucas.colley8@gmail.com>
dc92f02 to
11a312f
Compare
matthewfeickert
left a comment
matthewfeickert
left a comment
There was a problem hiding this comment.
@kratsg @henryiii @lucascolley If you have time can you give this a review pass?
| cooldown: | ||
| default-days: 7 |
There was a problem hiding this comment.
@henryiii we had some discussion on the appropriate duration for cooldowns. Do you have thoughts here?
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.pre-commit-config.yaml (1)
72-72: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the YAML sequence spacing.
Line [72] has too many spaces after the sequence marker. Change
- repo:to- repo:so the configured YAML lint check passes.Proposed fix
-- repo: https://github.com/codespell-project/codespell +- repo: https://github.com/codespell-project/codespell🤖 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 @.pre-commit-config.yaml at line 72, Correct the YAML sequence indentation in the pre-commit configuration by changing the repository entry from “- repo:” to “- repo:”, preserving the existing repository value and surrounding configuration.Source: Linters/SAST tools
.github/workflows/bump-version.yml (1)
52-57: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore authentication for the release push.
When
dry_runisfalse,git pushruns at Line 284.persist-credentials: falsepreventsactions/checkoutfrom leavingACCESS_TOKENconfigured, so the push fails. Setpersist-credentials: trueor configureACCESS_TOKENexplicitly for the push step.🤖 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/bump-version.yml around lines 52 - 57, Update the checkout configuration using actions/checkout@v7 to preserve authentication for the release push by setting persist-credentials to true, ensuring the existing ACCESS_TOKEN is available when git push runs for non-dry-run workflows.Source: MCP tools
🤖 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.
Inline comments:
In @.github/workflows/bump-version.yml:
- Around line 241-244: Ensure OLD_TAG is populated even when the force input
skips the script step by computing it in an unconditional step before the git
tag and annotation logic. Alternatively, detect an empty OLD_TAG and stop before
creating the release annotation; preserve the existing behavior for non-force
runs.
In @.github/zizmor.yml:
- Around line 5-6: Update the policies configuration so the wildcard "*" uses
"hash-pin" by default, and add explicit "ref-pin" entries only for trusted
action namespaces. Preserve SHA pinning as the default for all other actions.
---
Outside diff comments:
In @.github/workflows/bump-version.yml:
- Around line 52-57: Update the checkout configuration using actions/checkout@v7
to preserve authentication for the release push by setting persist-credentials
to true, ensuring the existing ACCESS_TOKEN is available when git push runs for
non-dry-run workflows.
In @.pre-commit-config.yaml:
- Line 72: Correct the YAML sequence indentation in the pre-commit configuration
by changing the repository entry from “- repo:” to “- repo:”, preserving the
existing repository value and surrounding configuration.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7114cdd7-61c1-4a09-8566-5bc55525dabc
📒 Files selected for processing (16)
.github/workflows/bump-version.yml.github/workflows/ci-windows.yml.github/workflows/ci.yml.github/workflows/codeql-analysis.yml.github/workflows/dependencies-head.yml.github/workflows/docker.yml.github/workflows/docs.yml.github/workflows/lint.yml.github/workflows/lower-bound-requirements.yml.github/workflows/merged.yml.github/workflows/notebooks.yml.github/workflows/publish-package.yml.github/workflows/release_tests.yml.github/workflows/semantic-pr-check.yml.github/zizmor.yml.pre-commit-config.yaml
| OLD_TAG=${STEPS_SCRIPT_OUTPUTS_OLD_TAG} | ||
| git tag -n99 --list "${OLD_TAG}" | ||
|
|
||
| NEW_TAG=v${{ github.event.inputs.new_version }} | ||
| NEW_TAG=v${GITHUB_EVENT_INPUTS_NEW_VERSION} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file=".github/workflows/bump-version.yml"
wc -l "$file"
sed -n '40,90p' "$file"
sed -n '230,295p' "$file"
printf '\nAll relevant identifiers and commands:\n'
rg -n -C 3 'old_tag|OLD_TAG|git log|git tag|git push|persist-credentials|force|script:' "$file"Repository: scikit-hep/pyhf
Length of output: 8436
🏁 Script executed:
#!/bin/bash
set -eu
file=".github/workflows/bump-version.yml"
sed -n '85,210p' "$file"
printf '\nRevision resolution for an empty OLD_TAG:\n'
git rev-parse --revs-only '..HEAD' || true
git rev-parse --symbolic --revs-only '..HEAD' || true
git log --pretty=format:'%s' '..HEAD' --regexp-ignore-case --extended-regexp --grep='^([a-z]*?):' | head -20
printf '\nRepository state:\n'
git status --short
git rev-parse --is-inside-work-treeRepository: scikit-hep/pyhf
Length of output: 6569
Populate OLD_TAG when force is true.
When force is true, the script step is skipped, so steps.script.outputs.old_tag is empty. The range at line 247 then becomes ..HEAD, equivalent to HEAD..HEAD, and the release annotation contains no changes. Compute OLD_TAG in an unconditional step, or stop before creating the annotation when it is empty.
🤖 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/bump-version.yml around lines 241 - 244, Ensure OLD_TAG is
populated even when the force input skips the script step by computing it in an
unconditional step before the git tag and annotation logic. Alternatively,
detect an empty OLD_TAG and stop before creating the release annotation;
preserve the existing behavior for non-force runs.
Source: MCP tools
| policies: | ||
| "*": ref-pin |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
pre-commit run zizmor --all-filesRepository: scikit-hep/pyhf
Length of output: 201
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- .github/zizmor.yml ---'
cat -n .github/zizmor.yml
printf '%s\n' '--- zizmor references ---'
rg -n -S 'zizmor|ref-pin|hash-pin|unpinned-uses|policies:' .github .pre-commit-config.yaml pyproject.toml setup.cfg tox.ini 2>/dev/null || true
printf '%s\n' '--- workflow action references ---'
rg -n 'uses:' .github/workflows .github 2>/dev/null || trueRepository: scikit-hep/pyhf
Length of output: 10608
🌐 Web query:
Zizmor policy ref-pin hash-pin default unpinned-uses configuration wildcard
💡 Result:
In Zizmor, the unpinned-uses audit is configured within a zizmor.yml configuration file to control how GitHub Actions uses clauses are validated [1][2]. The policy system uses a hierarchical approach based on repository patterns to determine whether an action must be pinned to a reference (e.g., a tag or branch) or a specific commit hash [3]. Policies for unpinned-uses: The configuration supports defining specific policies for different action repositories using a policies map [1][4][5]. The available policies are [3]: - hash-pin: Requires actions to be pinned to a full commit SHA [3][6]. - ref-pin: Requires actions to be pinned to a git reference, such as a branch or tag [3][6]. - Any: Allows any reference, effectively disabling pinning requirements for matching patterns [3][6]. Configuration and Wildcards: - Wildcard Matching: You can use the * wildcard to match repositories, owners, or subpaths [1][7]. For example, "actions/" matches all repositories under the actions organization, and "" serves as a catch-all pattern [1][5]. - Default Policy: If a configuration omits an explicit * catch-all rule, Zizmor defaults to hash-pin for any uses clause that does not match a more specific policy [3]. - Pattern Specificity: When multiple rules apply, Zizmor uses a specificity-based ordering (exact matches take precedence over globs) to resolve which policy to apply [3][7]. Example Configuration: The following example demonstrates how to enforce different pinning requirements based on repository source: rules: unpinned-uses: config: policies: "actions/": ref-pin "my-org/": hash-pin "*": hash-pin In this example, actions under the official actions organization are allowed to use ref-pin (e.g., v1), while all other actions (captured by * or the my-org pattern) must be pinned to a specific commit hash [3][4][5].
Citations:
- 1: https://docs.zizmor.sh/configuration/
- 2: https://docs.zizmor.sh/audits/
- 3: https://github.com/zizmorcore/zizmor/blob/563b7b25/crates/zizmor/src/config/mod.rs
- 4: https://exasol.github.io/python-toolbox/10.3.0/user_guide/features/managing_dependencies/zizmor_configuration.html
- 5: https://gitea.rs.h3.ink/actions/setup-java/raw/branch/copilot/support-pinning-java-version-latest/.github/zizmor.yml
- 6: https://github.com/zizmorcore/zizmor/blob/563b7b25/crates/zizmor/src/audit/unpinned_uses.rs
- 7: feat(uses): add glob pattern support for repository matching zizmorcore/zizmor#1559
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
config = Path(".github/zizmor.yml").read_text()
policy = re.search(r'^\s*"\*":\s*(\S+)\s*$', config, re.MULTILINE)
if not policy:
raise SystemExit("wildcard policy not found")
refs = []
for path in Path(".github/workflows").glob("*.yml"):
for line_no, line in enumerate(path.read_text().splitlines(), 1):
match = re.search(r"\buses:\s*([^#\s]+)", line)
if match:
action, ref = match.group(1).rsplit("@", 1)
refs.append((str(path), line_no, action, ref))
sha_refs = [item for item in refs if re.fullmatch(r"[0-9a-fA-F]{40}", item[3])]
symbolic_refs = [item for item in refs if not re.fullmatch(r"[0-9a-fA-F]{40}", item[3])]
print(f"wildcard policy: {policy.group(1)}")
print(f"action references: {len(refs)}")
print(f"full-SHA references: {len(sha_refs)}")
print(f"symbolic references: {len(symbolic_refs)}")
print("symbolic references outside actions/*:")
for path, line_no, action, ref in symbolic_refs:
if not action.startswith("actions/"):
print(f"{path}:{line_no}: {action}@{ref}")
PYRepository: scikit-hep/pyhf
Length of output: 1365
Keep SHA pinning as the default policy.
The "*": ref-pin rule permits mutable tags and branches for all actions, including third-party actions. Scope ref-pin to trusted namespaces and set "*": hash-pin.
🤖 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/zizmor.yml around lines 5 - 6, Update the policies configuration so
the wildcard "*" uses "hash-pin" by default, and add explicit "ref-pin" entries
only for trusted action namespaces. Preserve SHA pinning as the default for all
other actions.
lucascolley
left a comment
lucascolley
left a comment
There was a problem hiding this comment.
all seems reasonable to me!
Thanks very much for your time and help @lucascolley! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.pre-commit-config.yaml:
- Line 90: Align the `# use default 'regular' persona` comment with the
surrounding `zizmor` hook fields by reducing its indentation from six spaces to
four, without changing the comment text or configuration.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a03292ae-a4eb-423a-ab30-401ded1e3c95
📒 Files selected for processing (1)
.pre-commit-config.yaml
| - repo: https://github.com/zizmorcore/zizmor-pre-commit | ||
| rev: v1.29.0 | ||
| hooks: | ||
| # use default 'regular' persona |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
sed -n '75,100p' .pre-commit-config.yaml
printf '%s\n' '--- relevant lint and CI references ---'
rg -n -i 'yamllint|yaml.?lint|pre-commit|comments-indentation|failure|warnings' \
.github .pre-commit-config.yaml pyproject.toml setup.cfg tox.ini Makefile 2>/dev/null || true
printf '%s\n' '--- repository files related to lint configuration ---'
git ls-files | rg -i '(^|/)(yamllint|pre-commit|.*lint.*|.*workflow.*|.*ci.*)' | head -200Repository: scikit-hep/pyhf
Length of output: 3947
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- lint workflow ---'
sed -n '1,220p' .github/workflows/lint.yml
printf '%s\n' '--- target lines with whitespace markers ---'
nl -ba .pre-commit-config.yaml | sed -n '84,94p'
sed -n '90p' .pre-commit-config.yaml | cat -A
printf '%s\n' '--- available YAML lint tools ---'
command -v yamllint || true
command -v pre-commit || true
python3 - <<'PY'
try:
import yamllint
print("python yamllint: available")
except Exception:
print("python yamllint: unavailable")
PYRepository: scikit-hep/pyhf
Length of output: 838
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- current file ---'
yamllint -d default .pre-commit-config.yaml || true
printf '%s\n' '--- four-space comment variant (stdin only) ---'
sed '90s/^ / /' .pre-commit-config.yaml | yamllint -d default - || trueRepository: scikit-hep/pyhf
Length of output: 1591
Align the comment indentation.
yamllint reports comments-indentation because line 90 has six leading spaces instead of four. Match the comment indentation to the zizmor hook fields.
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 90-90: comment not indented like content
(comments-indentation)
🤖 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 @.pre-commit-config.yaml at line 90, Align the `# use default 'regular'
persona` comment with the surrounding `zizmor` hook fields by reducing its
indentation from six spaces to four, without changing the comment text or
configuration.
Source: Linters/SAST tools
github-project-automation
Bot
moved this from Review in progress
to Reviewer approved
in pyhf v0.8.0
4 participants
Description
Resolves #2685
Checklist Before Requesting Reviewer
Before Merging
For the PR Assignees:
Summary by CodeRabbit
Chores