chore: update workflows to be idiomatic, DRY, and efficient - #78
Conversation
…A pins, use matrix for Molecule tests to eliminate duplication (DRY), consistent org pins, timeouts, etc.
…A pins, timeouts, consistent with org standards
📝 WalkthroughWalkthroughThe workflows consolidate Molecule scenarios into a matrix job, pin GitHub Actions to commit SHAs, rewire integration testing, and scope Dependabot permissions to the merge job. ChangesCI Workflows
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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
🤖 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/ci.yml:
- Around line 55-65: The CI matrix currently lists duplicate scenario: default
and uses misleading name labels, and the checkout steps miss
persist-credentials: false; update the matrix include entries to match the
actual Molecule scenario keys (use scenario values that correspond to
molecule/default/molecule.yml which defines ubuntu-22, ubuntu-24, rocky-9 and
molecule/ubuntu/molecule.yml which defines ubuntu-22.04 and ubuntu-24.04) so
each matrix row maps to a unique scenario and adjust the matrix "name" strings
to accurately reflect the OS/scenario being run; also add persist-credentials:
false to every actions/checkout step invocation to avoid leaking GITHUB_TOKEN.
In @.github/workflows/dependabot-auto-merge.yml:
- Around line 15-16: Replace the fragile actor-based gate by checking the PR
author and same-repo origin: update the workflow's if condition (currently using
github.actor) to require github.event.pull_request.user.login ==
'dependabot[bot]' and that the PR head repo is the same repository (e.g.,
github.event.pull_request.head.repo.full_name == github.repository) so
auto-merge only runs for Dependabot-created PRs originating from the same repo.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 0173a465-3de9-495a-965a-451f069ace5a
📒 Files selected for processing (2)
.github/workflows/ci.yml.github/workflows/dependabot-auto-merge.yml
| timeout-minutes: 5 | ||
| if: ${{ github.actor == 'dependabot[bot]' }} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What do GitHub Actions docs and GitHub Security Lab recommend for securely identifying Dependabot PRs in workflow if conditions (actor vs pull_request.user.login, and same-repo checks)?
💡 Result:
To securely identify Dependabot-created pull requests in GitHub Actions workflow if conditions, authoritative sources and security best practices recommend avoiding github.actor and instead relying on the immutable author of the pull request, github.event.pull_request.user.login [1][2]. Using github.actor is considered insecure and susceptible to "confused deputy" attacks [1][2]. An attacker can trigger Dependabot-related commands (such as @dependabot recreate) on their own malicious pull request, which can cause the github.actor context to appear as dependabot[bot] even though the pull request was not authored by the bot [3]. For robust security, implement the following checks: 1. Use github.event.pull_request.user.login == 'dependabot[bot]' to verify that the pull request was genuinely created by Dependabot [1][2]. This value is immutable and cannot be spoofed by attackers [1]. 2. Add a repository check to ensure the workflow is only running on the expected repository, such as github.repository == 'owner/my_repo', to prevent unauthorized usage in forks or other contexts [4]. 3. If necessary, combine these with checks on the repository's origin (e.g., !github.event.pull_request.head.repo.fork) to ensure you are not acting on potentially compromised fork-based code [2]. For complex automation tasks, such as auto-merging, official guidance suggests using specialized tools like dependabot/fetch-metadata to securely retrieve and verify pull request metadata before taking action [2][4]. Finally, be aware that when Dependabot triggers a workflow, GitHub inherently applies security restrictions—such as providing a read-only GITHUB_TOKEN and withholding access to standard repository secrets—to prevent privilege escalation [5][6][7]. These restrictions apply even if a different user subsequently re-runs the workflow [6][8].
Citations:
- 1: https://sisaku-security.github.io/lint/docs/rules/botconditions/
- 2: https://boostsecurityio.github.io/poutine/rules/confused_deputy_auto_merge/
- 3: security: guard bot filtering against dependabot confused deputy attacks github/gh-aw#29432
- 4: https://docs.github.com/en/code-security/tutorials/secure-your-dependencies/automating-dependabot-with-github-actions
- 5: https://docs.github.com/en/code-security/reference/supply-chain-security/troubleshoot-dependabot/troubleshooting-dependabot-on-github-actions
- 6: https://github.com/github/docs/blob/main/content/code-security/reference/supply-chain-security/dependabot-on-actions.md
- 7: https://github.blog/changelog/2021-12-09-github-actions-changes-to-permissions-in-workflows-triggered-by-dependabot/
- 8: https://docs.github.com/en/code-security/reference/supply-chain-security/dependabot-on-actions
Harden the Dependabot auto-merge gate beyond github.actor
github.actor is not a reliable indicator of Dependabot identity; gate on the PR creator instead and keep a same-repo origin check.
Suggested patch
- if: ${{ github.actor == 'dependabot[bot]' }}
+ if: ${{ github.event_name == 'pull_request'
+ && github.event.pull_request.user.login == 'dependabot[bot]'
+ && github.event.pull_request.head.repo.full_name == github.repository }}📝 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.
| timeout-minutes: 5 | |
| if: ${{ github.actor == 'dependabot[bot]' }} | |
| timeout-minutes: 5 | |
| if: ${{ github.event_name == 'pull_request' | |
| && github.event.pull_request.user.login == 'dependabot[bot]' | |
| && github.event.pull_request.head.repo.full_name == github.repository }} |
🧰 Tools
🪛 zizmor (1.25.2)
[error] 16-16: spoofable bot actor check (bot-conditions): actor context may be spoofable
(bot-conditions)
🤖 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/dependabot-auto-merge.yml around lines 15 - 16, Replace
the fragile actor-based gate by checking the PR author and same-repo origin:
update the workflow's if condition (currently using github.actor) to require
github.event.pull_request.user.login == 'dependabot[bot]' and that the PR head
repo is the same repository (e.g., github.event.pull_request.head.repo.full_name
== github.repository) so auto-merge only runs for Dependabot-created PRs
originating from the same repo.
There was a problem hiding this comment.
Pull request overview
This PR aims to bring the repository’s GitHub Actions workflows in line with org CI standards by improving security (SHA-pinning, least-privilege permissions), efficiency (concurrency/timeouts), and reducing duplication (matrix-based Molecule runs).
Changes:
- Refactors CI Molecule testing jobs into a single matrix job and updates downstream dependencies.
- Switches several actions in
ci.ymlfrom tag-based references to full commit SHA pins. - Adds a job timeout to the Dependabot auto-merge workflow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.github/workflows/ci.yml |
Adds least-privilege permissions + SHA pins; consolidates Molecule jobs into a matrix; adjusts integration/release steps accordingly. |
.github/workflows/dependabot-auto-merge.yml |
Adds a short job timeout and keeps Dependabot metadata/merge flow. |
.github/workflows/release.yml |
Mentioned in PR description as part of the org-standard workflow updates, but currently still appears to use tag pins. |
.github/workflows/security-tests.yml |
Mentioned in PR description as part of the org-standard workflow updates, but currently still appears to use tag pins. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (2)
.github/workflows/ci.yml:99
- This workflow pins
docker/setup-buildx-actionto a commit annotated as# v3, while.github/workflows/security-tests.ymlpins the same action as# v4. Given the PR goal of consistent pins/versions across workflows, these should be aligned to the same major/version to avoid divergent behavior.
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@0d103c3126aa41d772a8362f6aa67afac040f80c # v3
.github/workflows/ci.yml:156
softprops/action-gh-releaseis pinned here as# v2, but.github/workflows/release.ymluses the same action pinned as# v3. Mixing major versions within the same repo increases maintenance risk (different inputs/behavior). Consider aligning to the same pinned version used inrelease.yml.
- name: Create Release
if: steps.check_tag.outputs.exists == 'false'
uses: softprops/action-gh-release@de2c0eb89ae2a093354c5cb36e48c5e2b1c5d3e0 # v2
| states=$(gh pr checks "$pr" \ | ||
| --repo "$GITHUB_REPOSITORY" \ | ||
| --json state \ | ||
| --jq '.[].state' 2>/dev/null || true) | ||
|
|
||
| # Fail closed when a repository has no CI checks. | ||
| [ -n "$states" ] || continue | ||
|
|
||
| blocked='^(PENDING|QUEUED|IN_PROGRESS|FAILURE|ERROR|CANCELLED|' | ||
| blocked+='STALE|ACTION_REQUIRED)$' | ||
| if grep -Eq "$blocked" <<<"$states"; then | ||
| continue | ||
| fi |
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
| checks: read |
| concurrency: | ||
| group: dependabot-auto-merge | ||
| cancel-in-progress: false |
| - name: Checkout code | ||
| uses: actions/checkout@v6 | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v6 | ||
| uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 |
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v6 | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v6 | ||
| uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 |
| - name: Checkout repository | ||
| uses: actions/checkout@v6 | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
|
|
||
| - name: Set up Python ${{ matrix.python-version }} | ||
| uses: actions/setup-python@v6 | ||
| uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 | ||
| with: |
|
@copilot resolve the merge conflicts on this branch. |
|
@copilot resolve the merge conflicts on this branch. |
Resolved. I merged |
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/dependabot-auto-merge.yml (1)
21-23: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winHarden the Dependabot auto-merge gate beyond
github.actor.
github.actoris not a reliable indicator of Dependabot identity and can be spoofed in some contexts (e.g., via a confused deputy attack using@dependabotcommands). Gate on the immutable PR creator instead, and keep a same-repo origin check when dealing withpull_request_target.🛡️ Proposed fix
if: >- github.event_name != 'pull_request_target' || - github.actor == 'dependabot[bot]' + (github.event.pull_request.user.login == 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository)🤖 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/dependabot-auto-merge.yml around lines 21 - 23, Update the workflow’s auto-merge condition to identify Dependabot using the immutable pull request creator rather than github.actor, and require the pull request head repository to match the base repository for pull_request_target events. Preserve the existing behavior for non-pull_request_target events and reference the workflow’s existing pull request event context.Source: Linters/SAST tools
🧹 Nitpick comments (2)
.github/workflows/security-tests.yml (1)
13-16: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScope elevated permissions to the job level.
Defining
pull-requests: writeat the workflow level grants this permission to all jobs in the workflow. It is a security best practice to enforce the principle of least privilege by setting onlycontents: readat the workflow level and explicitly granting elevated permissions only to the specific jobs that require them (in this case, the job that comments on the PR).♻️ Proposed refactor
permissions: contents: read - pull-requests: writeThen, add the required permissions to the
security-hardening-testsjob:security-hardening-tests: name: Security Hardening Tests runs-on: [self-hosted, linux, x64] timeout-minutes: 45 + permissions: + contents: read + pull-requests: write🤖 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/security-tests.yml around lines 13 - 16, Update the workflow-level permissions to retain only contents: read, then add pull-requests: write under the security-hardening-tests job that comments on pull requests. Keep elevated permissions scoped exclusively to that job.Source: Linters/SAST tools
.github/workflows/release.yml (1)
77-87: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider using the built-in
ghCLI instead of a third-party action.As highlighted by static analysis, the GitHub runner already includes the
ghCLI which can create releases natively. Usinggh release createeliminates the need for an external third-party action (softprops/action-gh-release), reducing the supply chain surface area.♻️ Proposed refactor
- - name: Create Release - uses: softprops/action-gh-release@c12583777ecdfd3be55c69cf75464299dc01057e # v3 - with: - tag_name: v${{ steps.version.outputs.version }} - name: Release v${{ steps.version.outputs.version }} - body: | - ## Release v${{ steps.version.outputs.version }} - - ### Changes - ${{ steps.changelog.outputs.changelog }} + - name: Create Release + env: + GH_TOKEN: ${{ github.token }} + run: | + gh release create "v${{ steps.version.outputs.version }}" \ + --title "Release v${{ steps.version.outputs.version }}" \ + --notes "## Release v${{ steps.version.outputs.version }} + + ### Changes + ${{ steps.changelog.outputs.changelog }}🤖 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/release.yml around lines 77 - 87, Replace the softprops/action-gh-release step with a shell step using the runner-provided gh CLI to create the release. Preserve the existing version-derived tag and release name, pass the changelog output as the release body, and ensure the workflow token is available for gh authentication.Source: Linters/SAST 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/security-tests.yml:
- Around line 31-33: Disable checkout credential persistence by adding
persist-credentials: false under the checkout steps for
security-hardening-tests, security-compliance-check, and
security-regression-test in .github/workflows/security-tests.yml at lines 31-33,
174-176, and 234-236 respectively.
---
Outside diff comments:
In @.github/workflows/dependabot-auto-merge.yml:
- Around line 21-23: Update the workflow’s auto-merge condition to identify
Dependabot using the immutable pull request creator rather than github.actor,
and require the pull request head repository to match the base repository for
pull_request_target events. Preserve the existing behavior for
non-pull_request_target events and reference the workflow’s existing pull
request event context.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 77-87: Replace the softprops/action-gh-release step with a shell
step using the runner-provided gh CLI to create the release. Preserve the
existing version-derived tag and release name, pass the changelog output as the
release body, and ensure the workflow token is available for gh authentication.
In @.github/workflows/security-tests.yml:
- Around line 13-16: Update the workflow-level permissions to retain only
contents: read, then add pull-requests: write under the security-hardening-tests
job that comments on pull requests. Keep elevated permissions scoped exclusively
to that job.
🪄 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: CHILL
Plan: Pro Plus
Run ID: ed6f6a36-fd41-4cf9-92eb-b156e86b5f0c
📒 Files selected for processing (4)
.github/workflows/ci.yml.github/workflows/dependabot-auto-merge.yml.github/workflows/release.yml.github/workflows/security-tests.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
| - name: Checkout repository | ||
| uses: actions/checkout@v7 | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Disable credential persistence in checkout across jobs.
By default, actions/checkout persists the GitHub token in the local .git/config. If malicious code runs during the tests, or if the workspace is packaged into an artifact, this token could be exposed. Explicitly set persist-credentials: false to prevent this across these jobs.
.github/workflows/security-tests.yml#L31-L33: Addwith: \n persist-credentials: falseto the checkout step insecurity-hardening-tests..github/workflows/security-tests.yml#L174-L176: Addwith: \n persist-credentials: falseto the checkout step insecurity-compliance-check..github/workflows/security-tests.yml#L234-L236: Addwith: \n persist-credentials: falseto the checkout step insecurity-regression-test.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 31-32: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
📍 Affects 1 file
.github/workflows/security-tests.yml#L31-L33(this comment).github/workflows/security-tests.yml#L174-L176.github/workflows/security-tests.yml#L234-L236
🤖 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/security-tests.yml around lines 31 - 33, Disable checkout
credential persistence by adding persist-credentials: false under the checkout
steps for security-hardening-tests, security-compliance-check, and
security-regression-test in .github/workflows/security-tests.yml at lines 31-33,
174-176, and 234-236 respectively.
Source: Linters/SAST tools
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
.github/workflows/ci.yml:103
- The PR description mentions consistent action pins across workflows, but
docker/setup-buildx-actionis pinned to v3 here whilesecurity-tests.ymlpins v4. Standardizing the pin helps reduce drift and simplifies maintenance.
uses: docker/setup-buildx-action@0d103c3126aa41d772a8362f6aa67afac040f80c # v3
.github/workflows/ci.yml:161
- The PR description mentions consistent action pins across workflows, but
softprops/action-gh-releaseis pinned to v2 here whilerelease.ymluses v3. Consider aligning these to the same pinned revision unless there is a concrete compatibility reason to stay on v2.
uses: softprops/action-gh-release@de2c0eb89ae2a093354c5cb36e48c5e2b1c5d3e0 # v2
.github/workflows/release.yml:26
- The PR description calls out a repo-wide pattern of top-level
permissions: contents: readwith job-level write overrides, but this workflow still setspermissions: contents: writeat the workflow level. That grants write permission to all jobs (includingtest) when only the release-creation job needs it.
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@v7 | ||
| - name: Set up Python ${{ matrix.python-version }} |
| - name: Checkout repository | ||
| uses: actions/checkout@v7 | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 |
Security Test Coverage Report 🔒Targets: Ubuntu and Rocky Linux
Full Coverage Report |
This PR updates the GitHub workflows in this repo to follow the org's standards for idiomatic, DRY, and efficient CI:
permissions: contents: read(least privilege) + job-level overrides.concurrencygroups withcancel-in-progress.timeout-minuteson jobs.Changes made to:
These changes were previously applied directly; this PR proposes them for review and merge via the proper process.
Summary by CodeRabbit