add: statically analyze CI workflows #51
Conversation
WalkthroughAdds a new workflow to run zizmor against repository workflow YAMLs and makes consistent workflow hygiene changes: non-persistent checkout, top-level/per-job permissions, env-var indirection for run inputs, publishing gating logic changes, and adds "zizmor" to the spellchecker words. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor GitHub as GitHub Events
participant WF as Workflow: check-ci-workflows
participant Runner as Ubuntu Runner
participant Z as zizmor
GitHub->>WF: push/pr to main or workflow_call
WF->>Runner: start job (permissions: contents: read)
Runner->>Runner: actions/checkout (persist-credentials: false)
Runner->>Runner: actions/setup-python (3.x)
Runner->>Z: pipx run zizmor --format=github .github/workflows/*.yml (GH_TOKEN)
Z-->>Runner: lint results
Runner-->>WF: job success/failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/pre-commit.yml (1)
66-76: Fix template injection when echoing PR titles.Echoing
${{ github.event.pull_request.title }} allows command substitution like $ (...) to execute. Use an env var and printf to avoid shell re-parsing.Apply:
- name: conventional-commit - run: >- - echo "${{ github.event.pull_request.title }}" - | committed --config ${{ github.workspace }}/org-repo/.github/committed.toml --commit-file - + env: + PR_TITLE: ${{ github.event.pull_request.title }} + run: | + printf '%s\n' "$PR_TITLE" \ + | committed --config "${{ github.workspace }}/org-repo/.github/committed.toml" --commit-file - - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 with: node-version: latest - name: spell check working-directory: project-repo - run: >- - echo "${{ github.event.pull_request.title }}" - | npx cspell-cli lint stdin + env: + PR_TITLE: ${{ github.event.pull_request.title }} + run: | + printf '%s\n' "$PR_TITLE" | npx cspell-cli lint stdin
🧹 Nitpick comments (9)
.github/workflows/pre-commit.yml (1)
29-33: Cache path may not expand due to single quotes.'~/.cache/pre-commit' is single-quoted; ~ won’t expand. Use $HOME or remove quotes.
Example:
- path: '~/.cache/pre-commit' + path: $HOME/.cache/pre-commit.github/workflows/main.yml (1)
9-10: Ensure jobs have needed permissions under permissions: {}.permissions: {} disables defaults. Verify:
- pre-commit job can still checkout (public repos OK, but consider setting contents: read per-job).
- stale job likely needs issues/pull-requests: write. Set per-job or ensure stale.yml declares them.
Example per-job override:
jobs: main: uses: ./.github/workflows/pre-commit.yml + permissions: + contents: read stale: uses: ./.github/workflows/stale.yml + permissions: + issues: write + pull-requests: write.github/workflows/py-publish.yml (1)
41-41: Consider narrowing TestPyPI publish triggers.Current condition publishes on any non-tag ref in cpp-linter repos (including feature branches). This can spam TestPyPI and burn tokens.
Example:
- if: startsWith(github.repository, 'cpp-linter') && !startsWith(github.ref, 'refs/tags/') + if: startsWith(github.repository, 'cpp-linter') + && github.event_name == 'push' + && github.ref == 'refs/heads/main'.github/workflows/ci-check.yml (6)
1-7: Add concurrency to auto-cancel superseded runs on the same refPrevents duplicate zizmor runs on force-pushes.
on: workflow_call: push: branches: [main] pull_request: branches: [main] +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true
12-15: Consider adding actions: read to job permissionsSome tooling needs
actions: readfor metadata lookups; harmless otherwise.permissions: contents: read + actions: read
12-13: Set a job timeoutGuards against a hung run.
check-ci-workflows: - runs-on: ubuntu-latest + runs-on: ubuntu-latest + timeout-minutes: 10
16-20: Checkout pin + no credential persistence = good; drop redundant repository override
repositorydefaults to the caller; not needed here and can confuse future readers.- uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: persist-credentials: false - repository: ${{ github.repository }}
20-22: Pin a concrete Python version for determinismPrevents environment drift.
- - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 #v5 + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 with: - python-version: '3.x' + python-version: '3.12'
23-26: Make target selection robust; avoid brittle globsLet zizmor scan the directory to catch both .yml and .yaml and avoid nullglob pitfalls.
- name: Run zizmor env: GH_TOKEN: ${{ github.token }} - run: pipx run zizmor .github/workflows/*.yml + run: pipx run zizmor .github/workflows
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
-
.github/workflows/ci-check.yml(1 hunks) -
.github/workflows/codeql.yml(1 hunks) -
.github/workflows/main.yml(1 hunks) -
.github/workflows/mkdocs.yml(1 hunks) -
.github/workflows/pre-commit.yml(1 hunks) -
.github/workflows/py-coverage.yml(1 hunks) -
.github/workflows/py-publish.yml(2 hunks) -
.github/workflows/snyk-container.yml(1 hunks) -
.github/workflows/sphinx.yml(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: .github/workflows/ci-check.yml
.github/workflows/pre-commit.yml
[error] 67-67: Template injection: code injection via template expansion. The line 'echo "${{ github.event.pull_request.title }}"' may expand into attacker-controllable code.
[error] 75-75: Template injection: code injection via template expansion. The line 'echo "${{ github.event.pull_request.title }}"' may expand into attacker-controllable code.
🔇 Additional comments (8)
.github/workflows/codeql.yml (1)
47-48: LGTM: safer checkout configuration.Disabling credential persistence is appropriate here; CodeQL doesn’t push.
.github/workflows/pre-commit.yml (1)
25-27: LGTM: using env var for commands reduces template-expansion risk..github/workflows/py-coverage.yml (1)
12-13: LGTM: credential persistence disabled..github/workflows/snyk-container.yml (1)
11-12: LGTM: credential persistence disabled; no pushes here..github/workflows/sphinx.yml (2)
17-18: LGTM: safer checkout; gh-pages uses its own token handling.
25-27: LGTM: passing input via env avoids template-expansion gotchas and keeps the command simple..github/workflows/py-publish.yml (2)
20-20: LGTM: disabling persisted credentials with fetch-depth: 0 is correct for setuptools_scm.
48-49: LGTM: PyPI publish gated to tags under org only.Also ensures secrets aren’t exposed to forks.
Confirm that TEST_PYPI_TOKEN and PYPI_API_TOKEN are only defined in org repos, not forks.
2bndy5
force-pushed
the
zizmor
branch
2 times, most recently
from
f7824fe to
f951911
Compare
resolves #46 - includes changes to satisfy warnings/errors raised by zizmor
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/snyk-container.yml (1)
7-16: Ensure SARIF upload has required permissionThe Code Scanning upload step needs security-events: write. Set it at job level so callers don’t have to remember to grant it.
jobs: snyk: runs-on: ubuntu-latest + permissions: + security-events: write steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 #v5 with: persist-credentials: false
🧹 Nitpick comments (6)
.github/workflows/codeql.yml (1)
45-49: Optional: fetch full history to improve CodeQL accuracyShallow clones can occasionally affect source tracing. Consider enabling full history.
- name: Checkout repository uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 #v5 with: persist-credentials: false + fetch-depth: 0.github/workflows/sphinx.yml (1)
23-25: Quote the output path to handle spacesMinor robustness tweak when paths contain spaces.
- run: uv run sphinx-build docs ${INPUTS_PATH_TO_DOC} + run: uv run sphinx-build docs "${INPUTS_PATH_TO_DOC}" env: INPUTS_PATH_TO_DOC: ${{ inputs.path-to-doc }}.github/workflows/ci-check.yml (1)
23-27: *Also lint .yaml filesCatches repos that mix .yml and .yaml.
- run: pipx run zizmor --format=github .github/workflows/*.yml + run: pipx run zizmor --format=github .github/workflows/*.{yml,yaml}.github/workflows/main.yml (1)
13-14: Confirm pre-commit’s needs; bump perms only if it auto-fixes.If the reusable pre-commit workflow ever pushes fixes/comments, it’ll need pull-requests: write (and possibly contents: write). If it’s read-only linting, contents: read is perfect.
.github/workflows/py-publish.yml (2)
41-41: Guard TestPyPI uploads to avoid publishing from PRs/forks (optional).Current condition will upload on any non-tag ref, including PR runs if the caller inherits secrets. If that’s not desired, restrict to pushes to default branch or require an explicit input.
Apply one of these minimal changes:
- if: startsWith(github.repository, 'cpp-linter') && !startsWith(github.ref, 'refs/tags/') + # Option A: only pushes to default branch + if: startsWith(github.repository, 'cpp-linter') && github.event_name == 'push' && github.ref == format('refs/heads/{0}', github.event.repository.default_branch)or
- if: startsWith(github.repository, 'cpp-linter') && !startsWith(github.ref, 'refs/tags/') + # Option B: require caller to opt-in + if: startsWith(github.repository, 'cpp-linter') && !startsWith(github.ref, 'refs/tags/') && inputs.publish_testpypi == true
48-48: Constrain PyPI publishes to version tags and ensure secrets inheritance.To reduce accidental releases from non-version tags, match a tag prefix; also confirm the caller uses secrets: inherit when invoking this reusable workflow.
- if: startsWith(github.repository, 'cpp-linter') && startsWith(github.ref, 'refs/tags/') + if: startsWith(github.repository, 'cpp-linter') && startsWith(github.ref, 'refs/tags/v')Example (in the caller):
jobs: release: uses: cpp-linter/.github/.github/workflows/py-publish.yml@main permissions: contents: read id-token: write attestations: write secrets: inherit
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/ci-check.yml(1 hunks).github/workflows/codeql.yml(1 hunks).github/workflows/main.yml(1 hunks).github/workflows/pre-commit.yml(2 hunks).github/workflows/py-coverage.yml(1 hunks).github/workflows/py-publish.yml(2 hunks).github/workflows/snyk-container.yml(1 hunks).github/workflows/sphinx.yml(1 hunks).github/workflows/stale.yml(1 hunks)cspell.config.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: 2bndy5
PR: cpp-linter/.github#51
File: .github/workflows/ci-check.yml:8-8
Timestamp: 2025-09-07T15:00:41.845Z
Learning: For zizmor GitHub Actions workflow integration: `actions: read` and `contents: read` permissions are only required for private repositories. For public repositories, zizmor only uses the GitHub token for API rate limiting and doesn't need special permissions.
📚 Learning: 2025-09-07T15:00:41.845Z
Learnt from: 2bndy5
PR: cpp-linter/.github#51
File: .github/workflows/ci-check.yml:8-8
Timestamp: 2025-09-07T15:00:41.845Z
Learning: For zizmor GitHub Actions workflow integration: `actions: read` and `contents: read` permissions are only required for private repositories. For public repositories, zizmor only uses the GitHub token for API rate limiting and doesn't need special permissions.
Applied to files:
.github/workflows/ci-check.yml.github/workflows/main.yml
🔇 Additional comments (13)
cspell.config.yml (1)
20-20: Add “zizmor” to dictionary — LGTMPrevents false positives from the spell checker.
.github/workflows/codeql.yml (1)
47-48: Disable persisted creds on checkout — LGTMGood hardening; keeps GITHUB_TOKEN out of the local git config.
.github/workflows/py-coverage.yml (1)
12-13: Disable persisted creds on checkout — LGTMMatches your credential-hardening pattern across workflows.
.github/workflows/snyk-container.yml (1)
11-12: Disable persisted creds on checkout — LGTMConsistent with other workflows.
.github/workflows/stale.yml (2)
7-10: Explicit minimal permissions — LGTMCorrect scopes for stale bot operations.
15-21: Message/labels formatting — LGTMFolded block and comma-separated labels look correct.
.github/workflows/pre-commit.yml (3)
25-27: Use env var for commands — LGTMCleaner than inline expressions; guarded by the if: condition.
66-70: PR title via env — LGTMReduces expression noise and improves readability.
76-79: Reuse PR title in spell check — LGTMConsistent with the previous step.
.github/workflows/ci-check.yml (1)
13-15: Permissions for public repos — LGTMcontents: read is fine here; zizmor doesn’t require actions: read on public repos. Noted in your docs reference.
.github/workflows/main.yml (2)
9-10: Good hardening with top-level minimal permissions.permissions: {} enforces least privilege by default; per-job overrides are then explicit. This aligns with the zizmor note (no extra perms needed for public repos).
17-20: Stale job permissions look right-sized.issues: write and pull-requests: write are required for labeling/closing; contents: read is sufficient otherwise.
.github/workflows/py-publish.yml (1)
20-20: Good credential hygiene on checkout.persist-credentials: false prevents leaking the default GITHUB_TOKEN to later git remotes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/py-coverage.yml (1)
40-40: Codecov step: consider minimal permissions and an optional guard.
If you want Codecov to create check runs, request checks: write (caller can still restrict):
YAML to add near the top of this workflow or job:
permissions:
contents: read
actions: read
checks: writeTo avoid failures when CODECOV_TOKEN is absent (e.g., forks), you can guard the step:
- if: env.CODECOV_TOKEN != ''
.github/workflows/py-publish.yml (3)
16-21: Checkout hardening LGTM; update the comment for accuracy.
The comment says “use fetch --all,” but you’re using fetch-depth: 0 (which is correct for setuptools_scm). Suggest correcting the comment.- # use fetch --all for setuptools_scm to work + # use full history (fetch-depth: 0) so setuptools_scm can find tags
26-34: Consider building both sdist and wheel.
Publishing only wheels can limit consumers. Using the build module is simpler and produces both:- - name: Install dependencies - run: python -m pip install --upgrade pip twine + - name: Install build and twine + run: python -m pip install --upgrade pip build twine - - name: Build wheel - run: python -m pip wheel -w dist --no-deps . + - name: Build distributions (sdist + wheel) + run: python -m build - - name: Check distribution - run: twine check dist/* + - name: Check distributions + run: twine check dist/*
40-52: Optional: move to trusted publishing to drop PyPI secrets.
You already grant id-token: write; switching to pypa/gh-action-pypi-publish removes token management and is recommended by PyPI.- name: Publish package (to TestPyPI) - if: startsWith(github.repository, 'cpp-linter') && !startsWith(github.ref, 'refs/tags/') - env: - TWINE_USERNAME: __token__ - TWINE_PASSWORD: ${{ secrets.TEST_PYPI_TOKEN }} - run: twine upload --repository testpypi dist/* + if: startsWith(github.repository, 'cpp-linter') && !startsWith(github.ref, 'refs/tags/') + uses: pypa/gh-action-pypi-publish@v1.13.0 + with: + repository-url: https://test.pypi.org/legacy/ - name: Publish package (to PyPI) - if: startsWith(github.repository, 'cpp-linter') && startsWith(github.ref, 'refs/tags/') - env: - TWINE_USERNAME: __token__ - TWINE_PASSWORD: ${{ secrets.PYPI_API_TOKEN }} - run: twine upload dist/* + if: startsWith(github.repository, 'cpp-linter') && startsWith(github.ref, 'refs/tags/') + uses: pypa/gh-action-pypi-publish@v1.13.0Note: Requires configuring Trusted Publishing in PyPI for this project.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci-check.yml(1 hunks).github/workflows/py-coverage.yml(2 hunks).github/workflows/py-publish.yml(2 hunks).github/workflows/snyk-container.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci-check.yml
- .github/workflows/snyk-container.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: 2bndy5
PR: cpp-linter/.github#51
File: .github/workflows/ci-check.yml:8-8
Timestamp: 2025-09-07T15:00:41.845Z
Learning: For zizmor GitHub Actions workflow integration: `actions: read` and `contents: read` permissions are only required for private repositories. For public repositories, zizmor only uses the GitHub token for API rate limiting and doesn't need special permissions.
🔇 Additional comments (8)
.github/workflows/py-coverage.yml (4)
11-13: Good hardening: disable persisted Git credentials on checkout.
Prevents accidental pushes and limits token exposure.
16-16: Pinned download-artifact to a specific commit.
Supply-chain friendly; version bump looks good.
22-22: Upgrade/setup-python pin looks good.
v6 pin by SHA is appropriate.
33-33: upload-artifact pin LGTM.
Nothing else to change here..github/workflows/py-publish.yml (4)
22-22: setup-python v6 pin LGTM.
Good supply-chain practice.
36-36: Attestations action pin looks correct.
Matches required permissions (id-token: write, attestations: write).
41-41: Gating logic: verify behavior under workflow_call.
Under reusable workflows, github.ref inherits the caller’s ref. Confirm this correctly targets non-tag runs for TestPyPI in all callers.
48-48: Tag-based publish gating looks right; verify callers pass tag refs.
Ensure callers invoke this reusable workflow on tag events so github.ref starts with refs/tags/.
resolves #46
Summary by CodeRabbit
New Features
Chores