🔒 Fix all warnings reported by zizmor#296
Conversation
Signed-off-by: burgholzer <burgholzer@me.com>
burgholzer
added
continuous integration
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds workflow-level concurrency and explicit permissions to multiple GitHub Actions reusable workflows, disables credential persistence for checkout steps, introduces a zizmor pre-commit hook, and records the fix in CHANGELOG.md. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-10-11T13:21:15.212ZApplied to files:
🔇 Additional comments (3)
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (20)
.github/workflows/release-drafter.yml.github/workflows/reusable-change-detection.yml.github/workflows/reusable-cpp-coverage.yml.github/workflows/reusable-cpp-linter.yml.github/workflows/reusable-cpp-tests-macos.yml.github/workflows/reusable-cpp-tests-ubuntu.yml.github/workflows/reusable-cpp-tests-windows.yml.github/workflows/reusable-mqt-core-update.yml.github/workflows/reusable-python-coverage.yml.github/workflows/reusable-python-linter.yml.github/workflows/reusable-python-packaging-sdist.yml.github/workflows/reusable-python-packaging-wheel-build.yml.github/workflows/reusable-python-packaging-wheel-cibuildwheel.yml.github/workflows/reusable-python-tests.yml.github/workflows/reusable-qiskit-upstream-issue.yml.github/workflows/reusable-qiskit-upstream-tests.yml.github/workflows/templating.yml.github/workflows/update-major-minor-tag.yml.pre-commit-config.yamlCHANGELOG.md
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core PR: 1326
File: python/mqt/core/__init__.py:22-22
Timestamp: 2025-11-24T10:19:41.147Z
Learning: In the munich-quantum-toolkit/core repository, Ruff is configured with 'ALL' rules enabled by default, and only specific rules are selectively disabled. When reviewing changes that enable previously-disabled rules (like PLC0415), noqa directives for those rules become necessary and should be retained.
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
📚 Learning: 2025-10-11T13:21:15.212Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/ddsim PR: 674
File: .github/workflows/cd.yml:56-56
Timestamp: 2025-10-11T13:21:15.212Z
Learning: In the ddsim repository, modifying .github/workflows/cd.yml intentionally triggers CD workflow testing in CI through a change-detection mechanism in ci.yml. The change-detection job outputs a run-cd flag that controls whether build-sdist and build-wheel jobs execute in CI, allowing testing of the CD workflow (including wheel building and testing) before actual releases.
Applied to files:
.github/workflows/reusable-python-packaging-wheel-cibuildwheel.yml.github/workflows/reusable-change-detection.yml.github/workflows/reusable-python-packaging-wheel-build.yml.github/workflows/reusable-python-packaging-sdist.yml
📚 Learning: 2025-12-02T10:08:36.022Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/core-plugins-catalyst PR: 1
File: .github/workflows/cd.yml:40-48
Timestamp: 2025-12-02T10:08:36.022Z
Learning: In munich-quantum-toolkit workflows, the reusable-python-packaging-sdist.yml workflow uploads the sdist artifact with the name `cibw-sdist` (or `dev-cibw-sdist` for pull requests), which means it is covered by the `cibw-*` glob pattern when downloading artifacts for deployment.
Applied to files:
.github/workflows/reusable-python-packaging-wheel-cibuildwheel.yml.github/workflows/reusable-python-packaging-sdist.yml
📚 Learning: 2025-10-14T14:37:38.047Z
Learnt from: burgholzer
Repo: munich-quantum-toolkit/yaqs PR: 212
File: CHANGELOG.md:12-15
Timestamp: 2025-10-14T14:37:38.047Z
Learning: In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: "- $TITLE ([#$NUMBER]($URL)) ([**AUTHOR**](https://github.com/$AUTHOR))". Issue references should not be included in changelog entries; the PR number is sufficient for traceability.
Applied to files:
CHANGELOG.md
📚 Learning: 2025-12-05T17:45:37.602Z
Learnt from: denialhaag
Repo: munich-quantum-toolkit/core PR: 1360
File: .github/workflows/reusable-mlir-tests.yml:40-43
Timestamp: 2025-12-05T17:45:37.602Z
Learning: In the munich-quantum-toolkit/core repository, patch releases of LLVM dependencies don't require documentation updates, changelog entries, or additional tests beyond what's validated by passing CI checks.
Applied to files:
CHANGELOG.md
🔇 Additional comments (43)
.github/workflows/templating.yml (3)
16-18: LGTM! Concurrency controls properly configured.The concurrency grouping pattern correctly uses
github.head_reffor PR-specific groups and falls back togithub.run_idfor non-PR events. Settingcancel-in-progress: trueprevents resource waste from outdated workflow runs.
20-21: LGTM! Least-privilege permissions model implemented.The top-level read-only permissions with job-level escalation follows security best practices. This ensures that any future jobs won't inherit unnecessary write permissions by default.
28-29: LGTM! Permission comments add clarity.The updated comments clearly document why elevated permissions are required for this templating workflow, improving maintainability and security audit trails.
.github/workflows/reusable-qiskit-upstream-tests.yml (3)
34-36: LGTM! Effective concurrency control.The concurrency group correctly uses
github.workflowwith either the PR head ref or run ID, and canceling in-progress runs prevents resource waste from duplicate executions. This is a standard security and efficiency pattern recommended by zizmor.
38-39: LGTM! Least-privilege permissions applied.Explicitly declaring
contents: readfollows the principle of least privilege and is sufficient for all steps in this workflow (checkout, tool setup, and testing). This addresses zizmor's recommendation to avoid implicit default permissions.
53-53: LGTM! Prevents credential leakage.Setting
persist-credentials: falseensures GitHub credentials are not stored in the local git configuration, preventing potential credential leakage or misuse in subsequent workflow steps. This is a security best practice recommended by zizmor..github/workflows/reusable-qiskit-upstream-issue.yml (2)
18-20: LGTM! Concurrency configuration is appropriate for this use case.The concurrency group correctly uses
github.workflowwhich, in reusable workflows, refers to the caller workflow's name. Combined withgithub.head_ref || github.run_id, this ensures that multiple runs from the same caller workflow will cancel previous in-progress runs, which is appropriate for an issue-creation workflow.
22-23: LGTM! Explicit permissions follow security best practices.The top-level
permissionsdeclaration restricts default access to read-only, following the principle of least privilege. The job at lines 29-31 appropriately declares the specific write permissions it needs (issues: write), ensuring the workflow has only the minimum necessary access.CHANGELOG.md (2)
12-22: LGTM! Changelog entry follows the correct format.The changelog entry is properly formatted and follows the project's template. The fix is appropriately categorized under the "Fixed" section with correct emoji, PR reference, and author attribution.
Based on learnings, the format matches the expected template: "- $TITLE (#$NUMBER) (AUTHOR)".
249-249: LGTM! PR link properly added.The PR link reference is correctly formatted and placed in the appropriate section.
.pre-commit-config.yaml (1)
81-94: Verify intent to use zizmor v1.19.0, which appears newer than officially documented releases.The repository URL
https://github.com/zizmorcore/zizmor-pre-commitis correct. However, while v1.19.0 is referenced in commits and PyPI artifacts exist, official documentation examples show v1.14.2. Confirm this is an intentional newer version or consider pinning to an officially documented release.The permissive settings (
--min-severity=informational,--min-confidence=low) are appropriate for comprehensive security coverage but may generate higher volumes of findings..github/workflows/update-major-minor-tag.yml (2)
27-28: LGTM! Proper implementation of least-privilege permissions.The two-tier permissions structure correctly implements least privilege by defaulting to read-only at the workflow level and escalating to write only at the job level where needed. The inline comment clearly documents why the elevated permission is required.
Also applies to: 35-35
38-39: LGTM! Security improvement by disabling credential persistence.Setting
persist-credentials: falseprevents the checkout action from persisting the GitHub token in the local git configuration, reducing the risk of accidental credential exposure. The downstreamjoerick/update-vX.Y-tag-actionwill still have access toGITHUB_TOKENthrough the runner context..github/workflows/reusable-python-coverage.yml (3)
18-19: LGTM! Follows least-privilege principle.The top-level
contents: readpermission follows security best practices. The job correctly overrides with more specific permissions (id-token: write for Codecov).
31-32: LGTM! Prevents credential leakage.Setting
persist-credentials: falseprevents the GITHUB_TOKEN from persisting between steps, which is a security best practice. This workflow only needs to download artifacts and upload to Codecov, so no push/write access is required.
14-16: LGTM! Excellent security improvement.The concurrency control with
cancel-in-progress: trueprevents resource waste from redundant runs and follows the recommended pattern for reusable workflows. This change was appropriately added as part of the zizmor security scanning initiative to address GitHub Actions security best practices..github/workflows/reusable-cpp-tests-macos.yml (1)
57-59: LGTM! Consistent security improvements.The additions of concurrency controls, explicit permissions, and disabled credential persistence align with security best practices and are consistent with the PR's objective of fixing zizmor warnings. The workflow performs build and test operations that don't require write access or credential persistence.
Also applies to: 61-62, 77-77
.github/workflows/release-drafter.yml (3)
18-20: LGTM! Appropriate concurrency control for release management.The concurrency grouping ensures only one release draft update runs at a time, preventing conflicts when multiple PRs or commits are processed simultaneously.
29-30: LGTM! Helpful documentation.The added comments clarify why these elevated permissions are needed, improving maintainability.
35-35: LGTM! Explicit token passing is a security best practice.Using
github.tokenexplicitly in the environment is the recommended approach (vs. relying on implicit token availability) and likely addresses a zizmor finding about explicit authentication..github/workflows/reusable-cpp-tests-ubuntu.yml (1)
57-59: LGTM! Consistent security hardening.The security improvements are correctly applied following the same pattern as other test workflows in this PR. The workflow doesn't require write permissions or credential persistence for its build and test operations.
Also applies to: 61-62, 77-77
.github/workflows/reusable-python-linter.yml (1)
42-44: LGTM! Security improvements complete the pattern.The additions follow the same security hardening pattern applied consistently across all workflows in this PR. The linter workflow appropriately has read-only permissions as it only performs static analysis without modifying the repository.
Also applies to: 46-47, 59-59
.github/workflows/reusable-cpp-linter.yml (3)
66-67: LGTM! Proper least-privilege permissions.The top-level
contents: readestablishes a minimal baseline, and the job-level permissions correctly grantpull-requests: writefor the linter to post PR comments.Also applies to: 73-75
83-83: LGTM! Security improvement.Setting
persist-credentials: falseprevents credential leakage by ensuring the token doesn't persist beyond the checkout step.
62-64: Verify concurrency grouping behavior for your use case.In reusable workflows,
github.workflowrefers to the caller workflow name, not the reusable workflow itself. This concurrency pattern is applied consistently across all workflows in this repository. Depending on how callers invoke this workflow (e.g., in a matrix with multiple parallel jobs), concurrent invocations may unintentionally share the same concurrency group and cancel each other.Confirm this behavior aligns with your intended usage in downstream workflows.
.github/workflows/reusable-change-detection.yml (2)
32-37: LGTM! Appropriate security hardening.The concurrency controls and minimal
contents: readpermission are correctly configured for this change-detection workflow.
52-53: LGTM! Security improvement.Disabling credential persistence is a security best practice.
.github/workflows/reusable-python-packaging-sdist.yml (1)
14-19: LGTM! Consistent security hardening.The concurrency controls, minimal permissions, and disabled credential persistence align with security best practices for this packaging workflow.
Also applies to: 31-31
.github/workflows/reusable-python-packaging-wheel-cibuildwheel.yml (1)
35-40: LGTM! Consistent security hardening.The added concurrency controls, permissions restrictions, and credential handling are appropriate for this wheel-building workflow.
Also applies to: 51-51
.github/workflows/reusable-cpp-tests-windows.yml (1)
55-60: LGTM! Consistent security hardening.The concurrency controls, minimal permissions, and disabled credential persistence are appropriate security improvements for this Windows testing workflow.
Also applies to: 79-79
.github/workflows/reusable-python-packaging-wheel-build.yml (3)
14-16: LGTM! Concurrency control improves CI efficiency.The concurrency configuration correctly groups workflow runs by workflow name and branch (for PRs) or run ID (for other events), with automatic cancellation of superseded runs. This prevents resource waste when multiple commits are pushed in quick succession.
18-19: LGTM! Explicit read-only permissions follow least privilege.Setting
contents: readat the workflow level is a security best practice, ensuring the workflow can only read repository contents without write access unless explicitly granted at the job level.
31-31: LGTM! Disabling credential persistence enhances security.Setting
persist-credentials: falseprevents the GitHub token from being persisted in the local git configuration, reducing the risk of credential leakage to subsequent steps or actions..github/workflows/reusable-mqt-core-update.yml (4)
26-28: LGTM! Concurrency control prevents duplicate update workflows.The concurrency configuration appropriately prevents multiple concurrent executions of the MQT Core update workflow for the same branch or workflow run.
30-31: LGTM! Workflow-level read-only permissions establish secure baseline.Setting
contents: readat the workflow level establishes a secure default that is appropriately overridden at the job level where write permissions are required.
38-39: LGTM! Job-level permissions are well-documented.The inline comments clearly document why write permissions are needed (pushing changes and creating PRs). The job correctly overrides the workflow-level read-only permissions with the elevated access required for its operations.
51-51: LGTM! Credential isolation is correctly implemented.Setting
persist-credentials: falseis appropriate here since the workflow creates a dedicated GitHub App token (lines 42-46) for authenticated git operations. This prevents potential conflicts or leakage between the checkout credentials and the App token..github/workflows/reusable-cpp-coverage.yml (3)
34-36: LGTM! Concurrency control optimizes coverage workflow runs.The concurrency configuration prevents redundant coverage runs when new commits are pushed, saving CI resources while ensuring the latest code is always tested.
38-39: LGTM! Read-only workflow permissions are appropriate.The workflow-level
contents: readpermission is sufficient for this coverage workflow, with job-level permissions appropriately granting additionalid-token: writeaccess for Codecov's OIDC authentication.
57-57: LGTM! Credential persistence is correctly disabled.Setting
persist-credentials: falseis appropriate since this workflow only needs to read the repository for building and testing, with no git operations required after checkout..github/workflows/reusable-python-tests.yml (3)
34-36: LGTM! Concurrency control prevents redundant test runs.The concurrency configuration efficiently manages test workflow executions by canceling outdated runs when new commits are pushed, reducing CI queue times and resource usage.
38-39: LGTM! Minimal permissions align with workflow requirements.The read-only
contentspermission is appropriate for this test workflow, which only needs to read the repository code without requiring write access.
75-75: LGTM! Disabled credential persistence is safe for test workflows.Setting
persist-credentials: falseis appropriate since this workflow only executes tests and uploads artifacts, with no git operations required after the initial checkout.
Signed-off-by: burgholzer <burgholzer@me.com>
1 participant
Description
This PR adds
zizmorto the list of pre-commit checks and fixes all reported warnings and issues.Checklist: